fix: let callers send meaningful zero values on request fields - #58
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
v0.12.1fixedfollow_preference, a request field whose zero value carries meaning but which was generated as a plainboolwithomitempty— so Go droppedfalseand the server never saw it. A sweep for the same shape across the whole spec — join "server models the field as a pointer and branches on nil" against "SDK generates a value scalar withomitempty" — turned up five more, plus a property whose name never matched the server at all.Fixed
RUMApplicationUpdateRequest.IsPrivate/.NoGeo/.NoIPbool*boolRUMApplicationAlerting.Enabledbool*boolRUMFieldListRequest.IsFacetbool*boolScheduleNotify.AdvanceInTimeint64*int64TemplateCreateRequest/TemplateUpdateRequest/TemplateItemfeishu_app_card_table_enabledfeishu_app_card_v2_table_enabled/rum/application/updateapplies these three only when present, so they were one-way: an application could be made private, or have geo inference and IP collection switched off, and never switched back through the API.alertingobject isomitzero. A minimal{"enabled": false}override was entirely zero-valued, so the whole key was dropped and alerting stayed on unless the caller happened to re-send unrelated fields. A pointer keeps the container on the wire.is_facet. Absent means "every field",truemeans "facet fields",falsemeans "non-facet fields" — that last mode was unreachable. The description said the opposite and is corrected.advance_in_time.0means "notify exactly at the shift start"; absent disables advance notification entirely. Only the second was reachable.feishu_app_card_v2_table_enabled. The spec spelled it withoutv2_, and unknown JSON keys are silently discarded, so no value the SDK sent for this property ever reached the server, in either direction, on either create or update — and the response schema misnamed it too.Source fix lands in the OpenAPI spec (flashcatcloud/flashduty-docs#263);
openapi/is re-synced andmodels_gen.goregenerated. No hand-edits to generated code, and no generator changes were needed — the existing nullable-request-scalar rule produces the pointers.Compatibility
Source-breaking for callers that set any of the fields above. Wrap literals with the
flashduty.Bool/flashduty.Int64helpers, and renameFeishuAppCardTableEnabledtoFeishuAppCardV2TableEnabled:Two notes on blast radius:
RUMApplicationAlertingis one schema shared by the create request, the update request and the application response, soRUMApplicationItem.Alerting.Enabledalso becomes*bool. Read paths need a nil check. Splitting the schema to avoid this was rejected as more churn than the fix is worth.RUMApplicationCreateRequest's three privacy booleans stay non-nullable, because a create has no prior state for an absent key to preserve.Validation
make check— gofmt/gci clean, golangci-lint 0 issues,go test -race ./...green, build OK."is_private":false,"advance_in_time":0,"alerting":{"enabled":false}, …), and three asserting a nil pointer stays off the wire while sibling fields still serialise.