Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4243d227 authored by Jonathan Klee's avatar Jonathan Klee
Browse files

refactor: add AI suggestion docs/login-architecture-refactor.md

parent f9173d6c
Loading
Loading
Loading
Loading
Loading
+295 −0
Original line number Diff line number Diff line
# Login Architecture Review And Refactor Proposal

This note summarizes the current login architecture, why it feels confusing, and a concrete target structure that is easier to reason about.

## Current Architecture

```text
UI
  SignInFragment
  GoogleSignInFragment
      |
      v
  LoginViewModel
      |
      +--> AnonymousLoginUseCase
      +--> GoogleLoginUseCase
      +--> MicrogLoginUseCase
      +--> NoGoogleLoginUseCase
      +--> LogoutUseCase
      +--> StoreAuthCoordinator
      +--> MicrogAccountFetcher
      |
      v
Domain
  LoginRepository
      |
      v
Data
  AppLoginRepository
      +--> Stores
      +--> SessionRepository
      +--> PlayStoreAuthStore

  AuthenticatorRepository
      +--> List<StoreAuthenticator>
      +--> PlayStoreAuthStore
      +--> SessionRepository

  PlayStoreAuthenticator
      +--> GoogleLoginManager
      +--> MicrogLoginManager
      +--> AnonymousLoginManager
      +--> OauthToAasTokenConverter
      +--> UserProfileFetcher
      +--> AuthDataCache

  CleanApkAuthenticator
```

## What Feels Wrong

### 1. The feature mixes three responsibilities

- Initial login mode setup:
  - anonymous
  - google
  - microG
  - no-google
- Runtime store authentication:
  - fetch auth objects
  - refresh credentials
  - persist auth data
- UI sign-in flow orchestration:
  - account picker
  - consent intent
  - WebView-based Google sign-in

Those are separate concerns, but they are currently spread across the same feature surface.

### 2. `LoginViewModel` is too broad

