From 2aa510c27b529618d580a7e15f9b4a4587c411a3 Mon Sep 17 00:00:00 2001
From: Paul Duffin <paulduffin@google.com>
Date: Fri, 26 Mar 2021 09:09:03 +0000
Subject: [PATCH] Always shard structs if they would exceed maxNameSize even if
 unfiltered

Previously, a struct (anonymous or named) whose fields all matched the
predicate would not be sharded and would simply be reused. However,
that could break the maxNameSize limitation which could cause problems
for the caller.

This change makes sure that the supplied struct is only reused if it
does not exceed the maxNameSize, and otherwise is sharded.

Bug: 183777071
Test: m nothing
Change-Id: I8af272ec121077a43333e72b67cfd0e493c83362
---
 proptools/filter.go      |  8 +++++++-
 proptools/filter_test.go | 44 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/proptools/filter.go b/proptools/filter.go
index e6b3336..54a20d5 100644
--- a/proptools/filter.go
+++ b/proptools/filter.go
@@ -173,7 +173,13 @@ func filterPropertyStruct(prop reflect.Type, prefix string, maxNameSize int,
 		return nil, true
 	}
 
-	if !filtered {
+	// If the predicate selected all fields in the structure then it is generally better to reuse the
+	// original type as it avoids the footprint of creating another type. Also, if the original type
+	// is a named type then it will reduce the size of any structs the caller may create that include
+	// fields of this type. However, the original type should only be reused if it does not exceed
+	// maxNameSize. That is, of course, more likely for an anonymous type than a named one but this
+	// treats them the same.
+	if !filtered && (maxNameSize < 0 || len(prop.String()) < maxNameSize) {
 		if ptr {
 			return []reflect.Type{reflect.PtrTo(prop)}, false
 		}
diff --git a/proptools/filter_test.go b/proptools/filter_test.go
index 0ea04bb..1c27cb2 100644
--- a/proptools/filter_test.go
+++ b/proptools/filter_test.go
@@ -240,6 +240,12 @@ func TestFilterPropertyStruct(t *testing.T) {
 }
 
 func TestFilterPropertyStructSharded(t *testing.T) {
+	type KeepAllWithAReallyLongNameThatExceedsTheMaxNameSize struct {
+		A *string `keep:"true"`
+		B *string `keep:"true"`
+		C *string `keep:"true"`
+	}
+
 	tests := []struct {
 		name        string
 		maxNameSize int
@@ -266,6 +272,44 @@ func TestFilterPropertyStructSharded(t *testing.T) {
 			},
 			filtered: true,
 		},
+		{
+			name:        "anonymous where all match but still needs sharding",
+			maxNameSize: 20,
+			in: &struct {
+				A *string `keep:"true"`
+				B *string `keep:"true"`
+				C *string `keep:"true"`
+			}{},
+			out: []interface{}{
+				&struct {
+					A *string
+				}{},
+				&struct {
+					B *string
+				}{},
+				&struct {
+					C *string
+				}{},
+			},
+			filtered: true,
+		},
+		{
+			name:        "named where all match but still needs sharding",
+			maxNameSize: 20,
+			in:          &KeepAllWithAReallyLongNameThatExceedsTheMaxNameSize{},
+			out: []interface{}{
+				&struct {
+					A *string
+				}{},
+				&struct {
+					B *string
+				}{},
+				&struct {
+					C *string
+				}{},
+			},
+			filtered: true,
+		},
 	}
 
 	for _, test := range tests {