diff --git a/broker/README.md b/broker/README.md index b5f56163..4f1d0314 100644 --- a/broker/README.md +++ b/broker/README.md @@ -15,8 +15,10 @@ The broker's API uses hyperlinks to connect JSON resources. If you're using Chrome or another browser to explore the API, consider using an extension like [JSON Formatter](https://chromewebstore.google.com/detail/json-formatter/bcjindcccaagfpapjjmafapmmgkkhgoa) which allows to easily navigate hyperlinked JSON. -Note that API is also available with base path `/broker` if env -`TENANT_TO_SYMBOL` is defined. +Note that selected read-only API endpoints are accessible with the base path `/broker` +if env `TENANT_TO_SYMBOL` is defined. +This is allows the broker to be used as a FOLIO/Okapi module, +see the [ModuleDescriptor](./descriptors/ModuleDescriptor-template.json) for details. # Configuration diff --git a/broker/api/api-handler.go b/broker/api/api-handler.go index b5fe7cd2..39380672 100644 --- a/broker/api/api-handler.go +++ b/broker/api/api-handler.go @@ -82,15 +82,15 @@ func (a *ApiHandler) GetEvents(w http.ResponseWriter, r *http.Request, params oa Other: logParams, }) tran, err := a.getIllTranFromParams(ctx, params.RequesterReqId, params.IllTransactionId) - if err != nil && !errors.Is(err, pgx.ErrNoRows) { //DB error + if err != nil { //DB error + if errors.Is(err, pgx.ErrNoRows) { + writeEmpty(w) + return + } addInternalError(ctx, w, err) return } if !a.isOwner(&tran, params.XOkapiTenant, params.RequesterSymbol) { - addForbiddenError(ctx, w) - return - } - if err != nil && errors.Is(err, pgx.ErrNoRows) { writeEmpty(w) return } @@ -116,22 +116,19 @@ func (a *ApiHandler) GetIllTransactions(w http.ResponseWriter, r *http.Request, Other: map[string]string{"method": "GetIllTransactions"}, }) tran, err := a.getIllTranFromParams(ctx, params.RequesterReqId, nil) - if err != nil && !errors.Is(err, pgx.ErrNoRows) { //DB error + if err != nil { //DB error + if errors.Is(err, pgx.ErrNoRows) { + writeEmpty(w) + return + } addInternalError(ctx, w, err) return } - //TODO filter the list down instead - if !a.isOwner(&tran, params.XOkapiTenant, params.RequesterSymbol) { - addForbiddenError(ctx, w) - return - } - if err != nil && errors.Is(err, pgx.ErrNoRows) { - writeEmpty(w) - return - } resp := []oapi.IllTransaction{} if tran.ID != "" { - resp = append(resp, toApiIllTransaction(r, tran)) + if a.isOwner(&tran, params.XOkapiTenant, params.RequesterSymbol) { + resp = append(resp, toApiIllTransaction(r, tran)) + } } else { trans, err := a.illRepo.ListIllTransactions(ctx) if err != nil { //DB error @@ -152,12 +149,16 @@ func (a *ApiHandler) GetIllTransactionsId(w http.ResponseWriter, r *http.Request Other: map[string]string{"method": "GetIllTransactionsId", "id": id}, }) trans, err := a.illRepo.GetIllTransactionById(ctx, id) - if err != nil && !errors.Is(err, pgx.ErrNoRows) { + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + addNotFoundError(w) + return + } addInternalError(ctx, w, err) return } if !a.isOwner(&trans, params.XOkapiTenant, params.RequesterSymbol) { - addForbiddenError(ctx, w) + addNotFoundError(w) return } if trans.ID == "" { @@ -509,15 +510,15 @@ func (a *ApiHandler) GetLocatedSuppliers(w http.ResponseWriter, r *http.Request, Other: logParams, }) tran, err := a.getIllTranFromParams(ctx, params.RequesterReqId, params.IllTransactionId) - if err != nil && !errors.Is(err, pgx.ErrNoRows) { + if err != nil { //DB error + if errors.Is(err, pgx.ErrNoRows) { + writeEmpty(w) + return + } addInternalError(ctx, w, err) return } if !a.isOwner(&tran, params.XOkapiTenant, params.RequesterSymbol) { - addForbiddenError(ctx, w) - return - } - if err != nil && errors.Is(err, pgx.ErrNoRows) { writeEmpty(w) return } @@ -576,16 +577,6 @@ func addInternalError(ctx extctx.ExtendedContext, w http.ResponseWriter, err err _ = json.NewEncoder(w).Encode(resp) } -func addForbiddenError(ctx extctx.ExtendedContext, w http.ResponseWriter) { - resp := ErrorMessage{ - Error: "forbidden", - } - ctx.Logger().Error("error serving api request", "error", "forbidden") - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusForbidden) - _ = json.NewEncoder(w).Encode(resp) -} - func addBadRequestError(ctx extctx.ExtendedContext, w http.ResponseWriter, err error) { resp := ErrorMessage{ Error: err.Error(), diff --git a/broker/oapi/open-api.yaml b/broker/oapi/open-api.yaml index 83f828cb..2505d8da 100644 --- a/broker/oapi/open-api.yaml +++ b/broker/oapi/open-api.yaml @@ -262,7 +262,7 @@ paths: type: array items: $ref: '#/components/schemas/Event' - '400': # Example error response + '400': description: Bad Request. Invalid query parameters. content: application/json: @@ -272,17 +272,7 @@ paths: error: type: string description: Error message - '403': # Example error response - description: Forbidden. Invalid tenant. - content: - application/json: - schema: - type: object - properties: - error: - type: string - description: Error message - '500': # Example error response + '500': description: Internal Server Error content: application/json: @@ -311,7 +301,7 @@ paths: application/json: schema: $ref: '#/components/schemas/IllTransaction' - '400': # Example error response + '400': description: Bad Request. Invalid query parameters. content: application/json: @@ -321,8 +311,8 @@ paths: error: type: string description: Error message - '403': # Example error response - description: Forbidden. Invalid tenant. + '404': + description: Not Found. ILL transaction not found. content: application/json: schema: @@ -331,7 +321,7 @@ paths: error: type: string description: Error message - '500': # Example error response + '500': description: Internal Server Error content: application/json: @@ -373,7 +363,7 @@ paths: type: array items: $ref: '#/components/schemas/IllTransaction' - '400': # Example error response + '400': description: Bad Request. Invalid query parameters. content: application/json: @@ -383,7 +373,7 @@ paths: error: type: string description: Error message - '403': # Example error response + '403': description: Forbidden. Invalid tenant. content: application/json: @@ -393,7 +383,7 @@ paths: error: type: string description: Error message - '500': # Example error response + '500': description: Internal Server Error content: application/json: @@ -525,16 +515,6 @@ paths: error: type: string description: Error message - '403': # Example error response - description: Forbidden. Invalid tenant. - content: - application/json: - schema: - type: object - properties: - error: - type: string - description: Error message '500': description: Internal Server Error content: @@ -544,4 +524,4 @@ paths: properties: error: type: string - description: Error message \ No newline at end of file + description: Error message diff --git a/broker/test/api/api-handler_test.go b/broker/test/api/api-handler_test.go index 5e5b6a02..52e09e84 100644 --- a/broker/test/api/api-handler_test.go +++ b/broker/test/api/api-handler_test.go @@ -238,20 +238,19 @@ func TestBrokerCRUD(t *testing.T) { assert.Equal(t, getLocalhostWithPort()+"/broker/events?ill_transaction_id="+url.PathEscape(illId), tran.EventsLink) assert.Equal(t, getLocalhostWithPort()+"/broker/located_suppliers?ill_transaction_id="+url.PathEscape(illId), tran.LocatedSuppliersLink) - httpGet(t, "/broker/ill_transactions/"+illId+"?requester_symbol="+url.QueryEscape("ISIL:DK-DIKU"), "ruc", http.StatusForbidden) - - httpGet(t, "/broker/ill_transactions/"+illId, "ruc", http.StatusForbidden) - - httpGet(t, "/broker/ill_transactions/"+illId, "", http.StatusForbidden) + httpGet(t, "/broker/ill_transactions/"+illId+"?requester_symbol="+url.QueryEscape("ISIL:DK-DIKU"), "diku", http.StatusOK) + httpGet(t, "/broker/ill_transactions/"+illId+"?requester_symbol="+url.QueryEscape("ISIL:DK-DIKU"), "ruc", http.StatusNotFound) + httpGet(t, "/broker/ill_transactions/"+illId, "ruc", http.StatusNotFound) + httpGet(t, "/broker/ill_transactions/"+illId, "", http.StatusNotFound) body = httpGet(t, "/broker/ill_transactions/"+illId+"?requester_symbol="+url.QueryEscape("ISIL:DK-DIKU"), "", http.StatusOK) err = json.Unmarshal(body, &tran) assert.NoError(t, err) assert.Equal(t, illId, tran.ID) - httpGet(t, "/broker/ill_transactions", "diku", http.StatusForbidden) + assert.Equal(t, 1, len(httpGetTrans(t, "/broker/ill_transactions", "diku", http.StatusOK))) - httpGet(t, "/broker/ill_transactions", "ruc", http.StatusForbidden) + assert.Equal(t, 0, len(httpGetTrans(t, "/broker/ill_transactions", "ruc", http.StatusOK))) body = httpGet(t, "/broker/ill_transactions?requester_req_id="+url.QueryEscape(reqReqId), "diku", http.StatusOK) var trans []oapi.IllTransaction @@ -276,9 +275,9 @@ func TestBrokerCRUD(t *testing.T) { assert.Len(t, supps, 1) assert.Equal(t, locSup.ID, supps[0].ID) - httpGet(t, "/broker/located_suppliers?requester_req_id="+url.QueryEscape(reqReqId), "ruc", http.StatusForbidden) + assert.Equal(t, []byte("[]"), httpGet(t, "/broker/located_suppliers?requester_req_id="+url.QueryEscape(reqReqId), "ruc", http.StatusOK)) - httpGet(t, "/broker/located_suppliers?requester_req_id="+url.QueryEscape(uuid.NewString()), "diku", http.StatusForbidden) + assert.Equal(t, []byte("[]"), httpGet(t, "/broker/located_suppliers?requester_req_id="+url.QueryEscape(uuid.NewString()), "diku", http.StatusOK)) eventId := test.GetEventId(t, eventRepo, illId, events.EventTypeNotice, events.EventStatusSuccess, events.EventNameMessageRequester) @@ -301,9 +300,9 @@ func TestBrokerCRUD(t *testing.T) { assert.Len(t, events, 1) assert.Equal(t, eventId, events[0].ID) - httpGet(t, "/broker/events?requester_req_id="+url.QueryEscape(reqReqId), "ruc", http.StatusForbidden) + assert.Equal(t, []byte("[]"), httpGet(t, "/broker/events?requester_req_id="+url.QueryEscape(reqReqId), "ruc", http.StatusOK)) - httpGet(t, "/broker/events?requester_req_id="+url.QueryEscape(uuid.NewString()), "diku", http.StatusForbidden) + assert.Equal(t, []byte("[]"), httpGet(t, "/broker/events?requester_req_id="+url.QueryEscape(uuid.NewString()), "diku", http.StatusOK)) } func TestPeersCRUD(t *testing.T) { @@ -561,6 +560,16 @@ func httpRequest(t *testing.T, method string, uriPath string, reqbytes []byte, t return body } +func httpGetTrans(t *testing.T, uriPath string, tenant string, expectStatus int) []oapi.IllTransaction { + body := httpRequest(t, "GET", uriPath, nil, tenant, expectStatus) + var res []oapi.IllTransaction + err := json.Unmarshal(body, &res) + if err != nil { + t.Errorf("Failed to unmarshal json: %s", err) + } + return res +} + func httpGet(t *testing.T, uriPath string, tenant string, expectStatus int) []byte { return httpRequest(t, "GET", uriPath, nil, tenant, expectStatus) }