From 51436ae5b58109b37f5e0a2b3ab3b7073ffaee82 Mon Sep 17 00:00:00 2001 From: Tor Colvin Date: Thu, 29 Aug 2024 00:01:11 -0400 Subject: [PATCH] [3.2.1 Backport] CBG-4201 CBG-4167 add missing Stringer for SubChangesParams and ChangesOptions (#7095) - simplify NewSubChangesParams when adding another option since the existing options were taking the same code --- db/blip_handler.go | 5 ++--- db/blip_sync_messages.go | 29 +++++++++++++++++++++-------- db/changes.go | 3 ++- db/changes_test.go | 26 ++++++++++++++++++++++++++ rest/blip_sync_messages_test.go | 4 ++-- 5 files changed, 53 insertions(+), 14 deletions(-) diff --git a/db/blip_handler.go b/db/blip_handler.go index f7804bbe9e..f9e8823810 100644 --- a/db/blip_handler.go +++ b/db/blip_handler.go @@ -259,12 +259,11 @@ func (bh *blipHandler) handleSetCheckpoint(rq *blip.Message) error { // Received a "subChanges" subscription request func (bh *blipHandler) handleSubChanges(rq *blip.Message) error { - defaultSince := CreateZeroSinceValue() latestSeq := func() (SequenceID, error) { seq, err := bh.collection.LastSequence(bh.loggingCtx) return SequenceID{Seq: seq}, err } - subChangesParams, err := NewSubChangesParams(bh.loggingCtx, rq, defaultSince, latestSeq, ParseJSONSequenceID) + subChangesParams, err := NewSubChangesParams(bh.loggingCtx, rq, latestSeq, bh.db.Options.ChangesRequestPlus) if err != nil { return base.HTTPErrorf(http.StatusBadRequest, "Invalid subChanges parameters") } @@ -318,7 +317,7 @@ func (bh *blipHandler) handleSubChanges(rq *blip.Message) error { requestPlusSeq := uint64(0) // If non-continuous, check whether requestPlus handling is set for request or via database config if continuous == false { - useRequestPlus := subChangesParams.requestPlus(bh.db.Options.ChangesRequestPlus) + useRequestPlus := subChangesParams.requestPlus() if useRequestPlus { seq, requestPlusErr := bh.db.GetRequestPlusSequence() if requestPlusErr != nil { diff --git a/db/blip_sync_messages.go b/db/blip_sync_messages.go index ca09ad30b9..68de19d6c6 100644 --- a/db/blip_sync_messages.go +++ b/db/blip_sync_messages.go @@ -147,26 +147,27 @@ type SubChangesParams struct { _since SequenceID // Since value on the incoming request _docIDs []string // Document ID filter specified on the incoming request _sendReplacementRevs bool // Whether to send a replacement rev in the event that we do not find the requested one. + _defaultRequestPlus bool // Default value for requestPlus, set on a database level } type SubChangesBody struct { DocIDs []string `json:"docIDs"` } -// Create a new subChanges helper -func NewSubChangesParams(logCtx context.Context, rq *blip.Message, zeroSeq SequenceID, latestSeq LatestSequenceFunc, sequenceIDParser SequenceIDParser) (*SubChangesParams, error) { - +// NewSubChangesParams creates a SubChangesParam from a request. latestSeq represnts a function to determine the latest sequence for a database. defaultRequestPlus is the default value of request plus for a database. +func NewSubChangesParams(logCtx context.Context, rq *blip.Message, latestSeq LatestSequenceFunc, defaultRequestPlus bool) (*SubChangesParams, error) { params := &SubChangesParams{ - rq: rq, + rq: rq, + _defaultRequestPlus: defaultRequestPlus, } // Determine incoming since and docIDs once, since there is some overhead associated with their calculation - sinceSequenceId := zeroSeq + sinceSequenceId := CreateZeroSinceValue() var err error if rq.Properties[SubChangesFuture] == trueProperty { sinceSequenceId, err = latestSeq() } else if sinceStr, found := rq.Properties[SubChangesSince]; found { - if sinceSequenceId, err = sequenceIDParser(sinceStr); err != nil { + if sinceSequenceId, err = ParseJSONSequenceID(sinceStr); err != nil { base.InfofCtx(logCtx, base.KeySync, "%s: Invalid sequence ID in 'since': %s", rq, sinceStr) } } @@ -240,10 +241,10 @@ func (s *SubChangesParams) activeOnly() bool { return (s.rq.Properties[SubChangesActiveOnly] == trueProperty) } -func (s *SubChangesParams) requestPlus(defaultValue bool) (value bool) { +func (s *SubChangesParams) requestPlus() (value bool) { propertyValue, isDefined := s.rq.Properties[SubChangesRequestPlus] if !isDefined { - return defaultValue + return s._defaultRequestPlus } return propertyValue == trueProperty } @@ -300,6 +301,14 @@ func (s *SubChangesParams) String() string { buffer.WriteString(fmt.Sprintf("BatchSize:%v ", s.batchSize())) } + requestPlus := s.requestPlus() + if requestPlus { + buffer.WriteString(fmt.Sprintf("RequestPlus:%v ", requestPlus)) + } + future, ok := s.rq.Properties[SubChangesFuture] + if ok { + buffer.WriteString(fmt.Sprintf("Future:%v ", future)) + } if len(s.docIDs()) > 0 { buffer.WriteString(fmt.Sprintf("DocIDs:%v ", s.docIDs())) } @@ -468,6 +477,10 @@ func (rm *RevMessage) String() string { buffer.WriteString(fmt.Sprintf("Sequence:%v ", sequence)) } + noConflicts, ok := rm.Properties[RevMessageNoConflicts] + if ok { + buffer.WriteString(fmt.Sprintf("NoConflicts:%v ", noConflicts)) + } return buffer.String() } diff --git a/db/changes.go b/db/changes.go index b0ae4595f8..55683af6d2 100644 --- a/db/changes.go +++ b/db/changes.go @@ -1338,7 +1338,7 @@ func createChangesEntry(ctx context.Context, docid string, db *DatabaseCollectio func (options ChangesOptions) String() string { return fmt.Sprintf( - `{Since: %s, Limit: %d, Conflicts: %t, IncludeDocs: %t, Wait: %t, Continuous: %t, HeartbeatMs: %d, TimeoutMs: %d, ActiveOnly: %t, RequestPlusSeq: %d}`, + `{Since: %s, Limit: %d, Conflicts: %t, IncludeDocs: %t, Wait: %t, Continuous: %t, HeartbeatMs: %d, TimeoutMs: %d, ActiveOnly: %t, Revocations: %t, RequestPlusSeq: %d}`, options.Since, options.Limit, options.Conflicts, @@ -1348,6 +1348,7 @@ func (options ChangesOptions) String() string { options.HeartbeatMs, options.TimeoutMs, options.ActiveOnly, + options.Revocations, options.RequestPlusSeq, ) } diff --git a/db/changes_test.go b/db/changes_test.go index c648de6f6c..3df9040f7c 100644 --- a/db/changes_test.go +++ b/db/changes_test.go @@ -13,6 +13,8 @@ import ( "context" "fmt" "log" + "reflect" + "strings" "testing" "github.com/couchbase/sync_gateway/base" @@ -517,3 +519,27 @@ func BenchmarkChangesFeedDocUnmarshalling(b *testing.B) { } } + +func TestChangesOptionsStringer(t *testing.T) { + opts := ChangesOptions{} + var stringerFields []string + for _, key := range strings.Split(opts.String()[1:len(opts.String())-1], ",") { + fieldName, _, found := strings.Cut(strings.Trim(key, `" ,`), ":") + require.True(t, found, "Expected , in %s", key) + stringerFields = append(stringerFields, fieldName) + } + ignoredFields := map[string]struct{}{ + "ChangesCtx": {}, + "clientType": {}, + } + var expectedFields []string + for _, field := range reflect.VisibleFields(reflect.TypeOf(ChangesOptions{})) { + // some field names are not in stringer + if _, ok := ignoredFields[field.Name]; ok { + continue + } + expectedFields = append(expectedFields, field.Name) + } + require.ElementsMatch(t, expectedFields, stringerFields) + +} diff --git a/rest/blip_sync_messages_test.go b/rest/blip_sync_messages_test.go index 5f8eb71991..dd1a2fe792 100644 --- a/rest/blip_sync_messages_test.go +++ b/rest/blip_sync_messages_test.go @@ -80,7 +80,7 @@ func TestSubChangesSince(t *testing.T) { rq := blip.NewRequest() rq.Properties["since"] = `"1"` - subChangesParams, err := db.NewSubChangesParams(base.TestCtx(t), rq, db.SequenceID{}, nil, db.ParseJSONSequenceID) + subChangesParams, err := db.NewSubChangesParams(base.TestCtx(t), rq, nil, rt.GetDatabase().Options.ChangesRequestPlus) require.NoError(t, err) seqID := subChangesParams.Since() @@ -102,7 +102,7 @@ func TestSubChangesFuture(t *testing.T) { rq.Properties["future"] = "true" rq.Properties["since"] = `"1"` - subChangesParams, err := db.NewSubChangesParams(base.TestCtx(t), rq, db.SequenceID{}, latestSeq, db.ParseJSONSequenceID) + subChangesParams, err := db.NewSubChangesParams(base.TestCtx(t), rq, latestSeq, rt.GetDatabase().Options.ChangesRequestPlus) require.NoError(t, err) seqID := subChangesParams.Since()