Skip to content

Commit 1c14fc1

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 3b0d05d commit 1c14fc1

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
@@ -438,8 +438,20 @@ func validateGSI(gsi *CreateTableGSI, index int) error {
438438
return err
439439
}
440440
}
441-
switch strings.TrimSpace(strings.ToUpper(gsi.Projection.Type)) {
441+
// Canonicalise the projection type in-place. The handler
442+
// accepts case-insensitive input ("include" / "ALL") for SPA
443+
// ergonomics, but the adapter's buildCreateTableProjection
444+
// only matches exact uppercase. Normalising once at the
445+
// boundary keeps that mismatch from surfacing as a confusing
446+
// post-validation 500 — the bridge and the AdminForward server
447+
// both forward whatever ends up in this field, so writing back
448+
// the canonical form here means every downstream consumer sees
449+
// the same shape. The empty string keeps its meaning ("default
450+
// to ALL") on both sides.
451+
canonical := strings.TrimSpace(strings.ToUpper(gsi.Projection.Type))
452+
switch canonical {
442453
case "", "ALL", "KEYS_ONLY", "INCLUDE":
454+
gsi.Projection.Type = canonical
443455
return nil
444456
default:
445457
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)