add Copy() method to configstore structs (#6025)

common/dynamicconfig/configstore/config_store_client.go

@@ -317,13 +317,13 @@ func (csc *configStoreClient) RestoreValue(name dc.Key, filters map[dc.Filter]in
 	if filters == nil {
 		for _, dcValue := range val.Values {
 			if dcValue.Filters != nil || len(dcValue.Filters) != 0 {
-				newValues = append(newValues, copyDynamicConfigValue(dcValue))
+				newValues = append(newValues, dcValue.Copy())
 			}
 		}
 	} else {
 		for _, dcValue := range val.Values {
 			if !matchFilters(dcValue, filters) || dcValue.Filters == nil || len(dcValue.Filters) == 0 {
-				newValues = append(newValues, copyDynamicConfigValue(dcValue))
+				newValues = append(newValues, dcValue.Copy())
 			}
 		}
 	}
@@ -355,7 +355,7 @@ func (csc *configStoreClient) ListValue(name dc.Key) ([]*types.DynamicConfigEntr
 		// if key is not known/specified, return all entries
 		resList = make([]*types.DynamicConfigEntry, 0, len(currentCached.dcEntries))
 		for _, entry := range currentCached.dcEntries {
-			resList = append(resList, copyDynamicConfigEntry(entry))
+			resList = append(resList, entry.Copy())
 		}
 	} else {
 		// if key is known, return just that specific entry
@@ -417,7 +417,7 @@ func (csc *configStoreClient) updateValue(name dc.Key, dcValues []*types.Dynamic
 			if entryExists && entry == existingEntry {
 				continue
 			} else {
-				newEntries = append(newEntries, copyDynamicConfigEntry(entry))
+				newEntries = append(newEntries, entry.Copy())
 			}
 		}
 	} else {
@@ -440,7 +440,7 @@ func (csc *configStoreClient) updateValue(name dc.Key, dcValues []*types.Dynamic
 						Values: dcValues,
 					})
 			} else {
-				newEntries = append(newEntries, copyDynamicConfigEntry(entry))
+				newEntries = append(newEntries, entry.Copy())
 			}
 		}
 	}
@@ -487,68 +487,6 @@ func (csc *configStoreClient) updateValue(name dc.Key, dcValues []*types.Dynamic
 	}
 }
 
-func copyDynamicConfigEntry(entry *types.DynamicConfigEntry) *types.DynamicConfigEntry {
-	if entry == nil {
-		return nil
-	}
-
-	newValues := make([]*types.DynamicConfigValue, 0, len(entry.Values))
-	for _, value := range entry.Values {
-		newValues = append(newValues, copyDynamicConfigValue(value))
-	}
-
-	return &types.DynamicConfigEntry{
-		Name:   entry.Name,
-		Values: newValues,
-	}
-}
-
-func copyDynamicConfigValue(value *types.DynamicConfigValue) *types.DynamicConfigValue {
-	if value == nil {
-		return nil
-	}
-
-	var newFilters []*types.DynamicConfigFilter
-	if value.Filters == nil {
-		newFilters = nil
-	} else {
-		newFilters = make([]*types.DynamicConfigFilter, 0, len(value.Filters))
-		for _, filter := range value.Filters {
-			newFilters = append(newFilters, copyDynamicConfigFilter(filter))
-		}
-	}
-
-	return &types.DynamicConfigValue{
-		Value:   copyDataBlob(value.Value),
-		Filters: newFilters,
-	}
-}
-
-func copyDynamicConfigFilter(filter *types.DynamicConfigFilter) *types.DynamicConfigFilter {
-	if filter == nil {
-		return nil
-	}
-
-	return &types.DynamicConfigFilter{
-		Name:  filter.Name,
-		Value: copyDataBlob(filter.Value),
-	}
-}
-
-func copyDataBlob(blob *types.DataBlob) *types.DataBlob {
-	if blob == nil {
-		return nil
-	}
-
-	newData := make([]byte, len(blob.Data))
-	copy(newData, blob.Data)
-
-	return &types.DataBlob{
-		EncodingType: blob.EncodingType,
-		Data:         newData,
-	}
-}
-
 func (csc *configStoreClient) update() error {
 	ctx, cancel := context.WithTimeout(context.Background(), csc.config.FetchTimeout)
 	defer cancel()

common/types/configStore.go

@@ -39,3 +39,51 @@ type DynamicConfigFilter struct {
 	Name  string    `json:"name,omitempty"`
 	Value *DataBlob `json:"value,omitempty"`
 }
+
+func (dcf *DynamicConfigFilter) Copy() *DynamicConfigFilter {
+	if dcf == nil {
+		return nil
+	}
+	return &DynamicConfigFilter{
+		Name:  dcf.Name,
+		Value: dcf.Value.DeepCopy(),
+	}
+}
+
+func (dcv *DynamicConfigValue) Copy() *DynamicConfigValue {
+	if dcv == nil {
+		return nil
+	}
+
+	var newFilters []*DynamicConfigFilter
+	if dcv.Filters != nil {
+		newFilters = make([]*DynamicConfigFilter, 0, len(dcv.Filters))
+		for _, filter := range dcv.Filters {
+			newFilters = append(newFilters, filter.Copy())
+		}
+	}
+
+	return &DynamicConfigValue{
+		Value:   dcv.Value.DeepCopy(),
+		Filters: newFilters,
+	}
+}
+
+func (dce *DynamicConfigEntry) Copy() *DynamicConfigEntry {
+	if dce == nil {
+		return nil
+	}
+
+	var newValues []*DynamicConfigValue
+	if dce.Values != nil {
+		newValues = make([]*DynamicConfigValue, 0, len(dce.Values))
+		for _, value := range dce.Values {
+			newValues = append(newValues, value.Copy())
+		}
+	}
+
+	return &DynamicConfigEntry{
+		Name:   dce.Name,
+		Values: newValues,
+	}
+}

common/types/configStore_test.go

@@ -0,0 +1,158 @@
+// The MIT License (MIT)
+
+// Copyright (c) 2017-2020 Uber Technologies Inc.
+
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in all
+// copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+// SOFTWARE.
+
+package types
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestDynamicConfigEntryCopy(t *testing.T) {
+	tests := []struct {
+		name  string
+		input *DynamicConfigEntry
+	}{
+		{
+			name:  "nil",
+			input: nil,
+		},
+		{
+			name:  "empty",
+			input: &DynamicConfigEntry{},
+		},
+		{
+			name: "nil values",
+			input: &DynamicConfigEntry{
+				Name:   "test-2",
+				Values: nil,
+			},
+		},
+		{
+			name: "non-nil values",
+			input: &DynamicConfigEntry{
+				Name: "test-2",
+				Values: []*DynamicConfigValue{
+					&DynamicConfigValue{
+						Value:   &DataBlob{Data: []byte("data")},
+						Filters: nil,
+					},
+				},
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			valCopy := tc.input.Copy()
+			assert.Equal(t, tc.input, valCopy)
+			// check if modifying the copy does not modify the original
+			if valCopy != nil && valCopy.Values != nil {
+				valCopy.Values[0].Value.Data = []byte("modified")
+				assert.NotEqual(t, tc.input, valCopy)
+			}
+			assert.Equal(t, tc.input, tc.input.Copy())
+		})
+	}
+}
+
+func TestDynamicConfigFilterCopy(t *testing.T) {
+	tests := []struct {
+		name  string
+		input *DynamicConfigFilter
+	}{
+		{
+			name:  "nil",
+			input: nil,
+		},
+		{
+			name:  "empty",
+			input: &DynamicConfigFilter{},
+		},
+		{
+			name: "nil value",
+			input: &DynamicConfigFilter{
+				Name:  "test-2",
+				Value: nil,
+			},
+		},
+		{
+			name: "non-nil values",
+			input: &DynamicConfigFilter{
+				Name:  "test-2",
+				Value: &DataBlob{Data: []byte("data")},
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			valCopy := tc.input.Copy()
+			assert.Equal(t, tc.input, valCopy)
+			// check if modifying the copy does not modify the original
+			if valCopy != nil {
+				valCopy.Name = "modified"
+				assert.NotEqual(t, tc.input, valCopy)
+			}
+		})
+	}
+}
+func TestDynamicConfigValueCopy(t *testing.T) {
+	tests := []struct {
+		name  string
+		input *DynamicConfigValue
+	}{
+		{
+			name:  "nil",
+			input: nil,
+		},
+		{
+			name:  "empty",
+			input: &DynamicConfigValue{},
+		},
+
+		{
+			name: "non-nil values",
+			input: &DynamicConfigValue{
+				Value: &DataBlob{Data: []byte("data")},
+				Filters: []*DynamicConfigFilter{
+					&DynamicConfigFilter{
+						Name:  "filter-1",
+						Value: &DataBlob{Data: []byte("data")},
+					}},
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			// check if modifying the copy does not modify the original
+			valCopy := tc.input.Copy()
+			assert.Equal(t, tc.input, valCopy)
+			if valCopy != nil && valCopy.Value != nil {
+				valCopy.Value.Data = []byte("modified")
+				assert.NotEqual(t, tc.input, valCopy)
+			}
+		})
+	}
+}