Merge ../../ibm-cloud/go-etcd-rules

rules/callback_listener.go

@@ -19,7 +19,7 @@ type httpCallbackListener struct {
 func (htcbl httpCallbackListener) callbackDone(ruleID string, attributes extendedAttributes) {
 	attributeMap := make(map[string]string)
 	for _, k := range attributes.names() {
-		attributeMap[k] = *attributes.GetAttribute(k)
+		attributeMap[k], _ = attributes.GetAttribute(k)
 	}
 	postObj := callbackEvent{
 		RuleID:     ruleID,

rules/dynamic_rule.go

@@ -202,10 +202,10 @@ func (na *nestingAttributes) names() []string {
 	return names
 }
 
-func (na *nestingAttributes) GetAttribute(key string) *string {
+func (na *nestingAttributes) GetAttribute(key string) (string, bool) {
 	for _, attribute := range na.attrs {
 		if attribute.key == key {
-			return attribute.value
+			return *attribute.value, true
 		}
 	}
 	return na.nested.GetAttribute(key)

rules/dynamic_rule_test.go

@@ -53,11 +53,9 @@ func TestEqualsLiteralRule(t *testing.T) {
 			staticRuleOks[i] = staticRuleOks[i] || ok
 			if ok {
 				for key, value := range expansionAttributes[i] {
-					attrValue := attr.GetAttribute(key)
-					assert.NotNil(t, attrValue)
-					if attrValue != nil {
-						assert.Equal(t, value, *attrValue)
-					}
+					attrValue, ok := attr.GetAttribute(key)
+					assert.True(t, ok)
+					assert.Equal(t, value, attrValue)
 				}
 			}
 		}
@@ -126,11 +124,9 @@ func TestAndRule(t *testing.T) {
 			staticRuleOks[i] = staticRuleOks[i] || ok
 			if ok {
 				for key, value := range expansionAttributes[i] {
-					attrValue := attr1.GetAttribute(key)
-					assert.NotNil(t, attrValue)
-					if attrValue != nil {
-						assert.Equal(t, value, *attrValue)
-					}
+					attrValue, ok := attr1.GetAttribute(key)
+					assert.True(t, ok)
+					assert.Equal(t, value, attrValue)
 				}
 				assert.Equal(t, pattern, attr1.Format("/:a/:b/:var/attr1"))
 			}

rules/matcher.go

@@ -19,7 +19,7 @@ type regexKeyMatcher struct {
 }
 
 type keyMatch interface {
-	GetAttribute(name string) *string
+	GetAttribute(name string) (string, bool)
 	Format(pattern string) string
 	names() []string
 }
@@ -94,13 +94,13 @@ func newKeyMatch(path string, kmr *regexKeyMatcher) *regexKeyMatch {
 	return km
 }
 
-func (m *regexKeyMatch) GetAttribute(name string) *string {
+func (m *regexKeyMatch) GetAttribute(name string) (string, bool) {
 	index, ok := m.fieldMap[name]
 	if !ok {
-		return nil
+		return "", false
 	}
 	result := m.matchStrings[index]
-	return &result
+	return result, true
 }
 
 func (m *regexKeyMatch) names() []string {
@@ -123,27 +123,32 @@ func FormatWithAttributes(pattern string, m Attributes) string {
 }
 
 func formatPath(pattern string, m Attributes) (string, bool) {
-	allFound := true
-	paths := strings.Split(pattern, "/")
-	result := strings.Builder{}
-	for _, path := range paths {
-		if len(path) == 0 {
-			continue
-		}
-		result.WriteString("/")
-		if strings.HasPrefix(path, ":") {
-			attr := m.GetAttribute(path[1:])
-			if attr == nil {
-				s := path
-				attr = &s
-				allFound = false
+	sb := new(strings.Builder)
+	// If the formatted string can fit into 2x the length of the pattern
+	// (and mapAttributes is the attribute implementation used)
+	// this will be the only allocation
+	sb.Grow(len(pattern) * 2)
+
+	all := true
+	var segment string
+	for found := true; found; {
+		segment, pattern, found = strings.Cut(pattern, "/")
+		switch {
+		case segment == "":
+		case strings.HasPrefix(segment, ":"):
+			sb.WriteByte('/')
+			if attr, ok := m.GetAttribute(segment[1:]); ok {
+				sb.WriteString(attr)
+			} else {
+				all = false
+				sb.WriteString(segment)
 			}
-			result.WriteString(*attr)
-		} else {
-			result.WriteString(path)
+		default:
+			sb.WriteByte('/')
+			sb.WriteString(segment)
 		}
 	}
-	return result.String(), allFound
+	return sb.String(), all
 }
 
 // Keep the bool return value, because it's tricky to check for null
@@ -179,12 +184,9 @@ type mapAttributes struct {
 	values map[string]string
 }
 
-func (ma *mapAttributes) GetAttribute(key string) *string {
+func (ma *mapAttributes) GetAttribute(key string) (string, bool) {
 	value, ok := ma.values[key]
-	if !ok {
-		return nil
-	}
-	return &value
+	return value, ok
 }
 
 func (ma *mapAttributes) Format(path string) string {

rules/matcher_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestBasic(t *testing.T) {
@@ -12,14 +13,14 @@ func TestBasic(t *testing.T) {
 	km, _ := newRegexKeyMatcher(test)
 	match, ok := km.match("/user/john/state")
 	assert.True(t, ok)
-	value := *match.GetAttribute("name")
+	value, _ := match.GetAttribute("name")
 	if value != "john" {
 		t.Logf("Incorrect attribute value: %s", value)
 		t.Fail()
 	}
-	missing := match.GetAttribute("missing")
-	if missing != nil {
-		t.Log("Attribute value should be nil")
+	_, ok = match.GetAttribute("missing")
+	if ok {
+		t.Log("Attribute value should not be found")
 		t.Fail()
 	}
 	format := match.Format("/current_user/:name")
@@ -73,51 +74,6 @@ func TestNoRegex(t *testing.T) {
 	}
 }
 
-func formatPathTest(pattern string, m Attributes) (string, bool) {
-	sb := new(strings.Builder)
-	// If the formatted string can fit into 2x the length of the pattern
-	// (and mapAttributes is the attribute implementation used)
-	// this will be the only allocation
-	sb.Grow(len(pattern) * 2)
-
-	all := true
-	var segment string
-	for found := true; found; {
-		segment, pattern, found = strings.Cut(pattern, "/")
-		switch {
-		case segment == "":
-		case strings.HasPrefix(segment, ":"):
-			sb.WriteByte('/')
-			// The Attributes interface is rather inefficient
-			// It requires an allocation/derefence to look up
-			// strings from a map because it replaces the built-in
-			// (string, bool) return with a *string return.
-			// To save allocations we'll avoid calling GetAttribute
-			// if we can directly access a map
-			if ma, ok := m.(*mapAttributes); ok {
-				if attr, ok := ma.values[segment[1:]]; ok {
-					sb.WriteString(attr)
-				} else {
-					all = false
-					sb.WriteString(segment)
-				}
-			} else {
-				attr := m.GetAttribute(segment[1:])
-				if attr == nil {
-					sb.WriteString(segment)
-					all = false
-				} else {
-					sb.WriteString(*attr)
-				}
-			}
-		default:
-			sb.WriteByte('/')
-			sb.WriteString(segment)
-		}
-	}
-	return sb.String(), all
-}
-
 func BenchmarkFormatPath(b *testing.B) {
 	cases := []struct {
 		name    string
@@ -157,25 +113,11 @@ func BenchmarkFormatPath(b *testing.B) {
 	}
 
 	for _, tc := range cases {
-		b.Run("old:"+tc.name, func(b *testing.B) {
+		b.Run(tc.name, func(b *testing.B) {
 			for n := 0; n < b.N; n++ {
 				formatPath(tc.pattern, tc.attr)
 			}
 		})
-		b.Run("new:"+tc.name, func(b *testing.B) {
-			for n := 0; n < b.N; n++ {
-				formatPathTest(tc.pattern, tc.attr)
-			}
-		})
-	}
-}
-
-func BenchmarkFormatPathProfile(b *testing.B) {
-	attr := NewAttributes(map[string]string{"a": "aaaaaaaaaa", "b": "aaaaaaaaaa", "c": "aaaaaaaaaa", "d": "aaaaaaaaaa", "e": "eeeeeeeeee"})
-	pattern := "first/:a/second/:b/third/:c/fourth/:d/fifth/:e/first/:a/second/:b/third/:c/fourth/:d/fifth/:e/sixth"
-
-	for n := 0; n < b.N; n++ {
-		formatPathTest(pattern, attr)
 	}
 }
 
@@ -251,10 +193,5 @@ func TestFormatPath(t *testing.T) {
 			require.Equal(t, tc.expectstr, actualstr)
 			require.Equal(t, tc.expectbool, actualbool)
 		})
-		t.Run("new:"+tc.name, func(t *testing.T) {
-			actualstr, actualbool := formatPathTest(tc.pattern, tc.attr)
-			require.Equal(t, tc.expectstr, actualstr)
-			require.Equal(t, tc.expectbool, actualbool)
-		})
 	}
 }

rules/static_rule_test.go

@@ -87,7 +87,8 @@ func getTestAttributes() extendedAttributes {
 
 func verifyTestAttributes(t *testing.T, rule staticRule) {
 	attr := rule.getAttributes()
-	assert.Equal(t, "testvalue", *attr.GetAttribute("testkey"))
+	testkey, _ := attr.GetAttribute("testkey")
+	assert.Equal(t, "testvalue", testkey)
 }
 
 func TestCompareLiteralEquals(t *testing.T) {

rules/task.go

@@ -12,7 +12,7 @@ import (
 // that is matched against "/static/value1" would contain an yield
 // an attribute with the key "dynamic" and the value "value1".
 type Attributes interface {
-	GetAttribute(string) *string
+	GetAttribute(string) (string, bool)
 	Format(string) string
 }
 

rules/worker.go

@@ -126,7 +126,7 @@ func (bw *baseWorker) doWork(loggerPtr **zap.Logger,
 		attributes := (*rulePtr).getAttributes()
 		attrMap := make(map[string]string)
 		for _, attrName := range attributes.names() {
-			attrMap[attrName] = *attributes.GetAttribute(attrName)
+			attrMap[attrName], _ = attributes.GetAttribute(attrName)
 		}
 		logger.Info("callback started", zap.Any("attributes", attrMap))
 		startTime := time.Now()

v3enginetest/main.go

@@ -128,7 +128,8 @@ func main() {
 		// value is set that will prevent further polling even after the polling key TTL has expired.
 		task.Logger.Info("Callback called")
 		// This is thread safe, because the map is only being read and not written to.
-		p := ps[*task.Attr.GetAttribute("id")]
+		id, _ := task.Attr.GetAttribute("id")
+		p := ps[id]
 		pPollCount := atomic.LoadInt32(&p.pollCount)
 		// Retrieve a value from etcd.
 		path := task.Attr.Format(dataPath)