Reach data implementations through domain contracts - #77
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
After the previous cleanup, three feature-to-data dependencies remained, and all three reached past
:domainstraight into concrete data-layer classes:editorandbuildrunconstructed and calleddata.gradle.GradleProjectReaderdirectly.buildrundepended ondata.remote.ActiveBuild/ActiveBuildRepository.settingsread thedata.ai.AiProviderCatalogobject.These are the last main-source edges from a feature to a data implementation. Removing them means every feature now talks to
:domainonly, which is the precondition for enforcing that rule mechanically.What changed
ActiveBuildandActiveBuildRepositorymoved from:data:buildto:domain.ActiveBuildStoreandInMemoryActiveBuildStorestay in:data:buildas implementations.AiProviderDefinitionandAiProviderCatalogmoved from:data:aito:domain. It is static reference data with no I/O, so it stays a plain object — wrapping it in an injectable interface would add indirection without buying anything.GradleProjectInspectorcontract in:domain, implemented by the existingGradleProjectReader.gradleModulenow also bindsGradleProjectInspectorto the sameGradleProjectReaderinstance, so there is one parser rather than two.editor,buildrunandsettingsbuild files updated:data:buildis nowtestImplementationfor both features, andsettingsdropsdata:aientirely.Why the Gradle contract is narrower than the class
GradleProjectReadResultexposesdiagnostics,catalogandgradlePropertiesalongside the model — types owned by the Gradle parser. Moving it wholesale would have draggedGradleDiagnosticandVersionCataloginto:domain.Tracing the call sites showed the features only ever use four things:
isGradleProject,model,gradleVersionandagpVersion.GradleProjectSummarycarries exactly those, andProjectModelwas already a domain type. The parser-specific types stay in:data:build, whereread()still returns the full result for the data layer's own use.Behavior
No behavior change. Types moved between modules and one interface was introduced; no logic was altered.
GradleProjectReader.inspect()is a projection of the sameread()call.Scope
Both features keep
testImplementation(projects.data.build)because their tests construct the realGradleProjectReaderto parse fixture projects on disk. That is deliberate: main source depends on the abstraction, tests exercise the real implementation.Not included: the
data.localanddata.buildsystemsplit packages, and the fact that:feature:buildrunstill ownsdata.buildsystem.install.*. Both are handled in a later change.Tests
No new tests. The existing suites plus two guards did the work:
KoinModuleGraphTestcaught the missingGradleProjectInspectorbinding immediately — Koin'sverify()failed withMissingKoinDefinitionExceptionforEditorViewModelbefore anything reached a device.Verification
Verified from a clean worktree at the branch tip:
./gradlew test detekt :app:assembleDebug— BUILD SUCCESSFULAfter this change no feature module has a main-source dependency on any data module. The only remaining
projects.data.*entries across all seven features are threetestImplementationlines.Note on the detekt baselines: renaming the injected type from
GradleProjectReadertoGradleProjectInspectorchanged the constructor signature that keys two pre-existingLongParameterListbaseline entries in:feature:editorand:feature:buildrun. Those entries were re-keyed to the new signature rather than regenerated, so no other finding is silently absorbed.