Skip to content
This repository was archived by the owner on Apr 4, 2025. It is now read-only.

Align Spring Session MongoDB with WebSession API. #54

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,19 @@ public MongoSession createSession() {

@Override
public void save(MongoSession session) {
this.mongoOperations.save(convertToDBObject(this.mongoSessionConverter, session), this.collectionName);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reported issue and this PR are for alignment with WebSession API, which is a reactive thing, so we likely shouldn't make changes to MongoOperationsSessionRepository.

We can revisit that depending on outcome of spring-projects/spring-session#1277 as ideally we'd make the behavior switch for all SessionRepository implementations in sync.

if (session.isNew()) {

session.setNew(false);
this.mongoOperations.save(convertToDBObject(this.mongoSessionConverter, session), this.collectionName);
} else {

if (findSession(session.getId()) == null) {
throw new IllegalStateException("Session was invalidated");
} else {
this.mongoOperations.save(convertToDBObject(this.mongoSessionConverter, session), this.collectionName);
}
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ public class MongoSession implements Session {
private long intervalSeconds;
@Getter @Setter private Date expireAt;
private Map<String, Object> attrs = new HashMap<>();
private boolean isNew = true;

public MongoSession() {
this(MongoOperationsSessionRepository.DEFAULT_INACTIVE_INTERVAL);
Expand Down Expand Up @@ -129,6 +130,14 @@ public boolean isExpired() {
return this.intervalSeconds >= 0 && new Date().after(this.expireAt);
}

public void setNew(boolean isNew) {
this.isNew = isNew;
}

public boolean isNew() {
return this.isNew;
}

static String coverDot(String attributeName) {
return attributeName.replace('.', DOT_COVER_CHAR);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,18 @@ public Mono<MongoSession> createSession() {
@Override
public Mono<Void> save(MongoSession session) {

return this.mongoOperations
.save(convertToDBObject(this.mongoSessionConverter, session), this.collectionName)
.then();
if (session.isNew()) {

session.setNew(false);
return this.mongoOperations.save(convertToDBObject(this.mongoSessionConverter, session), this.collectionName)
.then();
} else {

return findSession(session.getId())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this works, you could do probably do a ReactiveMongoOperations#collectionExists to optimize by removing deserialization cost from the mix, right?

You could also compose the implementation of this method in such way to avoid writing the duplicate ReactiveMongoOperations#save call - see ReactiveRedisOperationsSessionRepository#save for inspiration.

.map(document -> this.mongoOperations.save(convertToDBObject(this.mongoSessionConverter, session), this.collectionName))
.switchIfEmpty(Mono.error(new IllegalStateException("Session was invalidated")))
.then();
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,52 +62,97 @@ public class MongoOperationsSessionRepositoryTest {
private MongoOperationsSessionRepository repository;

@Before
public void setUp() throws Exception {
public void setUp() {

this.repository = new MongoOperationsSessionRepository(this.mongoOperations);
this.repository.setMongoSessionConverter(this.converter);
}

@Test
public void shouldCreateSession() throws Exception {
public void shouldCreateSession() {

// when
MongoSession session = this.repository.createSession();

// then
assertThat(session.getId()).isNotEmpty();
assertThat(session.isNew()).isTrue();
assertThat(session.getMaxInactiveInterval().getSeconds())
.isEqualTo(MongoOperationsSessionRepository.DEFAULT_INACTIVE_INTERVAL);
}

@Test
public void shouldCreateSessionWhenMaxInactiveIntervalNotDefined() throws Exception {
public void shouldCreateSessionWhenMaxInactiveIntervalNotDefined() {

// when
this.repository.setMaxInactiveIntervalInSeconds(null);
MongoSession session = this.repository.createSession();

// then
assertThat(session.getId()).isNotEmpty();
assertThat(session.isNew()).isTrue();
assertThat(session.getMaxInactiveInterval().getSeconds())
.isEqualTo(MongoOperationsSessionRepository.DEFAULT_INACTIVE_INTERVAL);
}

@Test
public void shouldSaveSession() throws Exception {
public void shouldSaveNewSession() {

// given
MongoSession session = new MongoSession();
BasicDBObject dbSession = new BasicDBObject();

given(this.converter.convert(session,
TypeDescriptor.valueOf(MongoSession.class),
TypeDescriptor.valueOf(DBObject.class))).willReturn(dbSession);

// when
this.repository.save(session);

// then
verify(this.mongoOperations).save(dbSession, MongoOperationsSessionRepository.DEFAULT_COLLECTION_NAME);

assertThat(session.isNew()).isFalse();
}

@Test
public void shouldGetSession() throws Exception {
public void shouldHandleInvalidatedSession() {

MongoSession session = new MongoSession();
session.setNew(false);

assertThatIllegalStateException().isThrownBy(() -> {
this.repository.save(session);
}).withMessage("Session was invalidated");
}

@Test
public void shouldSaveExistingSession() {

// given
MongoSession session = new MongoSession();
session.setNew(false);
BasicDBObject dbSession = new BasicDBObject();

Document sessionDocument = new Document();

given(this.mongoOperations.findById(session.getId(), Document.class,
MongoOperationsSessionRepository.DEFAULT_COLLECTION_NAME)).willReturn(sessionDocument);

given(this.converter.convert(session,
TypeDescriptor.valueOf(MongoSession.class),
TypeDescriptor.valueOf(DBObject.class))).willReturn(dbSession);

// when
this.repository.save(session);

// then
verify(this.mongoOperations).save(dbSession, MongoOperationsSessionRepository.DEFAULT_COLLECTION_NAME);
}

@Test
public void shouldGetSession() {

// given
String sessionId = UUID.randomUUID().toString();
Document sessionDocument = new Document();
Expand All @@ -128,7 +173,8 @@ public void shouldGetSession() throws Exception {
}

@Test
public void shouldHandleExpiredSession() throws Exception {
public void shouldHandleExpiredSession() {

// given
String sessionId = UUID.randomUUID().toString();
Document sessionDocument = new Document();
Expand All @@ -152,7 +198,8 @@ public void shouldHandleExpiredSession() throws Exception {
}

@Test
public void shouldDeleteSession() throws Exception {
public void shouldDeleteSession() {

// given
String sessionId = UUID.randomUUID().toString();

Expand All @@ -175,7 +222,8 @@ public void shouldDeleteSession() throws Exception {
}

@Test
public void shouldGetSessionsMapByPrincipal() throws Exception {
public void shouldGetSessionsMapByPrincipal() {

// given
String principalNameIndexName = FindByIndexNameSessionRepository.PRINCIPAL_NAME_INDEX_NAME;

Expand Down Expand Up @@ -203,7 +251,8 @@ public void shouldGetSessionsMapByPrincipal() throws Exception {
}

@Test
public void shouldReturnEmptyMapForNotSupportedIndex() throws Exception {
public void shouldReturnEmptyMapForNotSupportedIndex() {

// given
String index = "some_not_supported_index_name";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import static org.mockito.BDDMockito.mock;
import static org.mockito.BDDMockito.times;
import static org.mockito.Mockito.verify;
import static org.springframework.session.data.mongo.ReactiveMongoOperationsSessionRepository.DEFAULT_COLLECTION_NAME;

import java.util.UUID;

Expand Down Expand Up @@ -84,6 +85,7 @@ public void shouldCreateSession() {
.as(StepVerifier::create)
.expectNextMatches(mongoSession -> {
assertThat(mongoSession.getId()).isNotEmpty();
assertThat(mongoSession.isNew()).isTrue();
assertThat(mongoSession.getMaxInactiveInterval().getSeconds())
.isEqualTo(ReactiveMongoOperationsSessionRepository.DEFAULT_INACTIVE_INTERVAL);
return true;
Expand All @@ -102,6 +104,7 @@ public void shouldCreateSessionWhenMaxInactiveIntervalNotDefined() {
.as(StepVerifier::create)
.expectNextMatches(mongoSession -> {
assertThat(mongoSession.getId()).isNotEmpty();
assertThat(mongoSession.isNew()).isTrue();
assertThat(mongoSession.getMaxInactiveInterval().getSeconds())
.isEqualTo(ReactiveMongoOperationsSessionRepository.DEFAULT_INACTIVE_INTERVAL);
return true;
Expand All @@ -114,8 +117,12 @@ public void shouldSaveSession() {

// given
MongoSession session = new MongoSession();
Document sessionDocument = new Document();
BasicDBObject dbSession = new BasicDBObject();

given(this.mongoOperations.findById(session.getId(), Document.class,
DEFAULT_COLLECTION_NAME)).willReturn(Mono.just(sessionDocument));

given(this.converter.convert(session,
TypeDescriptor.valueOf(MongoSession.class),
TypeDescriptor.valueOf(DBObject.class))).willReturn(dbSession);
Expand All @@ -127,7 +134,30 @@ public void shouldSaveSession() {
.as(StepVerifier::create)
.verifyComplete();

verify(this.mongoOperations).save(dbSession, ReactiveMongoOperationsSessionRepository.DEFAULT_COLLECTION_NAME);
assertThat(session.isNew()).isFalse();
verify(this.mongoOperations).save(dbSession, DEFAULT_COLLECTION_NAME);
verifyNoMoreInteractions(this.mongoOperations);
}

@Test
public void shouldCreateAnErrorWhenSavingSessionNotInMongo() {

// given
MongoSession session = new MongoSession();
session.setNew(false);

given(this.mongoOperations.findById(session.getId(), Document.class,
DEFAULT_COLLECTION_NAME)).willReturn(Mono.empty());

// when
this.repository.save(session)
.as(StepVerifier::create)
.verifyErrorMessage("Session was invalidated");

assertThat(session.isNew()).isFalse();

verify(this.mongoOperations).findById(session.getId(), Document.class, DEFAULT_COLLECTION_NAME);
verifyNoMoreInteractions(this.mongoOperations);
}

@Test
Expand All @@ -138,7 +168,7 @@ public void shouldGetSession() {
Document sessionDocument = new Document();

given(this.mongoOperations.findById(sessionId, Document.class,
ReactiveMongoOperationsSessionRepository.DEFAULT_COLLECTION_NAME)).willReturn(Mono.just(sessionDocument));
DEFAULT_COLLECTION_NAME)).willReturn(Mono.just(sessionDocument));

MongoSession session = new MongoSession();

Expand All @@ -160,9 +190,9 @@ public void shouldHandleExpiredSession() {
Document sessionDocument = new Document();

given(this.mongoOperations.findById(sessionId, Document.class,
ReactiveMongoOperationsSessionRepository.DEFAULT_COLLECTION_NAME)).willReturn(Mono.just(sessionDocument));
DEFAULT_COLLECTION_NAME)).willReturn(Mono.just(sessionDocument));

given(this.mongoOperations.remove(sessionDocument, ReactiveMongoOperationsSessionRepository.DEFAULT_COLLECTION_NAME))
given(this.mongoOperations.remove(sessionDocument, DEFAULT_COLLECTION_NAME))
.willReturn(Mono.just(DeleteResult.acknowledged(1)));

MongoSession session = mock(MongoSession.class);
Expand All @@ -177,8 +207,7 @@ public void shouldHandleExpiredSession() {
.verifyComplete();

// then
verify(this.mongoOperations).remove(any(Document.class),
eq(ReactiveMongoOperationsSessionRepository.DEFAULT_COLLECTION_NAME));
verify(this.mongoOperations).remove(any(Document.class), eq(DEFAULT_COLLECTION_NAME));
}

@Test
Expand All @@ -189,7 +218,7 @@ public void shouldDeleteSession() {
Document sessionDocument = new Document();

given(this.mongoOperations.findById(sessionId, Document.class,
ReactiveMongoOperationsSessionRepository.DEFAULT_COLLECTION_NAME)).willReturn(Mono.just(sessionDocument));
DEFAULT_COLLECTION_NAME)).willReturn(Mono.just(sessionDocument));

given(this.mongoOperations.remove(sessionDocument, "sessions"))
.willReturn(Mono.just(DeleteResult.acknowledged(1)));
Expand All @@ -204,9 +233,7 @@ public void shouldDeleteSession() {
.as(StepVerifier::create)
.verifyComplete();

verify(this.mongoOperations).remove(any(Document.class),
eq(ReactiveMongoOperationsSessionRepository.DEFAULT_COLLECTION_NAME));

verify(this.mongoOperations).remove(any(Document.class), eq(DEFAULT_COLLECTION_NAME));
verify(this.eventPublisher).publishEvent(any(SessionDeletedEvent.class));
}

Expand All @@ -225,5 +252,8 @@ public void shouldInvokeMethodToCreateIndexesImperatively() {
// then
verify(this.blockingMongoOperations, times(1)).indexOps((String) any());
verify(this.converter, times(1)).ensureIndexes(indexOperations);

verifyNoMoreInteractions(this.blockingMongoOperations);
verifyNoMoreInteractions(this.converter);
}
}