fix(mappings): carry OriginalValue and the snippet formatting through a rewrite

mdl/backend/modelsdk/mapping_read.go

@@ -340,6 +340,7 @@ func exportMappingElementFromGen(el element.Element) *model.ExportMappingElement
 		e.ExposedName = o.ExposedName()
 		e.JsonPath = o.JsonPath()
 		e.XmlPath = o.XmlPath()
+		e.OriginalValue = o.OriginalValue()
 		e.MinOccurs = int(o.MinOccurs())
 		e.MaxOccurs = int(o.MaxOccurs())
 		e.MaxLength = int(o.MaxLength())

mdl/backend/modelsdk/mapping_write.go

@@ -436,7 +436,10 @@ func exportValueElementToGen(id string, elem *model.ExportMappingElement, parent
 	addBool(g, "IsKey", elem.IsKey)
 	addBool(g, "IsContent", false)
 	addBool(g, "IsXmlAttribute", false)
-	addStr(g, "OriginalValue", "")
+	// Carried, not hardcoded: whether a mapping stores the structure's sample is
+	// a per-document property, so a rewrite preserves what was there rather than
+	// deleting it (ako/mxcli#379). A newly authored mapping still gets "".
+	addStr(g, "OriginalValue", elem.OriginalValue)
 	addStr(g, "XmlPrimitiveType", xmlPrimitiveTypeName(elem.DataType))
 	return g
 }

mdl/executor/cmd_export_mappings.go

