googleapis / google-api-go-client

Auto-generated Google APIs for Go.
https://pkg.go.dev/google.golang.org/api
BSD 3-Clause "New" or "Revised" License
3.99k stars 1.12k forks source link

Allow skipping nil bytes for WithCredentialsJSON #2647

Closed lexcao closed 3 months ago

lexcao commented 3 months ago

Background

I added a small feature to a repository https://github.com/authzed/spicedb/pull/1942 that supports passing WithCredentialsJSON to Spanner options. Since this is just library code, it simply forwards the option directly.

I discovered that WithCredentialsJSON doesn't work correctly with nil slice, as converting them to an empty slice results in invalid JSON. To address this issue, I submitted another PR: https://github.com/authzed/spicedb/pull/1948.

Problem

I believe the issue lies in how the WithCredentialsJSON function handles a nil slice within the option pattern, treating it as an empty slice.

For user code, I agree with that use case; we call the option as needed.

However, for library code, it's more ergonomic to forward options directly without needing to consider how to pass them in such cases.

Solution

We can skip nil slice for the value. This way the library code can simply forward the option directly.

Additional context

It seems there aren't enough use cases for handling slices in the option pattern. I am looking for other libraries that handle this case, but I have only found one.

codyoss commented 3 months ago

Although I think this would fix the issue I think the bigger problem is under the hood there are a couple of spots that are checking != nil instead of len() > 0. I will send out a patch for this.