feat: implement envelope operation logging with cursor pagination#764
feat: implement envelope operation logging with cursor pagination#764
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds envelope operation logging: new DB types, DbService methods for appending and paginating logs, a /logs HTTP endpoint, GraphQL mutation hooks to emit logs (best-effort), migrations (index + backfill) run at startup/tests, and unit + e2e tests for storage and retrieval. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant GraphQL as GraphQL Server
participant HTTP as HTTP Server
participant DbService
participant DB as Neo4j
Client->>GraphQL: mutation (create/update/delete/update_envelope_value)
GraphQL->>DB: persist envelope change
GraphQL->>GraphQL: compute envelopeHash / lookup metaEnvelopeId & ontology
GraphQL->>DbService: appendEnvelopeOperationLog(params) (fire-and-forget)
DbService->>DB: CREATE EnvelopeOperationLog node
Client->>HTTP: GET /logs (X-ENAME, limit, cursor)
HTTP->>DbService: getEnvelopeOperationLogs(eName, {limit, cursor})
DbService->>DB: MATCH logs ORDER BY timestamp DESC, id ASC WITH cursor filter
DB-->>DbService: paginated records
DbService-->>HTTP: { logs, nextCursor, hasMore }
HTTP-->>Client: 200 { logs, nextCursor, hasMore }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@infrastructure/evault-core/src/core/db/db.service.ts`:
- Around line 978-1007: The cursor comparison in the cypher used by
runQueryInternal for EnvelopeOperationLog is inconsistent with the ORDER BY
clause: with "ORDER BY l.timestamp DESC, l.id ASC" the cursor predicate
"(l.timestamp < $cursorTs) OR (l.timestamp = $cursorTs AND l.id < $cursorId)" is
wrong and should use l.id > $cursorId for correct pagination; update the query
string to use "l.id > $cursorId" in the cursor branch (and keep the same
parameter names cursorTs/cursorId and limitPlusOne) so results for equal
timestamps are correctly ordered and no items are skipped or duplicated.
In `@infrastructure/evault-core/src/core/protocol/graphql-server.ts`:
- Around line 7-10: The import of computeEnvelopeHash and
computeEnvelopeHashForDelete in graphql-server.ts fails because
../db/envelope-hash does not exist or exports differ; either restore/create the
module exporting those two functions or correct the import to the actual module
that provides them (e.g., update the path or named exports to match where
computeEnvelopeHash and computeEnvelopeHashForDelete are defined), then run the
build to verify the import resolves.
- Around line 244-260: The post-write call to appendEnvelopeOperationLog (which
uses computeEnvelopeHash and result.metaEnvelope.id) must not cause the mutation
to fail if logging errors; change the logic so the main write completes
atomically and the log is recorded best-effort—either include the envelope log
in the same DB transaction as the write (so both commit or both rollback) or, if
a transaction is not feasible, invoke appendEnvelopeOperationLog without
blocking the response and wrap it in a try/catch (or attach .catch) to
swallow/log errors; update the code paths that call appendEnvelopeOperationLog
(e.g., in the create mutation where result.metaEnvelope is used) so failures in
appendEnvelopeOperationLog never bubble up and cause duplicate side effects.
- Around line 349-363: The audit log is being appended before the actual
deletion, and meta may be null; move logging to occur only after confirming
deletion succeeds (or perform both actions inside a DB transaction).
Specifically: call findMetaEnvelopeById(id, context.eName) to capture ontology
first, then call deleteMetaEnvelope(id, context.eName) and verify its
success/return value, and only then call appendEnvelopeOperationLog with
computeEnvelopeHashForDelete(id), platform, timestamp and ontology (use
null/explicit fallback if meta is missing). Alternatively wrap
deleteMetaEnvelope and appendEnvelopeOperationLog in a transaction to ensure
atomicity.
- Around line 379-405: Create the missing module ../db/envelope-hash that
exports computeEnvelopeHash and computeEnvelopeHashForDelete, implement
deterministic hashing of the provided envelope inputs (e.g., ontology and
payload) and ensure the functions are used where computeEnvelopeHash is called
in graphql-server.ts; specifically, make computeEnvelopeHash include an id field
in the payload for the "update_envelope_value" operation (use the envelopeId
parameter as id) so its inputs match the create/update hash shape, and implement
computeEnvelopeHashForDelete to accept the same identifying fields used when
deleting; ensure exports match the named imports used by graphql-server.ts.
In `@infrastructure/evault-core/src/e2e/evault-core.e2e.spec.ts`:
- Around line 600-630: The test uses the variable client inside the "GET /logs
endpoint" block but never initializes it; add a beforeAll (and matching afterAll
if needed) in that describe block to set up client the same way other suites do
(e.g., call whatever setup used elsewhere to assign client and ensure evault1 is
available), so that client.storeMetaEnvelope and subsequent fetch calls run
against a valid client instance; reference the describe block name "GET /logs
endpoint", the variable client, and the method storeMetaEnvelope to locate where
to add the beforeAll/afterAll setup and teardown.
In `@infrastructure/evault-core/src/index.ts`:
- Around line 120-126: The dynamic import/usage of backfillEnvelopeOperationLogs
in index.ts is referencing a missing migration module and will fail at runtime;
either add an exported async function backfillEnvelopeOperationLogs that accepts
the existing driver and performs the backfill, and place it where the dynamic
import in index.ts expects to load it, or remove the try/catch block and call
entirely (including the import and await backfillEnvelopeOperationLogs(driver))
if the migration isn’t needed yet—ensure the symbol name
backfillEnvelopeOperationLogs and the driver argument remain consistent with the
current code when you implement or remove the call.
1b7566b to
11e220c
Compare
bf4e3ab to
36b46c7
Compare
Description of change
Issue Number
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit
New Features
Chores
Documentation
Tests