Skip to content

Commit 783e28c

Browse files
hfl0506hfl0506
committed
fix(serve): preserve Allow header on 405 API responses
1 parent 67c2334 commit 783e28c

5 files changed

Lines changed: 112 additions & 30 deletions

File tree

‎api/openapi.yaml‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,8 +373,52 @@ components:
373373
enum: [bar, line, scatter, pie, heatmap, radar]
374374
configs:
375375
type: array
376+
description: Each chart type may appear at most once.
376377
items:
377378
$ref: '#/components/schemas/ChartConfig'
379+
allOf:
380+
- contains:
381+
type: object
382+
required: [type]
383+
properties:
384+
type: { const: bar }
385+
minContains: 0
386+
maxContains: 1
387+
- contains:
388+
type: object
389+
required: [type]
390+
properties:
391+
type: { const: line }
392+
minContains: 0
393+
maxContains: 1
394+
- contains:
395+
type: object
396+
required: [type]
397+
properties:
398+
type: { const: scatter }
399+
minContains: 0
400+
maxContains: 1
401+
- contains:
402+
type: object
403+
required: [type]
404+
properties:
405+
type: { const: pie }
406+
minContains: 0
407+
maxContains: 1
408+
- contains:
409+
type: object
410+
required: [type]
411+
properties:
412+
type: { const: heatmap }
413+
minContains: 0
414+
maxContains: 1
415+
- contains:
416+
type: object
417+
required: [type]
418+
properties:
419+
type: { const: radar }
420+
minContains: 0
421+
maxContains: 1
378422
StatisticsOptions:
379423
type: object
380424
additionalProperties: false

‎api/openapi_test.go‎

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -168,23 +168,32 @@ func (s *OpenAPISuite) TestDatasetSettingsRequireUniqueChartTypes() {
168168
contract := readContract(s.T())
169169
components := mustMap(s.T(), contract["components"], "components")
170170
schemas := mustMap(s.T(), components["schemas"], "components.schemas")
171-
datasetSchema := schemas["Dataset"]
172-
dataset := map[string]any{
173-
"name": "Bench",
174-
"axes": []any{},
175-
"settings": []any{
176-
map[string]any{"type": "bar"},
177-
map[string]any{"type": "line"},
171+
for _, location := range []struct {
172+
name string
173+
schema any
174+
value func(configs []any) map[string]any
175+
}{
176+
{
177+
name: "Dataset settings",
178+
schema: schemas["Dataset"],
179+
value: func(configs []any) map[string]any {
180+
return map[string]any{"name": "Bench", "axes": []any{}, "settings": configs, "data": []any{}}
181+
},
178182
},
179-
"data": []any{},
180-
}
181-
s.NoError(validateSchema(contract, datasetSchema, dataset, "dataset"))
182-
183-
dataset["settings"] = []any{
184-
map[string]any{"type": "bar"},
185-
map[string]any{"type": "bar", "showLabels": true},
183+
{
184+
name: "ChartSelection configs",
185+
schema: schemas["ChartSelection"],
186+
value: func(configs []any) map[string]any { return map[string]any{"configs": configs} },
187+
},
188+
} {
189+
for _, chartType := range []string{"bar", "line", "scatter", "pie", "heatmap", "radar"} {
190+
s.Run(location.name+" "+chartType, func() {
191+
configs := []any{map[string]any{"type": chartType}, map[string]any{"type": chartType}}
192+
err := validateSchema(contract, location.schema, location.value(configs), location.name)
193+
s.ErrorContains(err, "want at most 1")
194+
})
195+
}
186196
}
187-
s.ErrorContains(validateSchema(contract, datasetSchema, dataset, "dataset"), "want at most 1")
188197
}
189198

190199
func TestOpenAPISuite(t *testing.T) {

‎cmd/serve.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,15 @@ func composeRESTRoutes(handlers restHandlers) *http.ServeMux {
8787
mux.Handle("POST /{$}", handlers.convert)
8888
mux.Handle("POST /merge", handlers.merge)
8989
mux.Handle("POST /ui", handlers.ui)
90+
mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
91+
switch r.URL.Path {
92+
case "/", "/merge", "/ui":
93+
w.Header().Set("Allow", http.MethodPost)
94+
writeAPIProblem(w, r, http.StatusMethodNotAllowed, "Method not allowed", "Only POST is supported for this endpoint.")
95+
default:
96+
writeAPIProblem(w, r, http.StatusNotFound, "Not found", "The requested endpoint does not exist.")
97+
}
98+
})
9099
return mux
91100
}
92101

@@ -148,6 +157,7 @@ func runServer(ctx context.Context, opts serveOptions, deps serveDependencies) e
148157
shutdown = (*http.Server).Shutdown
149158
}
150159
if err := shutdown(server, shutdownCtx); err != nil {
160+
_ = server.Close()
151161
return fmt.Errorf("shutdown HTTP server: %w", err)
152162
}
153163
if err := <-serveResult; err != nil && !errors.Is(err, http.ErrServerClosed) {

‎cmd/serve_test.go‎

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"sync"
1414
"sync/atomic"
1515
"testing"
16+
"time"
1617

1718
"github.com/goptics/vizb/cmd/cli"
1819
internalcharts "github.com/goptics/vizb/internal/charts"
@@ -84,13 +85,20 @@ func (s *ServeSuite) TestRoutesRegisterOnlyTheThreePOSTOperations() {
8485
s.True(called[test.name])
8586
}
8687

87-
for _, request := range []*http.Request{
88-
httptest.NewRequest(http.MethodGet, "/", nil),
89-
httptest.NewRequest(http.MethodPost, "/unknown", nil),
88+
for _, test := range []struct {
89+
request *http.Request
90+
status int
91+
allow string
92+
}{
93+
{request: httptest.NewRequest(http.MethodGet, "/", nil), status: http.StatusMethodNotAllowed, allow: http.MethodPost},
94+
{request: httptest.NewRequest(http.MethodPost, "/unknown", nil), status: http.StatusNotFound},
9095
} {
9196
recorder := httptest.NewRecorder()
92-
routes.ServeHTTP(recorder, request)
93-
s.Contains([]int{http.StatusMethodNotAllowed, http.StatusNotFound}, recorder.Code)
97+
routes.ServeHTTP(recorder, test.request)
98+
s.Equal(test.status, recorder.Code)
99+
s.Equal(test.allow, recorder.Header().Get("Allow"))
100+
s.Contains(recorder.Header().Get("Content-Type"), "application/problem+json")
101+
s.Equal(float64(test.status), s.problemStatus(recorder))
94102
}
95103
}
96104

@@ -153,9 +161,9 @@ func (s *ServeSuite) TestCancellationShutsDownInMemoryListener() {
153161
listen: func(string, string) (net.Listener, error) { return listener, nil },
154162
})
155163
}()
156-
<-listener.acceptStarted
164+
receiveWithin(s.T(), listener.acceptStarted)
157165
cancel()
158-
s.Require().NoError(<-result)
166+
s.Require().NoError(receiveWithin(s.T(), result))
159167
}
160168

