Skip to content

Commit

Permalink
fix(api): health & root endpoints use middleware, which handles OPTIONS
Browse files Browse the repository at this point in the history
reviewing a frontend PR caught a regresssion from a recent change that dropped
logging from the `/health` and `/` endpoints: it also dropped CORS headers set
by middlware on each of those routes. Add it back with refactoring, and do a
bunch of cleanup to make routes work a little more consistently.

Now OPTIONS requests are handled entirely by middleware, which means middleware
MUST be added to all routes.
  • Loading branch information
b5 committed Nov 3, 2020
1 parent 08832c4 commit 5f421eb
Show file tree
Hide file tree
Showing 25 changed files with 283 additions and 385 deletions.
4 changes: 2 additions & 2 deletions api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,8 @@ func NewServerRoutes(s Server) *http.ServeMux {

m := http.NewServeMux()

m.HandleFunc("/", HomeHandler)
m.HandleFunc("/health", HealthCheckHandler)
m.Handle("/", s.noLogMiddleware(HomeHandler))
m.Handle("/health", s.noLogMiddleware(HealthCheckHandler))
m.Handle("/ipfs/", s.middleware(s.HandleIPFSPath))
m.Handle("/ipns/", s.middleware(s.HandleIPNSPath))

Expand Down
7 changes: 3 additions & 4 deletions api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ func TestHealthCheck(t *testing.T) {
}()

healthCheckCases := []handlerTestCase{
{"OPTIONS", "/", nil},
{"GET", "/", nil},
}
runHandlerTestCases(t, "health check", HealthCheckHandler, healthCheckCases, true)
Expand Down Expand Up @@ -235,14 +234,14 @@ func TestServerReadOnlyRoutes(t *testing.T) {
{"PUT", "/save/", 403},
{"POST", "/remove/", 403},
{"DELETE", "/remove/", 403},
{"POST", "/add/", 404},
{"PUT", "/add/", 404},
{"POST", "/add/", 403},
{"PUT", "/add/", 403},
{"POST", "/rename", 403},
{"PUT", "/rename", 403},
{"POST", "/diff", 403},
{"GET", "/diff", 403},
{"GET", "/body/", 403},
{"POST", "/registry/", 404},
{"POST", "/registry/", 403},
{"GET", "/checkout", 403},
{"GET", "/status", 403},
{"GET", "/init", 403},
Expand Down
220 changes: 0 additions & 220 deletions api/dataset_test.go

This file was deleted.

44 changes: 11 additions & 33 deletions api/datasets.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,7 @@ func NewDatasetHandlers(inst *lib.Instance, readOnly bool) *DatasetHandlers {
// ListHandler is a dataset list endpoint
func (h *DatasetHandlers) ListHandler(w http.ResponseWriter, r *http.Request) {
switch r.Method {
case "OPTIONS":
util.EmptyOkHandler(w, r)
case "GET":
case http.MethodGet:
if h.ReadOnly {
readOnlyResponse(w, "/list")
return
Expand All @@ -58,9 +56,7 @@ func (h *DatasetHandlers) ListHandler(w http.ResponseWriter, r *http.Request) {
// SaveHandler is a dataset save/update endpoint
func (h *DatasetHandlers) SaveHandler(w http.ResponseWriter, r *http.Request) {
switch r.Method {
case "OPTIONS":
util.EmptyOkHandler(w, r)
case "PUT", "POST":
case http.MethodPut, http.MethodPost:
h.saveHandler(w, r)
default:
util.NotFoundHandler(w, r)
Expand All @@ -70,9 +66,7 @@ func (h *DatasetHandlers) SaveHandler(w http.ResponseWriter, r *http.Request) {
// RemoveHandler is a a dataset delete endpoint
func (h *DatasetHandlers) RemoveHandler(w http.ResponseWriter, r *http.Request) {
switch r.Method {
case "OPTIONS":
util.EmptyOkHandler(w, r)
case "DELETE", "POST":
case http.MethodDelete, http.MethodPost:
h.removeHandler(w, r)
default:
util.NotFoundHandler(w, r)
Expand All @@ -82,9 +76,7 @@ func (h *DatasetHandlers) RemoveHandler(w http.ResponseWriter, r *http.Request)
// GetHandler is a dataset single endpoint
func (h *DatasetHandlers) GetHandler(w http.ResponseWriter, r *http.Request) {
switch r.Method {
case "OPTIONS":
util.EmptyOkHandler(w, r)
case "GET":
case http.MethodGet:
h.getHandler(w, r)
default:
util.NotFoundHandler(w, r)
Expand All @@ -94,9 +86,7 @@ func (h *DatasetHandlers) GetHandler(w http.ResponseWriter, r *http.Request) {
// DiffHandler is a dataset single endpoint
func (h *DatasetHandlers) DiffHandler(w http.ResponseWriter, r *http.Request) {
switch r.Method {
case "OPTIONS":
util.EmptyOkHandler(w, r)
case "POST", "GET":
case http.MethodPost, http.MethodGet:
if h.ReadOnly {
readOnlyResponse(w, "/diff")
return
Expand All @@ -110,9 +100,7 @@ func (h *DatasetHandlers) DiffHandler(w http.ResponseWriter, r *http.Request) {
// PeerListHandler is a dataset list endpoint
func (h *DatasetHandlers) PeerListHandler(w http.ResponseWriter, r *http.Request) {
switch r.Method {
case "OPTIONS":
util.EmptyOkHandler(w, r)
case "GET":
case http.MethodGet:
h.peerListHandler(w, r)
default:
util.NotFoundHandler(w, r)
Expand All @@ -122,9 +110,7 @@ func (h *DatasetHandlers) PeerListHandler(w http.ResponseWriter, r *http.Request
// PullHandler is an endpoint for creating new datasets
func (h *DatasetHandlers) PullHandler(w http.ResponseWriter, r *http.Request) {
switch r.Method {
case "OPTIONS":
util.EmptyOkHandler(w, r)
case "POST", "PUT":
case http.MethodPost, http.MethodPut:
h.pullHandler(w, r)
default:
util.NotFoundHandler(w, r)
Expand All @@ -134,9 +120,7 @@ func (h *DatasetHandlers) PullHandler(w http.ResponseWriter, r *http.Request) {
// RenameHandler is the endpoint for renaming datasets
func (h *DatasetHandlers) RenameHandler(w http.ResponseWriter, r *http.Request) {
switch r.Method {
case "OPTIONS":
util.EmptyOkHandler(w, r)
case "POST", "PUT":
case http.MethodPost, http.MethodPut:
h.renameHandler(w, r)
default:
util.NotFoundHandler(w, r)
Expand All @@ -146,9 +130,7 @@ func (h *DatasetHandlers) RenameHandler(w http.ResponseWriter, r *http.Request)
// BodyHandler gets the contents of a dataset
func (h *DatasetHandlers) BodyHandler(w http.ResponseWriter, r *http.Request) {
switch r.Method {
case "OPTIONS":
util.EmptyOkHandler(w, r)
case "GET":
case http.MethodGet:
if h.ReadOnly {
readOnlyResponse(w, "/body/")
return
Expand All @@ -162,9 +144,7 @@ func (h *DatasetHandlers) BodyHandler(w http.ResponseWriter, r *http.Request) {
// StatsHandler gets stats about the dataset
func (h *DatasetHandlers) StatsHandler(w http.ResponseWriter, r *http.Request) {
switch r.Method {
case "OPTIONS":
util.EmptyOkHandler(w, r)
case "GET":
case http.MethodGet:
h.statsHandler(w, r)
default:
util.NotFoundHandler(w, r)
Expand All @@ -174,9 +154,7 @@ func (h *DatasetHandlers) StatsHandler(w http.ResponseWriter, r *http.Request) {
// UnpackHandler unpacks a zip file and sends it back as json
func (h *DatasetHandlers) UnpackHandler(w http.ResponseWriter, r *http.Request) {
switch r.Method {
case "OPTIONS":
util.EmptyOkHandler(w, r)
case "POST":
case http.MethodPost:
postData, err := ioutil.ReadAll(r.Body)
if err != nil {
util.WriteErrResponse(w, http.StatusBadRequest, err)
Expand Down

0 comments on commit 5f421eb

Please sign in to comment.