Skip to content

Commit 4b07fce

Browse files
committed
admin: canonicalise GSI projection_type at validation (Codex P2)
validateGSI accepted case-insensitive projection values for SPA ergonomics, but only validated them — the original (possibly lowercase) string then flowed through the bridge to the adapter, where buildCreateTableProjection only matches exact uppercase. So "include" passed handler validation only to fail later at the adapter as ValidationException. Codex P2 flagged this as a boundary mismatch on PR #635. Fix: validateGSI now writes the uppercase form back to gsi.Projection.Type when the value is recognised. Every downstream consumer (the adapter bridge in main_admin.go and the AdminForward server's handleCreate) sees the canonical shape. Tests: round-trip "include" through the handler and confirm src.lastCreateInput.GSI[0].Projection.Type is "INCLUDE"; sweep the five mixed-case variants ALL/All/all + KEYS_ONLY/keys_only + Include to confirm 201 in every case.
1 parent 905d231 commit 4b07fce

2 files changed

Lines changed: 74 additions & 1 deletion

File tree

internal/admin/dynamo_handler.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -444,8 +444,20 @@ func validateGSI(gsi *CreateTableGSI, index int) error {
444444
return err
445445
}
446446
}
447-
switch strings.TrimSpace(strings.ToUpper(gsi.Projection.Type)) {
447+
// Canonicalise the projection type in-place. The handler
448+
// accepts case-insensitive input ("include" / "ALL") for SPA
449+
// ergonomics, but the adapter's buildCreateTableProjection
450+
// only matches exact uppercase. Normalising once at the
451+
// boundary keeps that mismatch from surfacing as a confusing
452+
// post-validation 500 — the bridge and the AdminForward server
453+
// both forward whatever ends up in this field, so writing back
454+
// the canonical form here means every downstream consumer sees
455+
// the same shape. The empty string keeps its meaning ("default
456+
// to ALL") on both sides.
457+
canonical := strings.TrimSpace(strings.ToUpper(gsi.Projection.Type))
458+
switch canonical {
448459
case "", "ALL", "KEYS_ONLY", "INCLUDE":
460+
gsi.Projection.Type = canonical
449461
return nil
450462
default:
451463
return errors.New(prefix + `.projection.type must be one of "ALL", "KEYS_ONLY", "INCLUDE"`)

internal/admin/dynamo_handler_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,67 @@ func TestDynamoHandler_CreateTable_HappyPath(t *testing.T) {
398398
require.Equal(t, "id", got.PartitionKey)
399399
}
400400

401+
// TestDynamoHandler_CreateTable_CanonicalisesProjectionType makes
402+
// sure validateGSI normalises a lowercase "include" value back to
403+
// the uppercase form the adapter expects. Without normalisation
404+
// the request would pass handler validation only to fail at the
405+
// adapter as ValidationException ("invalid projection") — exactly
406+
// the boundary mismatch Codex P2 flagged on PR #635.
407+
func TestDynamoHandler_CreateTable_CanonicalisesProjectionType(t *testing.T) {
408+
src := &stubTablesSource{tables: map[string]*DynamoTableSummary{}}
409+
h := newDynamoHandlerForTest(src)
410+
body := `{
411+
"table_name":"t",
412+
"partition_key":{"name":"id","type":"S"},
413+
"gsi":[{
414+
"name":"by-status",
415+
"partition_key":{"name":"status","type":"S"},
416+
"projection":{"type":"include","non_key_attributes":["author"]}
417+
}]
418+
}`
419+
req := httptest.NewRequest(http.MethodPost, pathDynamoTables, strings.NewReader(body))
420+
req = withWritePrincipal(req)
421+
rec := httptest.NewRecorder()
422+
h.ServeHTTP(rec, req)
423+
require.Equal(t, http.StatusCreated, rec.Code, rec.Body.String())
424+
require.Len(t, src.lastCreateInput.GSI, 1)
425+
require.Equal(t, "INCLUDE", src.lastCreateInput.GSI[0].Projection.Type,
426+
"projection type must reach the source canonicalised so the adapter does not re-reject it")
427+
}
428+
429+
// TestDynamoHandler_CreateTable_AcceptsMixedCaseProjection covers
430+
// the same canonicalisation for KEYS_ONLY / Keys_Only / keys_only.
431+
func TestDynamoHandler_CreateTable_AcceptsMixedCaseProjection(t *testing.T) {
432+
cases := []struct{ in, want string }{
433+
{"all", "ALL"},
434+
{"All", "ALL"},
435+
{"KEYS_ONLY", "KEYS_ONLY"},
436+
{"keys_only", "KEYS_ONLY"},
437+
{"Include", "INCLUDE"},
438+
}
439+
for _, tc := range cases {
440+
t.Run(tc.in, func(t *testing.T) {
441+
src := &stubTablesSource{tables: map[string]*DynamoTableSummary{}}
442+
h := newDynamoHandlerForTest(src)
443+
body := `{
444+
"table_name":"t",
445+
"partition_key":{"name":"id","type":"S"},
446+
"gsi":[{
447+
"name":"i",
448+
"partition_key":{"name":"k","type":"S"},
449+
"projection":{"type":"` + tc.in + `"}
450+
}]
451+
}`
452+
req := httptest.NewRequest(http.MethodPost, pathDynamoTables, strings.NewReader(body))
453+
req = withWritePrincipal(req)
454+
rec := httptest.NewRecorder()
455+
h.ServeHTTP(rec, req)
456+
require.Equal(t, http.StatusCreated, rec.Code, rec.Body.String())
457+
require.Equal(t, tc.want, src.lastCreateInput.GSI[0].Projection.Type)
458+
})
459+
}
460+
}
461+
401462
func TestDynamoHandler_CreateTable_RejectsReadOnlyPrincipal(t *testing.T) {
402463
src := &stubTablesSource{tables: map[string]*DynamoTableSummary{}}
403464
h := newDynamoHandlerForTest(src)

0 commit comments

Comments
 (0)