Skip to content

refactor: merge session classes into repository - #1651

Merged
aanorbel merged 2 commits into
issues/1486-update-descriptorfrom
issues/1486-session-update
Nov 4, 2026
Merged

aanorbel merged 2 commits into
issues/1486-update-descriptorfrom
issues/1486-session-update

Conversation

@aanorbel

@aanorbel aanorbel commented Nov 2, 2026

Copy link
Copy Markdown
Member

No description provided.

@aanorbel
aanorbel requested a review from sdsantos November 2, 2026 10:45
Comment on lines +6 to +8
data class Error(
val cause: Throwable? = null,
) : SessionState

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I feel this is just the same as LoggedOut, but with a Throwable that is never used. This could be removed and, if necessary, the errorCause could be inside LoggedOut.


val author = descriptor.author
val sessionEmail = getStoredSession()?.emailAddress
val sessionEmail = getSession()?.emailAddress

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should react to the session state flow here, instead of just getting it once no?

package org.ooni.probe.data.models

sealed interface SessionState {
data object Unknown : SessionState

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Never used.

Comment on lines +47 to +50
suspend fun getSession(): AuthSession? = (sessionState.first() as? SessionState.LoggedIn)?.session

suspend fun requireSession(): Result<AuthSession, AuthException> =
getSession()?.let { Success(it) } ?: Failure(AuthException.NotLoggedIn())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I feel these methods are unnecessary. We could just use the session state flow everywhere, instead of keeping these small differences. Specially since the Error and Unknown states could be removed.

@aanorbel
aanorbel merged commit 156f8d7 into issues/1486-update-descriptor Nov 4, 2026
16 of 18 checks passed
@aanorbel
aanorbel deleted the issues/1486-session-update branch November 4, 2026 07:35
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.

2 participants