feat(messaging): Avro Schema Registry and Kafka events (ADR-019)#91
Merged
Conversation
Add Axis.WorkflowBuilder.Contracts with Avro schemas, Confluent serializers, CloudEvents envelope rules, and WorkflowBuilder lifecycle event routing. Consumers use Contracts only; Testing uses local Wolverine routing. 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 (6)
📝 WalkthroughWalkthroughThis PR establishes Avro-based contract events for WorkflowBuilder lifecycle distribution via Kafka and Schema Registry. It adds a new contracts project with Avro schemas, maps domain events to Avro integration events, implements CloudEvents + Schema Registry wiring, updates handlers/tests to consume contract events, and configures the API to publish/listen via Avro. ChangesWorkflowBuilder Contracts and Event Infrastructure
Sequence Diagram(s)sequenceDiagram
participant App as Application Startup
participant Wolverine as WolverineOptions
participant SR as Schema Registry
participant Kafka as Kafka Broker
App->>Wolverine: UseAxisKafkaAvro(config, useKafkaEventTransport)
Wolverine->>Wolverine: Register CloudEventsEnvelopeRule
alt Kafka transport enabled
Wolverine->>SR: Read SchemaRegistry:Url
Wolverine->>Wolverine: Create SchemaRegistryAvroSerializer
Wolverine->>Kafka: PublishAndListenWithAvro(topic, serializer)
else Testing environment
Wolverine->>Wolverine: PublishLocally for workflow events
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/WorkflowBuilderEventExtensions.cs (1)
8-24: ⚡ Quick winHarden GUID parsing in
WorkflowBuilderEventExtensionsto improve poison-message diagnostics.
Guid.Parseis used directly forworkflowId,organizationId, andreferencedFormIdsinWorkflowBuilderEventExtensions.cs, and multiple handlers in WorkflowEngine/FormBuilder call these accessors—so invalid payloads will throw with low field-level context. Switch toGuid.TryParseand throw aFormatExceptionthat includes the field name (as proposed).Proposed refactor
public static class WorkflowBuilderEventExtensions { - public static Guid WorkflowId(this WorkflowPublishedEvent `@event`) => Guid.Parse(`@event.workflowId`); + public static Guid WorkflowId(this WorkflowPublishedEvent `@event`) + => ParseRequiredGuid(`@event.workflowId`, nameof(`@event.workflowId`)); - public static Guid OrganizationId(this WorkflowPublishedEvent `@event`) => Guid.Parse(`@event.organizationId`); + public static Guid OrganizationId(this WorkflowPublishedEvent `@event`) + => ParseRequiredGuid(`@event.organizationId`, nameof(`@event.organizationId`)); public static IReadOnlyList<Guid> ReferencedFormIds(this WorkflowPublishedEvent `@event`) - => `@event.referencedFormIds.Select`(Guid.Parse).ToList(); + => `@event.referencedFormIds` + .Select(id => ParseRequiredGuid(id, "referencedFormIds[]")) + .ToList(); - public static Guid WorkflowId(this WorkflowArchivedEvent `@event`) => Guid.Parse(`@event.workflowId`); + public static Guid WorkflowId(this WorkflowArchivedEvent `@event`) + => ParseRequiredGuid(`@event.workflowId`, nameof(`@event.workflowId`)); - public static Guid OrganizationId(this WorkflowArchivedEvent `@event`) => Guid.Parse(`@event.organizationId`); + public static Guid OrganizationId(this WorkflowArchivedEvent `@event`) + => ParseRequiredGuid(`@event.organizationId`, nameof(`@event.organizationId`)); - public static Guid WorkflowId(this WorkflowUnarchivedEvent `@event`) => Guid.Parse(`@event.workflowId`); + public static Guid WorkflowId(this WorkflowUnarchivedEvent `@event`) + => ParseRequiredGuid(`@event.workflowId`, nameof(`@event.workflowId`)); - public static Guid OrganizationId(this WorkflowUnarchivedEvent `@event`) => Guid.Parse(`@event.organizationId`); + public static Guid OrganizationId(this WorkflowUnarchivedEvent `@event`) + => ParseRequiredGuid(`@event.organizationId`, nameof(`@event.organizationId`)); public static IReadOnlyList<Guid> ReferencedFormIds(this WorkflowUnarchivedEvent `@event`) - => `@event.referencedFormIds.Select`(Guid.Parse).ToList(); + => `@event.referencedFormIds` + .Select(id => ParseRequiredGuid(id, "referencedFormIds[]")) + .ToList(); + + private static Guid ParseRequiredGuid(string value, string fieldName) + => Guid.TryParse(value, out Guid parsed) + ? parsed + : throw new FormatException($"Invalid GUID in field '{fieldName}'."); }🤖 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/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/WorkflowBuilderEventExtensions.cs` around lines 8 - 24, The current extension methods (WorkflowId, OrganizationId, ReferencedFormIds) on WorkflowPublishedEvent, WorkflowArchivedEvent and WorkflowUnarchivedEvent use Guid.Parse which throws without field-level context; replace those parses with Guid.TryParse and, on failure, throw a FormatException that includes the field name (e.g., "workflowId" or "organizationId") and the offending string value to aid poison-message diagnostics; for ReferencedFormIds iterate the collection, try-parse each entry (including index or value in the error), collect parsed GUIDs into the same IReadOnlyList<Guid> return, and keep method signatures (WorkflowId, OrganizationId, ReferencedFormIds) unchanged.src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/Schemas/WorkflowPublishedEvent.avsc (1)
17-43: 🏗️ Heavy liftConsolidate
StepSnapshotRecord/TransitionSnapshotRecordschema definitions to avoid drift
WorkflowPublishedEvent.avscinlinesStepSnapshotRecordandTransitionSnapshotRecordfor thesteps/transitionsarrays, and standalone schemas exist too (axis.workflowbuilder.events.StepSnapshotRecord/axis.workflowbuilder.events.TransitionSnapshotRecord). Sincescripts/register-avro-schemas.shregisters only the workflow event subjects and doesn’t register/express schema references for these standalone record schemas, make one set the single source-of-truth for codegen (e.g., generate the inline records from the standalone.avscor remove one set) so they can’t diverge.🤖 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/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/Schemas/WorkflowPublishedEvent.avsc` around lines 17 - 43, The inline record definitions for StepSnapshotRecord and TransitionSnapshotRecord inside WorkflowPublishedEvent.avsc are duplicating the standalone schemas (axis.workflowbuilder.events.StepSnapshotRecord / TransitionSnapshotRecord) and risk divergence; change WorkflowPublishedEvent.avsc so the "steps" and "transitions" array item types reference the existing named types instead of re-declaring them (use the named type references "axis.workflowbuilder.events.StepSnapshotRecord" and "axis.workflowbuilder.events.TransitionSnapshotRecord"), and update scripts/register-avro-schemas.sh to ensure the standalone record schemas are registered (or otherwise included) so codegen consumes the single source-of-truth records rather than the inline copies.scripts/register-avro-schemas.sh (2)
10-20: ⚡ Quick winConsider adding file existence validation.
The
register()function attempts to read and POST each schema file without verifying it exists. If a schema file is missing or the path is incorrect,trwill fail with a less-than-obvious error message.🛡️ Proposed enhancement for clearer errors
register() { local file="$1" local subject="$2" + if [[ ! -f "$file" ]]; then + echo "ERROR: schema file not found: $file" >&2 + exit 1 + fi local schema schema="$(tr -d '\n' <"$file" | sed 's/"/\\"/g')"🤖 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` around lines 10 - 20, The register() function reads schema files without checking they exist; add a pre-check for the "file" path (and readability) inside register() and handle missing files by printing a clear error referencing the subject and file, then exit non-zero or skip registration; update the error path to avoid calling tr/sed/curl when the file is absent and ensure SCHEMA_REGISTRY_URL and subject are still shown in the error message for easier debugging.
14-14: 💤 Low valueSchema JSON escaping may be fragile for complex Avro schemas.
The
sed 's/"/\\"/g'approach escapes double quotes but does not handle backslashes, control characters, or other JSON special characters. For simple Avro schemas this is sufficient, but more complex schemas (with string defaults containing escape sequences) could break.If schemas remain simple (as they are now), the current escaping is adequate. If you later encounter escaping issues, consider using
jq -cto canonicalize the schema JSON instead:schema="$(jq -c . <"$file" | sed 's/"/\\"/g')"This normalizes the JSON and removes formatting inconsistencies before escaping.
🤖 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 14, The schema JSON is currently built with the fragile line that assigns to the variable `schema` using a simple quote-escape which doesn't handle backslashes or control characters; update the `schema` assignment in scripts/register-avro-schemas.sh to canonicalize and compact the JSON first (use `jq -c .` on the file) and then perform the quote-escaping before exporting or using `schema` so backslashes and other special characters are preserved; ensure you replace the existing `schema="..."` assignment with this jq-based preprocessing and keep the subsequent code that uses the `schema` variable unchanged.src/Shared/Axis.Shared.Infrastructure/Messaging/CloudEventsEnvelopeRule.cs (1)
55-55: ⚡ Quick winReplace fully-qualified reflection type with a using import.
Line 55 should use a
usingimport instead ofSystem.Reflection.PropertyInfoinline.Suggested fix
+using System.Reflection; using System.Diagnostics; using Wolverine; @@ - System.Reflection.PropertyInfo? property = message.GetType().GetProperty("organizationId"); + PropertyInfo? property = message.GetType().GetProperty("organizationId");As per coding guidelines, "Use using declarations instead of fully-qualified names for imports in C# files."
🤖 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/Shared/Axis.Shared.Infrastructure/Messaging/CloudEventsEnvelopeRule.cs` at line 55, Replace the fully-qualified System.Reflection type with a using-import: add "using System.Reflection;" at the top of CloudEventsEnvelopeRule.cs and change the declaration "System.Reflection.PropertyInfo? property = message.GetType().GetProperty(\"organizationId\");" to use the short type name "PropertyInfo?" so the variable (property) uses PropertyInfo? from the imported namespace; ensure the file compiles after adding the using.tests/Modules/FormBuilder/Axis.FormBuilder.Infrastructure.Tests/Handlers/WorkflowPublishedHandlerTests.cs (1)
26-34: ⚡ Quick winAdd a regression test for duplicate
referencedFormIdsin one event.Current coverage validates redelivery idempotency but not duplicated form IDs inside a single payload. Add one case to assert only one reference row is persisted for repeated IDs.
Also applies to: 85-103
🤖 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/FormBuilder/Axis.FormBuilder.Infrastructure.Tests/Handlers/WorkflowPublishedHandlerTests.cs` around lines 26 - 34, The tests lack a regression case for duplicated referencedFormIds in a single WorkflowPublishedEvent payload; update the test suite by adding a new test (or extending the existing test range around WorkflowPublishedHandlerTests) that uses BuildEvent to construct a WorkflowPublishedEvent where referencedFormIds contains the same form GUID repeated, then assert that only one reference row is persisted for that form ID (i.e., deduplication happens inside the handler). Modify or overload BuildEvent if needed to allow constructing repeated IDs (referencedFormIds), and in the new test verify persistence/query results (or persisted rows count) for the deduplicated referencedFormIds after invoking the handler.
🤖 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/FormBuilder/Axis.FormBuilder.Infrastructure/Handlers/WorkflowPublishedHandler.cs`:
- Line 27: The handler is inserting (workflowId, formId, organizationId) entries
for each ID in referencedFormIds and can attempt duplicate inserts when
referencedFormIds contains duplicates; change the code to deduplicate
referencedFormIds before the insert loop (e.g., construct newFormIds as a
HashSet from referencedFormIds) and iterate over that set when calling the
repository/insert logic (the variables/methods to update: referencedFormIds ->
newFormIds/HashSet<Guid>, the insert loop that uses workflowId and
organizationId), and apply the same deduplication for the other similar
loop/section around lines 36-43 so duplicates won’t trigger
UniqueConstraintException.
In
`@src/Modules/WorkflowEngine/Axis.WorkflowEngine.Infrastructure/Handlers/WorkflowPublishedHandler.cs`:
- Around line 36-38: The lookup in WorkflowPublishedHandler that fetches
WorkflowActiveStatus currently matches only workflowId and must be scoped by
organizationId to preserve tenant boundaries: update the query against
context.WorkflowActiveStatuses (the one assigning to variable existing) to
include w.OrganizationId == organizationId (and do the same for the second
upsert query later in the same handler) so both reads/updates match on
(WorkflowId, OrganizationId); ensure any FirstOrDefaultAsync or update/create
logic (in WorkflowPublishedHandler) uses this composite predicate to avoid
cross-tenant updates.
---
Nitpick comments:
In `@scripts/register-avro-schemas.sh`:
- Around line 10-20: The register() function reads schema files without checking
they exist; add a pre-check for the "file" path (and readability) inside
register() and handle missing files by printing a clear error referencing the
subject and file, then exit non-zero or skip registration; update the error path
to avoid calling tr/sed/curl when the file is absent and ensure
SCHEMA_REGISTRY_URL and subject are still shown in the error message for easier
debugging.
- Line 14: The schema JSON is currently built with the fragile line that assigns
to the variable `schema` using a simple quote-escape which doesn't handle
backslashes or control characters; update the `schema` assignment in
scripts/register-avro-schemas.sh to canonicalize and compact the JSON first (use
`jq -c .` on the file) and then perform the quote-escaping before exporting or
using `schema` so backslashes and other special characters are preserved; ensure
you replace the existing `schema="..."` assignment with this jq-based
preprocessing and keep the subsequent code that uses the `schema` variable
unchanged.
In
`@src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/Schemas/WorkflowPublishedEvent.avsc`:
- Around line 17-43: The inline record definitions for StepSnapshotRecord and
TransitionSnapshotRecord inside WorkflowPublishedEvent.avsc are duplicating the
standalone schemas (axis.workflowbuilder.events.StepSnapshotRecord /
TransitionSnapshotRecord) and risk divergence; change
WorkflowPublishedEvent.avsc so the "steps" and "transitions" array item types
reference the existing named types instead of re-declaring them (use the named
type references "axis.workflowbuilder.events.StepSnapshotRecord" and
"axis.workflowbuilder.events.TransitionSnapshotRecord"), and update
scripts/register-avro-schemas.sh to ensure the standalone record schemas are
registered (or otherwise included) so codegen consumes the single
source-of-truth records rather than the inline copies.
In
`@src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/WorkflowBuilderEventExtensions.cs`:
- Around line 8-24: The current extension methods (WorkflowId, OrganizationId,
ReferencedFormIds) on WorkflowPublishedEvent, WorkflowArchivedEvent and
WorkflowUnarchivedEvent use Guid.Parse which throws without field-level context;
replace those parses with Guid.TryParse and, on failure, throw a FormatException
that includes the field name (e.g., "workflowId" or "organizationId") and the
offending string value to aid poison-message diagnostics; for ReferencedFormIds
iterate the collection, try-parse each entry (including index or value in the
error), collect parsed GUIDs into the same IReadOnlyList<Guid> return, and keep
method signatures (WorkflowId, OrganizationId, ReferencedFormIds) unchanged.
In `@src/Shared/Axis.Shared.Infrastructure/Messaging/CloudEventsEnvelopeRule.cs`:
- Line 55: Replace the fully-qualified System.Reflection type with a
using-import: add "using System.Reflection;" at the top of
CloudEventsEnvelopeRule.cs and change the declaration
"System.Reflection.PropertyInfo? property =
message.GetType().GetProperty(\"organizationId\");" to use the short type name
"PropertyInfo?" so the variable (property) uses PropertyInfo? from the imported
namespace; ensure the file compiles after adding the using.
In
`@tests/Modules/FormBuilder/Axis.FormBuilder.Infrastructure.Tests/Handlers/WorkflowPublishedHandlerTests.cs`:
- Around line 26-34: The tests lack a regression case for duplicated
referencedFormIds in a single WorkflowPublishedEvent payload; update the test
suite by adding a new test (or extending the existing test range around
WorkflowPublishedHandlerTests) that uses BuildEvent to construct a
WorkflowPublishedEvent where referencedFormIds contains the same form GUID
repeated, then assert that only one reference row is persisted for that form ID
(i.e., deduplication happens inside the handler). Modify or overload BuildEvent
if needed to allow constructing repeated IDs (referencedFormIds), and in the new
test verify persistence/query results (or persisted rows count) for the
deduplicated referencedFormIds after invoking the handler.
🪄 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: a8a99639-1029-4fe3-ad1a-6296c5cf33d1
⛔ Files ignored due to path filters (5)
src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/Generated/axis/workflowbuilder/events/StepSnapshotRecord.csis excluded by!**/generated/**src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/Generated/axis/workflowbuilder/events/TransitionSnapshotRecord.csis excluded by!**/generated/**src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/Generated/axis/workflowbuilder/events/WorkflowArchivedEvent.csis excluded by!**/generated/**src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/Generated/axis/workflowbuilder/events/WorkflowPublishedEvent.csis excluded by!**/generated/**src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/Generated/axis/workflowbuilder/events/WorkflowUnarchivedEvent.csis excluded by!**/generated/**
📒 Files selected for processing (40)
Axis.slnDirectory.Packages.propsdocker-compose.ymldocs/PROGRESS.mddocs/epics/E04-workflow-builder/README.mddocs/epics/E05-form-builder/README.mddocs/epics/E06-workflow-engine/README.mddocs/epics/README.mddocs/playbooks/patterns.mdscripts/register-avro-schemas.shsrc/Axis.Api/Program.cssrc/Axis.Api/appsettings.jsonsrc/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Axis.FormBuilder.Infrastructure.csprojsrc/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Handlers/WorkflowArchivedHandler.cssrc/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Handlers/WorkflowPublishedHandler.cssrc/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Handlers/WorkflowUnarchivedHandler.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/Axis.WorkflowBuilder.Contracts.csprojsrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/Schemas/StepSnapshotRecord.avscsrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/Schemas/TransitionSnapshotRecord.avscsrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/Schemas/WorkflowArchivedEvent.avscsrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/Schemas/WorkflowPublishedEvent.avscsrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/Schemas/WorkflowUnarchivedEvent.avscsrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/WorkflowBuilderEventExtensions.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/WorkflowBuilderKafkaTopics.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Axis.WorkflowBuilder.Infrastructure.csprojsrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Messaging/WorkflowBuilderEventMapper.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Persistence/WorkflowBuilderUnitOfWork.cssrc/Modules/WorkflowEngine/Axis.WorkflowEngine.Infrastructure/Axis.WorkflowEngine.Infrastructure.csprojsrc/Modules/WorkflowEngine/Axis.WorkflowEngine.Infrastructure/Handlers/WorkflowArchivedHandler.cssrc/Modules/WorkflowEngine/Axis.WorkflowEngine.Infrastructure/Handlers/WorkflowPublishedHandler.cssrc/Modules/WorkflowEngine/Axis.WorkflowEngine.Infrastructure/Handlers/WorkflowUnarchivedHandler.cssrc/Shared/Axis.Shared.Infrastructure/Axis.Shared.Infrastructure.csprojsrc/Shared/Axis.Shared.Infrastructure/Messaging/CloudEventsEnvelopeRule.cssrc/Shared/Axis.Shared.Infrastructure/Messaging/WolverineKafkaAvroExtensions.cstests/Modules/FormBuilder/Axis.FormBuilder.Infrastructure.Tests/Handlers/WorkflowArchivedHandlerTests.cstests/Modules/FormBuilder/Axis.FormBuilder.Infrastructure.Tests/Handlers/WorkflowPublishedHandlerTests.cstests/Modules/FormBuilder/Axis.FormBuilder.Infrastructure.Tests/Handlers/WorkflowUnarchivedHandlerTests.cstests/Modules/WorkflowEngine/Axis.WorkflowEngine.Infrastructure.Tests/Handlers/WorkflowArchivedHandlerTests.cstests/Modules/WorkflowEngine/Axis.WorkflowEngine.Infrastructure.Tests/Handlers/WorkflowPublishedHandlerTests.cstests/Modules/WorkflowEngine/Axis.WorkflowEngine.Infrastructure.Tests/Handlers/WorkflowUnarchivedHandlerTests.cs
|
|
||
| // Remove references no longer in the workflow | ||
| HashSet<Guid> newFormIds = [.. @event.ReferencedFormIds]; | ||
| HashSet<Guid> newFormIds = [.. referencedFormIds]; |
There was a problem hiding this comment.
Deduplicate referencedFormIds before insert loop.
If the incoming event contains duplicate form IDs, this loop can add the same (workflowId, formId, organizationId) twice in one unit of work and fall into UniqueConstraintException, skipping the sync.
Suggested fix
- foreach (Guid formId in referencedFormIds)
+ foreach (Guid formId in newFormIds)
{
FormWorkflowReference? existingRef = existing.FirstOrDefault(r => r.FormId == formId);
if (existingRef is null)
{
context.FormWorkflowReferences.Add(
FormWorkflowReference.Create(workflowId, formId, organizationId));
addedCount++;
}
else
existingRef.Reactivate();
}Also applies to: 36-43
🤖 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/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Handlers/WorkflowPublishedHandler.cs`
at line 27, The handler is inserting (workflowId, formId, organizationId)
entries for each ID in referencedFormIds and can attempt duplicate inserts when
referencedFormIds contains duplicates; change the code to deduplicate
referencedFormIds before the insert loop (e.g., construct newFormIds as a
HashSet from referencedFormIds) and iterate over that set when calling the
repository/insert logic (the variables/methods to update: referencedFormIds ->
newFormIds/HashSet<Guid>, the insert loop that uses workflowId and
organizationId), and apply the same deduplication for the other similar
loop/section around lines 36-43 so duplicates won’t trigger
UniqueConstraintException.
Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
7 tasks
This was referenced May 24, 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
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
Addresses CodeRabbit review on ADR-019 pilot: tenant-scoped WorkflowEngine upserts, deduplicated form references on publish, clearer Avro GUID parsing, CloudEvents
using, and schema registration script file checks. Adds regression test for duplicatereferencedFormIdsin one event.Linked spec
Requirements
WorkflowPublishedHandler(FormBuilder) iterates deduplicatednewFormIds; WorkflowEngine lookups filter byorganizationId.WorkflowBuilderEventExtensionsfield-levelFormatException;CloudEventsEnvelopeRuleusesPropertyInfo;register-avro-schemas.shvalidates schema files exist.avrogendoes not resolve cross-file type refs without a bundled protocol (inline definitions kept).Summary by CodeRabbit
New Features
Infrastructure Updates
Documentation
Scripts
Tests