Skip to content

Commit e85c7b2

Browse files
authored
Merge pull request #70 from jennybuckley/fix-removal
Implement list item and map key removal for multiple appliers
2 parents 8c17deb + fbe17a1 commit e85c7b2

File tree

13 files changed

+935
-89
lines changed

13 files changed

+935
-89
lines changed

fieldpath/managers.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ type APIVersion string
2020
type VersionedSet struct {
2121
*Set
2222
APIVersion APIVersion
23+
Applied bool
2324
}
2425

2526
// ManagedFields is a map from manager to VersionedSet (what they own in

fieldpath/set.go

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ func (s *Set) Intersection(s2 *Set) *Set {
8080
// Difference returns a Set containing elements which:
8181
// * appear in s
8282
// * do not appear in s2
83-
// * and are not children of elements that appear in s2.
8483
//
8584
// In other words, for leaf fields, this acts like a regular set difference
8685
// operation. When non leaf fields are compared with leaf fields ("parents"
@@ -159,10 +158,7 @@ func (s *Set) iteratePrefix(prefix Path, f func(Path)) {
159158
func (s *Set) WithPrefix(pe PathElement) *Set {
160159
subset, ok := s.Children.Get(pe)
161160
if !ok {
162-
subset = NewSet()
163-
}
164-
if s.Members.Has(pe) {
165-
subset.Insert(MakePathOrDie(""))
161+
return NewSet()
166162
}
167163
return subset
168164
}
@@ -287,9 +283,6 @@ func (s *SetNodeMap) Difference(s2 *Set) *SetNodeMap {
287283
out := &SetNodeMap{}
288284
for k, sn := range s.members {
289285
pe := sn.pathElement
290-
if s2.Members.Has(pe) {
291-
continue
292-
}
293286
if sn2, ok := s2.Children.members[k]; ok {
294287
diff := *sn.set.Difference(sn2.set)
295288
// We aren't permitted to add nodes with no elements.

fieldpath/set_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,7 @@ func TestSetIntersectionDifference(t *testing.T) {
292292
MakePathOrDie("foo", 0),
293293
MakePathOrDie("b0", nameFirst),
294294
MakePathOrDie("bar", "c0"),
295+
MakePathOrDie("cp", nameFirst, "child"),
295296
)
296297

297298
got := s1.Difference(s2)

internal/fixture/state.go

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,10 @@ func (s *State) Update(obj typed.YAMLObject, version fieldpath.APIVersion, manag
8888
return err
8989
}
9090
tv, err := s.Parser.FromYAML(obj)
91+
s.Live , err = s.Updater.Converter.Convert(s.Live, version)
92+
if err != nil {
93+
return err
94+
}
9195
managers, err := s.Updater.Update(s.Live, tv, version, s.Managers, manager)
9296
if err != nil {
9397
return err
@@ -108,6 +112,10 @@ func (s *State) Apply(obj typed.YAMLObject, version fieldpath.APIVersion, manage
108112
if err != nil {
109113
return err
110114
}
115+
s.Live , err = s.Updater.Converter.Convert(s.Live, version)
116+
if err != nil {
117+
return err
118+
}
111119
new, managers, err := s.Updater.Apply(s.Live, tv, version, s.Managers, manager, force)
112120
if err != nil {
113121
return err
@@ -135,6 +143,8 @@ func (s *State) CompareLive(obj typed.YAMLObject) (*typed.Comparison, error) {
135143
// dummyConverter doesn't convert, it just returns the same exact object, as long as a version is provided.
136144
type dummyConverter struct{}
137145

146+
var _ merge.Converter = dummyConverter{}
147+
138148
// Convert returns the object given in input, not doing any conversion.
139149
func (dummyConverter) Convert(v typed.TypedValue, version fieldpath.APIVersion) (typed.TypedValue, error) {
140150
if len(version) == 0 {
@@ -187,7 +197,7 @@ var _ Operation = &Apply{}
187197
func (a Apply) run(state *State) error {
188198
err := state.Apply(a.Object, a.APIVersion, a.Manager, false)
189199
if err != nil {
190-
if _, ok := err.(merge.Conflicts); !ok {
200+
if _, ok := err.(merge.Conflicts); !ok || a.Conflicts == nil {
191201
return err
192202
}
193203
}
@@ -254,10 +264,15 @@ type TestCase struct {
254264
Managed fieldpath.ManagedFields
255265
}
256266

257-
// Test runs the test-case using the given parser.
267+
// Test runs the test-case using the given parser and a dummy converter.
258268
func (tc TestCase) Test(parser typed.ParseableType) error {
269+
return tc.TestWithConverter(parser, &dummyConverter{})
270+
}
271+
272+
// TestWithConverter runs the test-case using the given parser and converter.
273+
func (tc TestCase) TestWithConverter(parser typed.ParseableType, converter merge.Converter) error {
259274
state := State{
260-
Updater: &merge.Updater{Converter: &dummyConverter{}},
275+
Updater: &merge.Updater{Converter: converter},
261276
Parser: parser,
262277
}
263278
// We currently don't have any test that converts, we can take

merge/key_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ func TestUpdateAssociativeLists(t *testing.T) {
8686
Managed: fieldpath.ManagedFields{
8787
"default": &fieldpath.VersionedSet{
8888
Set: _NS(
89+
_P("list", _KBF("name", _SV("b"))),
8990
_P("list", _KBF("name", _SV("b")), "name"),
9091
_P("list", _KBF("name", _SV("b")), "value"),
9192
),

0 commit comments

Comments
 (0)