From b9cfc069c03195b85eb4454244c58bf8fee55a9b Mon Sep 17 00:00:00 2001 From: Janis Saldabols Date: Wed, 4 Feb 2026 12:41:19 +0200 Subject: [PATCH 1/2] CROSSLINK-208 Make side as optional parameter --- broker/oapi/open-api.yaml | 1 - broker/patron_request/api/api-handler.go | 53 ++++++++++++------- broker/patron_request/api/api-handler_test.go | 29 +++++----- .../patron_request/api/api-handler_test.go | 28 +++++++--- 4 files changed, 70 insertions(+), 41 deletions(-) diff --git a/broker/oapi/open-api.yaml b/broker/oapi/open-api.yaml index c43d0fce..d7e35341 100644 --- a/broker/oapi/open-api.yaml +++ b/broker/oapi/open-api.yaml @@ -70,7 +70,6 @@ components: name: side in: query description: Patron request side, valid values "borrowing" and "lending" - required: true schema: type: string Symbol: diff --git a/broker/patron_request/api/api-handler.go b/broker/patron_request/api/api-handler.go index a4131499..ebad1fa0 100644 --- a/broker/patron_request/api/api-handler.go +++ b/broker/patron_request/api/api-handler.go @@ -55,7 +55,10 @@ func (a *PatronRequestApiHandler) GetStateModelModelsModel(w http.ResponseWriter func (a *PatronRequestApiHandler) GetPatronRequests(w http.ResponseWriter, r *http.Request, params proapi.GetPatronRequestsParams) { symbol, err := api.GetSymbolForRequest(r, a.tenant, params.XOkapiTenant, params.Symbol) - logParams := map[string]string{"method": "GetPatronRequests", "side": params.Side, "symbol": symbol} + logParams := map[string]string{"method": "GetPatronRequests", "symbol": symbol} + if params.Side != nil { + logParams["side"] = *params.Side + } ctx := common.CreateExtCtxWithArgs(context.Background(), &common.LoggerArgs{ Other: logParams, }) @@ -84,8 +87,8 @@ func (a *PatronRequestApiHandler) GetPatronRequests(w http.ResponseWriter, r *ht } var side pr_db.PatronRequestSide if isSideParamValid(params.Side) { - side = pr_db.PatronRequestSide(params.Side) - _, err = qb.And().Search("side").Term(params.Side).Build() + side = pr_db.PatronRequestSide(*params.Side) + _, err = qb.And().Search("side").Term(*params.Side).Build() if err != nil { addBadRequestError(ctx, w, err) return @@ -205,9 +208,12 @@ func (a *PatronRequestApiHandler) PostPatronRequests(w http.ResponseWriter, r *h func (a *PatronRequestApiHandler) DeletePatronRequestsId(w http.ResponseWriter, r *http.Request, id string, params proapi.DeletePatronRequestsIdParams) { symbol, err := api.GetSymbolForRequest(r, a.tenant, params.XOkapiTenant, params.Symbol) - ctx := common.CreateExtCtxWithArgs(context.Background(), &common.LoggerArgs{ - Other: map[string]string{"method": "DeletePatronRequestsId", "id": id, "side": params.Side, "symbol": symbol}, - }) + logParams := map[string]string{"method": "DeletePatronRequestsId", "id": id, "symbol": symbol} + if params.Side != nil { + logParams["side"] = *params.Side + } + ctx := common.CreateExtCtxWithArgs(context.Background(), &common.LoggerArgs{Other: logParams}) + if err != nil { addBadRequestError(ctx, w, err) return @@ -231,7 +237,7 @@ func (a *PatronRequestApiHandler) DeletePatronRequestsId(w http.ResponseWriter, w.WriteHeader(http.StatusNoContent) } -func (a *PatronRequestApiHandler) getPatronRequestById(ctx common.ExtendedContext, id string, side string, symbol string) (*pr_db.PatronRequest, error) { +func (a *PatronRequestApiHandler) getPatronRequestById(ctx common.ExtendedContext, id string, side *string, symbol string) (*pr_db.PatronRequest, error) { pr, err := a.prRepo.GetPatronRequestById(ctx, id) if err != nil { if errors.Is(err, pgx.ErrNoRows) { @@ -239,14 +245,14 @@ func (a *PatronRequestApiHandler) getPatronRequestById(ctx common.ExtendedContex } return nil, err } - if isOwner(pr, symbol) && (!isSideParamValid(side) || string(pr.Side) == side) { + if isOwner(pr, symbol) && (!isSideParamValid(side) || (side != nil && string(pr.Side) == *side)) { return &pr, nil } return nil, nil } -func isSideParamValid(side string) bool { - return side == string(prservice.SideBorrowing) || side == string(prservice.SideLending) +func isSideParamValid(side *string) bool { + return side != nil && (*side == string(prservice.SideBorrowing) || *side == string(prservice.SideLending)) } func isOwner(pr pr_db.PatronRequest, symbol string) bool { @@ -256,9 +262,12 @@ func isOwner(pr pr_db.PatronRequest, symbol string) bool { func (a *PatronRequestApiHandler) GetPatronRequestsId(w http.ResponseWriter, r *http.Request, id string, params proapi.GetPatronRequestsIdParams) { symbol, err := api.GetSymbolForRequest(r, a.tenant, params.XOkapiTenant, params.Symbol) - ctx := common.CreateExtCtxWithArgs(context.Background(), &common.LoggerArgs{ - Other: map[string]string{"method": "GetPatronRequestsId", "id": id, "side": params.Side, "symbol": symbol}, - }) + logParams := map[string]string{"method": "GetPatronRequestsId", "id": id, "symbol": symbol} + if params.Side != nil { + logParams["side"] = *params.Side + } + ctx := common.CreateExtCtxWithArgs(context.Background(), &common.LoggerArgs{Other: logParams}) + if err != nil { addBadRequestError(ctx, w, err) return @@ -283,9 +292,12 @@ func (a *PatronRequestApiHandler) GetPatronRequestsId(w http.ResponseWriter, r * func (a *PatronRequestApiHandler) GetPatronRequestsIdActions(w http.ResponseWriter, r *http.Request, id string, params proapi.GetPatronRequestsIdActionsParams) { symbol, err := api.GetSymbolForRequest(r, a.tenant, params.XOkapiTenant, params.Symbol) - ctx := common.CreateExtCtxWithArgs(context.Background(), &common.LoggerArgs{ - Other: map[string]string{"method": "GetPatronRequestsIdActions", "id": id, "side": params.Side, "symbol": symbol}, - }) + logParams := map[string]string{"method": "GetPatronRequestsIdActions", "id": id, "symbol": symbol} + if params.Side != nil { + logParams["side"] = *params.Side + } + ctx := common.CreateExtCtxWithArgs(context.Background(), &common.LoggerArgs{Other: logParams}) + if err != nil { addBadRequestError(ctx, w, err) return @@ -305,9 +317,12 @@ func (a *PatronRequestApiHandler) GetPatronRequestsIdActions(w http.ResponseWrit func (a *PatronRequestApiHandler) PostPatronRequestsIdAction(w http.ResponseWriter, r *http.Request, id string, params proapi.PostPatronRequestsIdActionParams) { symbol, err := api.GetSymbolForRequest(r, a.tenant, params.XOkapiTenant, params.Symbol) - ctx := common.CreateExtCtxWithArgs(context.Background(), &common.LoggerArgs{ - Other: map[string]string{"method": "PostPatronRequestsIdAction", "id": id, "side": params.Side, "symbol": symbol}, - }) + logParams := map[string]string{"method": "PostPatronRequestsIdAction", "id": id, "symbol": symbol} + if params.Side != nil { + logParams["side"] = *params.Side + } + ctx := common.CreateExtCtxWithArgs(context.Background(), &common.LoggerArgs{Other: logParams}) + if err != nil { addBadRequestError(ctx, w, err) return diff --git a/broker/patron_request/api/api-handler_test.go b/broker/patron_request/api/api-handler_test.go index 39f22803..73feef9f 100644 --- a/broker/patron_request/api/api-handler_test.go +++ b/broker/patron_request/api/api-handler_test.go @@ -25,6 +25,9 @@ import ( var mockEventBus = new(MockEventBus) var symbol = "ISIL:REQ" +var lendingString = string(prservice.SideLending) +var proapiBorrowingSide = proapi.Side(prservice.SideBorrowing) +var proapiLendingSide = proapi.Side(prservice.SideLending) func TestGetId(t *testing.T) { assert.True(t, getId("") != "") @@ -46,7 +49,7 @@ func TestGetPatronRequests(t *testing.T) { req, _ := http.NewRequest("GET", "/", nil) rr := httptest.NewRecorder() params := proapi.GetPatronRequestsParams{ - Side: string(prservice.SideLending), + Side: &lendingString, Symbol: &symbol, } handler.GetPatronRequests(rr, req, params) @@ -59,7 +62,7 @@ func TestGetPatronRequestsNoSymbol(t *testing.T) { req, _ := http.NewRequest("GET", "/", nil) rr := httptest.NewRecorder() params := proapi.GetPatronRequestsParams{ - Side: string(prservice.SideLending), + Side: &lendingString, } handler.GetPatronRequests(rr, req, params) assert.Equal(t, http.StatusBadRequest, rr.Code) @@ -74,7 +77,7 @@ func TestGetPatronRequestsWithLimits(t *testing.T) { limit := proapi.Limit(10) cql := "state = NEW" params := proapi.GetPatronRequestsParams{ - Side: string(prservice.SideLending), + Side: &lendingString, Symbol: &symbol, Offset: &offset, Limit: &limit, @@ -153,7 +156,7 @@ func TestDeletePatronRequestsId(t *testing.T) { handler := NewPrApiHandler(new(PrRepoError), mockEventBus, common.NewTenant(""), 10) req, _ := http.NewRequest("POST", "/", nil) rr := httptest.NewRecorder() - handler.DeletePatronRequestsId(rr, req, "3", proapi.DeletePatronRequestsIdParams{Symbol: &symbol, Side: proapi.Side(prservice.SideBorrowing)}) + handler.DeletePatronRequestsId(rr, req, "3", proapi.DeletePatronRequestsIdParams{Symbol: &symbol, Side: &proapiBorrowingSide}) assert.Equal(t, http.StatusInternalServerError, rr.Code) assert.Contains(t, rr.Body.String(), "DB error") } @@ -162,7 +165,7 @@ func TestDeletePatronRequestsIdDeleted(t *testing.T) { handler := NewPrApiHandler(new(PrRepoError), mockEventBus, common.NewTenant(""), 10) req, _ := http.NewRequest("POST", "/", nil) rr := httptest.NewRecorder() - handler.DeletePatronRequestsId(rr, req, "4", proapi.DeletePatronRequestsIdParams{Symbol: &symbol, Side: proapi.Side(prservice.SideBorrowing)}) + handler.DeletePatronRequestsId(rr, req, "4", proapi.DeletePatronRequestsIdParams{Symbol: &symbol, Side: &proapiBorrowingSide}) assert.Equal(t, http.StatusNoContent, rr.Code) } @@ -196,7 +199,7 @@ func TestGetPatronRequestsIdActions(t *testing.T) { handler := NewPrApiHandler(new(PrRepoError), mockEventBus, common.NewTenant(""), 10) req, _ := http.NewRequest("GET", "/", nil) rr := httptest.NewRecorder() - handler.GetPatronRequestsIdActions(rr, req, "3", proapi.GetPatronRequestsIdActionsParams{Symbol: &symbol, Side: proapi.Side(prservice.SideBorrowing)}) + handler.GetPatronRequestsIdActions(rr, req, "3", proapi.GetPatronRequestsIdActionsParams{Symbol: &symbol, Side: &proapiBorrowingSide}) assert.Equal(t, http.StatusOK, rr.Code) assert.Contains(t, rr.Body.String(), "validate") } @@ -205,7 +208,7 @@ func TestGetPatronRequestsIdActionsNoSymbol(t *testing.T) { handler := NewPrApiHandler(new(PrRepoError), mockEventBus, common.NewTenant(""), 10) req, _ := http.NewRequest("GET", "/", nil) rr := httptest.NewRecorder() - handler.GetPatronRequestsIdActions(rr, req, "3", proapi.GetPatronRequestsIdActionsParams{Side: proapi.Side(prservice.SideBorrowing)}) + handler.GetPatronRequestsIdActions(rr, req, "3", proapi.GetPatronRequestsIdActionsParams{Side: &proapiBorrowingSide}) assert.Equal(t, http.StatusBadRequest, rr.Code) assert.Contains(t, rr.Body.String(), "symbol must be specified") } @@ -214,7 +217,7 @@ func TestGetPatronRequestsIdActionsDbError(t *testing.T) { handler := NewPrApiHandler(new(PrRepoError), mockEventBus, common.NewTenant(""), 10) req, _ := http.NewRequest("GET", "/", nil) rr := httptest.NewRecorder() - handler.GetPatronRequestsIdActions(rr, req, "1", proapi.GetPatronRequestsIdActionsParams{Symbol: &symbol, Side: proapi.Side(prservice.SideBorrowing)}) + handler.GetPatronRequestsIdActions(rr, req, "1", proapi.GetPatronRequestsIdActionsParams{Symbol: &symbol, Side: &proapiBorrowingSide}) assert.Equal(t, http.StatusInternalServerError, rr.Code) assert.Contains(t, rr.Body.String(), "DB error") } @@ -223,7 +226,7 @@ func TestGetPatronRequestsIdActionsNotFoundBecauseOfSide(t *testing.T) { handler := NewPrApiHandler(new(PrRepoError), mockEventBus, common.NewTenant(""), 10) req, _ := http.NewRequest("GET", "/", nil) rr := httptest.NewRecorder() - handler.GetPatronRequestsIdActions(rr, req, "3", proapi.GetPatronRequestsIdActionsParams{Symbol: &symbol, Side: proapi.Side(prservice.SideLending)}) + handler.GetPatronRequestsIdActions(rr, req, "3", proapi.GetPatronRequestsIdActionsParams{Symbol: &symbol, Side: &proapiLendingSide}) assert.Equal(t, http.StatusNotFound, rr.Code) assert.Contains(t, rr.Body.String(), "not found") } @@ -232,7 +235,7 @@ func TestPostPatronRequestsIdActionNoSymbol(t *testing.T) { handler := NewPrApiHandler(new(PrRepoError), mockEventBus, common.NewTenant(""), 10) req, _ := http.NewRequest("GET", "/", nil) rr := httptest.NewRecorder() - handler.PostPatronRequestsIdAction(rr, req, "3", proapi.PostPatronRequestsIdActionParams{Side: proapi.Side(prservice.SideBorrowing)}) + handler.PostPatronRequestsIdAction(rr, req, "3", proapi.PostPatronRequestsIdActionParams{Side: &proapiBorrowingSide}) assert.Equal(t, http.StatusBadRequest, rr.Code) assert.Contains(t, rr.Body.String(), "symbol must be specified") } @@ -241,7 +244,7 @@ func TestPostPatronRequestsIdActionDbError(t *testing.T) { handler := NewPrApiHandler(new(PrRepoError), mockEventBus, common.NewTenant(""), 10) req, _ := http.NewRequest("GET", "/", nil) rr := httptest.NewRecorder() - handler.PostPatronRequestsIdAction(rr, req, "1", proapi.PostPatronRequestsIdActionParams{Symbol: &symbol, Side: proapi.Side(prservice.SideBorrowing)}) + handler.PostPatronRequestsIdAction(rr, req, "1", proapi.PostPatronRequestsIdActionParams{Symbol: &symbol, Side: &proapiBorrowingSide}) assert.Equal(t, http.StatusInternalServerError, rr.Code) assert.Contains(t, rr.Body.String(), "DB error") } @@ -250,7 +253,7 @@ func TestPostPatronRequestsIdActionNotFoundBecauseOfSide(t *testing.T) { handler := NewPrApiHandler(new(PrRepoError), mockEventBus, common.NewTenant(""), 10) req, _ := http.NewRequest("GET", "/", nil) rr := httptest.NewRecorder() - handler.PostPatronRequestsIdAction(rr, req, "3", proapi.PostPatronRequestsIdActionParams{Symbol: &symbol, Side: proapi.Side(prservice.SideLending)}) + handler.PostPatronRequestsIdAction(rr, req, "3", proapi.PostPatronRequestsIdActionParams{Symbol: &symbol, Side: &proapiLendingSide}) assert.Equal(t, http.StatusNotFound, rr.Code) assert.Contains(t, rr.Body.String(), "not found") } @@ -259,7 +262,7 @@ func TestPostPatronRequestsIdActionErrorParsing(t *testing.T) { handler := NewPrApiHandler(new(PrRepoError), mockEventBus, common.NewTenant(""), 10) req, _ := http.NewRequest("GET", "/", strings.NewReader("{")) rr := httptest.NewRecorder() - handler.PostPatronRequestsIdAction(rr, req, "3", proapi.PostPatronRequestsIdActionParams{Symbol: &symbol, Side: proapi.Side(prservice.SideBorrowing)}) + handler.PostPatronRequestsIdAction(rr, req, "3", proapi.PostPatronRequestsIdActionParams{Symbol: &symbol, Side: &proapiBorrowingSide}) assert.Equal(t, http.StatusBadRequest, rr.Code) assert.Contains(t, rr.Body.String(), "unexpected EOF") } diff --git a/broker/test/patron_request/api/api-handler_test.go b/broker/test/patron_request/api/api-handler_test.go index c3d32230..269e54c7 100644 --- a/broker/test/patron_request/api/api-handler_test.go +++ b/broker/test/patron_request/api/api-handler_test.go @@ -8,6 +8,7 @@ import ( "net/http" "os" "strconv" + "strings" "testing" "time" @@ -123,13 +124,27 @@ func TestCrud(t *testing.T) { assert.Equal(t, int64(1), foundPrs.About.Count) assert.Equal(t, *newPr.Id, foundPrs.Items[0].Id) - // GET by id + // GET list with offset in + respBytes = httpRequest(t, "GET", basePath+queryParams+"&offset=100000", []byte{}, 200) + err = json.Unmarshal(respBytes, &foundPrs) + assert.NoError(t, err, "failed to unmarshal patron request") + + assert.Equal(t, int64(1), foundPrs.About.Count) + assert.Len(t, foundPrs.Items, 0) + + // GET by id with symbol and side thisPrPath := basePath + "/" + *newPr.Id respBytes = httpRequest(t, "GET", thisPrPath+queryParams, []byte{}, 200) err = json.Unmarshal(respBytes, &foundPr) assert.NoError(t, err, "failed to unmarshal patron request") assert.Equal(t, *newPr.Id, foundPr.Id) + // GET by id with symbol + respBytes = httpRequest(t, "GET", thisPrPath+"?symbol="+*foundPr.RequesterSymbol, []byte{}, 200) + err = json.Unmarshal(respBytes, &foundPr) + assert.NoError(t, err, "failed to unmarshal patron request") + assert.Equal(t, *newPr.Id, foundPr.Id) + // GET actions by PR id test.WaitForPredicateToBeTrue(func() bool { respBytes = httpRequest(t, "GET", thisPrPath+"/actions"+queryParams, []byte{}, 200) @@ -187,9 +202,7 @@ func TestActionsToCompleteState(t *testing.T) { SupplierUniqueRecordId: "return-" + supplierSymbol + "::WILLSUPPLY_LOANED", }, } - id := uuid.NewString() newPr := proapi.CreatePatronRequest{ - Id: &id, SupplierSymbol: &supplierSymbol, RequesterSymbol: &requesterSymbol, Patron: &patron, @@ -204,8 +217,8 @@ func TestActionsToCompleteState(t *testing.T) { err = json.Unmarshal(respBytes, &foundPr) assert.NoError(t, err, "failed to unmarshal patron request") - assert.Equal(t, *newPr.Id, foundPr.Id) - requesterPrPath := basePath + "/" + *newPr.Id + assert.Equal(t, strings.ToUpper(strings.Split(requesterSymbol, ":")[1]+"-1"), foundPr.Id) + requesterPrPath := basePath + "/" + foundPr.Id queryParams := "?side=borrowing&symbol=" + *foundPr.RequesterSymbol // Wait till action available @@ -224,10 +237,10 @@ func TestActionsToCompleteState(t *testing.T) { // Find supplier patron request test.WaitForPredicateToBeTrue(func() bool { - supPr, _ := prRepo.GetPatronRequestBySupplierSymbolAndRequesterReqId(appCtx, supplierSymbol, *newPr.Id) + supPr, _ := prRepo.GetPatronRequestBySupplierSymbolAndRequesterReqId(appCtx, supplierSymbol, foundPr.Id) return supPr.ID != "" }) - supPr, err := prRepo.GetPatronRequestBySupplierSymbolAndRequesterReqId(appCtx, supplierSymbol, *newPr.Id) + supPr, err := prRepo.GetPatronRequestBySupplierSymbolAndRequesterReqId(appCtx, supplierSymbol, foundPr.Id) assert.NoError(t, err) assert.NotNil(t, supPr.ID) @@ -342,7 +355,6 @@ func TestActionsToCompleteState(t *testing.T) { respBytes = httpRequest(t, "GET", requesterPrPath+queryParams, []byte{}, 200) err = json.Unmarshal(respBytes, &foundPr) assert.NoError(t, err, "failed to unmarshal patron request") - assert.Equal(t, *newPr.Id, foundPr.Id) assert.Equal(t, string(prservice.BorrowerStateCompleted), foundPr.State) // Check supplier patron request done From 42e26f09a70c5305295a8f5c32293d83a83e95fc Mon Sep 17 00:00:00 2001 From: Janis Saldabols Date: Thu, 5 Feb 2026 13:32:25 +0200 Subject: [PATCH 2/2] CROSSLINK-208 Make side as optional parameter --- broker/patron_request/api/api-handler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/broker/patron_request/api/api-handler.go b/broker/patron_request/api/api-handler.go index ebad1fa0..9891e347 100644 --- a/broker/patron_request/api/api-handler.go +++ b/broker/patron_request/api/api-handler.go @@ -245,7 +245,7 @@ func (a *PatronRequestApiHandler) getPatronRequestById(ctx common.ExtendedContex } return nil, err } - if isOwner(pr, symbol) && (!isSideParamValid(side) || (side != nil && string(pr.Side) == *side)) { + if isOwner(pr, symbol) && (!isSideParamValid(side) || string(pr.Side) == *side) { return &pr, nil } return nil, nil