Skip to content

Commit 522fda9

Browse files
schauderschauder
committed
Fix OptimisticLockingException for MyBatis.
OptimisticLockingExceptions were not properly thrown when using MyBatis. Closes #2316
1 parent d1dd2d3 commit 522fda9

9 files changed

Lines changed: 294 additions & 4 deletions

File tree

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategy.java‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,18 @@
1818
import static java.lang.Boolean.TRUE;
1919

2020
import java.util.ArrayList;
21+
import java.util.Collections;
2122
import java.util.List;
2223
import java.util.Optional;
2324
import java.util.function.Consumer;
2425
import java.util.function.Function;
2526
import java.util.stream.Stream;
2627

2728
import org.jspecify.annotations.Nullable;
29+
import org.springframework.dao.OptimisticLockingFailureException;
2830
import org.springframework.data.domain.Pageable;
2931
import org.springframework.data.domain.Sort;
32+
import org.springframework.data.jdbc.core.convert.FunctionCollector.CombinedDataAccessException;
3033
import org.springframework.data.mapping.PersistentPropertyPath;
3134
import org.springframework.data.relational.core.conversion.IdValueSource;
3235
import org.springframework.data.relational.core.dialect.Dialect;
@@ -97,7 +100,8 @@ public <S> boolean update(S objectToSave, Class<S> domainType) {
97100

98101
@Override
99102
public <S> boolean updateWithVersion(S objectToSave, Class<S> domainType, Number previousVersion) {
100-
return collect(das -> das.updateWithVersion(objectToSave, domainType, previousVersion));
103+
return cascadePropagatingOptimisticLocking(
104+
das -> das.updateWithVersion(objectToSave, domainType, previousVersion));
101105
}
102106

103107
@Override
@@ -112,7 +116,10 @@ public void delete(Iterable<Object> ids, Class<?> domainType) {
112116

113117
@Override
114118
public <T> void deleteWithVersion(Object id, Class<T> domainType, Number previousVersion) {
115-
collectVoid(das -> das.deleteWithVersion(id, domainType, previousVersion));
119+
cascadePropagatingOptimisticLocking(das -> {
120+
das.deleteWithVersion(id, domainType, previousVersion);
121+
return TRUE;
122+
});
116123
}
117124

118125
@Override
@@ -241,4 +248,27 @@ private void collectVoid(Consumer<DataAccessStrategy> consumer) {
241248
return TRUE;
242249
});
243250
}
251+
252+
/**
253+
* Cascade through the configured strategies like {@link #collect(Function)} but propagate an
254+
* {@link OptimisticLockingFailureException} from the first strategy that raises it instead of treating it as a signal
255+
* to try the next strategy. An optimistic locking failure means the operation reached the database and the version
256+
* check failed; subsequent strategies would either repeat the failure or perform a duplicate, possibly successful,
257+
* write — both wrong.
258+
*/
259+
private <T> T cascadePropagatingOptimisticLocking(Function<DataAccessStrategy, T> function) {
260+
261+
List<Exception> exceptions = new ArrayList<>();
262+
for (DataAccessStrategy strategy : strategies) {
263+
try {
264+
return function.apply(strategy);
265+
} catch (OptimisticLockingFailureException ex) {
266+
throw ex;
267+
} catch (RuntimeException ex) {
268+
exceptions.add(ex);
269+
}
270+
}
271+
throw new CombinedDataAccessException("Failed to perform data access with all available strategies",
272+
Collections.unmodifiableList(exceptions));
273+
}
244274
}

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import org.springframework.data.mapping.PersistentPropertyPath;
4848
import org.springframework.data.relational.core.conversion.IdValueSource;
4949
import org.springframework.data.relational.core.dialect.Dialect;
50+
import org.springframework.data.relational.core.mapping.OptimisticLockingUtils;
5051
import org.springframework.data.relational.core.mapping.RelationalMappingContext;
5152
import org.springframework.data.relational.core.mapping.RelationalPersistentProperty;
5253
import org.springframework.data.relational.core.query.Query;
@@ -224,7 +225,10 @@ public <T> void deleteWithVersion(Object id, Class<T> domainType, Number previou
224225
String statement = namespace(domainType) + ".deleteWithVersion";
225226
MyBatisContext parameter = new MyBatisContext(id, null, domainType,
226227
Collections.singletonMap(VERSION_SQL_PARAMETER_NAME_OLD, previousVersion));
227-
sqlSession().delete(statement, parameter);
228+
int affectedRows = sqlSession().delete(statement, parameter);
229+
if (affectedRows == 0) {
230+
throw OptimisticLockingUtils.deleteFailed(id, previousVersion, domainType);
231+
}
228232
}
229233

