From 579913e0c00628dbfbaf1731d7478d3023d7ddc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Hern=C3=A1ndez?= Date: Wed, 21 Feb 2024 17:07:43 +0100 Subject: [PATCH] Add POST support (#66) This patch adds support for the `POST` HTTP method. The adapter will now accept the `POST` method and call the `Add` method defined in the `AddHandler` interface. An implementation of this interface will receive the request body already parsed, and should add the object to the collection and return it. For example, a simple implementation that stores the object in a map in memory could look like this: ```go func (h *MyHandler) Add(ctx context.Context, request *AddRequest) (respone *AddResponse, err error) { h.storeLock.Lock() defer h.storeLock.Unlock() h.storeMap[request.Variables[0]] = request.Object response := &AddResponse{ Object: request.Object, } return } ``` In order to make naming of the interfaces more consistent this patch also renames `ObjectHandler` to `GetHandler` and `CollectionHandler` to `ListHandler`. That way all interfaces are named after the operation they represent. Signed-off-by: Juan Hernandez --- internal/service/adapter.go | 157 +++++++++++++++++++++++++----- internal/service/adapter_test.go | 43 ++++++++ internal/service/handlers.go | 55 ++++++++--- internal/service/handlers_mock.go | 109 +++++++++++++++------ 4 files changed, 301 insertions(+), 63 deletions(-) diff --git a/internal/service/adapter.go b/internal/service/adapter.go index 15e04466..3c2f8d16 100644 --- a/internal/service/adapter.go +++ b/internal/service/adapter.go @@ -19,8 +19,10 @@ import ( "errors" "fmt" "log/slog" + "mime" "net/http" "slices" + "strings" "github.com/gorilla/mux" jsoniter "github.com/json-iterator/go" @@ -46,8 +48,9 @@ type AdapterBuilder struct { type Adapter struct { logger *slog.Logger pathVariables []string - collectionHandler CollectionHandler - objectHandler ObjectHandler + listHandler ListHandler + getHandler GetHandler + addHandler AddHandler includeFields []search.Path excludeFields []search.Path pathsParser *search.PathsParser @@ -114,20 +117,21 @@ func (b *AdapterBuilder) Build() (result *Adapter, err error) { } // Check that the handler implements at least one of the handler interfaces: - collectionHandler, _ := b.handler.(CollectionHandler) - objectHandler, _ := b.handler.(ObjectHandler) - if collectionHandler == nil && objectHandler == nil { + listHandler, _ := b.handler.(ListHandler) + getHandler, _ := b.handler.(GetHandler) + addHandler, _ := b.handler.(AddHandler) + if listHandler == nil && getHandler == nil && addHandler == nil { err = errors.New("handler doesn't implement any of the handler interfaces") return } - // If the handler implements the collection and object handler interfaces then we need to - // have at least one path variable because we use it to decide if the request is for the - // collection or for a specific object. - if collectionHandler != nil && objectHandler != nil && len(b.pathVariables) == 0 { + // If the handler implements the list and get handler interfaces then we need to have at + // least one path variable because we use it to decide if the request is for the collection + // or for a specific object. + if listHandler != nil && getHandler != nil && len(b.pathVariables) == 0 { err = errors.New( - "at least one path variable is required when both the collection and " + - "object handlers are implemented", + "at least one path variable is required when both the list and " + + "get handlers are implemented", ) return } @@ -193,8 +197,9 @@ func (b *AdapterBuilder) Build() (result *Adapter, err error) { logger: b.logger, pathVariables: variables, pathsParser: pathsParser, - collectionHandler: collectionHandler, - objectHandler: objectHandler, + listHandler: listHandler, + getHandler: getHandler, + addHandler: addHandler, includeFields: includePaths, excludeFields: excludePaths, selectorParser: selectorParser, @@ -213,21 +218,50 @@ func (a *Adapter) ServeHTTP(w http.ResponseWriter, r *http.Request) { pathVariables[i] = muxVariables[name] } + // Serve according to the HTTP method: + switch r.Method { + case http.MethodGet: + a.serveGetMethod(w, r, pathVariables) + case http.MethodPost: + a.servePostMethod(w, r, pathVariables) + default: + SendError(w, http.StatusMethodNotAllowed, "Method '%s' is not allowed", r.Method) + } +} + +func (a *Adapter) serveGetMethod(w http.ResponseWriter, r *http.Request, pathVariables []string) { + // Check that we have a compatible handler: + if a.listHandler == nil && a.getHandler == nil { + SendError(w, http.StatusMethodNotAllowed, "Method '%s' is not allowed", r.Method) + return + } + // If the handler only implements one of the interfaces then we use it unconditionally. // Otherwise we select the collection handler only if the first variable is empty. switch { - case a.collectionHandler != nil && a.objectHandler == nil: - a.serveCollection(w, r, pathVariables) - case a.collectionHandler == nil && a.objectHandler != nil: - a.serveObject(w, r, pathVariables) + case a.listHandler != nil && a.getHandler == nil: + a.serveList(w, r, pathVariables) + case a.listHandler == nil && a.getHandler != nil: + a.serveGet(w, r, pathVariables) case pathVariables[0] == "": - a.serveCollection(w, r, pathVariables[1:]) + a.serveList(w, r, pathVariables[1:]) default: - a.serveObject(w, r, pathVariables) + a.serveGet(w, r, pathVariables) } } -func (a *Adapter) serveObject(w http.ResponseWriter, r *http.Request, pathVariables []string) { +func (a *Adapter) servePostMethod(w http.ResponseWriter, r *http.Request, pathVariables []string) { + // Check that we have a compatible handler: + if a.addHandler == nil { + SendError(w, http.StatusMethodNotAllowed, "Method '%s' is not allowed", r.Method) + return + } + + // Call the handler: + a.serveAdd(w, r, pathVariables) +} + +func (a *Adapter) serveGet(w http.ResponseWriter, r *http.Request, pathVariables []string) { // Get the context: ctx := r.Context() @@ -244,7 +278,7 @@ func (a *Adapter) serveObject(w http.ResponseWriter, r *http.Request, pathVariab } // Call the handler: - response, err := a.objectHandler.Get(ctx, request) + response, err := a.getHandler.Get(ctx, request) if err != nil { a.logger.Error( "Failed to get object", @@ -288,7 +322,7 @@ func (a *Adapter) serveObject(w http.ResponseWriter, r *http.Request, pathVariab a.sendObject(ctx, w, object) } -func (a *Adapter) serveCollection(w http.ResponseWriter, r *http.Request, pathVariables []string) { +func (a *Adapter) serveList(w http.ResponseWriter, r *http.Request, pathVariables []string) { // Get the context: ctx := r.Context() @@ -309,7 +343,7 @@ func (a *Adapter) serveCollection(w http.ResponseWriter, r *http.Request, pathVa } // Call the handler: - response, err := a.collectionHandler.List(ctx, request) + response, err := a.listHandler.List(ctx, request) if err != nil { a.logger.Error( "Failed to get items", @@ -338,6 +372,83 @@ func (a *Adapter) serveCollection(w http.ResponseWriter, r *http.Request, pathVa a.sendItems(ctx, w, items) } +func (a *Adapter) serveAdd(w http.ResponseWriter, r *http.Request, pathVariables []string) { + // Get the context: + ctx := r.Context() + + // Check that the content type is acceptable: + contentType := r.Header.Get("Content-Type") + if contentType == "" { + a.logger.Error( + "Received empty content type header", + ) + SendError( + w, http.StatusBadRequest, + "Content type is mandatory, use 'application/json'", + ) + return + } + mediaType, _, err := mime.ParseMediaType(contentType) + if err != nil { + a.logger.Error( + "Failed to parse content type", + slog.String("header", contentType), + slog.String("error", err.Error()), + ) + SendError(w, http.StatusBadRequest, "Failed to parse content type '%s'", contentType) + } + if !strings.EqualFold(mediaType, "application/json") { + a.logger.Error( + "Unsupported content type", + slog.String("header", contentType), + slog.String("media", mediaType), + ) + SendError( + w, http.StatusBadRequest, + "Content type '%s' isn't supported, use 'application/json'", + mediaType, + ) + return + } + + // Parse the request body: + decoder := a.jsonAPI.NewDecoder(r.Body) + var object data.Object + err = decoder.Decode(&object) + if err != nil { + a.logger.Error( + "Failed to decode input", + slog.String("error", err.Error()), + ) + SendError(w, http.StatusBadRequest, "Failed to decode input") + return + } + + // Create the request: + request := &AddRequest{ + Variables: pathVariables, + Object: object, + } + + // Call the handler: + response, err := a.addHandler.Add(ctx, request) + if err != nil { + a.logger.Error( + "Failed to add item", + "error", err, + ) + SendError( + w, + http.StatusInternalServerError, + "Failed to add item", + ) + return + } + + // Send the added object: + a.sendObject(ctx, w, response.Object) +} + // extractSelector tries to extract the selector from the request. It return the selector and a // flag indicating if it is okay to continue processing the request. When this flag is false the // error response was already sent to the client, and request processing should stop. diff --git a/internal/service/adapter_test.go b/internal/service/adapter_test.go index 5bfd513a..f4ab39b2 100644 --- a/internal/service/adapter_test.go +++ b/internal/service/adapter_test.go @@ -20,6 +20,7 @@ import ( "io" "net/http" "net/http/httptest" + "strings" "github.com/gorilla/mux" . "github.com/onsi/ginkgo/v2/dsl/core" @@ -1040,6 +1041,48 @@ var _ = Describe("Adapter", func() { }) }) + Describe("Object creation", func() { + It("Creates object that doesn't exist", func() { + // Prepare the handler: + body := func(ctx context.Context, + request *AddRequest) (response *AddResponse, err error) { + response = &AddResponse{ + Object: data.Object{ + "myattr": "myvalue", + }, + } + return + } + handler := NewMockAddHandler(ctrl) + handler.EXPECT().Add(gomock.Any(), gomock.Any()).DoAndReturn(body) + + // Create the adapter: + adapter, err := NewAdapter(). + SetLogger(logger). + SetPathVariables("id"). + SetHandler(handler). + Build() + Expect(err).ToNot(HaveOccurred()) + router := mux.NewRouter() + router.Handle("/mycollection/{id}", adapter) + + // Send the request: + request := httptest.NewRequest( + http.MethodPost, + "/mycollection/123", + strings.NewReader("{}"), + ) + request.Header.Set("Content-Type", "application/json") + recorder := httptest.NewRecorder() + router.ServeHTTP(recorder, request) + + // Verify the response: + Expect(recorder.Body).To(MatchJSON(`{ + "myattr": "myvalue" + }`)) + }) + }) + DescribeTable( "JSON generation", func(items data.Stream, expected string) { diff --git a/internal/service/handlers.go b/internal/service/handlers.go index 6ad339e5..b26f104d 100644 --- a/internal/service/handlers.go +++ b/internal/service/handlers.go @@ -54,6 +54,12 @@ type ListResponse struct { Items data.Stream } +// ListHandler is the interface implemented by objects that know how to get list +// of items of a collection of objects. +type ListHandler interface { + List(ctx context.Context, request *ListRequest) (response *ListResponse, err error) +} + // GetRequest represents a request for an individual object. type GetRequest struct { // Variables contains the values of the path variables. For example, if the request path is @@ -78,21 +84,46 @@ type GetResponse struct { Object data.Object } -// CollectionHandler is the interface implemented by objects that know how to handle requests to -// list the items of a collection of objects. -type CollectionHandler interface { - List(ctx context.Context, request *ListRequest) (response *ListResponse, err error) +// GetHandler is the interface implemented by objects that now how to get the details of an object. +type GetHandler interface { + Get(ctx context.Context, request *GetRequest) (response *GetResponse, err error) } -// ObjectHandler is the interface implemented by objects that now how to handle requests to get the -// details of an object. -type ObjectHandler interface { - Get(ctx context.Context, request *GetRequest) (response *GetResponse, err error) +// AddRequest represents a request to create a new object inside a collection. +type AddRequest struct { + // Variables contains the values of the path variables. For example, if the request path is + // like this: + // + // /o2ims-infrastructureInventory/v1/resourcePools/123/resources/456 + // + // Then it will contain '456' and '123'. + // + // These path variables are ordered from more specific to less specific, the opposite of + // what appears in the request path. This is intended to simplify things because most + // handlers will only be interested in the most specific identifier and therefore they + // can just use index zero. + Variables []string + + // Object is the definition of the object. + Object data.Object +} + +// AddResponse represents the response to the request to create a new object inside a collection. +type AddResponse struct { + // Object is the definition of the object that was created. + Object data.Object +} + +// AddHandler is the interface implemented by objects that know how add items to a collection +// of objects. +type AddHandler interface { + Add(ctx context.Context, request *AddRequest) (response *AddResponse, err error) } -// Handler is the interface implemented by objects that knows how to handle requests to list the -// items of a collection, as well as requests to get a specific object. +// Handler aggregates all the other specific handlers. This is intended for unit/ tests, where it +// is convenient to have a single mock that implements all the operations. type Handler interface { - CollectionHandler - ObjectHandler + ListHandler + GetHandler + AddHandler } diff --git a/internal/service/handlers_mock.go b/internal/service/handlers_mock.go index af811c56..e2786726 100644 --- a/internal/service/handlers_mock.go +++ b/internal/service/handlers_mock.go @@ -15,31 +15,31 @@ import ( gomock "go.uber.org/mock/gomock" ) -// MockCollectionHandler is a mock of CollectionHandler interface. -type MockCollectionHandler struct { +// MockListHandler is a mock of ListHandler interface. +type MockListHandler struct { ctrl *gomock.Controller - recorder *MockCollectionHandlerMockRecorder + recorder *MockListHandlerMockRecorder } -// MockCollectionHandlerMockRecorder is the mock recorder for MockCollectionHandler. -type MockCollectionHandlerMockRecorder struct { - mock *MockCollectionHandler +// MockListHandlerMockRecorder is the mock recorder for MockListHandler. +type MockListHandlerMockRecorder struct { + mock *MockListHandler } -// NewMockCollectionHandler creates a new mock instance. -func NewMockCollectionHandler(ctrl *gomock.Controller) *MockCollectionHandler { - mock := &MockCollectionHandler{ctrl: ctrl} - mock.recorder = &MockCollectionHandlerMockRecorder{mock} +// NewMockListHandler creates a new mock instance. +func NewMockListHandler(ctrl *gomock.Controller) *MockListHandler { + mock := &MockListHandler{ctrl: ctrl} + mock.recorder = &MockListHandlerMockRecorder{mock} return mock } // EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockCollectionHandler) EXPECT() *MockCollectionHandlerMockRecorder { +func (m *MockListHandler) EXPECT() *MockListHandlerMockRecorder { return m.recorder } // List mocks base method. -func (m *MockCollectionHandler) List(ctx context.Context, request *ListRequest) (*ListResponse, error) { +func (m *MockListHandler) List(ctx context.Context, request *ListRequest) (*ListResponse, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "List", ctx, request) ret0, _ := ret[0].(*ListResponse) @@ -48,36 +48,36 @@ func (m *MockCollectionHandler) List(ctx context.Context, request *ListRequest) } // List indicates an expected call of List. -func (mr *MockCollectionHandlerMockRecorder) List(ctx, request any) *gomock.Call { +func (mr *MockListHandlerMockRecorder) List(ctx, request any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "List", reflect.TypeOf((*MockCollectionHandler)(nil).List), ctx, request) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "List", reflect.TypeOf((*MockListHandler)(nil).List), ctx, request) } -// MockObjectHandler is a mock of ObjectHandler interface. -type MockObjectHandler struct { +// MockGetHandler is a mock of GetHandler interface. +type MockGetHandler struct { ctrl *gomock.Controller - recorder *MockObjectHandlerMockRecorder + recorder *MockGetHandlerMockRecorder } -// MockObjectHandlerMockRecorder is the mock recorder for MockObjectHandler. -type MockObjectHandlerMockRecorder struct { - mock *MockObjectHandler +// MockGetHandlerMockRecorder is the mock recorder for MockGetHandler. +type MockGetHandlerMockRecorder struct { + mock *MockGetHandler } -// NewMockObjectHandler creates a new mock instance. -func NewMockObjectHandler(ctrl *gomock.Controller) *MockObjectHandler { - mock := &MockObjectHandler{ctrl: ctrl} - mock.recorder = &MockObjectHandlerMockRecorder{mock} +// NewMockGetHandler creates a new mock instance. +func NewMockGetHandler(ctrl *gomock.Controller) *MockGetHandler { + mock := &MockGetHandler{ctrl: ctrl} + mock.recorder = &MockGetHandlerMockRecorder{mock} return mock } // EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockObjectHandler) EXPECT() *MockObjectHandlerMockRecorder { +func (m *MockGetHandler) EXPECT() *MockGetHandlerMockRecorder { return m.recorder } // Get mocks base method. -func (m *MockObjectHandler) Get(ctx context.Context, request *GetRequest) (*GetResponse, error) { +func (m *MockGetHandler) Get(ctx context.Context, request *GetRequest) (*GetResponse, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Get", ctx, request) ret0, _ := ret[0].(*GetResponse) @@ -86,9 +86,47 @@ func (m *MockObjectHandler) Get(ctx context.Context, request *GetRequest) (*GetR } // Get indicates an expected call of Get. -func (mr *MockObjectHandlerMockRecorder) Get(ctx, request any) *gomock.Call { +func (mr *MockGetHandlerMockRecorder) Get(ctx, request any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockObjectHandler)(nil).Get), ctx, request) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockGetHandler)(nil).Get), ctx, request) +} + +// MockAddHandler is a mock of AddHandler interface. +type MockAddHandler struct { + ctrl *gomock.Controller + recorder *MockAddHandlerMockRecorder +} + +// MockAddHandlerMockRecorder is the mock recorder for MockAddHandler. +type MockAddHandlerMockRecorder struct { + mock *MockAddHandler +} + +// NewMockAddHandler creates a new mock instance. +func NewMockAddHandler(ctrl *gomock.Controller) *MockAddHandler { + mock := &MockAddHandler{ctrl: ctrl} + mock.recorder = &MockAddHandlerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockAddHandler) EXPECT() *MockAddHandlerMockRecorder { + return m.recorder +} + +// Add mocks base method. +func (m *MockAddHandler) Add(ctx context.Context, request *AddRequest) (*AddResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Add", ctx, request) + ret0, _ := ret[0].(*AddResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Add indicates an expected call of Add. +func (mr *MockAddHandlerMockRecorder) Add(ctx, request any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Add", reflect.TypeOf((*MockAddHandler)(nil).Add), ctx, request) } // MockHandler is a mock of Handler interface. @@ -114,6 +152,21 @@ func (m *MockHandler) EXPECT() *MockHandlerMockRecorder { return m.recorder } +// Add mocks base method. +func (m *MockHandler) Add(ctx context.Context, request *AddRequest) (*AddResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Add", ctx, request) + ret0, _ := ret[0].(*AddResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Add indicates an expected call of Add. +func (mr *MockHandlerMockRecorder) Add(ctx, request any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Add", reflect.TypeOf((*MockHandler)(nil).Add), ctx, request) +} + // Get mocks base method. func (m *MockHandler) Get(ctx context.Context, request *GetRequest) (*GetResponse, error) { m.ctrl.T.Helper()