refactor(assetregistry): close kernel and projection modules - #270
Conversation
|
Warning Review limit reached
Next review available in: 14 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe asset registry is reorganized into API, kernel, catalog, and authorization modules. New command/query contracts and kernel services replace direct repository access. Authorization projection uses a queue with explicit transaction boundaries. ChangesAsset registry boundaries
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Scheduler
participant ConvergenceService
participant ProjectionQueue
participant ProjectionService
participant ExternalWriter
Scheduler->>ConvergenceService: reconcile(limit)
ConvergenceService->>ProjectionQueue: claimPending(limit)
ProjectionService->>ProjectionQueue: claimForAsset(organizationId, assetId)
ProjectionService->>ExternalWriter: write relationship tuples
ExternalWriter-->>ProjectionService: authorization model ID or failure
ProjectionService->>ProjectionQueue: complete(batch, model ID)
ProjectionService->>ProjectionQueue: fail(batch, code, message)
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
TegamiThis repository uses Tegami to manage releases. When your changes affect published packages, add a changelog file under Create a changelog → · Changelog format Release preview
This PR does not add changelog files. Pending changelogs from other branches are included in the preview above. Run Managed by Tegami. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/src/main/java/com/orgmemory/core/assetregistry/api/AssetAuthorizationTarget.java (1)
5-11: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd null validation to match sibling API records.
AssetAuthorizationTargetis now public and part of the fail-closed authorization lookup contract, perAssetAuthorizationTargetQuery. UnlikeAssetIdentityandAssetRegistrationCommand.NewAssetin the same package, this record has no compact constructor validating required fields. Add the sameObjects.requireNonNullpattern for consistency and to fail loudly on malformed authorization targets.♻️ Proposed fix
+import java.util.Objects; import java.util.UUID; public record AssetAuthorizationTarget( UUID organizationId, UUID assetId, UUID knowledgeSpaceId, AssetType type, boolean authorizationReady) { + + public AssetAuthorizationTarget { + Objects.requireNonNull(organizationId, "organizationId"); + Objects.requireNonNull(assetId, "assetId"); + Objects.requireNonNull(knowledgeSpaceId, "knowledgeSpaceId"); + Objects.requireNonNull(type, "type"); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/src/main/java/com/orgmemory/core/assetregistry/api/AssetAuthorizationTarget.java` around lines 5 - 11, Add a compact constructor to AssetAuthorizationTarget that applies Objects.requireNonNull to each required reference field—organizationId, assetId, knowledgeSpaceId, and type—matching the validation pattern used by sibling API records. Leave the primitive authorizationReady unchanged and ensure malformed targets fail during construction.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@core/src/main/java/com/orgmemory/core/assetregistry/AssetDraftRepository.java`:
- Around line 15-23: Update the AssetDraftRepository.findForUpdate method to add
the jakarta.persistence.lock.timeout query hint alongside its PESSIMISTIC_WRITE
lock. Set the hint to the established finite timeout value used by the project
so concurrent requests fail fast while preserving the existing query and lock
behavior.
In
`@core/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryCoordinator.java`:
- Around line 872-878: Update the changeAvailability transition around
allReleasesWithdrawn and startSunsettingAfterReleaseChange so sunsetting is
attempted only for portfolios in a valid non-terminal state. Add a
coordinator-level state guard before
portfolios.startSunsettingAfterReleaseChange, or update that method to
explicitly handle terminal states, while preserving retireAfterFinalWithdrawal
for final withdrawals.
In
`@core/src/main/java/com/orgmemory/core/assetregistry/authorization/AssetAuthorizationProjectionService.java`:
- Around line 45-52: Add logging in the RuntimeException catch block of
AssetAuthorizationProjectionService before queue.fail, including the original
exception and contextual details about the failed OpenFGA authorization write.
Keep the existing queue.fail call and AssetUnavailableException conversion
unchanged.
In
`@core/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetKernelService.java`:
- Around line 101-118: Add a Flyway migration creating a partial unique index on
the asset-role assignment table over asset_id, principal_type, principal_id, and
role where valid_until is null, preserving the active-assignment uniqueness
enforced by AssetKernelService.assign for all database writers.
In
`@core/src/test/java/com/orgmemory/core/assetregistry/authorization/AssetAuthorizationProjectionServiceTests.java`:
- Around line 47-70: Add a test alongside
recordsAnOpaqueFailureWhenTheExternalWriterThrows that stubs
queue.claimForAsset(organizationId, assetId) to return Optional.empty() and
invokes AssetAuthorizationProjectionService.project with those IDs. Assert that
the call throws AssetUnavailableException, covering the already-being-projected
or unavailable branch without configuring the writer or batch.
- Around line 72-87: Replace the reflection-only assertions in
externalProjectionExplicitlyRejectsAnAmbientDatabaseTransaction with a Spring
proxy-based integration test that invokes both project(AssetAuthorizationBatch)
and reconcile(int) inside an existing transaction and verifies Propagation.NEVER
rejects execution. Ensure the test reaches the package-private batch overload
through the proxied service, rather than only checking annotations on the public
methods.
In
`@core/src/test/java/com/orgmemory/core/assetregistry/kernel/AssetKernelServiceTests.java`:
- Around line 117-139: Extend
kernelCommandsJoinTheParentTransactionAndQueueOperationsOwnShortTransactions in
AssetKernelServiceTests to assert that AssetAuthorizationCoordinator.complete
and fail use Propagation.REQUIRES_NEW, matching the existing claimForAsset and
claimPending assertions and covering completion and failure state updates.
In
`@core/src/test/java/com/orgmemory/core/assetregistry/SkillDistributionServiceTests.java`:
- Around line 185-195: Add a test in SkillDistributionServiceTests that creates
a valid-coordinate AssetIdentity with a non-SKILL AssetType, invokes
SkillDistributionService.manifest, and asserts AssetNotFoundException.
Parameterize or extend assetIdentity as needed while preserving existing
SKILL-based tests.
In `@core/src/test/java/com/orgmemory/core/ModulithVerificationTests.java`:
- Around line 1167-1181: Update the expected API type set in
ModulithVerificationTests to use class literals with getName() for each exposed
top-level AssetAuthorization, AssetIdentity, AssetPortfolio, AssetRegistration,
and AssetRole type. Represent nested types using nested class literals such as
AssetRegistrationCommand.NewAsset.class.getName(), preserving the existing
expected fully qualified names while enabling compile-time rename checks.
- Around line 1225-1227: Add a concise comment immediately above the
`resideInAnyPackage` entries explaining that `com.orgmemory.core.assetregistry`
intentionally matches only the exact package, while the authorization pattern
includes subpackages; preserve both patterns unchanged.
---
Outside diff comments:
In
`@core/src/main/java/com/orgmemory/core/assetregistry/api/AssetAuthorizationTarget.java`:
- Around line 5-11: Add a compact constructor to AssetAuthorizationTarget that
applies Objects.requireNonNull to each required reference field—organizationId,
assetId, knowledgeSpaceId, and type—matching the validation pattern used by
sibling API records. Leave the primitive authorizationReady unchanged and ensure
malformed targets fail during construction.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ca17a1ba-12c2-451d-b3a1-4190a27200f4
⛔ Files ignored due to path filters (5)
docs/increments/active/2026-07-31-spring-modulith-package-refactor/assetregistry-authorization-challenge-verdict.mdis excluded by!docs/**docs/increments/active/2026-07-31-spring-modulith-package-refactor/design.mdis excluded by!docs/**docs/increments/active/2026-07-31-spring-modulith-package-refactor/plan.mdis excluded by!docs/**docs/specs/domains/asset-registry.mdis excluded by!docs/**docs/tests/domains/asset-registry.mdis excluded by!docs/**
📒 Files selected for processing (41)
apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.javaapps/worker/src/main/java/com/orgmemory/worker/authorization/AssetAuthorizationConvergenceScheduler.javacore/src/main/java/com/orgmemory/core/assetregistry/AssetAuthorizationBatch.javacore/src/main/java/com/orgmemory/core/assetregistry/AssetCatalogReadModelRepository.javacore/src/main/java/com/orgmemory/core/assetregistry/AssetDraftRepository.javacore/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryCoordinator.javacore/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryService.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillDistributionService.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetAuthorizationProjectionCommand.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetAuthorizationTarget.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetAuthorizationTargetQuery.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetIdentity.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetIdentityQuery.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetPortfolioCommand.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetRegistrationCommand.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetRoleCommand.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetRoleQuery.javacore/src/main/java/com/orgmemory/core/assetregistry/authorization/AssetAuthorizationConvergenceReport.javacore/src/main/java/com/orgmemory/core/assetregistry/authorization/AssetAuthorizationConvergenceService.javacore/src/main/java/com/orgmemory/core/assetregistry/authorization/AssetAuthorizationProjectionService.javacore/src/main/java/com/orgmemory/core/assetregistry/authorization/package-info.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/Asset.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationBatch.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationCoordinator.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationOutbox.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationOutboxRepository.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationProjectionQueue.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationStatus.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetKernelService.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetRepository.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetRoleAssignment.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetRoleAssignmentRepository.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/package-info.javacore/src/test/java/com/orgmemory/core/ModulithVerificationTests.javacore/src/test/java/com/orgmemory/core/assetregistry/AssetRegistryServiceTests.javacore/src/test/java/com/orgmemory/core/assetregistry/AssetValidationTests.javacore/src/test/java/com/orgmemory/core/assetregistry/SkillDistributionServiceTests.javacore/src/test/java/com/orgmemory/core/assetregistry/authorization/AssetAuthorizationProjectionServiceTests.javacore/src/test/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationOutboxTests.javacore/src/test/java/com/orgmemory/core/assetregistry/kernel/AssetKernelServiceTests.javacore/src/test/java/com/orgmemory/core/assetregistry/kernel/AssetValidationTests.java
💤 Files with no reviewable changes (2)
- core/src/main/java/com/orgmemory/core/assetregistry/AssetAuthorizationBatch.java
- core/src/test/java/com/orgmemory/core/assetregistry/AssetValidationTests.java
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Treat the repository and runtime evidence as the engineering system of record; do not treat chat or Northstar as authoritative.
Before changing a domain, read its specification, test-coverage document, and applicable decision filenames.
Material decisions about domain boundaries, authorization, persistence, publication, concurrency, cache isolation, parity scope, or deployment require an independent architecture challenge and documented alternatives before implementation.
Do not use completed increment documents as the source for current behavior; use them only for history or archaeology.
Before using unfamiliar Spring Boot, Spring Modulith, Spring AI, Gradle, React, Vite, Tailwind, TypeScript, Next.js, or Fumadocs APIs, consult current official documentation, Context7, and the relevant verification skill.
Readdocs/guidelines/agent-safety.mdbefore retrieval, AI, MCP, permission, upload, graph, or export work; never commit secrets or customer data.
Keepddl-auto=validateand pair every persisted-model change with a Flyway migration.
Use the testing harness; a terminating clean test is the JVM context gate, andbootRunis not verification.
Files:
apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetAuthorizationProjectionCommand.javacore/src/main/java/com/orgmemory/core/assetregistry/authorization/AssetAuthorizationConvergenceReport.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetPortfolioCommand.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetAuthorizationTarget.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetIdentity.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetIdentityQuery.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetAuthorizationTargetQuery.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/Asset.javacore/src/main/java/com/orgmemory/core/assetregistry/AssetDraftRepository.javacore/src/test/java/com/orgmemory/core/assetregistry/kernel/AssetValidationTests.javacore/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryService.javaapps/worker/src/main/java/com/orgmemory/worker/authorization/AssetAuthorizationConvergenceScheduler.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationBatch.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetRegistrationCommand.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetRoleAssignment.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillDistributionService.javacore/src/main/java/com/orgmemory/core/assetregistry/authorization/package-info.javacore/src/main/java/com/orgmemory/core/assetregistry/authorization/AssetAuthorizationConvergenceService.javacore/src/main/java/com/orgmemory/core/assetregistry/authorization/AssetAuthorizationProjectionService.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationCoordinator.javacore/src/main/java/com/orgmemory/core/assetregistry/AssetCatalogReadModelRepository.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetRoleCommand.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationProjectionQueue.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationOutboxRepository.javacore/src/test/java/com/orgmemory/core/ModulithVerificationTests.javacore/src/test/java/com/orgmemory/core/assetregistry/authorization/AssetAuthorizationProjectionServiceTests.javacore/src/test/java/com/orgmemory/core/assetregistry/SkillDistributionServiceTests.javacore/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryCoordinator.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/package-info.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationOutbox.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetRoleQuery.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetRepository.javacore/src/test/java/com/orgmemory/core/assetregistry/AssetRegistryServiceTests.javacore/src/test/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationOutboxTests.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetRoleAssignmentRepository.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationStatus.javacore/src/test/java/com/orgmemory/core/assetregistry/kernel/AssetKernelServiceTests.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetKernelService.java
**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
Apply IDE inspection only to edited backend Java files.
Files:
apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetAuthorizationProjectionCommand.javacore/src/main/java/com/orgmemory/core/assetregistry/authorization/AssetAuthorizationConvergenceReport.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetPortfolioCommand.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetAuthorizationTarget.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetIdentity.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetIdentityQuery.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetAuthorizationTargetQuery.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/Asset.javacore/src/main/java/com/orgmemory/core/assetregistry/AssetDraftRepository.javacore/src/test/java/com/orgmemory/core/assetregistry/kernel/AssetValidationTests.javacore/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryService.javaapps/worker/src/main/java/com/orgmemory/worker/authorization/AssetAuthorizationConvergenceScheduler.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationBatch.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetRegistrationCommand.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetRoleAssignment.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillDistributionService.javacore/src/main/java/com/orgmemory/core/assetregistry/authorization/package-info.javacore/src/main/java/com/orgmemory/core/assetregistry/authorization/AssetAuthorizationConvergenceService.javacore/src/main/java/com/orgmemory/core/assetregistry/authorization/AssetAuthorizationProjectionService.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationCoordinator.javacore/src/main/java/com/orgmemory/core/assetregistry/AssetCatalogReadModelRepository.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetRoleCommand.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationProjectionQueue.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationOutboxRepository.javacore/src/test/java/com/orgmemory/core/ModulithVerificationTests.javacore/src/test/java/com/orgmemory/core/assetregistry/authorization/AssetAuthorizationProjectionServiceTests.javacore/src/test/java/com/orgmemory/core/assetregistry/SkillDistributionServiceTests.javacore/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryCoordinator.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/package-info.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationOutbox.javacore/src/main/java/com/orgmemory/core/assetregistry/api/AssetRoleQuery.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetRepository.javacore/src/test/java/com/orgmemory/core/assetregistry/AssetRegistryServiceTests.javacore/src/test/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationOutboxTests.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetRoleAssignmentRepository.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationStatus.javacore/src/test/java/com/orgmemory/core/assetregistry/kernel/AssetKernelServiceTests.javacore/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetKernelService.java
🔇 Additional comments (42)
core/src/main/java/com/orgmemory/core/assetregistry/authorization/AssetAuthorizationConvergenceReport.java (1)
1-1: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/authorization/AssetAuthorizationConvergenceService.java (2)
1-9: LGTM!
13-38: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/authorization/AssetAuthorizationProjectionService.java (3)
1-27: LGTM!
29-36: LGTM!
38-61: 🩺 Stability & AvailabilityAvoid package-private declaration order before the Spring proxy. The public
project(UUID, UUID)declaration comes before the package-private overload, but the annotation order in the source text is not relevant: the package-privateproject(AssetAuthorizationBatch)will not be advised under Spring’s default transaction proxy unless the public marker overload is removed or made package-private, and tests should not just check annotation presence.> Likely an incorrect or invalid review comment.core/src/main/java/com/orgmemory/core/assetregistry/authorization/package-info.java (1)
1-4: LGTM!apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.java (1)
24-24: LGTM!apps/worker/src/main/java/com/orgmemory/worker/authorization/AssetAuthorizationConvergenceScheduler.java (1)
3-3: LGTM!Also applies to: 10-48
core/src/test/java/com/orgmemory/core/assetregistry/authorization/AssetAuthorizationProjectionServiceTests.java (2)
27-45: LGTM!
47-70: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationProjectionQueue.java (1)
1-17: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/api/AssetAuthorizationProjectionCommand.java (1)
1-9: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/api/AssetAuthorizationTargetQuery.java (1)
1-11: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/api/AssetIdentity.java (1)
1-27: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/api/AssetIdentityQuery.java (1)
1-14: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/api/AssetPortfolioCommand.java (1)
1-14: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/api/AssetRegistrationCommand.java (1)
1-32: LGTM!core/src/test/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationOutboxTests.java (1)
1-1: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/AssetCatalogReadModelRepository.java (1)
13-13: 🗄️ Data Integrity & IntegrationRecord the intended JPQL access path for the closed
Assetentity.
AssetCatalogReadModelRepositoryno longer importscom.orgmemory.core.assetregistry.kernel.Asset, but every query still referencesEntitynameAssetand selects kernel columns such asasset.authorizationReady,asset.portfolioState, andasset.knowledgeSpaceId. JPQL entity names bypass Java import visibility, so this preserves full column-level access to the kernel entity while changing only the generic parameter toAssetDraft. If this read-model persistence bypass is intentional, document the domain-boundary exception and keepModulithVerificationTestsfrom treating theAssetstring reference as an acceptable import dependency.core/src/main/java/com/orgmemory/core/assetregistry/api/AssetRoleCommand.java (1)
1-27: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/api/AssetRoleQuery.java (1)
1-39: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/kernel/Asset.java (1)
1-1: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationBatch.java (1)
1-52: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationCoordinator.java (1)
1-1: LGTM!Also applies to: 10-10, 20-20, 38-76, 80-131
core/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationOutbox.java (1)
1-1: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetRoleAssignmentRepository.java (1)
1-1: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/kernel/package-info.java (1)
1-10: LGTM!core/src/test/java/com/orgmemory/core/assetregistry/kernel/AssetValidationTests.java (1)
1-30: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationStatus.java (1)
1-8: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetKernelService.java (1)
46-97: LGTM!Also applies to: 130-241
core/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetRepository.java (1)
11-28: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetRoleAssignment.java (1)
1-1: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryCoordinator.java (3)
207-207: LGTM!Also applies to: 243-254, 284-284, 128-190, 498-551, 889-909
916-919: 🩺 Stability & AvailabilityLock and tenant scope are already correct.
AssetDraftRepository.findForUpdateusesPESSIMISTIC_WRITEand filters by bothassetIdandorganizationId, so no change is needed.
983-989: 🗄️ Data Integrity & IntegrationNo change needed for these mappings. The
AssetRoleQuerynested records use the same component order as theAssetViewnested records for bothOwnershipHealthandRoleAssignment, so positional mapping is not a mismatch at these sites.core/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryService.java (1)
3-4: LGTM!Also applies to: 46-60
core/src/main/java/com/orgmemory/core/assetregistry/SkillDistributionService.java (1)
3-4: LGTM!Also applies to: 28-47, 64-83
core/src/test/java/com/orgmemory/core/assetregistry/AssetRegistryServiceTests.java (1)
3-4: LGTM!Also applies to: 64-64, 116-116, 161-161
core/src/test/java/com/orgmemory/core/assetregistry/SkillDistributionServiceTests.java (1)
10-13: LGTM!Also applies to: 82-90, 110-116, 128-128, 154-160, 219-219
core/src/test/java/com/orgmemory/core/ModulithVerificationTests.java (1)
1185-1216: LGTM!Also applies to: 1233-1245
core/src/main/java/com/orgmemory/core/assetregistry/kernel/AssetAuthorizationOutboxRepository.java (1)
22-56: 🗄️ Data Integrity & IntegrationNo duplicate types remain in the old package.
The only declarations are under
com.orgmemory.core.assetregistry.kernel, so the repository change does not introduce a duplicate entity/enum short-name conflict.
|
Outside-diff review finding addressed in 01e26c2: AssetAuthorizationTarget now validates organizationId, assetId, knowledgeSpaceId, and type with a compact constructor, with focused null-input coverage. Full :core:test passes 456/456 and the PR remains at 46 changed paths. |
1 participant
Summary
assetregistry.kernelaround canonical Asset identity, accountable roles, authorization outbox leases, and readinessassetregistry::apinamed interfaceassetregistry.authorizationaround OpenFGA projection/convergence and enforcePropagation.NEVERfor external writesArchitecture evidence
The planned intermediate parent-to-Kernel projection edge failed
modules.verify()with anassetregistry -> kernel -> assetregistrycycle. This PR therefore applies the already-reviewed projection-module topology in the same code-bearing slice instead of weakening verification. Both nested modules are closed with exact allowlists; no JPA entity, repository, or lock capability is public.Verification
clean test --no-parallel --max-workers=1: 99 tasks in 4m18smodules.verify(), import-boundary audit, andgit diff --checkScope
46 changed paths, including production code and focused tests. No endpoint, schema, persistence mapping, or authorization-semantic change. Public product docs are unaffected because this is an internal package and transaction-boundary refactor.
skip-release: intermediate modular refactor; release follows completion of the full refactor goal
Summary by CodeRabbit
New Features
Bug Fixes
Tests