230234
@Override

‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategyUnitTests.java‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.Collections;
2525

2626
import org.junit.jupiter.api.Test;
27+
import org.springframework.dao.OptimisticLockingFailureException;
2728
import org.springframework.data.jdbc.core.convert.FunctionCollector.CombinedDataAccessException;
2829
import org.springframework.data.mapping.PersistentPropertyPath;
2930
import org.springframework.data.relational.core.sql.SqlIdentifier;
@@ -85,4 +86,69 @@ public void findByPropertyReturnsFirstSuccess() {
8586
assertThat(findAll).containsExactly("success");
8687
}
8788

89+
@Test // GH-2316
90+
public void updateWithVersionPropagatesOptimisticLockingFailure() {
91+
92+
DataAccessStrategy throwsOlfe = mock(DataAccessStrategy.class);
93+
doThrow(new OptimisticLockingFailureException("version mismatch")) //
94+
.when(throwsOlfe).updateWithVersion("entity", String.class, 1L);
95+
CascadingDataAccessStrategy access = new CascadingDataAccessStrategy(asList(throwsOlfe, mayNotCall));
96+
97+
assertThatExceptionOfType(OptimisticLockingFailureException.class) //
98+
.isThrownBy(() -> access.updateWithVersion("entity", String.class, 1L)) //
99+
.withMessage("version mismatch");
100+
}
101+
102+
@Test // GH-2316
103+
public void updateWithVersionSkipsStrategyThatDoesNotSupportTheOperation() {
104+
105+
doReturn(true).when(succeeds).updateWithVersion("entity", String.class, 1L);
106+
CascadingDataAccessStrategy access = new CascadingDataAccessStrategy(asList(alwaysFails, succeeds, mayNotCall));
107+
108+
assertThat(access.updateWithVersion("entity", String.class, 1L)).isTrue();
109+
}
110+
111+
@Test // GH-2316
112+
public void updateWithVersionFailsIfAllStrategiesFail() {
113+
114+
CascadingDataAccessStrategy access = new CascadingDataAccessStrategy(asList(alwaysFails, alwaysFails));
115+
116+
assertThatExceptionOfType(CombinedDataAccessException.class) //
117+
.isThrownBy(() -> access.updateWithVersion("entity", String.class, 1L)) //
118+
.withMessageContaining("Failed to perform data access with all available strategies");
119+
}
120+
121+
@Test // GH-2316
122+
public void deleteWithVersionPropagatesOptimisticLockingFailure() {
123+
124+
DataAccessStrategy throwsOlfe = mock(DataAccessStrategy.class);
125+
doThrow(new OptimisticLockingFailureException("version mismatch")) //
126+
.when(throwsOlfe).deleteWithVersion(23L, String.class, 1L);
127+
CascadingDataAccessStrategy access = new CascadingDataAccessStrategy(asList(throwsOlfe, mayNotCall));
128+
129+
assertThatExceptionOfType(OptimisticLockingFailureException.class) //
130+
.isThrownBy(() -> access.deleteWithVersion(23L, String.class, 1L)) //
131+
.withMessage("version mismatch");
132+
}
133+
134+
@Test // GH-2316
135+
public void deleteWithVersionSkipsStrategyThatDoesNotSupportTheOperation() {
136+
137+
CascadingDataAccessStrategy access = new CascadingDataAccessStrategy(asList(alwaysFails, succeeds, mayNotCall));
138+
139+
access.deleteWithVersion(23L, String.class, 1L);
140+
141+
verify(succeeds).deleteWithVersion(23L, String.class, 1L);
142+
}
143+
144+
@Test // GH-2316
145+
public void deleteWithVersionFailsIfAllStrategiesFail() {
146+
147+
CascadingDataAccessStrategy access = new CascadingDataAccessStrategy(asList(alwaysFails, alwaysFails));
148+
149+
assertThatExceptionOfType(CombinedDataAccessException.class) //
150+
.isThrownBy(() -> access.deleteWithVersion(23L, String.class, 1L)) //
151+
.withMessageContaining("Failed to perform data access with all available strategies");
152+
}
153+
88154
}

‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisHsqlIntegrationTests.java‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919

2020
import junit.framework.AssertionFailedError;
2121

