Skip to content

Commit e670e13

Browse files
ryanrupprwinch
authored andcommitted
Only start a new transaction if there's a delta for session updates with JDBC based repository
Avoids eagerly acquiring a connection or any other eager initialization that may happen alongside a transaction starting if there's no actual update to be made.
1 parent 2c3ecda commit e670e13

File tree

2 files changed

+58
-26
lines changed

2 files changed

+58
-26
lines changed

spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcIndexedSessionRepository.java

+37-25
Original file line numberDiff line numberDiff line change
@@ -907,45 +907,57 @@ private void save() {
907907
});
908908
}
909909
else {
910-
JdbcIndexedSessionRepository.this.transactionOperations.executeWithoutResult((status) -> {
911-
if (JdbcSession.this.changed) {
910+
List<Runnable> deltaActions = new ArrayList<>();
911+
if (JdbcSession.this.changed) {
912+
deltaActions.add(() -> {
912913
Map<String, String> indexes = JdbcIndexedSessionRepository.this.indexResolver
913-
.resolveIndexesFor(JdbcSession.this);
914+
.resolveIndexesFor(JdbcSession.this);
914915
JdbcIndexedSessionRepository.this.jdbcOperations
915-
.update(JdbcIndexedSessionRepository.this.updateSessionQuery, (ps) -> {
916-
ps.setString(1, getId());
917-
ps.setLong(2, getLastAccessedTime().toEpochMilli());
918-
ps.setInt(3, (int) getMaxInactiveInterval().getSeconds());
919-
ps.setLong(4, getExpiryTime().toEpochMilli());
920-
ps.setString(5, indexes.get(PRINCIPAL_NAME_INDEX_NAME));
921-
ps.setString(6, JdbcSession.this.primaryKey);
922-
});
923-
}
924-
List<String> addedAttributeNames = JdbcSession.this.delta.entrySet()
916+
.update(JdbcIndexedSessionRepository.this.updateSessionQuery, (ps) -> {
917+
ps.setString(1, getId());
918+
ps.setLong(2, getLastAccessedTime().toEpochMilli());
919+
ps.setInt(3, (int) getMaxInactiveInterval().getSeconds());
920+
ps.setLong(4, getExpiryTime().toEpochMilli());
921+
ps.setString(5, indexes.get(PRINCIPAL_NAME_INDEX_NAME));
922+
ps.setString(6, JdbcSession.this.primaryKey);
923+
});
924+
});
925+
}
926+
927+
List<String> addedAttributeNames = JdbcSession.this.delta.entrySet()
925928
.stream()
926929
.filter((entry) -> entry.getValue() == DeltaValue.ADDED)
927930
.map(Map.Entry::getKey)
928931
.collect(Collectors.toList());
929-
if (!addedAttributeNames.isEmpty()) {
930-
insertSessionAttributes(JdbcSession.this, addedAttributeNames);
931-
}
932-
List<String> updatedAttributeNames = JdbcSession.this.delta.entrySet()
932+
if (!addedAttributeNames.isEmpty()) {
933+
deltaActions.add(() -> insertSessionAttributes(JdbcSession.this, addedAttributeNames));
934+
}
935+
936+
List<String> updatedAttributeNames = JdbcSession.this.delta.entrySet()
933937
.stream()
934938
.filter((entry) -> entry.getValue() == DeltaValue.UPDATED)
935939
.map(Map.Entry::getKey)
936940
.collect(Collectors.toList());
937-
if (!updatedAttributeNames.isEmpty()) {
938-
updateSessionAttributes(JdbcSession.this, updatedAttributeNames);
939-
}
940-
List<String> removedAttributeNames = JdbcSession.this.delta.entrySet()
941+
if (!updatedAttributeNames.isEmpty()) {
942+
deltaActions.add(() -> updateSessionAttributes(JdbcSession.this, updatedAttributeNames));
943+
}
944+
945+
List<String> removedAttributeNames = JdbcSession.this.delta.entrySet()
941946
.stream()
942947
.filter((entry) -> entry.getValue() == DeltaValue.REMOVED)
943948
.map(Map.Entry::getKey)
944949
.collect(Collectors.toList());
945-
if (!removedAttributeNames.isEmpty()) {
946-
deleteSessionAttributes(JdbcSession.this, removedAttributeNames);
947-
}
948-
});
950+
if (!removedAttributeNames.isEmpty()) {
951+
deltaActions.add(() -> deleteSessionAttributes(JdbcSession.this, removedAttributeNames));
952+
}
953+
954+
if (!deltaActions.isEmpty()) {
955+
JdbcIndexedSessionRepository.this.transactionOperations.executeWithoutResult((status) -> {
956+
for (Runnable action : deltaActions) {
957+
action.run();
958+
}
959+
});
960+
}
949961
}
950962
clearChangeFlags();
951963
}

spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcIndexedSessionRepositoryTests.java

+21-1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.util.List;
2424
import java.util.Map;
2525
import java.util.UUID;
26+
import java.util.function.Consumer;
2627
import java.util.function.Supplier;
2728

2829
import org.junit.jupiter.api.BeforeEach;
@@ -48,20 +49,25 @@
4849
import org.springframework.session.SaveMode;
4950
import org.springframework.session.Session;
5051
import org.springframework.session.jdbc.JdbcIndexedSessionRepository.JdbcSession;
52+
import org.springframework.transaction.TransactionStatus;
53+
import org.springframework.transaction.support.TransactionCallback;
5154
import org.springframework.transaction.support.TransactionOperations;
5255

5356
import static org.assertj.core.api.Assertions.assertThat;
5457
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
58+
import static org.mockito.ArgumentMatchers.any;
5559
import static org.mockito.ArgumentMatchers.anyLong;
5660
import static org.mockito.ArgumentMatchers.eq;
5761
import static org.mockito.ArgumentMatchers.isA;
5862
import static org.mockito.ArgumentMatchers.matches;
5963
import static org.mockito.ArgumentMatchers.startsWith;
6064
import static org.mockito.BDDMockito.given;
6165
import static org.mockito.Mockito.atLeastOnce;
66+
import static org.mockito.Mockito.lenient;
6267
import static org.mockito.Mockito.mock;
6368
import static org.mockito.Mockito.times;
6469
import static org.mockito.Mockito.verify;
70+
import static org.mockito.Mockito.verifyNoInteractions;
6571
import static org.mockito.Mockito.verifyNoMoreInteractions;
6672

6773
/**
@@ -78,12 +84,25 @@ class JdbcIndexedSessionRepositoryTests {
7884
@Mock
7985
private JdbcOperations jdbcOperations;
8086

87+
@Mock
88+
private TransactionOperations transactionOperations;
89+
8190
private JdbcIndexedSessionRepository repository;
8291

8392
@BeforeEach
8493
void setUp() {
94+
// Mock transaction callbacks to the real consumer
95+
lenient().doAnswer(answer -> {
96+
answer.getArgument(0, Consumer.class).accept(mock(TransactionStatus.class));
97+
return null;
98+
}).when(transactionOperations).executeWithoutResult(any());
99+
100+
lenient().doAnswer(answer ->
101+
answer.getArgument(0, TransactionCallback.class).doInTransaction(mock(TransactionStatus.class))
102+
).when(transactionOperations).execute(any());
103+
85104
this.repository = new JdbcIndexedSessionRepository(this.jdbcOperations,
86-
TransactionOperations.withoutTransaction());
105+
transactionOperations);
87106
}
88107

89108
@Test
@@ -467,6 +486,7 @@ void saveUpdatedAddAndRemoveAttribute() {
467486

468487
assertThat(session.isNew()).isFalse();
469488
verifyNoMoreInteractions(this.jdbcOperations);
489+
verifyNoMoreInteractions(this.transactionOperations);
470490
}
471491

472492
@Test // gh-1070

0 commit comments

Comments
 (0)