mindol1004/spring-session
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Optimize session retrieval in JdbcOperationsSessionRepository

Browse Source 
Previously, SessionResultSetExtractor used JdbcSession.setAttribute which had a side effect of freshly loaded session potentially having a non-empty delta and/or changed flag set. This commit optimizes session retrieval to invoke setAttribute directly on the delegate, therefore preventing unnecessary modifications of delta and change flags.

Closes gh-1053
This commit is contained in:
Vedran Pavic
2018-04-16 09:40:13 +02:00
parent a3195f1f4b
commit c433b01ee5
3 changed files with 84 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

61
spring-session/src/integration-test/java/org/springframework/session/jdbc/AbstractJdbcOperationsSessionRepositoryITests.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2014-2016 the original author or authors.
* Copyright 2014-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -34,10 +34,8 @@ import org.springframework.security.core.Authentication;
import org.springframework.security.core.authority.AuthorityUtils;
import org.springframework.security.core.context.SecurityContext;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.session.ExpiringSession;
import org.springframework.session.FindByIndexNameSessionRepository;
import org.springframework.session.MapSession;
import org.springframework.session.Session;
import org.springframework.session.jdbc.config.annotation.web.http.EnableJdbcHttpSession;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
@@ -82,6 +80,19 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
AuthorityUtils.createAuthorityList("ROLE_USER")));
}
@Test
public void saveWhenNoAttributesThenCanBeFound() {
JdbcOperationsSessionRepository.JdbcSession toSave = this.repository
.createSession();
this.repository.save(toSave);
JdbcOperationsSessionRepository.JdbcSession session = this.repository.getSession(toSave.getId());
assertThat(session).isNotNull();
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
}
@Test
public void saves() throws InterruptedException {
String username = "saves-" + System.currentTimeMillis();
@@ -100,9 +111,11 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
this.repository.save(toSave);
Session session = this.repository.getSession(toSave.getId());
JdbcOperationsSessionRepository.JdbcSession session = this.repository.getSession(toSave.getId());
assertThat(session.getId()).isEqualTo(toSave.getId());
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
assertThat(session.getAttributeNames()).isEqualTo(toSave.getAttributeNames());
assertThat(session.getAttribute(expectedAttributeName))
.isEqualTo(toSave.getAttribute(expectedAttributeName));
@@ -135,7 +148,9 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
this.repository.save(toSave);
toSave = this.repository.getSession(toSave.getId());
Session session = this.repository.getSession(toSave.getId());
JdbcOperationsSessionRepository.JdbcSession session = this.repository.getSession(toSave.getId());
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
assertThat(session.getAttributeNames().size()).isEqualTo(2);
assertThat(session.getAttribute("a")).isEqualTo("b");
assertThat(session.getAttribute("1")).isEqualTo("2");
@@ -156,9 +171,11 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
toSave.setLastAccessedTime(lastAccessedTime);
this.repository.save(toSave);
ExpiringSession session = this.repository.getSession(toSave.getId());
JdbcOperationsSessionRepository.JdbcSession session = this.repository.getSession(toSave.getId());
assertThat(session).isNotNull();
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
assertThat(session.isExpired()).isFalse();
assertThat(session.getLastAccessedTime()).isEqualTo(lastAccessedTime);
}
@@ -225,6 +242,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
assertThat(findByPrincipalName).hasSize(1);
assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
for (JdbcOperationsSessionRepository.JdbcSession session : findByPrincipalName.values()) {
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
}
}
@Test
@@ -247,6 +268,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
assertThat(findByPrincipalName).hasSize(1);
assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
for (JdbcOperationsSessionRepository.JdbcSession session : findByPrincipalName.values()) {
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
}
}
@Test
@@ -289,6 +314,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
assertThat(findByPrincipalName).hasSize(1);
assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
for (JdbcOperationsSessionRepository.JdbcSession session : findByPrincipalName.values()) {
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
}
}
@Test
@@ -336,6 +365,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
assertThat(findByPrincipalName).hasSize(1);
assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
for (JdbcOperationsSessionRepository.JdbcSession session : findByPrincipalName.values()) {
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
}
}
@Test
@@ -395,6 +428,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
assertThat(findByPrincipalName).hasSize(1);
assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
for (JdbcOperationsSessionRepository.JdbcSession session : findByPrincipalName.values()) {
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
}
}
@Test
@@ -416,6 +453,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
assertThat(findByPrincipalName).hasSize(1);
assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
for (JdbcOperationsSessionRepository.JdbcSession session : findByPrincipalName.values()) {
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
}
}
@Test
@@ -455,6 +496,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
assertThat(findByPrincipalName).hasSize(1);
assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
for (JdbcOperationsSessionRepository.JdbcSession session : findByPrincipalName.values()) {
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
}
}
@Test
@@ -499,6 +544,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
assertThat(findByPrincipalName).hasSize(1);
assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
for (JdbcOperationsSessionRepository.JdbcSession session : findByPrincipalName.values()) {
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
}
}
@Test

43
spring-session/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java
View File

@@ -188,8 +188,7 @@ public class JdbcOperationsSessionRepository implements
private final TransactionOperations transactionOperations;
private final ResultSetExtractor<List<ExpiringSession>> extractor =
new ExpiringSessionResultSetExtractor();
private final ResultSetExtractor<List<JdbcSession>> extractor = new SessionResultSetExtractor();
/**
* The name of database table used by Spring Session to store sessions.
@@ -484,10 +483,10 @@ public class JdbcOperationsSessionRepository implements
}
public JdbcSession getSession(final String id) {
final ExpiringSession session = this.transactionOperations.execute(new TransactionCallback<ExpiringSession>() {
final JdbcSession session = this.transactionOperations.execute(new TransactionCallback<JdbcSession>() {
public ExpiringSession doInTransaction(TransactionStatus status) {
List<ExpiringSession> sessions = JdbcOperationsSessionRepository.this.jdbcOperations.query(
public JdbcSession doInTransaction(TransactionStatus status) {
List<JdbcSession> sessions = JdbcOperationsSessionRepository.this.jdbcOperations.query(
JdbcOperationsSessionRepository.this.getSessionQuery,
new PreparedStatementSetter() {
@@ -511,7 +510,7 @@ public class JdbcOperationsSessionRepository implements
delete(id);
}
else {
return new JdbcSession(session);
return session;
}
}
return null;
@@ -534,9 +533,9 @@ public class JdbcOperationsSessionRepository implements
return Collections.emptyMap();
}
List<ExpiringSession> sessions = this.transactionOperations.execute(new TransactionCallback<List<ExpiringSession>>() {
List<JdbcSession> sessions = this.transactionOperations.execute(new TransactionCallback<List<JdbcSession>>() {
public List<ExpiringSession> doInTransaction(TransactionStatus status) {
public List<JdbcSession> doInTransaction(TransactionStatus status) {
return JdbcOperationsSessionRepository.this.jdbcOperations.query(
JdbcOperationsSessionRepository.this.listSessionsByPrincipalNameQuery,
new PreparedStatementSetter() {
@@ -555,8 +554,8 @@ public class JdbcOperationsSessionRepository implements
Map<String, JdbcSession> sessionMap = new HashMap<String, JdbcSession>(
sessions.size());
for (ExpiringSession session : sessions) {
sessionMap.put(session.getId(), new JdbcSession(session));
for (JdbcSession session : sessions) {
sessionMap.put(session.getId(), session);
}
return sessionMap;
@@ -764,33 +763,33 @@ public class JdbcOperationsSessionRepository implements
}
private class ExpiringSessionResultSetExtractor
implements ResultSetExtractor<List<ExpiringSession>> {
private class SessionResultSetExtractor implements ResultSetExtractor<List<JdbcSession>> {
public List<ExpiringSession> extractData(ResultSet rs) throws SQLException, DataAccessException {
List<ExpiringSession> sessions = new ArrayList<ExpiringSession>();
public List<JdbcSession> extractData(ResultSet rs) throws SQLException, DataAccessException {
List<JdbcSession> sessions = new ArrayList<JdbcSession>();
while (rs.next()) {
String id = rs.getString("SESSION_ID");
MapSession session;
JdbcSession session;
if (sessions.size() > 0 && getLast(sessions).getId().equals(id)) {
session = (MapSession) getLast(sessions);
session = getLast(sessions);
}
else {
session = new MapSession(id);
session.setCreationTime(rs.getLong("CREATION_TIME"));
session.setLastAccessedTime(rs.getLong("LAST_ACCESS_TIME"));
session.setMaxInactiveIntervalInSeconds(rs.getInt("MAX_INACTIVE_INTERVAL"));
MapSession delegate = new MapSession(id);
delegate.setCreationTime(rs.getLong("CREATION_TIME"));
delegate.setLastAccessedTime(rs.getLong("LAST_ACCESS_TIME"));
delegate.setMaxInactiveIntervalInSeconds(rs.getInt("MAX_INACTIVE_INTERVAL"));
session = new JdbcSession(delegate);
}
String attributeName = rs.getString("ATTRIBUTE_NAME");
if (attributeName != null) {
session.setAttribute(attributeName, deserialize(rs, "ATTRIBUTE_BYTES"));
session.delegate.setAttribute(attributeName, deserialize(rs, "ATTRIBUTE_BYTES"));
}
sessions.add(session);
}
return sessions;
}
private ExpiringSession getLast(List<ExpiringSession> sessions) {
private JdbcSession getLast(List<JdbcSession> sessions) {
return sessions.get(sessions.size() - 1);
}

14
spring-session/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2014-2016 the original author or authors.
* Copyright 2014-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -412,7 +412,7 @@ public class JdbcOperationsSessionRepositoryTests {
@Test
public void getSessionExpired() {
MapSession expired = new MapSession();
JdbcOperationsSessionRepository.JdbcSession expired = this.repository.new JdbcSession();
expired.setLastAccessedTime(System.currentTimeMillis() -
(MapSession.DEFAULT_MAX_INACTIVE_INTERVAL_SECONDS * 1000 + 1000));
given(this.jdbcOperations.query(isA(String.class),
@@ -432,7 +432,8 @@ public class JdbcOperationsSessionRepositoryTests {
@Test
public void getSessionFound() {
MapSession saved = new MapSession();
JdbcOperationsSessionRepository.JdbcSession saved = this.repository.new JdbcSession(
new MapSession());
saved.setAttribute("savedName", "savedValue");
given(this.jdbcOperations.query(isA(String.class),
isA(PreparedStatementSetter.class), isA(ResultSetExtractor.class)))
@@ -493,11 +494,12 @@ public class JdbcOperationsSessionRepositoryTests {
String principal = "username";
Authentication authentication = new UsernamePasswordAuthenticationToken(principal,
"notused", AuthorityUtils.createAuthorityList("ROLE_USER"));
List<MapSession> saved = new ArrayList<MapSession>(2);
MapSession saved1 = new MapSession();
List<JdbcOperationsSessionRepository.JdbcSession> saved =
new ArrayList<JdbcOperationsSessionRepository.JdbcSession>(2);
JdbcOperationsSessionRepository.JdbcSession saved1 = this.repository.new JdbcSession();
saved1.setAttribute(SPRING_SECURITY_CONTEXT, authentication);
saved.add(saved1);
MapSession saved2 = new MapSession();
JdbcOperationsSessionRepository.JdbcSession saved2 = this.repository.new JdbcSession();
saved2.setAttribute(SPRING_SECURITY_CONTEXT, authentication);
saved.add(saved2);
given(this.jdbcOperations.query(isA(String.class),