161169
func (s *ServeSuite) TestCancellationTriggersGracefulShutdown() {
@@ -175,9 +183,9 @@ func (s *ServeSuite) TestCancellationTriggersGracefulShutdown() {
175183
})
176184
}()
177185

178-
<-listener.acceptStarted
186+
receiveWithin(s.T(), listener.acceptStarted)
179187
cancel()
180-
s.Require().NoError(<-result)
188+
s.Require().NoError(receiveWithin(s.T(), result))
181189
s.True(shutdownCalled.Load())
182190
}
183191

@@ -192,15 +200,14 @@ func (s *ServeSuite) TestShutdownFailureIsReturned() {
192200
newHandler: http.NotFoundHandler,
193201
listen: func(string, string) (net.Listener, error) { return listener, nil },
194202
shutdown: func(server *http.Server, _ context.Context) error {
195-
_ = server.Close()
196203
return errors.New("drain failed")
197204
},
198205
})
199206
}()
200207

201-
<-listener.acceptStarted
208+
receiveWithin(s.T(), listener.acceptStarted)
202209
cancel()
203-
s.EqualError(<-result, "shutdown HTTP server: drain failed")
210+
s.EqualError(receiveWithin(s.T(), result), "shutdown HTTP server: drain failed")
204211
}
205212

206213
func (s *ServeSuite) TestServeFailureDuringShutdownIsReturned() {
@@ -219,9 +226,9 @@ func (s *ServeSuite) TestServeFailureDuringShutdownIsReturned() {
219226
})
220227
}()
221228

222-
<-listener.acceptStarted
229+
receiveWithin(s.T(), listener.acceptStarted)
223230
cancel()
224-
s.EqualError(<-result, "serve HTTP: accept failed during shutdown")
231+
s.EqualError(receiveWithin(s.T(), result), "serve HTTP: accept failed during shutdown")
225232
}
226233

227234
func (s *ServeSuite) TestServeAddress() {
@@ -946,6 +953,18 @@ func (s *ServeSuite) inlineInput(raw string) []byte {
946953
return input
947954
}
948955

956+
func receiveWithin[T any](t *testing.T, channel <-chan T) T {
957+
t.Helper()
958+
select {
959+
case value := <-channel:
960+
return value
961+
case <-time.After(time.Second):
962+
t.Fatal("timed out waiting for asynchronous operation")
963+
var zero T
964+
return zero
965+
}
966+
}
967+
949968
type testAddr string
950969

951970
func (a testAddr) Network() string { return "tcp" }

‎docs/src/content/docs/commands/serve.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ that the server is not exposed to a network unintentionally.
2626
Each request body is limited to 10 MiB. The server uses a 5-second header read
2727
timeout, a 15-second request read timeout, and 60-second write and idle
2828
timeouts. On `SIGINT` or `SIGTERM`, it stops accepting requests and allows up
29-
to 10 seconds for in-flight work to finish.
29+
to 10 seconds for in-flight work to finish, then closes remaining connections.
3030

3131
Browse the generated [API reference](/api/) for the complete OpenAPI contract,
3232
including request and response schemas and generated code snippets.

0 commit comments

Comments
 (0)