Spring/spring-session
1
0
Fork 0
Code Issues 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-1042
This commit is contained in:
Vedran Pavic
2018-04-13 23:03:08 +02:00
parent 5173026aa8
commit 2395582fe6
2 changed files with 52 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -1,5 +1,5 @@
/*
* Copyright 2014-2017 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.
@@ -37,7 +37,6 @@ import org.springframework.security.core.context.SecurityContext;
import org.springframework.security.core.context.SecurityContextHolder;
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.transaction.PlatformTransactionManager;
import org.springframework.transaction.annotation.Transactional;
@@ -82,8 +81,11 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
.createSession();
this.repository.save(toSave);
JdbcOperationsSessionRepository.JdbcSession session = this.repository.findById(toSave.getId());
assertThat(this.repository.findById(toSave.getId())).isNotNull();
assertThat(session).isNotNull();
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
}
@Test
@@ -104,9 +106,11 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
this.repository.save(toSave);
Session session = this.repository.findById(toSave.getId());
JdbcOperationsSessionRepository.JdbcSession session = this.repository.findById(toSave.getId());
assertThat(session.getId()).isEqualTo(toSave.getId());
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
assertThat(session.getAttributeNames()).isEqualTo(toSave.getAttributeNames());
assertThat(session.<String>getAttribute(expectedAttributeName))
.isEqualTo(toSave.getAttribute(expectedAttributeName));
@@ -139,7 +143,9 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
this.repository.save(toSave);
toSave = this.repository.findById(toSave.getId());
Session session = this.repository.findById(toSave.getId());
JdbcOperationsSessionRepository.JdbcSession session = this.repository.findById(toSave.getId());
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
assertThat(session.getAttributeNames().size()).isEqualTo(2);
assertThat(session.<String>getAttribute("a")).isEqualTo("b");
assertThat(session.<String>getAttribute("1")).isEqualTo("2");
@@ -160,9 +166,11 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
toSave.setLastAccessedTime(lastAccessedTime);
this.repository.save(toSave);
Session session = this.repository.findById(toSave.getId());
JdbcOperationsSessionRepository.JdbcSession session = this.repository.findById(toSave.getId());
assertThat(session).isNotNull();
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
assertThat(session.isExpired()).isFalse();
assertThat(session.getLastAccessedTime()).isEqualTo(lastAccessedTime);
}
@@ -229,6 +237,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
assertThat(findByPrincipalName).hasSize(1);
assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
findByPrincipalName.values().forEach(session -> {
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
});
}
@Test
@@ -251,6 +263,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
assertThat(findByPrincipalName).hasSize(1);
assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
findByPrincipalName.values().forEach(session -> {
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
});
}
@Test
@@ -293,6 +309,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
assertThat(findByPrincipalName).hasSize(1);
assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
findByPrincipalName.values().forEach(session -> {
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
});
}
@Test
@@ -340,6 +360,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
assertThat(findByPrincipalName).hasSize(1);
assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
findByPrincipalName.values().forEach(session -> {
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
});
}
@Test
@@ -399,6 +423,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
assertThat(findByPrincipalName).hasSize(1);
assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
findByPrincipalName.values().forEach(session -> {
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
});
}
@Test
@@ -420,6 +448,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
assertThat(findByPrincipalName).hasSize(1);
assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
findByPrincipalName.values().forEach(session -> {
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
});
}
@Test
@@ -459,6 +491,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
assertThat(findByPrincipalName).hasSize(1);
assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
findByPrincipalName.values().forEach(session -> {
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
});
}
@Test
@@ -503,6 +539,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
assertThat(findByPrincipalName).hasSize(1);
assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
findByPrincipalName.values().forEach(session -> {
assertThat(session.isChanged()).isFalse();
assertThat(session.getDelta()).isEmpty();
});
}
@Test
@@ -585,6 +625,8 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
JdbcOperationsSessionRepository.JdbcSession findByChangeSessionId = this.repository.findById(changeSessionId);
assertThat(findByChangeSessionId.isChanged()).isFalse();
assertThat(findByChangeSessionId.getDelta()).isEmpty();
assertThat(findByChangeSessionId.<String>getAttribute(attrName)).isEqualTo(attrValue);
}
@@ -627,6 +669,8 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
JdbcOperationsSessionRepository.JdbcSession findByChangeSessionId = this.repository.findById(changeSessionId);
assertThat(findByChangeSessionId.isChanged()).isFalse();
assertThat(findByChangeSessionId.getDelta()).isEmpty();
assertThat(findByChangeSessionId.<String>getAttribute(attrName)).isEqualTo(attrValue);
}

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

@@ -1,5 +1,5 @@
/*
* Copyright 2014-2017 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.
@@ -758,7 +758,7 @@ public class JdbcOperationsSessionRepository implements
}
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);
}