merge prerelease branch

Author: eli-darkly

ldattr/ref.go

@@ -2,11 +2,9 @@ package ldattr
 
 import (
 	"encoding/json"
-	"strconv"
 	"strings"
 
 	"github.com/launchdarkly/go-sdk-common/v3/lderrors"
-	"github.com/launchdarkly/go-sdk-common/v3/ldvalue"
 
 	"github.com/launchdarkly/go-jsonstream/v3/jreader"
 )
@@ -34,11 +32,10 @@ import (
 // An attribute name can contain any characters, but must not be empty.
 //
 // If the first character is a slash, the string is interpreted as a slash-delimited path where the
-// first path component is an attribute name, and each subsequent path component is either the name of
-// a property in a JSON object, or a decimal numeric string that is the index of an element in a JSON
-// array. Any instances of the characters "/" or "~" in a path component are escaped as "~1" or "~0"
-// respectively. This syntax deliberately resembles JSON Pointer, but no JSON Pointer behaviors other
-// than those mentioned here are supported.
+// first path component is an attribute name, and each subsequent path component is the name of a
+// property in a JSON object. Any instances of the characters "/" or "~" in a path component are
+// escaped as "~1" or "~0" respectively. This syntax deliberately resembles JSON Pointer, but no JSON
+// Pointer behaviors other than those mentioned here are supported.
 //
 // # Examples
 //
@@ -48,30 +45,25 @@ import (
 //	  "kind": "user",
 //	  "key": "value1",
 //	  "address": {
-//	    "street": "value2",
-//	    "city": "value3"
+//	    "street": {
+//	      "line1": "value2",
+//	      "line2": "value3"
+//	    },
+//	    "city": "value4"
 //	  },
-//	  "groups": [ "value4", "value5" ],
-//	  "good/bad": "value6"
+//	  "good/bad": "value5"
 //	}
 //
 // The attribute references "key" and "/key" would both point to "value1".
 //
-// The attribute reference "/address/street" would point to "value2".
+// The attribute reference "/address/street/line1" would point to "value2".
 //
-// The attribute reference "/groups/0" would point to "value4".
-//
-// The attribute references "good/bad" and "/good~1bad" would both point to "value6".
+// The attribute references "good/bad" and "/good~1bad" would both point to "value5".
 type Ref struct {
 	err                 error
 	rawPath             string
 	singlePathComponent string
-	components          []attrRefComponent
-}
-
-type attrRefComponent struct {
-	name     string
-	intValue ldvalue.OptionalInt
+	components          []string
 }
 
 // NewRef creates a Ref from a string. For the supported syntax and examples, see comments on the Ref type.
@@ -98,7 +90,7 @@ func NewRef(referenceString string) Ref {
 		return Ref{err: lderrors.ErrAttributeInvalidEscape{}, rawPath: referenceString}
 	}
 	parts := strings.Split(path, "/")
-	ret := Ref{rawPath: referenceString, components: make([]attrRefComponent, 0, len(parts))}
+	ret := Ref{rawPath: referenceString, components: make([]string, 0, len(parts))}
 	for _, p := range parts {
 		if p == "" {
 			ret.err = lderrors.ErrAttributeExtraSlash{}
@@ -108,13 +100,7 @@ func NewRef(referenceString string) Ref {
 		if !ok {
 			return Ref{err: lderrors.ErrAttributeInvalidEscape{}, rawPath: referenceString}
 		}
-		component := attrRefComponent{name: unescaped}
-		if p[0] >= '0' && p[0] <= '9' {
-			if n, err := strconv.Atoi(p); err == nil {
-				component.intValue = ldvalue.NewOptionalInt(n)
-			}
-		}
-		ret.components = append(ret.components, component)
+		ret.components = append(ret.components, unescaped)
 	}
 	return ret
 }
@@ -198,29 +184,23 @@ func (a Ref) Depth() int {
 // Component retrieves a single path component from the attribute reference.
 //
 // For a simple attribute reference such as "name" with no leading slash, if index is zero,
-// Component returns the attribute name and an empty ldvalue.OptionalInt{}.
+// Component returns the attribute name.
 //
 // For an attribute reference with a leading slash, if index is non-negative and less than
-// a.Depth(), Component returns the path component as a string for its first value. The
-// second value is an ldvalue.OptionalInt that is the integer value of that string as returned
-// by strconv.Atoi() if applicable, or an empty ldvalue.OptionalInt{} if the string does not
-// represent an integer; this is used to implement a "find a value by index within a JSON
-// array" behavior similar to JSON Pointer.
+// a.Depth(), Component returns the path component.
 //
-// If index is out of range, it returns "" and an empty ldvalue.OptionalInt{}.
+// If index is out of range, it returns "".
 //
-//	NewRef("a").Component(0)      // returns ("a", ldvalue.OptionalInt{})
-//	NewRef("/a/b").Component(1)   // returns ("b", ldvalue.OptionalInt{})
-//	NewRef("/a/3").Component(1)   // returns ("3", ldvalue.NewOptionalInt(3))
-func (a Ref) Component(index int) (string, ldvalue.OptionalInt) {
+//	NewRef("a").Component(0)      // returns "a"
+//	NewRef("/a/b").Component(1)   // returns "b"
+func (a Ref) Component(index int) string {
 	if index == 0 && len(a.components) == 0 {
-		return a.singlePathComponent, ldvalue.OptionalInt{}
+		return a.singlePathComponent
 	}
 	if index < 0 || index >= len(a.components) {
-		return "", ldvalue.OptionalInt{}
+		return ""
 	}
-	c := a.components[index]
-	return c.name, c.intValue
+	return a.components[index]
 }
 
 // String returns the attribute reference as a string, in the same format used by NewRef().

ldattr/ref_test.go

@@ -6,7 +6,6 @@ import (
 	"testing"
 
 	"github.com/launchdarkly/go-sdk-common/v3/lderrors"
-	"github.com/launchdarkly/go-sdk-common/v3/ldvalue"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -58,8 +57,7 @@ func TestRefWithNoLeadingSlash(t *testing.T) {
 			assert.NoError(t, a.Err())
 			assert.Equal(t, s, a.String())
 			assert.Equal(t, 1, a.Depth())
-			name, _ := a.Component(0)
-			assert.Equal(t, s, name)
+			assert.Equal(t, s, a.Component(0))
 		})
 	}
 }
@@ -82,8 +80,7 @@ func TestRefSimpleWithLeadingSlash(t *testing.T) {
 			assert.NoError(t, a.Err())
 			assert.Equal(t, params.input, a.String())
 			assert.Equal(t, 1, a.Depth())
-			name, _ := a.Component(0)
-			assert.Equal(t, params.path, name)
+			assert.Equal(t, params.path, a.Component(0))
 		})
 	}
 }
