fix(cdc): snapshot canonical change record maps - #222
Closed
seonghobae wants to merge 2 commits into
Closed
Conversation
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
This was referenced Aug 10, 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.
Purpose
Fix #221 test-first from exact protected
develop@622e5e6c3d534f230c390f10e3832efadfc01825.CanonicalChangeRecordexposed unmodifiable views of caller-owned maps rather than construction-time snapshots, so the record and its equality/hash identity could change after construction when a caller mutated the backing map.This direct-
developDraft changes only the canonical CDC value object and its focused test. It does not alter registry/discovery, Kafka publication, connector support, database state, migrations, dependencies, workflows, or canonical #149 documentation.Exact current identity
develop@622e5e6c3d534f230c390f10e3832efadfc01825;fix/cdc-canonical-record-snapshot-622e5e6;770c19c366cdbee0e92faea3312cbea747078cfd;621e7c25dae2081534d5fd94f2ac294128d1a8ff;ef61e5cd8abb4c3561d072c87220cd1f13b0388e;Every check/review from another head/base remains historical and does not transfer.
RED → GREEN
CanonicalChangeRecordTestreaches the real CDC value object and requires:before,after, andpkmaps after construction must not alter the record or itshashCode();Hosted fail-first CI
31369371587, macOS job93394688767, compiled the reactor and ran CDC with exactly 109 tests, exactly one failure, zero errors/skips:CanonicalChangeRecordTest.snapshotsMutableInputMapsexpected the construction-time valueoldbut observed caller mutationmutated. The direct-unmodifiable and null-value assertions were green. This is valid production-boundary RED rather than setup/import/fixture/dependency failure.Exact current source head
621e7c25dae2081534d5fd94f2ac294128d1a8ffapplies the smallest correction: null/empty maps normalize toMap.of(), non-empty maps are shallow-copied into insertion-order-preservingLinkedHashMapsnapshots and exposed as unmodifiable maps, and the public class/constructor/getter contracts now have beginner-readable Javadoc. Scalar metadata, nested-value semantics, equality/hashCode fields and public API shape remain unchanged.Current hosted evidence
All exposed workflow aggregates associated with exact current source head are terminal-success:
31370594745: success on Ubuntu, macOS and Windows;31370594733: success;31370594735: success;31370594683: success;31370594746: aggregate success.CI macOS job
93398461439checked out synthetic mergeef61e5cd8abb4c3561d072c87220cd1f13b0388e, not literal source head. On that merged treeCanonicalChangeRecordTestpassed 3/3, CDC passed 109/109, ETL passed 275/275 and the full reactor completedBUILD SUCCESS. This proves merged-tree behavior compatibility, not literal-source acceptance.The same CI log independently reproduces the protected JaCoCo false-green:
Analyzed bundle 'etl-service' with 0 classesfollowed by coverage checks met. #162/#164 owns that root-cause repair, while #205 owns repository-wide coverage scope.Security Scan Trivy job
93398461727also checked out synthetic mergeef61e5cd...and printedDependency version cannot be determined. Child dependencies will not be found.before reporting zero CRITICAL/HIGH/MEDIUM SARIF findings. #196 owns scanner-completeness; the aggregate green is therefore not accepted as complete same-revision vulnerability evidence. The scan also continues to observe the tracked 82 MBzipkin.jar, owned by #168/#169.Documentation / traceability
#159 now records this change as
active_prand the durable architecture invariant: canonical CDC row maps are construction-time snapshots so caller mutation cannot alter value-object identity. Unstable SHAs/runs stay in dated evidence rather than timeless architecture.Merge boundary
Keep Draft. The product defect is GREEN on the synthetic integration tree, but accepted literal-source execution/security evidence, complete dependency/vulnerability evidence, non-vacuous applicable owned-production coverage, current repository-wide quality scope, and any live protected merge/review requirements remain independent gates. Do not weaken those controls or copy shared workflow/coverage/scanner fixes into this two-file CDC slice.