Serialize batch state checkers per permission in dynamic distribution#25424
Merged
maliming merged 2 commits intoMay 14, 2026
Conversation
- Add state-aware overload to ISimpleStateCheckerSerializer/Contributor - Features and Permissions contributors recognise their batch checker and emit a per-state record - PermissionDefinitionSerializer threads the owning permission through - Pin equality semantics to match the batch runtime (default comparer)
Member
Author
|
@yagmurcelk 10.4 test branch: |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes dynamic permission distribution losing StateCheckers when permissions/features use the default shared batch state checkers (AsyncLocal singleton). The serializer pipeline now becomes state-aware so it can emit per-permission/per-state non-batch records, keeping publisher and consumer gating consistent.
Changes:
- Add state-aware serialization overloads and thread the owning
PermissionDefinitionthrough serialization. - Teach Features/Permissions serializer contributors to extract per-state models from batch checkers and serialize them as non-batch records.
- Add regression tests (including equality semantics parity with runtime lookup).
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/permission-management/test/Volo.Abp.PermissionManagement.Domain.Tests/Volo/Abp/PermissionManagement/PermissionDefinitionSerializer_Tests.cs | Adds regression coverage for default batch RequireFeatures/RequirePermissions serialization. |
| modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionDefinitionSerializer.cs | Passes the permission as “state” into state-checker serialization. |
| framework/test/Volo.Abp.Features.Tests/Volo/Abp/Features/RequireFeaturesSimpleBatchStateChecker_Tests.cs | Pins EqualityComparer<TState>.Default semantics for model lookup. |
| framework/src/Volo.Abp.GlobalFeatures/Volo/Abp/GlobalFeatures/GlobalFeaturesSimpleStateCheckerSerializerContributor.cs | Implements the new state-aware serializer contributor overload (delegating to existing logic). |
| framework/src/Volo.Abp.Features/Volo/Abp/Features/RequireFeaturesSimpleBatchStateChecker.cs | Adds per-state model lookup API (GetModelOrNull). |
| framework/src/Volo.Abp.Features/Volo/Abp/Features/FeaturesSimpleStateCheckerSerializerContributor.cs | Adds state-aware serialization that converts batch checkers into per-state JSON records. |
| framework/src/Volo.Abp.Core/Volo/Abp/SimpleStateChecking/SimpleStateCheckerSerializerExtensions.cs | Adds list serialization overload that accepts the owning state. |
| framework/src/Volo.Abp.Core/Volo/Abp/SimpleStateChecking/SimpleStateCheckerSerializer.cs | Adds state-aware Serialize(checker, state) pipeline. |
| framework/src/Volo.Abp.Core/Volo/Abp/SimpleStateChecking/ISimpleStateCheckerSerializerContributor.cs | Adds new state-aware contributor method (API surface change). |
| framework/src/Volo.Abp.Core/Volo/Abp/SimpleStateChecking/ISimpleStateCheckerSerializer.cs | Adds new state-aware serializer method (API surface change). |
| framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/RequirePermissionsSimpleBatchStateChecker.cs | Adds per-state model lookup API (GetModelOrNull). |
| framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionsSimpleStateCheckerSerializerContributor.cs | Adds state-aware serialization for permission batch checkers. |
| framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/AuthenticatedSimpleStateCheckerSerializerContributor.cs | Implements the new state-aware contributor overload (delegating to existing logic). |
- Mirror _models with Dictionary<TState, ...> populated in AddCheckModels - GetModelOrNull goes through the dict, matching IsEnabledAsync's first-wins - Pin first-wins via a regression test
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## rel-10.3 #25424 +/- ##
==========================================
Coverage 49.78% 49.78%
==========================================
Files 3609 3609
Lines 120714 120822 +108
Branches 9220 9232 +12
==========================================
+ Hits 60099 60156 +57
- Misses 58818 58859 +41
- Partials 1797 1807 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
EngincanV
approved these changes
May 14, 2026
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.
RequireFeatures(...)andRequirePermissions(...)default to a shared batch state checker (AsyncLocal singleton) that holds models for many permissions. The serializer contributor pipeline only recognised the non-batch types, so the batch instance was silently dropped fromPermissionDefinitionRecordduring dynamic permission distribution. Consumers ended up with emptyStateCheckers, the feature / permission gate disappeared on that side, and they disagreed with the publisher's in-memory verdict (typical symptom: a UI tab whose backing API rejects with 403 because the publisher still enforces the gate).ISimpleStateCheckerSerializerContributorandISimpleStateCheckerSerializernow have a state-aware overload. TheFeaturesandPermissionscontributors look up the per-state model on the batch checker (matched with the sameEqualityComparer<TState>.Defaultsemantics the runtime uses) and emit a non-batch record.PermissionDefinitionSerializerthreads the owning permission through. Includes a regression test pinning the equality contract.Regression introduced in #25276. Reported in https://github.com/volosoft/volo/issues/22319.