Skip to content

Commit

Permalink
[3.2.1 Backport] CBG-4201 CBG-4167 add missing Stringer for SubChange…
Browse files Browse the repository at this point in the history
…sParams and ChangesOptions (#7095)

- simplify NewSubChangesParams when adding another option since the
  existing options were taking the same code
  • Loading branch information
torcolvin authored and bbrks committed Sep 26, 2024
1 parent f96ea46 commit 51436ae
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 14 deletions.
5 changes: 2 additions & 3 deletions db/blip_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down Expand Up @@ -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 {
Expand Down
29 changes: 21 additions & 8 deletions db/blip_sync_messages.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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()))
}
Expand Down Expand Up @@ -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()

}
Expand Down
3 changes: 2 additions & 1 deletion db/changes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -1348,6 +1348,7 @@ func (options ChangesOptions) String() string {
options.HeartbeatMs,
options.TimeoutMs,
options.ActiveOnly,
options.Revocations,
options.RequestPlusSeq,
)
}
Expand Down
26 changes: 26 additions & 0 deletions db/changes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import (
"context"
"fmt"
"log"
"reflect"
"strings"
"testing"

"github.com/couchbase/sync_gateway/base"
Expand Down Expand Up @@ -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)

}
4 changes: 2 additions & 2 deletions rest/blip_sync_messages_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand Down

0 comments on commit 51436ae

Please sign in to comment.