Skip to content

Commit

Permalink
Fix MergeEmptyMap for MergeStructs and add schema_test (#686)
Browse files Browse the repository at this point in the history
* Add schema_test for MergeEmptyMap

* Add more tests and fix MergeStructs case
  • Loading branch information
wenovus authored May 27, 2022
1 parent 88f3a96 commit 2198f76
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 2 deletions.
61 changes: 61 additions & 0 deletions ygot/schema_tests/merge_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Copyright 2022 Google Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package schematest

import (
"testing"

"github.com/google/go-cmp/cmp"
oc "github.com/openconfig/ygot/exampleoc"
"github.com/openconfig/ygot/ygot"
)

func TestMergeEmptyMap(t *testing.T) {
want := &oc.Device{Interface: map[string]*oc.Interface{}}

hasNil := &oc.Device{}
hasEmpty := &oc.Device{Interface: map[string]*oc.Interface{}}
got, err := ygot.MergeStructs(hasNil, hasEmpty, &ygot.MergeEmptyMaps{})
if err != nil {
t.Fatal(err)
}
if diff := cmp.Diff(got, want); diff != "" {
t.Errorf("MergeStructs empty to nil (-got, +want):\n%s", diff)
}

hasNil = &oc.Device{}
hasEmpty = &oc.Device{Interface: map[string]*oc.Interface{}}
got, err = ygot.MergeStructs(hasEmpty, hasNil, &ygot.MergeEmptyMaps{})
if err != nil {
t.Fatal(err)
}
if diff := cmp.Diff(got, want); diff != "" {
t.Errorf("MergeStructs nil to empty (-got, +want):\n%s", diff)
}

hasNil = &oc.Device{}
hasEmpty = &oc.Device{Interface: map[string]*oc.Interface{}}
ygot.MergeStructInto(hasNil, hasEmpty, &ygot.MergeEmptyMaps{})
if diff := cmp.Diff(hasNil, want); diff != "" {
t.Errorf("MergeStructInto empty to nil (-got, +want):\n%s", diff)
}

hasNil = &oc.Device{}
hasEmpty = &oc.Device{Interface: map[string]*oc.Interface{}}
ygot.MergeStructInto(hasEmpty, hasNil, &ygot.MergeEmptyMaps{})
if diff := cmp.Diff(hasEmpty, want); diff != "" {
t.Errorf("MergeStructInto nil to empty (-got, +want):\n%s", diff)
}
}
16 changes: 14 additions & 2 deletions ygot/struct_validation_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ func MergeStructs(a, b GoStruct, opts ...MergeOpt) (GoStruct, error) {
return nil, fmt.Errorf("cannot merge structs that are not of matching types, %T != %T", a, b)
}

dst, err := DeepCopy(a)
dst, err := deepCopy(a, mergeEmptyMapsEnabled(opts))
if err != nil {
return nil, err
}
Expand All @@ -614,11 +614,23 @@ func MergeStructInto(dst, src GoStruct, opts ...MergeOpt) error {
// DeepCopy returns a deep copy of the supplied GoStruct. A new copy
// of the GoStruct is created, along with any underlying values.
func DeepCopy(s GoStruct) (GoStruct, error) {
return deepCopy(s, false)
}

// deepCopy returns a deep copy of the supplied GoStruct. A new copy
// of the GoStruct is created, along with any underlying values.
// If keepEmptyMaps is true, then empty but non-nil maps are kept in the deep
// copy.
func deepCopy(s GoStruct, keepEmptyMaps bool) (GoStruct, error) {
if util.IsNilOrInvalidValue(reflect.ValueOf(s)) {
return nil, fmt.Errorf("invalid input to DeepCopy, got nil value: %v", s)
}
n := reflect.New(reflect.TypeOf(s).Elem())
if err := copyStruct(n.Elem(), reflect.ValueOf(s).Elem()); err != nil {
var opts []MergeOpt
if keepEmptyMaps {
opts = append(opts, &MergeEmptyMaps{})
}
if err := copyStruct(n.Elem(), reflect.ValueOf(s).Elem(), opts...); err != nil {
return nil, fmt.Errorf("cannot DeepCopy struct: %v", err)
}
return n.Interface().(GoStruct), nil
Expand Down

0 comments on commit 2198f76

Please sign in to comment.