Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: allow ForceSendFields to work for map types #2670

Merged
merged 4 commits into from
Jul 8, 2024
Merged

fix: allow ForceSendFields to work for map types #2670

merged 4 commits into from
Jul 8, 2024

Conversation

codyoss
Copy link
Member

@codyoss codyoss commented Jul 3, 2024 •
edited
Loading

The way we generate map types today is map[string]T, where T is a non-pointer type. This means the pointer receiver version of MarshalJSON does not fulfill the Marshaler interface. This is turn means that MarshalJSON is not called in these cases so ForceSendFields overrides do not work. To fix this we can switch the impl to a value receiver which fulfills both value and pointer types. This is not a breaking change for the API, but there is a slight behaviour change if someone was calling MarshalJSON directly. Nil *T's that call MarshalJSON directly will now panic instead of returning JSON null. This seems like unlikely usage and worth breaking to fix a bug in common usage.

Internal Bug: 349580049

@codyoss
fix: allow ForceSendFields to work for map types
b6be984 
The way we generate map types today is map[string]T, where T is a
non-pointer type. This means the value receiver version of
MarshalJSON does not fullfill the Marshaler interface. This is turn
means that MarshalJSON is not called in these cases so ForceSendFields
overrides do not work. To fix this we can switch the impl to a value
reciever which fullfills both value and pointer types. This is not a
breaking change for the API, but there is a slight behaviour change
if someone was calling MarshalJSON directly. Nil *T's that call
MarshalJSON directly will now panic instead of returning JSON `null`.
This seems like unlikely usage and worth breaking to fix a bug in
common usage.
@codyoss codyoss requested a review from a team as a code owner July 3, 2024 18:49
@codyoss codyoss requested a review from shollyman July 3, 2024 19:06
@diviner524
Copy link

This means the value receiver version of MarshalJSON does not fulfill the Marshaler interface.

nit: The description looks to be a bit misleading. I think you mean the pointer receiver version of MarshalJSON.

@diviner524
Copy link

/lgtm

Thanks for the quick fix!

@codyoss codyoss added the automerge Merge the pull request once unit tests and other checks pass. label Jul 8, 2024
@gcf-merge-on-green gcf-merge-on-green bot merged commit 40b5113 into googleapis:main Jul 8, 2024
5 checks passed
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Jul 8, 2024
@release-please release-please bot mentioned this pull request Jul 8, 2024
Merged
@codyoss codyoss deleted the json-marshall branch July 8, 2024 19:58
gcf-merge-on-green bot pushed a commit that referenced this pull request Jul 9, 2024
@release-please
chore(main): release 0.188.0 (#2666)
9957790 
🤖 I have created a release *beep* *boop*
---


## [0.188.0](https://togithub.com/googleapis/google-api-go-client/compare/v0.187.0...v0.188.0) (2024-07-09)


### Features

* **all:** Auto-regenerate discovery clients ([#2665](https://togithub.com/googleapis/google-api-go-client/issues/2665)) ([e84fa65](https://togithub.com/googleapis/google-api-go-client/commit/e84fa6508ebc498c3435668c48001185fbc9ce83))
* **all:** Auto-regenerate discovery clients ([#2669](https://togithub.com/googleapis/google-api-go-client/issues/2669)) ([6df3749](https://togithub.com/googleapis/google-api-go-client/commit/6df37492965b6323c6bcefa2a1ccd192b92981b4))
* **all:** Auto-regenerate discovery clients ([#2671](https://togithub.com/googleapis/google-api-go-client/issues/2671)) ([0d54a85](https://togithub.com/googleapis/google-api-go-client/commit/0d54a8540060cc79f830892fdd1fba46d73034c1))
* **all:** Auto-regenerate discovery clients ([#2673](https://togithub.com/googleapis/google-api-go-client/issues/2673)) ([88240e3](https://togithub.com/googleapis/google-api-go-client/commit/88240e3d98f3e944398c50379372eb071ebac0a2))
* **all:** Auto-regenerate discovery clients ([#2674](https://togithub.com/googleapis/google-api-go-client/issues/2674)) ([d465cec](https://togithub.com/googleapis/google-api-go-client/commit/d465cec68dbc2616c665e6ea240cd1e32c01418d))
* **all:** Auto-regenerate discovery clients ([#2675](https://togithub.com/googleapis/google-api-go-client/issues/2675)) ([a9177bd](https://togithub.com/googleapis/google-api-go-client/commit/a9177bdbdbba60c86b22bda4a7061c31d3485e4a))
* **all:** Auto-regenerate discovery clients ([#2677](https://togithub.com/googleapis/google-api-go-client/issues/2677)) ([5dd2fb2](https://togithub.com/googleapis/google-api-go-client/commit/5dd2fb237802349250c97c0ebdbb54cbd088884d))
* **all:** Auto-regenerate discovery clients ([#2678](https://togithub.com/googleapis/google-api-go-client/issues/2678)) ([d17f6be](https://togithub.com/googleapis/google-api-go-client/commit/d17f6beb5a531910b563a4383acaa383dbd3ee43))


### Bug Fixes

* Allow ForceSendFields to work for map types ([#2670](https://togithub.com/googleapis/google-api-go-client/issues/2670)) ([40b5113](https://togithub.com/googleapis/google-api-go-client/commit/40b5113127c4d66d533df16fe201898855c7c0cc))
* Check []bytes > 0 instead of nil ([#2667](https://togithub.com/googleapis/google-api-go-client/issues/2667)) ([711eb91](https://togithub.com/googleapis/google-api-go-client/commit/711eb913fe455ffe5c9d717b44762801820c0e8c)), refs [#2647](https://togithub.com/googleapis/google-api-go-client/issues/2647)

---
This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
abs3ntdev pushed a commit to abs3ntdev/gspot that referenced this pull request Jul 10, 2024
fix(deps): update module google.golang.org/api to v0.188.0 (#17)
cceb5fb 
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [google.golang.org/api](https://github.com/googleapis/google-api-go-client) | require | minor | `v0.187.0` -> `v0.188.0` |

---

### Release Notes

<details>
<summary>googleapis/google-api-go-client (google.golang.org/api)</summary>

### [`v0.188.0`](https://github.com/googleapis/google-api-go-client/releases/tag/v0.188.0)

[Compare Source](googleapis/google-api-go-client@v0.187.0...v0.188.0)

##### Features

-   **all:** Auto-regenerate discovery clients ([#&#8203;2665](googleapis/google-api-go-client#2665)) ([e84fa65](googleapis/google-api-go-client@e84fa65))
-   **all:** Auto-regenerate discovery clients ([#&#8203;2669](googleapis/google-api-go-client#2669)) ([6df3749](googleapis/google-api-go-client@6df3749))
-   **all:** Auto-regenerate discovery clients ([#&#8203;2671](googleapis/google-api-go-client#2671)) ([0d54a85](googleapis/google-api-go-client@0d54a85))
-   **all:** Auto-regenerate discovery clients ([#&#8203;2673](googleapis/google-api-go-client#2673)) ([88240e3](googleapis/google-api-go-client@88240e3))
-   **all:** Auto-regenerate discovery clients ([#&#8203;2674](googleapis/google-api-go-client#2674)) ([d465cec](googleapis/google-api-go-client@d465cec))
-   **all:** Auto-regenerate discovery clients ([#&#8203;2675](googleapis/google-api-go-client#2675)) ([a9177bd](googleapis/google-api-go-client@a9177bd))
-   **all:** Auto-regenerate discovery clients ([#&#8203;2677](googleapis/google-api-go-client#2677)) ([5dd2fb2](googleapis/google-api-go-client@5dd2fb2))
-   **all:** Auto-regenerate discovery clients ([#&#8203;2678](googleapis/google-api-go-client#2678)) ([d17f6be](googleapis/google-api-go-client@d17f6be))

##### Bug Fixes

-   Allow ForceSendFields to work for map types ([#&#8203;2670](googleapis/google-api-go-client#2670)) ([40b5113](googleapis/google-api-go-client@40b5113))
-   Check \[]bytes > 0 instead of nil ([#&#8203;2667](googleapis/google-api-go-client#2667)) ([711eb91](googleapis/google-api-go-client@711eb91)), refs [#&#8203;2647](googleapis/google-api-go-client#2647)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40MjYuMiIsInVwZGF0ZWRJblZlciI6IjM3LjQyNi4yIiwidGFyZ2V0QnJhbmNoIjoibWFzdGVyIiwibGFiZWxzIjpbXX0=-->

Co-authored-by: Renovate Bot <renovate-bot@gitea.com>
Reviewed-on: https://git.asdf.cafe/abs3nt/gspot/pulls/17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shollyman shollyman shollyman approved these changes

@tritone tritone tritone approved these changes

@noahdietz noahdietz noahdietz approved these changes

@yoshi-approver yoshi-approver Awaiting requested review from yoshi-approver yoshi-approver is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@codyoss @diviner524 @shollyman @tritone @noahdietz