From 3f7d3ebe733d68c08e8181ad8cb35f300033fb9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Hern=C3=A1ndez?= Date: Wed, 21 Feb 2024 18:06:50 +0100 Subject: [PATCH] Add DELETE support (#67) This patch adds support for the `DELETE` HTTP method. The adapter will now accept the `DELETE` method and call the `Delete` method defined in the `DeleteHandler` interface. For example, a simple implementation that stores the object in a map in memory could look like this: ```go func (h *MyHandler) Delete(ctx context.Context, request *AddRequest) (respone *AddResponse, err error) { h.storeLock.Lock() defer h.storeLock.Unlock() delete(h.storeMap, request.Variables[0]) return } ``` Signed-off-by: Juan Hernandez --- internal/service/adapter.go | 46 ++++++++++++++++++++++++++- internal/service/adapter_test.go | 35 ++++++++++++++++++++ internal/service/handlers.go | 27 ++++++++++++++++ internal/service/handlers_mock.go | 53 +++++++++++++++++++++++++++++++ 4 files changed, 160 insertions(+), 1 deletion(-) diff --git a/internal/service/adapter.go b/internal/service/adapter.go index 3c2f8d16..c4590df5 100644 --- a/internal/service/adapter.go +++ b/internal/service/adapter.go @@ -51,6 +51,7 @@ type Adapter struct { listHandler ListHandler getHandler GetHandler addHandler AddHandler + deleteHandler DeleteHandler includeFields []search.Path excludeFields []search.Path pathsParser *search.PathsParser @@ -120,7 +121,8 @@ func (b *AdapterBuilder) Build() (result *Adapter, err error) { listHandler, _ := b.handler.(ListHandler) getHandler, _ := b.handler.(GetHandler) addHandler, _ := b.handler.(AddHandler) - if listHandler == nil && getHandler == nil && addHandler == nil { + deleteHandler, _ := b.handler.(DeleteHandler) + if listHandler == nil && getHandler == nil && addHandler == nil && deleteHandler == nil { err = errors.New("handler doesn't implement any of the handler interfaces") return } @@ -200,6 +202,7 @@ func (b *AdapterBuilder) Build() (result *Adapter, err error) { listHandler: listHandler, getHandler: getHandler, addHandler: addHandler, + deleteHandler: deleteHandler, includeFields: includePaths, excludeFields: excludePaths, selectorParser: selectorParser, @@ -224,6 +227,8 @@ func (a *Adapter) ServeHTTP(w http.ResponseWriter, r *http.Request) { a.serveGetMethod(w, r, pathVariables) case http.MethodPost: a.servePostMethod(w, r, pathVariables) + case http.MethodDelete: + a.serveDeleteMethod(w, r, pathVariables) default: SendError(w, http.StatusMethodNotAllowed, "Method '%s' is not allowed", r.Method) } @@ -261,6 +266,17 @@ func (a *Adapter) servePostMethod(w http.ResponseWriter, r *http.Request, pathVa a.serveAdd(w, r, pathVariables) } +func (a *Adapter) serveDeleteMethod(w http.ResponseWriter, r *http.Request, pathVariables []string) { + // Check that we have a compatible handler: + if a.deleteHandler == nil { + SendError(w, http.StatusMethodNotAllowed, "Method '%s' is not allowed", r.Method) + return + } + + // Call the handler: + a.serveDelete(w, r, pathVariables) +} + func (a *Adapter) serveGet(w http.ResponseWriter, r *http.Request, pathVariables []string) { // Get the context: ctx := r.Context() @@ -449,6 +465,34 @@ func (a *Adapter) serveAdd(w http.ResponseWriter, r *http.Request, pathVariables a.sendObject(ctx, w, response.Object) } +func (a *Adapter) serveDelete(w http.ResponseWriter, r *http.Request, pathVariables []string) { + // Get the context: + ctx := r.Context() + + // Create the request: + request := &DeleteRequest{ + Variables: pathVariables, + } + + // Call the handler: + _, err := a.deleteHandler.Delete(ctx, request) + if err != nil { + a.logger.Error( + "Failed to delete item", + "error", err, + ) + SendError( + w, + http.StatusInternalServerError, + "Failed to delete item", + ) + return + } + + // Send the result: + w.WriteHeader(http.StatusNoContent) +} + // 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 f4ab39b2..7c5391d2 100644 --- a/internal/service/adapter_test.go +++ b/internal/service/adapter_test.go @@ -1083,6 +1083,41 @@ var _ = Describe("Adapter", func() { }) }) + Describe("Object deletion", func() { + It("Deletes an object", func() { + // Prepare the handler: + body := func(ctx context.Context, + request *DeleteRequest) (response *DeleteResponse, err error) { + response = &DeleteResponse{} + return + } + handler := NewMockDeleteHandler(ctrl) + handler.EXPECT().Delete(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.MethodDelete, + "/mycollection/123", + nil, + ) + recorder := httptest.NewRecorder() + router.ServeHTTP(recorder, request) + + // Verify the response: + Expect(recorder.Code).To(Equal(http.StatusNoContent)) + }) + }) + DescribeTable( "JSON generation", func(items data.Stream, expected string) { diff --git a/internal/service/handlers.go b/internal/service/handlers.go index b26f104d..6c7ba111 100644 --- a/internal/service/handlers.go +++ b/internal/service/handlers.go @@ -120,10 +120,37 @@ type AddHandler interface { Add(ctx context.Context, request *AddRequest) (response *AddResponse, err error) } +// DeleteRequest represents a request to delete an object from a collection. +type DeleteRequest 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 +} + +// DeleteResponse represents the response to the request to delete an object from a collection. +type DeleteResponse struct { +} + +// DeleteHandler is the interface implemented by objects that know how delete items from a +// collection of objects. +type DeleteHandler interface { + Delete(ctx context.Context, request *DeleteRequest) (response *DeleteResponse, err error) +} + // 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 { ListHandler GetHandler AddHandler + DeleteHandler } diff --git a/internal/service/handlers_mock.go b/internal/service/handlers_mock.go index e2786726..280c9e66 100644 --- a/internal/service/handlers_mock.go +++ b/internal/service/handlers_mock.go @@ -129,6 +129,44 @@ func (mr *MockAddHandlerMockRecorder) Add(ctx, request any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Add", reflect.TypeOf((*MockAddHandler)(nil).Add), ctx, request) } +// MockDeleteHandler is a mock of DeleteHandler interface. +type MockDeleteHandler struct { + ctrl *gomock.Controller + recorder *MockDeleteHandlerMockRecorder +} + +// MockDeleteHandlerMockRecorder is the mock recorder for MockDeleteHandler. +type MockDeleteHandlerMockRecorder struct { + mock *MockDeleteHandler +} + +// NewMockDeleteHandler creates a new mock instance. +func NewMockDeleteHandler(ctrl *gomock.Controller) *MockDeleteHandler { + mock := &MockDeleteHandler{ctrl: ctrl} + mock.recorder = &MockDeleteHandlerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockDeleteHandler) EXPECT() *MockDeleteHandlerMockRecorder { + return m.recorder +} + +// Delete mocks base method. +func (m *MockDeleteHandler) Delete(ctx context.Context, request *DeleteRequest) (*DeleteResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Delete", ctx, request) + ret0, _ := ret[0].(*DeleteResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Delete indicates an expected call of Delete. +func (mr *MockDeleteHandlerMockRecorder) Delete(ctx, request any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*MockDeleteHandler)(nil).Delete), ctx, request) +} + // MockHandler is a mock of Handler interface. type MockHandler struct { ctrl *gomock.Controller @@ -167,6 +205,21 @@ func (mr *MockHandlerMockRecorder) Add(ctx, request any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Add", reflect.TypeOf((*MockHandler)(nil).Add), ctx, request) } +// Delete mocks base method. +func (m *MockHandler) Delete(ctx context.Context, request *DeleteRequest) (*DeleteResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Delete", ctx, request) + ret0, _ := ret[0].(*DeleteResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Delete indicates an expected call of Delete. +func (mr *MockHandlerMockRecorder) Delete(ctx, request any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*MockHandler)(nil).Delete), ctx, request) +} + // Get mocks base method. func (m *MockHandler) Get(ctx context.Context, request *GetRequest) (*GetResponse, error) { m.ctrl.T.Helper()