refactor(identity): layer the module into router/service/repository/github - #61
Merged
Conversation
…ithub - repository.py owns every query against github_users; the service no longer builds SQL - github.py is a typed boundary: GitHubOAuthToken and GitHubUserProfile replace the bare dicts that used to cross the domain, so a GitHub shape change fails at the edge instead of as a KeyError in a service - service.py exposes use cases (authenticate_with_code, sync_user, refresh_tokens) and returns a TokenPair object instead of an untyped 2-tuple; get_user_stats returns UserStatsResponse instead of a dict - container/repository.py is introduced to own the ContainerSession aggregates the dashboard needs, so identity stops writing SQL over another module's tables - router.py is thin: validate, call one use case, shape the response. Inline UnauthorizedError imports hoisted, cookie policy factored into one helper - logout now clears the cookie with the same attributes used to set it (it was hardcoded secure=True, so it silently failed over plain HTTP) and is rate-limited like its neighbours - dead RefreshRequest schema removed - tests rewritten off unittest.mock onto httpx.MockTransport: 17 -> 26 tests covering the boundary parsing, token claims and the stats path 317 tests pass, ruff and mypy clean.
|
helPRs session created for this PR. Skill: |
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.
First step of the layering work:
identitybecomes the reference shape for every module.What changed
exchange_code_for_tokenandfetch_github_userreturned baredict; callers indexed them blindly (token_data["access_token"]). They now return validated models, so a GitHub response change surfaces asExternalServiceErrorat the boundary instead of aKeyErrorthree layers in.get_user_statsreturned a dict built from SQL overContainerSession— a table identity does not own. The queries moved to a newcontainer/repository.py(typedStatusCounts/DailyCountresults) and the service returnsUserStatsResponse.create_token_pairreturned an untyped 2-tuple; it now returns aTokenPair.authenticate_with_codecall. InlineUnauthorizedErrorimports hoisted; cookie policy in one helper.Bug fixed along the way
logouthardcodedsecure=Truewhile every other cookie call computes it from the environment — over plain HTTP in local dev the browser kept the refresh cookie, so logout did nothing. It also had no rate limit, unlike every other route in the file.Verification
unittest.mockontohttpx.MockTransport, which exercises real request building — the new tests assert on the URL, headers and body actually sent.ruff+mypyclean. mypy caught a real latent type error during the refactor (adateflowing into adatetimefield, previously hidden by the untyped dict).