Skip to content

Commit 7a50915

Browse files
committed
fix(sdk): address v3 Go SDK review findings
- disable setup-go caching in the SDK release workflow (zizmor cache-poisoning gate) - cover api/v3/client in make mod and make lint-go - fail fast on discriminated unions with non-model variants in typespec-go - add wire-level query serialization tests for filters and scalar params
1 parent 7da0f9f commit 7a50915

6 files changed

Lines changed: 74 additions & 5 deletions

File tree

.github/workflows/release-go-sdk.yaml

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,11 @@ jobs:
3939
with:
4040
persist-credentials: false
4141

42-
# The SDK module is pure Go (no cgo/librdkafka), so plain setup-go is
43-
# enough here; the Nix shell is not needed.
4442
- name: Set up Go
4543
uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0
4644
with:
4745
go-version-file: api/v3/client/go.mod
48-
cache-dependency-path: api/v3/client/go.sum
46+
cache: false
4947

5048
- name: Run Go SDK checks
5149
run: make test-go-sdk

Makefile

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,7 @@ package-helm-chart: ## Package a helm chart for release (set CHART and VERSION)
293293
lint-go: ## Lint Go code
294294
$(call print-target)
295295
golangci-lint run -v $(GO_LINT_PATH)
296+
cd api/v3/client && golangci-lint run -v ./...
296297
go vet -C e2e ./...
297298
cd e2e && golangci-lint run -v ./...
298299

@@ -327,6 +328,7 @@ mod: ## go mod tidy
327328
$(call print-target)
328329
go mod tidy
329330
go mod tidy -C collector
331+
go mod tidy -C api/v3/client
330332
go mod tidy -C e2e
331333

332334
.PHONY: seed

api/spec/packages/typespec-go/src/components/GoUnion.tsx

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,23 @@ export function GoUnion({ program, union, name, mode, doc }: GoUnionProps) {
3636
`typespec-go: union ${name} has multiple object variants but no discriminator; add @discriminated so Go accessors can select variants safely`,
3737
)
3838
}
39+
// The discriminated path assumes every selectable variant is a model: the
40+
// From* constructors envelope the payload under the discriminator property
41+
// and the As* accessors match on it. A scalar or enum variant has neither,
42+
// so it would emit constructors/accessors that cannot round-trip.
43+
if (discriminator) {
44+
const nonModelVariants = [...union.variants.values()].filter(
45+
(variant) =>
46+
variant.type.kind !== 'Model' && variant.type.kind !== 'Intrinsic',
47+
)
48+
if (nonModelVariants.length > 0) {
49+
throw new Error(
50+
`typespec-go: discriminated union ${name} mixes model and non-model variants (${nonModelVariants
51+
.map((variant) => variant.type.kind)
52+
.join(', ')}); only model variants can carry the discriminator`,
53+
)
54+
}
55+
}
3956
const variants = [...union.variants.values()].flatMap((variant) => {
4057
if (variant.type.kind === 'Intrinsic') {
4158
return []

api/v3/client/errors_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010
"strings"
1111
"testing"
1212

13-
"github.com/openmeterio/openmeter/api/v3/client"
13+
openmeter "github.com/openmeterio/openmeter/api/v3/client"
1414
)
1515

1616
func TestAPIErrorParsesRFC7807(t *testing.T) {

api/v3/client/query_test.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// Hand-written query-serialization tests for the generated OpenMeter Go SDK.
2+
// The generator's output cleaner preserves *_test.go files, so these survive
3+
// regeneration. The recorder harness lives in transport_test.go.
4+
package openmeter_test
5+
6+
import (
7+
"net/http"
8+
"testing"
9+
"time"
10+
11+
openmeter "github.com/openmeterio/openmeter/api/v3/client"
12+
)
13+
14+
func TestFilterQuerySerialization(t *testing.T) {
15+
t.Parallel()
16+
17+
rec := &requestRecorder{}
18+
om := newTestClient(t, rec.handler(http.StatusOK, emptyPageBody))
19+
20+
params := openmeter.MeterListParams{Filter: &openmeter.MeterFilter{
21+
Key: &openmeter.StringFilter{Oeq: []string{"tokens", "requests"}},
22+
Name: &openmeter.StringFilter{Contains: openmeter.String("gpt")},
23+
}}
24+
if _, err := om.Meters.List(t.Context(), params); err != nil {
25+
t.Fatalf("Meters.List: %v", err)
26+
}
27+
28+
q := rec.last(t).query
29+
if got := q.Get("filter[key][oeq]"); got != "tokens,requests" {
30+
t.Errorf("filter[key][oeq] = %q, want %q", got, "tokens,requests")
31+
}
32+
if got := q.Get("filter[name][contains]"); got != "gpt" {
33+
t.Errorf("filter[name][contains] = %q, want %q", got, "gpt")
34+
}
35+
}
36+
37+
func TestScalarQueryParamSerialization(t *testing.T) {
38+
t.Parallel()
39+
40+
rec := &requestRecorder{}
41+
om := newTestClient(t, rec.handler(http.StatusOK, "{}"))
42+
43+
timestamp := time.Date(2026, 5, 11, 10, 30, 0, 0, time.UTC)
44+
params := openmeter.GetCustomerCreditBalanceParams{Timestamp: openmeter.Time(timestamp)}
45+
if _, err := om.Customers.Credits.Balance.Get(t.Context(), "cus-1", params); err != nil {
46+
t.Fatalf("Customers.Credits.Balance.Get: %v", err)
47+
}
48+
49+
if got := rec.last(t).query.Get("timestamp"); got != "2026-05-11T10:30:00Z" {
50+
t.Errorf("timestamp = %q, want %q", got, "2026-05-11T10:30:00Z")
51+
}
52+
}

api/v3/client/transport_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import (
1414
"testing"
1515
"time"
1616

17-
"github.com/openmeterio/openmeter/api/v3/client"
17+
openmeter "github.com/openmeterio/openmeter/api/v3/client"
1818
)
1919

2020
const emptyPageBody = `{"data":[],"meta":{"page":{"number":1,"size":100,"total":0}}}`

0 commit comments

Comments
 (0)