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.
This commit is contained in:
@@ -907,45 +907,57 @@ public class JdbcIndexedSessionRepository implements
|
||||
});
|
||||
}
|
||||
else {
|
||||
JdbcIndexedSessionRepository.this.transactionOperations.executeWithoutResult((status) -> {
|
||||
if (JdbcSession.this.changed) {
|
||||
List<Runnable> deltaActions = new ArrayList<>();
|
||||
if (JdbcSession.this.changed) {
|
||||
deltaActions.add(() -> {
|
||||
Map<String, String> indexes = JdbcIndexedSessionRepository.this.indexResolver
|
||||
.resolveIndexesFor(JdbcSession.this);
|
||||
.resolveIndexesFor(JdbcSession.this);
|
||||
JdbcIndexedSessionRepository.this.jdbcOperations
|
||||
.update(JdbcIndexedSessionRepository.this.updateSessionQuery, (ps) -> {
|
||||
ps.setString(1, getId());
|
||||
ps.setLong(2, getLastAccessedTime().toEpochMilli());
|
||||
ps.setInt(3, (int) getMaxInactiveInterval().getSeconds());
|
||||
ps.setLong(4, getExpiryTime().toEpochMilli());
|
||||
ps.setString(5, indexes.get(PRINCIPAL_NAME_INDEX_NAME));
|
||||
ps.setString(6, JdbcSession.this.primaryKey);
|
||||
});
|
||||
}
|
||||
List<String> addedAttributeNames = JdbcSession.this.delta.entrySet()
|
||||
.update(JdbcIndexedSessionRepository.this.updateSessionQuery, (ps) -> {
|
||||
ps.setString(1, getId());
|
||||
ps.setLong(2, getLastAccessedTime().toEpochMilli());
|
||||
ps.setInt(3, (int) getMaxInactiveInterval().getSeconds());
|
||||
ps.setLong(4, getExpiryTime().toEpochMilli());
|
||||
ps.setString(5, indexes.get(PRINCIPAL_NAME_INDEX_NAME));
|
||||
ps.setString(6, JdbcSession.this.primaryKey);
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
List<String> addedAttributeNames = JdbcSession.this.delta.entrySet()
|
||||
.stream()
|
||||
.filter((entry) -> entry.getValue() == DeltaValue.ADDED)
|
||||
.map(Map.Entry::getKey)
|
||||
.collect(Collectors.toList());
|
||||
if (!addedAttributeNames.isEmpty()) {
|
||||
insertSessionAttributes(JdbcSession.this, addedAttributeNames);
|
||||
}
|
||||
List<String> updatedAttributeNames = JdbcSession.this.delta.entrySet()
|
||||
if (!addedAttributeNames.isEmpty()) {
|
||||
deltaActions.add(() -> insertSessionAttributes(JdbcSession.this, addedAttributeNames));
|
||||
}
|
||||
|
||||
List<String> updatedAttributeNames = JdbcSession.this.delta.entrySet()
|
||||
.stream()
|
||||
.filter((entry) -> entry.getValue() == DeltaValue.UPDATED)
|
||||
.map(Map.Entry::getKey)
|
||||
.collect(Collectors.toList());
|
||||
if (!updatedAttributeNames.isEmpty()) {
|
||||
updateSessionAttributes(JdbcSession.this, updatedAttributeNames);
|
||||
}
|
||||
List<String> removedAttributeNames = JdbcSession.this.delta.entrySet()
|
||||
if (!updatedAttributeNames.isEmpty()) {
|
||||
deltaActions.add(() -> updateSessionAttributes(JdbcSession.this, updatedAttributeNames));
|
||||
}
|
||||
|
||||
List<String> removedAttributeNames = JdbcSession.this.delta.entrySet()
|
||||
.stream()
|
||||
.filter((entry) -> entry.getValue() == DeltaValue.REMOVED)
|
||||
.map(Map.Entry::getKey)
|
||||
.collect(Collectors.toList());
|
||||
if (!removedAttributeNames.isEmpty()) {
|
||||
deleteSessionAttributes(JdbcSession.this, removedAttributeNames);
|
||||
}
|
||||
});
|
||||
if (!removedAttributeNames.isEmpty()) {
|
||||
deltaActions.add(() -> deleteSessionAttributes(JdbcSession.this, removedAttributeNames));
|
||||
}
|
||||
|
||||
if (!deltaActions.isEmpty()) {
|
||||
JdbcIndexedSessionRepository.this.transactionOperations.executeWithoutResult((status) -> {
|
||||
for (Runnable action : deltaActions) {
|
||||
action.run();
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
clearChangeFlags();
|
||||
}
|
||||
|
||||
@@ -23,6 +23,7 @@ import java.util.Collections;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.UUID;
|
||||
import java.util.function.Consumer;
|
||||
import java.util.function.Supplier;
|
||||
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
@@ -48,10 +49,13 @@ import org.springframework.session.MapSession;
|
||||
import org.springframework.session.SaveMode;
|
||||
import org.springframework.session.Session;
|
||||
import org.springframework.session.jdbc.JdbcIndexedSessionRepository.JdbcSession;
|
||||
import org.springframework.transaction.TransactionStatus;
|
||||
import org.springframework.transaction.support.TransactionCallback;
|
||||
import org.springframework.transaction.support.TransactionOperations;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.ArgumentMatchers.anyLong;
|
||||
import static org.mockito.ArgumentMatchers.eq;
|
||||
import static org.mockito.ArgumentMatchers.isA;
|
||||
@@ -59,9 +63,11 @@ import static org.mockito.ArgumentMatchers.matches;
|
||||
import static org.mockito.ArgumentMatchers.startsWith;
|
||||
import static org.mockito.BDDMockito.given;
|
||||
import static org.mockito.Mockito.atLeastOnce;
|
||||
import static org.mockito.Mockito.lenient;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.times;
|
||||
import static org.mockito.Mockito.verify;
|
||||
import static org.mockito.Mockito.verifyNoInteractions;
|
||||
import static org.mockito.Mockito.verifyNoMoreInteractions;
|
||||
|
||||
/**
|
||||
@@ -78,12 +84,25 @@ class JdbcIndexedSessionRepositoryTests {
|
||||
@Mock
|
||||
private JdbcOperations jdbcOperations;
|
||||
|
||||
@Mock
|
||||
private TransactionOperations transactionOperations;
|
||||
|
||||
private JdbcIndexedSessionRepository repository;
|
||||
|
||||
@BeforeEach
|
||||
void setUp() {
|
||||
// Mock transaction callbacks to the real consumer
|
||||
lenient().doAnswer(answer -> {
|
||||
answer.getArgument(0, Consumer.class).accept(mock(TransactionStatus.class));
|
||||
return null;
|
||||
}).when(transactionOperations).executeWithoutResult(any());
|
||||
|
||||
lenient().doAnswer(answer ->
|
||||
answer.getArgument(0, TransactionCallback.class).doInTransaction(mock(TransactionStatus.class))
|
||||
).when(transactionOperations).execute(any());
|
||||
|
||||
this.repository = new JdbcIndexedSessionRepository(this.jdbcOperations,
|
||||
TransactionOperations.withoutTransaction());
|
||||
transactionOperations);
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -467,6 +486,7 @@ class JdbcIndexedSessionRepositoryTests {
|
||||
|
||||
assertThat(session.isNew()).isFalse();
|
||||
verifyNoMoreInteractions(this.jdbcOperations);
|
||||
verifyNoMoreInteractions(this.transactionOperations);
|
||||
}
|
||||
|
||||
@Test // gh-1070
|
||||
|
||||
Reference in New Issue
Block a user