|
| 1 | +# Feature/Core API/Internal split and dependency rules |
| 2 | + |
| 3 | +- Issue: [#10197](https://github.com/thunderbird/thunderbird-android/issues/10197) |
| 4 | +- Pull Request: [#10198](https://github.com/thunderbird/thunderbird-android/pull/10198) |
| 5 | + |
| 6 | +## Status |
| 7 | + |
| 8 | +- **proposed** |
| 9 | + |
| 10 | +## Context |
| 11 | + |
| 12 | +Thunderbird for Android uses a modular architecture. Many feature and core areas are split into `api` (public contracts) |
| 13 | +and `impl` (implementation) modules, e.g. `:feature:account:settings:api` and `:feature:account:settings:impl`. |
| 14 | + |
| 15 | +Over time, the main friction was ambiguity in naming and ownership: |
| 16 | +- Inconsistent use of `api` in package names made it unclear whether a type was part of a public contract. |
| 17 | +- Lack of a clear convention for naming feature-internal packages (`impl`, `internal`, or none) led to confusion. |
| 18 | +- Team wasn’t aligned on what belongs in `api` (stable contracts) vs. what should stay internal to the feature. |
| 19 | + |
| 20 | +To improve clarity and discoverability, we rename `impl` modules to `internal` and formalize module, package, and |
| 21 | +content rules for both API and internal code — for both feature and core modules. |
| 22 | + |
| 23 | +## Decision |
| 24 | + |
| 25 | +- `:feature:*:api` modules define the public contracts exposed to other features. |
| 26 | +- `:feature:*:impl` modules are renamed to `:feature:*:internal`, marking them as private implementation details. |
| 27 | +- `:core:*:api` modules define public contracts exposed to feature and other core modules. |
| 28 | +- `:core:*:impl` modules are renamed to `:core:*:internal`, marking them as private implementation details. |
| 29 | +- Other modules must only declare dependencies on `:feature:*:api` or `:core:*:api` of other areas. Depending on |
| 30 | + `:feature:*:internal` or `:core:*:internal` from a different area is prohibited. |
| 31 | +- Binding of contracts to implementations happens in central composition modules (application assembly): `:app-common` |
| 32 | + and the app-specific modules `:app-k9mail` and `:app-thunderbird`. |
| 33 | + |
| 34 | +### What goes into API vs. Internal |
| 35 | + |
| 36 | +Put only stable, intentionally shared contracts in `api`: |
| 37 | +- Public interfaces and abstractions other features depend on (e.g., repositories, use cases, service interfaces). |
| 38 | +- Data contracts exchanged across features (DTOs/value objects). |
| 39 | +- Navigation contracts and events that other features can trigger/observe. |
| 40 | +- DI entry points/interfaces needed by composition modules (`:app-common`, app modules). |
| 41 | + |
| 42 | +Keep everything else in `internal`: |
| 43 | +- Implementations of the above contracts (repositories, data sources, mappers, use case implementations). |
| 44 | +- UI implementations, view models, and UI state models scoped to the feature. |
| 45 | +- Feature-scoped DI wiring, modules, and factories. |
| 46 | +- Experimental or volatile details that must not be exposed as public API. |
| 47 | + |
| 48 | +Notes for core modules: |
| 49 | +- The same rules apply. Core `api` exposes stable, shared infrastructure contracts (e.g., logging, networking abstractions, serialization, clock, crypto interfaces). Core `internal` contains their implementations and wiring. |
| 50 | + |
| 51 | +### Module naming rules |
| 52 | + |
| 53 | +- Standard two-module shape for a feature area: |
| 54 | + - `:feature:<area>[:<subarea>]:api` |
| 55 | + - `:feature:<area>[:<subarea>]:internal` (formerly `impl`) |
| 56 | +- Standard two-module shape for a core area: |
| 57 | + - `:core:<area>[:<subarea>]:api` |
| 58 | + - `:core:<area>[:<subarea>]:internal` (formerly `impl`) |
| 59 | +- If there are multiple implementation variants, suffix the `internal` module with a qualifier, e.g. rename |
| 60 | + `:feature:mail:message:export:impl-eml` to `:feature:mail:message:export:internal-eml`. |
| 61 | +- Library/legacy modules may adopt this pattern later but are currently out of scope for this ADR. |
| 62 | + |
| 63 | +### Package naming rules |
| 64 | + |
| 65 | +- For features, in `:feature:*:api`, use `net.thunderbird.feature.<area>[.<subarea>]`, e.g.: |
| 66 | + `net.thunderbird.feature.account.settings`, `net.thunderbird.feature.mail.message.reader`. |
| 67 | +- For features, in `:feature:*:internal`, mirror the API package structure but place all implementation under an |
| 68 | + `.internal` segment, e.g.: `net.thunderbird.feature.account.settings.internal`, |
| 69 | + `net.thunderbird.feature.mail.message.reader.internal.data`, `net.thunderbird.feature.mail.message.reader.internal.domain`. |
| 70 | +- For core, in `:core:*:api`, use `net.thunderbird.core.<area>[.<subarea>]`, e.g.: `net.thunderbird.core.network`. |
| 71 | +- For core, in `:core:*:internal`, mirror the API package structure and place implementation under `.internal`, e.g.: |
| 72 | + `net.thunderbird.core.network.internal`, `net.thunderbird.core.crypto.internal`. |
| 73 | +- Multiple variants (several implementations of the same contract): reflect the module qualifier after the `.internal` |
| 74 | + segment. |
| 75 | + - Mapping: `:…:internal-<variant>` → package `…internal.<variant>`. |
| 76 | + - Feature examples: |
| 77 | + - Module `:feature:mail:message:export:internal-eml` → package root |
| 78 | + `net.thunderbird.feature.mail.message.export.internal.eml` (e.g., `…internal.eml.data`, `…internal.eml.domain`). |
| 79 | + - If a PDF exporter exists: `:feature:mail:message:export:internal-pdf` → |
| 80 | + `net.thunderbird.feature.mail.message.export.internal.pdf`. |
| 81 | + - Core examples: |
| 82 | + - Module `:core:network:internal-okhttp` → `net.thunderbird.core.network.internal.okhttp`. |
| 83 | + - Module `:core:crypto:internal-bouncycastle` → `net.thunderbird.core.crypto.internal.bouncycastle`. |
| 84 | + - Keep variant tokens lowercase and alphanumeric. Use dot separators for dimension/value patterns (see below). Avoid |
| 85 | + kebab-case in package names. |
| 86 | +- Multi-dimension variants: when a variant expresses a dimension and a value, prefer `internal.<dimension>.<value>`. |
| 87 | + - Examples: `net.thunderbird.core.storage.internal.database.sqlite`, |
| 88 | + `net.thunderbird.feature.search.internal.engine.lucene`. |
| 89 | + - Prefer at most two levels under `.internal` for the variant part to keep packages readable. |
| 90 | + |
| 91 | +> [!NOTE] |
| 92 | +> - Avoid adding `.api` to package names for new code — the module already is the API. |
| 93 | +> - Prefer small focused API packages with narrow sets of contracts; keep type stability in mind when promoting code to API. |
| 94 | +> - API packages should remain variant-agnostic in almost all cases; concrete variants live under `.internal.<variant>`. |
| 95 | +
|
| 96 | +### Dependency rules (Gradle) |
| 97 | + |
| 98 | +- Feature-to-feature dependencies must target `:feature:*:api` only. |
| 99 | +- Feature-to-core dependencies must target `:core:*:api` only. |
| 100 | +- Core-to-core dependencies must target `:core:*:api` only. |
| 101 | +- Core modules must not depend on `:feature:*` modules. |
| 102 | +- `:feature:*:internal` and `:core:*:internal` dependencies are only allowed from: |
| 103 | + - the same area’s `api` module (when strictly necessary), and |
| 104 | + - composition modules: `:app-common`, `:app-k9mail`, `:app-thunderbird`. |
| 105 | +- Build logic will add a check that fails the build if a module depends on a `:*:internal` outside of these exceptions. |
| 106 | + |
| 107 | +### Migration plan |
| 108 | + |
| 109 | +1. Rename modules in Gradle from `impl` to `internal`: |
| 110 | + - Examples in `settings.gradle.kts` to update: |
| 111 | + - `:feature:account:avatar:impl` → `:feature:account:avatar:internal` |
| 112 | + - `:feature:account:settings:impl` → `:feature:account:settings:internal` |
| 113 | + - `:feature:mail:message:export:impl-eml` → `:feature:mail:message:export:internal-eml` |
| 114 | + - `:core:<area>:impl` → `:core:<area>:internal` (and for subareas accordingly) |
| 115 | +2. Adjust `namespace` in `build.gradle.kts` and Kotlin/Java `package` declarations to include `.internal`. |
| 116 | +3. Update imports and references after package moves. |
| 117 | +4. Add build-plugin check to disallow external dependencies on `:feature:*:internal` and `:core:*:internal` (enforced |
| 118 | + in the root build). |
| 119 | +5. Move all composition wiring (DI, factory bindings, navigation registrations) to `:app-common` or app modules. |
| 120 | +6. When in doubt, prefer starting in `internal`. Promote types to `api` only once they’re needed and stable. |
| 121 | + |
| 122 | +## Consequences |
| 123 | + |
| 124 | +### Positive Consequences |
| 125 | + |
| 126 | +- Explicit and discoverable boundary between public and internal code. |
| 127 | +- Stronger enforcement of architectural intent; easier refactors. |
| 128 | +- Package-level naming makes internal code clearly visible as non-public. |
| 129 | +- Works cleanly with KMP source sets (common/platform-specific). |
| 130 | +- Reduced confusion around whether a type is public or internal. |
| 131 | +- Clear criteria for placing types in `api` vs. `internal` improves consistency. |
| 132 | + |
| 133 | +### Negative Consequences |
| 134 | + |
| 135 | +- Requires renaming existing modules and packages, plus updating imports. |
| 136 | +- Slight learning overhead for contributors until the pattern becomes familiar. |
| 137 | +- Temporary churn in open branches during the migration window. |
| 138 | + |
0 commit comments