From cb20568d6550466b5ea03df91a20fba47685f639 Mon Sep 17 00:00:00 2001
From: guregu <greg@toki.waseda.jp>
Date: Sun, 1 Sep 2024 20:09:49 +0900
Subject: [PATCH] tidy: move issue #247 test to the big table

---
 encode_test.go   | 34 -------------------------
 encoding_test.go | 65 ++++++++++++++++++++++++++++++++++++++++++++++++
 reflect.go       |  8 +++---
 3 files changed, 70 insertions(+), 37 deletions(-)

diff --git a/encode_test.go b/encode_test.go
index 7d2c71b..b570f71 100644
--- a/encode_test.go
+++ b/encode_test.go
@@ -152,40 +152,6 @@ func TestMarshalItemAsymmetric(t *testing.T) {
 	}
 }
 
-func TestIssue247(t *testing.T) {
-	// https: //github.com/guregu/dynamo/issues/247
-	type ServerResponse struct {
-		EmbeddedID int
-	}
-	type TestAddition struct {
-		ServerResponse
-	}
-	type TestItem struct {
-		ID       int          `dynamo:"id,hash" json:"id"`
-		Name     string       `dynamo:"name,range" json:"name"`
-		Addition TestAddition `dynamo:"addition,omitempty"`
-	}
-	x := TestItem{ID: 1, Name: "test"}
-	item, err := MarshalItem(x)
-	if err != nil {
-		t.Fatal(err)
-	}
-	_, ok := item["addition"]
-	if ok {
-		t.Error("should be omitted")
-	}
-
-	x.Addition.EmbeddedID = 123
-	item, err = MarshalItem(x)
-	if err != nil {
-		t.Fatal(err)
-	}
-	_, ok = item["addition"]
-	if !ok {
-		t.Error("should be present")
-	}
-}
-
 type isValue_Kind interface {
 	isValue_Kind()
 }
diff --git a/encoding_test.go b/encoding_test.go
index d07c512..67727bb 100644
--- a/encoding_test.go
+++ b/encoding_test.go
@@ -541,6 +541,50 @@ var itemEncodingTests = []struct {
 			"Embedded": &types.AttributeValueMemberS{Value: "OK"},
 		},
 	},
+	{
+		name: "field with embedded struct + omitempty (empty)",
+		in:   Issue247{ID: 1, Name: "test"},
+		out: Item{
+			"id":   &types.AttributeValueMemberN{Value: "1"},
+			"name": &types.AttributeValueMemberS{Value: "test"},
+		},
+	},
+	{
+		name: "field with embedded struct + omitempty (not empty)",
+		in: Issue247{
+			ID:       1,
+			Name:     "test",
+			Addition: Issue247Field{Issue247Embedded: Issue247Embedded{EmbeddedID: 123}},
+		},
+		out: Item{
+			"id":       &types.AttributeValueMemberN{Value: "1"},
+			"name":     &types.AttributeValueMemberS{Value: "test"},
+			"addition": &types.AttributeValueMemberM{Value: Item{"EmbeddedID": &types.AttributeValueMemberN{Value: "123"}}},
+		},
+	},
+	{
+		name: "field with embedded struct subfield + omitempty (empty)",
+		in:   Issue247Alt{ID: 1, Name: "test", Addition: Issue247FieldAlt{}},
+		out: Item{
+			"id":   &types.AttributeValueMemberN{Value: "1"},
+			"name": &types.AttributeValueMemberS{Value: "test"},
+		},
+	},
+	{
+		name: "field with embedded struct subfield + omitempty (not empty)",
+		in: Issue247Alt{ID: 1, Name: "test", Addition: Issue247FieldAlt{
+			Field: Issue247Embedded{EmbeddedID: 123},
+		}},
+		out: Item{
+			"id":   &types.AttributeValueMemberN{Value: "1"},
+			"name": &types.AttributeValueMemberS{Value: "test"},
+			"addition": &types.AttributeValueMemberM{Value: Item{
+				"Field": &types.AttributeValueMemberM{Value: Item{
+					"EmbeddedID": &types.AttributeValueMemberN{Value: "123"},
+				}},
+			}},
+		},
+	},
 	{
 		name: "sets",
 		in: struct {
@@ -969,6 +1013,27 @@ func byteSlicePtr(a []byte) *[]byte {
 	return &a
 }
 
+type Issue247 struct {
+	ID       int           `dynamo:"id,hash" json:"id"`
+	Name     string        `dynamo:"name,range" json:"name"`
+	Addition Issue247Field `dynamo:"addition,omitempty"`
+}
+type Issue247Field struct {
+	Issue247Embedded
+}
+type Issue247Embedded struct {
+	EmbeddedID int
+}
+
+type Issue247Alt struct {
+	ID       int              `dynamo:"id,hash" json:"id"`
+	Name     string           `dynamo:"name,range" json:"name"`
+	Addition Issue247FieldAlt `dynamo:"addition,omitempty"`
+}
+type Issue247FieldAlt struct {
+	Field Issue247Embedded `dynamo:",omitempty"`
+}
+
 var (
 	_ Marshaler                = new(customMarshaler)
 	_ Unmarshaler              = new(customMarshaler)
diff --git a/reflect.go b/reflect.go
index bc586f5..68098f2 100644
--- a/reflect.go
+++ b/reflect.go
@@ -250,12 +250,14 @@ func (info *structInfo) isZero(rv reflect.Value) bool {
 	}
 	for _, field := range info.fields {
 		fv := dig(rv, field.index)
-		if !fv.IsValid() /* field doesn't exist */ {
+		if !fv.IsValid() {
+			// field doesn't exist
 			continue
 		}
 		if field.isZero == nil {
-			// TODO: https://github.com/guregu/dynamo/issues/247
-			// need to give child structs an isZero
+			// see: https://github.com/guregu/dynamo/issues/247
+			// this happens when there's no substance in the field
+			// (e.g. when it's embedded and isZero is handled by the parent type)
 			continue
 		}
 		if !field.isZero(fv) {