diff --git a/internal/server/jobs.go b/internal/server/jobs.go index 75b69f2..9f33d16 100644 --- a/internal/server/jobs.go +++ b/internal/server/jobs.go @@ -17,8 +17,8 @@ import ( // maxJobBody bounds a POST /jobs request body. const maxJobBody = 64 << 10 -// jobRequest is the POST /jobs body. Payload is optional; an omitted -// payload is the empty object. +// jobRequest is the POST /jobs body. Payload is required; a job kind that +// takes no input is sent the empty object {}. type jobRequest struct { Kind string `json:"kind"` Payload json.RawMessage `json:"payload"` @@ -55,8 +55,9 @@ func postJob(logger *slog.Logger, runner *jobs.Runner) http.HandlerFunc { } } -// decodeJobRequest reads a single JSON object with a non-empty kind, -// refusing unknown fields, a null payload and trailing data. +// decodeJobRequest reads a single JSON object with a non-empty kind and a +// payload, refusing unknown fields, an omitted or null payload and trailing +// data. func decodeJobRequest(body io.Reader) (jobRequest, error) { data, err := io.ReadAll(body) if err != nil { @@ -70,7 +71,7 @@ func decodeJobRequest(body io.Reader) (jobRequest, error) { dec := json.NewDecoder(bytes.NewReader(data)) dec.DisallowUnknownFields() if err := dec.Decode(&req); err != nil { - return jobRequest{}, errors.New("request body must be a JSON object with kind and an optional payload") + return jobRequest{}, errors.New("request body must be a JSON object with kind and payload") } if _, err := dec.Token(); err != io.EOF { return jobRequest{}, errors.New("request body must be a single JSON object") @@ -78,10 +79,10 @@ func decodeJobRequest(body io.Reader) (jobRequest, error) { if req.Kind == "" { return jobRequest{}, errors.New("kind: required") } - switch { - case req.Payload == nil: - req.Payload = json.RawMessage(`{}`) - case bytes.Equal(req.Payload, []byte("null")): + if req.Payload == nil { + return jobRequest{}, errors.New("payload: required") + } + if bytes.Equal(req.Payload, []byte("null")) { return jobRequest{}, errors.New("payload: must not be null") } return req, nil diff --git a/internal/server/jobs_test.go b/internal/server/jobs_test.go index e2f1879..7e8005b 100644 --- a/internal/server/jobs_test.go +++ b/internal/server/jobs_test.go @@ -183,24 +183,6 @@ func TestPostJobThenGetProgressAndResult(t *testing.T) { } } -func TestPostJobWithoutPayloadEnqueues(t *testing.T) { - t.Parallel() - release := make(chan struct{}) - close(release) - runner, _ := startedRunner(t, release, nil) - router := newTestRouterWith(t, "dev", newTestSettings(t), runner) - - rec, body := serve(t, router, signedIn(http.MethodPost, "/api/v1/jobs", strings.NewReader(`{"kind":"draft"}`))) - if rec.Code != http.StatusAccepted { - t.Fatalf("status = %d, want 202: %s", rec.Code, rec.Body) - } - id, _ := body["id"].(string) - done := pollJob(t, router, id, func(b map[string]any) bool { return b["status"] == "succeeded" }) - if resultDays(done) != 7 { - t.Errorf("finished body = %v, want result {\"days\":7}", done) - } -} - func TestPostJobRefusesBadRequests(t *testing.T) { t.Parallel() runner, _ := startedRunner(t, nil, nil) @@ -209,15 +191,16 @@ func TestPostJobRefusesBadRequests(t *testing.T) { for _, tc := range []struct { name, body, code, message string }{ - {"unknown kind", `{"kind":"no_such_kind"}`, "unknown_kind", "unknown job kind"}, + {"unknown kind", `{"kind":"no_such_kind","payload":{}}`, "unknown_kind", "unknown job kind"}, {"payload for another type", `{"kind":"draft","payload":{"days":3}}`, "invalid_payload", "payload does not match the job kind"}, {"payload not an object", `{"kind":"draft","payload":[1]}`, "invalid_payload", "payload does not match the job kind"}, + {"missing payload", `{"kind":"draft"}`, "invalid_request", "payload: required"}, {"null payload", `{"kind":"draft","payload":null}`, "invalid_request", "payload: must not be null"}, {"missing kind", `{"payload":{}}`, "invalid_request", "kind: required"}, - {"empty body", ``, "invalid_request", "request body must be a JSON object with kind and an optional payload"}, - {"not an object", `["draft"]`, "invalid_request", "request body must be a JSON object with kind and an optional payload"}, - {"unknown field", `{"kind":"draft","extra":1}`, "invalid_request", "request body must be a JSON object with kind and an optional payload"}, - {"trailing data", `{"kind":"draft"} {}`, "invalid_request", "request body must be a single JSON object"}, + {"empty body", ``, "invalid_request", "request body must be a JSON object with kind and payload"}, + {"not an object", `["draft"]`, "invalid_request", "request body must be a JSON object with kind and payload"}, + {"unknown field", `{"kind":"draft","payload":{},"extra":1}`, "invalid_request", "request body must be a JSON object with kind and payload"}, + {"trailing data", `{"kind":"draft","payload":{}} {}`, "invalid_request", "request body must be a single JSON object"}, } { t.Run(tc.name, func(t *testing.T) { t.Parallel() @@ -240,7 +223,7 @@ func TestPostJobRefusesBadRequests(t *testing.T) { func TestPostJobNeedsSessionAndCSRF(t *testing.T) { t.Parallel() router := newTestRouter(t, "dev") - const body = `{"kind":"noop"}` + const body = `{"kind":"noop","payload":{}}` rec := httptest.NewRecorder() router.ServeHTTP(rec, httptest.NewRequest(http.MethodPost, "/api/v1/jobs", strings.NewReader(body)))