[object Object]

← back to Cli Printing Press

fix(cli): bind typed upsert values in column-declaration order (#1018)

fef70ba1e2ccfe93fe2ab51e524dabd898b69cfe · 2026-05-10 21:53:15 -0700 · Trevin Chow

Sub-resource tables declared columns as (id, fk, data, synced_at) but
the upsert<X>Tx template hard-coded the bindings as (id, string(data),
time.Now(), <fk>...), so paginated syncs silently corrupted every row:
JSON blobs landed in the FK column, timestamps landed in data, and FK
strings landed in synced_at. SQLite accepted the mismatch because the
columns were all TEXT-compatible. Top-level gravity-2+ tables and
dependent-sync tables happened to align because their non-base columns
were appended after data/synced_at.

Replace the three hard-coded leading bindings + filtered loop with a
single loop that walks .Columns in order and emits the matching
expression per column (id, string(data), time.Now(), or
lookupFieldValue). The argument order now follows columnNames by
construction, so adding a column anywhere in the slice cannot
desynchronize the bindings again.

Add TestGenerateStoreSubResourceUpsertBindingOrder, which generates a
spec with a sub-resource and asserts the emitted upsert<X>Tx binds
domains_id between id and data, matching the SQL column list. The
existing TestUpsertBatch_Sets<X>ParentID gated on a literal parent_id
column name and therefore never exercised the buildSubResourceTable
output.

Closes #1014

Files touched

Diff

commit fef70ba1e2ccfe93fe2ab51e524dabd898b69cfe
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Sun May 10 21:53:15 2026 -0700

    fix(cli): bind typed upsert values in column-declaration order (#1018)
    
    Sub-resource tables declared columns as (id, fk, data, synced_at) but
    the upsert<X>Tx template hard-coded the bindings as (id, string(data),
    time.Now(), <fk>...), so paginated syncs silently corrupted every row:
    JSON blobs landed in the FK column, timestamps landed in data, and FK
    strings landed in synced_at. SQLite accepted the mismatch because the
    columns were all TEXT-compatible. Top-level gravity-2+ tables and
    dependent-sync tables happened to align because their non-base columns
    were appended after data/synced_at.
    
    Replace the three hard-coded leading bindings + filtered loop with a
    single loop that walks .Columns in order and emits the matching
    expression per column (id, string(data), time.Now(), or
    lookupFieldValue). The argument order now follows columnNames by
    construction, so adding a column anywhere in the slice cannot
    desynchronize the bindings again.
    
    Add TestGenerateStoreSubResourceUpsertBindingOrder, which generates a
    spec with a sub-resource and asserts the emitted upsert<X>Tx binds
    domains_id between id and data, matching the SQL column list. The
    existing TestUpsertBatch_Sets<X>ParentID gated on a literal parent_id
    column name and therefore never exercised the buildSubResourceTable
    output.
    
    Closes #1014
---
 internal/generator/generator_test.go       | 72 ++++++++++++++++++++++++++++++
 internal/generator/templates/store.go.tmpl |  7 ++-
 2 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go
index bf54a5e9..e526bf96 100644
--- a/internal/generator/generator_test.go
+++ b/internal/generator/generator_test.go
@@ -2561,6 +2561,78 @@ func TestGenerateStoreUpsertBatchDispatchesToTypedTable(t *testing.T) {
 	runGoCommand(t, outputDir, "test", "./internal/store")
 }
 
+// TestGenerateStoreSubResourceUpsertBindingOrder asserts that the typed
+// upsert for a sub-resource table binds its argument values in the same
+// order as the SQL column declarations. buildSubResourceTable puts the
+// FK column between id and data, so the bindings have to match —
+// otherwise JSON blobs land in the FK column, timestamps land in data,
+// and FK values land in synced_at, silently corrupting every row.
+func TestGenerateStoreSubResourceUpsertBindingOrder(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := &spec.APISpec{
+		Name:    "subres",
+		Version: "0.1.0",
+		BaseURL: "https://api.example.com",
+		Auth: spec.AuthConfig{
+			Type:    "api_key",
+			Header:  "Authorization",
+			Format:  "Bearer {token}",
+			EnvVars: []string{"SUBRES_API_KEY"},
+		},
+		Config: spec.ConfigSpec{
+			Format: "toml",
+			Path:   "~/.config/subres-pp-cli/config.toml",
+		},
+		Resources: map[string]spec.Resource{
+			"domains": {
+				Description: "Manage domains",
+				Endpoints: map[string]spec.Endpoint{
+					"list": {Method: "GET", Path: "/domains", Description: "List domains"},
+				},
+				SubResources: map[string]spec.Resource{
+					"verify": {
+						Description: "Verify a domain",
+						Endpoints: map[string]spec.Endpoint{
+							"get": {
+								Method:      "GET",
+								Path:        "/domains/{domainId}/verify",
+								Description: "Get verification status",
+								Params:      []spec.Param{{Name: "domainId", Type: "string", Required: true, Positional: true}},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), naming.CLI(apiSpec.Name))
+	gen := New(apiSpec, outputDir)
+	require.NoError(t, gen.Generate())
+
+	storeSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "store", "store.go"))
+	require.NoError(t, err)
+	src := string(storeSrc)
+
+	// buildSubResourceTable inserts the FK column between id and data, so
+	// the column declaration order is (id, domains_id, data, synced_at).
+	assert.Contains(t, src, "INSERT INTO verify (id, domains_id, data, synced_at)",
+		"sub-resource table should declare FK column between id and data")
+
+	// The argument bindings must follow that same order.
+	assert.Regexp(t,
+		`(?s)id,\s+lookupFieldValue\(obj, "domains_id"\),\s+string\(data\),\s+time\.Now\(\),`,
+		src,
+		"upsertVerifyTx binding order must match (id, domains_id, data, synced_at) column order")
+
+	// And the swapped order must be absent.
+	assert.NotRegexp(t,
+		`(?s)id,\s+string\(data\),\s+time\.Now\(\),\s+lookupFieldValue\(obj, "domains_id"\),`,
+		src,
+		"swapped (id, data, synced_at, fk) binding order must not be emitted")
+}
+
 func TestGenerateSimilarCommandUsesCompositeResourceKey(t *testing.T) {
 	t.Parallel()
 
diff --git a/internal/generator/templates/store.go.tmpl b/internal/generator/templates/store.go.tmpl
index 7793a51e..9d8efcc6 100644
--- a/internal/generator/templates/store.go.tmpl
+++ b/internal/generator/templates/store.go.tmpl
@@ -786,11 +786,14 @@ func (s *Store) upsert{{pascal .Name}}Tx(tx *sql.Tx, id string, obj map[string]a
 		`INSERT INTO {{safeName .Name}} ({{columnNames .Columns}})
 		 VALUES ({{columnPlaceholders .Columns}})
 		 ON CONFLICT(id) DO UPDATE SET {{updateSet .Columns}}`,
+{{- range .Columns}}
+{{- if eq .Name "id"}}
 		id,
+{{- else if eq .Name "data"}}
 		string(data),
+{{- else if eq .Name "synced_at"}}
 		time.Now(),
-{{- range .Columns}}
-{{- if and (ne .Name "id") (ne .Name "data") (ne .Name "synced_at")}}
+{{- else}}
 		lookupFieldValue(obj, "{{.Name}}"),
 {{- end}}
 {{- end}}

← 182395ca feat(cli): add cliutil.ExtractNumber/ExtractInt for JSON-str  ·  back to Cli Printing Press  ·  fix(cli): emit AccessToken-only AuthHeader for all OAuth2 gr 3b050899 →