feat(datamodeling): Avro Contracts and Kafka lifecycle events#101
Merged
Conversation
- Axis.DataModeling.Contracts (9 lifecycle + field events) - DataModelingEventMapper and domain FieldAdded/Updated/Removed - Wolverine Kafka wiring in Axis.Api Program.cs - Update PROGRESS and register-avro-schemas.sh Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR completes DataModeling Phase 2 by adding Axis.DataModeling.Contracts (Avro schemas, topic constants, GUID-parsing extensions), domain events and emissions for field lifecycle, mapping domain events to Avro integration events for outbox publication, Kafka wiring, schema registration updates, docs, and tests. ChangesDataModeling Event Contracts and Publishing
Sequence Diagram(s)sequenceDiagram
participant DataModel as DataModelAggregate
participant UnitOfWork as DataModelingUnitOfWork
participant Mapper as DataModelingEventMapper
participant Bus as WolverineBus
participant Kafka as Kafka
DataModel->>DataModel: RaiseDomainEvent(FieldAdded)
UnitOfWork->>UnitOfWork: SaveChangesAsync()
UnitOfWork->>Mapper: ToIntegrationEvent(FieldAdded)
Mapper-->>UnitOfWork: FieldAddedEvent(obj) or null
UnitOfWork->>Bus: PublishAsync(integrationEvent) when not null
Bus->>Kafka: Send to DataModelingKafkaTopics.FieldAdded
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/register-avro-schemas.sh (1)
8-8:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDefine
WB_SCHEMA_DIRandDM_SCHEMA_DIRbefore use.With
set -u, the firstregister "$WB_SCHEMA_DIR/...call will abort with an unbound variable error.Suggested fix
-SCHEMA_DIR="$ROOT/src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/Schemas" +WB_SCHEMA_DIR="$ROOT/src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/Schemas" +DM_SCHEMA_DIR="$ROOT/src/Modules/DataModeling/Axis.DataModeling.Contracts/Schemas"Also applies to: 26-38
🤖 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 `@scripts/register-avro-schemas.sh` at line 8, Define and export the WB_SCHEMA_DIR and DM_SCHEMA_DIR variables before any use to avoid unbound variable errors under set -u; initialize them (e.g., WB_SCHEMA_DIR="$ROOT/src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/Schemas" and DM_SCHEMA_DIR="$ROOT/…/DesiredModule/…/Schemas") near the top of the script, then use those variables in the register calls instead of relying on SCHEMA_DIR being present, ensuring any register "$WB_SCHEMA_DIR/..." and register "$DM_SCHEMA_DIR/..." invocations reference existing variables.
🧹 Nitpick comments (1)
tests/Modules/DataModeling/Axis.DataModeling.Infrastructure.Tests/Messaging/DataModelingEventMapperTests.cs (1)
51-51: ⚡ Quick winReplace the fully-qualified interface type with a
usingimport.Use
IDomainEventdirectly instead ofAxis.Shared.Domain.Primitives.IDomainEventin the test record declaration.Suggested fix
using Axis.DataModeling.Infrastructure.Messaging; +using Axis.Shared.Domain.Primitives; using FluentAssertions; @@ - private sealed record UnknownTestDomainEvent : Axis.Shared.Domain.Primitives.IDomainEvent; + private sealed record UnknownTestDomainEvent : IDomainEvent;As per coding guidelines, "Use
usingstatements instead of fully qualified class names (FQCN)".🤖 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 `@tests/Modules/DataModeling/Axis.DataModeling.Infrastructure.Tests/Messaging/DataModelingEventMapperTests.cs` at line 51, The test declares the record UnknownTestDomainEvent implementing the fully-qualified interface Axis.Shared.Domain.Primitives.IDomainEvent; replace the FQCN with a using directive and implement IDomainEvent directly. Add a `using Axis.Shared.Domain.Primitives;` at the top of the test file (or ensure the namespace is already imported) and change the record declaration to `private sealed record UnknownTestDomainEvent : IDomainEvent;` so the code uses the simple interface name instead of the fully-qualified name.
🤖 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 `@src/Modules/DataModeling/Axis.DataModeling.Domain/Aggregates/DataModel.cs`:
- Around line 88-89: The UpdateField and RemoveField code paths in the DataModel
aggregate must raise domain events like AddField does: after mutating the field
in DataModel.UpdateField call RaiseDomainEvent(new FieldUpdated(...)) with the
same payload shape used by FieldAdded (Id, OrganizationId, field.Id, field.Name,
field.Type, field.Label, field.IsRequired, field.DisplayOrder) and after
removing a field in DataModel.RemoveField call RaiseDomainEvent(new
FieldRemoved(...)) with those same identifying field properties; place the
RaiseDomainEvent calls immediately after the state change so downstream
consumers receive the lifecycle events.
---
Outside diff comments:
In `@scripts/register-avro-schemas.sh`:
- Line 8: Define and export the WB_SCHEMA_DIR and DM_SCHEMA_DIR variables before
any use to avoid unbound variable errors under set -u; initialize them (e.g.,
WB_SCHEMA_DIR="$ROOT/src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/Schemas"
and DM_SCHEMA_DIR="$ROOT/…/DesiredModule/…/Schemas") near the top of the script,
then use those variables in the register calls instead of relying on SCHEMA_DIR
being present, ensuring any register "$WB_SCHEMA_DIR/..." and register
"$DM_SCHEMA_DIR/..." invocations reference existing variables.
---
Nitpick comments:
In
`@tests/Modules/DataModeling/Axis.DataModeling.Infrastructure.Tests/Messaging/DataModelingEventMapperTests.cs`:
- Line 51: The test declares the record UnknownTestDomainEvent implementing the
fully-qualified interface Axis.Shared.Domain.Primitives.IDomainEvent; replace
the FQCN with a using directive and implement IDomainEvent directly. Add a
`using Axis.Shared.Domain.Primitives;` at the top of the test file (or ensure
the namespace is already imported) and change the record declaration to `private
sealed record UnknownTestDomainEvent : IDomainEvent;` so the code uses the
simple interface name instead of the fully-qualified name.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3c895cd5-8214-479f-abce-521f12c46930
⛔ Files ignored due to path filters (9)
src/Modules/DataModeling/Axis.DataModeling.Contracts/Generated/axis/datamodeling/events/DataClassCreatedEvent.csis excluded by!**/generated/**src/Modules/DataModeling/Axis.DataModeling.Contracts/Generated/axis/datamodeling/events/DataClassDeletedEvent.csis excluded by!**/generated/**src/Modules/DataModeling/Axis.DataModeling.Contracts/Generated/axis/datamodeling/events/DataRecordCreatedEvent.csis excluded by!**/generated/**src/Modules/DataModeling/Axis.DataModeling.Contracts/Generated/axis/datamodeling/events/DataRecordDeletedEvent.csis excluded by!**/generated/**src/Modules/DataModeling/Axis.DataModeling.Contracts/Generated/axis/datamodeling/events/FieldAddedEvent.csis excluded by!**/generated/**src/Modules/DataModeling/Axis.DataModeling.Contracts/Generated/axis/datamodeling/events/FieldRemovedEvent.csis excluded by!**/generated/**src/Modules/DataModeling/Axis.DataModeling.Contracts/Generated/axis/datamodeling/events/FieldUpdatedEvent.csis excluded by!**/generated/**src/Modules/DataModeling/Axis.DataModeling.Contracts/Generated/axis/datamodeling/events/ModelCreatedEvent.csis excluded by!**/generated/**src/Modules/DataModeling/Axis.DataModeling.Contracts/Generated/axis/datamodeling/events/ModelDeletedEvent.csis excluded by!**/generated/**
📒 Files selected for processing (29)
Axis.slndocs/PROGRESS.mddocs/epics/E03-data-modeling/README.mddocs/epics/E03-data-modeling/features/F01-model-definition.mddocs/epics/README.mdscripts/register-avro-schemas.shsrc/Axis.Api/Program.cssrc/Modules/DataModeling/Axis.DataModeling.Contracts/Axis.DataModeling.Contracts.csprojsrc/Modules/DataModeling/Axis.DataModeling.Contracts/DataModelingEventExtensions.cssrc/Modules/DataModeling/Axis.DataModeling.Contracts/DataModelingKafkaTopics.cssrc/Modules/DataModeling/Axis.DataModeling.Contracts/Schemas/DataClassCreatedEvent.avscsrc/Modules/DataModeling/Axis.DataModeling.Contracts/Schemas/DataClassDeletedEvent.avscsrc/Modules/DataModeling/Axis.DataModeling.Contracts/Schemas/DataRecordCreatedEvent.avscsrc/Modules/DataModeling/Axis.DataModeling.Contracts/Schemas/DataRecordDeletedEvent.avscsrc/Modules/DataModeling/Axis.DataModeling.Contracts/Schemas/FieldAddedEvent.avscsrc/Modules/DataModeling/Axis.DataModeling.Contracts/Schemas/FieldRemovedEvent.avscsrc/Modules/DataModeling/Axis.DataModeling.Contracts/Schemas/FieldUpdatedEvent.avscsrc/Modules/DataModeling/Axis.DataModeling.Contracts/Schemas/ModelCreatedEvent.avscsrc/Modules/DataModeling/Axis.DataModeling.Contracts/Schemas/ModelDeletedEvent.avscsrc/Modules/DataModeling/Axis.DataModeling.Domain/Aggregates/DataModel.cssrc/Modules/DataModeling/Axis.DataModeling.Domain/Events/FieldAdded.cssrc/Modules/DataModeling/Axis.DataModeling.Domain/Events/FieldRemoved.cssrc/Modules/DataModeling/Axis.DataModeling.Domain/Events/FieldUpdated.cssrc/Modules/DataModeling/Axis.DataModeling.Infrastructure/Axis.DataModeling.Infrastructure.csprojsrc/Modules/DataModeling/Axis.DataModeling.Infrastructure/Messaging/DataModelingEventMapper.cssrc/Modules/DataModeling/Axis.DataModeling.Infrastructure/Persistence/DataModelingUnitOfWork.cstests/Architecture/Axis.Architecture.Tests/Axis.Architecture.Tests.csprojtests/Modules/DataModeling/Axis.DataModeling.Domain.Tests/DataModelTests.cstests/Modules/DataModeling/Axis.DataModeling.Infrastructure.Tests/Messaging/DataModelingEventMapperTests.cs
…101 review) Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
6 tasks
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
2 participants
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.
Summary
Completes the DataModeling Phase 2 service-boundary retrofit: new
Axis.DataModeling.Contractsproject with nine Avro event schemas published via Wolverine outbox → Kafka (CloudEvents, ADR-019). Domain field mutations now raiseFieldAdded,FieldUpdated, andFieldRemovedevents;DataModelingEventMapperreplaces raw domain-event publishing inDataModelingUnitOfWork. UpdatesPROGRESS.mdto reflect that WorkflowBuilder/FormBuilder/WorkflowEngine contract and per-module DB work was already in place.Linked spec
Requirements
Axis.DataModeling.Contractswith Avro schemas + generated recordsProgram.cs(publish + local testing)scripts/register-avro-schemas.shextended for DataModeling subjectsdotnet buildanddotnet format --verify-no-changesgreenPROGRESS.mdand epics README updatedDeferred (follow-up PR): Kafka consumers in FormBuilder/WorkflowBuilder for model/field delete (broken-reference detection); DataModeling gRPC; WorkflowEngine
IScriptExecutor/INotificationSender; E01 provisioning retry/UI.Summary by CodeRabbit