Skip to content

Commit 7d04114

Browse files
apelisseapelisse
committed
typed: Update compare algorithm to handle duplicates
1 parent 66711bf commit 7d04114

File tree

2 files changed

+155
-26
lines changed

2 files changed

+155
-26
lines changed

Diff for: ‎typed/compare.go

+71-24
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,14 @@ func (w *compareWalker) visitListItems(t *schema.List, lhs, rhs value.List) (err
213213
lLen = lhs.Length()
214214
}
215215

216+
maxLength := rLen
217+
if lLen > maxLength {
218+
maxLength = lLen
219+
}
220+
// Contains all the unique PEs between lhs and rhs, exactly once.
221+
allPEs := make([]fieldpath.PathElement, 0, maxLength)
222+
223+
// So for each value that we find, we save them in a list, we do that for both sides.
216224
lValues := fieldpath.MakePathElementValueMap(lLen)
217225
for i := 0; i < lLen; i++ {
218226
child := lhs.At(i)
@@ -224,11 +232,14 @@ func (w *compareWalker) visitListItems(t *schema.List, lhs, rhs value.List) (err
224232
// this element.
225233
continue
226234
}
227-
// Ignore repeated occurences of `pe`.
228-
if _, found := lValues.Get(pe); found {
229-
continue
235+
236+
if v, found := lValues.Get(pe); found {
237+
list := v.Unstructured().([]value.Value)
238+
lValues.Insert(pe, value.NewValueInterface(append(list, child)))
239+
} else {
240+
lValues.Insert(pe, value.NewValueInterface([]value.Value{child}))
241+
allPEs = append(allPEs, pe)
230242
}
231-
lValues.Insert(pe, child)
232243
}
233244

234245
rValues := fieldpath.MakePathElementValueMap(rLen)
@@ -242,31 +253,67 @@ func (w *compareWalker) visitListItems(t *schema.List, lhs, rhs value.List) (err
242253
// this element.
243254
continue
244255
}
245-
// Ignore repeated occurences of `pe`.
246-
if _, found := rValues.Get(pe); found {
247-
continue
256+
if v, found := rValues.Get(pe); found {
257+
list := v.Unstructured().([]value.Value)
258+
rValues.Insert(pe, value.NewValueInterface(append(list, rValue)))
259+
} else {
260+
rValues.Insert(pe, value.NewValueInterface([]value.Value{rValue}))
261+
if _, found := lValues.Get(pe); !found {
262+
allPEs = append(allPEs, pe)
263+
}
248264
}
249-
rValues.Insert(pe, rValue)
250-
// We can compare with nil if lValue is not present.
251-
lValue, _ := lValues.Get(pe)
252-
errs = append(errs, w.compareListItem(t, pe, lValue, rValue)...)
253265
}
254266

255-
// Add items from left that are not in right.
256-
for i := 0; i < lLen; i++ {
257-
lValue := lhs.At(i)
258-
pe, err := listItemToPathElement(w.allocator, w.schema, t, lValue)
259-
if err != nil {
260-
errs = append(errs, errorf("element %v: %v", i, err.Error())...)
261-
// If we can't construct the path element, we can't
262-
// even report errors deeper in the schema, so bail on
263-
// this element.
264-
continue
267+
for _, pe := range allPEs {
268+
lList := []value.Value(nil)
269+
if l, ok := lValues.Get(pe); ok {
270+
lList = l.Unstructured().([]value.Value)
265271
}
266-
if _, found := rValues.Get(pe); found {
267-
continue
272+
rList := []value.Value(nil)
273+
if l, ok := rValues.Get(pe); ok {
274+
rList = l.Unstructured().([]value.Value)
275+
}
276+
277+
switch {
278+
case len(lList) == 0 && len(rList) == 0:
279+
// We shouldn't be here anyway.
280+
return
281+
case len(lList) <= 1 && len(rList) <= 1:
282+
lValue := value.Value(nil)
283+
if len(lList) != 0 {
284+
lValue = lList[0]
285+
}
286+
rValue := value.Value(nil)
287+
if len(rList) != 0 {
288+
rValue = rList[0]
289+
}
290+
errs = append(errs, w.compareListItem(t, pe, lValue, rValue)...)
291+
case len(lList) >= 2 && len(rList) >= 2:
292+
listEqual := func(lList, rList []value.Value) bool {
293+
if len(lList) != len(rList) {
294+
return false
295+
}
296+
for i := range lList {
297+
if !value.Equals(lList[i], rList[i]) {
298+
return false
299+
}
300+
}
301+
return true
302+
}
303+
if !listEqual(lList, rList) {
304+
w.comparison.Modified.Insert(append(w.path, pe))
305+
}
306+
case len(lList) >= 2:
307+
if len(rList) != 0 {
308+
errs = append(errs, w.compareListItem(t, pe, nil, rList[0])...)
309+
}
310+
w.comparison.Removed.Insert(append(w.path, pe))
311+
case len(rList) >= 2:
312+
if len(lList) != 0 {
313+
errs = append(errs, w.compareListItem(t, pe, lList[0], nil)...)
314+
}
315+
w.comparison.Added.Insert(append(w.path, pe))
268316
}
269-
errs = append(errs, w.compareListItem(t, pe, lValue, nil)...)
270317
}
271318

272319
return

Diff for: ‎typed/symdiff_test.go

+84-2
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,24 @@ var symdiffCases = []symdiffTestCase{{
596596
removed: _NS(),
597597
modified: _NS(),
598598
added: _NS(_P("setStr", _V("c"))),
599+
}, {
600+
lhs: `{"setStr":["a"]}`,
601+
rhs: `{"setStr":["a","b","b"]}`,
602+
removed: _NS(),
603+
modified: _NS(),
604+
added: _NS(_P("setStr", _V("b"))),
605+
}, {
606+
lhs: `{"setStr":["a","b"]}`,
607+
rhs: `{"setStr":["a","b","b"]}`,
608+
removed: _NS(_P("setStr", _V("b"))),
609+
modified: _NS(),
610+
added: _NS(_P("setStr", _V("b"))),
611+
}, {
612+
lhs: `{"setStr":["b","b"]}`,
613+
rhs: `{"setStr":["a","b","b"]}`,
614+
removed: _NS(),
615+
modified: _NS(),
616+
added: _NS(_P("setStr", _V("a"))),
599617
}, {
600618
lhs: `{"setStr":["a","b","c"]}`,
601619
rhs: `{"setStr":[]}`,
@@ -618,6 +636,28 @@ var symdiffCases = []symdiffTestCase{{
618636
removed: _NS(_P("setNumeric", _V(3.14159))),
619637
modified: _NS(),
620638
added: _NS(_P("setNumeric", _V(3))),
639+
}, {
640+
lhs: `{"setStr":["a","b","b","c","a"]}`,
641+
rhs: `{"setStr":[]}`,
642+
removed: _NS(
643+
_P("setStr", _V("a")),
644+
_P("setStr", _V("b")),
645+
_P("setStr", _V("c")),
646+
),
647+
modified: _NS(),
648+
added: _NS(),
649+
}, {
650+
lhs: `{"setBool":[true,true]}`,
651+
rhs: `{"setBool":[false]}`,
652+
removed: _NS(_P("setBool", _V(true))),
653+
modified: _NS(),
654+
added: _NS(_P("setBool", _V(false))),
655+
}, {
656+
lhs: `{"setNumeric":[1,2,2,3.14159,1]}`,
657+
rhs: `{"setNumeric":[1,2,3]}`,
658+
removed: _NS(_P("setNumeric", _V(1)), _P("setNumeric", _V(2)), _P("setNumeric", _V(3.14159))),
659+
modified: _NS(),
660+
added: _NS(_P("setNumeric", _V(1)), _P("setNumeric", _V(2)), _P("setNumeric", _V(3))),
621661
}},
622662
}, {
623663
name: "associative list",
@@ -743,6 +783,48 @@ var symdiffCases = []symdiffTestCase{{
743783
removed: _NS(),
744784
modified: _NS(_P("atomicList")),
745785
added: _NS(),
786+
}, {
787+
lhs: `{"list":[{"key":"a","id":1,"nv":2},{"key":"b","id":1},{"key":"a","id":1,"bv":true}]}`,
788+
rhs: `{"list":[{"key":"a","id":1},{"key":"a","id":2}]}`,
789+
removed: _NS(
790+
_P("list", _KBF("key", "b", "id", 1)),
791+
_P("list", _KBF("key", "b", "id", 1), "key"),
792+
_P("list", _KBF("key", "b", "id", 1), "id"),
793+
_P("list", _KBF("key", "a", "id", 1)),
794+
),
795+
modified: _NS(),
796+
added: _NS(
797+
_P("list", _KBF("key", "a", "id", 1)),
798+
_P("list", _KBF("key", "a", "id", 1), "key"),
799+
_P("list", _KBF("key", "a", "id", 1), "id"),
800+
_P("list", _KBF("key", "a", "id", 2)),
801+
_P("list", _KBF("key", "a", "id", 2), "key"),
802+
_P("list", _KBF("key", "a", "id", 2), "id"),
803+
),
804+
}, {
805+
lhs: `{"list":[{"key":"a","id":1},{"key":"b","id":1},{"key":"a","id":1,"bv":true}]}`,
806+
rhs: `{"list":[{"key":"a","id":1},{"key":"b","id":1},{"key":"a","id":1,"bv":true,"nv":1}]}`,
807+
removed: _NS(),
808+
modified: _NS(_P("list", _KBF("key", "a", "id", 1))),
809+
added: _NS(),
810+
}, {
811+
lhs: `{"list":[{"key":"a","id":1},{"key":"b","id":1},{"key":"a","id":1,"bv":true}]}`,
812+
rhs: `{"list":[{"key":"a","id":1},{"key":"b","id":1},{"key":"a","id":1,"bv":true}]}`,
813+
removed: _NS(),
814+
modified: _NS(),
815+
added: _NS(),
816+
}, {
817+
lhs: `{"list":[{"key":"a","id":1},{"key":"b","id":1},{"key":"a","id":1,"bv":true}]}`,
818+
rhs: `{"list":[{"key":"a","id":1},{"key":"b","id":1,"bv":true},{"key":"a","id":1,"bv":true}]}`,
819+
removed: _NS(),
820+
modified: _NS(),
821+
added: _NS(_P("list", _KBF("key", "b", "id", 1), "bv")),
822+
}, {
823+
lhs: `{"list":[{"key":"a","id":1},{"key":"b","id":1},{"key":"a","id":1,"bv":true}]}`,
824+
rhs: `{"list":[{"key":"a","id":1},{"key":"b","id":1},{"key":"a","id":1,"bv":true}],"atomicList":["unrelated"]}`,
825+
removed: _NS(),
826+
modified: _NS(),
827+
added: _NS(_P("atomicList")),
746828
}},
747829
}}
748830

@@ -757,11 +839,11 @@ func (tt symdiffTestCase) test(t *testing.T) {
757839
t.Parallel()
758840
pt := parser.Type(tt.rootTypeName)
759841

760-
tvLHS, err := pt.FromYAML(quint.lhs)
842+
tvLHS, err := pt.FromYAML(quint.lhs, typed.AllowDuplicates)
761843
if err != nil {
762844
t.Fatalf("failed to parse lhs: %v", err)
763845
}
764-
tvRHS, err := pt.FromYAML(quint.rhs)
846+
tvRHS, err := pt.FromYAML(quint.rhs, typed.AllowDuplicates)
765847
if err != nil {
766848
t.Fatalf("failed to parse rhs: %v", err)
767849
}

0 commit comments

Comments
 (0)