googleapis / googleapis/google-cloud-go

datastore: Put fails with "invalid Value type []datastore.Property" when a PropertyLoadSaver map field is nil

Open
#20,176 0 comments 0 reactions 1 assignee Claimed by @shollyman View on GitHub
api: datastore
Dominant language
Go
Stars
4.5k
Forks
1.6k
Avg merge
1d 13h
Merged PRs (30d)
109

Description

## Client
Datastore

## Environment
Any. Reproduced on macOS arm64 / go1.26 against both the Datastore emulator and unit-level proto conversion, on every release from datastore/v1.3.0 through current HEAD.

## Code

```go
// Custom map type implementing PropertyLoadSaver, since maps are not
// natively supported property values:
type CustomMap map[string]string

func (m *CustomMap) Save() ([]datastore.Property, error) {
b, err := json.Marshal(m)
if err != nil {
return nil, err
}
return []datastore.Property{{Name: "json", Value: b, NoIndex: true}}, nil
}

func (m *CustomMap) Load(props []datastore.Property) error {
for _, p := range props {
if p.Name == "json" {
return json.Unmarshal(p.Value.([]byte), m)
}
}
return nil
}

type MyEntity struct {
Name string `datastore:"name"`
MyMap CustomMap `datastore:"mymap"`
Count int `datastore:"count"`
}

// Works: MyMap is non-nil.
_, err := client.Put(ctx, key, &MyEntity{Name: "a", MyMap: CustomMap{"k": "v"}})

// Fails: MyMap is nil (the zero value of any entity that never initialized it).
_, err = client.Put(ctx, key, &MyEntity{Name: "a", Count: 2})
```

## Expected behavior
The nil map field is saved the same way the non-nil field is: as a nested entity built from the `Save()` output (or flattened, with the `flatten` tag).

## Actual behavior

```
datastore: invalid Value type []datastore.Property for a Property with Name "mymap"
```

## Root cause
Non-nil PropertyLoadSaver fields are handled by `plsFieldSave`, which wraps the `Save()` output correctly: `p.Value = &Entity{Properties: subProps}`. But `plsForSave` intentionally skips nil values of nil-able kinds, so a nil map falls through to the fallback PropertyLoadSaver block in `reflectFieldSave` (save.go), which stores the slice raw:

```go
val, err := pSaver.Save()
...
p.Value = val // []Property — not a valid Property value type
```

`[]Property` is not a legal `Property.Value` type, so `interfaceToProto` rejects it and every `Put`/`Mutate` through this path fails. The same block also ignores the `flatten` option, unlike `plsFieldSave`.

This block was added by #2794 to fix #2611 ("PropertyLoadSaver ignored with nil map types") — it correctly made `Save()` get called via the field's address, but stored the raw `[]Property` instead of wrapping it. Its unit test only asserted `SaveStruct` output (where the invalid value goes unnoticed), so #2611's scenario has kept failing since, just with a different error message.

## Workaround
Initialize the map field before saving.

I have a fix ready (wrap the `Save()` output in `&Entity{...}` and honor `flatten`, matching `plsFieldSave`) and will send a PR shortly.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.