CBG-4753: Add version_type preference for /_changes to allow CV-aware consumers

[bbrks]

CBG-4753

• Adds a version_type option to REST API Changes feeds which has two possible options - rev (default) or cv
  • This controls whether the change entries preferentially show a revtree ID or current version value to notify clients of a new document change.
  • If old documents do not have a cv (they were written prior to 4.0, we'll still continue to send the rev version of the change).
  • This parameter is documented as a preference only, and notes the case where rev may still be produced on a feed requesting cv

Integration Tests

• GSI=true,xattrs=true https://jenkins.sgwdev.com/job/SyncGatewayIntegration/37/

[github-actions]

Redocly previews

• Admin API
• Public API
• Metric API
• Diagnostic API

[torcolvin]

There are some general thoughts in this code review but the

[torcolvin]

Since this is pulled into a function, do you think this makes sense to check if len(ce.Changes) > 1 before going into this function to avoid a panic if this is called in a place that is unexpected and there are somehow no changes?

I think in practice a ChangeEntry should always have changes but it is sometimes constructed from an empty struct so there could be a case where it is buggy and I don't want Sync Gateway to panic in prod.

[bbrks]

I added an AssertfCtx - all of this codepath isn't set up to handle bubbling up errors, but realistically it's an assertion failure anyway and at least avoids a potential panic in prod.

[torcolvin]

If I read this right, VersionType will be empty string for blip or active replicator. Do you think this is confusing if this code was used later? or we should log as %q?

For blip changes feeds probably using cv mode makes sense, but it also doesn't really matter. I'm trying to figure out a way to make debugging this the least amount of confusing.

Should we enforce when calling MultiChangesFeed that VersionType is not an empty string, and then I think this would make some of the other code easier to understand:

	row.Changes = []ChangeByVersionType{{ChangesVersionTypeRevTreeID: populatedDoc.CurrentRev}}
	switch options.VersionType {
	case ChangesVersionTypeCV:
		row.Changes[0] = ChangeByVersionType{options.VersionType: populatedDoc.HLV.GetCurrentVersionString()} 2Code has comments. Press enter to view.
	case ChangesVersionTypeRevTreeID:
		fallthrough
	default:
		// already initialized with a 'rev' change entry above
	}

could become which is more obvious to me than knowing everywhere that options.VersionType = "" means ChangesVersionTypeRevTreeID:

	switch options.VersionType {
	case ChangesVersionTypeCV:
		row.Changes[0] = ChangeByVersionType{options.VersionType: populatedDoc.HLV.GetCurrentVersionString()} 2Code has comments. Press enter to view.
	case ChangesVersionTypeRevTreeID:
        row.Changes = []ChangeByVersionType{{ChangesVersionTypeRevTreeID: populatedDoc.CurrentRev}}
	default:
         < err here>
	}

[bbrks]

Replaced with %q to make the empty string logging clearer.

There's already a ticket filed and a TODO to make BLIP use the version type option, but it's not really relevant for this PR.

I kind of agree on setting explicit VersionType values and removing the fallbacks, but it's affecting BLIP code that isn't implemented yet, and it would technically introduce more error codepaths deeper into changes code, which are awkward to handle, since there's no fallback available.

[torcolvin]

I think you could actually avoid changing this API this by just figuring out what ce.changes[0] key is.

It might also be possible to just change ce.ChangeVersionString to just return whichever key is present, since it seems like it should always use cv if it is present and otherwise fall back to rev

One way I thought of to make ce.ChangeVersionString easier (or other code) is to change ChangeByVersionType to be a different type of struct:

type ChangeVersion struct {
	revtreeID string
	cv string
}

// basically this validates that we never have both when we go to marshal the data to conform to the rest api or blip api?
func (c *ChangeVersion) MarshalJSON() ([]byte, error) {
	if c.revtreeID != "" && c.cv != "" {
		return nil, fmt.Errorf("having both revtree %q and cv %q is invalid", c.revtreeID, c.cv)
	}
	<marshal as map>
}

[bbrks]

What does this change give us other than indirection in the JSON Marshalling? Accessing fields by .revtreeID instead of ["rev"]?

Do you think that's more beneficial than a closer representation of the wire-protocol for this data type? I'm not convinced

[bbrks]

Ok, I changed to use a ce.ChangeVersionString(), which just returns the first version type it found in ce

We have to assume there's only one version type present with this approach - which should be true. That's the cost of being less explicit with preferred version type than the approach previously in this PR.

[Copilot]

Pull Request Overview

This PR adds a version_type parameter to the REST API /_changes endpoint that allows CV-aware consumers to request Current Version (CV) values instead of the default Revision Tree IDs. The parameter accepts either rev (default) or cv values, enabling clients to choose their preferred document versioning scheme while maintaining backward compatibility.

Key changes:

• Added version_type parameter support for both GET and POST /_changes requests
• Implemented fallback logic to return rev for older documents that don't have CV values
• Updated test utilities and existing tests to work with the new versioning system

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
rest/changes_api.go	Adds parsing and validation of version_type parameter for changes feed requests
rest/changes_test.go	Comprehensive test coverage for different version types and feed configurations
rest/utilities_testing.go	Updates test helper function to support both revision and CV version types
db/changes.go	Core implementation of version type handling in changes feed logic
db/sequence_id.go	Improves error messages with actual sequence values
docs/api/	OpenAPI specification updates for the new parameter
Various test files	Updates existing tests to use new test helper function signature

---

_{You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.}