From ca44c5628e0620545ef474161746d208bb365d13 Mon Sep 17 00:00:00 2001 From: Craig Andrews Date: Thu, 10 May 2018 12:44:54 -0400 Subject: [PATCH 1/4] Don't set delta to updated if already set to added If an attribute is added multiple times, it's delta value should still be ADDED - not UPDATED. --- .../jdbc/JdbcOperationsSessionRepository.java | 2 +- .../JdbcOperationsSessionRepositoryTests.java | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java b/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java index 55a9f8361..7567bec0a 100644 --- a/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java +++ b/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java @@ -735,7 +735,7 @@ public void setAttribute(String attributeName, Object attributeValue) { if (attributeValue == null) { this.delta.put(attributeName, DeltaValue.REMOVED); } - else if (this.delegate.getAttribute(attributeName) != null) { + else if (this.delta.get(attributeName) != DeltaValue.ADDED && this.delegate.getAttribute(attributeName) != null) { this.delta.put(attributeName, DeltaValue.UPDATED); } else { diff --git a/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java b/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java index 140f66c22..1cce6a9cb 100644 --- a/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java +++ b/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java @@ -338,6 +338,23 @@ public void saveUpdatedAddSingleAttribute() { verifyZeroInteractions(this.jdbcOperations); } + @Test + public void saveUpdatedAddSingleAttributeSetTwice() { + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", + new MapSession()); + session.setAttribute("testName", "testValue"); + session.setAttribute("testName", "testValue"); + + this.repository.save(session); + + assertThat(session.isNew()).isFalse(); + assertPropagationRequiresNew(); + verify(this.jdbcOperations, times(1)).update( + startsWith("INSERT INTO SPRING_SESSION_ATTRIBUTES("), + isA(PreparedStatementSetter.class)); + verifyZeroInteractions(this.jdbcOperations); + } + @Test public void saveUpdatedAddMultipleAttributes() { JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", From 8430af278a6e2fd81938157f58a1648d71fd02d3 Mon Sep 17 00:00:00 2001 From: Craig Andrews Date: Sat, 12 May 2018 22:07:49 -0400 Subject: [PATCH 2/4] When an attribute is added then removed, do not execute a query --- .../jdbc/JdbcOperationsSessionRepository.java | 7 ++++++- .../JdbcOperationsSessionRepositoryTests.java | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java b/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java index 7567bec0a..279f676a9 100644 --- a/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java +++ b/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java @@ -733,7 +733,12 @@ public Set getAttributeNames() { @Override public void setAttribute(String attributeName, Object attributeValue) { if (attributeValue == null) { - this.delta.put(attributeName, DeltaValue.REMOVED); + if (this.delta.get(attributeName) == DeltaValue.ADDED) { + this.delta.remove(attributeName); + } + else { + this.delta.put(attributeName, DeltaValue.REMOVED); + } } else if (this.delta.get(attributeName) != DeltaValue.ADDED && this.delegate.getAttribute(attributeName) != null) { this.delta.put(attributeName, DeltaValue.UPDATED); diff --git a/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java b/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java index 1cce6a9cb..3813e121e 100644 --- a/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java +++ b/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java @@ -43,12 +43,14 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isA; import static org.mockito.ArgumentMatchers.startsWith; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyZeroInteractions; @@ -448,6 +450,23 @@ public void saveUpdatedRemoveMultipleAttributes() { verifyZeroInteractions(this.jdbcOperations); } + @Test + public void saveUpdatedAddThenRemoveSingleAttribute() { + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", + new MapSession()); + session.setAttribute("testName", "testValue"); + session.removeAttribute("testName"); + + this.repository.save(session); + + assertThat(session.isNew()).isFalse(); + assertPropagationRequiresNew(); + verify(this.jdbcOperations, never()).update( + anyString(), + isA(PreparedStatementSetter.class)); + verifyZeroInteractions(this.jdbcOperations); + } + @Test public void saveUpdatedLastAccessedTime() { JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", From 70a9012ea7047ddc653b9557dfdb4ee7cd29e429 Mon Sep 17 00:00:00 2001 From: Craig Andrews Date: Sat, 12 May 2018 22:09:17 -0400 Subject: [PATCH 3/4] When an attribute is removed then set again, an UPDATE query should result, not an INSERT --- .../jdbc/JdbcOperationsSessionRepository.java | 7 ++++++- .../JdbcOperationsSessionRepositoryTests.java | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java b/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java index 279f676a9..ae42620e8 100644 --- a/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java +++ b/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java @@ -744,7 +744,12 @@ else if (this.delta.get(attributeName) != DeltaValue.ADDED && this.delegate.getA this.delta.put(attributeName, DeltaValue.UPDATED); } else { - this.delta.put(attributeName, DeltaValue.ADDED); + if (this.delta.get(attributeName) == DeltaValue.REMOVED) { + this.delta.put(attributeName, DeltaValue.UPDATED); + } + else { + this.delta.put(attributeName, DeltaValue.ADDED); + } } this.delegate.setAttribute(attributeName, attributeValue); if (PRINCIPAL_NAME_INDEX_NAME.equals(attributeName) || diff --git a/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java b/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java index 3813e121e..fca217c5a 100644 --- a/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java +++ b/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java @@ -467,6 +467,25 @@ public void saveUpdatedAddThenRemoveSingleAttribute() { verifyZeroInteractions(this.jdbcOperations); } + @Test + public void saveUpdatedRemoveThenModifySingleAttribute() { + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", + new MapSession()); + session.setAttribute("testName", "testValue"); + session.clearChangeFlags(); + session.removeAttribute("testName"); + session.setAttribute("testName", "testValueModifed"); + + this.repository.save(session); + + assertThat(session.isNew()).isFalse(); + assertPropagationRequiresNew(); + verify(this.jdbcOperations, times(1)).update( + startsWith("UPDATE SPRING_SESSION_ATTRIBUTES SET"), + isA(PreparedStatementSetter.class)); + verifyZeroInteractions(this.jdbcOperations); + } + @Test public void saveUpdatedLastAccessedTime() { JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", From be52a1cfebe0ff0b4f939ade72b82d4facb18a2c Mon Sep 17 00:00:00 2001 From: Craig Andrews Date: Sat, 12 May 2018 22:10:21 -0400 Subject: [PATCH 4/4] Add a test to ensure that modifying then removing an attribute results in a DELETE --- .../JdbcOperationsSessionRepositoryTests.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java b/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java index fca217c5a..f1c2d3d51 100644 --- a/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java +++ b/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java @@ -467,6 +467,25 @@ public void saveUpdatedAddThenRemoveSingleAttribute() { verifyZeroInteractions(this.jdbcOperations); } + @Test + public void saveUpdatedModifyThenRemoveSingleAttribute() { + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", + new MapSession()); + session.setAttribute("testName", "testValue"); + session.clearChangeFlags(); + session.setAttribute("testName", "testValueModifed"); + session.removeAttribute("testName"); + + this.repository.save(session); + + assertThat(session.isNew()).isFalse(); + assertPropagationRequiresNew(); + verify(this.jdbcOperations, times(1)).update( + startsWith("DELETE FROM SPRING_SESSION_ATTRIBUTES WHERE"), + isA(PreparedStatementSetter.class)); + verifyZeroInteractions(this.jdbcOperations); + } + @Test public void saveUpdatedRemoveThenModifySingleAttribute() { JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey",