-
Notifications
You must be signed in to change notification settings - Fork 140
CBG-4751: Allow CV to be used as OCC value in REST API for document writes (updates/deletes) #7693
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ociated attachment endpoints) using existing `rev` parameter - which can automatically detect RevTreeID/CV. Switch test helpers to use CV in writes when available for coverage, and allow fallback to RevTreeID. Prevent CV OCC value in doc updates for docs in conflict or against non-current versions, since we don't maintain linear version history like we do for RevTrees and can't correlate an old CV with a branched revision ID.
Redocly previews |
… Rev generation but makes the code simpler
torcolvin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM in general.
| // Can't use CV as an OCC Value when a document is in conflict, or we're updating the non-winning leaf | ||
| // There's no way to get from a given old CV to a RevTreeID to perform the update correctly, since we don't maintain linear history for a given SourceID. | ||
| // Reject the request and force the user to resolve the conflict using RevTree IDs which does have linear history available. | ||
| conflictErr = base.HTTPErrorf(http.StatusBadRequest, "Cannot use CV to modify a document in conflict - resolve first with RevTree ID") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any way that this is going to get hit for a blip code pathway? If it does, will the blip code know how to handle a 400 with this error message?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, it can't happen.
BLIP goes through PutExistingCurrentVersion since the client is the one generating a new CV value and pushing it, not relying on SG to generate the HLV entry. Put is SG/REST API-only writes.
There was a problem hiding this 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 allows Current Version (CV) to be used as an optimistic concurrency control (OCC) value in REST API document operations (updates/deletes) using the existing rev parameter, which can now automatically detect whether the value is a RevTreeID or CV. The changes also update test helpers to use CV when available for better test coverage and prevent CV usage on conflicted documents since linear version history isn't maintained for CVs.
- Add CV support for OCC in document write operations (PUT/DELETE)
- Update test utilities to prefer CV over RevTreeID when available
- Add validation to prevent CV usage on documents in conflict
Reviewed Changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| rest/doc_api.go | Add CV support for OCC operations and new helper functions for version type detection |
| rest/utilities_testing_resttester.go | Update test helpers to use CV when available with fallback to RevTreeID |
| rest/utilities_testing.go | Add GetRawDoc helper and update DocVersionFromPutResponse to handle CV responses |
| db/crud.go | Update DeleteDoc to accept DocVersion and add CV validation in Put operations |
| db/document.go | Move DocVersion struct definition and add new helper methods |
| topologytest/sync_gateway_peer_test.go | Update DeleteDocument to use ExtractDocVersion() |
| docs/api/* | Update OpenAPI documentation to reflect CV support and terminology improvements |
| base/util.go | Add IsRevTreeID helper function for version type detection |
CBG-4751
revparameter - which can automatically detect RevTreeID/CV.Integration Tests
GSI=true,xattrs=truehttps://jenkins.sgwdev.com/job/SyncGatewayIntegration/47/