ankidroid / ankidroid/Anki-Android

Architecture Discussion: Module Extraction

Đang mở
#20,737 12 bình luận 0 reaction 0 người được giao Xem trên GitHub
Keep Open
Ngôn ngữ chính
Kotlin
Star
11.8k
Fork
2.9k
Merge trung bình
2 ngày 3 giờ
Pull request đã merge (30 ngày)
171

Mô tả

## 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.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.