refactor(web): validate item writes via internal/itemwrite/
Phase 5c slice B. Three web write paths now pre-validate via the itemwrite package before calling store.Create / Update / Reparent. - handleDetailWrite: ValidateFormat + ValidateAgainstStore on (title, slug, status, parent_ids) before the store.Update call. - handleNewSubmit: same pair, scoped to a new item (no ID yet). - handleReparent: format + DB-aware checks; validator catches self-parent, unknown-parent, cycle. The existing "parent_ids required" guard stays as a separate fast-fail. - handleBulkApply: set_status pre-flight against the validator. Other bulk actions (add_tag / set_mgmt / set_public / timeline_todos) don't mutate validated fields so they pass through unchanged. On ValidationError the handler responds 400 + a human banner keyed on err.Kind via the new s.itemWriteFailure helper. itemWriteBannerCopy centralises the Kind→copy mapping so web/server.go and web/bulk.go share one phrasing. No web test source touched — all web/*_test.go assert on observable behaviour (HTTP status, response body) and the new validator path preserves both for valid AND invalid inputs the SQL trigger would have rejected anyway. Tests stay green unmodified. Task: t-projax-5c-itemwrite
This commit is contained in:
@@ -16,9 +16,47 @@ import (
|
||||
|
||||
"github.com/m/projax/internal/aggregate"
|
||||
"github.com/m/projax/internal/cache"
|
||||
"github.com/m/projax/internal/itemwrite"
|
||||
"github.com/m/projax/store"
|
||||
)
|
||||
|
||||
// itemWriteFailure surfaces an *itemwrite.ValidationError to the client.
|
||||
// HTTP code: 400 for invalid input. The body is a one-line human banner
|
||||
// keyed on Kind so handlers don't have to duplicate copy-table fragments.
|
||||
// Phase 5c uses this instead of the pre-existing raw-pgErr-on-failure
|
||||
// pattern in handleDetailWrite / handleNewSubmit / handleReparent.
|
||||
func (s *Server) itemWriteFailure(w http.ResponseWriter, r *http.Request, ve *itemwrite.ValidationError) {
|
||||
w.Header().Set("Content-Type", "text/plain; charset=utf-8")
|
||||
w.WriteHeader(http.StatusBadRequest)
|
||||
fmt.Fprintln(w, itemWriteBannerCopy(ve))
|
||||
s.Logger.Warn("itemwrite reject", "path", r.URL.Path, "kind", ve.Kind, "detail", ve.Detail)
|
||||
}
|
||||
|
||||
// itemWriteBannerCopy maps a ValidationError.Kind to the human-facing
|
||||
// banner copy. Centralised so web/server.go + web/bulk.go share one
|
||||
// authoritative phrasing.
|
||||
func itemWriteBannerCopy(ve *itemwrite.ValidationError) string {
|
||||
switch ve.Kind {
|
||||
case itemwrite.KindMissingRequired:
|
||||
return "Missing required field: " + ve.Detail
|
||||
case itemwrite.KindInvalidSlugFormat:
|
||||
return ve.Detail
|
||||
case itemwrite.KindInvalidStatus:
|
||||
return ve.Detail
|
||||
case itemwrite.KindSelfParent:
|
||||
return "An item cannot be its own parent."
|
||||
case itemwrite.KindUnknownParent:
|
||||
return ve.Detail
|
||||
case itemwrite.KindSlugCollision:
|
||||
return ve.Detail
|
||||
case itemwrite.KindCycle:
|
||||
return "Cannot reparent: this move would put the item in its own ancestor closure."
|
||||
case itemwrite.KindUnresolvablePath:
|
||||
return ve.Detail
|
||||
}
|
||||
return "Invalid input: " + ve.Detail
|
||||
}
|
||||
|
||||
// Register MIME types stdlib doesn't ship by default. The web-app manifest
|
||||
// spec requires application/manifest+json for the `<link rel=manifest>` →
|
||||
// without this Go's FileServer falls back to text/plain and Chrome refuses
|
||||
@@ -511,12 +549,27 @@ func (s *Server) handleDetailWrite(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
}
|
||||
parentIDs = dedupeStrings(parentIDs)
|
||||
title := strings.TrimSpace(r.FormValue("title"))
|
||||
slug := strings.TrimSpace(r.FormValue("slug"))
|
||||
status := strings.TrimSpace(r.FormValue("status"))
|
||||
if ve := itemwrite.ValidateFormat(itemwrite.Input{
|
||||
ID: it.ID, Title: title, Slug: slug, Status: status, ParentIDs: parentIDs, Path: it.PrimaryPath(),
|
||||
}); ve != nil {
|
||||
s.itemWriteFailure(w, r, ve)
|
||||
return
|
||||
}
|
||||
if ve := itemwrite.ValidateAgainstStore(r.Context(), s.Store, itemwrite.Input{
|
||||
ID: it.ID, Title: title, Slug: slug, Status: status, ParentIDs: parentIDs, Path: it.PrimaryPath(),
|
||||
}); ve != nil {
|
||||
s.itemWriteFailure(w, r, ve)
|
||||
return
|
||||
}
|
||||
in := store.UpdateInput{
|
||||
Title: strings.TrimSpace(r.FormValue("title")),
|
||||
Slug: strings.TrimSpace(r.FormValue("slug")),
|
||||
Title: title,
|
||||
Slug: slug,
|
||||
ParentIDs: parentIDs,
|
||||
ContentMD: r.FormValue("content_md"),
|
||||
Status: strings.TrimSpace(r.FormValue("status")),
|
||||
Status: status,
|
||||
Pinned: r.FormValue("pinned") == "1",
|
||||
Archived: r.FormValue("archived") == "1",
|
||||
Tags: parseCSV(r.FormValue("tags")),
|
||||
@@ -567,6 +620,21 @@ func (s *Server) handleReparent(w http.ResponseWriter, r *http.Request, path str
|
||||
http.Error(w, "reparent: parent_ids required", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
// Reparent doesn't change title/slug/status, so the validator only
|
||||
// exercises rules around parent_ids: self-parent, unknown-parent,
|
||||
// cycle. Format check runs against the existing item's fields.
|
||||
if ve := itemwrite.ValidateFormat(itemwrite.Input{
|
||||
ID: it.ID, Title: it.Title, Slug: it.Slug, Status: it.Status, ParentIDs: parentIDs, Path: it.PrimaryPath(),
|
||||
}); ve != nil {
|
||||
s.itemWriteFailure(w, r, ve)
|
||||
return
|
||||
}
|
||||
if ve := itemwrite.ValidateAgainstStore(r.Context(), s.Store, itemwrite.Input{
|
||||
ID: it.ID, Title: it.Title, Slug: it.Slug, Status: it.Status, ParentIDs: parentIDs, Path: it.PrimaryPath(),
|
||||
}); ve != nil {
|
||||
s.itemWriteFailure(w, r, ve)
|
||||
return
|
||||
}
|
||||
moved, err := s.Store.Reparent(r.Context(), it.ID, parentIDs)
|
||||
if err != nil {
|
||||
s.fail(w, r, err)
|
||||
@@ -700,13 +768,30 @@ func (s *Server) handleNewSubmit(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
}
|
||||
parentIDs = dedupeStrings(parentIDs)
|
||||
title := strings.TrimSpace(r.FormValue("title"))
|
||||
slug := strings.TrimSpace(r.FormValue("slug"))
|
||||
status := strings.TrimSpace(r.FormValue("status"))
|
||||
// New items have no ID yet — pre-flight format checks (title/slug/status)
|
||||
// then DB-aware checks (parent existence + slug collision under parents).
|
||||
if ve := itemwrite.ValidateFormat(itemwrite.Input{
|
||||
Title: title, Slug: slug, Status: status, ParentIDs: parentIDs,
|
||||
}); ve != nil {
|
||||
s.itemWriteFailure(w, r, ve)
|
||||
return
|
||||
}
|
||||
if ve := itemwrite.ValidateAgainstStore(r.Context(), s.Store, itemwrite.Input{
|
||||
Title: title, Slug: slug, Status: status, ParentIDs: parentIDs,
|
||||
}); ve != nil {
|
||||
s.itemWriteFailure(w, r, ve)
|
||||
return
|
||||
}
|
||||
in := store.CreateInput{
|
||||
Kind: []string{kind},
|
||||
Title: strings.TrimSpace(r.FormValue("title")),
|
||||
Slug: strings.TrimSpace(r.FormValue("slug")),
|
||||
Title: title,
|
||||
Slug: slug,
|
||||
ParentIDs: parentIDs,
|
||||
ContentMD: r.FormValue("content_md"),
|
||||
Status: strings.TrimSpace(r.FormValue("status")),
|
||||
Status: status,
|
||||
Tags: parseCSV(r.FormValue("tags")),
|
||||
Management: parseCSV(r.FormValue("management")),
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user