Skip to content

Commit b634873

Browse files
vpavicvpavic
committed
Polish contribution
Closes gh-1070
1 parent 60581c6 commit b634873

File tree

3 files changed

+116
-47
lines changed

3 files changed

+116
-47
lines changed

‎spring-session-jdbc/src/integration-test/java/org/springframework/session/jdbc/AbstractJdbcOperationsSessionRepositoryITests.java

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -689,6 +689,60 @@ public void changeSessionIdWhenHasNotSaved() throws Exception {
689689
assertThat(this.repository.findById(originalId)).isNull();
690690
}
691691

692+
@Test // gh-1070
693+
public void saveUpdatedAddAndModifyAttribute() {
694+
JdbcOperationsSessionRepository.JdbcSession session = this.repository.createSession();
695+
this.repository.save(session);
696+
session = this.repository.findById(session.getId());
697+
session.setAttribute("testName", "testValue1");
698+
session.setAttribute("testName", "testValue2");
699+
this.repository.save(session);
700+
session = this.repository.findById(session.getId());
701+
702+
assertThat(session.<String>getAttribute("testName")).isEqualTo("testValue2");
703+
}
704+
705+
@Test // gh-1070
706+
public void saveUpdatedAddAndRemoveAttribute() {
707+
JdbcOperationsSessionRepository.JdbcSession session = this.repository.createSession();
708+
this.repository.save(session);
709+
session = this.repository.findById(session.getId());
710+
session.setAttribute("testName", "testValue");
711+
session.removeAttribute("testName");
712+
this.repository.save(session);
713+
session = this.repository.findById(session.getId());
714+
715+
assertThat(session.<String>getAttribute("testName")).isNull();
716+
}
717+
718+
@Test // gh-1070
719+
public void saveUpdatedModifyAndRemoveAttribute() {
720+
JdbcOperationsSessionRepository.JdbcSession session = this.repository.createSession();
721+
session.setAttribute("testName", "testValue1");
722+
this.repository.save(session);
723+
session = this.repository.findById(session.getId());
724+
session.setAttribute("testName", "testValue2");
725+
session.removeAttribute("testName");
726+
this.repository.save(session);
727+
session = this.repository.findById(session.getId());
728+
729+
assertThat(session.<String>getAttribute("testName")).isNull();
730+
}
731+
732+
@Test // gh-1070
733+
public void saveUpdatedRemoveAndAddAttribute() {
734+
JdbcOperationsSessionRepository.JdbcSession session = this.repository.createSession();
735+
session.setAttribute("testName", "testValue1");
736+
this.repository.save(session);
737+
session = this.repository.findById(session.getId());
738+
session.removeAttribute("testName");
739+
session.setAttribute("testName", "testValue2");
740+
this.repository.save(session);
741+
session = this.repository.findById(session.getId());
742+
743+
assertThat(session.<String>getAttribute("testName")).isEqualTo("testValue2");
744+
}
745+
692746
private String getSecurityName() {
693747
return this.context.getAuthentication().getName();
694748
}