[LoginViewModel](/home/jonathanklee/murena/apps/app/src/main/java/foundation/e/apps/ui/LoginViewModel.kt#L48) does all of these:

- starts auth object loading
- owns selected microG account state
- performs initial login setup
- handles microG error mapping
- triggers follow-up refresh with `startLoginFlow()`
- invalidates auth objects
- clears HTTP cache on auth invalidation

That is too much orchestration for one view model.

### 3. The naming hides orchestration

- [AppLoginRepository](/home/jonathanklee/murena/apps/app/src/main/java/foundation/e/apps/data/login/repository/AppLoginRepository.kt#L32) is not really a repository in the usual sense. It is closer to a `LoginSetupService`.
- [AuthenticatorRepository](/home/jonathanklee/murena/apps/app/src/main/java/foundation/e/apps/data/login/repository/AuthenticatorRepository.kt#L35) is not really a repository either. It is closer to a `StoreAuthenticationService` or `StoreAuthCoordinatorImpl`.

### 4. The domain layer is mostly pass-through

The use cases under `domain/login` mainly forward directly into `LoginRepository`, so the real behavior still lives in data/app code. The domain layer exists, but it is not the place where the login decisions actually happen.

### 5. `PlayStoreAuthenticator` is a god object

[PlayStoreAuthenticator](/home/jonathanklee/murena/apps/app/src/main/java/foundation/e/apps/data/login/playstore/PlayStoreAuthenticator.kt#L50) currently handles:

- deciding which auth source to use
- choosing between Google and microG paths
- AAS token conversion fallback
- OAuth fallback
- cached auth reuse
- profile enrichment
- auth result construction

That is too much policy and too much branching in one class.

## Target Architecture

The simplest improvement is to split the current feature into three explicit services.

```text
UI
  SignInFragment
  GoogleSignInFragment
      |
      v
  SignInFlowViewModel
      +--> SignInFlowController
      +--> LoginSetupUseCase
      +--> RefreshStoreSessionsUseCase

Domain
  LoginSetupUseCase
  RefreshStoreSessionsUseCase
  LogoutUseCase

  LoginSetupGateway
  StoreSessionGateway
  MicrogAccountGateway

Data
  LoginSetupService
      +--> Stores
      +--> SessionRepository
      +--> PlayStoreAuthStore

  StoreSessionService
      +--> List<StoreAuthenticator>
      +--> PlayStoreAuthStore
      +--> SessionRepository

  MicrogAccountService
      +--> AccountManager
      +--> PlayStoreAuthStore

  PlayStoreAuthenticator
      +--> AuthSourceResolver
      +--> GoogleAuthProvider
      +--> MicrogAuthProvider
      +--> AnonymousAuthProvider
      +--> AuthDataCache
      +--> UserProfileFetcher

  CleanApkAuthenticator
```

## Proposed Structure

### UI / app layer

- `SignInFlowViewModel`
  - owns UI state only
  - exposes events/results for navigation, dialogs, and consent/account-picker handoff
- `SignInFlowController`
  - handles UI-specific login flow branching
  - turns account-picker or WebView outputs into domain calls

### Domain layer

- `LoginSetupUseCase`
  - set initial user mode and required persisted login inputs
- `RefreshStoreSessionsUseCase`
  - load or refresh store auth for active stores
- `LogoutUseCase`
  - clear user/session state

Interfaces:

- `LoginSetupGateway`
- `StoreSessionGateway`
- `MicrogAccountGateway`

This makes the domain language match the actual use cases instead of hiding everything behind `LoginRepository`.

### Data layer

- `LoginSetupService`
  - current `AppLoginRepository` behavior
- `StoreSessionService`
  - current `AuthenticatorRepository` behavior
- `MicrogAccountService`
  - current `MicrogLoginManager` account-fetching behavior

Split `PlayStoreAuthenticator` into smaller collaborators:

- `AuthSourceResolver`
  - decides `GOOGLE` vs `MICROG`
- `GoogleAuthProvider`
  - Google backend login path
- `MicrogAuthProvider`
  - microG token login path
- `AnonymousAuthProvider`
  - anonymous dispenser login path

`PlayStoreAuthenticator` then becomes a coordinator over those parts instead of owning every branch itself.

## Refactor Plan

### Phase 1: Rename by responsibility

No behavior change.

- `AppLoginRepository` -> `LoginSetupService`
- `AuthenticatorRepository` -> `StoreSessionService`
- `LoginViewModel` -> `SignInFlowViewModel` or split it first

This alone would reduce confusion.

### Phase 2: Split `LoginViewModel`

Move out:

- microG account fetch/orchestration
- auth object invalidation policy
- cache eviction side effects

Keep in the view model:

- UI state
- navigation events
- calling use cases

### Phase 3: Replace `LoginRepository` with clearer contracts

Instead of one broad contract:

- `LoginSetupGateway`
- `StoreSessionGateway`
- `SessionTerminationGateway`

This reduces the false idea that “login” is one thing.

### Phase 4: Break up `PlayStoreAuthenticator`

Extract:

- `AuthSourceResolver`
- `GoogleAuthProvider`
- `MicrogAuthProvider`
- `AnonymousAuthProvider`
- optional `AuthResultFactory`

That leaves `PlayStoreAuthenticator` as a thin coordinator.

### Phase 5: Make auth refresh explicit

Right now, initial sign-in and subsequent auth refresh are coupled through `startLoginFlow()`.

Split them into two explicit actions:

- `completeInitialLogin()`
- `refreshActiveStoreSessions()`

That makes the app behavior easier to follow and test.

## What I Would Change First

If you want the best payoff with the least churn, I would do this first:

1. Rename `AppLoginRepository` and `AuthenticatorRepository` to reflect orchestration, not storage.
2. Split `LoginViewModel` into:
   - `SignInFlowViewModel`
   - `StoreSessionViewModel` or a separate auth-refresh coordinator
3. Extract `AuthSourceResolver` from `PlayStoreAuthenticator`.

That gets most of the clarity benefit without forcing a full rewrite.

## Bottom Line

The current design is not fundamentally broken, but it is overloaded and misnamed.

The confusion comes from:

- one feature name covering too many responsibilities
- orchestration hidden behind repository names
- UI flow logic mixed with login setup and store session refresh
- one very large Play Store authenticator deciding too much

The target shape should be:

- UI flow orchestration
- login setup
- store session refresh

as three separate concepts with names that match their responsibilities.