diff --git a/bundle/config/resources/postgres_branch.go b/bundle/config/resources/postgres_branch.go index d6dd538febb..2713c0cdd58 100644 --- a/bundle/config/resources/postgres_branch.go +++ b/bundle/config/resources/postgres_branch.go @@ -42,7 +42,7 @@ func (c *PostgresBranchConfig) UnmarshalJSON(b []byte) error { return marshal.Unmarshal(b, c) } -func (c *PostgresBranchConfig) MarshalJSON() ([]byte, error) { +func (c PostgresBranchConfig) MarshalJSON() ([]byte, error) { return marshal.Marshal(c) } diff --git a/bundle/config/resources/postgres_branch_test.go b/bundle/config/resources/postgres_branch_test.go new file mode 100644 index 00000000000..03f87de8ac6 --- /dev/null +++ b/bundle/config/resources/postgres_branch_test.go @@ -0,0 +1,44 @@ +package resources + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestPostgresBranchConfigMarshalHonorsForceSendFields pins the reason +// PostgresBranchConfig.MarshalJSON is declared on a value receiver. +// +// PurgeOnDelete is omitempty, so its zero value is emitted only because +// ForceSendFields names it, and the SDK marshaler is what honors ForceSendFields. +// With a pointer-receiver MarshalJSON, only *T satisfies json.Marshaler, so +// marshalling a value falls back to plain encoding/json -- which ignores +// ForceSendFields (tagged json:"-") and would drop the field. +func TestPostgresBranchConfigMarshalHonorsForceSendFields(t *testing.T) { + c := PostgresBranchConfig{BranchId: "b1", Parent: "projects/p1"} + c.ForceSendFields = []string{"PurgeOnDelete"} + + b, err := json.Marshal(c) + require.NoError(t, err) + assert.JSONEq(t, `{"branch_id":"b1","parent":"projects/p1","purge_on_delete":false}`, string(b)) +} + +// TestPostgresBranchMarshalValueAndPointerAgree guards the invariant directly: +// a value-receiver MarshalJSON makes json.Marshal(x) and json.Marshal(&x) +// produce the same bytes. A pointer-only marshaler would send them down two +// different code paths. +func TestPostgresBranchMarshalValueAndPointerAgree(t *testing.T) { + b := PostgresBranch{} + b.ID = "the-id" + b.BranchId = "b1" + b.Parent = "projects/p1" + + byValue, err := json.Marshal(b) + require.NoError(t, err) + byPointer, err := json.Marshal(&b) + require.NoError(t, err) + assert.Equal(t, string(byPointer), string(byValue)) + assert.JSONEq(t, `{"id":"the-id","lifecycle":{},"branch_id":"b1","parent":"projects/p1"}`, string(byValue)) +} diff --git a/bundle/config/resources/postgres_catalog.go b/bundle/config/resources/postgres_catalog.go index 8295d70deec..d1c788aece6 100644 --- a/bundle/config/resources/postgres_catalog.go +++ b/bundle/config/resources/postgres_catalog.go @@ -23,7 +23,7 @@ func (c *PostgresCatalogConfig) UnmarshalJSON(b []byte) error { return marshal.Unmarshal(b, c) } -func (c *PostgresCatalogConfig) MarshalJSON() ([]byte, error) { +func (c PostgresCatalogConfig) MarshalJSON() ([]byte, error) { return marshal.Marshal(c) } diff --git a/bundle/config/resources/postgres_database.go b/bundle/config/resources/postgres_database.go index ef9e013bdaa..ad16d720bb9 100644 --- a/bundle/config/resources/postgres_database.go +++ b/bundle/config/resources/postgres_database.go @@ -31,7 +31,7 @@ func (c *PostgresDatabaseConfig) UnmarshalJSON(b []byte) error { return marshal.Unmarshal(b, c) } -func (c *PostgresDatabaseConfig) MarshalJSON() ([]byte, error) { +func (c PostgresDatabaseConfig) MarshalJSON() ([]byte, error) { return marshal.Marshal(c) } diff --git a/bundle/config/resources/postgres_endpoint.go b/bundle/config/resources/postgres_endpoint.go index 3427a382bab..1ae81fdf8ad 100644 --- a/bundle/config/resources/postgres_endpoint.go +++ b/bundle/config/resources/postgres_endpoint.go @@ -30,7 +30,7 @@ func (c *PostgresEndpointConfig) UnmarshalJSON(b []byte) error { return marshal.Unmarshal(b, c) } -func (c *PostgresEndpointConfig) MarshalJSON() ([]byte, error) { +func (c PostgresEndpointConfig) MarshalJSON() ([]byte, error) { return marshal.Marshal(c) } diff --git a/bundle/config/resources/postgres_project.go b/bundle/config/resources/postgres_project.go index c8ffdb01142..1f0165fd9b7 100644 --- a/bundle/config/resources/postgres_project.go +++ b/bundle/config/resources/postgres_project.go @@ -34,7 +34,7 @@ func (c *PostgresProjectConfig) UnmarshalJSON(b []byte) error { return marshal.Unmarshal(b, c) } -func (c *PostgresProjectConfig) MarshalJSON() ([]byte, error) { +func (c PostgresProjectConfig) MarshalJSON() ([]byte, error) { return marshal.Marshal(c) } diff --git a/bundle/config/resources/postgres_role.go b/bundle/config/resources/postgres_role.go index 6f39cba6f1a..b45c6928557 100644 --- a/bundle/config/resources/postgres_role.go +++ b/bundle/config/resources/postgres_role.go @@ -32,7 +32,7 @@ func (c *PostgresRoleConfig) UnmarshalJSON(b []byte) error { return marshal.Unmarshal(b, c) } -func (c *PostgresRoleConfig) MarshalJSON() ([]byte, error) { +func (c PostgresRoleConfig) MarshalJSON() ([]byte, error) { return marshal.Marshal(c) } diff --git a/bundle/config/resources/postgres_synced_table.go b/bundle/config/resources/postgres_synced_table.go index bd7dcd1a172..e92f19c2ab4 100644 --- a/bundle/config/resources/postgres_synced_table.go +++ b/bundle/config/resources/postgres_synced_table.go @@ -25,7 +25,7 @@ func (c *PostgresSyncedTableConfig) UnmarshalJSON(b []byte) error { return marshal.Unmarshal(b, c) } -func (c *PostgresSyncedTableConfig) MarshalJSON() ([]byte, error) { +func (c PostgresSyncedTableConfig) MarshalJSON() ([]byte, error) { return marshal.Marshal(c) } diff --git a/bundle/config/resources/secret.go b/bundle/config/resources/secret.go index 445b56fa1d3..d7fe4ed1a71 100644 --- a/bundle/config/resources/secret.go +++ b/bundle/config/resources/secret.go @@ -25,7 +25,7 @@ func (s *Secret) UnmarshalJSON(b []byte) error { return marshal.Unmarshal(b, s) } -func (s *Secret) MarshalJSON() ([]byte, error) { +func (s Secret) MarshalJSON() ([]byte, error) { return marshal.Marshal(s) } diff --git a/bundle/direct/dresources/serialize_test.go b/bundle/direct/dresources/serialize_test.go index b7f253f24c9..17b943a4e3e 100644 --- a/bundle/direct/dresources/serialize_test.go +++ b/bundle/direct/dresources/serialize_test.go @@ -148,6 +148,106 @@ func TestRoundtripAllFieldsInputConfigType(t *testing.T) { testRoundtripAllFields(t, "InputConfigType", (*Adapter).InputConfigType, []string{"cluster_policies"}) } +var jsonMarshalerType = reflect.TypeFor[json.Marshaler]() + +// hasPointerOnlyMarshaler reports whether *T marshals itself but T does not. +// +// A type with no marshaler at all is not a defect: encoding/json treats it the +// same by value and by pointer. Only the asymmetry is. Kind is not restricted: +// a named scalar, slice or map can declare MarshalJSON on a pointer receiver and +// diverges exactly the same way. +func hasPointerOnlyMarshaler(t reflect.Type) bool { + return reflect.PointerTo(t).Implements(jsonMarshalerType) && + !t.Implements(jsonMarshalerType) +} + +// collectPointerOnlyMarshalers records into found every type reachable from t +// whose MarshalJSON is declared on a pointer receiver only. +// +// Pointers are followed as edges rather than stripped up front, so `type L *L` +// terminates on seen rather than spinning in Elem(). +func collectPointerOnlyMarshalers(t reflect.Type, seen, found map[reflect.Type]bool) { + if seen[t] { + return + } + seen[t] = true + + if hasPointerOnlyMarshaler(t) { + found[t] = true + } + + switch t.Kind() { + case reflect.Pointer, reflect.Slice, reflect.Array, reflect.Map: + collectPointerOnlyMarshalers(t.Elem(), seen, found) + case reflect.Struct: + for field := range t.Fields() { + // json:"-" is never serialized, so a type reachable only through one + // cannot diverge. Skipped for the same reason as unexported fields. + if !field.IsExported() || structtag.JSONTag(field.Tag.Get("json")).Name() == "-" { + continue + } + collectPointerOnlyMarshalers(field.Type, seen, found) + } + default: + // Scalars hold no reachable named type, and an interface field's dynamic + // type is not knowable from the static type. + } +} + +// TestMarshalerValueReceiver asserts that no type reachable from an adapter +// surface declares MarshalJSON on a pointer receiver only. +// +// Such a method is satisfied by *T but not by T, and encoding/json reaches it +// only for an addressable value. json.Marshal(&x) then uses the marshaler while +// json.Marshal(x) silently falls back to plain struct-field encoding -- two code +// paths for one type. They disagree on more than key order, because encoding/json +// knows nothing about ForceSendFields (tagged json:"-"), so a force-sent zero +// value of an omitempty field survives one path and vanishes on the other. +// +// The walk covers embedded members and named fields, not just the surface type +// itself. Both matter, for different reasons: an embedded member's pointer +// receiver is promoted to *T only, which makes T itself asymmetric unless T +// declares its own marshaler -- and if it does, the member's asymmetry is hidden +// from a top-level check while still applying wherever that member is marshalled +// directly. A named field is stronger still: marshal's structAsMap stores every +// field into a map via .Interface(), and a map value is not addressable, so the +// pointer receiver is unreachable there. (Slice elements, by contrast, stay +// addressable and do reach it -- the walk covers them for the embedded-member +// reason, not this one.) +// +// The round-trip tests above cannot catch any of this: they build values with +// reflect.New, so they only ever marshal a pointer. +func TestMarshalerValueReceiver(t *testing.T) { + for resourceType, resource := range SupportedResources { + adapter, err := NewAdapter(resource, resourceType, nil) + require.NoError(t, err) + + t.Run(resourceType, func(t *testing.T) { + // Each subtest walks with its own maps. Sharing them across subtests + // would attribute a type to whichever one reached it first, and + // iteration order over SupportedResources is random. + seen := make(map[reflect.Type]bool) + found := make(map[reflect.Type]bool) + for _, typeOf := range []func(*Adapter) reflect.Type{ + (*Adapter).InputConfigType, + (*Adapter).StateType, + (*Adapter).RemoteType, + } { + collectPointerOnlyMarshalers(typeOf(adapter), seen, found) + } + + names := make([]string, 0, len(found)) + for typ := range found { + names = append(names, typ.String()) + } + slices.Sort(names) + require.Empty(t, names, + "reachable from %s: these types declare MarshalJSON on a pointer receiver only; give each a value receiver so marshalling by value and by pointer agree:\n %s", + resourceType, strings.Join(names, "\n ")) + }) + } +} + // fillNonZero recursively populates v with non-zero values so that every // serializable field is observable in a round-trip. It skips ForceSendFields // (json:"-") and bounds recursion depth to avoid runaway on self-referential