Skip to content

feat: add repositories, session scope, actions, form objects and broadcast listeners - #205

Merged
anilcancakir merged 5 commits into
masterfrom
feature/app-architecture
Sep 27, 2026
Merged

anilcancakir merged 5 commits into
masterfrom
feature/app-architecture

Conversation

@anilcancakir

Copy link
Copy Markdown
Member

Summary

Laravel-shaped application primitives for magic apps, extracted from what Uptizm hand-rolled per controller:

  • Read side: Repository<T> (id-keyed cache of one remote resource: upsertFromList, showOnlyKeys merge, 404 evicts, an epoch guard drops answers that straddle a session reset) and RepositoryQuery<T> (ordered, filtered, paginated view over a repository on MagicPaginator.fetcher; items resolve live, a non-list 2xx is a failed read, the load-start notification is deferred so a view's initState may call reload()).
  • Session scope: SessionScope / SessionScoped, a tenant-reset boundary over every cached controller and repository, attached explicitly as the last Auth.stateNotifier listener. Magic.delete<T>() now disposes the controller it removes (MagicController.dispose is idempotent).
  • Write side: MagicAction<I, O> with bind / resolve / flush (Fortify's action-swap pattern) and the RunsActions mixin; MagicFormObject, a Livewire-style form object over ValidatesRequests and CollapsesIndexedErrorKeys.
  • Timing: LatestRead, Poll.until (sealed PollOutcome), Countdown, Debouncer, OwnsTimers.
  • Broadcasting: BroadcastListeners and ListensToBroadcasts, one AuthChannelSubscription per alias shared by every listening controller (Livewire getListeners style).
  • URLs: UrlGenerator and the top-level url() helper.
  • Events: Event.listenAny and the ReportsBreadcrumb contract, so a crash reporter plugin records a whitelisted breadcrumb trail without a hard dependency.
  • Validation: Uuid, Boolean, Numeric, Integer, Gt / Gte / Lt / Lte, Between, Regex, Date, Nullable, RequiredIf, ArrayRule.
  • Generators: make:action, make:form, make:repository.

Docs, the magic-framework skill and CHANGELOG are updated.

Verification

flutter analyze clean, flutter test 2034 passing, dart format --set-exit-if-changed clean. Uptizm was rebuilt on top of these primitives and walked live on Chrome (1440 and 500 px) and the iOS simulator.

Part of a coordinated set

This PR is one of eight that move reusable, app-agnostic pieces out of Uptizm into the magic framework and its plugins, so any magic app can use them the Laravel way.

Merge order:

  1. fluttersdk/magic first: every plugin PR compiles against its new API (SessionScope, Repository, MagicAction, Event.listenAny, ReportsBreadcrumb).
  2. Then magic_deeplink, magic_notifications, magic_payments and magic_sentry, in any order.
  3. Then magic_starter, which uses magic core's SessionScope, magic_payments' StoreIdentitySync and magic_deeplink's gate.
  4. magic-starter-laravel is independent of the Flutter side.
  5. anilcancakir/uptizm last.

Until magic merges and ships, CI on the plugin PRs resolves the published magic and is expected to be red. Each branch was verified locally against the sibling working trees (analyze, the full test suite, format check).

No version bump, publish or tag is included; a release is a separate step.

@kodizm

kodizm Bot commented Sep 26, 2026

Copy link
Copy Markdown

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

Solid, well-tested set of primitives; two correctness issues are worth fixing before the plugin PRs start depending on this API.

Major

lib/src/broadcasting/broadcast_listeners.dart:74: correctness. Each alias gets its own AuthChannelSubscription, but they all share one Echo socket, and that class's _teardown calls Echo.disconnect() (auth_channel_subscription.dart:202). Suppose a team alias resolves to null while a user alias stays live (for example, the user leaves their last team). BroadcastListeners.sync() tears down team, which drops the whole socket. user's _reconcile then finds _subscribedName == name and returns early, so it never reconnects and stays deaf until its name changes. That class was built for a single subscription. Nothing in broadcast_listeners_test.dart covers more than one alias. I'm fairly confident about this from reading the code, but no test exercises it.

lib/src/validation/rules/between.dart:35: correctness. Gt/Gte/Lt/Lte read a numeric string as a number (comparison.dart:15), but Between always measures a String by its length. Form input is always a string, so [Numeric(), Between(1, 100)] on a text field accepts "500" (length 3). The same field with [Lte(100)] rejects it. The CHANGELOG and validation.md describe the two families as sizing values the same way. Suggest giving Between the same _sizeOf logic and adding a numeric-string test.

Minor

lib/src/actions/runs_actions.dart:61: If an action throws ValidationException from a controller that does not mix in ValidatesRequests, runAction returns null and shows nothing: no toast, and onFailure is not called. The user sees a silent no-op. Consider falling through to the failure path in that case.

Tests

Every new primitive has its own test file (repository, query, session scope, actions, form object, timers, broadcast listeners, rules, generators). No test covers a multi-alias broadcast teardown or Between on a numeric string.

CI

  • Lint & Test: success
  • Internal Links & Anchors: success
  • External Links: skipped
  • Auto-merge low-risk Dependabot PRs: skipped

What I read: the core lib/ sources (repository, repository query, session scope, broadcasting, actions, form object, poll, URL generator, the new rules, and the Magic.delete, MagicController.dispose and EventDispatcher changes) and the CHANGELOG. I did not review the docs, stubs, generators, SKILL.md, countdown/debouncer/latest_read/owns_timers, or the test bodies line by line.

runAction answered null for a void success, a failure and a refused
same-key re-entry alike, so callers read a refused double tap as success.
It now answers ActionSucceeded, ActionFailed or ActionRefused. A
ValidationException on a host without ValidatesRequests reaches onFailure
or the toast instead of vanishing, and the fallback toast no longer shows
the raw exception text. Also: RepositoryQuery clears only its own first
load, Magic.delete disposes through ChangeNotifier without importing the
http layer, and the MagicAction doc samples compile.
@kodizm

kodizm Bot commented Sep 27, 2026

Copy link
Copy Markdown

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

The new commits look correct. They fix my earlier runAction silent-failure finding, and ActionOutcome is a real improvement. One small regression came in with the Magic.delete change.

What changed since my last review (87cc680 → efee6e5, incremental): runAction now returns a sealed ActionOutcome<O>. A ValidationException on a host without ValidatesRequests now shows the generic toast (this resolves my earlier Minor). The toast body no longer repeats the exception's raw text. RepositoryQuery now checks identity before clearing _firstLoad. Magic.delete now disposes any ChangeNotifier, where before it only disposed MagicController. The two earlier Majors (multi-alias BroadcastListeners teardown, Between on numeric strings) are in files these commits don't touch, so I'm not repeating them here.

Minor

lib/src/foundation/magic.dart:256: correctness. The dispose check moved from is MagicController to is ChangeNotifier, but Magic.put<T> takes any value, and only MagicController.dispose is idempotent. The comment just above says so. Take a plain ChangeNotifier or ValueNotifier that was registered with Magic.put and already disposed by its owner. Magic.delete now calls dispose() on it a second time, and Flutter's debug debugAssertNotDisposed throws. The same call also silently disposes a notifier that another holder is still listening to. Two options: test for MagicController's interface through a small contract in foundation/, which still avoids the upward import, or keep the old type check. magic_delete_disposes_test.dart has no plain-ChangeNotifier case.

Tests

  • runs_actions_test.dart covers all three outcome variants, the fallback toast on a non-validating host, that raw exception text never reaches the UI, and logging with and without a bound log driver.
  • repository_query_test.dart adds the mid-flight reset / stale first-load race.
  • Nothing covers Magic.delete on a plain ChangeNotifier.

CI

  • Lint & Test: success
  • codecov/patch: success (90.02% of diff hit)
  • Internal Links & Anchors: success
  • External Links: skipped
  • Auto-merge low-risk Dependabot PRs: skipped

Read in full: all 11 files in this increment (the new action_outcome.dart, runs_actions.dart, magic_action.dart, repository_query.dart, foundation/magic.dart, the barrel, both test files, doc/basics/actions.md and the CHANGELOG). I also grepped for runAction callers in docs and SKILL.md. They either discard the result or return it as Future<void>, so the return-type change doesn't break them.

@kodizm

kodizm Bot commented Sep 27, 2026

Copy link
Copy Markdown

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

These commits look correct. The Magic.delete behaviour is now deliberate, documented and tested, so I consider my earlier Minor settled. One small documentation gap remains.

What changed since my last review (efee6e5 → 89c4daa, incremental): Magic.delete keeps the is ChangeNotifier check. Its dartdoc now says plainly that any registered notifier gets disposed, and that the caller must not dispose it again. A new test covers a plain ChangeNotifier. The runAction samples in controllers.md, application-structure.md and SKILL.md now await the new ActionOutcome, and succeeded does exist on every variant. The two earlier Majors (multi-alias BroadcastListeners teardown, Between on numeric strings) are in files these commits don't touch, so I'm not repeating them.

Minor

CHANGELOG.md:12: maintainability. The entry still says Magic.delete disposes "the controller it removes". It doesn't say that a plain ChangeNotifier or ValueNotifier registered with Magic.put is now disposed too. That is the breaking case for an app that disposes such a notifier itself, and until now it is only written down in the dartdoc. CLAUDE.md's post-change sync asks for the CHANGELOG to record changes like this. A short note in doc/architecture/service-container.md near its Magic.delete example would help too.

Tests

magic_delete_disposes_test.dart now checks that Magic.delete disposes a plain ChangeNotifier, and that calling dispose() on it a second time throws.

CI

  • Lint & Test: success
  • codecov/patch: success (90.02% of diff hit)
  • Internal Links & Anchors: success
  • External Links: skipped
  • Auto-merge low-risk Dependabot PRs: skipped

I read all 5 files in this increment in full.

@anilcancakir

Copy link
Copy Markdown
Member Author

Round 4 fixes. The two round 1 Majors were still open in code (round 2 and 3 only noted they sat in untouched files), so this round closes them too.

Major, lib/src/validation/rules/between.dart:35 (Between sizing a numeric string by length): fixed in 4c802c1, but not the way the finding suggested. Folding every numeric string into a number, as comparison.dart's _sizeOf did, breaks the documented 'password': [Required(), Between(8, 64)]: "12345678" would be read as 12345678 and refused. The same fold already made [Gte(8)] refuse the eight-character "00000001". Laravel's getSize answers this by the attribute's rule list: a numeric string is a number only when the attribute also has a numeric rule, otherwise a length.

  • New SizeRule contract (lib/src/validation/contracts/size_rule.dart): passesSized(..., {required bool numeric}) plus a shared sizeOf. passes() delegates with numeric: false.
  • Min, Max, Between, Gt, Gte, Lt, Lte extend it; the private _sizeOf is gone.
  • Validator._runValidation (validator.dart:180) and FormValidator.rules (form_validator.dart:129) compute SizeRule.numericIn(rules) (Numeric or Integer present) and pass it in.
  • Min/Max changing for [Numeric(), Max(100)] on "500" is filed under BREAKING in the CHANGELOG; the validation doc gets a "Sizing a Numeric String" section and the skill's forms-validation.md table says the same.
  • test/validation/size_rules_numeric_test.dart, 9 tests. Before the fix 6 failed: Between/Max accepted "500" beside Numeric, Min refused "12" beside Integer, the numeric message key, Gte(8) refused "00000001", and Lt(10) refused "123" called directly.

Major, lib/src/broadcasting/broadcast_listeners.dart:74 (one alias going null disconnects the socket under the others): fixed in 225bb01.

  • AuthChannelSubscription takes disconnectOnTeardown (default true, the existing contract for a lone subscription; the class is unreleased since 0.0.21, so this is additive).
  • Each alias's subscription is built with disconnectOnTeardown: false. BroadcastListeners.sync() records whether any alias was live, syncs every alias, then calls Echo.disconnect() only when it was live and none is now.
  • The class doc, doc/digging-deeper/broadcasting.md and the CHANGELOG entries for both classes now say this instead of "resolve them together".
  • New test in test/broadcasting/broadcast_listeners_test.dart: team and user live, team goes null, the connection stays up and private-users.1 stays subscribed; user goes null, it disconnects. Red before the fix with Expected the broadcast driver to be connected but it was disconnected.

Minor, CHANGELOG.md:12 (Magic.delete disposing a plain notifier): fixed in 4c802c1. The BREAKING entry now says delete disposes any ChangeNotifier it removes, a plain ChangeNotifier or ValueNotifier put there included, and that code disposing one itself must stop. doc/architecture/service-container.md gets the same note under its Magic.delete example, and the skill's bootstrap-lifecycle.md row too.

Gates on 225bb01: dart format . 445 files, 0 changed; flutter analyze no issues; flutter test 2050 passed.

@kodizm please re-review.

@kodizm

kodizm Bot commented Sep 27, 2026

Copy link
Copy Markdown

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

These commits fix both open Majors and the CHANGELOG Minor. I found nothing new to flag.

What changed since my last review (89c4daa → 225bb01, incremental). My earlier Between finding asked for the size rules to "share _sizeOf". You replied that doing so would break [Between(8, 64)] on a password, and you're right: what Laravel's getSize actually does is decide by the attribute's rule list, which is what you built. That settles it.

  • SizeRule (lib/src/validation/contracts/size_rule.dart): passesSized receives numeric from Validator._runValidation (validator.dart:179) and FormValidator.rules (form_validator.dart:130). A grep shows no other place in lib/ that calls rule.passes, so the new path always receives the numeric flag. sizeOf parses with num.tryParse, the same parser Numeric uses, so the two rules can't disagree about whether a string is a number. Integer's int.tryParse accepts a subset of that. In Min/Max, empty strings and null still pass as before; the only change is the one filed under BREAKING.
  • BroadcastListeners.sync(): each alias's subscription is now built with disconnectOnTeardown: false, and the socket disconnects only on the change from some alias live to none. I also traced two overlapping sync() calls, for example a fast logout followed by a login. currentName is set inside the serialised channelName() callback before any subscribe, so the first call's _anyLive() check cannot disconnect a channel that the second call has already started to resolve. If it does disconnect first, _reconcile reconnects.
  • CHANGELOG / service-container.md: both now say that Magic.delete disposes a plain ChangeNotifier or ValueNotifier, which settles my earlier Minor.

Tests

  • size_rules_numeric_test.dart covers Between, Max, Min and Gte beside Numeric and Integer and on their own, the numeric message key, the eight-digit password, and a direct passes() call.
  • broadcast_listeners_test.dart adds the two-alias case: team goes null while user stays subscribed, then user goes null and the socket disconnects.

CI

  • Lint & Test: success
  • codecov/patch: success (91.94% of diff hit)
  • Internal Links & Anchors: success
  • External Links: skipped
  • Auto-merge low-risk Dependabot PRs: skipped

I read the full diff of all 12 code, test and CHANGELOG files in this increment, plus the call sites I grepped for. I did not open the doc and skill markdown changes (validation.md, broadcasting.md, service-container.md, forms-validation.md, bootstrap-lifecycle.md) beyond their file list.

@anilcancakir
anilcancakir merged commit 73f56cc into master Sep 27, 2026
6 checks passed
@anilcancakir anilcancakir mentioned this pull request Sep 27, 2026
@anilcancakir
anilcancakir deleted the feature/app-architecture branch September 27, 2026 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant