Skip to content

873: Migrate session to use optional#872

Open
alfardil wants to merge 1 commit intomainfrom
873
Open

873: Migrate session to use optional#872
alfardil wants to merge 1 commit intomainfrom
873

Conversation

@alfardil
Copy link
Collaborator

@alfardil alfardil commented Mar 24, 2026

873

Description of changes

Migrate SessionRepository to use optional

Checklist before review

  • I have done a thorough self-review of the PR
  • Copilot has reviewed my latest changes, and all comments have been fixed and/or closed.
  • If I have made database changes, I have made sure I followed all the db repo rules listed in the wiki here. (check if no db changes)
  • All tests have passed
  • I have successfully deployed this PR to staging
  • I have done manual QA in both dev (and staging if possible) and attached screenshots below.

Screenshots

Dev

Staging

@github-actions
Copy link
Contributor

Available PR Commands

  • /ai - Triggers all AI review commands at once
  • /review - AI review of the PR changes
  • /describe - AI-powered description of the PR
  • /improve - AI-powered suggestions
  • /deploy - Deploy to staging

See: https://github.com/tahminator/codebloom/wiki/CI-Commands

@github-actions
Copy link
Contributor

Title

873: migrate session to use optional


PR Type

Enhancement


Description

  • Migrate Session.id field to Optional<String>.

  • Update SessionRepository methods to return Optional<Session>.

  • Adjust session creation and retrieval logic to handle Optional.

  • Refactor dependent components and tests for Optional usage.


Diagram Walkthrough

flowchart LR
  A["Session.java (Model)"] --> B["SessionRepository.java (Interface)"]
  B --> C["SessionSqlRepository.java (Implementation)"]
  B --> D["AuthController.java"]
  B --> E["CustomAuthenticationSuccessHandler.java"]
  B --> F["Protector.java"]
  A --> G["SessionDto.java"]
Loading

File Walkthrough

Relevant files
Enhancement
7 files
AuthController.java
Update logout to handle Optional session ID                           
+4/-3     
CustomAuthenticationSuccessHandler.java
Update cookie creation to handle Optional session ID         
+1/-1     
Session.java
Change session ID to Optional and initialize as empty
+3/-2     
SessionRepository.java
Update getSessionById signature to return Optional
+2/-1     
SessionSqlRepository.java
Implement Optional handling for session ID in repository operations
+13/-12 
SessionDto.java
Update fromSession to retrieve session ID using orElseThrow
+1/-1     
Protector.java
Update validateSession to retrieve session using orElseThrow
+1/-1     
Tests
4 files
AuthControllerTest.java
Update AuthController tests for Optional session ID handling
+6/-5     
CustomAuthenticationSuccessHandlerTest.java
Update authentication success handler tests for Optional session ID
+9/-9     
SessionRepositoryTest.java
Update SessionRepository tests for Optional session and ID handling
+7/-3     
TestProtector.java
Update TestProtector to handle Optional session retrieval
+3/-1     

@github-actions
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ No major issues detected

@github-actions
Copy link
Contributor

Title

873: migrate session to use optional


PR Type

Enhancement


Description

  • Migrate Session.id to Optional<String>.

  • Update SessionRepository to return Optional<Session>.

  • Adapt session handling in controllers and services.

  • Enhance null-safety across session-related logic.


Diagram Walkthrough

flowchart LR
  A[Old Session Model] --> B{Refactor ID to Optional};
  B --> C[New Session Model];
  C --> D{Update Session Repository};
  D --> E{Update Auth Controllers & Services};
  E --> F[Update Tests];
Loading

File Walkthrough

