Migrate to com.extopy:api/client 0.1.5, with a data/domain/presentation split - #13
Merged
Merged
Conversation
The app has to be on Kotlin 2.3 or later to read the klibs that
`com.extopy:api` publishes, which is what pulls the rest of the toolchain
forward with it. This brings the build in line with what nutrimaxing-app
already runs: a version catalog instead of versions written out in each
build script, `com.android.kotlin.multiplatform.library` for the shared
module instead of the old `com.android.library` plus an `android {}` block,
and `androidApp`/`iosApp` as module names.
Room and multiplatform-settings arrive here as dependencies; what actually
uses them is the commit that follows.
Two versions are pinned rather than taken at their latest:
- lifecycle stays on 2.11.0-beta01, since 2.11.0 requires compileSdk 37 and
AGP 9.1, neither of which is available yet.
- sentry-cocoa moves to 8.58.2, which is what sentry-kotlin-multiplatform
0.27 links against — the previous 8.4.0 floor resolved to a version
missing the symbols it needs.
minSdk goes from 21 to 26, which is what lets the core library desugaring go
away.
`com.extopy:commons` is gone. What the backend publishes now is an API contract (`com.extopy:api`) and a client for it (`com.extopy:client`), and the two are shaped quite differently from what the app was talking to: - Models are `PostResponse`/`UserResponse`/`TimelineResponse` rather than `Post`/`User`/`Timeline`, on `kotlin.uuid.Uuid` and `kotlin.time.Instant`. - Reads throw rather than return null, since the client sets `expectSuccess`. - The client holds the tokens and refreshes them itself, through a `TokenStore` the app supplies. `RenewTokenUseCase`, `GetRefreshTokenUseCase` and the token getters/setters had nothing left to do. - Listings take `limit`/`offset`/`search` rather than a `Pagination`. kaccelero is dropped along with it: its 0.7 release removed the client half the app depended on (`IAPIClient`, `APIException`, the auth use cases), and the backend has moved off it too. multiplatform-settings takes over storage, and the generic use case interfaces are gone rather than replaced — a use case is now a class with `operator fun invoke`, and only exists where there is a decision to make. Plain reads go through the repositories, which the view models take directly. The package layout follows nutrimaxing-app: `data/` for how things are fetched and stored, `domain/` for what the app is about, `presentation/` for Compose, `di/` for the wiring. `domain` and `presentation` import neither the API contract nor Ktor, Room or the settings library — everything crosses over in `data/mappers`, so the next change to the contract stops there. The local cache moves from SQLDelight to Room, and now holds the counters and the viewer-relative flags it used to drop, which is why signing out clears it: those rows belong to the session that filled them. That closes the "clear cache" TODO that logging out carried. Tests move to `commonTest` and cover the two use cases that carry logic, the token store, and both directions of both mappers.
The README said three lines about KMM. It now says where things live, what the boundary between the layers actually is, and how to point either platform at a backend running on your own machine — which is new: there was no way to do that before, only the two deployed environments.
The cache regression is the one that matters. Caching the viewer-relative fields — whether you liked a post, whether you follow someone — is what makes a list draw right on the way back, but nothing dropped the row after the write that changed them. Liking a post and reopening it inside the minute showed it unliked again, with the old count. Both writes now delete the row rather than try to patch it: the server is the only thing that knows what the count became. `purgeExpired` was declared and never called, so rows cached once and never read again stayed forever. It runs when the app opens, which is the moment those rows are known to be stale. `onLikeClicked` crashed on the post at the top of a thread: that post is not one of the replies, so `indexOf` returned -1 and `set(-1, …)` threw. It is updated on its own now, and the three view models match by id rather than by position. On iOS, `Dispatchers.IO` does exist on Kotlin/Native — the comment saying otherwise was wrong, and SQLite was blocking the CPU-sized pool. It needs importing by name there, since it is an extension property. The keychain doc was wrong too: `KeychainSettings(service)` sets `kSecAttrService`, not an access group, and this app declares no `keychain-access-groups` entitlement, so nothing was ever shared with it — an App Group is a different capability. The service is now named for what it is, and per-environment so the development build cannot read the production session. Dropped along the way: the unqualified `Settings` definition neither platform was injecting, `Picker`, and five dependencies nothing imported (compose-animation, kotlinx-datetime, koin-compose-viewmodel-navigation, compottie-resources, ktor-client-mock).
The bug was invisible to the test suite: nothing exercised what happens to a cached post after it is liked. These four cover it, with fakes for the two API clients and the two DAOs the repository touches. Checked by removing the fix again: `forgetsACachedPostOnceItHasBeenLiked` and `goesToTheNetworkAgainAfterALike` both fail without it.
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
1 participant
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.
com.extopy:commonsis gone. The backend now publishes an API contract (com.extopy:api) and aclient for it (
com.extopy:client), both at 0.1.5, and they are shaped differently enough that theapp could not simply bump a version. This migrates onto them, and takes the opportunity to bring the
project in line with the architecture nutrimaxing-app already runs.
What moved
The API. Models are
PostResponse/UserResponse/TimelineResponseonkotlin.uuid.Uuidandkotlin.time.Instant; reads throw rather than return null; listings takelimit/offset/searchrather than a
Pagination. The client holds the tokens and refreshes them itself through aTokenStorethe app supplies —RenewTokenUseCase,GetRefreshTokenUseCaseand the tokengetters/setters had nothing left to do.
kaccelero is dropped. Its 0.7 release removed the client half the app depended on (
IAPIClient,APIException, the auth use cases), and the backend has moved off it too. multiplatform-settingstakes over storage. The generic use case interfaces are gone rather than replaced: a use case is now
a class with
operator fun invoke, and only exists where there is a decision to make — the ~30IXxxUseCase.ktfiles are deleted. Plain reads go through repositories, which the view models takedirectly.
The layers.
data/for how things are fetched and stored,domain/for what the app is about,presentation/for Compose,di/for the wiring.domainandpresentationimport neither the APIcontract nor Ktor, Room or the settings library — everything crosses over in
data/mappers, so thenext change to the contract stops there.
The local cache moves from SQLDelight to Room, and now holds the counters and viewer-relative
flags it used to drop. That is why signing out clears it: those rows belong to the session that
filled them. Closes the "clear cache" TODO logging out was carrying.
The toolchain. Kotlin 2.4, AGP 9.0.1 with
com.android.kotlin.multiplatform.library, Gradle 9.1,Compose Multiplatform 1.11.1, a version catalog,
androidApp/iosAppmodule names, minSdk 26.Kotlin 2.3+ is not optional — it is what can read the klibs
com.extopy:apipublishes.New: running against a local backend
There was no way to do this before, only the two deployed environments.
localflavor points at10.0.2.2:8080and is the only flavor allowed to talkover cleartext HTTP (
./gradlew :androidApp:installLocalDebug).EXTOPY_ENV=localin the scheme; ignored in release builds so it cannot ship.Verified
commonTestthe timeline and its posts all return 200, and the posts render with their author, counters and
relative timestamps.
Worth knowing before merging
(kaccelero → multiplatform-settings), with no migration. Users will be signed out once on update.
com.extopy:apihas no Android target, so Android resolves itsjvmvariant. That worksthrough the KMP compatibility rules, but adding
androidTarget()backend-side would be cleaner.extopy-backend/client, not here:logout()clears theTokenStorebut not theBearerAuthProvider's in-memory copy, so signing back in within the sameprocess reuses the stale token and wipes the fresh one. To be handled with the next backend pass.
2.11.0-beta01(2.11.0 needs compileSdk37 + AGP 9.1), and sentry-cocoa moves to 8.58.2, which is what sentry-kotlin-multiplatform 0.27
links against.
Review pass
An architecture review ran over the result. Its findings are fixed in
da391f1: cache invalidationon writes (a regression this PR introduced by caching the viewer-relative fields),
purgeExpirednever being called, a crash liking the post at the top of a thread, a wrong
Dispatchers.IOcomment,a misleading keychain doc, and a handful of dead declarations.
7b49c78adds the tests that weremissing to catch the first one — checked by reverting the fix and watching them fail.
Pre-existing dette the review surfaced and this PR deliberately leaves alone: going offline reads as
signed out,
LogoutUseCasehas no UI entry point, the Direct Messages tab opens a blank screen, andthe timeline's suggested-users list is never populated.