diff --git a/pkg/util/strategicpatch/patch.go b/pkg/util/strategicpatch/patch.go index 9e66d62e1cb..0ff10ec7875 100644 --- a/pkg/util/strategicpatch/patch.go +++ b/pkg/util/strategicpatch/patch.go @@ -35,27 +35,100 @@ import ( // Some of the content of this package was borrowed with minor adaptations from // evanphx/json-patch and openshift/origin. -const specialKey = "$patch" -const specialValue = "delete" +const ( + directiveMarker = "$patch" + deleteDirective = "delete" + replaceDirective = "replace" + mergeDirective = "merge" +) + +// IsPreconditionFailed returns true if the provided error indicates +// a precondition failed. +func IsPreconditionFailed(err error) bool { + _, ok := err.(errPreconditionFailed) + return ok +} + +type errPreconditionFailed struct { + message string +} + +func newErrPreconditionFailed(target map[string]interface{}) errPreconditionFailed { + s := fmt.Sprintf("precondition failed for: %v", target) + return errPreconditionFailed{s} +} + +func (err errPreconditionFailed) Error() string { + return err.message +} + +type errConflict struct { + message string +} + +func newErrConflict(patch, current []byte) errConflict { + s := fmt.Sprintf("patch:\n%s\nconflicts with current:\n%s\n", patch, current) + return errConflict{s} +} + +func (err errConflict) Error() string { + return err.message +} + +// IsConflict returns true if the provided error indicates +// a conflict between the patch and the current configuration. +func IsConflict(err error) bool { + _, ok := err.(errConflict) + return ok +} var errBadJSONDoc = fmt.Errorf("Invalid JSON document") var errNoListOfLists = fmt.Errorf("Lists of lists are not supported") -// CreateStrategicMergePatch creates a patch that can be passed to StrategicMergePatch. -// The original and modified documents must be passed to the method as json encoded content. -// It will return a mergeable json document with differences from original to modified, or an error -// if either of the two documents is invalid. +// The following code is adapted from github.com/openshift/origin/pkg/util/jsonmerge. +// Instead of defining a Delta that holds an original, a patch and a set of preconditions, +// the reconcile method accepts a set of preconditions as an argument. + +// PreconditionFunc asserts that an incompatible change is not present within a patch. +type PreconditionFunc func(interface{}) bool + +// RequireKeyUnchanged returns a precondition function that fails if the provided key +// is present in the patch (indicating that its value has changed). +func RequireKeyUnchanged(key string) PreconditionFunc { + return func(patch interface{}) bool { + patchMap, ok := patch.(map[string]interface{}) + if !ok { + return true + } + + // The presence of key means that its value has been changed, so the test fails. + _, ok = patchMap[key] + return !ok + } +} + +// Deprecated: Use the synonym CreateTwoWayMergePatch, instead. func CreateStrategicMergePatch(original, modified []byte, dataStruct interface{}) ([]byte, error) { + return CreateTwoWayMergePatch(original, modified, dataStruct) +} + +// CreateTwoWayMergePatch creates a patch that can be passed to StrategicMergePatch from an original +// document and a modified documernt, which are passed to the method as json encoded content. It will +// return a patch that yields the modified document when applied to the original document, or an error +// if either of the two documents is invalid. +func CreateTwoWayMergePatch(original, modified []byte, dataStruct interface{}, fns ...PreconditionFunc) ([]byte, error) { originalMap := map[string]interface{}{} - err := json.Unmarshal(original, &originalMap) - if err != nil { - return nil, errBadJSONDoc + if len(original) > 0 { + if err := json.Unmarshal(original, &originalMap); err != nil { + return nil, errBadJSONDoc + } } modifiedMap := map[string]interface{}{} - err = json.Unmarshal(modified, &modifiedMap) - if err != nil { - return nil, errBadJSONDoc + if len(modified) > 0 { + if err := json.Unmarshal(modified, &modifiedMap); err != nil { + return nil, errBadJSONDoc + } } t, err := getTagStructType(dataStruct) @@ -68,11 +141,18 @@ func CreateStrategicMergePatch(original, modified []byte, dataStruct interface{} return nil, err } + // Apply the preconditions to the patch, and return an error if any of them fail. + for _, fn := range fns { + if !fn(patchMap) { + return nil, newErrPreconditionFailed(patchMap) + } + } + return json.Marshal(patchMap) } // Returns a (recursive) strategic merge patch that yields modified when applied to original. -func diffMaps(original, modified map[string]interface{}, t reflect.Type, ignoreAdditions, ignoreChangesAndDeletions bool) (map[string]interface{}, error) { +func diffMaps(original, modified map[string]interface{}, t reflect.Type, ignoreChangesAndAdditions, ignoreDeletions bool) (map[string]interface{}, error) { patch := map[string]interface{}{} if t.Kind() == reflect.Ptr { t = t.Elem() @@ -80,42 +160,43 @@ func diffMaps(original, modified map[string]interface{}, t reflect.Type, ignoreA for key, modifiedValue := range modified { originalValue, ok := original[key] - // value was added if !ok { - if !ignoreAdditions { + // Key was added, so add to patch + if !ignoreChangesAndAdditions { patch[key] = modifiedValue } continue } - if key == specialKey { + if key == directiveMarker { originalString, ok := originalValue.(string) if !ok { - return nil, fmt.Errorf("invalid value for special key: %s", specialKey) + return nil, fmt.Errorf("invalid value for special key: %s", directiveMarker) } modifiedString, ok := modifiedValue.(string) if !ok { - return nil, fmt.Errorf("invalid value for special key: %s", specialKey) + return nil, fmt.Errorf("invalid value for special key: %s", directiveMarker) } if modifiedString != originalString { + patch[directiveMarker] = modifiedValue + } + + continue + } + + if reflect.TypeOf(originalValue) != reflect.TypeOf(modifiedValue) { + // Types have changed, so add to patch + if !ignoreChangesAndAdditions { patch[key] = modifiedValue } continue } - if !ignoreChangesAndDeletions { - // If types have changed, replace completely - if reflect.TypeOf(originalValue) != reflect.TypeOf(modifiedValue) { - patch[key] = modifiedValue - continue - } - } - - // Types are the same, compare values + // Types are the same, so compare values switch originalValueTyped := originalValue.(type) { case map[string]interface{}: modifiedValueTyped := modifiedValue.(map[string]interface{}) @@ -124,7 +205,7 @@ func diffMaps(original, modified map[string]interface{}, t reflect.Type, ignoreA return nil, err } - patchValue, err := diffMaps(originalValueTyped, modifiedValueTyped, fieldType, ignoreAdditions, ignoreChangesAndDeletions) + patchValue, err := diffMaps(originalValueTyped, modifiedValueTyped, fieldType, ignoreChangesAndAdditions, ignoreDeletions) if err != nil { return nil, err } @@ -141,8 +222,8 @@ func diffMaps(original, modified map[string]interface{}, t reflect.Type, ignoreA return nil, err } - if fieldPatchStrategy == "merge" { - patchValue, err := diffLists(originalValueTyped, modifiedValueTyped, fieldType.Elem(), fieldPatchMergeKey, ignoreAdditions, ignoreChangesAndDeletions) + if fieldPatchStrategy == mergeDirective { + patchValue, err := diffLists(originalValueTyped, modifiedValueTyped, fieldType.Elem(), fieldPatchMergeKey, ignoreChangesAndAdditions, ignoreDeletions) if err != nil { return nil, err } @@ -155,15 +236,16 @@ func diffMaps(original, modified map[string]interface{}, t reflect.Type, ignoreA } } - if !ignoreChangesAndDeletions { + if !ignoreChangesAndAdditions { if !reflect.DeepEqual(originalValue, modifiedValue) { + // Values are different, so add to patch patch[key] = modifiedValue } } } - if !ignoreChangesAndDeletions { - // Now add all deleted values as nil + if !ignoreDeletions { + // Add nils for deleted values for key := range original { _, found := modified[key] if !found { @@ -177,9 +259,9 @@ func diffMaps(original, modified map[string]interface{}, t reflect.Type, ignoreA // Returns a (recursive) strategic merge patch that yields modified when applied to original, // for a pair of lists with merge semantics. -func diffLists(original, modified []interface{}, t reflect.Type, mergeKey string, ignoreAdditions, ignoreChangesAndDeletions bool) ([]interface{}, error) { +func diffLists(original, modified []interface{}, t reflect.Type, mergeKey string, ignoreChangesAndAdditions, ignoreDeletions bool) ([]interface{}, error) { if len(original) == 0 { - if len(modified) == 0 || ignoreAdditions { + if len(modified) == 0 || ignoreChangesAndAdditions { return nil, nil } @@ -193,11 +275,10 @@ func diffLists(original, modified []interface{}, t reflect.Type, mergeKey string var patch []interface{} - // If the elements are not maps... if elementType.Kind() == reflect.Map { - patch, err = diffListsOfMaps(original, modified, t, mergeKey, ignoreAdditions, ignoreChangesAndDeletions) - } else { - patch, err = diffListsOfScalars(original, modified, ignoreAdditions) + patch, err = diffListsOfMaps(original, modified, t, mergeKey, ignoreChangesAndAdditions, ignoreDeletions) + } else if !ignoreChangesAndAdditions { + patch, err = diffListsOfScalars(original, modified) } if err != nil { @@ -209,7 +290,7 @@ func diffLists(original, modified []interface{}, t reflect.Type, mergeKey string // Returns a (recursive) strategic merge patch that yields modified when applied to original, // for a pair of lists of scalars with merge semantics. -func diffListsOfScalars(original, modified []interface{}, ignoreAdditions bool) ([]interface{}, error) { +func diffListsOfScalars(original, modified []interface{}) ([]interface{}, error) { if len(modified) == 0 { // There is no need to check the length of original because there is no way to create // a patch that deletes a scalar from a list of scalars with merge semantics. @@ -229,9 +310,7 @@ loopB: modifiedString := fmt.Sprintf("%v", modified[modifiedIndex]) if originalString >= modifiedString { if originalString != modifiedString { - if !ignoreAdditions { - patch = append(patch, modified[modifiedIndex]) - } + patch = append(patch, modified[modifiedIndex]) } continue loopB @@ -243,11 +322,9 @@ loopB: break } - if !ignoreAdditions { - // Add any remaining items found only in modified - for ; modifiedIndex < len(modifiedScalars); modifiedIndex++ { - patch = append(patch, modified[modifiedIndex]) - } + // Add any remaining items found only in modified + for ; modifiedIndex < len(modifiedScalars); modifiedIndex++ { + patch = append(patch, modified[modifiedIndex]) } return patch, nil @@ -258,7 +335,7 @@ var errBadArgTypeFmt = "expected a %s, but received a %t" // Returns a (recursive) strategic merge patch that yields modified when applied to original, // for a pair of lists of maps with merge semantics. -func diffListsOfMaps(original, modified []interface{}, t reflect.Type, mergeKey string, ignoreAdditions, ignoreChangesAndDeletions bool) ([]interface{}, error) { +func diffListsOfMaps(original, modified []interface{}, t reflect.Type, mergeKey string, ignoreChangesAndAdditions, ignoreDeletions bool) ([]interface{}, error) { patch := make([]interface{}, 0) originalSorted, err := sortMergeListsByNameArray(original, t, mergeKey, false) @@ -301,7 +378,8 @@ loopB: modifiedString := fmt.Sprintf("%v", modifiedValue) if originalString >= modifiedString { if originalString == modifiedString { - patchValue, err := diffMaps(originalMap, modifiedMap, t, ignoreAdditions, ignoreChangesAndDeletions) + // Merge key values are equal, so recurse + patchValue, err := diffMaps(originalMap, modifiedMap, t, ignoreChangesAndAdditions, ignoreDeletions) if err != nil { return nil, err } @@ -311,22 +389,24 @@ loopB: patchValue[mergeKey] = modifiedValue patch = append(patch, patchValue) } - } else if !ignoreAdditions { + } else if !ignoreChangesAndAdditions { + // Item was added, so add to patch patch = append(patch, modifiedMap) } continue loopB } - if !ignoreChangesAndDeletions { - patch = append(patch, map[string]interface{}{mergeKey: originalValue, specialKey: specialValue}) + if !ignoreDeletions { + // Item was deleted, so add delete directive + patch = append(patch, map[string]interface{}{mergeKey: originalValue, directiveMarker: deleteDirective}) } } break } - if !ignoreChangesAndDeletions { + if !ignoreDeletions { // Delete any remaining items found only in original for ; originalIndex < len(originalSorted); originalIndex++ { originalMap, ok := originalSorted[originalIndex].(map[string]interface{}) @@ -339,11 +419,11 @@ loopB: return nil, fmt.Errorf(errNoMergeKeyFmt, originalMap, mergeKey) } - patch = append(patch, map[string]interface{}{mergeKey: originalValue, specialKey: specialValue}) + patch = append(patch, map[string]interface{}{mergeKey: originalValue, directiveMarker: deleteDirective}) } } - if !ignoreAdditions { + if !ignoreChangesAndAdditions { // Add any remaining items found only in modified for ; modifiedIndex < len(modifiedSorted); modifiedIndex++ { patch = append(patch, modified[modifiedIndex]) @@ -353,7 +433,6 @@ loopB: return patch, nil } -// StrategicMergePatchData applies a patch using strategic merge patch semantics. // Deprecated: StrategicMergePatchData is deprecated. Use the synonym StrategicMergePatch, // instead, which follows the naming convention of evanphx/json-patch. func StrategicMergePatchData(original, patch []byte, dataStruct interface{}) ([]byte, error) { @@ -364,6 +443,14 @@ func StrategicMergePatchData(original, patch []byte, dataStruct interface{}) ([] // must be json encoded content. A patch can be created from an original and a modified document // by calling CreateStrategicMergePatch. func StrategicMergePatch(original, patch []byte, dataStruct interface{}) ([]byte, error) { + if original == nil { + original = []byte{} + } + + if patch == nil { + patch = []byte{} + } + originalMap := map[string]interface{}{} err := json.Unmarshal(original, &originalMap) if err != nil { @@ -390,6 +477,10 @@ func StrategicMergePatch(original, patch []byte, dataStruct interface{}) ([]byte } func getTagStructType(dataStruct interface{}) (reflect.Type, error) { + if dataStruct == nil { + return nil, fmt.Errorf(errBadArgTypeFmt, "struct", "nil") + } + t := reflect.TypeOf(dataStruct) if t.Kind() == reflect.Ptr { t = t.Elem() @@ -408,21 +499,26 @@ var errBadPatchTypeFmt = "unknown patch type: %s in map: %v" // both the original map and the patch because getting a deep copy of a map in // golang is highly non-trivial. func mergeMap(original, patch map[string]interface{}, t reflect.Type) (map[string]interface{}, error) { - // If the map contains "$patch: replace", don't merge it, just use the - // patch map directly. Later on, can add a non-recursive replace that only - // affects the map that the $patch is in. - if v, ok := patch[specialKey]; ok { - if v == "replace" { - delete(patch, specialKey) + if v, ok := patch[directiveMarker]; ok { + if v == replaceDirective { + // If the patch contains "$patch: replace", don't merge it, just use the + // patch directly. Later on, we can add a single level replace that only + // affects the map that the $patch is in. + delete(patch, directiveMarker) return patch, nil } + if v == deleteDirective { + // If the patch contains "$patch: delete", don't merge it, just return + // an empty map. + return map[string]interface{}{}, nil + } + return nil, fmt.Errorf(errBadPatchTypeFmt, v, patch) } // nil is an accepted value for original to simplify logic in other places. - // If original is nil, create a map so if patch requires us to modify the - // map, it'll work. + // If original is nil, replace it with an empty map and then apply the patch. if original == nil { original = map[string]interface{}{} } @@ -461,7 +557,7 @@ func mergeMap(original, patch map[string]interface{}, t reflect.Type) (map[strin return nil, err } - if originalType.Kind() == reflect.Map && fieldPatchStrategy != "replace" { + if originalType.Kind() == reflect.Map && fieldPatchStrategy != replaceDirective { typedOriginal := original[k].(map[string]interface{}) typedPatch := patchV.(map[string]interface{}) var err error @@ -473,7 +569,7 @@ func mergeMap(original, patch map[string]interface{}, t reflect.Type) (map[strin continue } - if originalType.Kind() == reflect.Slice && fieldPatchStrategy == "merge" { + if originalType.Kind() == reflect.Slice && fieldPatchStrategy == mergeDirective { elemType := fieldType.Elem() typedOriginal := original[k].([]interface{}) typedPatch := patchV.([]interface{}) @@ -527,9 +623,9 @@ func mergeSlice(original, patch []interface{}, elemType reflect.Type, mergeKey s replace := false for _, v := range patch { typedV := v.(map[string]interface{}) - patchType, ok := typedV[specialKey] + patchType, ok := typedV[directiveMarker] if ok { - if patchType == specialValue { + if patchType == deleteDirective { mergeValue, ok := typedV[mergeKey] if ok { _, originalKey, found, err := findMapInSliceBasedOnKeyValue(original, mergeKey, mergeValue) @@ -544,10 +640,10 @@ func mergeSlice(original, patch []interface{}, elemType reflect.Type, mergeKey s } else { return nil, fmt.Errorf("delete patch type with no merge key defined") } - } else if patchType == "replace" { + } else if patchType == replaceDirective { replace = true // Continue iterating through the array to prune any other $patch elements. - } else if patchType == "merge" { + } else if patchType == mergeDirective { return nil, fmt.Errorf("merging lists cannot yet be specified in the patch") } else { return nil, fmt.Errorf(errBadPatchTypeFmt, patchType, typedV) @@ -636,7 +732,7 @@ func sortMergeListsByName(mapJSON []byte, dataStruct interface{}) ([]byte, error func sortMergeListsByNameMap(s map[string]interface{}, t reflect.Type) (map[string]interface{}, error) { newS := map[string]interface{}{} for k, v := range s { - if k != specialKey { + if k != directiveMarker { fieldType, fieldPatchStrategy, fieldPatchMergeKey, err := forkedjson.LookupPatchMetadata(t, k) if err != nil { return nil, err @@ -650,7 +746,7 @@ func sortMergeListsByNameMap(s map[string]interface{}, t reflect.Type) (map[stri return nil, err } } else if typedV, ok := v.([]interface{}); ok { - if fieldPatchStrategy == "merge" { + if fieldPatchStrategy == mergeDirective { var err error v, err = sortMergeListsByNameArray(typedV, fieldType.Elem(), fieldPatchMergeKey, true) if err != nil { @@ -829,10 +925,8 @@ func sliceElementType(slices ...[]interface{}) (reflect.Type, error) { } // HasConflicts returns true if the left and right JSON interface objects overlap with -// different values in any key. The code will panic if an unrecognized type is passed -// (anything not returned by a JSON decode). All keys are required to be strings. -// Since patches of the same Type have congruent keys, this is valid for multiple patch -// types. +// different values in any key. All keys are required to be strings. Since patches of the +// same Type have congruent keys, this is valid for multiple patch types. func HasConflicts(left, right interface{}) (bool, error) { switch typedLeft := left.(type) { case map[string]interface{}: @@ -868,3 +962,69 @@ func HasConflicts(left, right interface{}) (bool, error) { return true, fmt.Errorf("unknown type: %v", reflect.TypeOf(left)) } } + +// CreateThreeWayMergePatch reconciles a modified configuration with an original configuration, +// while preserving any changes or deletions made to the original configuration in the interim, +// and not overridden by the current configuration. All three documents must be passed to the +// method as json encoded content. It will return a strategic merge patch, or an error if any +// of the documents is invalid, or if there are any preconditions that fail against the modified +// configuration, or, if force is false and there are conflicts between the modified and current +// configurations. +func CreateThreeWayMergePatch(original, modified, current []byte, dataStruct interface{}, force bool, fns ...PreconditionFunc) ([]byte, error) { + originalMap := map[string]interface{}{} + if len(original) > 0 { + if err := json.Unmarshal(original, &originalMap); err != nil { + return nil, errBadJSONDoc + } + } + + modifiedMap := map[string]interface{}{} + if len(modified) > 0 { + if err := json.Unmarshal(modified, &modifiedMap); err != nil { + return nil, errBadJSONDoc + } + } + + currentMap := map[string]interface{}{} + if len(current) > 0 { + if err := json.Unmarshal(current, ¤tMap); err != nil { + return nil, errBadJSONDoc + } + } + + t, err := getTagStructType(dataStruct) + if err != nil { + return nil, err + } + + // The patch is the difference from current to modified without deletions, plus deletions + // from original to modified. To find it, we compute deletions, which are the deletions from + // original to modified, and delta, which is the difference from current to modified without + // deletions, and then apply delta to deletions as a patch, which should be strictly additive. + deltaMap, err := diffMaps(currentMap, modifiedMap, t, false, true) + if err != nil { + return nil, err + } + + deletionsMap, err := diffMaps(originalMap, modifiedMap, t, true, false) + if err != nil { + return nil, err + } + + patchMap, err := mergeMap(deletionsMap, deltaMap, t) + if err != nil { + return nil, err + } + + // Apply the preconditions to the patch, and return an error if any of them fail. + for _, fn := range fns { + if !fn(patchMap) { + return nil, newErrPreconditionFailed(patchMap) + } + } + + // TODO(jackgr): If force is false, and the patch contains any keys that are also in current, + // then return a conflict error. + + return json.Marshal(patchMap) +} diff --git a/pkg/util/strategicpatch/patch_test.go b/pkg/util/strategicpatch/patch_test.go index be1b382138a..3fff3e52f6d 100644 --- a/pkg/util/strategicpatch/patch_test.go +++ b/pkg/util/strategicpatch/patch_test.go @@ -26,18 +26,18 @@ import ( "github.com/ghodss/yaml" ) -type StrategicMergePatchTestCases struct { - TestCases []StrategicMergePatchTestCase -} - type SortMergeListTestCases struct { TestCases []SortMergeListTestCase } -type StrategicMergePatchTestCaseData struct { - Original map[string]interface{} - Patch map[string]interface{} - Modified map[string]interface{} +type SortMergeListTestCase struct { + Description string + Original map[string]interface{} + Sorted map[string]interface{} +} + +type StrategicMergePatchTestCases struct { + TestCases []StrategicMergePatchTestCase } type StrategicMergePatchTestCase struct { @@ -45,10 +45,13 @@ type StrategicMergePatchTestCase struct { StrategicMergePatchTestCaseData } -type SortMergeListTestCase struct { - Description string - Original map[string]interface{} - Sorted map[string]interface{} +type StrategicMergePatchTestCaseData struct { + Original map[string]interface{} + TwoWay map[string]interface{} + Modified map[string]interface{} + Current map[string]interface{} + ThreeWay map[string]interface{} + Result map[string]interface{} } type MergeItem struct { @@ -63,6 +66,8 @@ type MergeItem struct { SimpleMap map[string]string } +var mergeItem MergeItem + // These are test cases for SortMergeList, used to assert that it (recursively) // sorts both merging and non merging lists correctly. var sortMergeListTestCaseData = []byte(` @@ -225,26 +230,22 @@ func TestSortMergeLists(t *testing.T) { tc := SortMergeListTestCases{} err := yaml.Unmarshal(sortMergeListTestCaseData, &tc) if err != nil { - t.Errorf("can't unmarshal test cases: %v", err) + t.Errorf("can't unmarshal test cases:%s\n", err) return } - var e MergeItem for _, c := range tc.TestCases { - sorted, err := sortMergeListsByName(toJSONOrFail(c.Original, t), e) - if err != nil { - t.Errorf("sort arrays returned error: %v", err) - } - - if !reflect.DeepEqual(sorted, toJSONOrFail(c.Sorted, t)) { - t.Errorf("sorting failed: %v\ntried to sort:\n%vexpected:\n%vgot:\n%v", - c.Description, toYAMLOrError(c.Original), toYAMLOrError(c.Sorted), jsonToYAMLOrError(sorted)) + original := testObjectToJSONOrFail(t, c.Original, c.Description) + sorted := testObjectToJSONOrFail(t, c.Sorted, c.Description) + if !reflect.DeepEqual(original, sorted) { + t.Errorf("error in test case: %s\ncannot sort object:\n%s\n%sexpected:\n%s\ngot:\n%s\n", + c.Description, toYAMLOrError(c.Original), toYAMLOrError(c.Sorted), jsonToYAMLOrError(original)) } } } // These are test cases for StrategicMergePatch that cannot be generated using -// CreateStrategicMergePatch because it doesn't use the replace directive, generate +// CreateTwoWayMergePatch because it doesn't use the replace directive, generate // duplicate integers for a merging list patch, or generate empty merging lists. var customStrategicMergePatchTestCaseData = []byte(` testCases: @@ -253,7 +254,7 @@ testCases: mergingIntList: - 1 - 2 - patch: + twoWay: mergingIntList: - 2 - 3 @@ -267,7 +268,7 @@ testCases: mergingList: - name: 1 - name: 2 - patch: + twoWay: mergingList: - $patch: replace modified: @@ -275,7 +276,7 @@ testCases: - description: merge empty merging lists original: mergingList: [] - patch: + twoWay: mergingList: [] modified: mergingList: [] @@ -283,14 +284,14 @@ testCases: original: name: 1 value: 1 - patch: + twoWay: $patch: replace modified: {} - description: add key and delete all keys from map original: name: 1 value: 1 - patch: + twoWay: other: a $patch: replace modified: @@ -301,110 +302,165 @@ func TestCustomStrategicMergePatch(t *testing.T) { tc := StrategicMergePatchTestCases{} err := yaml.Unmarshal(customStrategicMergePatchTestCaseData, &tc) if err != nil { - t.Errorf("can't unmarshal test cases: %v", err) + t.Errorf("can't unmarshal test cases:%v\n", err) return } for _, c := range tc.TestCases { - cOriginal, cPatch, cModified := testCaseToJSONOrFail(t, c) - testPatchApplication(t, cOriginal, cPatch, cModified, c.Description) + original, twoWay, modified := twoWayTestCaseToJSONOrFail(t, c) + testPatchApplication(t, original, twoWay, modified, c.Description) } } -func testCaseToJSONOrFail(t *testing.T, c StrategicMergePatchTestCase) ([]byte, []byte, []byte) { - var e MergeItem - cOriginal := toJSONOrFail(c.Original, t) - cPatch, err := sortMergeListsByName(toJSONOrFail(c.Patch, t), e) - if err != nil { - t.Errorf("error:%v sorting patch object:\n%v", err, c.Patch) - } - - cModified, err := sortMergeListsByName(toJSONOrFail(c.Modified, t), e) - if err != nil { - t.Errorf("error: %v sorting modified object:\n%v", err, c.Modified) - } - - return cOriginal, cPatch, cModified -} - -func testPatchApplication(t *testing.T, cOriginal, cPatch, cModified []byte, description string) { - var e MergeItem - modified, err := StrategicMergePatch(cOriginal, cPatch, e) - if err != nil { - t.Errorf("error applying patch: %v:\noriginal:\n%vpatch:\n%v", - err, jsonToYAMLOrError(cOriginal), jsonToYAMLOrError(cPatch)) - } - - // Sort the lists that have merged maps, since order is not significant. - modified, err = sortMergeListsByName(modified, e) - if err != nil { - t.Errorf("error: %v sorting modified object:\n%v", err, modified) - } - - if !reflect.DeepEqual(modified, cModified) { - t.Errorf("patch application failed: %v\noriginal:\n%vpatch:\n%vexpected modified:\n%vgot modified:\n%v", - description, jsonToYAMLOrError(cOriginal), jsonToYAMLOrError(cPatch), - jsonToYAMLOrError(cModified), jsonToYAMLOrError(modified)) - } -} - -// These are test cases for CreateStrategicMergePatch, used to assert that it -// generates the correct patch for a given outcome. They are also test cases for -// StrategicMergePatch, used to assert that applying a patch yields the correct -// outcome. +// These are test cases for StrategicMergePatch, to assert that applying a patch +// yields the correct outcome. They are also test cases for CreateTwoWayMergePatch +// and CreateThreeWayMergePatch, to assert that they both generate the correct patch +// for the given set of input documents. +// var createStrategicMergePatchTestCaseData = []byte(` testCases: - description: add field to map original: name: 1 - patch: + twoWay: value: 1 modified: name: 1 value: 1 + current: + name: 1 + other: a + threeWay: + value: 1 + result: + name: 1 + value: 1 + other: a + - description: add field to map with conflict + original: + name: 1 + twoWay: + value: 1 + modified: + name: 1 + value: 1 + current: + name: a + other: a + threeWay: + name: 1 + value: 1 + result: + name: 1 + value: 1 + other: a - description: add field and delete field from map original: name: 1 - patch: + twoWay: name: null value: 1 modified: value: 1 + current: + name: 1 + other: a + threeWay: + name: null + value: 1 + result: + value: 1 + other: a - description: delete field from nested map original: simpleMap: key1: 1 key2: 1 - patch: + twoWay: simpleMap: key2: null modified: simpleMap: key1: 1 + current: + simpleMap: + key1: 1 + key2: 1 + other: a + threeWay: + simpleMap: + key2: null + result: + simpleMap: + key1: 1 + other: a + - description: delete field from nested map with conflict + original: + simpleMap: + key1: 1 + key2: 1 + twoWay: + simpleMap: + key2: null + modified: + simpleMap: + key1: 1 + current: + simpleMap: + key1: a + key2: 1 + other: a + threeWay: + simpleMap: + key1: 1 + key2: null + result: + simpleMap: + key1: 1 + other: a - description: delete all fields from map original: name: 1 value: 1 - patch: + twoWay: name: null value: null modified: {} + current: + name: 1 + value: 1 + other: a + threeWay: + name: null + value: null + result: + other: a - description: add field and delete all fields from map original: name: 1 value: 1 - patch: - other: a + twoWay: name: null value: null + other: a modified: other: a + current: + name: 1 + value: a + other: b + threeWay: + name: null + value: null + other: a + result: + other: a - description: replace list of scalars original: nonMergingIntList: - 1 - 2 - patch: + twoWay: nonMergingIntList: - 2 - 3 @@ -412,12 +468,23 @@ testCases: nonMergingIntList: - 2 - 3 + current: + nonMergingIntList: + - 1 + threeWay: + nonMergingIntList: + - 2 + - 3 + result: + nonMergingIntList: + - 2 + - 3 - description: merge lists of scalars original: mergingIntList: - 1 - 2 - patch: + twoWay: mergingIntList: - 3 modified: @@ -425,13 +492,27 @@ testCases: - 1 - 2 - 3 + current: + mergingIntList: + - 1 + - 2 + - 4 + threeWay: + mergingIntList: + - 3 + result: + mergingIntList: + - 1 + - 2 + - 3 + - 4 - description: merge lists of maps original: mergingList: - name: 1 - name: 2 value: 2 - patch: + twoWay: mergingList: - name: 3 value: 3 @@ -442,13 +523,33 @@ testCases: value: 2 - name: 3 value: 3 + current: + mergingList: + - name: 1 + other: a + - name: 2 + value: 2 + other: b + threeWay: + mergingList: + - name: 3 + value: 3 + result: + mergingList: + - name: 1 + other: a + - name: 2 + value: 2 + other: b + - name: 3 + value: 3 - description: add field to map in merging list original: mergingList: - name: 1 - name: 2 value: 2 - patch: + twoWay: mergingList: - name: 1 value: 1 @@ -458,13 +559,32 @@ testCases: value: 1 - name: 2 value: 2 + current: + mergingList: + - name: 1 + other: a + - name: 2 + value: 2 + other: b + threeWay: + mergingList: + - name: 1 + value: 1 + result: + mergingList: + - name: 1 + value: 1 + other: a + - name: 2 + value: 2 + other: b - description: add duplicate field to map in merging list original: mergingList: - name: 1 - name: 2 value: 2 - patch: + twoWay: mergingList: - name: 1 value: 1 @@ -474,6 +594,24 @@ testCases: value: 1 - name: 2 value: 2 + current: + mergingList: + - name: 1 + value: 1 + other: a + - name: 2 + value: 2 + other: b + threeWay: + {} + result: + mergingList: + - name: 1 + value: 1 + other: a + - name: 2 + value: 2 + other: b - description: replace map field value in merging list original: mergingList: @@ -481,7 +619,7 @@ testCases: value: 1 - name: 2 value: 2 - patch: + twoWay: mergingList: - name: 1 value: a @@ -491,36 +629,145 @@ testCases: value: a - name: 2 value: 2 + current: + mergingList: + - name: 1 + value: 1 + other: a + - name: 2 + value: 2 + other: b + threeWay: + mergingList: + - name: 1 + value: a + result: + mergingList: + - name: 1 + value: a + other: a + - name: 2 + value: 2 + other: b + - description: replace map field value in merging list with conflict + original: + mergingList: + - name: 1 + value: 1 + - name: 2 + value: 2 + twoWay: + mergingList: + - name: 1 + value: a + modified: + mergingList: + - name: 1 + value: a + - name: 2 + value: 2 + current: + mergingList: + - name: 1 + value: 1 + other: a + - name: 2 + value: b + other: b + threeWay: + mergingList: + - name: 1 + value: a + - name: 2 + value: 2 + result: + mergingList: + - name: 1 + value: a + other: a + - name: 2 + value: 2 + other: b - description: delete map from merging list original: mergingList: - name: 1 - name: 2 - patch: + twoWay: mergingList: - name: 1 $patch: delete modified: mergingList: - name: 2 + current: + mergingList: + - name: 1 + other: a + - name: 2 + other: b + threeWay: + mergingList: + - name: 1 + $patch: delete + result: + mergingList: + - name: 2 + other: b - description: delete missing map from merging list original: mergingList: - name: 1 - name: 2 - patch: + twoWay: mergingList: - name: 1 $patch: delete modified: mergingList: - name: 2 + current: + mergingList: + - name: 2 + other: b + threeWay: + mergingList: + - name: 1 + $patch: delete + result: + mergingList: + - name: 2 + other: b + - description: delete map from merging list with conflict + original: + mergingList: + - name: 1 + - name: 2 + twoWay: + mergingList: + - name: 1 + $patch: delete + modified: + mergingList: + - name: 2 + current: + mergingList: + - name: 1 + other: a + threeWay: + mergingList: + - name: 1 + $patch: delete + - name: 2 + result: + mergingList: + - name: 2 - description: add map and delete map from merging list original: merginglist: - name: 1 - name: 2 - patch: + twoWay: merginglist: - name: 1 $patch: delete @@ -529,12 +776,64 @@ testCases: merginglist: - name: 2 - name: 3 + current: + merginglist: + - name: 1 + other: a + - name: 2 + other: b + - name: 4 + other: c + threeWay: + merginglist: + - name: 1 + $patch: delete + - name: 3 + result: + merginglist: + - name: 2 + other: b + - name: 3 + - name: 4 + other: c + - description: add map and delete map from merging list with conflict + original: + merginglist: + - name: 1 + - name: 2 + twoWay: + merginglist: + - name: 1 + $patch: delete + - name: 3 + modified: + merginglist: + - name: 2 + - name: 3 + current: + merginglist: + - name: 1 + other: a + - name: 4 + other: c + threeWay: + merginglist: + - name: 1 + $patch: delete + - name: 2 + - name: 3 + result: + merginglist: + - name: 2 + - name: 3 + - name: 4 + other: c - description: delete all maps from merging list original: mergingList: - name: 1 - name: 2 - patch: + twoWay: mergingList: - name: 1 $patch: delete @@ -542,12 +841,26 @@ testCases: $patch: delete modified: mergingList: [] + current: + mergingList: + - name: 1 + other: a + - name: 2 + other: b + threeWay: + mergingList: + - name: 1 + $patch: delete + - name: 2 + $patch: delete + result: + mergingList: [] - description: delete all maps from partially empty merging list original: mergingList: - name: 1 - name: 2 - patch: + twoWay: mergingList: - name: 1 $patch: delete @@ -555,12 +868,24 @@ testCases: $patch: delete modified: mergingList: [] + current: + mergingList: + - name: 1 + other: a + threeWay: + mergingList: + - name: 1 + $patch: delete + - name: 2 + $patch: delete + result: + mergingList: [] - description: delete all maps from empty merging list original: mergingList: - name: 1 - name: 2 - patch: + twoWay: mergingList: - name: 1 $patch: delete @@ -568,6 +893,16 @@ testCases: $patch: delete modified: mergingList: [] + current: + mergingList: [] + threeWay: + mergingList: + - name: 1 + $patch: delete + - name: 2 + $patch: delete + result: + mergingList: [] - description: delete field from map in merging list original: mergingList: @@ -575,7 +910,7 @@ testCases: value: 1 - name: 2 value: 2 - patch: + twoWay: mergingList: - name: 1 value: null @@ -584,6 +919,59 @@ testCases: - name: 1 - name: 2 value: 2 + current: + mergingList: + - name: 1 + value: 1 + other: a + - name: 2 + other: b + threeWay: + mergingList: + - name: 1 + value: null + - name: 2 + value: 2 + result: + mergingList: + - name: 1 + other: a + - name: 2 + value: 2 + other: b + - description: delete field from map in merging list with conflict + original: + mergingList: + - name: 1 + value: 1 + - name: 2 + value: 2 + twoWay: + mergingList: + - name: 1 + value: null + modified: + mergingList: + - name: 1 + - name: 2 + value: 2 + current: + mergingList: + - name: 1 + value: a + other: a + threeWay: + mergingList: + - name: 1 + value: null + - name: 2 + value: 2 + result: + mergingList: + - name: 1 + other: a + - name: 2 + value: 2 - description: replace non merging list nested in merging list original: mergingList: @@ -593,7 +981,7 @@ testCases: - name: 2 value: 2 - name: 2 - patch: + twoWay: mergingList: - name: 1 nonMergingList: @@ -606,6 +994,31 @@ testCases: - name: 1 value: 1 - name: 2 + current: + mergingList: + - name: 1 + other: a + nonMergingList: + - name: 1 + - name: 2 + value: 2 + - name: 2 + other: b + threeWay: + mergingList: + - name: 1 + nonMergingList: + - name: 1 + value: 1 + result: + mergingList: + - name: 1 + other: a + nonMergingList: + - name: 1 + value: 1 + - name: 2 + other: b - description: add field to map in merging list nested in merging list original: mergingList: @@ -615,7 +1028,7 @@ testCases: - name: 2 value: 2 - name: 2 - patch: + twoWay: mergingList: - name: 1 mergingList: @@ -630,28 +1043,175 @@ testCases: - name: 2 value: 2 - name: 2 + current: + mergingList: + - name: 1 + other: a + mergingList: + - name: 1 + - name: 2 + value: 2 + - name: 2 + other: b + threeWay: + mergingList: + - name: 1 + mergingList: + - name: 1 + value: 1 + result: + mergingList: + - name: 1 + other: a + mergingList: + - name: 1 + value: 1 + - name: 2 + value: 2 + - name: 2 + other: b + - description: add field to map in merging list nested in merging list with value conflict + original: + mergingList: + - name: 1 + mergingList: + - name: 1 + - name: 2 + value: 2 + - name: 2 + twoWay: + mergingList: + - name: 1 + mergingList: + - name: 1 + value: 1 + modified: + mergingList: + - name: 1 + mergingList: + - name: 1 + value: 1 + - name: 2 + value: 2 + - name: 2 + current: + mergingList: + - name: 1 + other: a + mergingList: + - name: 1 + value: a + - name: 2 + value: b + - name: 2 + other: b + threeWay: + mergingList: + - name: 1 + mergingList: + - name: 1 + value: 1 + - name: 2 + value: 2 + result: + mergingList: + - name: 1 + other: a + mergingList: + - name: 1 + value: 1 + - name: 2 + value: 2 + - name: 2 + other: b + - description: add field to map in merging list nested in merging list with deletion conflict + original: + mergingList: + - name: 1 + mergingList: + - name: 1 + - name: 2 + value: 2 + - name: 2 + twoWay: + mergingList: + - name: 1 + mergingList: + - name: 1 + value: 1 + modified: + mergingList: + - name: 1 + mergingList: + - name: 1 + value: 1 + - name: 2 + value: 2 + - name: 2 + current: + mergingList: + - name: 1 + other: a + mergingList: + - name: 2 + value: 2 + - name: 2 + other: b + threeWay: + mergingList: + - name: 1 + mergingList: + - name: 1 + value: 1 + result: + mergingList: + - name: 1 + other: a + mergingList: + - name: 1 + value: 1 + - name: 2 + value: 2 + - name: 2 + other: b - description: merge empty merging lists original: mergingList: [] - patch: + twoWay: {} modified: mergingList: [] current: mergingList: [] + threeWay: + {} result: mergingList: [] - description: add map to merging list by pointer original: mergeItemPtr: - name: 1 - patch: + twoWay: mergeItemPtr: - name: 2 modified: mergeItemPtr: - name: 1 - name: 2 + current: + mergeItemPtr: + - name: 1 + other: a + - name: 3 + threeWay: + mergeItemPtr: + - name: 2 + result: + mergeItemPtr: + - name: 1 + other: a + - name: 2 + - name: 3 - description: add field to map in merging list by pointer original: mergeItemPtr: @@ -661,7 +1221,7 @@ testCases: - name: 2 value: 2 - name: 2 - patch: + twoWay: mergeItemPtr: - name: 1 mergeItemPtr: @@ -676,41 +1236,155 @@ testCases: - name: 2 value: 2 - name: 2 + current: + mergeItemPtr: + - name: 1 + other: a + mergeItemPtr: + - name: 1 + other: a + - name: 2 + value: 2 + other: b + - name: 2 + other: b + threeWay: + mergeItemPtr: + - name: 1 + mergeItemPtr: + - name: 1 + value: 1 + result: + mergeItemPtr: + - name: 1 + other: a + mergeItemPtr: + - name: 1 + value: 1 + other: a + - name: 2 + value: 2 + other: b + - name: 2 + other: b `) func TestStrategicMergePatch(t *testing.T) { tc := StrategicMergePatchTestCases{} err := yaml.Unmarshal(createStrategicMergePatchTestCaseData, &tc) if err != nil { - t.Errorf("can't unmarshal test cases: %v", err) + t.Errorf("can't unmarshal test cases:%s\n", err) return } - var e MergeItem for _, c := range tc.TestCases { - cOriginal, cPatch, cModified := testCaseToJSONOrFail(t, c) - - // Test patch generation - patch, err := CreateStrategicMergePatch(cOriginal, cModified, e) - if err != nil { - t.Errorf("error generating patch: %s:\n%v", err, toYAMLOrError(c.StrategicMergePatchTestCaseData)) - } - - // Sort the lists that have merged maps, since order is not significant. - patch, err = sortMergeListsByName(patch, e) - if err != nil { - t.Errorf("error: %s sorting patch object:\n%v", err, patch) - } - - if !reflect.DeepEqual(patch, cPatch) { - t.Errorf("patch generation failed:\n%vgot patch:\n%v", toYAMLOrError(c.StrategicMergePatchTestCaseData), jsonToYAMLOrError(patch)) - } - - // Test patch application - testPatchApplication(t, cOriginal, cPatch, cModified, c.Description) + testTwoWayPatch(t, c) + testThreeWayPatch(t, c) } } +func testTwoWayPatch(t *testing.T, c StrategicMergePatchTestCase) { + original, expected, modified := twoWayTestCaseToJSONOrFail(t, c) + + actual, err := CreateTwoWayMergePatch(original, modified, mergeItem) + if err != nil { + t.Errorf("error: %s in test case: %s\ncannot create two way patch:%s:\n%s\n", + err, c.Description, toYAMLOrError(c.StrategicMergePatchTestCaseData)) + } + + testPatchCreation(t, expected, actual, c.Description) + testPatchApplication(t, original, actual, modified, c.Description) +} + +func twoWayTestCaseToJSONOrFail(t *testing.T, c StrategicMergePatchTestCase) ([]byte, []byte, []byte) { + return testObjectToJSONOrFail(t, c.Original, c.Description), + testObjectToJSONOrFail(t, c.TwoWay, c.Description), + testObjectToJSONOrFail(t, c.Modified, c.Description) +} + +func testThreeWayPatch(t *testing.T, c StrategicMergePatchTestCase) { + original, modified, current, expected, result := threeWayTestCaseToJSONOrFail(t, c) + + actual, err := CreateThreeWayMergePatch(original, modified, current, mergeItem, false) + if err != nil { + if IsConflict(err) { + if len(c.Result) > 0 { + t.Errorf("error in test case: %s\nunexpected conflict occurred:\n%s\n", + c.Description, toYAMLOrError(c.StrategicMergePatchTestCaseData)) + } + + return + } + + t.Errorf("error: %s in test case: %s\ncannot create three way patch:\n%s\n", + err, c.Description, toYAMLOrError(c.StrategicMergePatchTestCaseData)) + } + + if len(c.Result) < 1 { + t.Errorf("error in test case: %s\nexpected conflict did not occur:\n%s\n", + c.Description, toYAMLOrError(c.StrategicMergePatchTestCaseData)) + } + + testPatchCreation(t, expected, actual, c.Description) + testPatchApplication(t, current, actual, result, c.Description) +} + +func threeWayTestCaseToJSONOrFail(t *testing.T, c StrategicMergePatchTestCase) ([]byte, []byte, []byte, []byte, []byte) { + return testObjectToJSONOrFail(t, c.Original, c.Description), + testObjectToJSONOrFail(t, c.Modified, c.Description), + testObjectToJSONOrFail(t, c.Current, c.Description), + testObjectToJSONOrFail(t, c.ThreeWay, c.Description), + testObjectToJSONOrFail(t, c.Result, c.Description) +} + +func testPatchCreation(t *testing.T, expected, actual []byte, description string) { + sorted, err := sortMergeListsByName(actual, mergeItem) + if err != nil { + t.Errorf("error: %s in test case: %s\ncannot sort patch:\n%s\n", + err, description, jsonToYAMLOrError(actual)) + } + + if !reflect.DeepEqual(sorted, expected) { + t.Errorf("error in test case: %s\nexpected patch:\n%s\ngot:\n%s\n", + description, jsonToYAMLOrError(expected), jsonToYAMLOrError(sorted)) + } +} + +func testPatchApplication(t *testing.T, original, patch, expected []byte, description string) { + result, err := StrategicMergePatch(original, patch, mergeItem) + if err != nil { + t.Errorf("error: %s in test case: %s\ncannot apply patch:\n%s\nto original:\n%s\n", + err, description, jsonToYAMLOrError(patch), jsonToYAMLOrError(original)) + } + + sorted, err := sortMergeListsByName(result, mergeItem) + if err != nil { + t.Errorf("error: %s in test case: %s\ncannot sort result object:\n%s\n", + err, description, jsonToYAMLOrError(result)) + } + + if !reflect.DeepEqual(sorted, expected) { + format := "error in test case: %s\npatch application failed:\noriginal:\n%s\npatch:\n%s\nexpected:\n%s\ngot:\n%s\n" + t.Errorf(format, description, + jsonToYAMLOrError(original), jsonToYAMLOrError(patch), + jsonToYAMLOrError(expected), jsonToYAMLOrError(sorted)) + } +} + +func testObjectToJSONOrFail(t *testing.T, o map[string]interface{}, description string) []byte { + j, err := toJSON(o) + if err != nil { + t.Error(err) + } + + r, err := sortMergeListsByName(j, mergeItem) + if err != nil { + t.Errorf("error: %s in test case: %s\ncannot sort object:\n%s\n", err, description, j) + } + + return r +} + func toYAMLOrError(v interface{}) string { y, err := toYAML(v) if err != nil { @@ -720,15 +1394,6 @@ func toYAMLOrError(v interface{}) string { return y } -func toJSONOrFail(v interface{}, t *testing.T) []byte { - theJSON, err := toJSON(v) - if err != nil { - t.Error(err) - } - - return theJSON -} - func jsonToYAMLOrError(j []byte) string { y, err := jsonToYAML(j) if err != nil { @@ -741,7 +1406,7 @@ func jsonToYAMLOrError(j []byte) string { func toYAML(v interface{}) (string, error) { y, err := yaml.Marshal(v) if err != nil { - return "", fmt.Errorf("yaml marshal failed: %v\n%v", err, spew.Sdump(v)) + return "", fmt.Errorf("yaml marshal failed:%v\n%v\n", err, spew.Sdump(v)) } return string(y), nil @@ -750,7 +1415,7 @@ func toYAML(v interface{}) (string, error) { func toJSON(v interface{}) ([]byte, error) { j, err := json.Marshal(v) if err != nil { - return nil, fmt.Errorf("json marshal failed: %v\n%v", err, spew.Sdump(v)) + return nil, fmt.Errorf("json marshal failed:%v\n%v\n", err, spew.Sdump(v)) } return j, nil @@ -759,7 +1424,7 @@ func toJSON(v interface{}) ([]byte, error) { func jsonToYAML(j []byte) ([]byte, error) { y, err := yaml.JSONToYAML(j) if err != nil { - return nil, fmt.Errorf("json to yaml failed: %v\n%v", err, j) + return nil, fmt.Errorf("json to yaml failed:%v\n%v\n", err, j) } return y, nil