Skip to content

CBG-5260: remove panic call in rev cache Put#8237

Open
gregns1 wants to merge 4 commits intomainfrom
CBG-5260
Open

CBG-5260: remove panic call in rev cache Put#8237
gregns1 wants to merge 4 commits intomainfrom
CBG-5260

Conversation

@gregns1
Copy link
Copy Markdown
Contributor

@gregns1 gregns1 commented May 6, 2026

CBG-5260

Removes explicit panic call in Put on revision cache. Adds validation on document revision and if invalid pass error up the stack for Put and Upsert methods.

Pre-review checklist

  • Removed debug logging (fmt.Print, log.Print, ...)
  • Logging sensitive data? Make sure it's tagged (e.g. base.UD(docID), base.MD(dbName))
  • Updated relevant information in the API specifications (such as endpoint descriptions, schemas, ...) in docs/api

Dependencies (if applicable)

  • Link upstream PRs
  • Update Go module dependencies when merged

Integration Tests

Copilot AI review requested due to automatic review settings May 6, 2026 14:42
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 cache Put and Upsert to return error, 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).

Comment thread db/revision_cache_test.go
Comment thread db/revision_cache_test.go Outdated
Comment thread db/revision_cache_test.go Outdated
Comment thread db/revision_cache_test.go Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants