Skip to content

873: Migrate session to use optional #872

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alfardil merged 4 commits into from
Mar 30, 2026
Merged

873: Migrate session to use optional #872

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
214bac1
873: Migrate Session repository to use Optional
alfardil Mar 23, 2026
aaa2623
873: Fixed exception
alfardil Mar 29, 2026
52ae215
873: added throw for missing id
alfardil Mar 30, 2026
82b117e
873: removed errant files
alfardil Mar 30, 2026
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
8 changes: 5 additions & 3 deletions src/main/java/org/patinanetwork/codebloom/api/auth/AuthController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,9 @@ public RedirectView logout(final HttpServletRequest request, final HttpServletRe

Session session = authenticationObject.getSession();

boolean sessionDeleted = sessionRepository.deleteSessionById(session.getId());
String sessionId =
session.getId().orElseThrow(() -> new IllegalStateException("Authenticated session has no id"));
boolean sessionDeleted = sessionRepository.deleteSessionById(sessionId);
Comment thread
angelayu0530 marked this conversation as resolved.

if (!sessionDeleted) {
return new RedirectView("/login?success=false&message=You are not logged in.");
Expand Down Expand Up @@ -190,8 +192,8 @@ public RedirectView logoutAll(final HttpServletRequest request, final HttpServle
@Operation(
summary = "Enroll with a school email (if supported)",
description = """
Allows users to submit a school-specific email if supported. Emails will be verified with a magic link sent to their email.
""",
Allows users to submit a school-specific email if supported. Emails will be verified with a magic link sent to their email.
""",
responses = {
@ApiResponse(responseCode = "200", description = "email send successfully"),
@ApiResponse(responseCode = "500", description = "not implemented"),
Expand Down
4 changes: 2 additions & 2 deletions ...ava/org/patinanetwork/codebloom/api/auth/security/CustomAuthenticationSuccessHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,12 +178,12 @@ public void onAuthenticationSuccess(
.build();
sessionRepository.createSession(session);

if (session == null || session.getId() == null) {
if (session.getId().isEmpty()) {
response.sendRedirect("/login?success=false&message=Failed to log in.");
throw new RuntimeException("Failed to create new session.");
}

Cookie cookie = new Cookie("session_token", session.getId());
Cookie cookie = new Cookie("session_token", session.getId().get());
cookie.setMaxAge(maxAgeSeconds);

cookie.setHttpOnly(true);
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/org/patinanetwork/codebloom/common/db/models/Session.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.patinanetwork.codebloom.common.db.models;

import java.time.LocalDateTime;
import java.util.Optional;
import lombok.Builder;
import lombok.EqualsAndHashCode;
import lombok.Getter;
Expand All @@ -15,8 +16,8 @@
@ToString
public class Session {

@NotNullColumn
private String id;
@Builder.Default
private Optional<String> id = Optional.empty();
Comment thread
alfardil marked this conversation as resolved.
Comment thread
alfardil marked this conversation as resolved.

@NotNullColumn
private String userId;
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/patinanetwork/codebloom/common/db/repos/session/SessionRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.patinanetwork.codebloom.common.db.repos.session;

import java.util.ArrayList;
import java.util.Optional;
import org.patinanetwork.codebloom.common.db.models.Session;

public interface SessionRepository {
Expand All @@ -14,7 +15,7 @@ public interface SessionRepository {
*/
void createSession(Session session);

Session getSessionById(String id);
Optional<Session> getSessionById(String id);

ArrayList<Session> getSessionsByUserId(String userId);

Expand Down
28 changes: 16 additions & 12 deletions src/main/java/org/patinanetwork/codebloom/common/db/repos/session/SessionSqlRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Optional;
import java.util.UUID;
import javax.sql.DataSource;
import org.patinanetwork.codebloom.common.db.helper.NamedPreparedStatement;
Expand All @@ -22,26 +23,29 @@ public SessionSqlRepository(final DataSource ds) {

private Session parseResultSetToSession(final ResultSet resultSet) throws SQLException {
return Session.builder()
.id(resultSet.getString("id"))
.id(Optional.of(resultSet.getString("id")))
.userId(resultSet.getString("userId"))
.expiresAt(resultSet.getTimestamp("expiresAt").toLocalDateTime())
.build();
}

private void updateSessionWithResultSet(final ResultSet resultSet, final Session session) throws SQLException {
session.setId(resultSet.getString("id"));
session.setId(Optional.of(resultSet.getString("id")));
}

@Override
public void createSession(final Session session) {
String sql = "INSERT INTO \"Session\" (id, \"userId\", \"expiresAt\") VALUES (?, ?, ?) RETURNING \"id\"";
// Don't want dashes inside of the cookie, so better to just remove it from the
// ID altogether.
session.setId(UUID.randomUUID().toString().replace("-", ""));
session.setId(Optional.of(UUID.randomUUID().toString().replace("-", "")));

try (Connection conn = ds.getConnection();
PreparedStatement stmt = conn.prepareStatement(sql)) {
stmt.setString(1, session.getId());
stmt.setString(
1,
session.getId()
.orElseThrow(() -> new IllegalStateException("Session ID must be present for insertion.")));
stmt.setObject(2, UUID.fromString(session.getUserId()));
Comment thread
alfardil marked this conversation as resolved.
stmt.setObject(3, session.getExpiresAt());

Expand All @@ -56,16 +60,16 @@ public void createSession(final Session session) {
}

@Override
public Session getSessionById(final String id) {
Session session = null;
public Optional<Session> getSessionById(final String id) {
Optional<Session> session = Optional.empty();
String sql = "SELECT id, \"userId\", \"expiresAt\" FROM \"Session\" WHERE id=?";

try (Connection conn = ds.getConnection();
PreparedStatement stmt = conn.prepareStatement(sql)) {
stmt.setString(1, id);
try (ResultSet rs = stmt.executeQuery()) {
if (rs.next()) {
return parseResultSetToSession(rs);
return Optional.of(parseResultSetToSession(rs));
}
}
} catch (SQLException e) {
Expand Down Expand Up @@ -113,11 +117,11 @@ public boolean deleteSessionById(final String id) {
@Override
public boolean deleteSessionsByUserId(final String userId) {
String sql = """
DELETE FROM
"Session"
WHERE
"userId" = :userId
""";
DELETE FROM
"Session"
WHERE
"userId" = :userId
""";

try (Connection conn = ds.getConnection();
NamedPreparedStatement stmt = new NamedPreparedStatement(conn, sql)) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/patinanetwork/codebloom/common/dto/session/SessionDto.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public class SessionDto {

public static SessionDto fromSession(final Session session) {
return SessionDto.builder()
.id(session.getId())
.id(session.getId().orElseThrow())
.userId(session.getUserId())
.expiresAt(session.getExpiresAt())
.build();
Comment thread
alfardil marked this conversation as resolved.
Comment thread
alfardil marked this conversation as resolved.
Expand Down
7 changes: 5 additions & 2 deletions src/main/java/org/patinanetwork/codebloom/common/security/Protector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import jakarta.servlet.http.Cookie;
import jakarta.servlet.http.HttpServletRequest;
import java.time.LocalDateTime;
import java.util.Optional;
import org.patinanetwork.codebloom.common.db.models.Session;
import org.patinanetwork.codebloom.common.db.models.user.User;
import org.patinanetwork.codebloom.common.db.repos.session.SessionRepository;
Expand Down Expand Up @@ -38,12 +39,14 @@ public AuthenticationObject validateSession(final HttpServletRequest request) {
if ("session_token".equals(cookie.getName()) && !cookie.getValue().isEmpty()) {
String sessionToken = cookie.getValue();

Session session = sessionRepository.getSessionById(sessionToken);
Optional<Session> optSession = sessionRepository.getSessionById(sessionToken);

if (session == null) {
if (optSession.isEmpty()) {
throw new ResponseStatusException(HttpStatus.UNAUTHORIZED, "Unauthorized");
}

Session session = optSession.get();

LocalDateTime now = StandardizedLocalDateTime.now();

if (session.getExpiresAt().isBefore(now)) {
Expand Down
11 changes: 6 additions & 5 deletions src/test/java/org/patinanetwork/codebloom/api/auth/AuthControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import jakarta.servlet.http.HttpServletResponse;
import java.time.Duration;
import java.time.LocalDateTime;
import java.util.Optional;
import java.util.UUID;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
Expand Down Expand Up @@ -96,7 +97,7 @@ private User createRandomUser() {

private Session createRandomSession(final String userId) {
return Session.builder()
.id(UUID.randomUUID().toString().replace("-", ""))
.id(Optional.of(UUID.randomUUID().toString().replace("-", "")))
.userId(userId)
.expiresAt(LocalDateTime.now().plusDays(1))
.build();
Expand Down Expand Up @@ -135,15 +136,15 @@ void logoutHappyPath() {
HttpServletResponse response = mock(HttpServletResponse.class);

when(protector.validateSession(request)).thenReturn(authObj);
when(sessionRepository.deleteSessionById(session.getId())).thenReturn(true);
when(sessionRepository.deleteSessionById(session.getId().orElseThrow())).thenReturn(true);

RedirectView redirectView = authController.logout(request, response);

assertNotNull(redirectView);
assertEquals("/login?success=true&message=You have been logged out!", redirectView.getUrl());

verify(protector, times(1)).validateSession(request);
verify(sessionRepository, times(1)).deleteSessionById(session.getId());
verify(sessionRepository, times(1)).deleteSessionById(session.getId().orElseThrow());
}

@Test
Expand All @@ -157,15 +158,15 @@ void logoutSessionNotFound() {
HttpServletResponse response = mock(HttpServletResponse.class);

when(protector.validateSession(request)).thenReturn(authObj);
when(sessionRepository.deleteSessionById(session.getId())).thenReturn(false);
when(sessionRepository.deleteSessionById(session.getId().orElseThrow())).thenReturn(false);

RedirectView redirectView = authController.logout(request, response);

assertNotNull(redirectView);
assertEquals("/login?success=false&message=You are not logged in.", redirectView.getUrl());

verify(protector, times(1)).validateSession(request);
verify(sessionRepository, times(1)).deleteSessionById(session.getId());
verify(sessionRepository, times(1)).deleteSessionById(session.getId().orElseThrow());
}

@Test
Expand Down
18 changes: 9 additions & 9 deletions ...org/patinanetwork/codebloom/api/auth/security/CustomAuthenticationSuccessHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ void updatesNameAndSetsCookie() throws Exception {
when(userRepository.getUserByDiscordId(DISCORD_ID)).thenReturn(existingUser);
doAnswer(inv -> {
Session s = inv.getArgument(0);
s.setId("session-abc");
s.setId(Optional.of("session-abc"));
return null;
})
.when(sessionRepository)
Expand Down Expand Up @@ -121,7 +121,7 @@ void updatesProfileUrl() throws Exception {
when(leetcodeClient.getUserProfile("leet_user")).thenReturn(profile);
doAnswer(inv -> {
Session s = inv.getArgument(0);
s.setId("session-xyz");
s.setId(Optional.of("session-xyz"));
return null;
})
.when(sessionRepository)
Expand All @@ -146,7 +146,7 @@ void survivesLeetcodeLookupFailure() throws Exception {
when(leetcodeClient.getUserProfile("bad_user")).thenThrow(new RuntimeException("API down"));
doAnswer(inv -> {
Session s = inv.getArgument(0);
s.setId("session-fail-safe");
s.setId(Optional.of("session-fail-safe"));
return null;
})
.when(sessionRepository)
Expand All @@ -165,7 +165,7 @@ void createsUserAndAddsToLeaderboard() throws Exception {
when(leaderboardRepository.getRecentLeaderboardMetadata()).thenReturn(Optional.of(lb));
doAnswer(inv -> {
Session s = inv.getArgument(0);
s.setId("new-session-id");
s.setId(Optional.of("new-session-id"));
return null;
})
.when(sessionRepository)
Expand Down Expand Up @@ -194,7 +194,7 @@ void assignsTagForGuildMember() throws Exception {
when(userRepository.getUserByDiscordId(DISCORD_ID)).thenReturn(existingUser);
doAnswer(inv -> {
Session s = inv.getArgument(0);
s.setId("s-club");
s.setId(Optional.of("s-club"));
return null;
})
.when(sessionRepository)
Expand Down Expand Up @@ -236,7 +236,7 @@ void skipsExistingTag() throws Exception {
when(userRepository.getUserByDiscordId(DISCORD_ID)).thenReturn(existingUser);
doAnswer(inv -> {
Session s = inv.getArgument(0);
s.setId("s-club");
s.setId(Optional.of("s-club"));
return null;
})
.when(sessionRepository)
Expand Down Expand Up @@ -277,7 +277,7 @@ void setsNicknameForPatinaClub() throws Exception {
when(userRepository.getUserByDiscordId(DISCORD_ID)).thenReturn(existingUser);
doAnswer(inv -> {
Session s = inv.getArgument(0);
s.setId("s-club");
s.setId(Optional.of("s-club"));
return null;
})
.when(sessionRepository)
Expand Down Expand Up @@ -318,7 +318,7 @@ void fallsBackToGlobalNameForPatina() throws Exception {
when(userRepository.getUserByDiscordId(DISCORD_ID)).thenReturn(existingUser);
doAnswer(inv -> {
Session s = inv.getArgument(0);
s.setId("s-club");
s.setId(Optional.of("s-club"));
return null;
})
.when(sessionRepository)
Expand Down Expand Up @@ -362,7 +362,7 @@ void skipsClubWithNoGuildId() throws Exception {
when(userRepository.getUserByDiscordId(DISCORD_ID)).thenReturn(existingUser);
doAnswer(inv -> {
Session s = inv.getArgument(0);
s.setId("s-club");
s.setId(Optional.of("s-club"));
return null;
})
.when(sessionRepository)
Expand Down
8 changes: 8 additions & 0 deletions src/test/java/org/patinanetwork/codebloom/common/db/repos/BaseRepositoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import org.patinanetwork.codebloom.common.email.client.github.GithubOAuthEmailClient;
import org.patinanetwork.codebloom.jda.JDAClientManager;
import org.patinanetwork.codebloom.jda.command.JDASlashCommandInitializer;
import org.patinanetwork.codebloom.scheduled.auth.LeetcodeAuthStealer;
import org.patinanetwork.codebloom.scheduled.submission.SubmissionScheduler;
import org.springframework.test.context.bean.override.mockito.MockitoBean;

/**
Expand All @@ -24,4 +26,10 @@ public class BaseRepositoryTest {

@MockitoBean
private GithubOAuthEmailClient githubOAuthEmailClient;

@MockitoBean
private SubmissionScheduler submissionScheduler;

@MockitoBean
private LeetcodeAuthStealer leetcodeAuthStealer;
}
20 changes: 12 additions & 8 deletions src/test/java/org/patinanetwork/codebloom/common/db/repos/session/SessionRepositoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public class SessionRepositoryTest extends BaseRepositoryTest {

private SessionRepository sessionRepository;
private Session testSession;
private String mockUserId = "ed3bfe18-e42a-467f-b4fa-07e8da4d2555";
private String mockUserId = "0c9b2e77-74cc-4b9e-b7f9-cfe0fd05e50b";

@Autowired
public SessionRepositoryTest(final SessionRepository sessionRepository) {
Expand All @@ -44,25 +44,28 @@ void createSession() {
.build();

sessionRepository.createSession(testSession);
log.info("Created test session with ID: {}", testSession.getId());
log.info("Created test session with ID: {}", testSession.getId().get());
}

@AfterAll
void deleteSession() {
boolean isSuccessful = sessionRepository.deleteSessionById(testSession.getId());
String sessionId = testSession.getId().get();
log.info("The test session to be deleted has an id of {}", sessionId);
boolean isSuccessful = sessionRepository.deleteSessionById(sessionId);

if (!isSuccessful) {
fail("Failed to delete test announcement");
} else {
log.info("Deleted test session with ID: {}", testSession.getId());
log.info("Deleted test session with ID: {}", sessionId);
}
}

@Test
void testGetSessionById() {
Session found = sessionRepository.getSessionById(testSession.getId());
String sessionId = testSession.getId().get();
Session found = sessionRepository.getSessionById(sessionId).get();
assertNotNull(found);
assertEquals(testSession.getId(), found.getId());
assertEquals(testSession.getId().get(), found.getId().get());
}

@Test
Expand All @@ -82,9 +85,10 @@ void testDeleteSessionById() {

sessionRepository.createSession(tempSession);

boolean isSuccessful = sessionRepository.deleteSessionById(tempSession.getId());
String sessionId = tempSession.getId().get();
boolean isSuccessful = sessionRepository.deleteSessionById(sessionId);
assertTrue(isSuccessful);
log.info("Deleted session with ID: {}", tempSession.getId());
log.info("Deleted session with ID: {}", tempSession.getId().get());
}

@Test
Expand Down
Loading
Loading