From 18cebf1d5ca8889ae82f660c96fecc8bc5b73be5 Mon Sep 17 00:00:00 2001 From: Miles Yucht Date: Thu, 23 Jan 2025 13:09:44 +0100 Subject: [PATCH] [Fix] Do not send query parameters when set to zero value (#1136) ## What changes are proposed in this pull request? #1124 fixed the handling of APIs that use query parameters and a body simultaneously. After that change, query parameters are always sent, even if set to the zero value. This PR addresses this, using the same logic for query parameters as for fields in the body: if a field is set to its zero value, it will be included in the query parameter if it is present in ForceSendFields. ## How is this tested? This behavior is dependend upon in the Terraform provider. I'll be using it in https://github.com/databricks/terraform-provider-databricks/pull/4430 and verifying that there is no behavior change in the generated request. When using this PR in Terraform, tests asserting that the path doesn't include query parameters pass. --- service/apps/impl.go | 5 ++++- service/files/impl.go | 5 ++++- service/oauth2/impl.go | 17 +++++++++++++---- service/pkg.go | 4 ++-- service/sharing/impl.go | 9 +++++++-- 5 files changed, 30 insertions(+), 10 deletions(-) diff --git a/service/apps/impl.go b/service/apps/impl.go index 21121c39..b83aec39 100755 --- a/service/apps/impl.go +++ b/service/apps/impl.go @@ -8,6 +8,7 @@ import ( "net/http" "github.com/databricks/databricks-sdk-go/client" + "golang.org/x/exp/slices" ) // unexported type that holds implementations of just Apps API methods @@ -19,7 +20,9 @@ func (a *appsImpl) Create(ctx context.Context, request CreateAppRequest) (*App, var app App path := "/api/2.0/apps" queryParams := make(map[string]any) - queryParams["no_compute"] = request.NoCompute + if request.NoCompute != false || slices.Contains(request.ForceSendFields, "NoCompute") { + queryParams["no_compute"] = request.NoCompute + } headers := make(map[string]string) headers["Accept"] = "application/json" headers["Content-Type"] = "application/json" diff --git a/service/files/impl.go b/service/files/impl.go index 6e83c42c..e3f6930b 100755 --- a/service/files/impl.go +++ b/service/files/impl.go @@ -9,6 +9,7 @@ import ( "github.com/databricks/databricks-sdk-go/client" "github.com/databricks/databricks-sdk-go/httpclient" + "golang.org/x/exp/slices" ) // unexported type that holds implementations of just Dbfs API methods @@ -197,7 +198,9 @@ func (a *filesImpl) Upload(ctx context.Context, request UploadRequest) error { var uploadResponse UploadResponse path := fmt.Sprintf("/api/2.0/fs/files%v", httpclient.EncodeMultiSegmentPathParameter(request.FilePath)) queryParams := make(map[string]any) - queryParams["overwrite"] = request.Overwrite + if request.Overwrite != false || slices.Contains(request.ForceSendFields, "Overwrite") { + queryParams["overwrite"] = request.Overwrite + } headers := make(map[string]string) headers["Content-Type"] = "application/octet-stream" err := a.client.Do(ctx, http.MethodPut, path, headers, queryParams, request.Contents, &uploadResponse) diff --git a/service/oauth2/impl.go b/service/oauth2/impl.go index 484596f1..d8db890d 100755 --- a/service/oauth2/impl.go +++ b/service/oauth2/impl.go @@ -8,6 +8,7 @@ import ( "net/http" "github.com/databricks/databricks-sdk-go/client" + "golang.org/x/exp/slices" ) // unexported type that holds implementations of just AccountFederationPolicy API methods @@ -19,7 +20,9 @@ func (a *accountFederationPolicyImpl) Create(ctx context.Context, request Create var federationPolicy FederationPolicy path := fmt.Sprintf("/api/2.0/accounts/%v/federationPolicies", a.client.ConfiguredAccountID()) queryParams := make(map[string]any) - queryParams["policy_id"] = request.PolicyId + if request.PolicyId != "" || slices.Contains(request.ForceSendFields, "PolicyId") { + queryParams["policy_id"] = request.PolicyId + } headers := make(map[string]string) headers["Accept"] = "application/json" headers["Content-Type"] = "application/json" @@ -61,7 +64,9 @@ func (a *accountFederationPolicyImpl) Update(ctx context.Context, request Update var federationPolicy FederationPolicy path := fmt.Sprintf("/api/2.0/accounts/%v/federationPolicies/%v", a.client.ConfiguredAccountID(), request.PolicyId) queryParams := make(map[string]any) - queryParams["update_mask"] = request.UpdateMask + if request.UpdateMask != "" || slices.Contains(request.ForceSendFields, "UpdateMask") { + queryParams["update_mask"] = request.UpdateMask + } headers := make(map[string]string) headers["Accept"] = "application/json" headers["Content-Type"] = "application/json" @@ -207,7 +212,9 @@ func (a *servicePrincipalFederationPolicyImpl) Create(ctx context.Context, reque var federationPolicy FederationPolicy path := fmt.Sprintf("/api/2.0/accounts/%v/servicePrincipals/%v/federationPolicies", a.client.ConfiguredAccountID(), request.ServicePrincipalId) queryParams := make(map[string]any) - queryParams["policy_id"] = request.PolicyId + if request.PolicyId != "" || slices.Contains(request.ForceSendFields, "PolicyId") { + queryParams["policy_id"] = request.PolicyId + } headers := make(map[string]string) headers["Accept"] = "application/json" headers["Content-Type"] = "application/json" @@ -249,7 +256,9 @@ func (a *servicePrincipalFederationPolicyImpl) Update(ctx context.Context, reque var federationPolicy FederationPolicy path := fmt.Sprintf("/api/2.0/accounts/%v/servicePrincipals/%v/federationPolicies/%v", a.client.ConfiguredAccountID(), request.ServicePrincipalId, request.PolicyId) queryParams := make(map[string]any) - queryParams["update_mask"] = request.UpdateMask + if request.UpdateMask != "" || slices.Contains(request.ForceSendFields, "UpdateMask") { + queryParams["update_mask"] = request.UpdateMask + } headers := make(map[string]string) headers["Accept"] = "application/json" headers["Content-Type"] = "application/json" diff --git a/service/pkg.go b/service/pkg.go index a1e01556..f26f56ed 100644 --- a/service/pkg.go +++ b/service/pkg.go @@ -50,10 +50,10 @@ // // - [marketplace.ConsumerProvidersAPI]: Providers are the entities that publish listings to the Marketplace. // -// - [catalog.CredentialsAPI]: A credential represents an authentication and authorization mechanism for accessing services on your cloud tenant. -// // - [provisioning.CredentialsAPI]: These APIs manage credential configurations for this workspace. // +// - [catalog.CredentialsAPI]: A credential represents an authentication and authorization mechanism for accessing services on your cloud tenant. +// // - [settings.CredentialsManagerAPI]: Credentials manager interacts with with Identity Providers to to perform token exchanges using stored credentials and refresh tokens. // // - [settings.CspEnablementAccountAPI]: The compliance security profile settings at the account level control whether to enable it for new workspaces. diff --git a/service/sharing/impl.go b/service/sharing/impl.go index da8223ee..e6b6d33e 100755 --- a/service/sharing/impl.go +++ b/service/sharing/impl.go @@ -8,6 +8,7 @@ import ( "net/http" "github.com/databricks/databricks-sdk-go/client" + "golang.org/x/exp/slices" "github.com/databricks/databricks-sdk-go/service/catalog" ) @@ -253,8 +254,12 @@ func (a *sharesImpl) UpdatePermissions(ctx context.Context, request UpdateShareP var updatePermissionsResponse UpdatePermissionsResponse path := fmt.Sprintf("/api/2.1/unity-catalog/shares/%v/permissions", request.Name) queryParams := make(map[string]any) - queryParams["max_results"] = request.MaxResults - queryParams["page_token"] = request.PageToken + if request.MaxResults != 0 || slices.Contains(request.ForceSendFields, "MaxResults") { + queryParams["max_results"] = request.MaxResults + } + if request.PageToken != "" || slices.Contains(request.ForceSendFields, "PageToken") { + queryParams["page_token"] = request.PageToken + } headers := make(map[string]string) headers["Accept"] = "application/json" headers["Content-Type"] = "application/json"