feat(projections): preserve definition write provenance#424
Conversation
PR SummaryMedium Risk Overview
The embedded UI supplies JSON annotations ( Reviewed by Cursor Bugbot for commit f2dfe87. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
Warning Review limit reached
Next review available in: 58 seconds 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: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
WalkthroughThis PR introduces "definition metadata" for projections, carrying tool/operation/property information from the embedded UI and gRPC clients through command messages, ProjectionManager, and ManagedProjection persistence. The proto schema adds properties maps to Create/Update options. A separate unrelated fixture fix updates parked-message metadata parsing and PrepareFlags handling. ChangesProjection Definition Metadata
Estimated code review effort: 3 (Moderate) | ~25 minutes Test Fixture Parked-Message Fix
Sequence Diagram(s)sequenceDiagram
participant Client
participant ProjectionManagementCreate as ProjectionManagement.Create
participant Publisher
participant ProjectionManager
participant ManagedProjection
Client->>ProjectionManagementCreate: CreateReq(options.Properties)
ProjectionManagementCreate->>ProjectionManagementCreate: ToMetadata(options.Properties)
ProjectionManagementCreate->>Publisher: Post(..., definitionMetadata)
Publisher->>ProjectionManager: NewProjectionInitializer(definitionMetadata)
ProjectionManager->>ManagedProjection: InitializeNew(definitionMetadata)
ManagedProjection->>ManagedProjection: TryCreatePersistedStateEvent(_pendingDefinitionMetadata)
Estimated code review effort: 3 (Moderate) | ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/EventStore.Projections.Core/Services/Management/ManagedProjection.cs (1)
936-977: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDon't clear
_pendingDefinitionMetadataunconditionally on success.UpdateQueryandInitializeNewreuse this shared staging field, so an older completion can erase a newer pending value before the next persisted-state event is built. Clear only the metadata snapshot that belongs to the completed write.🤖 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 `@src/EventStore.Projections.Core/Services/Management/ManagedProjection.cs` around lines 936 - 977, The success path in WritePersistedStateCompleted is clearing the shared _pendingDefinitionMetadata field unconditionally, which can erase newer staged metadata from UpdateQuery or InitializeNew before it is used. Update the completion handling so it only clears the metadata snapshot associated with the completed write, and avoid resetting the shared staging field unless it still matches the write being finalized.
🧹 Nitpick comments (1)
src/EventStore.Projections.Core/Services/Management/ProjectionManager.cs (1)
1487-1527: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: no default value for
definitionMetadata.Unlike
ProjectionPost/Post/UpdateQuery(all defaultdefinitionMetadatatonull), this constructor requires it explicitly. SinceNewProjectionInitializerandPendingProjectionare public nested types, adding a default (byte[] definitionMetadata = null) would keep the API consistent with the rest of the stack and avoid breaking any external callers of this constructor.🤖 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 `@src/EventStore.Projections.Core/Services/Management/ProjectionManager.cs` around lines 1487 - 1527, The NewProjectionInitializer constructor currently requires definitionMetadata explicitly, unlike the related ProjectionPost, Post, and UpdateQuery APIs that default it to null. Update the NewProjectionInitializer signature in ProjectionManager to make definitionMetadata optional with a null default, and keep the existing fallback to Empty.ByteArray in the constructor body so external callers of this public nested type remain compatible.
🤖 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.
Outside diff comments:
In `@src/EventStore.Projections.Core/Services/Management/ManagedProjection.cs`:
- Around line 936-977: The success path in WritePersistedStateCompleted is
clearing the shared _pendingDefinitionMetadata field unconditionally, which can
erase newer staged metadata from UpdateQuery or InitializeNew before it is used.
Update the completion handling so it only clears the metadata snapshot
associated with the completed write, and avoid resetting the shared staging
field unless it still matches the write being finalized.
---
Nitpick comments:
In `@src/EventStore.Projections.Core/Services/Management/ProjectionManager.cs`:
- Around line 1487-1527: The NewProjectionInitializer constructor currently
requires definitionMetadata explicitly, unlike the related ProjectionPost, Post,
and UpdateQuery APIs that default it to null. Update the
NewProjectionInitializer signature in ProjectionManager to make
definitionMetadata optional with a null default, and keep the existing fallback
to Empty.ByteArray in the constructor body so external callers of this public
nested type remain compatible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 399fd764-7afc-4cf4-b4c8-af873bbbb0aa
⛔ Files ignored due to path filters (1)
proto.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
src/EventStore.ClusterNode/Components/Services/ProjectionBrowserService.cssrc/EventStore.Core.Tests/Helpers/TestFixtureWithExistingEvents.cssrc/EventStore.Projections.Core.Tests/Services/projections_manager/when_posting_a_persistent_projection_and_writes_succeed.cssrc/EventStore.Projections.Core.Tests/Services/projections_manager/when_updating_a_persistent_projection_query_text.cssrc/EventStore.Projections.Core/Messages/ProjectionManagementMessage.cssrc/EventStore.Projections.Core/Services/Grpc/ProjectionManagement.Create.cssrc/EventStore.Projections.Core/Services/Grpc/ProjectionManagement.Update.cssrc/EventStore.Projections.Core/Services/Grpc/ProjectionManagement.cssrc/EventStore.Projections.Core/Services/Management/ManagedProjection.cssrc/EventStore.Projections.Core/Services/Management/ProjectionManager.cssrc/Protos/Grpc/projections.proto
yordis
force-pushed
the
yordis/feat-projection-definition-metadata
branch
from
635cb6c to
4a77d53
Compare
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis
force-pushed
the
yordis/feat-projection-definition-metadata
branch
from
4a77d53 to
f2dfe87
Compare
1 participant
Uh oh!
There was an error while loading. Please reload this page.