Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the panic in the revision cache Put path by introducing structural validation for DocumentRevision and propagating errors from Put/Upsert up through the revision cache layers. It also updates callers/tests to handle the new error-returning signatures and adjusts memory-accounting tests to reflect history bytes being counted.
Changes:
- Change
RevisionCache/collection wrappers/orchestrator/sharded cachePutandUpsertto returnerror, and update call sites to handle it. - Add
DocumentRevision.IsValid()validation and return a redactable error instead of panicking when required fields are missing. - Update revision/delta cache tests to use valid history/body inputs and to assert the revised memory byte accounting; add a new test for invalid revision validation.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| rest/replicatortest/replicator_test.go | Updates test to assert Upsert returns no error after signature change. |
| db/revision_cache_test.go | Updates tests for new Put/Upsert error returns and revised memory accounting; adds invalid-revision test. |
| db/revision_cache_orchestrator.go | Propagates Put/Upsert errors and only triggers eviction on success. |
| db/revision_cache_lru.go | Removes panic, adds IsValid and returns validation errors from Put/Upsert. |
| db/revision_cache_interface.go | Updates RevisionCache interface and collection wrapper methods to return errors. |
| db/revision_cache_bypass.go | Updates bypass implementation to match new interface (returns nil error). |
| db/delta_cache_test.go | Adjusts delta cache test inputs/expectations to satisfy new validation and byte accounting. |
| db/crud.go | Handles revision cache insertion errors by logging (without failing the write). |
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.
CBG-5260
Removes explicit panic call in
Puton revision cache. Adds validation on document revision and if invalid pass error up the stack forPutandUpsertmethods.Pre-review checklist
fmt.Print,log.Print, ...)base.UD(docID),base.MD(dbName))docs/apiDependencies (if applicable)
Integration Tests