@@ -410,6 +410,9 @@ func finishExportMapping(ctx *ExecContext, s *ast.CreateExportMappingStmt,
 ) error {
 	if existing != nil {
 		em.ID = existing.ID
+		// A rewrite must not delete the samples the stored document carries
+		// (ako/mxcli#379).
+		carryExportOriginalValues(em, existing)
 		if err := ctx.Backend.UpdateExportMapping(em); err != nil {
 			return mdlerrors.NewBackend("update export mapping", err)
 		}

mdl/executor/cmd_import_mappings.go

@@ -512,6 +512,9 @@ func finishImportMapping(ctx *ExecContext, s *ast.CreateImportMappingStmt,
 	}
 	if existing != nil {
 		im.ID = existing.ID
+		// A rewrite must not delete the samples the stored document carries
+		// (ako/mxcli#379).
+		carryImportOriginalValues(im, existing)
 		if err := ctx.Backend.UpdateImportMapping(im); err != nil {
 			return mdlerrors.NewBackend("update import mapping", err)
 		}
@@ -609,14 +612,14 @@ func buildImportMappingElementModel(moduleName string, def *ast.ImportMappingEle
 		elem.MinOccurs = jsElem.MinOccurs
 		elem.MaxOccurs = jsElem.MaxOccurs
 		elem.Nillable = jsElem.Nillable
-		// OriginalValue is deliberately NOT cloned. It is the sample value parsed
-		// out of the JSON structure's snippet ("42", "\"Widget\""), and it belongs
-		// to the STRUCTURE — Studio Pro leaves it empty on every mapping element.
-		// Measured across the two Studio-Pro-authored mappings a blank app ships
-		// (FeedbackModule's IMM_PostResponse and EMM_PostFeedback, ~15 value
-		// elements between them): all "", while their structures carry 17 non-empty
-		// samples. Copying the sample in makes an mxcli-written mapping differ from
-		// a Studio-Pro-written one over the same structure. (issue #882)
+		// OriginalValue is deliberately NOT cloned from the structure — a NEW
+		// mapping gets an empty one (#882). That decision stands, but its
+		// original measurement was too narrow: it read two mappings a blank app
+		// ships, and at corpus scale 2,322 of 3,042 value elements DO carry the
+		// sample. The split is per document (145 mappings all, 107 none, 2
+		// mixed), so it is not derivable — which is why a REWRITE carries the
+		// stored value forward instead of choosing. See
+		// carryImportOriginalValues (ako/mxcli#379).
 		elem.FractionDigits = jsElem.FractionDigits
 		elem.TotalDigits = jsElem.TotalDigits
 		elem.MaxLength = jsElem.MaxLength

mdl/executor/cmd_jsonstructures.go

@@ -4,7 +4,9 @@
 package executor
 
 import (
+	"encoding/json"
 	"fmt"
+	"reflect"
 	"sort"
 	"strings"
 	"unicode"
@@ -295,6 +297,14 @@ func execCreateJsonStructure(ctx *ExecContext, s *ast.CreateJsonStructureStmt) e
 		JsonSnippet:   types.PrettyPrintJSON(s.JsonSnippet),
 		Elements:      elements,
 	}
+	// Keep the stored snippet's FORMATTING when the content is the same. mxcli
+	// pretty-prints on describe, so describe -> exec — how a document is copied
+	// — otherwise rewrote a snippet Studio Pro had stored on one line into a
+	// multi-line one. Same JSON, different bytes, and a diff against the
+	// original for nothing (ako/mxcli#379).
+	if existing != nil && sameJSONContent(existing.JsonSnippet, js.JsonSnippet) {
+		js.JsonSnippet = existing.JsonSnippet
+	}
 	// A rewrite that carried no doc comment keeps the stored one (#1018).
 	if existing != nil {
 		js.Documentation = carriedDocumentation(s.DocumentationSet, s.Documentation, existing.Documentation)
@@ -365,3 +375,23 @@ func findJsonStructure(ctx *ExecContext, moduleName, structName string) *types.J
 	}
 	return nil
 }
+
+// sameJSONContent reports whether two snippets carry the same JSON, ignoring
+// whitespace. Comparing the decoded values rather than the strings is the point:
+// the question is whether a rewrite would change anything that matters.
+//
+// Anything that does not parse is treated as different, so a malformed snippet
+// is replaced rather than silently kept.
+func sameJSONContent(a, b string) bool {
+	if a == b {
+		return true
+	}
+	var va, vb any
+	if err := json.Unmarshal([]byte(a), &va); err != nil {
+		return false
+	}
+	if err := json.Unmarshal([]byte(b), &vb); err != nil {
+		return false
+	}
+	return reflect.DeepEqual(va, vb)
+}

mdl/executor/mapping_original_value.go

@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: Apache-2.0
+
+// Carrying a mapping value element's OriginalValue through a rewrite
+// (ako/mxcli#379).
+//
+// OriginalValue is the sample parsed out of the JSON structure's snippet
+// ("42", "\"Widget\""). mxcli wrote it empty on every element, on the strength
+// of a measurement over two mappings a blank app ships (#882) — and a rewrite
+// therefore DELETED it from every mapping that had one.
+//
+// The wider measurement says neither "always empty" nor "always copy the
+// sample" is right. Across 3,042 value elements whose structure carries a
+// sample, 2,322 (76%) store it and 720 do not — and the split is PER DOCUMENT,
+// not per element:
+//
+//	145 mappings carry the sample on EVERY element
+//	107 carry it on NONE
+//	  2 are mixed
+//
+// So which one a mapping gets is a property of how and when it was authored,
+// which mxcli cannot compute. Choosing a global default is wrong for roughly
+// half the corpus either way.
+//
+// A REWRITE does not have to choose. It knows what was stored, so it carries it
+// — guard-don't-drop, ADR-0005. That leaves #882's actual decision intact: a
+// NEWLY created mapping still writes empty, which is what that issue was about.
+package executor
+
+import "github.com/mendixlabs/mxcli/model"
+
+// carryImportOriginalValues copies each stored element's OriginalValue onto the
+// rebuilt element at the same JsonPath.
+//
+// Matching is by JsonPath because that is what identifies an element against
+// the schema: names can be renamed and order can change, but a value element
+// bound to a different path is a different element.
+func carryImportOriginalValues(rebuilt, stored *model.ImportMapping) {
+	if rebuilt == nil || stored == nil {
+		return
+	}
+	byPath := map[string]string{}
+	collectImportOriginalValues(stored.Elements, byPath)
+	if len(byPath) == 0 {
+		return
+	}
+	applyImportOriginalValues(rebuilt.Elements, byPath)
+}
+
+func collectImportOriginalValues(elems []*model.ImportMappingElement, out map[string]string) {
+	for _, e := range elems {
+		if e == nil {
+			continue
+		}
+		if e.OriginalValue != "" {
+			out[e.JsonPath] = e.OriginalValue
+		}
+		collectImportOriginalValues(e.Children, out)
+	}
+}
+
+func applyImportOriginalValues(elems []*model.ImportMappingElement, byPath map[string]string) {
+	for _, e := range elems {
+		if e == nil {
+			continue
+		}
+		// Only fill an element the rebuild left empty: a statement that somehow
+		// set one should win over what was stored.
+		if e.OriginalValue == "" {
+			if v, ok := byPath[e.JsonPath]; ok {
+				e.OriginalValue = v
+			}
+		}
+		applyImportOriginalValues(e.Children, byPath)
+	}
+}
+
+// carryExportOriginalValues is the export twin. An export mapping's value
+// elements hardcoded "" in the codec writer rather than carrying the field at
+// all, so this needed the semantic type to reach the writer as well.
+func carryExportOriginalValues(rebuilt, stored *model.ExportMapping) {
+	if rebuilt == nil || stored == nil {
+		return
+	}
+	byPath := map[string]string{}
+	collectExportOriginalValues(stored.Elements, byPath)
+	if len(byPath) == 0 {
+		return
+	}
+	applyExportOriginalValues(rebuilt.Elements, byPath)
+}
+
+func collectExportOriginalValues(elems []*model.ExportMappingElement, out map[string]string) {
+	for _, e := range elems {
+		if e == nil {
+			continue
+		}
+		if e.OriginalValue != "" {
+			out[e.JsonPath] = e.OriginalValue
+		}
+		collectExportOriginalValues(e.Children, out)
+	}
+}
+
+func applyExportOriginalValues(elems []*model.ExportMappingElement, byPath map[string]string) {
+	for _, e := range elems {
+		if e == nil {
+			continue
+		}
+		if e.OriginalValue == "" {
+			if v, ok := byPath[e.JsonPath]; ok {
+				e.OriginalValue = v
+			}
+		}
+		applyExportOriginalValues(e.Children, byPath)
+	}
+}

mdl/executor/mapping_original_value_test.go

@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package executor
+
+import (
+	"testing"
+
+	"github.com/mendixlabs/mxcli/model"
+)
+
+// OriginalValue is the sample parsed out of a JSON structure's snippet. mxcli
+// wrote it empty on every element, so a rewrite DELETED it from every mapping
+// that had one — 2,322 of 3,042 value elements in the demo corpus carry one
+// (ako/mxcli#379).
+//
+// Neither global default is right. The split is PER DOCUMENT: 145 mappings
+// carry the sample on every element, 107 on none, 2 mixed. So a rewrite
+// preserves what was stored instead of choosing, and a NEWLY authored mapping
+// still writes empty — which is what #882 actually decided.
+
+func importElem(path, value string, kids ...*model.ImportMappingElement) *model.ImportMappingElement {
+	return &model.ImportMappingElement{JsonPath: path, OriginalValue: value, Children: kids}
+}
+
+func TestImportOriginalValuesAreCarriedThroughARewrite(t *testing.T) {
+	stored := &model.ImportMapping{Elements: []*model.ImportMappingElement{
+		importElem("(Object)", "",
+			importElem("(Object)|title", `"hello"`),
+			importElem("(Object)|nested", "",
+				importElem("(Object)|nested|qty", `"3"`)),
+		),
+	}}
+	rebuilt := &model.ImportMapping{Elements: []*model.ImportMappingElement{
+		importElem("(Object)", "",
+			importElem("(Object)|title", ""),
+			importElem("(Object)|nested", "",
+				importElem("(Object)|nested|qty", "")),
+		),
+	}}
+
+	carryImportOriginalValues(rebuilt, stored)
+
+	root := rebuilt.Elements[0]
+	if got := root.Children[0].OriginalValue; got != `"hello"` {
+		t.Errorf("title = %q, want \"hello\"", got)
+	}
+	// Nesting matters: a mapping's samples are not all at the top level.
+	if got := root.Children[1].Children[0].OriginalValue; got != `"3"` {
+		t.Errorf("nested qty = %q, want \"3\"", got)
+	}
+}
+
+// TestOriginalValuesMatchOnJsonPath pins the matching key. Names can be renamed
+// and order can change; a value element bound to a different path is a
+// different element, so the path is what identifies it against the schema.
+func TestOriginalValuesMatchOnJsonPath(t *testing.T) {
+	stored := &model.ImportMapping{Elements: []*model.ImportMappingElement{
+		importElem("(Object)", "", importElem("(Object)|title", `"hello"`)),
+	}}
+	rebuilt := &model.ImportMapping{Elements: []*model.ImportMappingElement{
+		importElem("(Object)", "", importElem("(Object)|somethingElse", "")),
+	}}
+
+	carryImportOriginalValues(rebuilt, stored)
+
+	if got := rebuilt.Elements[0].Children[0].OriginalValue; got != "" {
+		t.Errorf("carried %q onto a different path — the sample belongs to the element it was measured on", got)
+	}
+}
+
+// TestARewriteDoesNotOverwriteAnExplicitValue pins the precedence: what the
+// rebuild produced wins, and the stored value only fills a gap.
+func TestARewriteDoesNotOverwriteAnExplicitValue(t *testing.T) {
+	stored := &model.ImportMapping{Elements: []*model.ImportMappingElement{
+		importElem("(Object)|x", `"old"`),
+	}}
+	rebuilt := &model.ImportMapping{Elements: []*model.ImportMappingElement{
+		importElem("(Object)|x", `"new"`),
+	}}
+
+	carryImportOriginalValues(rebuilt, stored)
+
+	if got := rebuilt.Elements[0].OriginalValue; got != `"new"` {
+		t.Errorf("OriginalValue = %q, want the rebuilt value", got)
+	}
+}
+
+// TestANewMappingKeepsEmptyOriginalValues is the control for #882. With no
+// stored document there is nothing to carry, so a newly authored mapping still
+// writes empty — the decision that issue made, left intact.
+func TestANewMappingKeepsEmptyOriginalValues(t *testing.T) {
+	rebuilt := &model.ImportMapping{Elements: []*model.ImportMappingElement{
+		importElem("(Object)|title", ""),
+	}}
+
+	carryImportOriginalValues(rebuilt, nil)
+
+	if got := rebuilt.Elements[0].OriginalValue; got != "" {
+		t.Errorf("OriginalValue = %q on a new mapping, want empty", got)
+	}
+}
+
+// TestExportOriginalValuesAreCarriedToo pins the export twin, whose codec
+// writer hardcoded "" rather than carrying the field at all.
+func TestExportOriginalValuesAreCarriedToo(t *testing.T) {
+	stored := &model.ExportMapping{Elements: []*model.ExportMappingElement{
+		{JsonPath: "(Object)", Children: []*model.ExportMappingElement{
+			{JsonPath: "(Object)|title", OriginalValue: `"hello"`},
+		}},
+	}}
+	rebuilt := &model.ExportMapping{Elements: []*model.ExportMappingElement{
+		{JsonPath: "(Object)", Children: []*model.ExportMappingElement{
+			{JsonPath: "(Object)|title"},
+		}},
+	}}
+
+	carryExportOriginalValues(rebuilt, stored)
+
+	if got := rebuilt.Elements[0].Children[0].OriginalValue; got != `"hello"` {
+		t.Errorf("title = %q, want \"hello\"", got)
+	}
+}
+
+// TestSameJSONContentIgnoresFormatting pins the snippet half. describe
+// pretty-prints, so describe -> exec rewrote a one-line snippet into a
+// multi-line one — same JSON, different bytes, a diff for nothing.
+func TestSameJSONContentIgnoresFormatting(t *testing.T) {
+	cases := []struct {
+		name string
+		a, b string
+		want bool
+	}{
+		{"formatting only", `{"title": "hello", "qty": "3"}`, "{\n  \"title\": \"hello\",\n  \"qty\": \"3\"\n}", true},
+		{"key order", `{"a":1,"b":2}`, `{"b":2,"a":1}`, true},
+		{"different value", `{"a":1}`, `{"a":2}`, false},
+		{"added key", `{"a":1}`, `{"a":1,"b":2}`, false},
+		// Anything that does not parse counts as different, so a malformed
+		// snippet is replaced rather than silently kept.
+		{"malformed", `{"a":`, `{"a":1}`, false},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := sameJSONContent(tc.a, tc.b); got != tc.want {
+				t.Errorf("sameJSONContent = %v, want %v", got, tc.want)
+			}
+		})
+	}
+}

model/types.go

@@ -1359,6 +1359,11 @@ type ExportMappingElement struct {
 	// Shared fields
 	ExposedName string `json:"exposedName,omitempty"`
 	JsonPath    string `json:"jsonPath,omitempty"`
+	// OriginalValue is the sample parsed out of the JSON structure's snippet.
+	// Carried rather than derived: whether a mapping stores it is a per-document
+	// property mxcli cannot compute, so a rewrite preserves what was there
+	// instead of choosing (ako/mxcli#379).
+	OriginalValue string `json:"originalValue,omitempty"`
 	// XmlPath — see the note on ImportMappingElement.
 	XmlPath  string                  `json:"xmlPath,omitempty"`
 	Children []*ExportMappingElement `json:"children,omitempty"`

sdk/mpr/parser_export_mapping.go

@@ -151,6 +151,9 @@ func parseExportValueMappingElement(raw map[string]any) *model.ExportMappingElem
 	if v, ok := raw["Converter"].(string); ok {
 		elem.Converter = v
 	}
+	if v, ok := raw["OriginalValue"].(string); ok {
+		elem.OriginalValue = v
+	}
 
 	// Extract the primitive type from the nested Type object
 	if typeObj, ok := raw["Type"].(map[string]any); ok {