Repository navigation
Conversation
| journalEventSequencer, | ||
| targetSystem.getTxWrapper(), | ||
| journalEventsEnabled); | ||
| new SqlTxWrapper(new TransactionManager<>(this::openAuditConnection)), |
Contributor
There was a problem hiding this comment.
This opens its own separate DB connection instead of reusing the target system's one. That means the audit record and the actual change can now commit/fail independently — losing the all-or-nothing guarantee.
| }); | ||
| journalEventSequencer.confirm(); | ||
| return result; | ||
| synchronized (journalEventSequencer) { |
Contributor
There was a problem hiding this comment.
If the journal feature flag gets turned on after this object was created, journalEventSequencer can be null here and this will crash. MongoDB's version checks for null first; this one doesn't.
| // consumer tell "in flight" from "lost". | ||
| journalEventSequencer.confirm(); | ||
| return result; | ||
| synchronized (journalEventSequencer) { |
Contributor
There was a problem hiding this comment.
Same issue as the DynamoDB version: possible null journalEventSequencer here if the flag flips on later, causing a crash. No null check like MongoDB has.
| try { | ||
| TransactWriteItemsEnhancedRequest.Builder builder = TransactWriteItemsEnhancedRequest.builder(); | ||
| io.flamingock.internal.util.Result result = journalWriter.write(builder, sequencer, entry); | ||
| new DynamoDBUtil(client).getEnhancedClient().transactWriteItems(builder.build()); |
Contributor
There was a problem hiding this comment.
This writes directly to DynamoDB, skipping the shared transaction wrapper every other store uses here. Errors from this call won't be handled/classified the same way as the rest of the codebase.
| @MethodSource("dialectProvider") | ||
| @DisplayName("a stage snapshots the journal flag once and uses the captured value") | ||
| @DisplayName("a journal-enabled store bootstraps storage before a stage writes") | ||
| void stageSnapshotsJournalFlagOnce(SqlDialect sqlDialect, String dialectName) throws Exception { |
Contributor
There was a problem hiding this comment.
This test used to check what happens if the flag changes value between setup and write. That check was removed and not replaced, so there's no test left covering the null-crash risk mentioned above.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Mutable feature-flag reads can diverge from initialization-time sequencer and storage setup across four audit stores.
Review effort: Balanced
Findings: 4
Open (4)
What changed in this PR
Adds injectable audit-history and journal-only appenders across supported audit stores, with shared stream sequencing and durable-tail recovery.
Changes:
- Registers independent history appenders in the runtime context.
- Implements appenders for SQL, MongoDB, DynamoDB, and Couchbase.
- Moves journal storage preparation to store initialization and expands tests.
| File | Description |
|---|---|
core/flamingock-core/src/test/java/io/flamingock/internal/core/journal/JournalEventSequencerTest.java |
Tests shared sequencing and reconciliation. |
core/flamingock-core/src/test/java/io/flamingock/internal/core/builder/HistoryAppenderProviderTest.java |
Tests appender dependency registration. |
core/flamingock-core/src/main/java/io/flamingock/internal/core/journal/JournalEventSequencerFactory.java |
Shares sequencers per stream. |
core/flamingock-core/src/main/java/io/flamingock/internal/core/journal/JournalEventSequencer.java |
Reconciles uncertain writes. |
core/flamingock-core/src/main/java/io/flamingock/internal/core/external/store/HistoryAppenderProvider.java |
Defines store appender capability. |
core/flamingock-core/src/main/java/io/flamingock/internal/core/external/store/audit/community/AbstractCommunityAuditPersistence.java |
Simplifies persistence initialization. |
core/flamingock-core/src/main/java/io/flamingock/internal/core/builder/AbstractChangeRunnerBuilder.java |
Registers history appenders. |
core/flamingock-core-commons/src/main/java/io/flamingock/internal/common/core/audit/JournalHistoryAppender.java |
Defines journal-only append API. |
core/flamingock-core-commons/src/main/java/io/flamingock/internal/common/core/audit/AuditHistoryAppender.java |
Defines audit-history append API. |
community/flamingock-sql-auditstore/src/test/java/io/flamingock/store/sql/SqlAuditStoreTest.java |
Covers SQL initialization and independent writes. |
community/flamingock-sql-auditstore/src/test/java/io/flamingock/store/sql/internal/SqlJournalDialectHelperTest.java |
Updates schema helper assertions. |
community/flamingock-sql-auditstore/src/test/java/io/flamingock/store/sql/internal/SqlAuditPersistenceJournalTest.java |
Tests SQL journal writer behavior. |
community/flamingock-sql-auditstore/src/main/java/io/flamingock/store/sql/SqlAuditStore.java |
Provides SQL history appenders. |
community/flamingock-sql-auditstore/src/main/java/io/flamingock/store/sql/internal/SqlJournalHistoryAppender.java |
Implements transactional journal-only writes. |
community/flamingock-sql-auditstore/src/main/java/io/flamingock/store/sql/internal/SqlAuditPersistence.java |
Reuses SQL journal writer. |
community/flamingock-mongodb-sync-auditstore/src/test/java/io/flamingock/store/mongodb/sync/MongoDBSyncHistoryProviderTest.java |
Tests synchronous MongoDB appenders. |
community/flamingock-mongodb-sync-auditstore/src/test/java/io/flamingock/store/mongodb/sync/internal/MongoDBSyncAuditPersistenceJournalTest.java |
Updates synchronous persistence tests. |
community/flamingock-mongodb-sync-auditstore/src/main/java/io/flamingock/store/mongodb/sync/MongoDBSyncAuditStore.java |
Provides synchronous MongoDB appenders. |
community/flamingock-mongodb-sync-auditstore/src/main/java/io/flamingock/store/mongodb/sync/internal/MongoDBSyncJournalWriter.java |
Shares synchronous journal writes. |
community/flamingock-mongodb-sync-auditstore/src/main/java/io/flamingock/store/mongodb/sync/internal/MongoDBSyncJournalEventStore.java |
Exposes journal initialization. |
community/flamingock-mongodb-sync-auditstore/src/main/java/io/flamingock/store/mongodb/sync/internal/MongoDBSyncAuditRepository.java |
Exposes historical append operations. |
community/flamingock-mongodb-sync-auditstore/src/main/java/io/flamingock/store/mongodb/sync/internal/MongoDBSyncAuditPersistence.java |
Integrates shared journal writing. |
community/flamingock-mongodb-reactive-auditstore/src/test/java/io/flamingock/store/mongodb/reactive/MongoDBReactiveAuditStoreJournalTest.java |
Tests reactive appenders and initialization. |
community/flamingock-mongodb-reactive-auditstore/src/test/java/io/flamingock/store/mongodb/reactive/internal/MongoDBReactiveAuditPersistenceTest.java |
Updates reactive persistence setup. |
community/flamingock-mongodb-reactive-auditstore/src/test/java/io/flamingock/store/mongodb/reactive/internal/MongoDBReactiveAuditPersistenceJournalTest.java |
Updates reactive journal tests. |
community/flamingock-mongodb-reactive-auditstore/src/main/java/io/flamingock/store/mongodb/reactive/MongoDBReactiveAuditStore.java |
Provides reactive MongoDB appenders. |
community/flamingock-mongodb-reactive-auditstore/src/main/java/io/flamingock/store/mongodb/reactive/internal/MongoDBReactiveJournalWriter.java |
Shares reactive journal writes. |
community/flamingock-mongodb-reactive-auditstore/src/main/java/io/flamingock/store/mongodb/reactive/internal/MongoDBReactiveAuditRepository.java |
Exposes historical append operations. |
community/flamingock-mongodb-reactive-auditstore/src/main/java/io/flamingock/store/mongodb/reactive/internal/MongoDBReactiveAuditPersistence.java |
Integrates shared journal writing. |
community/flamingock-dynamodb-auditstore/src/test/java/io/flamingock/store/dynamodb/internal/DynamoDBAuditPersistenceJournalTest.java |
Updates DynamoDB persistence tests. |
community/flamingock-dynamodb-auditstore/src/test/java/io/flamingock/store/dynamodb/DynamoDBAuditStoreJournalTest.java |
Tests DynamoDB history appenders. |
community/flamingock-dynamodb-auditstore/src/main/java/io/flamingock/store/dynamodb/internal/DynamoDBJournalWriter.java |
Stages journal transaction writes. |
community/flamingock-dynamodb-auditstore/src/main/java/io/flamingock/store/dynamodb/internal/DynamoDBAuditRepository.java |
Exposes historical append operations. |
community/flamingock-dynamodb-auditstore/src/main/java/io/flamingock/store/dynamodb/internal/DynamoDBAuditPersistence.java |
Integrates shared journal writing. |
community/flamingock-dynamodb-auditstore/src/main/java/io/flamingock/store/dynamodb/DynamoDBAuditStore.java |
Provides DynamoDB history appenders. |
community/flamingock-couchbase-auditstore/src/test/java/io/flamingock/store/couchbase/internal/CouchbaseAuditPersistenceJournalTest.java |
Tests shared Couchbase writer behavior. |
community/flamingock-couchbase-auditstore/src/test/java/io/flamingock/store/couchbase/CouchbaseAuditStoreFacadeTest.java |
Tests Couchbase appender facade. |
community/flamingock-couchbase-auditstore/src/main/java/io/flamingock/store/couchbase/internal/CouchbaseJournalWriter.java |
Stages Couchbase journal events. |
community/flamingock-couchbase-auditstore/src/main/java/io/flamingock/store/couchbase/internal/CouchbaseAuditPersistence.java |
Integrates shared journal writing. |
community/flamingock-couchbase-auditstore/src/main/java/io/flamingock/store/couchbase/internal/CouchbaseAuditor.java |
Exposes historical append operations. |
community/flamingock-couchbase-auditstore/src/main/java/io/flamingock/store/couchbase/CouchbaseAuditStore.java |
Provides Couchbase history appenders. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| @@ -88,24 +59,24 @@ public Result writeEntry(AuditEntry auditEntry) { | |||
| // without them, the audit record set is itself the history. | |||
| if (FeatureFlag.isEnabled(Features.JOURNAL_EVENTS)) { | |||
| @@ -79,18 +61,24 @@ public List<AuditEntry> getAuditHistory() { | |||
| @Override | |||
| public Result writeEntry(AuditEntry auditEntry) { | |||
| if (isJournalEventsEnabled()) { | |||
Comment on lines
87
to
88
| throw new IllegalStateException("MongoDB reactive journal writes require a sequencer"); | ||
| } |
| } | ||
| return writeJournalAndAuditInTransaction(auditEntry); | ||
| } else { | ||
| if (FeatureFlag.isDisabled(Features.JOURNAL_EVENTS, false)) { |
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.
Why
System changes currently have the normal audit writer, whose behavior depends on
JOURNAL_EVENTS: with the feature enabled, it updates current audit state and appends a journal event. That path cannot preserve every imported historical audit record or append journal events without changing audit state.These operations need independent write paths that system changes can use without handling store-specific transactions.
What changed
JOURNAL_EVENTSis enabled while preserving normal-write behavior.