From 2198f765694bd3c5bee79a652dd1912af89874eb Mon Sep 17 00:00:00 2001
From: Wen Bo Li <50884368+wenovus@users.noreply.github.com>
Date: Fri, 27 May 2022 11:28:48 -0700
Subject: [PATCH] Fix MergeEmptyMap for MergeStructs and add schema_test (#686)

* Add schema_test for MergeEmptyMap

* Add more tests and fix MergeStructs case
---
 ygot/schema_tests/merge_test.go | 61 +++++++++++++++++++++++++++++++++
 ygot/struct_validation_map.go   | 16 +++++++--
 2 files changed, 75 insertions(+), 2 deletions(-)
 create mode 100644 ygot/schema_tests/merge_test.go

diff --git a/ygot/schema_tests/merge_test.go b/ygot/schema_tests/merge_test.go
new file mode 100644
index 000000000..7f14142c8
--- /dev/null
+++ b/ygot/schema_tests/merge_test.go
@@ -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)
+	}
+}
diff --git a/ygot/struct_validation_map.go b/ygot/struct_validation_map.go
index bbf6876cc..a08e74dba 100644
--- a/ygot/struct_validation_map.go
+++ b/ygot/struct_validation_map.go
@@ -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
 	}
@@ -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