Repository navigation
Conversation
Introduces AuditEntry (audit_log collection) and AuditService, which records actor/timestamp/before/after for every schema publish, deprecate, and rollback, plus every record patch, replace, and soft delete. Exposes GET /admin/audit with optional entityType/entityId/ action filters and pagination (reusing the existing query guardrail for max page size). Verified end-to-end against a real MongoDB/Kafka stack: full test suite passes (101/101), and a live run confirms publish/patch/delete actions are correctly recorded with before/after snapshots and are filterable by entity and action. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- AuditService.record() now catches and logs write failures instead of propagating them, so a transient Mongo error can no longer turn an already-successful schema/record mutation into a client-visible 500 (6 call sites across SchemaLifecycleService and DynamicRecordService were all affected). - Extract actor resolution into a shared CurrentActorResolver component, used by both SchemaLifecycleService and AuditService. - Extract pagination guardrail logic into a shared PageRequestGuard, used by both DynamicQueryService and AuditService. - Add ADMIN-role security coverage for GET /admin/audit to SchemaAdminControllerSecurityIntegrationTest. - Split AuditEntry's entityType/entityId into entityType (fixed category: SCHEMA/RECORD) + entityName (the dynamic entity), so /admin/audit?entityType=RECORD now returns mutations across every entity instead of requiring an exact "RECORD:<entity>" match. - Add the audit hook that was still missing: FormSubmissionService's record-create path (the one that was never covered) now emits a RECORD_CREATED entry. Found by re-running the real MongoDB/Kafka end-to-end check after the entityName fix and noticing created records never showed up in the audit log. Full suite: 103/103 passing against real MongoDB + Kafka. Verified live that entityType=RECORD without entityName returns audit rows across multiple entities, and that entityName narrows it back down. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Reject reserved fields (_id, deleted, etc.) in FormSubmissionService via a shared ReservedFieldGuard, closing a hole where a client could silently overwrite an existing record by including its _id in a form submission (and have it mislabeled RECORD_CREATED). - Add audit hooks to SchemaAdminController's field-definition and field-group CRUD endpoints (create/update/delete) -- the same class of "forgot to audit a mutation path" gap the previous fix round found and closed for FormSubmissionService, missed here. - Add CurrentActorResolverTest exercising a real Authentication/ SecurityContext instead of always mocking the resolver. - Introduce AuditEntityType enum (SCHEMA/FIELD_DEFINITION/FIELD_GROUP/ RECORD) for compile-time-safe writes and validated, 400-on-typo reads instead of an unvalidated, freeform entityType string. - Narrow AuditService.record()'s catch to DataAccessException so a future programming bug in the method isn't silently swallowed alongside genuine Mongo infra failures. - Make AuditService.record() @async so audit writes no longer add synchronous latency to every mutation's hot path, backed by a dedicated bounded executor. - Extract MongoDocumentIds.stringify() to replace three independent copies of the same null-safe "_id" -> String extraction idiom. - Extract a local addEqualsIfPresent() helper in AuditService.query() instead of repeating the same blank-check four times. - Extract recordSchemaAudit()/statusSnapshot() helpers in SchemaLifecycleService and recordAudit() in DynamicRecordService to remove duplicated before/after construction across publish/ deprecate/rollback and patch/replace/softDelete. - Record audit timestamps in UTC instead of the JVM's default zone. Regression caught and fixed during end-to-end verification of the above: making record() @async moved it onto a separate thread, and Spring Security's ThreadLocal-based SecurityContextHolder doesn't propagate there by default, so every audit entry's actor silently became "system" regardless of who was authenticated. Fixed by wrapping the executor with Spring Security's DelegatingSecurityContextAsyncTaskExecutor, and re-verified live with a real JWT that the actor is now correctly captured. Full suite: 117/117 passing against real MongoDB + Kafka. Verified live: _id-bearing form submissions are rejected with 400, field- definition/group CRUD now produces FIELD_DEFINITION_*/FIELD_GROUP_* audit entries, an unknown entityType filter returns 400 with the allowed values, and actor attribution survives the async write. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The @async fix from the previous round introduced 5 confirmed regressions, found via decompiled-bytecode verification of Spring's actual runtime behavior: - AsyncAuditConfig's executor had no RejectedExecutionHandler, so a full queue threw RejectedExecutionException synchronously on the caller's thread (at task-submission time, before record()'s own try/catch ever runs) -- completely bypassing the "audit failures never surface to the caller" contract. Fixed with a discard-and-log handler. - AuditService.record()'s try block had been narrowed to wrap only mongoTemplate.save(), no longer covering entry construction or actor resolution; and its catch had been narrowed to DataAccessException, missing MappingException (which extends RuntimeException directly, not DataAccessException, and is exactly what a converter failure on the untyped before/after payload would throw). Reverted to wrapping the whole method body in catch(RuntimeException) -- correct here since this method's sole job is "never let an audit-logging failure fail the mutation", regardless of failure type. - The UTC timestamp fix from the previous round was itself broken: decompiled bytecode confirms Spring Data's LocalDateTime write path uses ZoneId.systemDefault() while MongoDB's Date->LocalDateTime read path uses a hardcoded UTC zone, so a UTC LocalDateTime written and read back would be shifted by the server's UTC offset on any non-UTC-configured host. Switched AuditEntry.timestamp to Instant, which round-trips correctly through both converters. - SchemaLifecycleService.deprecate()'s statusSnapshot() called Map.of() with a nullable Integer version; Map.of() throws NPE on a null value, and since Java evaluates method arguments eagerly on the caller's thread before the @Async-proxied record() call is even dispatched, this NPE would happen synchronously and uncaught. Switched to a plain LinkedHashMap. - Only the DelegatingSecurityContextAsyncTaskExecutor wrapper was registered as a Spring bean; the inner ThreadPoolTaskExecutor (which implements DisposableBean) was a plain object Spring never saw, so its thread pool was never shut down on context close. Registered it as its own bean so Spring manages its lifecycle. Also addressed: - ReservedFieldGuard only checked reserved field names at the top level; nested reserved fields (e.g. {"profile":{"_id":"x"}}) passed through untouched on both the DynamicRecordService and FormSubmissionService paths. Made rejection recursive. - SchemaAdminController's 6 audit call sites repeated a duplication pattern already extracted into a helper twice elsewhere in this PR (DynamicRecordService.recordAudit(), SchemaLifecycleService's recordSchemaAudit()/statusSnapshot()). Added the same helper here. - No test exercised AuditService.record() through a real Spring @async proxy -- every existing test bypassed Spring AOP by constructing the service directly, so the actor-propagation fix from the previous round had zero regression coverage. Added AuditServiceAsyncIntegrationTest, which boots a minimal real Spring context and confirmed (while writing it) that record() genuinely runs on a different, dedicated-executor thread and that the actor survives the boundary. - SchemaLifecycleE2EIntegrationTest was the one full-context test using the real, now-async AuditService bean against a real MongoTemplate with a per-test full-database drop -- a latent race between an in-flight audit write and the next test's drop(). Now mocks AuditService like every other test, since this suite tests schema lifecycle behavior, not the audit trail itself. Full suite: 130/130 passing against real MongoDB + Kafka. Verified live: a nested reserved field ({"nested":{"_id":"..."}}) is now rejected, audit timestamps now serialize as proper UTC instants (...Z suffix, unambiguous), and the deprecate flow still works end-to-end after the statusSnapshot() fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
AuditEntry(audit_logcollection) +AuditService.record()to capture actor/timestamp/before/after for every schema publish/deprecate/rollback and every record patch/replace/soft-delete.GET /admin/auditwith optionalentityType/entityId/actionfilters and pagination (reuses the existingQueryGuardrailProperties.maxPageSizeguardrail).Test plan
SchemaLifecycleServiceTest(21/21),DynamicRecordServiceTest(5/5), newAuditServiceTest(4/4)/admin/auditwith correct before/after snapshots, correct actor resolution, and correct filtering byentityIdandaction🤖 Generated with Claude Code