From b6348736acf03c1bc217b22157c61c4889038218 Mon Sep 17 00:00:00 2001 From: Vedran Pavic Date: Mon, 14 May 2018 10:28:45 +0200 Subject: [PATCH] Polish contribution Closes gh-1070 --- ...JdbcOperationsSessionRepositoryITests.java | 54 +++++++++++++ .../jdbc/JdbcOperationsSessionRepository.java | 32 +++++--- .../JdbcOperationsSessionRepositoryTests.java | 79 +++++++++++-------- 3 files changed, 117 insertions(+), 48 deletions(-) diff --git a/spring-session-jdbc/src/integration-test/java/org/springframework/session/jdbc/AbstractJdbcOperationsSessionRepositoryITests.java b/spring-session-jdbc/src/integration-test/java/org/springframework/session/jdbc/AbstractJdbcOperationsSessionRepositoryITests.java index 92041a07..272b8580 100644 --- a/spring-session-jdbc/src/integration-test/java/org/springframework/session/jdbc/AbstractJdbcOperationsSessionRepositoryITests.java +++ b/spring-session-jdbc/src/integration-test/java/org/springframework/session/jdbc/AbstractJdbcOperationsSessionRepositoryITests.java @@ -689,6 +689,60 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests { assertThat(this.repository.findById(originalId)).isNull(); } + @Test // gh-1070 + public void saveUpdatedAddAndModifyAttribute() { + JdbcOperationsSessionRepository.JdbcSession session = this.repository.createSession(); + this.repository.save(session); + session = this.repository.findById(session.getId()); + session.setAttribute("testName", "testValue1"); + session.setAttribute("testName", "testValue2"); + this.repository.save(session); + session = this.repository.findById(session.getId()); + + assertThat(session.getAttribute("testName")).isEqualTo("testValue2"); + } + + @Test // gh-1070 + public void saveUpdatedAddAndRemoveAttribute() { + JdbcOperationsSessionRepository.JdbcSession session = this.repository.createSession(); + this.repository.save(session); + session = this.repository.findById(session.getId()); + session.setAttribute("testName", "testValue"); + session.removeAttribute("testName"); + this.repository.save(session); + session = this.repository.findById(session.getId()); + + assertThat(session.getAttribute("testName")).isNull(); + } + + @Test // gh-1070 + public void saveUpdatedModifyAndRemoveAttribute() { + JdbcOperationsSessionRepository.JdbcSession session = this.repository.createSession(); + session.setAttribute("testName", "testValue1"); + this.repository.save(session); + session = this.repository.findById(session.getId()); + session.setAttribute("testName", "testValue2"); + session.removeAttribute("testName"); + this.repository.save(session); + session = this.repository.findById(session.getId()); + + assertThat(session.getAttribute("testName")).isNull(); + } + + @Test // gh-1070 + public void saveUpdatedRemoveAndAddAttribute() { + JdbcOperationsSessionRepository.JdbcSession session = this.repository.createSession(); + session.setAttribute("testName", "testValue1"); + this.repository.save(session); + session = this.repository.findById(session.getId()); + session.removeAttribute("testName"); + session.setAttribute("testName", "testValue2"); + this.repository.save(session); + session = this.repository.findById(session.getId()); + + assertThat(session.getAttribute("testName")).isEqualTo("testValue2"); + } + private String getSecurityName() { return this.context.getAuthentication().getName(); } 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 ae42620e..619ae4f2 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 @@ -732,24 +732,30 @@ public class JdbcOperationsSessionRepository implements @Override public void setAttribute(String attributeName, Object attributeValue) { - if (attributeValue == null) { - if (this.delta.get(attributeName) == DeltaValue.ADDED) { - this.delta.remove(attributeName); + boolean attributeExists = (this.delegate.getAttribute(attributeName) != null); + boolean attributeRemoved = (attributeValue == null); + if (!attributeExists && attributeRemoved) { + return; + } + if (attributeExists) { + if (attributeRemoved) { + this.delta.merge(attributeName, DeltaValue.REMOVED, + (oldDeltaValue, deltaValue) -> oldDeltaValue == DeltaValue.ADDED + ? null + : deltaValue); } else { - this.delta.put(attributeName, DeltaValue.REMOVED); + this.delta.merge(attributeName, DeltaValue.UPDATED, + (oldDeltaValue, deltaValue) -> oldDeltaValue == DeltaValue.ADDED + ? oldDeltaValue + : deltaValue); } } - else if (this.delta.get(attributeName) != DeltaValue.ADDED && this.delegate.getAttribute(attributeName) != null) { - this.delta.put(attributeName, DeltaValue.UPDATED); - } else { - if (this.delta.get(attributeName) == DeltaValue.REMOVED) { - this.delta.put(attributeName, DeltaValue.UPDATED); - } - else { - this.delta.put(attributeName, DeltaValue.ADDED); - } + this.delta.merge(attributeName, DeltaValue.ADDED, + (oldDeltaValue, deltaValue) -> oldDeltaValue == DeltaValue.ADDED + ? oldDeltaValue + : DeltaValue.UPDATED); } 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 f1c2d3d5..557879fe 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,14 +43,12 @@ import org.springframework.transaction.TransactionDefinition; 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; @@ -59,6 +57,7 @@ import static org.mockito.Mockito.verifyZeroInteractions; * Tests for {@link JdbcOperationsSessionRepository}. * * @author Vedran Pavic + * @author Craig Andrews * @since 1.2.0 */ public class JdbcOperationsSessionRepositoryTests { @@ -340,23 +339,6 @@ public class JdbcOperationsSessionRepositoryTests { 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", @@ -430,6 +412,19 @@ public class JdbcOperationsSessionRepositoryTests { verifyZeroInteractions(this.jdbcOperations); } + @Test + public void saveUpdatedRemoveNonExistingAttribute() { + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", + new MapSession()); + session.removeAttribute("testName"); + + this.repository.save(session); + + assertThat(session.isNew()).isFalse(); + assertPropagationRequiresNew(); + verifyZeroInteractions(this.jdbcOperations); + } + @Test public void saveUpdatedRemoveMultipleAttributes() { JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", @@ -450,56 +445,70 @@ public class JdbcOperationsSessionRepositoryTests { verifyZeroInteractions(this.jdbcOperations); } - @Test - public void saveUpdatedAddThenRemoveSingleAttribute() { + @Test // gh-1070 + public void saveUpdatedAddAndModifyAttribute() { JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", new MapSession()); - session.setAttribute("testName", "testValue"); - session.removeAttribute("testName"); + session.setAttribute("testName", "testValue1"); + session.setAttribute("testName", "testValue2"); this.repository.save(session); assertThat(session.isNew()).isFalse(); assertPropagationRequiresNew(); - verify(this.jdbcOperations, never()).update( - anyString(), + verify(this.jdbcOperations).update( + startsWith("INSERT INTO SPRING_SESSION_ATTRIBUTES("), isA(PreparedStatementSetter.class)); verifyZeroInteractions(this.jdbcOperations); } - @Test - public void saveUpdatedModifyThenRemoveSingleAttribute() { + @Test // gh-1070 + public void saveUpdatedAddAndRemoveAttribute() { 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( + verifyZeroInteractions(this.jdbcOperations); + } + + @Test // gh-1070 + public void saveUpdatedModifyAndRemoveAttribute() { + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", + new MapSession()); + session.setAttribute("testName", "testValue1"); + session.clearChangeFlags(); + session.setAttribute("testName", "testValue2"); + session.removeAttribute("testName"); + + this.repository.save(session); + + assertThat(session.isNew()).isFalse(); + assertPropagationRequiresNew(); + verify(this.jdbcOperations).update( startsWith("DELETE FROM SPRING_SESSION_ATTRIBUTES WHERE"), isA(PreparedStatementSetter.class)); verifyZeroInteractions(this.jdbcOperations); } - @Test - public void saveUpdatedRemoveThenModifySingleAttribute() { + @Test // gh-1070 + public void saveUpdatedRemoveAndAddAttribute() { JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", new MapSession()); - session.setAttribute("testName", "testValue"); + session.setAttribute("testName", "testValue1"); session.clearChangeFlags(); session.removeAttribute("testName"); - session.setAttribute("testName", "testValueModifed"); + session.setAttribute("testName", "testValue2"); this.repository.save(session); assertThat(session.isNew()).isFalse(); assertPropagationRequiresNew(); - verify(this.jdbcOperations, times(1)).update( + verify(this.jdbcOperations).update( startsWith("UPDATE SPRING_SESSION_ATTRIBUTES SET"), isA(PreparedStatementSetter.class)); verifyZeroInteractions(this.jdbcOperations);