ankidroid / ankidroid/Anki-Android

Architecture Discussion: Module Extraction

Ouverte
#20,737 12 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
Keep Open
Langage dominant
Kotlin
Étoiles
11.8k
Forks
2.9k
Merge moyen
2 j 3 h
PR mergées (30 j)
171

Description

## Architecture: Multi-module structure for AnkiDroid

### Context

We want AnkiDroid to move to a multi-module structure. `:widgets` is our first feature module extraction target.

**Current module layout:**

```
:AnkiDroid (app) → :common, :compat, :libanki, :vbpd, :api, :widgets
:widgets → :common, :libanki
:compat → :common
:libanki → :common
```

**Current `:widgets` progress**:

- 18 files moved to `:widgets` (infrastructure, preferences, bridge interfaces, business logic)
- 7 files remain in `:AnkiDroid` (4 widget providers blocked by `R` resources, 2 config activities + 1 adapter blocked by `AnkiActivity`/dialogs/databinding)
- 7 bridge interfaces in `:widgets` to abstract `:AnkiDroid` dependencies

### Problem: Bridge interfaces per feature module don't scale

To break circular dependencies between `:widgets` and `:AnkiDroid`, we created 7 bridge interfaces inside the `:widgets` module:

| Bridge in `:widgets` | Wraps dependency in `:AnkiDroid` |
|---|---|
| `WidgetAnalytics` | `UsageAnalytics.sendAnalyticsEvent()` |
| `WidgetCrashReporter` | `CrashReportService.sendExceptionReport()` |
| `WidgetCollectionAccess` | `CollectionManager.withCol {}` |
| `WidgetIntentFactory` | `IntentHandler`, `NoteEditorLauncher`, `DeckOptionsDestination` |
| `WidgetAppState` | `AnkiDroidApp` (scope, sdcard, instance) |
| `WidgetMetaStorage` | `MetaDB` widget operations |
| `WidgetPreferences` | `sharedPrefs()`, `Prefs` |

Each bridge requires an interface in `:widgets` + an implementation class in `:AnkiDroid` + wiring in `AnkiDroidApp.onCreate()`.

**If every feature module does this, we'd get:**

```
:widgets → WidgetAnalytics, WidgetCrashReporter, WidgetCollectionAccess...
:browser → BrowserAnalytics, BrowserCrashReporter, BrowserCollectionAccess...
:reviewer → ReviewerAnalytics, ReviewerCrashReporter, ReviewerCollectionAccess...
```

Duplicated interfaces with different prefixes, each needing its own `XImpl` in `:AnkiDroid`. This doesn't scale.

### Proposed approach: Extract dependencies to lower modules

Instead of creating per-feature bridge interfaces, move the dependencies themselves (or their interfaces) to `:common` or `:libanki`. Feature modules then use them directly — no bridges, no per-module boilerplate.

**Example — Analytics (already extracted to `:common`):**

```kotlin
// :common — interface defined once
interface UsageAnalytics {
fun sendAnalyticsEvent(category: String, action: String, value: Int? = null, label: String? = null)
fun sendAnalyticsScreenView(screenName: String)
}

// :common — short accessor object
object Analytics {
fun setAnalytics(analytics: UsageAnalytics) { ... }
fun sendAnalyticsEvent(...) = instance.sendAnalyticsEvent(...)
}

// :AnkiDroid — implementation (internal, invisible to other modules)
internal object AnkiDroidUsageAnalytics : UsageAnalytics { ... }

// AnkiDroidApp.onCreate()
Analytics.setAnalytics(AnkiDroidUsageAnalytics)

// :widgets or any module — just use it
Analytics.sendAnalyticsEvent("CardAnalysisWidget", "enabled")
```

One interface. One implementation. Used by every module. No `WidgetAnalytics`, no `BrowserAnalytics`.

This replaces `WidgetAnalytics` bridge and its `WidgetAnalyticsImpl` — we can delete both.

### Per-dependency analysis

#### Dependencies that should be extracted (used across the entire app)

These are used by 6 out of 7 bridge interfaces. Extracting them benefits all future feature modules, not just `:widgets`.