@@ -108,44 +105,37 @@ func TestNewLiteralRef(t *testing.T) {
 }
 
 func TestRefComponents(t *testing.T) {
-	undefined := -99
 	for _, params := range []struct {
-		input         string
-		depth         int
-		index         int
-		expectedName  string
-		expectedIndex int
+		input        string
+		depth        int
+		index        int
+		expectedName string
 	}{
-		{"", 0, 0, "", undefined},
-		{"key", 1, 0, "key", undefined},
-		{"/key", 1, 0, "key", undefined},
-		{"/a/b", 2, 0, "a", undefined},
-		{"/a/b", 2, 1, "b", undefined},
-		{"/a~1b/c", 2, 0, "a/b", undefined},
-		{"/a~0b/c", 2, 0, "a~b", undefined},
-		{"/a/10/20/30x", 4, 1, "10", 10},
-		{"/a/10/20/30x", 4, 2, "20", 20},
-		{"/a/10/20/30x", 4, 3, "30x", undefined},
+		{"", 0, 0, ""},
+		{"key", 1, 0, "key"},
+		{"/key", 1, 0, "key"},
+		{"/a/b", 2, 0, "a"},
+		{"/a/b", 2, 1, "b"},
+		{"/a~1b/c", 2, 0, "a/b"},
+		{"/a~0b/c", 2, 0, "a~b"},
+		{"/a/10/20/30x", 4, 1, "10"},
+		{"/a/10/20/30x", 4, 2, "20"},
+		{"/a/10/20/30x", 4, 3, "30x"},
 
 		// invalid arguments don't cause an error, they just return empty values
-		{"", 0, -1, "", undefined},
-		{"key", 1, -1, "", undefined},
-		{"key", 1, 1, "", undefined},
-		{"/key", 1, -1, "", undefined},
-		{"/key", 1, 1, "", undefined},
-		{"/a/b", 2, -1, "", undefined},
-		{"/a/b", 2, 2, "", undefined},
+		{"", 0, -1, ""},
+		{"key", 1, -1, ""},
+		{"key", 1, 1, ""},
+		{"/key", 1, -1, ""},
+		{"/key", 1, 1, ""},
+		{"/a/b", 2, -1, ""},
+		{"/a/b", 2, 2, ""},
 	} {
 		t.Run(fmt.Sprintf("input string %q, index %d", params.input, params.index), func(t *testing.T) {
 			a := NewRef(params.input)
 			assert.Equal(t, params.depth, a.Depth())
-			name, index := a.Component(params.index)
+			name := a.Component(params.index)
 			assert.Equal(t, params.expectedName, name)
-			if params.expectedIndex == undefined {
-				assert.Equal(t, ldvalue.OptionalInt{}, index)
-			} else {
-				assert.Equal(t, ldvalue.NewOptionalInt(params.expectedIndex), index)
-			}
 		})
 	}
 }

