merge prerelease branch

Author: eli-darkly

.golangci.yml

@@ -46,6 +46,10 @@ linters-settings:
     local-prefixes: github.com/launchdarkly
 
 issues:
+  exclude-rules:
+    - path: ldvalue/optional_.*.go
+      linters:
+        - dupl  # This linter would produce false positives due to necessary boilerplate code in ldvalue/optional_*.go
   exclude-use-default: false
   max-same-issues: 1000
   max-per-linter: 1000

go.mod

@@ -6,13 +6,15 @@ require (
 	github.com/launchdarkly/go-jsonstream/v3 v3.0.0
 	github.com/launchdarkly/go-test-helpers/v3 v3.0.1
 	github.com/mailru/easyjson v0.7.6
-	github.com/stretchr/testify v1.6.1
+	github.com/stretchr/testify v1.7.0
 	golang.org/x/exp v0.0.0-20220823124025-807a23277127
 )
 
 require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/josharian/intern v1.0.0 // indirect
+	github.com/kr/text v0.2.0 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
+	gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
 	gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c // indirect
 )

go.sum

@@ -1,8 +1,15 @@
+github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY=
 github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y=
+github.com/kr/pretty v0.2.1 h1:Fmg33tUaq4/8ym9TJN1x7sLJnHVwhP33CNkpYV/7rwI=
+github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
+github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
+github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
+github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
+github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
 github.com/launchdarkly/go-jsonstream/v3 v3.0.0 h1:qJF/WI09EUJ7kSpmP5d1Rhc81NQdYUhP17McKfUq17E=
 github.com/launchdarkly/go-jsonstream/v3 v3.0.0/go.mod h1:/1Gyml6fnD309JOvunOSfyysWbZ/ZzcA120gF/cQtC4=
 github.com/launchdarkly/go-test-helpers/v3 v3.0.1 h1:Z4lUVrh7+hIvL47KVjEBE/owbqqjKUEYTp4aBX/5OZM=
@@ -12,11 +19,12 @@ github.com/mailru/easyjson v0.7.6/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJ
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
-github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0=
-github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY=
+github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 golang.org/x/exp v0.0.0-20220823124025-807a23277127 h1:S4NrSKDfihhl3+4jSTgwoIevKxX9p7Iv9x++OEIptDo=
 golang.org/x/exp v0.0.0-20220823124025-807a23277127/go.mod h1:cyybsKvd6eL0RnXn6p/Grxp8F5bW7iYuBgsNCOHpMYE=
-gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk=
+gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

ldcontext/builder_benchmark_test.go

@@ -20,14 +20,13 @@ func BenchmarkBuildFromLocalBuilderNoCustomAttrsNoAlloc(b *testing.B) {
 		var b Builder
 		b.Key("key")
 		b.Name("x")
-		b.Secondary("y")
 		benchmarkContext = b.Build()
 	}
 }
 
 func BenchmarkBuildWithNoCustomAttrs(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		benchmarkContext = NewBuilder("key").Name("x").Secondary("y").Build()
+		benchmarkContext = NewBuilder("key").Name("x").Build()
 	}
 }
 

ldcontext/builder_multi.go

@@ -80,11 +80,9 @@ func (m *MultiBuilder) Build() Context {
 			}
 			individualErrors[string(c.Kind())] = err
 		default:
-			for j := 0; j < i; j++ {
-				if c.Kind() == m.contexts[j].Kind() { // same kind was seen twice
-					duplicates = true
-					break
-				}
+			// duplicate check's correctness relies on m.contexts being sorted by kind.
+			if i > 0 && m.contexts[i-1].Kind() == c.Kind() {
+				duplicates = true
 			}
 		}
 	}

ldcontext/builder_simple.go

@@ -35,7 +35,6 @@ type Builder struct {
 	allowEmptyKey      bool
 	name               ldvalue.OptionalString
 	attributes         ldvalue.ValueMapBuilder
-	secondary          ldvalue.OptionalString
 	anonymous          bool
 	privateAttrs       []ldattr.Ref
 	privateCopyOnWrite bool
@@ -113,7 +112,6 @@ func (b *Builder) Build() Context {
 		key:       b.key,
 		name:      b.name,
 		anonymous: b.anonymous,
-		secondary: b.secondary,
 	}
 
 	ret.fullyQualifiedKey = makeFullyQualifiedKeySingleKind(actualKind, ret.key, true)
@@ -360,35 +358,6 @@ func (b *Builder) TrySetValue(attributeName string, value ldvalue.Value) bool {
 	return true
 }
 
-// Secondary sets a secondary key for the Context.
-//
-// This affects feature flag targeting
-// (https://docs.launchdarkly.com/home/flags/targeting-users#targeting-rules-based-on-user-attributes)
-// as follows: if you have chosen to bucket users by a specific attribute, the secondary key (if set)
-// is used to further distinguish between users who are otherwise identical according to that attribute.
-//
-// This is a metadata property, rather than an attribute that can be addressed in evaluations: that is,
-// a rule clause that references the attribute name "secondary" will not use this value, but instead will
-// use whatever value (if any) you have set for the name "secondary" with a method such as SetString.
-//
-// Setting this value to an empty string is not the same as leaving it unset. If you need to clear this
-// attribute to a "no value" state, use OptSecondary().
-func (b *Builder) Secondary(value string) *Builder {
-	return b.OptSecondary(ldvalue.NewOptionalString(value))
-}
-
-// OptSecondary sets a secondary key for the Context.
-//
-// Calling b.OptSecondary(ldvalue.NewOptionalString("x")) is equivalent to b.Secondary("x"), but since it uses
-// the OptionalString type, it also allows clearing a previously set name with
-// b.OptSecondary(ldvalue.OptionalString{}).
-func (b *Builder) OptSecondary(value ldvalue.OptionalString) *Builder {
-	if b != nil {
-		b.secondary = value
-	}
-	return b
-}
-
 // Anonymous sets whether the Context is only intended for flag evaluations and should not be indexed by
 // LaunchDarkly.
 //
@@ -516,7 +485,6 @@ func (b *Builder) copyFrom(fromContext Context) {
 	b.kind = fromContext.kind
 	b.key = fromContext.key
 	b.name = fromContext.name
-	b.secondary = fromContext.secondary
 	b.anonymous = fromContext.anonymous
 	b.attributes = ldvalue.ValueMapBuilder{}
 	b.attributes.SetAllFromValueMap(fromContext.attributes)

ldcontext/builder_simple_test.go

@@ -122,15 +122,6 @@ func TestBuilderBasicSetters(t *testing.T) {
 	t.Run("Secondary", func(t *testing.T) {
 		c0 := makeBasicBuilder().Build()
 		assert.Equal(t, ldvalue.OptionalString{}, c0.Secondary())
-
-		c1 := makeBasicBuilder().Secondary("value").Build()
-		assert.Equal(t, ldvalue.NewOptionalString("value"), c1.Secondary())
-
-		c2 := makeBasicBuilder().OptSecondary(ldvalue.OptionalString{}).Build()
-		assert.Equal(t, ldvalue.OptionalString{}, c2.Secondary())
-
-		c3 := makeBasicBuilder().OptSecondary(ldvalue.NewOptionalString("value")).Build()
-		assert.Equal(t, ldvalue.NewOptionalString("value"), c3.Secondary())
 	})
 
 	t.Run("Anonymous", func(t *testing.T) {
@@ -410,7 +401,7 @@ func TestBuilderPrivate(t *testing.T) {
 func TestNewBuilderFromContext(t *testing.T) {
 	value1, value2 := ldvalue.String("value1"), ldvalue.String("value2")
 
-	b1 := NewBuilder("key1").Kind("kind1").Name("name1").Secondary("sec1").Anonymous(true).SetValue("attr", value1)
+	b1 := NewBuilder("key1").Kind("kind1").Name("name1").Anonymous(true).SetValue("attr", value1)
 	b1.Private("private1")
 	c1 := b1.Build()
 	jsonhelpers.AssertEqual(t, value1, c1.attributes.Get("attr"))
@@ -420,7 +411,6 @@ func TestNewBuilderFromContext(t *testing.T) {
 	c2 := b2.Build()
 	assert.Equal(t, Kind("kind1"), c2.Kind())
 	assert.Equal(t, "key1", c2.Key())
-	assert.Equal(t, ldvalue.NewOptionalString("sec1"), c2.Secondary())
 	assert.True(t, c2.Anonymous())
 	jsonhelpers.AssertEqual(t, value1, c2.attributes.Get("attr"))
 	assert.Equal(t, c1.privateAttrs, c2.privateAttrs)
@@ -450,7 +440,6 @@ func TestBuilderSafety(t *testing.T) {
 	var nilPtr *Builder
 	assert.Nil(t, nilPtr.Key("a"))
 	assert.Nil(t, nilPtr.Name("a"))
-	assert.Nil(t, nilPtr.Secondary("a"))
 	assert.Nil(t, nilPtr.Anonymous(true))
 	assert.Nil(t, nilPtr.SetValue("a", ldvalue.Bool(true)))
 	assert.Nil(t, nilPtr.Private("a"))

ldcontext/constants.go

@@ -1,11 +1,11 @@
 package ldcontext
 
 const (
-	jsonPropMeta            = "_meta"
-	jsonPropPrivate         = "privateAttributes"
-	jsonPropRedacted        = "redactedAttributes"
-	jsonPropSecondary       = "secondary"
-	jsonPropOldUserCustom   = "custom"
-	jsonPropOldUserPrivate  = "privateAttributeNames"
-	jsonPropOldUserRedacted = "privateAttrs"
+	jsonPropMeta             = "_meta"
+	jsonPropPrivate          = "privateAttributes"
+	jsonPropRedacted         = "redactedAttributes"
+	jsonPropOldUserCustom    = "custom"
+	jsonPropOldUserPrivate   = "privateAttributeNames"
+	jsonPropOldUserRedacted  = "privateAttrs"
+	jsonPropOldUserSecondary = "secondary"
 )

ldcontext/context.go

@@ -32,6 +32,10 @@ type Context struct {
 	secondary         ldvalue.OptionalString
 	anonymous         bool
 	privateAttrs      []ldattr.Ref
+
+	// Note that the secondary field cannot be set by any builder method. We support this
+	// meta-attribute internally in order to be able to evaluate flags for old-style users,
+	// and the only way it can be set is from the user JSON unmarshaling logic.
 }
 
 // IsDefined returns true if this is a Context that was created with a constructor or builder
@@ -224,14 +228,14 @@ func (c Context) Anonymous() bool {
 	return c.anonymous
 }
 
-// Secondary returns the secondary key attribute for the Context, if any.
+// Secondary returns the deprecated secondary key meta-attribute for the Context, if any.
 //
-// For a single-kind context, this value can be set by Builder.Secondary(), and is an empty
-// ldvalue.OptionalString{} value if not specified.
+// This corresponds to the "secondary" attribute in the older LaunchDarkly user schema. Since LaunchDarkly
+// still supports evaluating feature flags for old-style users, this attribute is still available to the
+// evaluation logic if it was present in user JSON, but it cannot be set via the Context or Builder APIs.
 //
-// For a multi-kind context, there is no single value, so Secondary() always returns an empty value; use
-// IndividualContextByIndex(), IndividualContextByName(), or GetAllIndividualContexts() to inspect
-// a Context for a particular kind and then call Secondary() on it.
+// Deprecated: Application code should not rely on the availability of this value. It is likely to be
+// removed from the product in the future.
 func (c Context) Secondary() ldvalue.OptionalString {
 	return c.secondary
 }

ldcontext/context_easyjson.go

@@ -1,5 +1,4 @@
 //go:build launchdarkly_easyjson
-// +build launchdarkly_easyjson
 
 package ldcontext
 
@@ -171,8 +170,6 @@ func unmarshalSingleKindEasyJSON(c *Context, in *jlexer.Lexer, knownKind Kind, u
 				key := in.UnsafeBytes() // see comment above
 				in.WantColon()
 				switch string(key) {
-				case jsonPropSecondary:
-					c.secondary = readOptStringEasyJSON(in)
 				case jsonPropPrivate:
 					if usingEventFormat {
 						in.SkipRecursive()
@@ -266,7 +263,7 @@ func unmarshalOldUserSchemaEasyJSON(c *Context, in *jlexer.Lexer, usingEventForm
 			hasKey = true
 		case ldattr.NameAttr:
 			c.name = readOptStringEasyJSON(in)
-		case jsonPropSecondary:
+		case jsonPropOldUserSecondary:
 			c.secondary = readOptStringEasyJSON(in)
 		case ldattr.AnonymousAttr:
 			if in.IsNull() {