| Dependency | Current location | Used by (files) | Proposed home | Approach |
|---|---|---|---|---|
| `UsageAnalytics` | `:AnkiDroid` | ~15 | `:common` | **Done** — interface in `:common`, `internal` impl in `:AnkiDroid` |
| `CrashReportService` | `:AnkiDroid` | 49 | `:common` | Same pattern — `CrashReporter` interface in `:common` |
| `sharedPrefs()` | `:AnkiDroid` | 51 | `:common` | **Done** — moved function to `:common`(locally) |
| `AnkiDroidApp.applicationScope` | `:AnkiDroid` | ~20 | `:common` | Provide `AppScope` object in `:common` with a settable `CoroutineScope` |
| `AnkiDroidApp.isSdCardMounted` | `:AnkiDroid` | ~5 | `:common` | One-line utility, move to `:common` |

After extracting these, the `WidgetAnalytics`, `WidgetCrashReporter`, `WidgetPreferences`, and `WidgetAppState` bridges can be **deleted**.

#### Dependencies that need further discussion (separate issues)

| Dependency | Why it's complex | Replaces bridge |
|---|---|---|
| **`CollectionManager` / `withCol`** | Core architectural component. Depends on `:libanki` backend. Candidate for `:libanki:ext[:android]`. Needs its own design discussion. | `WidgetCollectionAccess` |
| **String resources** (`R.string.*`, `R.layout.*`) | Widget providers need `RemoteViews` with layout/string resources at runtime. Options: move widget resources to `:widgets`, shared `:resources` module, or keep widget providers in `:AnkiDroid`. Intersects with Crowdin localization pipeline. | N/A (blocks file moves, not a bridge) |

#### Dependencies where a bridge interface is appropriate

| Dependency | Why bridge is OK | Bridge |
|---|---|---|
| `MetaDB` (widget status storage) | Only used by widgets. Other feature modules won't need `storeSmallWidgetStatus()`. A widget-specific interface is fine here. | `WidgetMetaStorage` |
| `IntentHandler` (navigation intents) | Creates `Intent`s to specific Activities (`IntentHandler::class.java`, `DeckOptionsDestination`). Navigation is inherently app-level. Could evolve into a shared navigation component later, but a bridge is pragmatic for now. | `WidgetIntentFactory` |

### Impact on `:widgets` after extractions

| After extracting... | Bridges we can delete | Files unblocked |
|---|---|---|
| `CrashReportService` → `:common` | `WidgetCrashReporter` + impl | — |
| `applicationScope` → `:common` | `WidgetAppState` (partially) | — |
| `CollectionManager` → `:libanki:ext` | `WidgetCollectionAccess` + impl | — |
| String resources → `:widgets` or `:resources` | — | `AddNoteWidget`, `AnkiDroidWidgetSmall`, `CardAnalysisWidget`, `DeckPickerWidget` (4 files) |
| All of the above | 5 of 7 bridges deleted | All widget files movable |

### Open questions for the team

1. **What should `:common` be?**
Proposal: the core of the system. All cross-module concerns live here — analytics, crash reporting, preferences, scopes, shared utilities. No Anki-specific business logic.

2. **Should `:common` sit below `:libanki`?**
Current: `:libanki → :common`. Proposal: cross-module concerns that depend on `:libanki` (e.g., `CollectionManager`) go in `:libanki:ext[:android]`, not in `:common`.

3. **How does a new developer know where to put a file?**
| You're writing... | Put it in... |
|---|---|
| UI (Activity, Fragment, layout) | `:AnkiDroid` or feature module |
| Cross-module service interface | `:common` |
| Cross-module service implementation | `:AnkiDroid` (mark `internal`) |
| Shared utility with no Anki knowledge | `:common` |
| Anki collection/backend operation | `:libanki` or `:libanki:ext` |

4. **How do we prevent coupling to concrete implementations?**
Proposal: implementations are `internal` to their module. Only interfaces + accessor objects from `:common` are public. This is compile-enforced — `internal` classes are invisible to other modules.

### Out of scope (separate issues)

- [ ] **`CollectionManager` extraction** — moving to `:libanki:ext[:android]`. Large scope, needs its own design. Currently abstracted via `WidgetCollectionAccess` bridge.
- [ ] **String resource strategy** — shared `:resources` module vs per-feature resources vs keeping in `:AnkiDroid`. Impacts Crowdin pipeline. Blocks moving 4 widget provider files.
- [ ] **Navigation architecture** — how feature modules create Intents to Activities in other modules. Currently handled by `WidgetIntentFactory` bridge.
- [ ] **Testing infrastructure** — `RobolectricTest` base class is in `:AnkiDroid`, but feature module tests need it. Blocks moving test files.

Guide de contribution

Ouvrir le guide de contribution

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.