ldcontext/context.go

@@ -188,7 +188,7 @@ func (c Context) GetValueForRef(ref ldattr.Ref) ldvalue.Value {
 		return ldvalue.Null()
 	}
 
-	firstPathComponent, _ := ref.Component(0)
+	firstPathComponent := ref.Component(0)
 
 	if c.Multiple() {
 		if ref.Depth() == 1 && firstPathComponent == ldattr.KindAttr {
@@ -203,13 +203,9 @@ func (c Context) GetValueForRef(ref ldattr.Ref) ldvalue.Value {
 		return ldvalue.Null()
 	}
 	for i := 1; i < ref.Depth(); i++ {
-		name, index := ref.Component(i)
-		if index.IsDefined() && value.Type() == ldvalue.ArrayType {
-			value, ok = value.TryGetByIndex(index.IntValue())
-		} else {
-			value, ok = value.TryGetByKey(name)
-		}
-		if !ok {
+		name := ref.Component(i)
+		value, ok = value.TryGetByKey(name)
+		if !ok { // ok is false if either the name is not found or the value was not an object
 			return ldvalue.Null()
 		}
 	}

ldcontext/context_test.go

@@ -270,38 +270,22 @@ func TestGetValueForRefCustomAttributeSingleKind(t *testing.T) {
 		expectAttributeNotFoundForRef(t, c, "/my-attr/my-prop")
 	})
 
-	t.Run("element in array", func(t *testing.T) {
+	t.Run("property whose name is a numeric string", func(t *testing.T) {
 		expected := ldvalue.String("good")
-		array := ldvalue.ArrayOf(ldvalue.String("bad"), expected, ldvalue.String("worse"))
-		c := makeBasicBuilder().SetValue("my-attr", array).Build()
-		expectAttributeFoundForRef(t, expected, c, "/my-attr/1")
-	})
-
-	t.Run("element in nested array in object", func(t *testing.T) {
-		expected := ldvalue.String("good")
-		array := ldvalue.ArrayOf(ldvalue.String("bad"), expected, ldvalue.String("worse"))
-		object := ldvalue.ObjectBuild().Set("my-prop", array).Build()
+		object := ldvalue.ObjectBuild().Set("1", expected).Build()
 		c := makeBasicBuilder().SetValue("my-attr", object).Build()
-		expectAttributeFoundForRef(t, expected, c, "/my-attr/my-prop/1")
-	})
-
-	t.Run("index too low in array", func(t *testing.T) {
-		expected := ldvalue.String("good")
-		array := ldvalue.ArrayOf(ldvalue.String("bad"), expected, ldvalue.String("worse"))
-		c := makeBasicBuilder().SetValue("my-attr", array).Build()
-		expectAttributeNotFoundForRef(t, c, "/my-attr/-1")
+		expectAttributeFoundForRef(t, expected, c, "/my-attr/1")
 	})
 
-	t.Run("index too high in array", func(t *testing.T) {
-		expected := ldvalue.String("good")
-		array := ldvalue.ArrayOf(ldvalue.String("bad"), expected, ldvalue.String("worse"))
+	t.Run("property not applicable to array", func(t *testing.T) {
+		array := ldvalue.ArrayOf(ldvalue.String("bad"), ldvalue.String("worse"))
 		c := makeBasicBuilder().SetValue("my-attr", array).Build()
-		expectAttributeNotFoundForRef(t, c, "/my-attr/3")
+		expectAttributeNotFoundForRef(t, c, "/my-attr/1")
 	})
 
-	t.Run("index in value that is not an object", func(t *testing.T) {
+	t.Run("property not applicable to simple value", func(t *testing.T) {
 		c := makeBasicBuilder().SetValue("my-attr", ldvalue.String("xyz")).Build()
-		expectAttributeNotFoundForRef(t, c, "/my-attr/1")
+		expectAttributeNotFoundForRef(t, c, "/my-attr/a")
 	})
 }