Skip to content

refactor(api): extract trial app usage service - #40208

Open
hjlarry wants to merge 1 commit into
p485from
p486
Open

refactor(api): extract trial app usage service#40208
hjlarry wants to merge 1 commit into
p485from
p486

Conversation

@hjlarry

@hjlarry hjlarry commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Part of #39993.

This is PR 2 of the recommended-app controller stack. Its Git base is #40207.

Move recommended trial-app usage writes out of RecommendedAppService and behind a persistence-neutral application service plus a SQLAlchemy repository.

Why this layer

The previous implementation accepted the controller's request Session and called commit() from RecommendedAppService.add_trial_app_record(). That coupled the application behavior to SQLAlchemy and could commit unrelated pending work owned by the request.

This PR gives usage accounting its own repository-owned transaction:

  • trial controllers pass only app_id and account_id;
  • TrialAppUsageService defines the application boundary;
  • TrialAppUsageRepository owns the model query and transaction;
  • the composition root wires the concrete adapter;
  • an import-linter contract prevents persistence dependencies from moving back into the application service.

Intentional boundaries

This is deliberately not the complete recommended-app migration:

  • RecommendedAppService.get_app() remains because runtime trial admission is a separate flow.
  • RecommendedAppService.is_trial_app_enabled() remains the deployment-level policy source.
  • current_user, RequestContext, setup_required, and admission decorators are unchanged.
  • Existing quota admission behavior is unchanged; this PR does not introduce atomic quota reservation.
  • Legacy builtin, remote, and database catalog adapters remain behind the compatibility gateway introduced by refactor(api): extract recommended app query service #40207.

These boundaries keep the transaction-ownership change independently reviewable. Runtime admission and the remaining legacy catalog adapters can move in later stack layers.

Verification

  • 133 focused unit tests passed.
  • Ruff check and formatting passed for the changed Python files.
  • Pyrefly reported 0 diagnostics for the changed application paths.
  • Import-linter kept all 9 contracts with 0 broken.

From Codex

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Pyrefly Type Coverage

Metric Base PR Delta
Type coverage 58.15% 58.16% +0.01%
Strict coverage 57.72% 57.73% +0.01%
Typed symbols 38,082 38,101 +19
Untyped symbols 27,614 27,613 -1
Modules 3150 3154 +4

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-08-09 04:14:00.763733415 +0000
+++ /tmp/pyrefly_pr.txt	2026-08-09 04:13:51.064722925 +0000
@@ -2075,7 +2075,7 @@
 ERROR Argument `Literal['account']` is not assignable to parameter `created_by_role` with type `CreatorUserRole` in function `models.model.UploadFile.__init__` [bad-argument-type]
     --> tests/unit_tests/controllers/console/datasets/test_datasets_segments.py:1102:29
 ERROR Object of class `FunctionType` has no attribute `assert_called_once_with` [missing-attribute]
-   --> tests/unit_tests/controllers/console/explore/test_trial.py:215:5
+   --> tests/unit_tests/controllers/console/explore/test_trial.py:226:5
 ERROR Argument `str` is not assignable to parameter `view` with type `(InstalledApp) -> str` in function `controllers.console.explore.wraps.decorator` [bad-argument-type]
   --> tests/unit_tests/controllers/console/explore/test_wraps.py:73:18
 ERROR Argument `str` is not assignable to parameter `view` with type `(InstalledApp) -> str` in function `controllers.console.explore.wraps.decorator` [bad-argument-type]

@hjlarry
hjlarry marked this pull request as ready for review August 9, 2026 06:06
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. refactor labels Aug 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant