Merge pull request #78 from speakeasy-api/subomi/fix/fix-syncing-array

fix: correct array sync to preserve identity matching

Author: subomi

marshaller/syncer.go

@@ -306,30 +306,6 @@ func syncArraySlice(ctx context.Context, source any, target any, valueNode *yaml
 		targetVal.Set(reflect.MakeSlice(targetVal.Type(), 0, 0))
 	}
 
-	if targetVal.Len() > sourceVal.Len() {
-		// Shorten the slice
-		tempVal := reflect.MakeSlice(targetVal.Type(), sourceVal.Len(), sourceVal.Len())
-		for i := 0; i < sourceVal.Len(); i++ {
-			tempVal.Index(i).Set(targetVal.Index(i))
-		}
-
-		targetVal.Set(tempVal)
-	}
-
-	if targetVal.Len() < sourceVal.Len() {
-		// Equalize the slice
-		tempVal := reflect.MakeSlice(targetVal.Type(), sourceVal.Len(), sourceVal.Len())
-
-		for i := 0; i < targetVal.Len(); i++ {
-			tempVal.Index(i).Set(targetVal.Index(i))
-		}
-		for i := targetVal.Len(); i < sourceVal.Len(); i++ {
-			tempVal.Index(i).Set(reflect.Zero(targetVal.Type().Elem()))
-		}
-
-		targetVal.Set(tempVal)
-	}
-
 	// When arrays are reordered at the high level (e.g., moving workflows around),
 	// we need to match source elements with their corresponding target core models
 	// by identity (RootNode) rather than by array position to preserve elements.
@@ -363,6 +339,24 @@ func syncArraySlice(ctx context.Context, source any, target any, valueNode *yaml
 		elements[i] = currentElementNode
 	}
 
+	// Update targetVal to contain only the synced elements in the correct order
+	// This ensures the target slice reflects the reordering and removals
+	tempVal := reflect.MakeSlice(targetVal.Type(), sourceVal.Len(), sourceVal.Len())
+	for i := 0; i < sourceVal.Len(); i++ {
+		// reorderedTargets[i] contains pointers from .Addr().Interface()
+		// Use dereferenceToLastPtr to handle chains of pointers (e.g., **T -> *T)
+		targetPtr := dereferenceToLastPtr(reflect.ValueOf(reorderedTargets[i]))
+
+		if targetVal.Type().Elem().Kind() == reflect.Ptr {
+			// Target slice expects pointers (*T), set the pointer directly
+			tempVal.Index(i).Set(targetPtr)
+		} else {
+			// Target slice expects values (T), dereference the pointer
+			tempVal.Index(i).Set(targetPtr.Elem())
+		}
+	}
+	targetVal.Set(tempVal)
+
 	return yml.CreateOrUpdateSliceNode(ctx, elements, valueNode), nil
 }
 

marshaller/syncing_test.go

@@ -1143,3 +1143,94 @@ func TestSync_EmbeddedMapComparison_PointerVsValue_Success(t *testing.T) {
 		require.Equal(t, "shared_key: shared_value\n", string(ptrYAML))
 	})
 }
+
+func TestSync_ArraySubset_Success(t *testing.T) {
+	t.Parallel()
+
+	// Create high-level model with 3 items
+	itemOne := &tests.TestItemHighModel{
+		Name:        "one",
+		Description: "First item",
+	}
+	itemFour := &tests.TestItemHighModel{
+		Name:        "four",
+		Description: "Fourth item",
+	}
+	itemSix := &tests.TestItemHighModel{
+		Name:        "six",
+		Description: "Sixth item",
+	}
+
+	highModel := tests.TestArrayOfObjectsHighModel{
+		Items: []*tests.TestItemHighModel{itemOne, itemFour, itemSix},
+	}
+
+	// Populate core model with 6 items from YAML
+	initialYAML := `items:
+    - name: three
+      description: Third item
+    - name: five
+      description: Fifth item
+    - name: one
+      description: First item
+    - name: four
+      description: Fourth item
+    - name: second
+      description: Second item
+    - name: six
+      description: Sixth item
+`
+	var rootNode yaml.Node
+	err := yaml.Unmarshal([]byte(initialYAML), &rootNode)
+	require.NoError(t, err)
+
+	coreModel := highModel.GetCore()
+	coreModel.SetRootNode(rootNode.Content[0])
+
+	_, err = marshaller.UnmarshalModel(t.Context(), rootNode.Content[0], coreModel)
+	require.NoError(t, err)
+
+	// Link high-level items to their corresponding core items
+	for _, item := range coreModel.Items.Value {
+		switch item.Name.Value {
+		case "one":
+			itemOne.SetCore(item)
+		case "four":
+			itemFour.SetCore(item)
+		case "six":
+			itemSix.SetCore(item)
+		}
+	}
+
+	// Sync high model subset to core model
+	resultNode, err := marshaller.SyncValue(t.Context(), &highModel, highModel.GetCore(), highModel.GetRootNode(), false)
+	require.NoError(t, err)
+	require.NotNil(t, resultNode)
+
+	// Verify synced array matches high model subset
+	items := coreModel.Items.Value
+	require.Len(t, items, 3)
+
+	require.Equal(t, "one", items[0].Name.Value)
+	require.Equal(t, "First item", items[0].Description.Value)
+
+	require.Equal(t, "four", items[1].Name.Value)
+	require.Equal(t, "Fourth item", items[1].Description.Value)
+
+	require.Equal(t, "six", items[2].Name.Value)
+	require.Equal(t, "Sixth item", items[2].Description.Value)
+
+	// Verify YAML output
+	expectedYAML := `items:
+    - name: one
+      description: First item
+    - name: four
+      description: Fourth item
+    - name: six
+      description: Sixth item
+`
+
+	actualYAML, err := yaml.Marshal(coreModel.GetRootNode())
+	require.NoError(t, err)
+	require.Equal(t, expectedYAML, string(actualYAML))
+}

marshaller/tests/core/models.go

@@ -183,3 +183,18 @@ type TestTypeConversionCoreModel struct {
 	HTTPMethodField marshaller.Node[*string] `key:"httpMethodField"`
 	Extensions      core.Extensions          `key:"extensions"`
 }
+
+// TestItemModel represents an item with a name and description
+type TestItemModel struct {
+	marshaller.CoreModel `model:"testItemModel"`
+
+	Name        marshaller.Node[string] `key:"name"`
+	Description marshaller.Node[string] `key:"description"`
+}
+
+// TestArrayOfObjectsModel contains an array of items
+type TestArrayOfObjectsModel struct {
+	marshaller.CoreModel `model:"testArrayOfObjectsModel"`
+
+	Items marshaller.Node[[]*TestItemModel] `key:"items"`
+}

marshaller/tests/models.go

@@ -128,3 +128,16 @@ type TestEmbeddedMapWithFieldsPointerHighModel struct {
 	NameField  string
 	Extensions *extensions.Extensions
 }
+
+// TestItemHighModel represents an item with a name and description
+type TestItemHighModel struct {
+	marshaller.Model[core.TestItemModel]
+	Name        string
+	Description string
+}
+
+// TestArrayOfObjectsHighModel contains an array of items
+type TestArrayOfObjectsHighModel struct {
+	marshaller.Model[core.TestArrayOfObjectsModel]
+	Items []*TestItemHighModel
+}