fix: `utils.UpdateObject` duplication of array members (#546)

* test: create test case for bug repro

* test: simplify test

* fix: bug

* fix: implement suggestion by @mcleanbc and update tests

* chore: remove unused func

* Revert "fix: implement suggestion by @mcleanbc and update tests"

This reverts commit 6031bffbd7.

* Revert "chore: remove unused func"

This reverts commit bdab7fd380.

* fix: implement suggestion from @ms-henglu and reinstate test for inconsistent ordering

utils/json.go

@@ -3,6 +3,7 @@ package utils
 import (
 	"encoding/json"
 	"fmt"
+	"reflect"
 	"regexp"
 	"strings"
 )
@@ -65,6 +66,9 @@ type UpdateJsonOption struct {
 
 // UpdateObject is used to get an updated object which has same schema as old, but with new value
 func UpdateObject(old interface{}, new interface{}, option UpdateJsonOption) interface{} {
+	if reflect.DeepEqual(old, new) {
+		return old
+	}
 	switch oldValue := old.(type) {
 	case map[string]interface{}:
 		if newMap, ok := new.(map[string]interface{}); ok {
@@ -101,8 +105,20 @@ func UpdateObject(old interface{}, new interface{}, option UpdateJsonOption) int
 			used := make([]bool, len(newArr))
 
 			for _, oldItem := range oldValue {
+				found := false
 				for index, newItem := range newArr {
-					if areSameArrayItems(oldItem, newItem) {
+					if reflect.DeepEqual(oldItem, newItem) && !used[index] {
+						res = append(res, UpdateObject(oldItem, newItem, option))
+						used[index] = true
+						found = true
+						break
+					}
+				}
+				if found {
+					continue
+				}
+				for index, newItem := range newArr {
+					if areSameArrayItems(oldItem, newItem) && !used[index] {
 						res = append(res, UpdateObject(oldItem, newItem, option))
 						used[index] = true
 						break

utils/json_test.go

@@ -574,3 +574,145 @@ func Test_OverrideWithPaths(t *testing.T) {
 		}
 	}
 }
+
+func Test_UpdateObjectDuplicateIdentifiers(t *testing.T) {
+	OldJson := `
+[
+	{
+		"apiVersion": "2021-05-01-preview",
+		"condition": "[startsWith(parameters('resourceType'),'Microsoft.DBforPostgreSQL/flexibleServers')]",
+		"dependsOn": [],
+		"location": "[parameters('location')]",
+		"name": "[concat(parameters('resourceName'), '/', 'Microsoft.Insights/', parameters('profileName'))]",
+		"properties": {
+			"logs": [
+				{
+					"category": "PostgreSQLLogs",
+					"enabled": "[parameters('logsEnabled')]"
+				}
+			],
+			"metrics": [
+				{
+					"category": "AllMetrics",
+					"enabled": "[parameters('metricsEnabled')]",
+					"retentionPolicy": {
+						"days": 0,
+						"enabled": false
+					},
+					"timeGrain": null
+				}
+			],
+			"workspaceId": "[parameters('logAnalytics')]"
+		},
+		"type": "Microsoft.DBforPostgreSQL/flexibleServers/providers/diagnosticSettings"
+	},
+	{
+		"apiVersion": "2021-05-01-preview",
+		"condition": "[startsWith(parameters('resourceType'),'Microsoft.DBforPostgreSQL/servers')]",
+		"dependsOn": [],
+		"location": "[parameters('location')]",
+		"name": "[concat(parameters('resourceName'), '/', 'Microsoft.Insights/', parameters('profileName'))]",
+		"properties": {
+			"logs": [
+				{
+					"category": "PostgreSQLLogs",
+					"enabled": "[parameters('logsEnabled')]"
+				},
+				{
+					"category": "QueryStoreRuntimeStatistics",
+					"enabled": "[parameters('logsEnabled')]"
+				},
+				{
+					"category": "QueryStoreWaitStatistics",
+					"enabled": "[parameters('logsEnabled')]"
+				}
+			],
+			"metrics": [
+				{
+					"category": "AllMetrics",
+					"enabled": "[parameters('metricsEnabled')]",
+					"retentionPolicy": {
+						"days": 0,
+						"enabled": false
+					},
+					"timeGrain": null
+				}
+			],
+			"workspaceId": "[parameters('logAnalytics')]"
+		},
+		"type": "Microsoft.DBforPostgreSQL/servers/providers/diagnosticSettings"
+	}
+]
+`
+	var old, new, expected any
+	_ = json.Unmarshal([]byte(OldJson), &old)
+	_ = json.Unmarshal([]byte(OldJson), &new)
+	_ = json.Unmarshal([]byte(OldJson), &expected)
+
+	got := utils.UpdateObject(old, new, utils.UpdateJsonOption{
+		IgnoreCasing:          false,
+		IgnoreMissingProperty: true,
+	})
+	if !reflect.DeepEqual(got, expected) {
+		expectedJson, _ := json.MarshalIndent(expected, "", "  ")
+		gotJson, _ := json.MarshalIndent(got, "", "  ")
+		t.Fatalf("Expected:\n%s\n\n but got\n%s", expectedJson, gotJson)
+	}
+}
+
+func Test_UpdateObjectDuplicateIdentifiersWithInconsistentOrdering(t *testing.T) {
+	OldJson := `
+{
+	"contentFilters": [
+		{
+			"name": "Hate",
+			"allowedContentLevel": "Medium",
+			"blocking": true,
+			"enabled": true,
+			"source": "Prompt"
+		},
+		{
+			"name": "Hate",
+			"allowedContentLevel": "Medium",
+			"blocking": true,
+			"enabled": true,
+			"source": "Completion"
+		}
+  ]
+}
+`
+	NewJson := `
+{
+	"contentFilters": [
+		{
+			"name": "Hate",
+			"allowedContentLevel": "Medium",
+			"blocking": true,
+			"enabled": true,
+			"source": "Completion"
+			},
+			{
+				"name": "Hate",
+				"allowedContentLevel": "Medium",
+				"blocking": true,
+				"enabled": true,
+				"source": "Prompt"
+			}
+  ]
+}
+`
+	var old, new, expected any
+	_ = json.Unmarshal([]byte(OldJson), &old)
+	_ = json.Unmarshal([]byte(NewJson), &new)
+	_ = json.Unmarshal([]byte(OldJson), &expected)
+
+	got := utils.UpdateObject(old, new, utils.UpdateJsonOption{
+		IgnoreCasing:          false,
+		IgnoreMissingProperty: true,
+	})
+	if !reflect.DeepEqual(got, expected) {
+		expectedJson, _ := json.MarshalIndent(expected, "", "  ")
+		gotJson, _ := json.MarshalIndent(got, "", "  ")
+		t.Fatalf("Expected:\n%s\n\n but got\n%s", expectedJson, gotJson)
+	}
+}