Relevant files
Enhancement
3 files
AuthController.java
Update logout to handle optional session ID                           
+4/-3     
CustomAuthenticationSuccessHandler.java
Adjust session ID handling for optional type                         
+2/-2     
Protector.java
Adapt session validation for `Optional` return type
+5/-2     
Data model refactoring
1 files
Session.java
Change session ID to `Optional` with default empty
+3/-2     
Interface refactoring
1 files
SessionRepository.java
Update `getSessionById` to return `Optional`       
+2/-1     
Implementation refactoring
1 files
SessionSqlRepository.java
Implement optional handling for session ID and retrieval 
+13/-12 
Data transfer object update
1 files
SessionDto.java
Update `fromSession` to extract session ID using `orElseThrow`
+1/-1     
Tests
4 files
AuthControllerTest.java
Update authentication controller tests for optional session ID
+6/-5     
CustomAuthenticationSuccessHandlerTest.java
Adjust authentication success handler tests for optional session ID
+9/-9     
SessionRepositoryTest.java
Update session repository tests to handle optional types 
+7/-3     
TestProtector.java
Update test protector configuration for optional session retrieval
+3/-1     

@github-actions
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅

Migrate-Session-SessionRepository-to-use-Optional-3267c85563aa802397aef5741f96ce6f - Fully compliant

Compliant requirements:

  • Migrate SessionRepository to use Optional for methods that might return null.
  • Update the Session model to use Optional for its id field.
  • Adjust all consuming code (controllers, services, DTOs, tests) to correctly handle Optional types.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ No major issues detected

@github-actions
Copy link
Contributor

Title

873: migrate session to use optional


PR Type

Enhancement, Tests


Description

  • Migrate Session model's id field to Optional<String>.

  • Update SessionRepository methods to return Optional<Session>.

  • Adjust authentication, DTOs, and security to handle Optional Session IDs.

  • Refactor unit and integration tests for Session type changes.


File Walkthrough

Relevant files
Enhancement
7 files
AuthController.java
Update logout to handle `Optional` `Session` ID                   
+4/-3     
CustomAuthenticationSuccessHandler.java
Adjust session creation and validation for `Optional` `Session` ID
+2/-2     
Session.java
Migrate `id` field to `Optional` with default empty value
+3/-2     
SessionRepository.java
Update `getSessionById` to return `Optional`       
+2/-1     
SessionSqlRepository.java
Implement `Optional` handling for `Session` ID in SQL operations
+13/-12 
SessionDto.java
Update `fromSession` method to extract `Session` ID from `Optional`
+1/-1     
Protector.java
Modify `validateSession` to handle `Optional` from repository
+5/-2     
Tests
4 files
AuthControllerTest.java
Update session creation and assertions to use `Optional` `Session` ID
+6/-5     
CustomAuthenticationSuccessHandlerTest.java
Adjust mock session ID setting to use `Optional`                 
+9/-9     
SessionRepositoryTest.java
Update session tests to handle `Optional` `Session` objects and IDs
+19/-8   
TestProtector.java
Adjust mock session retrieval to handle `Optional` return type
+3/-1     
Dependencies
1 files
BaseRepositoryTest.java
Add Mockito beans for scheduled tasks in base test class 
+8/-0     

@github-actions
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

Migrate-Session-SessionRepository-to-use-Optional-3267c85563aa802397aef5741f96ce6f - Partially compliant

Compliant requirements:

  • Migrate the Session model to use Optional for fields that can be null.
  • Update SessionRepository interface methods to return Optional<Session> where a session might not be found.
  • Implement the changes in SessionSqlRepository to handle Optional correctly during database operations (create, read).
  • Adjust all consuming code (e.g., AuthController, CustomAuthenticationSuccessHandler, Protector, DTOs) to correctly interact with Optional<Session> and Optional<String>.
  • Update relevant tests to reflect the Optional changes.

Non-compliant requirements:

Requires further human verification:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Unused Method

The private method updateSessionWithResultSet is defined but not called anywhere in the provided code diff. While not directly introduced by the Optional migration, it's good practice to remove unused code.

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

@github-actions
Copy link
Contributor

PR Code Suggestions ✨

No code suggestions found for the PR.

@github-actions
Copy link
Contributor

Failed to generate code suggestions for PR

@github-actions
Copy link
Contributor

Failed to generate code suggestions for PR

@alfardil alfardil changed the title 873: migrate session to use optional 873: Migrate session to use optional Mar 25, 2026
@github-actions
Copy link
Contributor

Failed to generate code suggestions for PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant