Skip to content

Commit 250da0e

Browse files
dsabetidsabeti
committed
fix off by one for :before/:after
1 parent a9941e6 commit 250da0e

File tree

5 files changed

+91
-52
lines changed

5 files changed

+91
-52
lines changed

‎patch/array_insertion.go

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ type ArrayInsertion struct {
1111
}
1212

1313
type ArrayInsertionIndex struct {
14-
Number int
15-
Insert bool
14+
number int
15+
insert bool
1616
}
1717

1818
func (i ArrayInsertion) Concrete() (ArrayInsertionIndex, error) {
@@ -48,16 +48,22 @@ func (i ArrayInsertion) Concrete() (ArrayInsertionIndex, error) {
4848
return ArrayInsertionIndex{}, err
4949
}
5050

51-
if before {
52-
num -= 1
53-
if num < 0 {
54-
num = 0
55-
}
56-
}
57-
58-
if after {
51+
if after && num != len(i.Array) {
5952
num += 1
6053
}
6154

6255
return ArrayInsertionIndex{num, before || after}, nil
6356
}
57+
58+
func (i ArrayInsertionIndex) Update(array []interface{}, obj interface{}) []interface{} {
59+
if i.insert {
60+
var newAry []interface{}
61+
newAry = append(newAry, array[:i.number]...) // not inclusive
62+
newAry = append(newAry, obj)
63+
newAry = append(newAry, array[i.number:]...) // inclusive
64+
return newAry
65+
}
66+
67+
array[i.number] = obj
68+
return array
69+
}

‎patch/array_insertion_test.go

Lines changed: 52 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,67 +9,95 @@ import (
99

1010
var _ = Describe("ArrayInsertion", func() {
1111
Describe("Concrete", func() {
12+
act := func(insertion ArrayInsertion, array []interface{}, obj interface{}) ([]interface{}, error) {
13+
insertion.Array = array
14+
15+
idx, err := insertion.Concrete()
16+
if err != nil {
17+
return nil, err
18+
}
19+
20+
return idx.Update(array, obj), nil
21+
}
22+
1223
It("returns specified index when not using any modifiers", func() {
13-
idx := ArrayInsertion{Index: 1, Modifiers: []Modifier{}, Array: []interface{}{1, 2, 3}}
14-
Expect(idx.Concrete()).To(Equal(ArrayInsertionIndex{1, false}))
24+
result, err := act(ArrayInsertion{Index: 1, Modifiers: []Modifier{}}, []interface{}{1, 2, 3}, 10)
25+
Expect(err).ToNot(HaveOccurred())
26+
Expect(result).To(Equal([]interface{}{1, 10, 3}))
1527
})
1628

1729
It("returns index adjusted for previous and next modifiers", func() {
1830
p := PrevModifier{}
1931
n := NextModifier{}
2032

21-
idx := ArrayInsertion{Index: 1, Modifiers: []Modifier{p, n, n}, Array: []interface{}{1, 2, 3}}
22-
Expect(idx.Concrete()).To(Equal(ArrayInsertionIndex{2, false}))
33+
result, err := act(ArrayInsertion{Index: 1, Modifiers: []Modifier{p, n, n}}, []interface{}{1, 2, 3}, 10)
34+
Expect(err).ToNot(HaveOccurred())
35+
Expect(result).To(Equal([]interface{}{1, 2, 10}))
2336
})
2437

2538
It("returns error if both after and before are used", func() {
26-
idx := ArrayInsertion{Index: 0, Modifiers: []Modifier{BeforeModifier{}, AfterModifier{}}, Array: []interface{}{}}
27-
_, err := idx.Concrete()
39+
_, err := act(ArrayInsertion{Index: 0, Modifiers: []Modifier{BeforeModifier{}, AfterModifier{}}}, []interface{}{}, 10)
2840
Expect(err.Error()).To(Equal("Expected to not find any modifiers after 'before' modifier, but found modifier 'patch.AfterModifier'"))
2941

30-
idx = ArrayInsertion{Index: 0, Modifiers: []Modifier{AfterModifier{}, BeforeModifier{}}, Array: []interface{}{}}
31-
_, err = idx.Concrete()
42+
_, err = act(ArrayInsertion{Index: 0, Modifiers: []Modifier{AfterModifier{}, BeforeModifier{}}}, []interface{}{}, 10)
3243
Expect(err.Error()).To(Equal("Expected to not find any modifiers after 'after' modifier, but found modifier 'patch.BeforeModifier'"))
3344

34-
idx = ArrayInsertion{Index: 0, Modifiers: []Modifier{AfterModifier{}, PrevModifier{}}, Array: []interface{}{}}
35-
_, err = idx.Concrete()
45+
_, err = act(ArrayInsertion{Index: 0, Modifiers: []Modifier{AfterModifier{}, PrevModifier{}}}, []interface{}{}, 10)
3646
Expect(err.Error()).To(Equal("Expected to not find any modifiers after 'after' modifier, but found modifier 'patch.PrevModifier'"))
3747
})
3848

3949
It("returns (0, true) when inserting in the beginning", func() {
40-
idx := ArrayInsertion{Index: 0, Modifiers: []Modifier{BeforeModifier{}}, Array: []interface{}{1, 2, 3}}
41-
Expect(idx.Concrete()).To(Equal(ArrayInsertionIndex{0, true}))
50+
result, err := act(ArrayInsertion{Index: 0, Modifiers: []Modifier{BeforeModifier{}}}, []interface{}{1, 2, 3}, 10)
51+
Expect(err).ToNot(HaveOccurred())
52+
Expect(result).To(Equal([]interface{}{10, 1, 2, 3}))
53+
54+
result, err = act(ArrayInsertion{Index: 0, Modifiers: []Modifier{AfterModifier{}}}, []interface{}{1, 2, 3}, 10)
55+
Expect(err).ToNot(HaveOccurred())
56+
Expect(result).To(Equal([]interface{}{1, 10, 2, 3}))
4257
})
4358

44-
It("returns (last+1, true) when inserting in the end", func() {
45-
idx := ArrayInsertion{Index: 2, Modifiers: []Modifier{AfterModifier{}}, Array: []interface{}{1, 2, 3}}
46-
Expect(idx.Concrete()).To(Equal(ArrayInsertionIndex{3, true}))
59+
It("returns (last, true) when inserting in the end", func() {
60+
result, err := act(ArrayInsertion{Index: 2, Modifiers: []Modifier{AfterModifier{}}}, []interface{}{1, 2, 3}, 10)
61+
Expect(err).ToNot(HaveOccurred())
62+
Expect(result).To(Equal([]interface{}{1, 2, 3, 10}))
4763

48-
idx = ArrayInsertion{Index: -1, Modifiers: []Modifier{AfterModifier{}}, Array: []interface{}{1, 2, 3}}
49-
Expect(idx.Concrete()).To(Equal(ArrayInsertionIndex{3, true}))
64+
result, err = act(ArrayInsertion{Index: -1, Modifiers: []Modifier{AfterModifier{}}}, []interface{}{1, 2, 3}, 10)
65+
Expect(err).ToNot(HaveOccurred())
66+
Expect(result).To(Equal([]interface{}{1, 2, 3, 10}))
5067
})
5168

52-
It("returns (mid+1, true) when inserting in the middle", func() {
53-
idx := ArrayInsertion{Index: 1, Modifiers: []Modifier{AfterModifier{}}, Array: []interface{}{1, 2, 3}}
54-
Expect(idx.Concrete()).To(Equal(ArrayInsertionIndex{2, true}))
69+
It("returns (mid, true) when inserting in the middle", func() {
70+
result, err := act(ArrayInsertion{Index: 1, Modifiers: []Modifier{AfterModifier{}}}, []interface{}{1, 2, 3}, 10)
71+
Expect(err).ToNot(HaveOccurred())
72+
Expect(result).To(Equal([]interface{}{1, 2, 10, 3}))
73+
74+
result, err = act(ArrayInsertion{Index: 1, Modifiers: []Modifier{BeforeModifier{}}}, []interface{}{1, 2, 3}, 10)
75+
Expect(err).ToNot(HaveOccurred())
76+
Expect(result).To(Equal([]interface{}{1, 10, 2, 3}))
77+
78+
result, err = act(ArrayInsertion{Index: 2, Modifiers: []Modifier{BeforeModifier{}}}, []interface{}{1, 2, 3}, 10)
79+
Expect(err).ToNot(HaveOccurred())
80+
Expect(result).To(Equal([]interface{}{1, 2, 10, 3}))
5581
})
5682

5783
It("returns index adjusted for previous, next modifiers and before modifier", func() {
5884
p := PrevModifier{}
5985
n := NextModifier{}
6086
b := BeforeModifier{}
6187

62-
idx := ArrayInsertion{Index: 1, Modifiers: []Modifier{p, n, n, b}, Array: []interface{}{1, 2, 3}}
63-
Expect(idx.Concrete()).To(Equal(ArrayInsertionIndex{1, true}))
88+
result, err := act(ArrayInsertion{Index: 1, Modifiers: []Modifier{p, n, n, b}}, []interface{}{1, 2, 3}, 10)
89+
Expect(err).ToNot(HaveOccurred())
90+
Expect(result).To(Equal([]interface{}{1, 2, 10, 3}))
6491
})
6592

6693
It("returns index adjusted for previous, next modifiers and after modifier", func() {
6794
p := PrevModifier{}
6895
n := NextModifier{}
6996
a := AfterModifier{}
7097

71-
idx := ArrayInsertion{Index: 1, Modifiers: []Modifier{p, n, n, a}, Array: []interface{}{1, 2, 3}}
72-
Expect(idx.Concrete()).To(Equal(ArrayInsertionIndex{3, true}))
98+
result, err := act(ArrayInsertion{Index: 1, Modifiers: []Modifier{p, n, n, a}}, []interface{}{1, 2, 3}, 10)
99+
Expect(err).ToNot(HaveOccurred())
100+
Expect(result).To(Equal([]interface{}{1, 2, 3, 10}))
73101
})
74102
})
75103
})

‎patch/integration_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ releases:
1616
version: 0.1
1717
1818
instance_groups:
19+
- name: foo
20+
1921
- name: cloud_controller
2022
instances: 0
2123
jobs:
@@ -111,6 +113,8 @@ releases:
111113
version: latest
112114
113115
instance_groups:
116+
- name: foo
117+
114118
- name: cc-before
115119
instances: 2
116120

‎patch/replace_op.go

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,7 @@ func (op ReplaceOp) Apply(doc interface{}) (interface{}, error) {
4343
return nil, err
4444
}
4545

46-
if idx.Insert {
47-
var newAry []interface{}
48-
newAry = append(newAry, typedObj[:idx.Number]...)
49-
newAry = append(newAry, clonedValue)
50-
newAry = append(newAry, typedObj[idx.Number:]...)
51-
prevUpdate(newAry)
52-
} else {
53-
typedObj[idx.Number] = clonedValue
54-
}
46+
prevUpdate(idx.Update(typedObj, clonedValue))
5547
} else {
5648
idx, err := ArrayIndex{Index: typedToken.Index, Modifiers: typedToken.Modifiers, Array: typedObj}.Concrete()
5749
if err != nil {
@@ -110,15 +102,7 @@ func (op ReplaceOp) Apply(doc interface{}) (interface{}, error) {
110102
return nil, err
111103
}
112104

113-
if idx.Insert {
114-
var newAry []interface{}
115-
newAry = append(newAry, typedObj[:idx.Number]...)
116-
newAry = append(newAry, clonedValue)
117-
newAry = append(newAry, typedObj[idx.Number:]...)
118-
prevUpdate(newAry)
119-
} else {
120-
typedObj[idx.Number] = clonedValue
121-
}
105+
prevUpdate(idx.Update(typedObj, clonedValue))
122106
} else {
123107
idx, err := ArrayIndex{Index: idxs[0], Modifiers: typedToken.Modifiers, Array: typedObj}.Concrete()
124108
if err != nil {

‎patch/replace_op_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,10 @@ var _ = Describe("ReplaceOp.Apply", func() {
5555
Expect(err).ToNot(HaveOccurred())
5656
Expect(res).To(Equal([]interface{}{10, 1, 2, 3}))
5757

58+
res, err = ReplaceOp{Path: MustNewPointerFromString("/1:before"), Value: 10}.Apply([]interface{}{1, 2, 3})
59+
Expect(err).ToNot(HaveOccurred())
60+
Expect(res).To(Equal([]interface{}{1, 10, 2, 3}))
61+
5862
res, err = ReplaceOp{Path: MustNewPointerFromString("/3:prev:after"), Value: 10}.Apply([]interface{}{1, 2, 3})
5963
Expect(err).ToNot(HaveOccurred())
6064
Expect(res).To(Equal([]interface{}{1, 2, 3, 10}))
@@ -203,6 +207,19 @@ var _ = Describe("ReplaceOp.Apply", func() {
203207
map[interface{}]interface{}{"key": "val2"},
204208
}
205209

210+
res, err = ReplaceOp{Path: MustNewPointerFromString("/key=val2:before"), Value: 100}.Apply(doc)
211+
Expect(err).ToNot(HaveOccurred())
212+
Expect(res).To(Equal([]interface{}{
213+
map[interface{}]interface{}{"key": "val"},
214+
100,
215+
map[interface{}]interface{}{"key": "val2"},
216+
}))
217+
218+
doc = []interface{}{
219+
map[interface{}]interface{}{"key": "val"},
220+
map[interface{}]interface{}{"key": "val2"},
221+
}
222+
206223
res, err = ReplaceOp{Path: MustNewPointerFromString("/key=val:next:after"), Value: 100}.Apply(doc)
207224
Expect(err).ToNot(HaveOccurred())
208225
Expect(res).To(Equal([]interface{}{

0 commit comments

Comments
 (0)