‎spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -732,24 +732,30 @@ public Set<String> getAttributeNames() {
732732

733733
@Override
734734
public void setAttribute(String attributeName, Object attributeValue) {
735-
if (attributeValue == null) {
736-
if (this.delta.get(attributeName) == DeltaValue.ADDED) {
737-
this.delta.remove(attributeName);
735+
boolean attributeExists = (this.delegate.getAttribute(attributeName) != null);
736+
boolean attributeRemoved = (attributeValue == null);
737+
if (!attributeExists && attributeRemoved) {
738+
return;
739+
}
740+
if (attributeExists) {
741+
if (attributeRemoved) {
742+
this.delta.merge(attributeName, DeltaValue.REMOVED,
743+
(oldDeltaValue, deltaValue) -> oldDeltaValue == DeltaValue.ADDED
744+
? null
745+
: deltaValue);
738746
}
739747
else {
740-
this.delta.put(attributeName, DeltaValue.REMOVED);
748+
this.delta.merge(attributeName, DeltaValue.UPDATED,
749+
(oldDeltaValue, deltaValue) -> oldDeltaValue == DeltaValue.ADDED
750+
? oldDeltaValue
751+
: deltaValue);
741752
}
742753
}
743-
else if (this.delta.get(attributeName) != DeltaValue.ADDED && this.delegate.getAttribute(attributeName) != null) {
744-
this.delta.put(attributeName, DeltaValue.UPDATED);
745-
}
746754
else {
747-
if (this.delta.get(attributeName) == DeltaValue.REMOVED) {
748-
this.delta.put(attributeName, DeltaValue.UPDATED);
749-
}
750-
else {
751-
this.delta.put(attributeName, DeltaValue.ADDED);
752-
}
755+
this.delta.merge(attributeName, DeltaValue.ADDED,
756+
(oldDeltaValue, deltaValue) -> oldDeltaValue == DeltaValue.ADDED
757+
? oldDeltaValue
758+
: DeltaValue.UPDATED);
753759
}
754760
this.delegate.setAttribute(attributeName, attributeValue);
755761
if (PRINCIPAL_NAME_INDEX_NAME.equals(attributeName) ||

‎spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java

Lines changed: 43 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,12 @@
4343
import static org.assertj.core.api.Assertions.assertThat;
4444
import static org.assertj.core.api.Assertions.assertThatThrownBy;
4545
import static org.mockito.ArgumentMatchers.anyLong;
46-
import static org.mockito.ArgumentMatchers.anyString;
4746
import static org.mockito.ArgumentMatchers.eq;
4847
import static org.mockito.ArgumentMatchers.isA;
4948
import static org.mockito.ArgumentMatchers.startsWith;
5049
import static org.mockito.BDDMockito.given;
5150
import static org.mockito.Mockito.atLeastOnce;
5251
import static org.mockito.Mockito.mock;
53-
import static org.mockito.Mockito.never;
5452
import static org.mockito.Mockito.times;
5553
import static org.mockito.Mockito.verify;
5654
import static org.mockito.Mockito.verifyZeroInteractions;
@@ -59,6 +57,7 @@
5957
* Tests for {@link JdbcOperationsSessionRepository}.
6058
*
6159
* @author Vedran Pavic
60+
* @author Craig Andrews
6261
* @since 1.2.0
6362
*/
6463
public class JdbcOperationsSessionRepositoryTests {
@@ -340,23 +339,6 @@ public void saveUpdatedAddSingleAttribute() {
340339
verifyZeroInteractions(this.jdbcOperations);
341340
}
342341

343-
@Test
344-
public void saveUpdatedAddSingleAttributeSetTwice() {
345-
JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey",
346-
new MapSession());
347-
session.setAttribute("testName", "testValue");
348-
session.setAttribute("testName", "testValue");
349-
350-
this.repository.save(session);
351-
352-
assertThat(session.isNew()).isFalse();
353-
assertPropagationRequiresNew();
354-
verify(this.jdbcOperations, times(1)).update(
355-
startsWith("INSERT INTO SPRING_SESSION_ATTRIBUTES("),
356-
isA(PreparedStatementSetter.class));
357-
verifyZeroInteractions(this.jdbcOperations);
358-
}
359-
360342
@Test
361343
public void saveUpdatedAddMultipleAttributes() {
362344
JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey",
@@ -430,6 +412,19 @@ public void saveUpdatedRemoveSingleAttribute() {
430412
verifyZeroInteractions(this.jdbcOperations);
431413
}
432414

415+
@Test
416+
public void saveUpdatedRemoveNonExistingAttribute() {
417+
JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey",
418+
new MapSession());
419+
session.removeAttribute("testName");
420+
421+
this.repository.save(session);
422+
423+
assertThat(session.isNew()).isFalse();
424+
assertPropagationRequiresNew();
425+
verifyZeroInteractions(this.jdbcOperations);
426+
}
427+
433428
@Test
434429
public void saveUpdatedRemoveMultipleAttributes() {
435430
JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey",
@@ -450,56 +445,70 @@ public void saveUpdatedRemoveMultipleAttributes() {
450445
verifyZeroInteractions(this.jdbcOperations);
451446
}
452447

453-
@Test
454-
public void saveUpdatedAddThenRemoveSingleAttribute() {
448+
@Test // gh-1070
449+
public void saveUpdatedAddAndModifyAttribute() {
455450
JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey",
456451
new MapSession());
457-
session.setAttribute("testName", "testValue");
458-
session.removeAttribute("testName");
452+
session.setAttribute("testName", "testValue1");
453+
session.setAttribute("testName", "testValue2");
459454

460455
this.repository.save(session);
461456

462457
assertThat(session.isNew()).isFalse();
463458
assertPropagationRequiresNew();
464-
verify(this.jdbcOperations, never()).update(
465-
anyString(),
459+
verify(this.jdbcOperations).update(
460+
startsWith("INSERT INTO SPRING_SESSION_ATTRIBUTES("),
466461
isA(PreparedStatementSetter.class));
467462
verifyZeroInteractions(this.jdbcOperations);
468463
}
469464

470-
@Test
471-
public void saveUpdatedModifyThenRemoveSingleAttribute() {
465+
@Test // gh-1070
466+
public void saveUpdatedAddAndRemoveAttribute() {
472467
JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey",
473468
new MapSession());
474469
session.setAttribute("testName", "testValue");
470+
session.removeAttribute("testName");
471+
472+
this.repository.save(session);
473+
474+
assertThat(session.isNew()).isFalse();
475+
assertPropagationRequiresNew();
476+
verifyZeroInteractions(this.jdbcOperations);
477+
}
478+
479+
@Test // gh-1070
480+
public void saveUpdatedModifyAndRemoveAttribute() {
481+
JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey",
482+
new MapSession());
483+
session.setAttribute("testName", "testValue1");
475484
session.clearChangeFlags();
476-
session.setAttribute("testName", "testValueModifed");
485+
session.setAttribute("testName", "testValue2");
477486
session.removeAttribute("testName");
478487

479488
this.repository.save(session);
480489

481490
assertThat(session.isNew()).isFalse();
482491
assertPropagationRequiresNew();
483-
verify(this.jdbcOperations, times(1)).update(
492+
verify(this.jdbcOperations).update(
484493
startsWith("DELETE FROM SPRING_SESSION_ATTRIBUTES WHERE"),
485494
isA(PreparedStatementSetter.class));
486495
verifyZeroInteractions(this.jdbcOperations);
487496
}
488497

489-
@Test
490-
public void saveUpdatedRemoveThenModifySingleAttribute() {
498+
@Test // gh-1070
499+
public void saveUpdatedRemoveAndAddAttribute() {
491500
JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey",
492501
new MapSession());
493-
session.setAttribute("testName", "testValue");
502+
session.setAttribute("testName", "testValue1");
494503
session.clearChangeFlags();
495504
session.removeAttribute("testName");
496-
session.setAttribute("testName", "testValueModifed");
505+
session.setAttribute("testName", "testValue2");
497506

498507
this.repository.save(session);
499508

500509
assertThat(session.isNew()).isFalse();
501510
assertPropagationRequiresNew();
502-
verify(this.jdbcOperations, times(1)).update(
511+
verify(this.jdbcOperations).update(
503512
startsWith("UPDATE SPRING_SESSION_ATTRIBUTES SET"),
504513
isA(PreparedStatementSetter.class));
505514
verifyZeroInteractions(this.jdbcOperations);

0 commit comments

Comments
 (0)