switch last key token to be required

Author: dpb587-pivotal

patch/integration_test.go

@@ -1,16 +1,16 @@
 package patch_test
 
 import (
-  . "github.com/onsi/ginkgo"
-  . "github.com/onsi/gomega"
-  "gopkg.in/yaml.v2"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"gopkg.in/yaml.v2"
 
-  . "github.com/cppforlife/go-patch/patch"
+	. "github.com/cppforlife/go-patch/patch"
 )
 
 var _ = Describe("Integration", func() {
-  It("works in a basic way", func() {
-    inStr := `
+	It("works in a basic way", func() {
+		inStr := `
 releases:
 - name: capi
   version: 0.1
@@ -26,12 +26,12 @@ instance_groups:
   instances: 0
 `
 
-    var in interface{}
+		var in interface{}
 
-    err := yaml.Unmarshal([]byte(inStr), &in)
-    Expect(err).ToNot(HaveOccurred())
+		err := yaml.Unmarshal([]byte(inStr), &in)
+		Expect(err).ToNot(HaveOccurred())
 
-    ops1Str := `
+		ops1Str := `
 - type: replace
   path: /instance_groups/name=cloud_controller/instances
   value: 1
@@ -56,34 +56,34 @@ instance_groups:
     instances: 2
 `
 
-    var opDefs1 []OpDefinition
+		var opDefs1 []OpDefinition
 
-    err = yaml.Unmarshal([]byte(ops1Str), &opDefs1)
-    Expect(err).ToNot(HaveOccurred())
+		err = yaml.Unmarshal([]byte(ops1Str), &opDefs1)
+		Expect(err).ToNot(HaveOccurred())
 
-    ops1, err := NewOpsFromDefinitions(opDefs1)
-    Expect(err).ToNot(HaveOccurred())
+		ops1, err := NewOpsFromDefinitions(opDefs1)
+		Expect(err).ToNot(HaveOccurred())
 
-    ops2Str := `
+		ops2Str := `
 - type: replace
   path: /releases/name=capi/version
   value: latest
 `
 
-    var opDefs2 []OpDefinition
+		var opDefs2 []OpDefinition
 
-    err = yaml.Unmarshal([]byte(ops2Str), &opDefs2)
-    Expect(err).ToNot(HaveOccurred())
+		err = yaml.Unmarshal([]byte(ops2Str), &opDefs2)
+		Expect(err).ToNot(HaveOccurred())
 
-    ops2, err := NewOpsFromDefinitions(opDefs2)
-    Expect(err).ToNot(HaveOccurred())
+		ops2, err := NewOpsFromDefinitions(opDefs2)
+		Expect(err).ToNot(HaveOccurred())
 
-    ops := append(ops1, ops2...)
+		ops := append(ops1, ops2...)
 
-    res, err := ops.Apply(in)
-    Expect(err).ToNot(HaveOccurred())
+		res, err := ops.Apply(in)
+		Expect(err).ToNot(HaveOccurred())
 
-    outStr := `
+		outStr := `
 releases:
 - name: capi
   version: latest
@@ -109,11 +109,11 @@ instance_groups:
   instances: 2
 `
 
-    var out interface{}
+		var out interface{}
 
-    err = yaml.Unmarshal([]byte(outStr), &out)
-    Expect(err).ToNot(HaveOccurred())
+		err = yaml.Unmarshal([]byte(outStr), &out)
+		Expect(err).ToNot(HaveOccurred())
 
-    Expect(res).To(Equal(out))
-  })
+		Expect(res).To(Equal(out))
+	})
 })

patch/pointer.go

@@ -39,7 +39,7 @@ func NewPointerFromString(str string) (Pointer, error) {
 	tokenStrs := strings.Split(str, "/")
 	tokenStrs = tokenStrs[1:]
 
-	expecting := true
+	optional := false
 
 	for i, tok := range tokenStrs {
 		isLast := i == len(tokenStrs)-1
@@ -64,12 +64,12 @@ func NewPointerFromString(str string) (Pointer, error) {
 		}
 
 		if strings.HasSuffix(tok, "?") {
-			expecting = false
+			optional = true
 		}
 
 		token := KeyToken{
 			Key:      strings.TrimSuffix(tok, "?"),
-			Expected: expecting && !isLast,
+			Optional: optional,
 		}
 
 		tokens = append(tokens, token)
@@ -96,11 +96,9 @@ func (p Pointer) Tokens() []Token { return p.tokens }
 func (p Pointer) String() string {
 	var strs []string
 
-	expecting := true
-
-	for i, token := range p.tokens {
-		isLast := i == len(p.tokens)-1
+	optional := false
 
+	for _, token := range p.tokens {
 		switch typedToken := token.(type) {
 		case RootToken:
 			strs = append(strs, "")
@@ -116,10 +114,10 @@ func (p Pointer) String() string {
 
 		case KeyToken:
 			str := rfc6901Encoder.Replace(typedToken.Key)
-			if !isLast && !typedToken.Expected { // /key?/key2/key3
-				if expecting {
+			if typedToken.Optional { // /key?/key2/key3
+				if !optional {
 					str += "?"
-					expecting = false
+					optional = true
 				}
 			}
 			strs = append(strs, str)

patch/pointer_test.go

@@ -26,7 +26,7 @@ var testCases = []PointerTestCase{
 	{"/a~01b", []Token{RootToken{}, KeyToken{Key: "a~1b"}}},
 	{"/a~1b", []Token{RootToken{}, KeyToken{Key: "a/b"}}},
 
-	// Speacial chars
+	// Special chars
 	{"/c%d", []Token{RootToken{}, KeyToken{Key: "c%d"}}},
 	{"/e^f", []Token{RootToken{}, KeyToken{Key: "e^f"}}},
 	{"/g|h", []Token{RootToken{}, KeyToken{Key: "g|h"}}},
@@ -35,18 +35,23 @@ var testCases = []PointerTestCase{
 
 	// Maps
 	{"/key", []Token{RootToken{}, KeyToken{Key: "key"}}},
-	{"/key/", []Token{RootToken{}, KeyToken{Key: "key", Expected: true}, KeyToken{Key: ""}}},
-	{"/key/key2", []Token{RootToken{}, KeyToken{Key: "key", Expected: true}, KeyToken{Key: "key2"}}},
-	{"/key?/key2/key3", []Token{RootToken{}, KeyToken{Key: "key"}, KeyToken{Key: "key2"}, KeyToken{Key: "key3"}}},
+	{"/key/", []Token{RootToken{}, KeyToken{Key: "key"}, KeyToken{Key: ""}}},
+	{"/key/key2", []Token{RootToken{}, KeyToken{Key: "key"}, KeyToken{Key: "key2"}}},
+	{"/key?/key2/key3", []Token{
+		RootToken{},
+		KeyToken{Key: "key", Optional: true},
+		KeyToken{Key: "key2", Optional: true},
+		KeyToken{Key: "key3", Optional: true},
+	}},
 
 	// Array indices
 	{"/0", []Token{RootToken{}, IndexToken{0}}},
 	{"/1000001", []Token{RootToken{}, IndexToken{1000001}}},
 	{"/-2", []Token{RootToken{}, IndexToken{-2}}},
 
 	{"/-", []Token{RootToken{}, AfterLastIndexToken{}}},
-	{"/ary/-", []Token{RootToken{}, KeyToken{Key: "ary", Expected: true}, AfterLastIndexToken{}}},
-	{"/-/key", []Token{RootToken{}, KeyToken{Key: "-", Expected: true}, KeyToken{Key: "key"}}},
+	{"/ary/-", []Token{RootToken{}, KeyToken{Key: "ary"}, AfterLastIndexToken{}}},
+	{"/-/key", []Token{RootToken{}, KeyToken{Key: "-"}, KeyToken{Key: "key"}}},
 
 	// Matching index token
 	{"/name=val", []Token{RootToken{}, MatchingIndexToken{Key: "name", Value: "val"}}},
@@ -89,7 +94,11 @@ var _ = Describe("Pointer.String", func() {
 
 var _ = Describe("Pointer.Tokens", func() {
 	parsingTestCases := []PointerTestCase{
-		{"/key/key2?", []Token{RootToken{}, KeyToken{Key: "key", Expected: true}, KeyToken{Key: "key2"}}},
+		{"/key/key2?", []Token{
+			RootToken{},
+			KeyToken{Key: "key"},
+			KeyToken{Key: "key2", Optional: true},
+		}},
 	}
 
 	parsingTestCases = append(parsingTestCases, testCases...)

patch/remove_op.go

@@ -85,17 +85,21 @@ func (op RemoveOp) Apply(doc interface{}) (interface{}, error) {
 				return nil, newOpMapMismatchTypeErr(tokens[:i+2], obj)
 			}
 
-			if isLast {
-				delete(typedObj, typedToken.Key)
-			} else {
-				var found bool
+			var found bool
 
-				obj, found = typedObj[typedToken.Key]
-				if !found {
-					errMsg := "Expected to find a map key '%s' for path '%s'"
-					return nil, fmt.Errorf(errMsg, typedToken.Key, NewPointer(tokens[:i+2]))
+			obj, found = typedObj[typedToken.Key]
+			if !found {
+				if typedToken.Optional {
+					return doc, nil
 				}
 
+				errMsg := "Expected to find a map key '%s' for path '%s'"
+				return nil, fmt.Errorf(errMsg, typedToken.Key, NewPointer(tokens[:i+2]))
+			}
+
+			if isLast {
+				delete(typedObj, typedToken.Key)
+			} else {
 				prevUpdate = func(newObj interface{}) { typedObj[typedToken.Key] = newObj }
 			}
 

patch/remove_op_test.go

@@ -204,7 +204,7 @@ var _ = Describe("RemoveOp.Apply", func() {
 				"xyz": "xyz",
 			}
 
-			res, err := RemoveOp{Path: MustNewPointerFromString("/abc/efg")}.Apply(doc)
+			res, err := RemoveOp{Path: MustNewPointerFromString("/abc/efg?")}.Apply(doc)
 			Expect(err).ToNot(HaveOccurred())
 
 			Expect(res).To(Equal(map[interface{}]interface{}{
@@ -213,6 +213,23 @@ var _ = Describe("RemoveOp.Apply", func() {
 			}))
 		})
 
+		It("removes super nested map key that does not exist", func() {
+			doc := map[interface{}]interface{}{
+				"abc": map[interface{}]interface{}{
+					"efg": map[interface{}]interface{}{}, // wrong level
+				},
+			}
+
+			res, err := RemoveOp{Path: MustNewPointerFromString("/abc/opr?/efg")}.Apply(doc)
+			Expect(err).ToNot(HaveOccurred())
+
+			Expect(res).To(Equal(map[interface{}]interface{}{
+				"abc": map[interface{}]interface{}{
+					"efg": map[interface{}]interface{}{}, // wrong level
+				},
+			}))
+		})
+
 		It("returns an error if parent key does not exist", func() {
 			doc := map[interface{}]interface{}{"xyz": "xyz"}
 
@@ -222,6 +239,15 @@ var _ = Describe("RemoveOp.Apply", func() {
 				"Expected to find a map key 'abc' for path '/abc'"))
 		})
 
+		It("returns an error if key does not exist", func() {
+			doc := map[interface{}]interface{}{"xyz": "xyz"}
+
+			_, err := RemoveOp{Path: MustNewPointerFromString("/abc")}.Apply(doc)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(Equal(
+				"Expected to find a map key 'abc' for path '/abc'"))
+		})
+
 		It("returns an error if it's not a map when key is being accessed", func() {
 			_, err := RemoveOp{Path: MustNewPointerFromString("/abc")}.Apply([]interface{}{1, 2, 3})
 			Expect(err).To(HaveOccurred())

patch/replace_op.go

@@ -92,18 +92,20 @@ func (op ReplaceOp) Apply(doc interface{}) (interface{}, error) {
 				return nil, newOpMapMismatchTypeErr(tokens[:i+2], obj)
 			}
 
+			var found bool
+
+			obj, found = typedObj[typedToken.Key]
+			if !found && !typedToken.Optional {
+				errMsg := "Expected to find a map key '%s' for path '%s'"
+				return nil, fmt.Errorf(errMsg, typedToken.Key, NewPointer(tokens[:i+2]))
+			}
+
 			if isLast {
 				typedObj[typedToken.Key] = op.Value
 			} else {
-				var found bool
+				prevUpdate = func(newObj interface{}) { typedObj[typedToken.Key] = newObj }
 
-				obj, found = typedObj[typedToken.Key]
 				if !found {
-					if typedToken.Expected {
-						errMsg := "Expected to find a map key '%s' for path '%s'"
-						return nil, fmt.Errorf(errMsg, typedToken.Key, NewPointer(tokens[:i+2]))
-					}
-
 					// Determine what type of value to create based on next token
 					switch tokens[i+2].(type) {
 					case AfterLastIndexToken:
@@ -117,8 +119,6 @@ func (op ReplaceOp) Apply(doc interface{}) (interface{}, error) {
 
 					typedObj[typedToken.Key] = obj
 				}
-
-				prevUpdate = func(newObj interface{}) { typedObj[typedToken.Key] = newObj }
 			}
 
 		default:

patch/replace_op_test.go

@@ -261,7 +261,7 @@ var _ = Describe("ReplaceOp.Apply", func() {
 				"xyz": "xyz",
 			}
 
-			res, err := ReplaceOp{Path: MustNewPointerFromString("/abc/efg"), Value: 1}.Apply(doc)
+			res, err := ReplaceOp{Path: MustNewPointerFromString("/abc/efg?"), Value: 1}.Apply(doc)
 			Expect(err).ToNot(HaveOccurred())
 
 			Expect(res).To(Equal(map[interface{}]interface{}{
@@ -270,6 +270,24 @@ var _ = Describe("ReplaceOp.Apply", func() {
 			}))
 		})
 
+		It("replaces super nested map key that does not exist", func() {
+			doc := map[interface{}]interface{}{
+				"abc": map[interface{}]interface{}{
+					"efg": map[interface{}]interface{}{}, // wrong level
+				},
+			}
+
+			res, err := ReplaceOp{Path: MustNewPointerFromString("/abc/opr?/efg"), Value: 1}.Apply(doc)
+			Expect(err).ToNot(HaveOccurred())
+
+			Expect(res).To(Equal(map[interface{}]interface{}{
+				"abc": map[interface{}]interface{}{
+					"efg": map[interface{}]interface{}{}, // wrong level
+					"opr": map[interface{}]interface{}{"efg": 1},
+				},
+			}))
+		})
+
 		It("returns an error if parent key does not exist", func() {
 			doc := map[interface{}]interface{}{"xyz": "xyz"}
 
@@ -279,6 +297,15 @@ var _ = Describe("ReplaceOp.Apply", func() {
 				"Expected to find a map key 'abc' for path '/abc'"))
 		})
 
+		It("returns an error if key does not exist", func() {
+			doc := map[interface{}]interface{}{"xyz": "xyz"}
+
+			_, err := ReplaceOp{Path: MustNewPointerFromString("/abc")}.Apply(doc)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(Equal(
+				"Expected to find a map key 'abc' for path '/abc'"))
+		})
+
 		It("creates missing key if key is not expected to exist", func() {
 			doc := map[interface{}]interface{}{"xyz": "xyz"}
 

patch/tokens.go

@@ -17,5 +17,5 @@ type MatchingIndexToken struct {
 
 type KeyToken struct {
 	Key      string
-	Expected bool
+	Optional bool
 }