22+
import java.util.Collections;
23+
import java.util.Map;
24+
2225
import org.apache.ibatis.session.Configuration;
2326
import org.apache.ibatis.session.SqlSession;
2427
import org.apache.ibatis.session.SqlSessionFactory;
@@ -29,6 +32,8 @@
2932
import org.springframework.context.annotation.Bean;
3033
import org.springframework.context.annotation.Import;
3134
import org.springframework.context.annotation.Primary;
35+
import org.springframework.dao.OptimisticLockingFailureException;
36+
import org.springframework.data.jdbc.core.JdbcAggregateOperations;
3237
import org.springframework.data.jdbc.core.convert.DataAccessStrategy;
3338
import org.springframework.data.jdbc.core.convert.JdbcConverter;
3439
import org.springframework.data.jdbc.core.convert.QueryMappingConfiguration;
@@ -41,6 +46,7 @@
4146
import org.springframework.data.jdbc.testing.TestConfiguration;
4247
import org.springframework.data.relational.core.mapping.RelationalMappingContext;
4348
import org.springframework.data.repository.CrudRepository;
49+
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
4450
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
4551
import org.springframework.jdbc.datasource.embedded.EmbeddedDatabase;
4652

@@ -58,6 +64,8 @@ public class MyBatisHsqlIntegrationTests {
5864

5965
@Autowired SqlSessionFactory sqlSessionFactory;
6066
@Autowired DummyEntityRepository repository;
67+
@Autowired JdbcAggregateOperations template;
68+
@Autowired NamedParameterJdbcOperations jdbc;
6169

6270
@Test // DATAJDBC-123
6371
public void mybatisSelfTest() {
@@ -80,6 +88,81 @@ public void myBatisGetsUsedForInsertAndSelect() {
8088
assertThat(reloaded).isNotNull().extracting(e -> e.id, e -> e.name);
8189
}
8290

91+
@Test // GH-2316
92+
public void deleteOfVersionedAggregateWithCurrentVersionSucceeds() {
93+
94+
VersionedEntity saved = template.save(new VersionedEntity(null, null, "first"));
95+
96+
template.delete(saved);
97+
98+
Long remaining = jdbc.queryForObject("SELECT COUNT(*) FROM versioned_entity", Collections.emptyMap(),
99+
Long.class);
100+
assertThat(remaining).isZero();
101+
}
102+
103+
@Test // GH-2316
104+
public void deleteOfVersionedAggregateWithStaleVersionThrowsOptimisticLockingFailureException() {
105+
106+
VersionedEntity saved = template.save(new VersionedEntity(null, null, "first"));
107+
108+
// simulate a concurrent update bumping the version in the database
109+
jdbc.update("UPDATE versioned_entity SET version = version + 1 WHERE id = :id",
110+
Map.of("id", saved.id));
111+
112+
assertThatThrownBy(() -> template.delete(saved))
113+
.isInstanceOf(OptimisticLockingFailureException.class);
114+
}
115+
116+
@Test // GH-2316
117+
public void deleteOfConcurrentlyDeletedVersionedAggregateThrowsOptimisticLockingFailureException() {
118+
119+
VersionedEntity saved = template.save(new VersionedEntity(null, null, "first"));
120+
121+
// simulate a concurrent deletion
122+
jdbc.update("DELETE FROM versioned_entity WHERE id = :id", Map.of("id", saved.id));
123+
124+
assertThatThrownBy(() -> template.delete(saved))
125+
.isInstanceOf(OptimisticLockingFailureException.class);
126+
}
127+
128+
@Test // GH-2316
129+
public void updateOfVersionedAggregateWithCurrentVersionSucceeds() {
130+
131+
VersionedEntity saved = template.save(new VersionedEntity(null, null, "first"));
132+
133+
VersionedEntity updated = template.save(new VersionedEntity(saved.id, saved.version, "second"));
134+
135+
assertThat(updated.version).isEqualTo(saved.version + 1);
136+
String name = jdbc.queryForObject("SELECT name FROM versioned_entity WHERE id = :id",
137+
Map.of("id", saved.id), String.class);
138+
assertThat(name).isEqualTo("second");
139+
}
140+
141+
@Test // GH-2316
142+
public void updateOfVersionedAggregateWithStaleVersionThrowsOptimisticLockingFailureException() {
143+
144+
VersionedEntity saved = template.save(new VersionedEntity(null, null, "first"));
145+
146+
// simulate a concurrent update bumping the version in the database
147+
jdbc.update("UPDATE versioned_entity SET version = version + 1 WHERE id = :id",
148+
Map.of("id", saved.id));
149+
150+
assertThatThrownBy(() -> template.save(new VersionedEntity(saved.id, saved.version, "second")))
151+
.isInstanceOf(OptimisticLockingFailureException.class);
152+
}
153+
154+
@Test // GH-2316
155+
public void updateOfConcurrentlyDeletedVersionedAggregateThrowsOptimisticLockingFailureException() {
156+
157+
VersionedEntity saved = template.save(new VersionedEntity(null, null, "first"));
158+
159+
// simulate a concurrent deletion
160+
jdbc.update("DELETE FROM versioned_entity WHERE id = :id", Map.of("id", saved.id));
161+
162+
assertThatThrownBy(() -> template.save(new VersionedEntity(saved.id, saved.version, "second")))
163+
.isInstanceOf(OptimisticLockingFailureException.class);
164+
}
165+
83166
interface DummyEntityRepository extends CrudRepository<DummyEntity, Long> {
84167

85168
}
@@ -103,6 +186,9 @@ SqlSessionFactoryBean createSessionFactory(EmbeddedDatabase db) {
103186
configuration.getTypeAliasRegistry().registerAlias(DummyEntity.class);
104187
configuration.addMapper(DummyEntityMapper.class);
105188

189+
configuration.getTypeAliasRegistry().registerAlias(VersionedEntity.class);
190+
configuration.addMapper(VersionedEntityMapper.class);
191+
106192
SqlSessionFactoryBean sqlSessionFactoryBean = new SqlSessionFactoryBean();
107193
sqlSessionFactoryBean.setDataSource(db);
108194
sqlSessionFactoryBean.setConfiguration(configuration);

‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/mybatis/VersionedEntity.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* Copyright 2026-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.jdbc.mybatis;
17+
18+
import org.apache.ibatis.type.Alias;
19+
import org.springframework.data.annotation.Id;
20+
import org.springframework.data.annotation.Version;
21+
22+
/**
23+
* Entity with an optimistic-locking version, used to exercise
24+
* {@link MyBatisDataAccessStrategy#deleteWithVersion(Object, Class, Number)}.
25+
*
26+
* @author Jens Schauder
27+
*/
28+
@Alias("VersionedEntity")
29+
class VersionedEntity {
30+
31+
@Id final Long id;
32+
@Version final Long version;
33+
final String name;
34+
35+
public VersionedEntity(Long id, Long version, String name) {
36+
37+
this.id = id;
38+
this.version = version;
39+
this.name = name;
40+
}
41+
}

‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/mybatis/VersionedEntityMapper.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/*
2+
* Copyright 2026-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.jdbc.mybatis;
17+
18+
/**
19+
* Marker mapper interface used by MyBatis to discover {@code VersionedEntityMapper.xml}.
20+
*
21+
* @author Jens Schauder
22+
*/
23+
public interface VersionedEntityMapper {
24+
25+
}

‎spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.mybatis/MyBatisHsqlIntegrationTests-hsql.sql‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,7 @@
1-
CREATE TABLE dummyentity(id BIGINT GENERATED BY DEFAULT AS IDENTITY(START WITH 1) PRIMARY KEY);
1+
CREATE TABLE dummyentity(id BIGINT GENERATED BY DEFAULT AS IDENTITY(START WITH 1) PRIMARY KEY);
2+
3+
CREATE TABLE versioned_entity (
4+
id BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 1) PRIMARY KEY,
5+
version BIGINT NOT NULL,
6+
name VARCHAR(255)
7+
);

‎spring-data-jdbc/src/test/resources/org/springframework/data/jdbc/mybatis/VersionedEntityMapper.xml‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?xml version="1.0" encoding="UTF-8" ?>
2+
<!DOCTYPE mapper
3+
PUBLIC "-//mybatis.org//DTD Mapper 3.0//EN"
4+
"https://www.mybatis.org/dtd/mybatis-3-mapper.dtd">
5+
<mapper namespace="org.springframework.data.jdbc.mybatis.VersionedEntityMapper">
6+
<update id="updateWithVersion" parameterType="MyBatisContext">
7+
UPDATE versioned_entity
8+
SET version = #{instance.version,javaType=java.lang.Long,jdbcType=BIGINT},
9+
name = #{instance.name}
10+
WHERE id = #{instance.id}
11+
AND version = #{additionalValues[___oldOptimisticLockingVersion],javaType=java.lang.Long,jdbcType=BIGINT}
12+
</update>
13+
<delete id="deleteWithVersion" parameterType="MyBatisContext">
14+
DELETE FROM versioned_entity
15+
WHERE id = #{id}
16+
AND version = #{additionalValues[___oldOptimisticLockingVersion],javaType=java.lang.Long,jdbcType=BIGINT}
17+
</delete>
18+
</mapper>

0 commit comments

Comments
 (0)