Skip to content

Commit 8bc0a77

Browse files
committed
Remove unions logic while keeping the API
We keep the whole API surface intact to stay within the same major release of structured-merge-diff even though the union code will now panic if used in any place. Note that the schema will also allow unions to still be specified since we want to be backwards compatible, but will be merely ignored (which it's always been anyway).
1 parent 9c99f49 commit 8bc0a77

File tree

6 files changed

+12
-909
lines changed

6 files changed

+12
-909
lines changed

Diff for: internal/fixture/state.go

-14
Original file line numberDiff line numberDiff line change
@@ -535,8 +535,6 @@ type TestCase struct {
535535
// Managed, if not nil, is the ManagedFields as expected
536536
// after all operations are run.
537537
Managed fieldpath.ManagedFields
538-
// Set to true if the test case needs the union behavior enabled.
539-
RequiresUnions bool
540538
// ReportInputOnNoop if we don't want to compare the output and
541539
// always return it.
542540
ReturnInputOnNoop bool
@@ -576,7 +574,6 @@ func (tc TestCase) BenchWithConverter(parser Parser, converter merge.Converter)
576574
Converter: converter,
577575
IgnoredFields: tc.IgnoredFields,
578576
ReturnInputOnNoop: tc.ReturnInputOnNoop,
579-
EnableUnions: tc.RequiresUnions,
580577
}
581578
state := State{
582579
Updater: updaterBuilder.BuildUpdater(),
@@ -599,7 +596,6 @@ func (tc TestCase) TestWithConverter(parser Parser, converter merge.Converter) e
599596
Converter: converter,
600597
IgnoredFields: tc.IgnoredFields,
601598
ReturnInputOnNoop: tc.ReturnInputOnNoop,
602-
EnableUnions: tc.RequiresUnions,
603599
}
604600
state := State{
605601
Updater: updaterBuilder.BuildUpdater(),
@@ -636,15 +632,5 @@ func (tc TestCase) TestWithConverter(parser Parser, converter merge.Converter) e
636632
}
637633
}
638634

639-
if !tc.RequiresUnions {
640-
// Re-run the test with unions on.
641-
tc2 := tc
642-
tc2.RequiresUnions = true
643-
err := tc2.TestWithConverter(parser, converter)
644-
if err != nil {
645-
return fmt.Errorf("fails if unions are on: %v", err)
646-
}
647-
}
648-
649635
return nil
650636
}

Diff for: merge/union_test.go

-234
This file was deleted.

Diff for: merge/update.go

+6-23
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ type UpdaterBuilder struct {
3434
Converter Converter
3535
IgnoredFields map[fieldpath.APIVersion]*fieldpath.Set
3636

37+
// Deprecated: Unions don't work, this flag will panic if used.
3738
EnableUnions bool
3839

3940
// Stop comparing the new object with old object after applying.
@@ -46,10 +47,12 @@ type UpdaterBuilder struct {
4647
}
4748

4849
func (u *UpdaterBuilder) BuildUpdater() *Updater {
50+
if u.EnableUnions {
51+
panic("unions don't work and have been disabled.")
52+
}
4953
return &Updater{
5054
Converter: u.Converter,
5155
IgnoredFields: u.IgnoredFields,
52-
enableUnions: u.EnableUnions,
5356
returnInputOnNoop: u.ReturnInputOnNoop,
5457
}
5558
}
@@ -63,17 +66,15 @@ type Updater struct {
6366
// Deprecated: This will eventually become private.
6467
IgnoredFields map[fieldpath.APIVersion]*fieldpath.Set
6568

66-
enableUnions bool
67-
6869
returnInputOnNoop bool
6970
}
7071

7172
// EnableUnionFeature turns on union handling. It is disabled by default until the
7273
// feature is complete.
7374
//
74-
// Deprecated: Use the builder instead.
75+
// Deprecated: Unions don't work, this function will panic if used.
7576
func (s *Updater) EnableUnionFeature() {
76-
s.enableUnions = true
77+
panic("unions don't work and have been disabled.")
7778
}
7879

7980
func (s *Updater) update(oldObject, newObject *typed.TypedValue, version fieldpath.APIVersion, managers fieldpath.ManagedFields, workflow string, force bool) (fieldpath.ManagedFields, *typed.Comparison, error) {
@@ -160,12 +161,6 @@ func (s *Updater) Update(liveObject, newObject *typed.TypedValue, version fieldp
160161
if err != nil {
161162
return nil, fieldpath.ManagedFields{}, err
162163
}
163-
if s.enableUnions {
164-
newObject, err = liveObject.NormalizeUnions(newObject)
165-
if err != nil {
166-
return nil, fieldpath.ManagedFields{}, err
167-
}
168-
}
169164
managers, compare, err := s.update(liveObject, newObject, version, managers, manager, true)
170165
if err != nil {
171166
return nil, fieldpath.ManagedFields{}, err
@@ -198,22 +193,10 @@ func (s *Updater) Apply(liveObject, configObject *typed.TypedValue, version fiel
198193
if err != nil {
199194
return nil, fieldpath.ManagedFields{}, err
200195
}
201-
if s.enableUnions {
202-
configObject, err = configObject.NormalizeUnionsApply(configObject)
203-
if err != nil {
204-
return nil, fieldpath.ManagedFields{}, err
205-
}
206-
}
207196
newObject, err := liveObject.Merge(configObject)
208197
if err != nil {
209198
return nil, fieldpath.ManagedFields{}, fmt.Errorf("failed to merge config: %v", err)
210199
}
211-
if s.enableUnions {
212-
newObject, err = configObject.NormalizeUnionsApply(newObject)
213-
if err != nil {
214-
return nil, fieldpath.ManagedFields{}, err
215-
}
216-
}
217200
lastSet := managers[manager]
218201
set, err := configObject.ToFieldSet()
219202
if err != nil {

0 commit comments

Comments
 (0)