From de60824b58bd06c220f203870257bb38d29a5ffa Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Fri, 12 Dec 2025 15:59:31 +0100 Subject: [PATCH 01/12] WIP-190 NCIP requester side --- broker/api/api-handler.go | 20 +- broker/app/app.go | 13 +- broker/common/tenant_to_symbol.go | 21 ++ broker/lms/lms_adapter.go | 42 +++ broker/lms/lms_adapter_mock.go | 61 ++++ broker/lms/lms_adapter_ncip.go | 138 +++++++++ broker/lms/lms_adapter_test.go | 192 ++++++++++++ broker/lms/lms_creator.go | 12 + broker/lms/lms_creator_impl.go | 43 +++ broker/lms/lms_creator_test.go | 73 +++++ broker/ncipclient/ncipclient.go | 34 +-- broker/ncipclient/ncipclient_impl.go | 215 ++++++-------- broker/ncipclient/ncipclient_test.go | 266 ++++++----------- broker/patron_request/api/api-handler.go | 33 ++- broker/patron_request/api/api-handler_test.go | 16 +- broker/patron_request/service/action.go | 70 ++++- broker/patron_request/service/action_test.go | 277 ++++++++++++++++-- .../service/message-handler_test.go | 6 + broker/test/api/api-handler_test.go | 2 +- .../patron_request/api/api-handler_test.go | 4 +- 20 files changed, 1131 insertions(+), 407 deletions(-) create mode 100644 broker/common/tenant_to_symbol.go create mode 100644 broker/lms/lms_adapter.go create mode 100644 broker/lms/lms_adapter_mock.go create mode 100644 broker/lms/lms_adapter_ncip.go create mode 100644 broker/lms/lms_adapter_test.go create mode 100644 broker/lms/lms_creator.go create mode 100644 broker/lms/lms_creator_impl.go create mode 100644 broker/lms/lms_creator_test.go diff --git a/broker/api/api-handler.go b/broker/api/api-handler.go index 20dc0851..529629a4 100644 --- a/broker/api/api-handler.go +++ b/broker/api/api-handler.go @@ -41,10 +41,10 @@ type ApiHandler struct { limitDefault int32 eventRepo events.EventRepo illRepo ill_db.IllRepo - tenantToSymbol string // non-empty if in /broker mode + tenantToSymbol common.TenantToSymbol } -func NewApiHandler(eventRepo events.EventRepo, illRepo ill_db.IllRepo, tenentToSymbol string, limitDefault int32) ApiHandler { +func NewApiHandler(eventRepo events.EventRepo, illRepo ill_db.IllRepo, tenentToSymbol common.TenantToSymbol, limitDefault int32) ApiHandler { return ApiHandler{ eventRepo: eventRepo, illRepo: illRepo, @@ -53,25 +53,17 @@ func NewApiHandler(eventRepo events.EventRepo, illRepo ill_db.IllRepo, tenentToS } } -func (a *ApiHandler) isTenantMode() bool { - return a.tenantToSymbol != "" -} - -func (a *ApiHandler) getSymbolFromTenant(tenant string) string { - return strings.ReplaceAll(a.tenantToSymbol, "{tenant}", strings.ToUpper(tenant)) -} - func (a *ApiHandler) isOwner(trans *ill_db.IllTransaction, tenant *string, requesterSymbol *string) bool { if tenant == nil && requesterSymbol != nil { return trans.RequesterSymbol.String == *requesterSymbol } - if !a.isTenantMode() { + if !a.tenantToSymbol.TenantMode() { return true } if tenant == nil { return false } - return trans.RequesterSymbol.String == a.getSymbolFromTenant(*tenant) + return trans.RequesterSymbol.String == a.tenantToSymbol.GetSymbolFromTenant(*tenant) } func (a *ApiHandler) getIllTranFromParams(ctx common.ExtendedContext, w http.ResponseWriter, @@ -176,10 +168,10 @@ func (a *ApiHandler) GetIllTransactions(w http.ResponseWriter, r *http.Request, fullCount = 1 resp.Items = append(resp.Items, toApiIllTransaction(r, *tran)) } - } else if a.isTenantMode() { + } else if a.tenantToSymbol.TenantMode() { var tenantSymbol string if params.XOkapiTenant != nil { - tenantSymbol = a.getSymbolFromTenant(*params.XOkapiTenant) + tenantSymbol = a.tenantToSymbol.GetSymbolFromTenant(*params.XOkapiTenant) } else if params.RequesterSymbol != nil { tenantSymbol = *params.RequesterSymbol } diff --git a/broker/app/app.go b/broker/app/app.go index 42fd3ec4..f5b559cc 100644 --- a/broker/app/app.go +++ b/broker/app/app.go @@ -36,6 +36,8 @@ import ( "github.com/indexdata/crosslink/broker/ill_db" "github.com/indexdata/go-utils/utils" "github.com/jackc/pgx/v5/pgxpool" + + "github.com/indexdata/crosslink/broker/lms" ) var HTTP_PORT = utils.Must(utils.GetEnvInt("HTTP_PORT", 8081)) @@ -157,8 +159,9 @@ func Init(ctx context.Context) (Context, error) { iso18626Handler := handler.CreateIso18626Handler(eventBus, eventRepo, illRepo, dirAdapter) supplierLocator := service.CreateSupplierLocator(eventBus, illRepo, dirAdapter, holdingsAdapter) workflowManager := service.CreateWorkflowManager(eventBus, illRepo, service.WorkflowConfig{}) - prActionService := prservice.CreatePatronRequestActionService(prRepo, illRepo, eventBus, &iso18626Handler) - prApiHandler := prapi.NewApiHandler(prRepo, eventBus) + lmsCreator := lms.NewLmsCreator(illRepo, dirAdapter) + prActionService := prservice.CreatePatronRequestActionService(prRepo, eventBus, &iso18626Handler, lmsCreator) + prApiHandler := prapi.NewApiHandler(prRepo, eventBus, common.NewTenantToSymbol(TENANT_TO_SYMBOL)) AddDefaultHandlers(eventBus, iso18626Client, supplierLocator, workflowManager, iso18626Handler, prActionService, prApiHandler, prMessageHandler) err = StartEventBus(ctx, eventBus) @@ -196,14 +199,16 @@ func StartServer(ctx Context) error { http.ServeFile(w, r, "handler/open-api.yaml") }) - apiHandler := api.NewApiHandler(ctx.EventRepo, ctx.IllRepo, "", API_PAGE_SIZE) + apiHandler := api.NewApiHandler(ctx.EventRepo, ctx.IllRepo, common.NewTenantToSymbol(""), API_PAGE_SIZE) oapi.HandlerFromMux(&apiHandler, ServeMux) if TENANT_TO_SYMBOL != "" { - apiHandler := api.NewApiHandler(ctx.EventRepo, ctx.IllRepo, TENANT_TO_SYMBOL, API_PAGE_SIZE) + apiHandler := api.NewApiHandler(ctx.EventRepo, ctx.IllRepo, common.NewTenantToSymbol(TENANT_TO_SYMBOL), API_PAGE_SIZE) oapi.HandlerFromMuxWithBaseURL(&apiHandler, ServeMux, "/broker") } proapi.HandlerFromMux(&ctx.PrApiHandler, ServeMux) + // TODO: proapi.HandlerFromMuxWithBaseURL(&ctx.PrApiHandler, ServeMux, "/broker") + signatureHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Server", vcs.GetSignature()) ServeMux.ServeHTTP(w, r) diff --git a/broker/common/tenant_to_symbol.go b/broker/common/tenant_to_symbol.go new file mode 100644 index 00000000..50c0a6f7 --- /dev/null +++ b/broker/common/tenant_to_symbol.go @@ -0,0 +1,21 @@ +package common + +import ( + "strings" +) + +type TenantToSymbol struct { + mapping string +} + +func NewTenantToSymbol(tenantSymbol string) TenantToSymbol { + return TenantToSymbol{mapping: tenantSymbol} +} + +func (t *TenantToSymbol) TenantMode() bool { + return t.mapping != "" +} + +func (t *TenantToSymbol) GetSymbolFromTenant(tenant string) string { + return strings.ReplaceAll(t.mapping, "{tenant}", strings.ToUpper(tenant)) +} diff --git a/broker/lms/lms_adapter.go b/broker/lms/lms_adapter.go new file mode 100644 index 00000000..b62431ff --- /dev/null +++ b/broker/lms/lms_adapter.go @@ -0,0 +1,42 @@ +package lms + +// LmsAdapter is an interface defining methods for interacting with a Library Management System (LMS) +// https://github.com/openlibraryenvironment/mod-rs/blob/master/service/src/main/groovy/org/olf/rs/lms/HostLMSActions.groovy +type LmsAdapter interface { + LookupUser(patron string) (string, error) + + AcceptItem( + itemId string, + requestId string, + userId string, + author string, + title string, + isbn string, + callNumber string, + pickupLocation string, + requestedAction string, + ) error + + DeleteItem(itemId string) error + + RequestItem( + requestId string, + itemId string, + borrowerBarcode string, + pickupLocation string, + itemLocation string, + ) error + + CancelRequestItem(requestId string, userId string) error + + CheckInItem(itemId string) error + + CheckOutItem( + requestId string, + itemBarcode string, + borrowerBarcode string, + externalReferenceValue string, + ) error + + CreateUserFiscalTransaction(userId string, itemId string) error +} diff --git a/broker/lms/lms_adapter_mock.go b/broker/lms/lms_adapter_mock.go new file mode 100644 index 00000000..25ffecb3 --- /dev/null +++ b/broker/lms/lms_adapter_mock.go @@ -0,0 +1,61 @@ +package lms + +type LmsAdapterMock struct { +} + +func (l *LmsAdapterMock) LookupUser(patron string) (string, error) { + return patron, nil +} + +func (l *LmsAdapterMock) AcceptItem( + itemId string, + requestId string, + userId string, + author string, + title string, + isbn string, + callNumber string, + pickupLocation string, + requestedAction string, +) error { + return nil +} + +func (l *LmsAdapterMock) DeleteItem(itemId string) error { + return nil +} + +func (l *LmsAdapterMock) RequestItem( + requestId string, + itemId string, + borrowerBarcode string, + pickupLocation string, + itemLocation string, +) error { + return nil +} + +func (l *LmsAdapterMock) CancelRequestItem(requestId string, userId string) error { + return nil +} + +func (l *LmsAdapterMock) CheckInItem(itemId string) error { + return nil +} + +func (l *LmsAdapterMock) CheckOutItem( + requestId string, + itemBarcode string, + borrowerBarcode string, + externalReferenceValue string, +) error { + return nil +} + +func (l *LmsAdapterMock) CreateUserFiscalTransaction(userId string, itemId string) error { + return nil +} + +func CreateLmsAdapterMockOK() LmsAdapter { + return &LmsAdapterMock{} +} diff --git a/broker/lms/lms_adapter_ncip.go b/broker/lms/lms_adapter_ncip.go new file mode 100644 index 00000000..907076df --- /dev/null +++ b/broker/lms/lms_adapter_ncip.go @@ -0,0 +1,138 @@ +package lms + +import ( + "fmt" + "net/http" + + "github.com/indexdata/crosslink/broker/ncipclient" + "github.com/indexdata/crosslink/ncip" +) + +// NCIP LMS Adapter, based on: +// https://github.com/openlibraryenvironment/mod-rs/blob/master/service/grails-app/services/org/olf/rs/hostlms/BaseHostLMSService.groovy + +type LmsAdapterNcip struct { + ncipInfo map[string]any + ncipClient ncipclient.NcipClient +} + +func CreateLmsAdapterNcip(ncipInfo map[string]any) (LmsAdapter, error) { + nc := ncipclient.NewNcipClient(http.DefaultClient, ncipInfo) + return &LmsAdapterNcip{ + ncipInfo: ncipInfo, + ncipClient: nc, + }, nil +} + +func (l *LmsAdapterNcip) LookupUser(patron string) (string, error) { + if patron == "" { + return "", fmt.Errorf("empty patron identifier") + } + // first try to check if patron is actually user Id + arg := ncip.LookupUser{ + UserId: &ncip.UserId{UserIdentifierValue: patron}, + } + _, err := l.ncipClient.LookupUser(arg) + if err == nil { + return patron, nil + } + // then try by user user name + // a better solution would be that the LookupUser had type argument (eg barcode or PIN) + // but this is now mod-rs does it + var authenticationInput []ncip.AuthenticationInput + authenticationInput = append(authenticationInput, ncip.AuthenticationInput{ + AuthenticationInputType: ncip.SchemeValuePair{Text: "username"}, + AuthenticationInputData: patron, + }) + elements := []ncip.SchemeValuePair{{Text: "User Id"}} + arg = ncip.LookupUser{ + AuthenticationInput: authenticationInput, + UserElementType: elements, + } + response, err := l.ncipClient.LookupUser(arg) + if err != nil { + return "", err + } + if response.UserId == nil { + return "", fmt.Errorf("missing User ID in LookupUser response") + } + return response.UserId.UserIdentifierValue, nil +} + +func (l *LmsAdapterNcip) AcceptItem( + itemId string, + requestId string, + userId string, + author string, + title string, + isbn string, + callNumber string, + pickupLocation string, + requestedAction string, +) error { + arg := ncip.AcceptItem{ + UserId: &ncip.UserId{UserIdentifierValue: userId}, + ItemId: &ncip.ItemId{ItemIdentifierValue: itemId}, + } + _, err := l.ncipClient.AcceptItem(arg) + return err +} + +func (l *LmsAdapterNcip) DeleteItem(itemId string) error { + arg := ncip.DeleteItem{ + ItemId: ncip.ItemId{ItemIdentifierValue: itemId}, + } + _, err := l.ncipClient.DeleteItem(arg) + return err +} +func (l *LmsAdapterNcip) RequestItem( + requestId string, + itemId string, + borrowerBarcode string, + pickupLocation string, + itemLocation string, +) error { + arg := ncip.RequestItem{ + ItemId: []ncip.ItemId{{ItemIdentifierValue: itemId}}, + } + _, err := l.ncipClient.RequestItem(arg) + return err +} + +func (l *LmsAdapterNcip) CancelRequestItem(requestId string, userId string) error { + arg := ncip.CancelRequestItem{ + UserId: &ncip.UserId{UserIdentifierValue: userId}, + RequestId: &ncip.RequestId{RequestIdentifierValue: requestId}, + } + _, err := l.ncipClient.CancelRequestItem(arg) + return err +} + +func (l *LmsAdapterNcip) CheckInItem(itemId string) error { + arg := ncip.CheckInItem{ + ItemId: ncip.ItemId{ItemIdentifierValue: itemId}, + } + _, err := l.ncipClient.CheckInItem(arg) + return err +} + +func (l *LmsAdapterNcip) CheckOutItem( + requestId string, + itemBarcode string, + borrowerBarcode string, + externalReferenceValue string, +) error { + arg := ncip.CheckOutItem{ + RequestId: &ncip.RequestId{RequestIdentifierValue: requestId}, + } + _, err := l.ncipClient.CheckOutItem(arg) + return err +} + +func (l *LmsAdapterNcip) CreateUserFiscalTransaction(userId string, itemId string) error { + arg := ncip.CreateUserFiscalTransaction{ + UserId: &ncip.UserId{UserIdentifierValue: userId}, + } + _, err := l.ncipClient.CreateUserFiscalTransaction(arg) + return err +} diff --git a/broker/lms/lms_adapter_test.go b/broker/lms/lms_adapter_test.go new file mode 100644 index 00000000..c7a9e0b3 --- /dev/null +++ b/broker/lms/lms_adapter_test.go @@ -0,0 +1,192 @@ +package lms + +import ( + "fmt" + "strings" + "testing" + + "github.com/indexdata/crosslink/broker/ncipclient" + "github.com/indexdata/crosslink/ncip" + "github.com/stretchr/testify/assert" +) + +func TestLookupUser(t *testing.T) { + var mock ncipclient.NcipClient = new(ncipClientMock) + ad := &LmsAdapterNcip{ + ncipInfo: map[string]any{}, + ncipClient: mock, + } + _, err := ad.LookupUser("") + assert.Error(t, err) + assert.Equal(t, "empty patron identifier", err.Error()) + + userId, err := ad.LookupUser("testuser") + assert.NoError(t, err) + assert.Equal(t, "testuser", userId) + + _, err = ad.LookupUser("bad user") + assert.Error(t, err) + assert.Equal(t, "unknown user name", err.Error()) + + userId, err = ad.LookupUser("pass") + assert.NoError(t, err) + assert.Equal(t, "pass", userId) + + _, err = ad.LookupUser("missing data") + assert.Error(t, err) + assert.Equal(t, "missing User ID in LookupUser response", err.Error()) + + userId, err = ad.LookupUser("good user") + assert.NoError(t, err) + assert.Equal(t, "user123", userId) +} + +func TestAcceptItem(t *testing.T) { + var mock ncipclient.NcipClient = new(ncipClientMock) + ad := &LmsAdapterNcip{ + ncipInfo: map[string]any{}, + ncipClient: mock, + } + err := ad.AcceptItem("item1", "req1", "testuser", "author", "title", "isbn", "callnum", "loc", "action") + assert.NoError(t, err) + req := mock.(*ncipClientMock).lastRequest.(ncip.AcceptItem) + assert.Equal(t, "testuser", req.UserId.UserIdentifierValue) + assert.Equal(t, "item1", req.ItemId.ItemIdentifierValue) +} + +func TestDeleteItem(t *testing.T) { + var mock ncipclient.NcipClient = new(ncipClientMock) + ad := &LmsAdapterNcip{ + ncipInfo: map[string]any{}, + ncipClient: mock, + } + err := ad.DeleteItem("item1") + assert.NoError(t, err) + req := mock.(*ncipClientMock).lastRequest.(ncip.DeleteItem) + assert.Equal(t, "item1", req.ItemId.ItemIdentifierValue) +} + +func TestRequestItem(t *testing.T) { + var mock ncipclient.NcipClient = new(ncipClientMock) + ad := &LmsAdapterNcip{ + ncipInfo: map[string]any{}, + ncipClient: mock, + } + err := ad.RequestItem("req1", "item1", "testuser", "loc", "itemloc") + assert.NoError(t, err) + req := mock.(*ncipClientMock).lastRequest.(ncip.RequestItem) + assert.Equal(t, "item1", req.ItemId[0].ItemIdentifierValue) +} + +func TestCancelRequestItem(t *testing.T) { + var mock ncipclient.NcipClient = new(ncipClientMock) + ad := &LmsAdapterNcip{ + ncipInfo: map[string]any{}, + ncipClient: mock, + } + err := ad.CancelRequestItem("req1", "testuser") + assert.NoError(t, err) + req := mock.(*ncipClientMock).lastRequest.(ncip.CancelRequestItem) + assert.Equal(t, "testuser", req.UserId.UserIdentifierValue) + assert.Equal(t, "req1", req.RequestId.RequestIdentifierValue) +} + +func TestCheckInItem(t *testing.T) { + var mock ncipclient.NcipClient = new(ncipClientMock) + ad := &LmsAdapterNcip{ + ncipInfo: map[string]any{}, + ncipClient: mock, + } + err := ad.CheckInItem("item1") + assert.NoError(t, err) + req := mock.(*ncipClientMock).lastRequest.(ncip.CheckInItem) + assert.Equal(t, "item1", req.ItemId.ItemIdentifierValue) +} + +func TestCheckOutItem(t *testing.T) { + var mock ncipclient.NcipClient = new(ncipClientMock) + ad := &LmsAdapterNcip{ + ncipInfo: map[string]any{}, + ncipClient: mock, + } + err := ad.CheckOutItem("req1", "item1", "barcodeid", "extref") + assert.NoError(t, err) + req := mock.(*ncipClientMock).lastRequest.(ncip.CheckOutItem) + assert.Equal(t, "req1", req.RequestId.RequestIdentifierValue) +} + +func TestCreateUserFiscalTransaction(t *testing.T) { + var mock ncipclient.NcipClient = new(ncipClientMock) + ad := &LmsAdapterNcip{ + ncipInfo: map[string]any{}, + ncipClient: mock, + } + err := ad.CreateUserFiscalTransaction("testuser", "item1") + assert.NoError(t, err) + req := mock.(*ncipClientMock).lastRequest.(ncip.CreateUserFiscalTransaction) + assert.Equal(t, "testuser", req.UserId.UserIdentifierValue) +} + +type ncipClientMock struct { + lastRequest any + LookupUserFunc func(lookup ncip.LookupUser) (*ncip.LookupUserResponse, error) +} + +func (n *ncipClientMock) LookupUser(lookup ncip.LookupUser) (*ncip.LookupUserResponse, error) { + n.lastRequest = lookup + if lookup.UserId != nil { + if lookup.UserId.UserIdentifierValue == "pass" { + return nil, nil + } + if strings.Contains(lookup.UserId.UserIdentifierValue, " ") { + return nil, fmt.Errorf("unknown user id") + } + return &ncip.LookupUserResponse{ + UserId: &ncip.UserId{UserIdentifierValue: lookup.UserId.UserIdentifierValue}, + }, nil + } + if lookup.AuthenticationInput[0].AuthenticationInputData == "bad user" { + return nil, fmt.Errorf("unknown user name") + } + if lookup.AuthenticationInput[0].AuthenticationInputData == "missing data" { + return &ncip.LookupUserResponse{}, nil + } + return &ncip.LookupUserResponse{ + UserId: &ncip.UserId{UserIdentifierValue: "user123"}, + }, nil +} + +func (n *ncipClientMock) AcceptItem(accept ncip.AcceptItem) (*ncip.AcceptItemResponse, error) { + n.lastRequest = accept + return nil, nil +} + +func (n *ncipClientMock) DeleteItem(delete ncip.DeleteItem) (*ncip.DeleteItemResponse, error) { + n.lastRequest = delete + return nil, nil +} + +func (n *ncipClientMock) RequestItem(request ncip.RequestItem) (*ncip.RequestItemResponse, error) { + n.lastRequest = request + return nil, nil +} + +func (n *ncipClientMock) CancelRequestItem(cancel ncip.CancelRequestItem) (*ncip.CancelRequestItemResponse, error) { + n.lastRequest = cancel + return nil, nil +} + +func (n *ncipClientMock) CheckInItem(checkin ncip.CheckInItem) (*ncip.CheckInItemResponse, error) { + n.lastRequest = checkin + return nil, nil +} + +func (n *ncipClientMock) CheckOutItem(checkout ncip.CheckOutItem) (*ncip.CheckOutItemResponse, error) { + n.lastRequest = checkout + return nil, nil +} + +func (n *ncipClientMock) CreateUserFiscalTransaction(create ncip.CreateUserFiscalTransaction) (*ncip.CreateUserFiscalTransactionResponse, error) { + n.lastRequest = create + return nil, nil +} diff --git a/broker/lms/lms_creator.go b/broker/lms/lms_creator.go new file mode 100644 index 00000000..8d4c8ac6 --- /dev/null +++ b/broker/lms/lms_creator.go @@ -0,0 +1,12 @@ +package lms + +import ( + "github.com/indexdata/crosslink/broker/common" + "github.com/jackc/pgx/v5/pgtype" +) + +// LmsCreator is an interface for creating LMS adapters based on a symbol +// Symbol is used to look up directory entry with information about LMS in use +type LmsCreator interface { + GetAdapter(ctx common.ExtendedContext, symbol pgtype.Text) (LmsAdapter, error) +} diff --git a/broker/lms/lms_creator_impl.go b/broker/lms/lms_creator_impl.go new file mode 100644 index 00000000..0adde854 --- /dev/null +++ b/broker/lms/lms_creator_impl.go @@ -0,0 +1,43 @@ +package lms + +import ( + "errors" + + "github.com/indexdata/crosslink/broker/adapter" + "github.com/indexdata/crosslink/broker/common" + "github.com/indexdata/crosslink/broker/ill_db" + "github.com/jackc/pgx/v5/pgtype" +) + +type lmsCreatorImpl struct { + illRepo ill_db.IllRepo + directoryLookupAdapter adapter.DirectoryLookupAdapter +} + +func NewLmsCreator(illRepo ill_db.IllRepo, directoryLookupAdapter adapter.DirectoryLookupAdapter) LmsCreator { + return &lmsCreatorImpl{ + illRepo: illRepo, + directoryLookupAdapter: directoryLookupAdapter, + } +} + +func (l *lmsCreatorImpl) GetAdapter(ctx common.ExtendedContext, symbol pgtype.Text) (LmsAdapter, error) { + if !symbol.Valid { + return nil, errors.New("missing requester symbol") + } + return l.getAdapterInt(ctx, symbol.String) +} + +func (l *lmsCreatorImpl) getAdapterInt(ctx common.ExtendedContext, symbol string) (LmsAdapter, error) { + peers, _, err := l.illRepo.GetCachedPeersBySymbols(ctx, []string{symbol}, l.directoryLookupAdapter) + if err != nil { + return nil, err + } + for _, peer := range peers { + ncipInfo, ok := peer.CustomData["ncip"].(map[string]any) + if ok { + return CreateLmsAdapterNcip(ncipInfo) + } + } + return CreateLmsAdapterMockOK(), nil +} diff --git a/broker/lms/lms_creator_test.go b/broker/lms/lms_creator_test.go new file mode 100644 index 00000000..86d10606 --- /dev/null +++ b/broker/lms/lms_creator_test.go @@ -0,0 +1,73 @@ +package lms + +import ( + "context" + "testing" + + "github.com/indexdata/crosslink/broker/adapter" + "github.com/indexdata/crosslink/broker/common" + "github.com/indexdata/crosslink/broker/ill_db" + "github.com/jackc/pgx/v5/pgtype" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" +) + +func TestGetAdapterMissingSymbol(t *testing.T) { + illRepo := &MockIllRepo{} + creator := NewLmsCreator(illRepo, nil) + ctx := common.CreateExtCtxWithArgs(context.Background(), nil) + var missingSymbol pgtype.Text + _, err := creator.GetAdapter(ctx, missingSymbol) + assert.Error(t, err) + assert.Equal(t, "missing requester symbol", err.Error()) +} + +func TestGetAdapterGetCachedByPeersByPeersFail(t *testing.T) { + illRepo := &MockIllRepo{} + illRepo.On("GetCachedPeersBySymbols", mock.Anything).Return([]ill_db.Peer{}, "", assert.AnError) + creator := NewLmsCreator(illRepo, nil) + ctx := common.CreateExtCtxWithArgs(context.Background(), nil) + symbol := pgtype.Text{String: "TEST", Valid: true} + _, err := creator.GetAdapter(ctx, symbol) + assert.Error(t, err) + assert.Equal(t, "assert.AnError general error for testing", err.Error()) +} + +func TestGetAdapterNoPeers(t *testing.T) { + illRepo := &MockIllRepo{} + illRepo.On("GetCachedPeersBySymbols", mock.Anything).Return([]ill_db.Peer{}, "", nil) + creator := NewLmsCreator(illRepo, nil) + ctx := common.CreateExtCtxWithArgs(context.Background(), nil) + symbol := pgtype.Text{String: "TEST", Valid: true} + LmsAdapter, err := creator.GetAdapter(ctx, symbol) + assert.NoError(t, err) + assert.IsType(t, &LmsAdapterMock{}, LmsAdapter) +} + +func TestGetAdapterNcip(t *testing.T) { + illRepo := &MockIllRepo{} + peer := ill_db.Peer{ + CustomData: map[string]any{ + "ncip": map[string]any{ + "some_key": "some_value", + }, + }, + } + illRepo.On("GetCachedPeersBySymbols", mock.Anything).Return([]ill_db.Peer{peer}, "", nil) + creator := NewLmsCreator(illRepo, nil) + ctx := common.CreateExtCtxWithArgs(context.Background(), nil) + symbol := pgtype.Text{String: "TEST", Valid: true} + LmsAdapter, err := creator.GetAdapter(ctx, symbol) + assert.NoError(t, err) + assert.IsType(t, &LmsAdapterNcip{}, LmsAdapter) +} + +type MockIllRepo struct { + mock.Mock + ill_db.PgIllRepo +} + +func (r *MockIllRepo) GetCachedPeersBySymbols(ctx common.ExtendedContext, symbols []string, directoryLookupAdapter adapter.DirectoryLookupAdapter) ([]ill_db.Peer, string, error) { + args := r.Called(symbols) + return args.Get(0).([]ill_db.Peer), args.String(1), args.Error(2) +} diff --git a/broker/ncipclient/ncipclient.go b/broker/ncipclient/ncipclient.go index 2df7e2f1..7dc2cdd0 100644 --- a/broker/ncipclient/ncipclient.go +++ b/broker/ncipclient/ncipclient.go @@ -24,40 +24,22 @@ const ( CreateUserFiscalTransactionMode NcipProperty = "create_user_fiscal_transaction_mode" ) -// NcipClient defines the interface for NCIP operations -// customData is from the DirectoryEntry.CustomData field type NcipClient interface { - // LookupUser performs user authentication. - // Returns true if authentication is successful (disabled or auto and NCIP lookup succeeded) - // Returns false if authentication is manual - // Returns an error otherwise (failed NCIP lookup, misconfiguration, etc) - LookupUser(customData map[string]any, arg ncip.LookupUser) (bool, error) + LookupUser(arg ncip.LookupUser) (*ncip.LookupUserResponse, error) - // AcceptItem accepts an item. - // Returns true if accept is successful (disabled or auto and NCIP accept succeeded) - // Returns false if accept is manual - // Returns an error otherwise (failed NCIP accept, misconfiguration, etc) - AcceptItem(customData map[string]any, arg ncip.AcceptItem) (bool, error) + AcceptItem(arg ncip.AcceptItem) (*ncip.AcceptItemResponse, error) - DeleteItem(customData map[string]any, arg ncip.DeleteItem) error + DeleteItem(arg ncip.DeleteItem) (*ncip.DeleteItemResponse, error) - // RequestItem requests an item. - // Returns true if request is successful (disabled or auto and NCIP request succeeded) - // Returns false if request is manual - // Returns an error otherwise (failed NCIP request, misconfiguration, etc) - RequestItem(customData map[string]any, arg ncip.RequestItem) (bool, error) + RequestItem(arg ncip.RequestItem) (*ncip.RequestItemResponse, error) - CancelRequestItem(customData map[string]any, arg ncip.CancelRequestItem) error + CancelRequestItem(arg ncip.CancelRequestItem) (*ncip.CancelRequestItemResponse, error) - CheckInItem(customData map[string]any, arg ncip.CheckInItem) error + CheckInItem(arg ncip.CheckInItem) (*ncip.CheckInItemResponse, error) - CheckOutItem(customData map[string]any, arg ncip.CheckOutItem) error + CheckOutItem(arg ncip.CheckOutItem) (*ncip.CheckOutItemResponse, error) - // CreateUserFiscalTransaction creates a user fiscal transaction. - // Returns true if creation is successful (disabled or auto and NCIP creation succeeded) - // Returns false if creation is manual - // Returns an error otherwise (failed NCIP creation, misconfiguration, etc) - CreateUserFiscalTransaction(customData map[string]any, arg ncip.CreateUserFiscalTransaction) (bool, error) + CreateUserFiscalTransaction(arg ncip.CreateUserFiscalTransaction) (*ncip.CreateUserFiscalTransactionResponse, error) } type NcipError struct { diff --git a/broker/ncipclient/ncipclient_impl.go b/broker/ncipclient/ncipclient_impl.go index ce08d476..78cf2366 100644 --- a/broker/ncipclient/ncipclient_impl.go +++ b/broker/ncipclient/ncipclient_impl.go @@ -10,191 +10,164 @@ import ( ) type NcipClientImpl struct { - client *http.Client + client *http.Client + ncipInfo map[string]any } -func CreateNcipClient(client *http.Client) NcipClient { +func NewNcipClient(client *http.Client, ncipInfo map[string]any) NcipClient { return &NcipClientImpl{ - client: client, + client: client, + ncipInfo: ncipInfo, } } -func (n *NcipClientImpl) LookupUser(customData map[string]any, lookup ncip.LookupUser) (bool, error) { - ncipInfo, err := n.getNcipInfo(customData) - if err != nil { - return false, err - } - handle, ret, err := n.checkMode(ncipInfo, LookupUserMode) +func (n *NcipClientImpl) LookupUser(lookup ncip.LookupUser) (*ncip.LookupUserResponse, error) { + handle, _, err := n.checkMode(LookupUserMode) if handle { - return ret, err + return nil, err + } + if lookup.UserId != nil && lookup.UserId.UserIdentifierValue == "" { + return nil, fmt.Errorf("empty user Id") } - lookup.InitiationHeader = n.prepareHeader(ncipInfo, lookup.InitiationHeader) + lookup.InitiationHeader = n.prepareHeader(lookup.InitiationHeader) ncipMessage := &ncip.NCIPMessage{ LookupUser: &lookup, } - ncipResponse, err := n.sendReceiveMessage(ncipInfo, ncipMessage) + ncipResponse, err := n.sendReceiveMessage(ncipMessage) if err != nil { - return false, err + return nil, err } - lookupUserResponse := ncipResponse.LookupUserResponse - if lookupUserResponse == nil { - return false, fmt.Errorf("invalid NCIP response: missing LookupUserResponse") + response := ncipResponse.LookupUserResponse + if response == nil { + return nil, fmt.Errorf("invalid NCIP response: missing LookupUserResponse") } - return true, n.checkProblem("NCIP user lookup", lookupUserResponse.Problem) + return response, n.checkProblem("NCIP user lookup", response.Problem) } -func (n *NcipClientImpl) AcceptItem(customData map[string]any, accept ncip.AcceptItem) (bool, error) { - ncipInfo, err := n.getNcipInfo(customData) - if err != nil { - return false, err - } - handle, ret, err := n.checkMode(ncipInfo, AcceptItemMode) +func (n *NcipClientImpl) AcceptItem(accept ncip.AcceptItem) (*ncip.AcceptItemResponse, error) { + handle, _, err := n.checkMode(AcceptItemMode) if handle { - return ret, err + return nil, err } - accept.InitiationHeader = n.prepareHeader(ncipInfo, accept.InitiationHeader) + accept.InitiationHeader = n.prepareHeader(accept.InitiationHeader) ncipMessage := &ncip.NCIPMessage{ AcceptItem: &accept, } - ncipResponse, err := n.sendReceiveMessage(ncipInfo, ncipMessage) + ncipResponse, err := n.sendReceiveMessage(ncipMessage) if err != nil { - return false, err + return nil, err } - acceptItemResponse := ncipResponse.AcceptItemResponse - if acceptItemResponse == nil { - return false, fmt.Errorf("invalid NCIP response: missing AcceptItemResponse") + response := ncipResponse.AcceptItemResponse + if response == nil { + return nil, fmt.Errorf("invalid NCIP response: missing AcceptItemResponse") } - return true, n.checkProblem("NCIP accept item", acceptItemResponse.Problem) + return response, n.checkProblem("NCIP accept item", response.Problem) } -func (n *NcipClientImpl) DeleteItem(customData map[string]any, delete ncip.DeleteItem) error { - ncipInfo, err := n.getNcipInfo(customData) - if err != nil { - return err - } - delete.InitiationHeader = n.prepareHeader(ncipInfo, delete.InitiationHeader) +func (n *NcipClientImpl) DeleteItem(delete ncip.DeleteItem) (*ncip.DeleteItemResponse, error) { + delete.InitiationHeader = n.prepareHeader(delete.InitiationHeader) ncipMessage := &ncip.NCIPMessage{ DeleteItem: &delete, } - ncipResponse, err := n.sendReceiveMessage(ncipInfo, ncipMessage) + ncipResponse, err := n.sendReceiveMessage(ncipMessage) if err != nil { - return err + return nil, err } - deleteItemResponse := ncipResponse.DeleteItemResponse - if deleteItemResponse == nil { - return fmt.Errorf("invalid NCIP response: missing DeleteItemResponse") + response := ncipResponse.DeleteItemResponse + if response == nil { + return nil, fmt.Errorf("invalid NCIP response: missing DeleteItemResponse") } - return n.checkProblem("NCIP delete item", deleteItemResponse.Problem) + return response, n.checkProblem("NCIP delete item", response.Problem) } -func (n *NcipClientImpl) RequestItem(customData map[string]any, request ncip.RequestItem) (bool, error) { - ncipInfo, err := n.getNcipInfo(customData) - if err != nil { - return false, err - } - handle, ret, err := n.checkMode(ncipInfo, RequestItemMode) +func (n *NcipClientImpl) RequestItem(request ncip.RequestItem) (*ncip.RequestItemResponse, error) { + handle, _, err := n.checkMode(RequestItemMode) if handle { - return ret, err + return nil, err } - request.InitiationHeader = n.prepareHeader(ncipInfo, request.InitiationHeader) + request.InitiationHeader = n.prepareHeader(request.InitiationHeader) ncipMessage := &ncip.NCIPMessage{ RequestItem: &request, } - ncipResponse, err := n.sendReceiveMessage(ncipInfo, ncipMessage) + ncipResponse, err := n.sendReceiveMessage(ncipMessage) if err != nil { - return false, err + return nil, err } - requestItemResponse := ncipResponse.RequestItemResponse - if requestItemResponse == nil { - return false, fmt.Errorf("invalid NCIP response: missing RequestItemResponse") + response := ncipResponse.RequestItemResponse + if response == nil { + return nil, fmt.Errorf("invalid NCIP response: missing RequestItemResponse") } - return true, n.checkProblem("NCIP request item", requestItemResponse.Problem) + return response, n.checkProblem("NCIP request item", response.Problem) } -func (n *NcipClientImpl) CancelRequestItem(customData map[string]any, request ncip.CancelRequestItem) error { - ncipInfo, err := n.getNcipInfo(customData) - if err != nil { - return err - } - request.InitiationHeader = n.prepareHeader(ncipInfo, request.InitiationHeader) +func (n *NcipClientImpl) CancelRequestItem(request ncip.CancelRequestItem) (*ncip.CancelRequestItemResponse, error) { + request.InitiationHeader = n.prepareHeader(request.InitiationHeader) ncipMessage := &ncip.NCIPMessage{ CancelRequestItem: &request, } - ncipResponse, err := n.sendReceiveMessage(ncipInfo, ncipMessage) + ncipResponse, err := n.sendReceiveMessage(ncipMessage) if err != nil { - return err + return nil, err } - cancelRequestItemResponse := ncipResponse.CancelRequestItemResponse - if cancelRequestItemResponse == nil { - return fmt.Errorf("invalid NCIP response: missing CancelRequestItemResponse") + response := ncipResponse.CancelRequestItemResponse + if response == nil { + return nil, fmt.Errorf("invalid NCIP response: missing CancelRequestItemResponse") } - return n.checkProblem("NCIP cancel request item", cancelRequestItemResponse.Problem) + return response, n.checkProblem("NCIP cancel request item", response.Problem) } -func (n *NcipClientImpl) CheckInItem(customData map[string]any, request ncip.CheckInItem) error { - ncipInfo, err := n.getNcipInfo(customData) - if err != nil { - return err - } - request.InitiationHeader = n.prepareHeader(ncipInfo, request.InitiationHeader) +func (n *NcipClientImpl) CheckInItem(request ncip.CheckInItem) (*ncip.CheckInItemResponse, error) { + request.InitiationHeader = n.prepareHeader(request.InitiationHeader) ncipMessage := &ncip.NCIPMessage{ CheckInItem: &request, } - ncipResponse, err := n.sendReceiveMessage(ncipInfo, ncipMessage) + ncipResponse, err := n.sendReceiveMessage(ncipMessage) if err != nil { - return err + return nil, err } - checkInItemResponse := ncipResponse.CheckInItemResponse - if checkInItemResponse == nil { - return fmt.Errorf("invalid NCIP response: missing CheckInItemResponse") + response := ncipResponse.CheckInItemResponse + if response == nil { + return nil, fmt.Errorf("invalid NCIP response: missing CheckInItemResponse") } - return n.checkProblem("NCIP check in item", checkInItemResponse.Problem) + return response, n.checkProblem("NCIP check in item", response.Problem) } -func (n *NcipClientImpl) CheckOutItem(customData map[string]any, request ncip.CheckOutItem) error { - ncipInfo, err := n.getNcipInfo(customData) - if err != nil { - return err - } - request.InitiationHeader = n.prepareHeader(ncipInfo, request.InitiationHeader) +func (n *NcipClientImpl) CheckOutItem(request ncip.CheckOutItem) (*ncip.CheckOutItemResponse, error) { + request.InitiationHeader = n.prepareHeader(request.InitiationHeader) ncipMessage := &ncip.NCIPMessage{ CheckOutItem: &request, } - ncipResponse, err := n.sendReceiveMessage(ncipInfo, ncipMessage) + ncipResponse, err := n.sendReceiveMessage(ncipMessage) if err != nil { - return err + return nil, err } - checkOutItemResponse := ncipResponse.CheckOutItemResponse - if checkOutItemResponse == nil { - return fmt.Errorf("invalid NCIP response: missing CheckOutItemResponse") + response := ncipResponse.CheckOutItemResponse + if response == nil { + return nil, fmt.Errorf("invalid NCIP response: missing CheckOutItemResponse") } - return n.checkProblem("NCIP check out item", checkOutItemResponse.Problem) + return response, n.checkProblem("NCIP check out item", response.Problem) } -func (n *NcipClientImpl) CreateUserFiscalTransaction(customData map[string]any, request ncip.CreateUserFiscalTransaction) (bool, error) { - ncipInfo, err := n.getNcipInfo(customData) - if err != nil { - return false, err - } - handle, ret, err := n.checkMode(ncipInfo, CreateUserFiscalTransactionMode) +func (n *NcipClientImpl) CreateUserFiscalTransaction(request ncip.CreateUserFiscalTransaction) (*ncip.CreateUserFiscalTransactionResponse, error) { + handle, _, err := n.checkMode(CreateUserFiscalTransactionMode) if handle { - return ret, err + return nil, err } - request.InitiationHeader = n.prepareHeader(ncipInfo, request.InitiationHeader) + request.InitiationHeader = n.prepareHeader(request.InitiationHeader) ncipMessage := &ncip.NCIPMessage{ CreateUserFiscalTransaction: &request, } - ncipResponse, err := n.sendReceiveMessage(ncipInfo, ncipMessage) + ncipResponse, err := n.sendReceiveMessage(ncipMessage) if err != nil { - return false, err + return nil, err } - createUserFiscalTransactionResponse := ncipResponse.CreateUserFiscalTransactionResponse - if createUserFiscalTransactionResponse == nil { - return false, fmt.Errorf("invalid NCIP response: missing CreateUserFiscalTransactionResponse") + response := ncipResponse.CreateUserFiscalTransactionResponse + if response == nil { + return nil, fmt.Errorf("invalid NCIP response: missing CreateUserFiscalTransactionResponse") } - return true, n.checkProblem("NCIP create user fiscal transaction", createUserFiscalTransactionResponse.Problem) + return response, n.checkProblem("NCIP create user fiscal transaction", response.Problem) } func (n *NcipClientImpl) checkProblem(op string, responseProblems []ncip.Problem) error { @@ -207,18 +180,10 @@ func (n *NcipClientImpl) checkProblem(op string, responseProblems []ncip.Problem return nil } -func (n *NcipClientImpl) getNcipInfo(customData map[string]any) (map[string]any, error) { - ncipInfo, ok := customData["ncip"].(map[string]any) - if !ok { - return nil, fmt.Errorf("missing ncip configuration in customData") - } - return ncipInfo, nil -} - -func (n *NcipClientImpl) checkMode(ncipInfo map[string]any, key NcipProperty) (bool, bool, error) { - mode, ok := ncipInfo[string(key)].(string) +func (n *NcipClientImpl) checkMode(key NcipProperty) (bool, bool, error) { + mode, ok := n.ncipInfo[string(key)].(string) if !ok { - return true, false, fmt.Errorf("missing %s in ncip configuration", key) + return true, false, fmt.Errorf("missing %s in NCIP configuration", key) } switch mode { case string(ModeDisabled): @@ -233,34 +198,34 @@ func (n *NcipClientImpl) checkMode(ncipInfo map[string]any, key NcipProperty) (b return false, false, nil } -func (n *NcipClientImpl) prepareHeader(ncipInfo map[string]any, header *ncip.InitiationHeader) *ncip.InitiationHeader { +func (n *NcipClientImpl) prepareHeader(header *ncip.InitiationHeader) *ncip.InitiationHeader { if header == nil { header = &ncip.InitiationHeader{} } - from_agency, ok := ncipInfo[string(FromAgency)].(string) + from_agency, ok := n.ncipInfo[string(FromAgency)].(string) if !ok || from_agency == "" { from_agency = "default-from-agency" } header.FromAgencyId.AgencyId = ncip.SchemeValuePair{ Text: from_agency, } - to_agency, ok := ncipInfo[string(ToAgency)].(string) + to_agency, ok := n.ncipInfo[string(ToAgency)].(string) if !ok || to_agency == "" { to_agency = "default-to-agency" } header.ToAgencyId.AgencyId = ncip.SchemeValuePair{ Text: to_agency, } - if auth, ok := ncipInfo[string(FromAgencyAuthentication)].(string); ok { + if auth, ok := n.ncipInfo[string(FromAgencyAuthentication)].(string); ok { header.FromAgencyAuthentication = auth } return header } -func (n *NcipClientImpl) sendReceiveMessage(ncipInfo map[string]any, message *ncip.NCIPMessage) (*ncip.NCIPMessage, error) { - url, ok := ncipInfo[string(Address)].(string) +func (n *NcipClientImpl) sendReceiveMessage(message *ncip.NCIPMessage) (*ncip.NCIPMessage, error) { + url, ok := n.ncipInfo[string(Address)].(string) if !ok { - return nil, fmt.Errorf("missing NCIP address in customData") + return nil, fmt.Errorf("missing NCIP address in configuration") } message.Version = ncip.NCIP_V2_02_XSD diff --git a/broker/ncipclient/ncipclient_test.go b/broker/ncipclient/ncipclient_test.go index 30cc30d5..4ac0add2 100644 --- a/broker/ncipclient/ncipclient_test.go +++ b/broker/ncipclient/ncipclient_test.go @@ -34,10 +34,11 @@ func TestMain(m *testing.M) { } func TestPrepareHeaderEmpty(t *testing.T) { - ncipClient := NcipClientImpl{} ncipData := make(map[string]any) + ncipClient := NcipClientImpl{} + ncipClient.ncipInfo = ncipData - header := ncipClient.prepareHeader(ncipData, nil) + header := ncipClient.prepareHeader(nil) assert.Equal(t, "default-from-agency", header.FromAgencyId.AgencyId.Text) assert.Equal(t, "default-to-agency", header.ToAgencyId.AgencyId.Text) assert.Equal(t, "", header.FromAgencyAuthentication) @@ -46,160 +47,133 @@ func TestPrepareHeaderEmpty(t *testing.T) { func TestPrepareHeaderValues(t *testing.T) { ncipClient := NcipClientImpl{} ncipData := make(map[string]any) + ncipClient.ncipInfo = ncipData + ncipData["to_agency"] = "ILL-MOCK1" ncipData["from_agency"] = "ILL-MOCK2" ncipData["from_agency_authentication"] = "pass" - header := ncipClient.prepareHeader(ncipData, nil) + header := ncipClient.prepareHeader(nil) assert.Equal(t, "ILL-MOCK1", header.ToAgencyId.AgencyId.Text) assert.Equal(t, "ILL-MOCK2", header.FromAgencyId.AgencyId.Text) assert.Equal(t, "pass", header.FromAgencyAuthentication) } func TestLookupUserAutoOK(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - - customData := make(map[string]any) ncipData := make(map[string]any) ncipData["lookup_user_mode"] = "auto" ncipData["from_agency"] = "ILL-MOCK" ncipData["from_agency_authentication"] = "pass" ncipData["to_agency"] = "ILL-MOCK" ncipData["address"] = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" - customData["ncip"] = ncipData + ncipClient := NewNcipClient(http.DefaultClient, ncipData) lookup := ncip.LookupUser{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", }, } - b, err := ncipClient.LookupUser(customData, lookup) + res, err := ncipClient.LookupUser(lookup) assert.NoError(t, err) - assert.True(t, b) + assert.NotNil(t, res) } func TestLookupUserAutoInvalidUser(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - - customData := make(map[string]any) ncipData := make(map[string]any) ncipData["lookup_user_mode"] = "auto" ncipData["from_agency"] = "ILL-MOCK" ncipData["from_agency_authentication"] = "pass" ncipData["to_agency"] = "ILL-MOCK" ncipData["address"] = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" - customData["ncip"] = ncipData + ncipClient := NewNcipClient(http.DefaultClient, ncipData) lookup := ncip.LookupUser{ UserId: &ncip.UserId{ UserIdentifierValue: "foo", }, } - _, err := ncipClient.LookupUser(customData, lookup) + _, err := ncipClient.LookupUser(lookup) assert.Error(t, err) assert.Equal(t, "NCIP user lookup failed: Unknown User: foo", err.Error()) } func TestLookupUserModeManual(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - - customData := make(map[string]any) ncipData := make(map[string]any) ncipData["lookup_user_mode"] = "manual" - customData["ncip"] = ncipData + ncipClient := NewNcipClient(http.DefaultClient, ncipData) lookup := ncip.LookupUser{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", }, } - b, err := ncipClient.LookupUser(customData, lookup) + res, err := ncipClient.LookupUser(lookup) assert.NoError(t, err) - assert.False(t, b) + assert.Nil(t, res) } func TestLookupUserModeDisabled(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - - customData := make(map[string]any) ncipData := make(map[string]any) ncipData["lookup_user_mode"] = "disabled" - customData["ncip"] = ncipData + ncipClient := NewNcipClient(http.DefaultClient, ncipData) lookup := ncip.LookupUser{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", }, } - b, err := ncipClient.LookupUser(customData, lookup) + res, err := ncipClient.LookupUser(lookup) assert.NoError(t, err) - assert.True(t, b) + assert.Nil(t, res) } func TestLookupUserMissingAddress(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - - customData := make(map[string]any) ncipData := make(map[string]any) ncipData["lookup_user_mode"] = "auto" ncipData["from_agency"] = "ILL-MOCK" ncipData["to_agency"] = "ILL-MOCK" - customData["ncip"] = ncipData + ncipClient := NewNcipClient(http.DefaultClient, ncipData) lookup := ncip.LookupUser{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", }, } - _, err := ncipClient.LookupUser(customData, lookup) + res, err := ncipClient.LookupUser(lookup) assert.Error(t, err) - assert.Equal(t, "missing NCIP address in customData", err.Error()) -} - -func TestLookupUserMissingNcipInfo(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - customData := make(map[string]any) - lookup := ncip.LookupUser{} - _, err := ncipClient.LookupUser(customData, lookup) - assert.Error(t, err) - assert.Equal(t, "missing ncip configuration in customData", err.Error()) + assert.Equal(t, "missing NCIP address in configuration", err.Error()) + assert.Nil(t, res) } func TestLookupUserMissingAuthUserInfo(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - - customData := make(map[string]any) ncipData := make(map[string]any) ncipData["from_agency"] = "ILL-MOCK" ncipData["to_agency"] = "ILL-MOCK" - customData["ncip"] = ncipData + ncipClient := NewNcipClient(http.DefaultClient, ncipData) lookup := ncip.LookupUser{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", }, } - _, err := ncipClient.LookupUser(customData, lookup) + _, err := ncipClient.LookupUser(lookup) assert.Error(t, err) - assert.Equal(t, "missing lookup_user_mode in ncip configuration", err.Error()) + assert.Equal(t, "missing lookup_user_mode in NCIP configuration", err.Error()) } func TestLookupUserBadMode(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - - customData := make(map[string]any) ncipData := make(map[string]any) ncipData["lookup_user_mode"] = "foo" ncipData["from_agency"] = "ILL-MOCK" ncipData["to_agency"] = "ILL-MOCK" - customData["ncip"] = ncipData + ncipClient := NewNcipClient(http.DefaultClient, ncipData) lookup := ncip.LookupUser{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", }, } - _, err := ncipClient.LookupUser(customData, lookup) + _, err := ncipClient.LookupUser(lookup) assert.Error(t, err) assert.Equal(t, "unknown value for lookup_user_mode: foo", err.Error()) } @@ -214,8 +188,6 @@ func TestBadNcipMessageResponse(t *testing.T) { server := httptest.NewServer(handler) defer server.Close() - ncipClient := CreateNcipClient(http.DefaultClient) - customData := make(map[string]any) ncipData := make(map[string]any) ncipData["lookup_user_mode"] = "auto" ncipData["accept_item_mode"] = "auto" @@ -225,14 +197,14 @@ func TestBadNcipMessageResponse(t *testing.T) { ncipData["from_agency_authentication"] = "pass" ncipData["to_agency"] = "ILL-MOCK" ncipData["address"] = server.URL - customData["ncip"] = ncipData + ncipClient := NewNcipClient(http.DefaultClient, ncipData) lookup := ncip.LookupUser{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", }, } - _, err := ncipClient.LookupUser(customData, lookup) + _, err := ncipClient.LookupUser(lookup) assert.Error(t, err) assert.Contains(t, err.Error(), "NCIP message exchange failed:") @@ -241,37 +213,37 @@ func TestBadNcipMessageResponse(t *testing.T) { RequestIdentifierValue: "validrequest", }, } - _, err = ncipClient.AcceptItem(customData, accept) + _, err = ncipClient.AcceptItem(accept) assert.Error(t, err) assert.Contains(t, err.Error(), "NCIP message exchange failed:") delete := ncip.DeleteItem{} - err = ncipClient.DeleteItem(customData, delete) + _, err = ncipClient.DeleteItem(delete) assert.Error(t, err) assert.Contains(t, err.Error(), "NCIP message exchange failed:") request := ncip.RequestItem{} - _, err = ncipClient.RequestItem(customData, request) + _, err = ncipClient.RequestItem(request) assert.Error(t, err) assert.Contains(t, err.Error(), "NCIP message exchange failed:") cancelRequest := ncip.CancelRequestItem{} - err = ncipClient.CancelRequestItem(customData, cancelRequest) + _, err = ncipClient.CancelRequestItem(cancelRequest) assert.Error(t, err) assert.Contains(t, err.Error(), "NCIP message exchange failed:") checkInItem := ncip.CheckInItem{} - err = ncipClient.CheckInItem(customData, checkInItem) + _, err = ncipClient.CheckInItem(checkInItem) assert.Error(t, err) assert.Contains(t, err.Error(), "NCIP message exchange failed:") checkOutItem := ncip.CheckOutItem{} - err = ncipClient.CheckOutItem(customData, checkOutItem) + _, err = ncipClient.CheckOutItem(checkOutItem) assert.Error(t, err) assert.Contains(t, err.Error(), "NCIP message exchange failed:") createUserFiscalTransaction := ncip.CreateUserFiscalTransaction{} - _, err = ncipClient.CreateUserFiscalTransaction(customData, createUserFiscalTransaction) + _, err = ncipClient.CreateUserFiscalTransaction(createUserFiscalTransaction) assert.Error(t, err) assert.Contains(t, err.Error(), "NCIP message exchange failed:") } @@ -293,8 +265,6 @@ func TestEmptyNcipResponse(t *testing.T) { server := httptest.NewServer(handler) defer server.Close() - ncipClient := CreateNcipClient(http.DefaultClient) - customData := make(map[string]any) ncipData := make(map[string]any) ncipData["lookup_user_mode"] = "auto" ncipData["accept_item_mode"] = "auto" @@ -304,14 +274,14 @@ func TestEmptyNcipResponse(t *testing.T) { ncipData["from_agency_authentication"] = "pass" ncipData["to_agency"] = "ILL-MOCK" ncipData["address"] = server.URL - customData["ncip"] = ncipData + ncipClient := NewNcipClient(http.DefaultClient, ncipData) lookup := ncip.LookupUser{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", }, } - _, err := ncipClient.LookupUser(customData, lookup) + _, err := ncipClient.LookupUser(lookup) assert.Error(t, err) assert.Equal(t, "invalid NCIP response: missing LookupUserResponse", err.Error()) @@ -320,37 +290,37 @@ func TestEmptyNcipResponse(t *testing.T) { RequestIdentifierValue: "validrequest", }, } - _, err = ncipClient.AcceptItem(customData, accept) + _, err = ncipClient.AcceptItem(accept) assert.Error(t, err) assert.Contains(t, err.Error(), "invalid NCIP response: missing AcceptItemResponse") delete := ncip.DeleteItem{} - err = ncipClient.DeleteItem(customData, delete) + _, err = ncipClient.DeleteItem(delete) assert.Error(t, err) assert.Contains(t, err.Error(), "invalid NCIP response: missing DeleteItemResponse") request := ncip.RequestItem{} - _, err = ncipClient.RequestItem(customData, request) + _, err = ncipClient.RequestItem(request) assert.Error(t, err) assert.Contains(t, err.Error(), "invalid NCIP response: missing RequestItemResponse") cancelRequest := ncip.CancelRequestItem{} - err = ncipClient.CancelRequestItem(customData, cancelRequest) + _, err = ncipClient.CancelRequestItem(cancelRequest) assert.Error(t, err) assert.Contains(t, err.Error(), "invalid NCIP response: missing CancelRequestItemResponse") checkInItem := ncip.CheckInItem{} - err = ncipClient.CheckInItem(customData, checkInItem) + _, err = ncipClient.CheckInItem(checkInItem) assert.Error(t, err) assert.Contains(t, err.Error(), "invalid NCIP response: missing CheckInItemResponse") checkOutItem := ncip.CheckOutItem{} - err = ncipClient.CheckOutItem(customData, checkOutItem) + _, err = ncipClient.CheckOutItem(checkOutItem) assert.Error(t, err) assert.Contains(t, err.Error(), "invalid NCIP response: missing CheckOutItemResponse") createUserFiscalTransaction := ncip.CreateUserFiscalTransaction{} - _, err = ncipClient.CreateUserFiscalTransaction(customData, createUserFiscalTransaction) + _, err = ncipClient.CreateUserFiscalTransaction(createUserFiscalTransaction) assert.Error(t, err) assert.Contains(t, err.Error(), "invalid NCIP response: missing CreateUserFiscalTransactionResponse") } @@ -383,38 +353,33 @@ func TestLookupUserProblemResponse(t *testing.T) { server := httptest.NewServer(handler) defer server.Close() - ncipClient := CreateNcipClient(http.DefaultClient) - customData := make(map[string]any) ncipData := make(map[string]any) ncipData["lookup_user_mode"] = "auto" ncipData["from_agency"] = "ILL-MOCK" ncipData["from_agency_authentication"] = "pass" ncipData["to_agency"] = "ILL-MOCK" ncipData["address"] = server.URL - customData["ncip"] = ncipData + ncipClient := NewNcipClient(http.DefaultClient, ncipData) lookup := ncip.LookupUser{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", }, } - _, err := ncipClient.LookupUser(customData, lookup) + _, err := ncipClient.LookupUser(lookup) assert.Error(t, err) assert.Equal(t, "NCIP message processing failed: Some Problem: Details about the problem", err.Error()) } func TestAcceptItemOK(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - - customData := make(map[string]any) ncipData := make(map[string]any) ncipData["accept_item_mode"] = "auto" ncipData["from_agency"] = "ILL-MOCK" ncipData["from_agency_authentication"] = "pass" ncipData["to_agency"] = "ILL-MOCK" ncipData["address"] = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" - customData["ncip"] = ncipData + ncipClient := NewNcipClient(http.DefaultClient, ncipData) accept := ncip.AcceptItem{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", @@ -423,23 +388,20 @@ func TestAcceptItemOK(t *testing.T) { RequestIdentifierValue: "validrequest", }, } - b, err := ncipClient.AcceptItem(customData, accept) + res, err := ncipClient.AcceptItem(accept) assert.NoError(t, err) - assert.True(t, b) + assert.NotNil(t, res) } func TestAcceptItemInvalidUser(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - - customData := make(map[string]any) ncipData := make(map[string]any) ncipData["accept_item_mode"] = "auto" ncipData["from_agency"] = "ILL-MOCK" ncipData["from_agency_authentication"] = "pass" ncipData["to_agency"] = "ILL-MOCK" ncipData["address"] = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" - customData["ncip"] = ncipData + ncipClient := NewNcipClient(http.DefaultClient, ncipData) accept := ncip.AcceptItem{ UserId: &ncip.UserId{ UserIdentifierValue: "foo", @@ -448,18 +410,14 @@ func TestAcceptItemInvalidUser(t *testing.T) { RequestIdentifierValue: "validrequest", }, } - _, err := ncipClient.AcceptItem(customData, accept) + _, err := ncipClient.AcceptItem(accept) assert.Error(t, err) assert.Equal(t, "NCIP accept item failed: Unknown User: foo", err.Error()) } func TestAcceptItemModeManual(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - - customData := make(map[string]any) ncipData := make(map[string]any) ncipData["accept_item_mode"] = "manual" - customData["ncip"] = ncipData lookup := ncip.AcceptItem{ UserId: &ncip.UserId{ @@ -469,57 +427,51 @@ func TestAcceptItemModeManual(t *testing.T) { RequestIdentifierValue: "validrequest", }, } - b, err := ncipClient.AcceptItem(customData, lookup) + ncipClient := NewNcipClient(http.DefaultClient, ncipData) + res, err := ncipClient.AcceptItem(lookup) assert.NoError(t, err) - assert.False(t, b) + assert.Nil(t, res) } func TestAcceptItemMissingNcipInfo(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - customData := make(map[string]any) + ncipClient := NewNcipClient(http.DefaultClient, nil) accept := ncip.AcceptItem{} - _, err := ncipClient.AcceptItem(customData, accept) + _, err := ncipClient.AcceptItem(accept) assert.Error(t, err) - assert.Equal(t, "missing ncip configuration in customData", err.Error()) + assert.Equal(t, "missing accept_item_mode in NCIP configuration", err.Error()) } func TestDeleteItemOK(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - - customData := make(map[string]any) ncipData := make(map[string]any) ncipData["from_agency"] = "ILL-MOCK" ncipData["from_agency_authentication"] = "pass" ncipData["to_agency"] = "ILL-MOCK" ncipData["address"] = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" - customData["ncip"] = ncipData + ncipClient := NewNcipClient(http.DefaultClient, ncipData) delete := ncip.DeleteItem{} - err := ncipClient.DeleteItem(customData, delete) + res, err := ncipClient.DeleteItem(delete) assert.NoError(t, err) + assert.NotNil(t, res) } func TestDeleteItemMissingNcipInfo(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - customData := make(map[string]any) + ncipClient := NewNcipClient(http.DefaultClient, nil) delete := ncip.DeleteItem{} - err := ncipClient.DeleteItem(customData, delete) + _, err := ncipClient.DeleteItem(delete) assert.Error(t, err) - assert.Equal(t, "missing ncip configuration in customData", err.Error()) + assert.Equal(t, "missing NCIP address in configuration", err.Error()) } func TestRequestItemOK(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - - customData := make(map[string]any) ncipData := make(map[string]any) ncipData["request_item_mode"] = "auto" ncipData["from_agency"] = "ILL-MOCK" ncipData["from_agency_authentication"] = "pass" ncipData["to_agency"] = "ILL-MOCK" ncipData["address"] = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" - customData["ncip"] = ncipData + ncipClient := NewNcipClient(http.DefaultClient, ncipData) request := ncip.RequestItem{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", @@ -534,49 +486,33 @@ func TestRequestItemOK(t *testing.T) { Text: "Hold", }, } - b, err := ncipClient.RequestItem(customData, request) + _, err := ncipClient.RequestItem(request) assert.NoError(t, err) - assert.True(t, b) } func TestRequestItemModeManual(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - - customData := make(map[string]any) ncipData := make(map[string]any) ncipData["request_item_mode"] = "manual" - customData["ncip"] = ncipData + ncipClient := NewNcipClient(http.DefaultClient, ncipData) lookup := ncip.RequestItem{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", }, } - b, err := ncipClient.RequestItem(customData, lookup) + res, err := ncipClient.RequestItem(lookup) assert.NoError(t, err) - assert.False(t, b) -} - -func TestRequestItemMissingNcipInfo(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - customData := make(map[string]any) - request := ncip.RequestItem{} - _, err := ncipClient.RequestItem(customData, request) - assert.Error(t, err) - assert.Equal(t, "missing ncip configuration in customData", err.Error()) + assert.Nil(t, res) } func TestCancelRequestItemOK(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - - customData := make(map[string]any) ncipData := make(map[string]any) ncipData["from_agency"] = "ILL-MOCK" ncipData["from_agency_authentication"] = "pass" ncipData["to_agency"] = "ILL-MOCK" ncipData["address"] = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" - customData["ncip"] = ncipData + ncipClient := NewNcipClient(http.DefaultClient, ncipData) request := ncip.CancelRequestItem{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", @@ -588,59 +524,45 @@ func TestCancelRequestItemOK(t *testing.T) { Text: "Hold", }, } - err := ncipClient.CancelRequestItem(customData, request) + res, err := ncipClient.CancelRequestItem(request) assert.NoError(t, err) -} - -func TestCancelRequestItemMissingNcipInfo(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - customData := make(map[string]any) - request := ncip.CancelRequestItem{} - err := ncipClient.CancelRequestItem(customData, request) - assert.Error(t, err) - assert.Equal(t, "missing ncip configuration in customData", err.Error()) + assert.NotNil(t, res) } func TestCheckInItemOK(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - - customData := make(map[string]any) ncipData := make(map[string]any) ncipData["from_agency"] = "ILL-MOCK" ncipData["from_agency_authentication"] = "pass" ncipData["to_agency"] = "ILL-MOCK" ncipData["address"] = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" - customData["ncip"] = ncipData + ncipClient := NewNcipClient(http.DefaultClient, ncipData) request := ncip.CheckInItem{ ItemId: ncip.ItemId{ ItemIdentifierValue: "item-001", }, } - err := ncipClient.CheckInItem(customData, request) + res, err := ncipClient.CheckInItem(request) assert.NoError(t, err) + assert.NotNil(t, res) } func TestCheckInItemMissingNcipInfo(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - customData := make(map[string]any) + ncipClient := NewNcipClient(http.DefaultClient, nil) request := ncip.CheckInItem{} - err := ncipClient.CheckInItem(customData, request) + _, err := ncipClient.CheckInItem(request) assert.Error(t, err) - assert.Equal(t, "missing ncip configuration in customData", err.Error()) + assert.Equal(t, "missing NCIP address in configuration", err.Error()) } func TestCheckOutItemOK(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - - customData := make(map[string]any) ncipData := make(map[string]any) ncipData["from_agency"] = "ILL-MOCK" ncipData["from_agency_authentication"] = "pass" ncipData["to_agency"] = "ILL-MOCK" ncipData["address"] = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" - customData["ncip"] = ncipData + ncipClient := NewNcipClient(http.DefaultClient, ncipData) request := ncip.CheckOutItem{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", @@ -649,31 +571,27 @@ func TestCheckOutItemOK(t *testing.T) { ItemIdentifierValue: "item-001", }, } - err := ncipClient.CheckOutItem(customData, request) + _, err := ncipClient.CheckOutItem(request) assert.NoError(t, err) } func TestCheckOutItemMissingNcipInfo(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - customData := make(map[string]any) + ncipClient := NewNcipClient(http.DefaultClient, nil) request := ncip.CheckOutItem{} - err := ncipClient.CheckOutItem(customData, request) + _, err := ncipClient.CheckOutItem(request) assert.Error(t, err) - assert.Equal(t, "missing ncip configuration in customData", err.Error()) + assert.Equal(t, "missing NCIP address in configuration", err.Error()) } func TestCreateUserFiscalTransactionOK(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - - customData := make(map[string]any) ncipData := make(map[string]any) ncipData["create_user_fiscal_transaction_mode"] = "auto" ncipData["from_agency"] = "ILL-MOCK" ncipData["from_agency_authentication"] = "pass" ncipData["to_agency"] = "ILL-MOCK" ncipData["address"] = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" - customData["ncip"] = ncipData + ncipClient := NewNcipClient(http.DefaultClient, ncipData) lookup := ncip.CreateUserFiscalTransaction{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", @@ -685,36 +603,32 @@ func TestCreateUserFiscalTransactionOK(t *testing.T) { }, }, } - b, err := ncipClient.CreateUserFiscalTransaction(customData, lookup) + res, err := ncipClient.CreateUserFiscalTransaction(lookup) assert.NoError(t, err) - assert.True(t, b) + assert.NotNil(t, res) } func TestCreateUserFiscalTransactionBadMode(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - - customData := make(map[string]any) ncipData := make(map[string]any) ncipData["create_user_fiscal_transaction_mode"] = "foo" ncipData["from_agency"] = "ILL-MOCK" ncipData["to_agency"] = "ILL-MOCK" - customData["ncip"] = ncipData + ncipClient := NewNcipClient(http.DefaultClient, ncipData) lookup := ncip.CreateUserFiscalTransaction{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", }, } - _, err := ncipClient.CreateUserFiscalTransaction(customData, lookup) + _, err := ncipClient.CreateUserFiscalTransaction(lookup) assert.Error(t, err) assert.Equal(t, "unknown value for create_user_fiscal_transaction_mode: foo", err.Error()) } func TestCreateUserFiscalTransactionMissingNcipInfo(t *testing.T) { - ncipClient := CreateNcipClient(http.DefaultClient) - customData := make(map[string]any) + ncipClient := NewNcipClient(http.DefaultClient, nil) request := ncip.CreateUserFiscalTransaction{} - _, err := ncipClient.CreateUserFiscalTransaction(customData, request) + _, err := ncipClient.CreateUserFiscalTransaction(request) assert.Error(t, err) - assert.Equal(t, "missing ncip configuration in customData", err.Error()) + assert.Equal(t, "missing create_user_fiscal_transaction_mode in NCIP configuration", err.Error()) } diff --git a/broker/patron_request/api/api-handler.go b/broker/patron_request/api/api-handler.go index 8d6aa2ca..deb5ac8e 100644 --- a/broker/patron_request/api/api-handler.go +++ b/broker/patron_request/api/api-handler.go @@ -20,14 +20,16 @@ import ( var waitingReqs = map[string]RequestWait{} type PatronRequestApiHandler struct { - prRepo pr_db.PrRepo - eventBus events.EventBus + prRepo pr_db.PrRepo + eventBus events.EventBus + tenantToSymbol common.TenantToSymbol } -func NewApiHandler(prRepo pr_db.PrRepo, eventBus events.EventBus) PatronRequestApiHandler { +func NewApiHandler(prRepo pr_db.PrRepo, eventBus events.EventBus, tenantToSymbol common.TenantToSymbol) PatronRequestApiHandler { return PatronRequestApiHandler{ - prRepo: prRepo, - eventBus: eventBus, + prRepo: prRepo, + eventBus: eventBus, + tenantToSymbol: tenantToSymbol, } } @@ -58,12 +60,12 @@ func (a *PatronRequestApiHandler) PostPatronRequests(w http.ResponseWriter, r *h addBadRequestError(ctx, w, err) return } - tenant := params.XOkapiTenant - if tenant == nil { - addBadRequestError(ctx, w, errors.New("X-Okapi-Tenant header is required")) + dbreq, err := toDbPatronRequest(a.tenantToSymbol, newPr, params.XOkapiTenant) + if err != nil { + addBadRequestError(ctx, w, err) return } - pr, err := a.prRepo.SavePatronRequest(ctx, (pr_db.SavePatronRequestParams)(toDbPatronRequest(newPr, *tenant))) + pr, err := a.prRepo.SavePatronRequest(ctx, (pr_db.SavePatronRequestParams)(dbreq)) if err != nil { addInternalError(ctx, w, err) return @@ -258,7 +260,14 @@ func toStringFromBytes(bytes []byte) *string { return value } -func toDbPatronRequest(request proapi.CreatePatronRequest, tenant string) pr_db.PatronRequest { +func toDbPatronRequest(tenantToSymbol common.TenantToSymbol, request proapi.CreatePatronRequest, tenant *string) (pr_db.PatronRequest, error) { + if tenant == nil { + return pr_db.PatronRequest{}, errors.New("X-Okapi-Tenant header is required") + } + if tenantToSymbol.TenantMode() { + symbol := tenantToSymbol.GetSymbolFromTenant(*tenant) + request.RequesterSymbol = &symbol + } var illRequest []byte if request.IllRequest != nil { illRequest = []byte(*request.IllRequest) @@ -272,8 +281,8 @@ func toDbPatronRequest(request proapi.CreatePatronRequest, tenant string) pr_db. RequesterSymbol: getDbText(request.RequesterSymbol), SupplierSymbol: getDbText(request.SupplierSymbol), IllRequest: illRequest, - Tenant: getDbText(&tenant), - } + Tenant: getDbText(tenant), + }, nil } func getId(id string) string { diff --git a/broker/patron_request/api/api-handler_test.go b/broker/patron_request/api/api-handler_test.go index 5a4aa500..68a0d7c3 100644 --- a/broker/patron_request/api/api-handler_test.go +++ b/broker/patron_request/api/api-handler_test.go @@ -36,7 +36,7 @@ func TestGetDbText(t *testing.T) { } func TestGetPatronRequests(t *testing.T) { - handler := NewApiHandler(new(PrRepoError), mockEventBus) + handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenantToSymbol("")) req, _ := http.NewRequest("GET", "/", nil) rr := httptest.NewRecorder() handler.GetPatronRequests(rr, req) @@ -47,7 +47,7 @@ func TestGetPatronRequests(t *testing.T) { } func TestPostPatronRequests(t *testing.T) { - handler := NewApiHandler(new(PrRepoError), mockEventBus) + handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenantToSymbol("")) toCreate := proapi.PatronRequest{ID: "1"} jsonBytes, err := json.Marshal(toCreate) assert.NoError(t, err, "failed to marshal patron request") @@ -61,7 +61,7 @@ func TestPostPatronRequests(t *testing.T) { } func TestPostPatronRequestsMissingTenant(t *testing.T) { - handler := NewApiHandler(new(PrRepoError), mockEventBus) + handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenantToSymbol("")) toCreate := proapi.PatronRequest{ID: "1"} jsonBytes, err := json.Marshal(toCreate) assert.NoError(t, err, "failed to marshal patron request") @@ -74,7 +74,7 @@ func TestPostPatronRequestsMissingTenant(t *testing.T) { } func TestPostPatronRequestsInvalidJson(t *testing.T) { - handler := NewApiHandler(new(PrRepoError), mockEventBus) + handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenantToSymbol("")) req, _ := http.NewRequest("POST", "/", bytes.NewBuffer([]byte("a\": v\""))) rr := httptest.NewRecorder() tenant := proapi.Tenant("test-lib") @@ -84,7 +84,7 @@ func TestPostPatronRequestsInvalidJson(t *testing.T) { } func TestDeletePatronRequestsIdNotFound(t *testing.T) { - handler := NewApiHandler(new(PrRepoError), mockEventBus) + handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenantToSymbol("")) req, _ := http.NewRequest("POST", "/", nil) rr := httptest.NewRecorder() handler.DeletePatronRequestsId(rr, req, "2", proapi.DeletePatronRequestsIdParams{}) @@ -92,7 +92,7 @@ func TestDeletePatronRequestsIdNotFound(t *testing.T) { } func TestDeletePatronRequestsId(t *testing.T) { - handler := NewApiHandler(new(PrRepoError), mockEventBus) + handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenantToSymbol("")) req, _ := http.NewRequest("POST", "/", nil) rr := httptest.NewRecorder() handler.DeletePatronRequestsId(rr, req, "3", proapi.DeletePatronRequestsIdParams{}) @@ -101,7 +101,7 @@ func TestDeletePatronRequestsId(t *testing.T) { } func TestGetPatronRequestsIdNotFound(t *testing.T) { - handler := NewApiHandler(new(PrRepoError), mockEventBus) + handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenantToSymbol("")) req, _ := http.NewRequest("POST", "/", nil) rr := httptest.NewRecorder() handler.GetPatronRequestsId(rr, req, "2", proapi.GetPatronRequestsIdParams{}) @@ -109,7 +109,7 @@ func TestGetPatronRequestsIdNotFound(t *testing.T) { } func TestGetPatronRequestsId(t *testing.T) { - handler := NewApiHandler(new(PrRepoError), mockEventBus) + handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenantToSymbol("")) req, _ := http.NewRequest("POST", "/", nil) rr := httptest.NewRecorder() handler.GetPatronRequestsId(rr, req, "1", proapi.GetPatronRequestsIdParams{}) diff --git a/broker/patron_request/service/action.go b/broker/patron_request/service/action.go index f196f82c..ff4089f4 100644 --- a/broker/patron_request/service/action.go +++ b/broker/patron_request/service/action.go @@ -10,9 +10,10 @@ import ( "github.com/indexdata/crosslink/broker/common" "github.com/indexdata/crosslink/broker/events" "github.com/indexdata/crosslink/broker/handler" - "github.com/indexdata/crosslink/broker/ill_db" + "github.com/indexdata/crosslink/broker/lms" pr_db "github.com/indexdata/crosslink/broker/patron_request/db" "github.com/indexdata/crosslink/iso18626" + "github.com/jackc/pgx/v5/pgtype" ) const COMP = "pr_action_service" @@ -60,17 +61,17 @@ var BorrowerStateActionMapping = map[string][]string{ type PatronRequestActionService struct { prRepo pr_db.PrRepo - illRepo ill_db.IllRepo eventBus events.EventBus iso18626Handler handler.Iso18626HandlerInterface + lmsCreator lms.LmsCreator } -func CreatePatronRequestActionService(prRepo pr_db.PrRepo, illRepo ill_db.IllRepo, eventBus events.EventBus, iso18626Handler handler.Iso18626HandlerInterface) PatronRequestActionService { +func CreatePatronRequestActionService(prRepo pr_db.PrRepo, eventBus events.EventBus, iso18626Handler handler.Iso18626HandlerInterface, lmsCreator lms.LmsCreator) PatronRequestActionService { return PatronRequestActionService{ prRepo: prRepo, - illRepo: illRepo, eventBus: eventBus, iso18626Handler: iso18626Handler, + lmsCreator: lmsCreator, } } func GetBorrowerActionsByState(state string) []string { @@ -136,9 +137,21 @@ func (a *PatronRequestActionService) updateStateAndReturnResult(ctx common.Exten } func (a *PatronRequestActionService) validateBorrowingRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest) (events.EventStatus, *events.EventResult) { - if !pr.Tenant.Valid { - return events.LogErrorAndReturnResult(ctx, "missing tenant", nil) + user := "" + if pr.Patron.Valid { + user = pr.Patron.String } + lmsAdapter, err := a.lmsCreator.GetAdapter(ctx, pr.RequesterSymbol) + if err != nil { + return events.LogErrorAndReturnResult(ctx, "failed to create LMS adapter", err) + } + userId, err := lmsAdapter.LookupUser(user) + if err != nil { + return events.LogErrorAndReturnResult(ctx, "LMS LookupUser failed", err) + } + // change patron to canonical user id + // perhaps it would be better to have both original and canonical id stored? + pr.Patron = pgtype.Text{String: userId, Valid: true} return a.updateStateAndReturnResult(ctx, pr, BorrowerStateValidated, nil) } @@ -180,6 +193,19 @@ func (a *PatronRequestActionService) sendBorrowingRequest(ctx common.ExtendedCon } func (a *PatronRequestActionService) receiveBorrowingRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest) (events.EventStatus, *events.EventResult) { + user := "" + if pr.Patron.Valid { + user = pr.Patron.String + } + lmsAdapter, err := a.lmsCreator.GetAdapter(ctx, pr.RequesterSymbol) + if err != nil { + return events.LogErrorAndReturnResult(ctx, "failed to create LMS adapter", err) + } + // TODO: get all these parameters from the patron request + err = lmsAdapter.AcceptItem("itemId", "requestId", user, "author", "title", "isbn", "callNumber", "pickupLocation", "requestedAction") + if err != nil { + return events.LogErrorAndReturnResult(ctx, "LMS AcceptItem failed", err) + } result := events.EventResult{} status, eventResult, httpStatus := a.sendRequestingAgencyMessage(ctx, pr, &result, iso18626.TypeActionReceived) if httpStatus == nil { @@ -193,16 +219,44 @@ func (a *PatronRequestActionService) receiveBorrowingRequest(ctx common.Extended } func (a *PatronRequestActionService) checkoutBorrowingRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest) (events.EventStatus, *events.EventResult) { - // TODO Make NCIP calls + user := "" + if pr.Patron.Valid { + user = pr.Patron.String + } + lmsAdapter, err := a.lmsCreator.GetAdapter(ctx, pr.RequesterSymbol) + if err != nil { + return events.LogErrorAndReturnResult(ctx, "failed to create LMS adapter", err) + } + err = lmsAdapter.CheckOutItem("requestId", "itemBarcode", user, "externalReferenceValue") + if err != nil { + return events.LogErrorAndReturnResult(ctx, "LMS CheckOutItem failed", err) + } return a.updateStateAndReturnResult(ctx, pr, BorrowerStateCheckedOut, nil) } func (a *PatronRequestActionService) checkinBorrowingRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest) (events.EventStatus, *events.EventResult) { - // TODO Make NCIP calls + lmsAdapter, err := a.lmsCreator.GetAdapter(ctx, pr.RequesterSymbol) + if err != nil { + return events.LogErrorAndReturnResult(ctx, "failed to create LMS adapter", err) + } + item := "" // TODO Get item identifier from the request + err = lmsAdapter.CheckInItem(item) + if err != nil { + return events.LogErrorAndReturnResult(ctx, "LMS CheckInItem failed", err) + } return a.updateStateAndReturnResult(ctx, pr, BorrowerStateCheckedIn, nil) } func (a *PatronRequestActionService) shipReturnBorrowingRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest) (events.EventStatus, *events.EventResult) { + lmsAdapter, err := a.lmsCreator.GetAdapter(ctx, pr.RequesterSymbol) + if err != nil { + return events.LogErrorAndReturnResult(ctx, "failed to create LMS adapter", err) + } + item := "" // TODO Get item identifier from the request + err = lmsAdapter.DeleteItem(item) + if err != nil { + return events.LogErrorAndReturnResult(ctx, "LMS DeleteItem failed", err) + } result := events.EventResult{} status, eventResult, httpStatus := a.sendRequestingAgencyMessage(ctx, pr, &result, iso18626.TypeActionShippedReturn) if httpStatus == nil { diff --git a/broker/patron_request/service/action_test.go b/broker/patron_request/service/action_test.go index 3cb868e4..ee64b3ff 100644 --- a/broker/patron_request/service/action_test.go +++ b/broker/patron_request/service/action_test.go @@ -11,7 +11,7 @@ import ( "github.com/indexdata/crosslink/broker/common" "github.com/indexdata/crosslink/broker/events" "github.com/indexdata/crosslink/broker/handler" - "github.com/indexdata/crosslink/broker/ill_db" + "github.com/indexdata/crosslink/broker/lms" pr_db "github.com/indexdata/crosslink/broker/patron_request/db" "github.com/indexdata/crosslink/iso18626" @@ -35,7 +35,7 @@ func TestIsBorrowerActionAvailable(t *testing.T) { } func TestInvokeAction(t *testing.T) { mockEventBus := new(MockEventBus) - prAction := CreatePatronRequestActionService(*new(pr_db.PrRepo), *new(ill_db.IllRepo), mockEventBus, new(handler.Iso18626Handler)) + prAction := CreatePatronRequestActionService(*new(pr_db.PrRepo), mockEventBus, new(handler.Iso18626Handler), nil) event := events.Event{ ID: "action-1", } @@ -47,7 +47,7 @@ func TestInvokeAction(t *testing.T) { } func TestHandleInvokeActionNotSpecifiedAction(t *testing.T) { - prAction := CreatePatronRequestActionService(*new(pr_db.PrRepo), *new(ill_db.IllRepo), *new(events.EventBus), new(handler.Iso18626Handler)) + prAction := CreatePatronRequestActionService(*new(pr_db.PrRepo), *new(events.EventBus), new(handler.Iso18626Handler), nil) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{}) @@ -57,7 +57,7 @@ func TestHandleInvokeActionNotSpecifiedAction(t *testing.T) { func TestHandleInvokeActionNoPR(t *testing.T) { mockPrRepo := new(MockPrRepo) - prAction := CreatePatronRequestActionService(mockPrRepo, *new(ill_db.IllRepo), *new(events.EventBus), new(handler.Iso18626Handler)) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), new(handler.Iso18626Handler), nil) mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{}, errors.New("not fund")) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionValidate}}}) @@ -68,7 +68,7 @@ func TestHandleInvokeActionNoPR(t *testing.T) { func TestHandleInvokeActionWhichIsNotAllowed(t *testing.T) { mockPrRepo := new(MockPrRepo) - prAction := CreatePatronRequestActionService(mockPrRepo, *new(ill_db.IllRepo), *new(events.EventBus), new(handler.Iso18626Handler)) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), new(handler.Iso18626Handler), nil) mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateValidated}, nil) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionValidate}}}) @@ -77,10 +77,12 @@ func TestHandleInvokeActionWhichIsNotAllowed(t *testing.T) { assert.Equal(t, "state VALIDATED does not support action validate", resultData.EventError.Message) } -func TestHandleInvokeActionValidate(t *testing.T) { +func TestHandleInvokeActionValidateOK(t *testing.T) { mockPrRepo := new(MockPrRepo) - prAction := CreatePatronRequestActionService(mockPrRepo, new(MockIllRepo), *new(events.EventBus), new(handler.Iso18626Handler)) - mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateNew, Tenant: pgtype.Text{Valid: true, String: "testlib"}}, nil) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:x"}).Return(lms.CreateLmsAdapterMockOK(), nil) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), new(handler.Iso18626Handler), lmsCreator) + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:x"}, State: BorrowerStateNew, Tenant: pgtype.Text{Valid: true, String: "testlib"}}, nil) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionValidate}}}) @@ -89,10 +91,37 @@ func TestHandleInvokeActionValidate(t *testing.T) { assert.Equal(t, BorrowerStateValidated, mockPrRepo.savedPr.State) } +func TestHandleInvokeActionValidateGetAdapterFailed(t *testing.T) { + mockPrRepo := new(MockPrRepo) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:x"}).Return(lms.CreateLmsAdapterMockOK(), assert.AnError) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), new(handler.Iso18626Handler), lmsCreator) + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:x"}, State: BorrowerStateNew, Tenant: pgtype.Text{Valid: true, String: "testlib"}}, nil) + + status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionValidate}}}) + + assert.Equal(t, events.EventStatusError, status) + assert.Equal(t, "failed to create LMS adapter", resultData.EventError.Message) + assert.Equal(t, "assert.AnError general error for testing", resultData.EventError.Cause) +} + +func TestHandleInvokeActionValidateLookupFailed(t *testing.T) { + mockPrRepo := new(MockPrRepo) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(createLmsAdapterMockFail(), nil) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), new(handler.Iso18626Handler), lmsCreator) + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, State: BorrowerStateNew, Tenant: pgtype.Text{Valid: true, String: "testlib"}}, nil) + + status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionValidate}}}) + + assert.Equal(t, events.EventStatusError, status) + assert.Equal(t, "LMS LookupUser failed", resultData.EventError.Message) +} + func TestHandleInvokeActionSendRequest(t *testing.T) { mockPrRepo := new(MockPrRepo) mockIso18626Handler := new(MockIso18626Handler) - prAction := CreatePatronRequestActionService(mockPrRepo, new(MockIllRepo), *new(events.EventBus), mockIso18626Handler) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, nil) mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateValidated, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}, nil) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionSendRequest}}}) @@ -102,10 +131,12 @@ func TestHandleInvokeActionSendRequest(t *testing.T) { assert.Equal(t, BorrowerStateSent, mockPrRepo.savedPr.State) } -func TestHandleInvokeActionReceive(t *testing.T) { +func TestHandleInvokeActionReceiveOK(t *testing.T) { mockPrRepo := new(MockPrRepo) mockIso18626Handler := new(MockIso18626Handler) - prAction := CreatePatronRequestActionService(mockPrRepo, new(MockIllRepo), *new(events.EventBus), mockIso18626Handler) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), nil) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateShipped, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}, nil) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionReceive}}}) @@ -115,10 +146,40 @@ func TestHandleInvokeActionReceive(t *testing.T) { assert.Equal(t, BorrowerStateReceived, mockPrRepo.savedPr.State) } -func TestHandleInvokeActionCheckOut(t *testing.T) { +func TestHandleInvokeActionReceiveFailCreator(t *testing.T) { + mockPrRepo := new(MockPrRepo) + mockIso18626Handler := new(MockIso18626Handler) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), assert.AnError) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateShipped, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}, nil) + + status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionReceive}}}) + + assert.Equal(t, events.EventStatusError, status) + assert.Equal(t, "failed to create LMS adapter", resultData.EventError.Message) +} + +func TestHandleInvokeActionReceiveAcceptItemFailed(t *testing.T) { mockPrRepo := new(MockPrRepo) - prAction := CreatePatronRequestActionService(mockPrRepo, *new(ill_db.IllRepo), *new(events.EventBus), new(handler.Iso18626Handler)) - mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateReceived}, nil) + mockIso18626Handler := new(MockIso18626Handler) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(createLmsAdapterMockFail(), nil) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateShipped, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}, nil) + + status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionReceive}}}) + + assert.Equal(t, events.EventStatusError, status) + assert.Equal(t, "LMS AcceptItem failed", resultData.EventError.Message) +} + +func TestHandleInvokeActionCheckOutOK(t *testing.T) { + mockPrRepo := new(MockPrRepo) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), nil) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), new(handler.Iso18626Handler), lmsCreator) + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{Patron: pgtype.Text{Valid: true, String: "patron1"}, State: BorrowerStateReceived, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}, nil) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionCheckOut}}}) @@ -127,10 +188,36 @@ func TestHandleInvokeActionCheckOut(t *testing.T) { assert.Equal(t, BorrowerStateCheckedOut, mockPrRepo.savedPr.State) } -func TestHandleInvokeActionCheckIn(t *testing.T) { +func TestHandleInvokeActionCheckOutFailCreator(t *testing.T) { + mockPrRepo := new(MockPrRepo) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), assert.AnError) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), new(handler.Iso18626Handler), lmsCreator) + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateReceived, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}, nil) + + status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionCheckOut}}}) + assert.Equal(t, events.EventStatusError, status) + assert.Equal(t, "failed to create LMS adapter", resultData.EventError.Message) +} + +func TestHandleInvokeActionCheckOutFails(t *testing.T) { + mockPrRepo := new(MockPrRepo) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(createLmsAdapterMockFail(), nil) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), new(handler.Iso18626Handler), lmsCreator) + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateReceived, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}, nil) + + status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionCheckOut}}}) + assert.Equal(t, events.EventStatusError, status) + assert.Equal(t, "LMS CheckOutItem failed", resultData.EventError.Message) +} + +func TestHandleInvokeActionCheckInOK(t *testing.T) { mockPrRepo := new(MockPrRepo) - prAction := CreatePatronRequestActionService(mockPrRepo, *new(ill_db.IllRepo), *new(events.EventBus), new(handler.Iso18626Handler)) - mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateCheckedOut}, nil) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), nil) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), new(handler.Iso18626Handler), lmsCreator) + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateCheckedOut, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}, nil) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionCheckIn}}}) @@ -139,10 +226,36 @@ func TestHandleInvokeActionCheckIn(t *testing.T) { assert.Equal(t, BorrowerStateCheckedIn, mockPrRepo.savedPr.State) } -func TestHandleInvokeActionShipReturn(t *testing.T) { +func TestHandleInvokeActionCheckInFailCreator(t *testing.T) { + mockPrRepo := new(MockPrRepo) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), assert.AnError) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), new(handler.Iso18626Handler), lmsCreator) + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateCheckedOut, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}, nil) + + status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionCheckIn}}}) + assert.Equal(t, events.EventStatusError, status) + assert.Equal(t, "failed to create LMS adapter", resultData.EventError.Message) +} + +func TestHandleInvokeActionCheckInFails(t *testing.T) { + mockPrRepo := new(MockPrRepo) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(createLmsAdapterMockFail(), nil) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), new(handler.Iso18626Handler), lmsCreator) + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateCheckedOut, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}, nil) + + status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionCheckIn}}}) + assert.Equal(t, events.EventStatusError, status) + assert.Equal(t, "LMS CheckInItem failed", resultData.EventError.Message) +} + +func TestHandleInvokeActionShipReturnOK(t *testing.T) { mockPrRepo := new(MockPrRepo) mockIso18626Handler := new(MockIso18626Handler) - prAction := CreatePatronRequestActionService(mockPrRepo, new(MockIllRepo), *new(events.EventBus), mockIso18626Handler) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), nil) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateCheckedIn, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}, nil) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionShipReturn}}}) @@ -152,10 +265,38 @@ func TestHandleInvokeActionShipReturn(t *testing.T) { assert.Equal(t, BorrowerStateShippedReturned, mockPrRepo.savedPr.State) } +func TestHandleInvokeActionShipReturnFailCreator(t *testing.T) { + mockPrRepo := new(MockPrRepo) + mockIso18626Handler := new(MockIso18626Handler) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), assert.AnError) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateCheckedIn, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}, nil) + + status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionShipReturn}}}) + + assert.Equal(t, events.EventStatusError, status) + assert.Equal(t, "failed to create LMS adapter", resultData.EventError.Message) +} + +func TestHandleInvokeActionShipReturnFails(t *testing.T) { + mockPrRepo := new(MockPrRepo) + mockIso18626Handler := new(MockIso18626Handler) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(createLmsAdapterMockFail(), nil) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateCheckedIn, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}, nil) + + status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionShipReturn}}}) + + assert.Equal(t, events.EventStatusError, status) + assert.Equal(t, "LMS DeleteItem failed", resultData.EventError.Message) +} + func TestHandleInvokeActionCancelRequest(t *testing.T) { mockPrRepo := new(MockPrRepo) mockIso18626Handler := new(MockIso18626Handler) - prAction := CreatePatronRequestActionService(mockPrRepo, new(MockIllRepo), *new(events.EventBus), mockIso18626Handler) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, nil) mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateWillSupply, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}, nil) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionCancelRequest}}}) @@ -168,7 +309,7 @@ func TestHandleInvokeActionCancelRequest(t *testing.T) { func TestHandleInvokeActionAcceptCondition(t *testing.T) { mockPrRepo := new(MockPrRepo) mockIso18626Handler := new(MockIso18626Handler) - prAction := CreatePatronRequestActionService(mockPrRepo, new(MockIllRepo), *new(events.EventBus), mockIso18626Handler) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, nil) mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateConditionPending, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}, nil) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionAcceptCondition}}}) @@ -181,7 +322,7 @@ func TestHandleInvokeActionAcceptCondition(t *testing.T) { func TestHandleInvokeActionRejectCondition(t *testing.T) { mockPrRepo := new(MockPrRepo) mockIso18626Handler := new(MockIso18626Handler) - prAction := CreatePatronRequestActionService(mockPrRepo, new(MockIllRepo), *new(events.EventBus), mockIso18626Handler) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, nil) mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateConditionPending, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}, nil) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionRejectCondition}}}) @@ -194,7 +335,7 @@ func TestHandleInvokeActionRejectCondition(t *testing.T) { func TestSendBorrowingRequestInvalidSymbol(t *testing.T) { mockPrRepo := new(MockPrRepo) mockIso18626Handler := new(MockIso18626Handler) - prAction := CreatePatronRequestActionService(mockPrRepo, new(MockIllRepo), *new(events.EventBus), mockIso18626Handler) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, nil) status, resultData := prAction.sendBorrowingRequest(appCtx, pr_db.PatronRequest{State: BorrowerStateValidated, RequesterSymbol: pgtype.Text{Valid: true, String: "x"}}) @@ -205,7 +346,7 @@ func TestSendBorrowingRequestInvalidSymbol(t *testing.T) { func TestSendBorrowingRequestMissingSymbol(t *testing.T) { mockPrRepo := new(MockPrRepo) mockIso18626Handler := new(MockIso18626Handler) - prAction := CreatePatronRequestActionService(mockPrRepo, new(MockIllRepo), *new(events.EventBus), mockIso18626Handler) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, nil) status, resultData := prAction.sendBorrowingRequest(appCtx, pr_db.PatronRequest{State: BorrowerStateValidated}) @@ -216,9 +357,11 @@ func TestSendBorrowingRequestMissingSymbol(t *testing.T) { func TestShipReturnBorrowingRequestMissingSupplierSymbol(t *testing.T) { mockPrRepo := new(MockPrRepo) mockIso18626Handler := new(MockIso18626Handler) - prAction := CreatePatronRequestActionService(mockPrRepo, new(MockIllRepo), *new(events.EventBus), mockIso18626Handler) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), nil) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) - status, resultData := prAction.shipReturnBorrowingRequest(appCtx, pr_db.PatronRequest{ID: "1", State: BorrowerStateValidated, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}) + status, resultData := prAction.shipReturnBorrowingRequest(appCtx, pr_db.PatronRequest{Patron: pgtype.Text{Valid: true, String: "patron1"}, ID: "1", State: BorrowerStateValidated, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}) assert.Equal(t, events.EventStatusError, status) assert.Equal(t, "missing supplier symbol", resultData.EventError.Message) @@ -227,7 +370,9 @@ func TestShipReturnBorrowingRequestMissingSupplierSymbol(t *testing.T) { func TestShipReturnBorrowingRequestMissingRequesterSymbol(t *testing.T) { mockPrRepo := new(MockPrRepo) mockIso18626Handler := new(MockIso18626Handler) - prAction := CreatePatronRequestActionService(mockPrRepo, new(MockIllRepo), *new(events.EventBus), mockIso18626Handler) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{}).Return(lms.CreateLmsAdapterMockOK(), nil) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) status, resultData := prAction.shipReturnBorrowingRequest(appCtx, pr_db.PatronRequest{ID: "1", State: BorrowerStateValidated, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}) @@ -238,7 +383,9 @@ func TestShipReturnBorrowingRequestMissingRequesterSymbol(t *testing.T) { func TestShipReturnBorrowingRequestInvalidSupplierSymbol(t *testing.T) { mockPrRepo := new(MockPrRepo) mockIso18626Handler := new(MockIso18626Handler) - prAction := CreatePatronRequestActionService(mockPrRepo, new(MockIllRepo), *new(events.EventBus), mockIso18626Handler) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), nil) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) status, resultData := prAction.shipReturnBorrowingRequest(appCtx, pr_db.PatronRequest{ID: "1", State: BorrowerStateValidated, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "x"}}) @@ -249,7 +396,9 @@ func TestShipReturnBorrowingRequestInvalidSupplierSymbol(t *testing.T) { func TestShipReturnBorrowingRequestInvalidRequesterSymbol(t *testing.T) { mockPrRepo := new(MockPrRepo) mockIso18626Handler := new(MockIso18626Handler) - prAction := CreatePatronRequestActionService(mockPrRepo, new(MockIllRepo), *new(events.EventBus), mockIso18626Handler) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "x"}).Return(lms.CreateLmsAdapterMockOK(), nil) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) status, resultData := prAction.shipReturnBorrowingRequest(appCtx, pr_db.PatronRequest{ID: "1", State: BorrowerStateValidated, RequesterSymbol: pgtype.Text{Valid: true, String: "x"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}) @@ -326,6 +475,7 @@ func (h *MockIso18626Handler) HandleRequest(ctx common.ExtendedContext, illMessa w.WriteHeader(http.StatusOK) w.Write(output) } + func (h *MockIso18626Handler) HandleRequestingAgencyMessage(ctx common.ExtendedContext, illMessage *iso18626.ISO18626Message, w http.ResponseWriter) { status := iso18626.TypeMessageStatusOK if illMessage.RequestingAgencyMessage.Header.RequestingAgencyRequestId == "error" { @@ -348,7 +498,72 @@ func (h *MockIso18626Handler) HandleRequestingAgencyMessage(ctx common.ExtendedC w.Write(output) } -type MockIllRepo struct { +type MockLmsCreator struct { mock.Mock - ill_db.PgIllRepo + lms.LmsCreator +} + +func (m *MockLmsCreator) GetAdapter(ctx common.ExtendedContext, symbol pgtype.Text) (lms.LmsAdapter, error) { + args := m.Called(symbol) + return args.Get(0).(lms.LmsAdapter), args.Error(1) +} + +func createLmsAdapterMockFail() lms.LmsAdapter { + return &MockLmsAdapterFail{} +} + +type MockLmsAdapterFail struct { +} + +func (l *MockLmsAdapterFail) LookupUser(patron string) (string, error) { + return "", errors.New("LookupUser failed") +} + +func (l *MockLmsAdapterFail) AcceptItem( + itemId string, + requestId string, + userId string, + author string, + title string, + isbn string, + callNumber string, + pickupLocation string, + requestedAction string, +) error { + return errors.New("AcceptItem failed") +} + +func (l *MockLmsAdapterFail) DeleteItem(itemId string) error { + return errors.New("DeleteItem failed") +} + +func (l *MockLmsAdapterFail) RequestItem( + requestId string, + itemId string, + borrowerBarcode string, + pickupLocation string, + itemLocation string, +) error { + return errors.New("RequestItem failed") +} + +func (l *MockLmsAdapterFail) CancelRequestItem(requestId string, userId string) error { + return errors.New("CancelRequestItem failed") +} + +func (l *MockLmsAdapterFail) CheckInItem(itemId string) error { + return errors.New("CheckInItem failed") +} + +func (l *MockLmsAdapterFail) CheckOutItem( + requestId string, + itemBarcode string, + borrowerBarcode string, + externalReferenceValue string, +) error { + return errors.New("CheckOutItem failed") +} + +func (l *MockLmsAdapterFail) CreateUserFiscalTransaction(userId string, itemId string) error { + return errors.New("CreateUserFiscalTransaction failed") } diff --git a/broker/patron_request/service/message-handler_test.go b/broker/patron_request/service/message-handler_test.go index df11b4a4..a25530aa 100644 --- a/broker/patron_request/service/message-handler_test.go +++ b/broker/patron_request/service/message-handler_test.go @@ -9,6 +9,7 @@ import ( pr_db "github.com/indexdata/crosslink/broker/patron_request/db" "github.com/indexdata/crosslink/iso18626" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" ) func TestGetPatronRequestId(t *testing.T) { @@ -279,3 +280,8 @@ func TestHandleSupplyingAgencyMessageNoImplemented(t *testing.T) { assert.Equal(t, "status change no allowed", resp.SupplyingAgencyMessageConfirmation.ErrorData.ErrorValue) assert.Equal(t, "status change no allowed", err.Error()) } + +type MockIllRepo struct { + mock.Mock + ill_db.PgIllRepo +} diff --git a/broker/test/api/api-handler_test.go b/broker/test/api/api-handler_test.go index 1b7eacc5..d4a2f5c7 100644 --- a/broker/test/api/api-handler_test.go +++ b/broker/test/api/api-handler_test.go @@ -43,7 +43,7 @@ var illRepo ill_db.IllRepo var eventRepo events.EventRepo var mockIllRepoError = new(mocks.MockIllRepositoryError) var mockEventRepoError = new(mocks.MockEventRepositoryError) -var handlerMock = api.NewApiHandler(mockEventRepoError, mockIllRepoError, "", api.LIMIT_DEFAULT) +var handlerMock = api.NewApiHandler(mockEventRepoError, mockIllRepoError, common.NewTenantToSymbol(""), api.LIMIT_DEFAULT) func TestMain(m *testing.M) { app.TENANT_TO_SYMBOL = "ISIL:DK-{tenant}" diff --git a/broker/test/patron_request/api/api-handler_test.go b/broker/test/patron_request/api/api-handler_test.go index 76c06600..662a7552 100644 --- a/broker/test/patron_request/api/api-handler_test.go +++ b/broker/test/patron_request/api/api-handler_test.go @@ -30,7 +30,7 @@ var basePath = "/patron_requests" var illRepo ill_db.IllRepo func TestMain(m *testing.M) { - app.TENANT_TO_SYMBOL = "ISIL:DK-{tenant}" + app.TENANT_TO_SYMBOL = "" ctx := context.Background() pgContainer, err := postgres.Run(ctx, "postgres", @@ -166,7 +166,7 @@ func httpRequest(t *testing.T, method string, uriPath string, reqbytes []byte, e client := http.DefaultClient hreq, err := http.NewRequest(method, getLocalhostWithPort()+uriPath, bytes.NewBuffer(reqbytes)) assert.NoError(t, err) - hreq.Header.Set("X-Okapi-Tenant", "test-lib") + hreq.Header.Set("X-Okapi-Tenant", "testlib") if method == "POST" || method == "PUT" { hreq.Header.Set("Content-Type", "application/json") From fe6a02920dfef37db468fe50ed71b27b0d695eb7 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Fri, 12 Dec 2025 18:34:32 +0100 Subject: [PATCH 02/12] Map AcceptItem to NCIP --- broker/lms/lms_adapter_ncip.go | 35 ++++++++++++++++++++++++++++++++-- broker/lms/lms_adapter_test.go | 21 ++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/broker/lms/lms_adapter_ncip.go b/broker/lms/lms_adapter_ncip.go index 907076df..af3ddba4 100644 --- a/broker/lms/lms_adapter_ncip.go +++ b/broker/lms/lms_adapter_ncip.go @@ -70,9 +70,40 @@ func (l *LmsAdapterNcip) AcceptItem( pickupLocation string, requestedAction string, ) error { + var bibliographicItemId *ncip.BibliographicItemId + if isbn != "" { + bibliographicItemId = &ncip.BibliographicItemId{ + BibliographicItemIdentifier: isbn, + BibliographicItemIdentifierCode: &ncip.SchemeValuePair{Text: "ISBN"}, + } + } + biblioInfo := &ncip.BibliographicDescription{ + Author: author, + Title: title, + BibliographicItemId: bibliographicItemId, + } + var itemDescription *ncip.ItemDescription + if callNumber != "" { + itemDescription = &ncip.ItemDescription{CallNumber: callNumber} + } + itemOptionalFields := &ncip.ItemOptionalFields{ + BibliographicDescription: biblioInfo, + ItemDescription: itemDescription, + } + var pickupLocationField *ncip.SchemeValuePair + if pickupLocation != "" { + pickupLocationField = &ncip.SchemeValuePair{Text: pickupLocation} + } + if requestedAction == "" { + requestedAction = "Hold For Pickup" + } arg := ncip.AcceptItem{ - UserId: &ncip.UserId{UserIdentifierValue: userId}, - ItemId: &ncip.ItemId{ItemIdentifierValue: itemId}, + RequestId: ncip.RequestId{RequestIdentifierValue: requestId}, + RequestedActionType: ncip.SchemeValuePair{Text: requestedAction}, + UserId: &ncip.UserId{UserIdentifierValue: userId}, + ItemId: &ncip.ItemId{ItemIdentifierValue: itemId}, + ItemOptionalFields: itemOptionalFields, + PickupLocation: pickupLocationField, } _, err := l.ncipClient.AcceptItem(arg) return err diff --git a/broker/lms/lms_adapter_test.go b/broker/lms/lms_adapter_test.go index c7a9e0b3..6d383724 100644 --- a/broker/lms/lms_adapter_test.go +++ b/broker/lms/lms_adapter_test.go @@ -52,6 +52,27 @@ func TestAcceptItem(t *testing.T) { req := mock.(*ncipClientMock).lastRequest.(ncip.AcceptItem) assert.Equal(t, "testuser", req.UserId.UserIdentifierValue) assert.Equal(t, "item1", req.ItemId.ItemIdentifierValue) + assert.Equal(t, "req1", req.RequestId.RequestIdentifierValue) + assert.Equal(t, "author", req.ItemOptionalFields.BibliographicDescription.Author) + assert.Equal(t, "title", req.ItemOptionalFields.BibliographicDescription.Title) + assert.Equal(t, "isbn", req.ItemOptionalFields.BibliographicDescription.BibliographicItemId.BibliographicItemIdentifier) + assert.Equal(t, "ISBN", req.ItemOptionalFields.BibliographicDescription.BibliographicItemId.BibliographicItemIdentifierCode.Text) + assert.Equal(t, "callnum", req.ItemOptionalFields.ItemDescription.CallNumber) + assert.Equal(t, "loc", req.PickupLocation.Text) + assert.Equal(t, "action", req.RequestedActionType.Text) + + err = ad.AcceptItem("item1", "req1", "testuser", "author", "title", "", "", "", "") + assert.NoError(t, err) + req = mock.(*ncipClientMock).lastRequest.(ncip.AcceptItem) + assert.Equal(t, "testuser", req.UserId.UserIdentifierValue) + assert.Equal(t, "item1", req.ItemId.ItemIdentifierValue) + assert.Equal(t, "req1", req.RequestId.RequestIdentifierValue) + assert.Equal(t, "author", req.ItemOptionalFields.BibliographicDescription.Author) + assert.Equal(t, "title", req.ItemOptionalFields.BibliographicDescription.Title) + assert.Nil(t, req.ItemOptionalFields.BibliographicDescription.BibliographicItemId) + assert.Nil(t, req.ItemOptionalFields.ItemDescription) + assert.Nil(t, req.PickupLocation) + assert.Equal(t, "Hold For Pickup", req.RequestedActionType.Text) } func TestDeleteItem(t *testing.T) { From 29467036d56fa9150253256edcf4374ff49ff266 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Sat, 13 Dec 2025 12:45:47 +0100 Subject: [PATCH 03/12] Working on CheckoutItem, CheckInItem --- broker/lms/lms_adapter_ncip.go | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/broker/lms/lms_adapter_ncip.go b/broker/lms/lms_adapter_ncip.go index af3ddba4..fe1d0146 100644 --- a/broker/lms/lms_adapter_ncip.go +++ b/broker/lms/lms_adapter_ncip.go @@ -1,6 +1,7 @@ package lms import ( + "encoding/xml" "fmt" "net/http" @@ -10,6 +11,7 @@ import ( // NCIP LMS Adapter, based on: // https://github.com/openlibraryenvironment/mod-rs/blob/master/service/grails-app/services/org/olf/rs/hostlms/BaseHostLMSService.groovy +// https://github.com/openlibraryenvironment/lib-ncip-client/tree/master/lib-ncip-client/src/main/java/org/olf/rs/circ/client type LmsAdapterNcip struct { ncipInfo map[string]any @@ -44,10 +46,10 @@ func (l *LmsAdapterNcip) LookupUser(patron string) (string, error) { AuthenticationInputType: ncip.SchemeValuePair{Text: "username"}, AuthenticationInputData: patron, }) - elements := []ncip.SchemeValuePair{{Text: "User Id"}} + userElements := []ncip.SchemeValuePair{{Text: "User Id"}} arg = ncip.LookupUser{ AuthenticationInput: authenticationInput, - UserElementType: elements, + UserElementType: userElements, } response, err := l.ncipClient.LookupUser(arg) if err != nil { @@ -140,10 +142,15 @@ func (l *LmsAdapterNcip) CancelRequestItem(requestId string, userId string) erro } func (l *LmsAdapterNcip) CheckInItem(itemId string) error { + itemElements := []ncip.SchemeValuePair{ + {Text: "Bibliographic Description"}, + } arg := ncip.CheckInItem{ - ItemId: ncip.ItemId{ItemIdentifierValue: itemId}, + ItemId: ncip.ItemId{ItemIdentifierValue: itemId}, + ItemElementType: itemElements, } _, err := l.ncipClient.CheckInItem(arg) + // TODO: perhaps extend and return bibliographic info return err } @@ -153,8 +160,20 @@ func (l *LmsAdapterNcip) CheckOutItem( borrowerBarcode string, externalReferenceValue string, ) error { + var ext *ncip.Ext + if externalReferenceValue != "" { + externalId := ncip.RequestId{RequestIdentifierValue: externalReferenceValue} + bytes, err := xml.Marshal(externalId) + if err != nil { + return err + } + ext = &ncip.Ext{XMLContent: bytes} + } arg := ncip.CheckOutItem{ RequestId: &ncip.RequestId{RequestIdentifierValue: requestId}, + UserId: &ncip.UserId{UserIdentifierValue: borrowerBarcode}, + ItemId: ncip.ItemId{ItemIdentifierValue: itemBarcode}, + Ext: ext, } _, err := l.ncipClient.CheckOutItem(arg) return err From 465a4a9372d3aad917ecff37dbb8449b55889f58 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Sat, 13 Dec 2025 18:00:46 +0100 Subject: [PATCH 04/12] Further --- broker/lms/lms_adapter.go | 2 +- broker/lms/lms_adapter_mock.go | 4 +-- broker/lms/lms_adapter_ncip.go | 37 +++++++++++++++++--- broker/lms/lms_adapter_test.go | 28 ++++++++++++--- broker/ncipclient/ncipclient_impl.go | 3 -- broker/patron_request/service/action.go | 2 +- broker/patron_request/service/action_test.go | 4 +-- 7 files changed, 62 insertions(+), 18 deletions(-) diff --git a/broker/lms/lms_adapter.go b/broker/lms/lms_adapter.go index b62431ff..81150a76 100644 --- a/broker/lms/lms_adapter.go +++ b/broker/lms/lms_adapter.go @@ -17,7 +17,7 @@ type LmsAdapter interface { requestedAction string, ) error - DeleteItem(itemId string) error + DeleteItem(itemId string) (string, error) RequestItem( requestId string, diff --git a/broker/lms/lms_adapter_mock.go b/broker/lms/lms_adapter_mock.go index 25ffecb3..0ec69d3e 100644 --- a/broker/lms/lms_adapter_mock.go +++ b/broker/lms/lms_adapter_mock.go @@ -21,8 +21,8 @@ func (l *LmsAdapterMock) AcceptItem( return nil } -func (l *LmsAdapterMock) DeleteItem(itemId string) error { - return nil +func (l *LmsAdapterMock) DeleteItem(itemId string) (string, error) { + return itemId, nil } func (l *LmsAdapterMock) RequestItem( diff --git a/broker/lms/lms_adapter_ncip.go b/broker/lms/lms_adapter_ncip.go index fe1d0146..209b069b 100644 --- a/broker/lms/lms_adapter_ncip.go +++ b/broker/lms/lms_adapter_ncip.go @@ -111,13 +111,17 @@ func (l *LmsAdapterNcip) AcceptItem( return err } -func (l *LmsAdapterNcip) DeleteItem(itemId string) error { +func (l *LmsAdapterNcip) DeleteItem(itemId string) (string, error) { arg := ncip.DeleteItem{ ItemId: ncip.ItemId{ItemIdentifierValue: itemId}, } - _, err := l.ncipClient.DeleteItem(arg) - return err + res, err := l.ncipClient.DeleteItem(arg) + if err == nil && res != nil && res.ItemId != nil { + return res.ItemId.ItemIdentifierValue, nil + } + return itemId, err } + func (l *LmsAdapterNcip) RequestItem( requestId string, itemId string, @@ -125,8 +129,31 @@ func (l *LmsAdapterNcip) RequestItem( pickupLocation string, itemLocation string, ) error { + // mod-rs: see getRequestItemPickupLocation which in some cases overrides pickupLocation + var pickupLocationField *ncip.SchemeValuePair + if pickupLocation != "" { + pickupLocationField = &ncip.SchemeValuePair{Text: pickupLocation} + } + var userIdField *ncip.UserId + if borrowerBarcode != "" { + userIdField = &ncip.UserId{UserIdentifierValue: borrowerBarcode} + } + bibIdField := ncip.BibliographicId{ + BibliographicRecordId: &ncip.BibliographicRecordId{ + BibliographicRecordIdentifier: itemId, + BibliographicRecordIdentifierCode: &ncip.SchemeValuePair{Text: "SYSNUMBER"}, + }} + requestScopeTypeField := ncip.SchemeValuePair{Text: "Item"} // or "Title" + + // mod-rs: getRequestItemRequestType() + requestTypeField := ncip.SchemeValuePair{Text: "Page"} // "Loan" in Base arg := ncip.RequestItem{ - ItemId: []ncip.ItemId{{ItemIdentifierValue: itemId}}, + RequestId: &ncip.RequestId{RequestIdentifierValue: requestId}, + BibliographicId: []ncip.BibliographicId{bibIdField}, + UserId: userIdField, + PickupLocation: pickupLocationField, + RequestType: requestTypeField, + RequestScopeType: requestScopeTypeField, } _, err := l.ncipClient.RequestItem(arg) return err @@ -150,7 +177,7 @@ func (l *LmsAdapterNcip) CheckInItem(itemId string) error { ItemElementType: itemElements, } _, err := l.ncipClient.CheckInItem(arg) - // TODO: perhaps extend and return bibliographic info + // mod-rs does not seem to use the Bibliographic Description in response return err } diff --git a/broker/lms/lms_adapter_test.go b/broker/lms/lms_adapter_test.go index 6d383724..b8e27d90 100644 --- a/broker/lms/lms_adapter_test.go +++ b/broker/lms/lms_adapter_test.go @@ -1,6 +1,7 @@ package lms import ( + "encoding/xml" "fmt" "strings" "testing" @@ -81,10 +82,15 @@ func TestDeleteItem(t *testing.T) { ncipInfo: map[string]any{}, ncipClient: mock, } - err := ad.DeleteItem("item1") + res, err := ad.DeleteItem("item1") assert.NoError(t, err) + assert.Equal(t, "item1", res) req := mock.(*ncipClientMock).lastRequest.(ncip.DeleteItem) assert.Equal(t, "item1", req.ItemId.ItemIdentifierValue) + + _, err = ad.DeleteItem("error") + assert.Error(t, err) + assert.Equal(t, "deletion error", err.Error()) } func TestRequestItem(t *testing.T) { @@ -96,7 +102,12 @@ func TestRequestItem(t *testing.T) { err := ad.RequestItem("req1", "item1", "testuser", "loc", "itemloc") assert.NoError(t, err) req := mock.(*ncipClientMock).lastRequest.(ncip.RequestItem) - assert.Equal(t, "item1", req.ItemId[0].ItemIdentifierValue) + assert.Equal(t, "testuser", req.UserId.UserIdentifierValue) + assert.Equal(t, "item1", req.BibliographicId[0].BibliographicRecordId.BibliographicRecordIdentifier) + assert.Equal(t, "req1", req.RequestId.RequestIdentifierValue) + assert.Equal(t, "loc", req.PickupLocation.Text) + assert.Equal(t, "Page", req.RequestType.Text) + assert.Equal(t, "Item", req.RequestScopeType.Text) } func TestCancelRequestItem(t *testing.T) { @@ -122,6 +133,8 @@ func TestCheckInItem(t *testing.T) { assert.NoError(t, err) req := mock.(*ncipClientMock).lastRequest.(ncip.CheckInItem) assert.Equal(t, "item1", req.ItemId.ItemIdentifierValue) + assert.Equal(t, 1, len(req.ItemElementType)) + assert.Equal(t, "Bibliographic Description", req.ItemElementType[0].Text) } func TestCheckOutItem(t *testing.T) { @@ -134,6 +147,11 @@ func TestCheckOutItem(t *testing.T) { assert.NoError(t, err) req := mock.(*ncipClientMock).lastRequest.(ncip.CheckOutItem) assert.Equal(t, "req1", req.RequestId.RequestIdentifierValue) + assert.Equal(t, "item1", req.ItemId.ItemIdentifierValue) + assert.Equal(t, "barcodeid", req.UserId.UserIdentifierValue) + bytes, err := xml.Marshal(ncip.RequestId{RequestIdentifierValue: "extref"}) + assert.NoError(t, err) + assert.Equal(t, bytes, req.Ext.XMLContent) } func TestCreateUserFiscalTransaction(t *testing.T) { @@ -149,8 +167,7 @@ func TestCreateUserFiscalTransaction(t *testing.T) { } type ncipClientMock struct { - lastRequest any - LookupUserFunc func(lookup ncip.LookupUser) (*ncip.LookupUserResponse, error) + lastRequest any } func (n *ncipClientMock) LookupUser(lookup ncip.LookupUser) (*ncip.LookupUserResponse, error) { @@ -183,6 +200,9 @@ func (n *ncipClientMock) AcceptItem(accept ncip.AcceptItem) (*ncip.AcceptItemRes } func (n *ncipClientMock) DeleteItem(delete ncip.DeleteItem) (*ncip.DeleteItemResponse, error) { + if delete.ItemId.ItemIdentifierValue == "error" { + return nil, fmt.Errorf("deletion error") + } n.lastRequest = delete return nil, nil } diff --git a/broker/ncipclient/ncipclient_impl.go b/broker/ncipclient/ncipclient_impl.go index 78cf2366..7f39e6bb 100644 --- a/broker/ncipclient/ncipclient_impl.go +++ b/broker/ncipclient/ncipclient_impl.go @@ -26,9 +26,6 @@ func (n *NcipClientImpl) LookupUser(lookup ncip.LookupUser) (*ncip.LookupUserRes if handle { return nil, err } - if lookup.UserId != nil && lookup.UserId.UserIdentifierValue == "" { - return nil, fmt.Errorf("empty user Id") - } lookup.InitiationHeader = n.prepareHeader(lookup.InitiationHeader) ncipMessage := &ncip.NCIPMessage{ diff --git a/broker/patron_request/service/action.go b/broker/patron_request/service/action.go index ff4089f4..84a322fa 100644 --- a/broker/patron_request/service/action.go +++ b/broker/patron_request/service/action.go @@ -253,7 +253,7 @@ func (a *PatronRequestActionService) shipReturnBorrowingRequest(ctx common.Exten return events.LogErrorAndReturnResult(ctx, "failed to create LMS adapter", err) } item := "" // TODO Get item identifier from the request - err = lmsAdapter.DeleteItem(item) + _, err = lmsAdapter.DeleteItem(item) if err != nil { return events.LogErrorAndReturnResult(ctx, "LMS DeleteItem failed", err) } diff --git a/broker/patron_request/service/action_test.go b/broker/patron_request/service/action_test.go index ee64b3ff..8d2c7d99 100644 --- a/broker/patron_request/service/action_test.go +++ b/broker/patron_request/service/action_test.go @@ -533,8 +533,8 @@ func (l *MockLmsAdapterFail) AcceptItem( return errors.New("AcceptItem failed") } -func (l *MockLmsAdapterFail) DeleteItem(itemId string) error { - return errors.New("DeleteItem failed") +func (l *MockLmsAdapterFail) DeleteItem(itemId string) (string, error) { + return "", errors.New("DeleteItem failed") } func (l *MockLmsAdapterFail) RequestItem( From 32627258430a86033525948f629e748683637010 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Mon, 15 Dec 2025 13:43:51 +0100 Subject: [PATCH 05/12] ncip config parse in lms_adapter_ncip; simpler ncipclient --- broker/lms/lms_adapter_ncip.go | 60 ++++- broker/lms/lms_adapter_test.go | 60 ++++- broker/lms/lms_creator_test.go | 7 +- broker/ncipclient/ncipclient_impl.go | 67 ++--- broker/ncipclient/ncipclient_test.go | 360 +++++++-------------------- 5 files changed, 213 insertions(+), 341 deletions(-) diff --git a/broker/lms/lms_adapter_ncip.go b/broker/lms/lms_adapter_ncip.go index 209b069b..c135681a 100644 --- a/broker/lms/lms_adapter_ncip.go +++ b/broker/lms/lms_adapter_ncip.go @@ -9,21 +9,67 @@ import ( "github.com/indexdata/crosslink/ncip" ) +type NcipProperty string + +const ( + FromAgency NcipProperty = "from_agency" + FromAgencyAuthentication NcipProperty = "from_agency_authentication" + ToAgency NcipProperty = "to_agency" + Address NcipProperty = "address" +) + // NCIP LMS Adapter, based on: // https://github.com/openlibraryenvironment/mod-rs/blob/master/service/grails-app/services/org/olf/rs/hostlms/BaseHostLMSService.groovy // https://github.com/openlibraryenvironment/lib-ncip-client/tree/master/lib-ncip-client/src/main/java/org/olf/rs/circ/client type LmsAdapterNcip struct { - ncipInfo map[string]any - ncipClient ncipclient.NcipClient + ncipClient ncipclient.NcipClient + address string + toAgency string + fromAgency string + fromAgencyAuthentication string +} + +func setField(m map[string]any, field NcipProperty, dst *string) error { + v, ok := m[string(field)].(string) + if !ok || v == "" { + return fmt.Errorf("missing required NCIP configuration field: %s", field) + } + *dst = v + return nil +} + +func optField(m map[string]any, field NcipProperty, dst *string, def string) { + v, ok := m[string(field)].(string) + if !ok || v == "" { + *dst = def + } else { + *dst = v + } +} + +func (l *LmsAdapterNcip) parseConfig(ncipInfo map[string]any) error { + err := setField(ncipInfo, Address, &l.address) + if err != nil { + return err + } + err = setField(ncipInfo, FromAgency, &l.fromAgency) + if err != nil { + return err + } + optField(ncipInfo, FromAgencyAuthentication, &l.fromAgencyAuthentication, "") + optField(ncipInfo, ToAgency, &l.toAgency, "default-to-agency") + return nil } func CreateLmsAdapterNcip(ncipInfo map[string]any) (LmsAdapter, error) { - nc := ncipclient.NewNcipClient(http.DefaultClient, ncipInfo) - return &LmsAdapterNcip{ - ncipInfo: ncipInfo, - ncipClient: nc, - }, nil + l := &LmsAdapterNcip{} + err := l.parseConfig(ncipInfo) + if err != nil { + return nil, err + } + l.ncipClient = ncipclient.NewNcipClient(http.DefaultClient, l.address, l.fromAgency, l.toAgency, l.fromAgencyAuthentication) + return l, nil } func (l *LmsAdapterNcip) LookupUser(patron string) (string, error) { diff --git a/broker/lms/lms_adapter_test.go b/broker/lms/lms_adapter_test.go index b8e27d90..1aa67096 100644 --- a/broker/lms/lms_adapter_test.go +++ b/broker/lms/lms_adapter_test.go @@ -11,10 +11,61 @@ import ( "github.com/stretchr/testify/assert" ) +func TestParseConfigFull(t *testing.T) { + ncipConfig := map[string]any{ + "address": "http://ncip.example.com", + "from_agency": "AGENCY1", + "to_agency": "AGENCY2", + "from_agency_authentication": "auth", + } + lmsAdapaterNcip := &LmsAdapterNcip{} + err := lmsAdapaterNcip.parseConfig(ncipConfig) + assert.NoError(t, err) + assert.Equal(t, "http://ncip.example.com", lmsAdapaterNcip.address) + assert.Equal(t, "AGENCY1", lmsAdapaterNcip.fromAgency) + assert.Equal(t, "AGENCY2", lmsAdapaterNcip.toAgency) + assert.Equal(t, "auth", lmsAdapaterNcip.fromAgencyAuthentication) +} + +func TestParseConfigMissing(t *testing.T) { + // Missing required address field + ncipConfigMissing := map[string]any{ + "from_agency": "AGENCY1", + } + lmsAdapaterNcip := &LmsAdapterNcip{} + err := lmsAdapaterNcip.parseConfig(ncipConfigMissing) + assert.Error(t, err) + assert.Equal(t, "missing required NCIP configuration field: address", err.Error()) + + // Missing required from_agency field + ncipConfigMissing = map[string]any{ + "address": "http://ncip.example.com", + } + lmsAdapaterNcip = &LmsAdapterNcip{} + err = lmsAdapaterNcip.parseConfig(ncipConfigMissing) + assert.Error(t, err) + assert.Equal(t, "missing required NCIP configuration field: from_agency", err.Error()) +} + +func TestParseConfigOptional(t *testing.T) { + // Missing optional to_agency and from_agency_authentication fields + ncipConfig := map[string]any{ + "address": "http://ncip.example.com", + "from_agency": "AGENCY1", + "from_agency_authentication": "auth", + } + lmsAdapaterNcip := &LmsAdapterNcip{} + err := lmsAdapaterNcip.parseConfig(ncipConfig) + assert.NoError(t, err) + assert.Equal(t, "http://ncip.example.com", lmsAdapaterNcip.address) + assert.Equal(t, "AGENCY1", lmsAdapaterNcip.fromAgency) + assert.Equal(t, "default-to-agency", lmsAdapaterNcip.toAgency) + assert.Equal(t, "auth", lmsAdapaterNcip.fromAgencyAuthentication) +} + func TestLookupUser(t *testing.T) { var mock ncipclient.NcipClient = new(ncipClientMock) ad := &LmsAdapterNcip{ - ncipInfo: map[string]any{}, ncipClient: mock, } _, err := ad.LookupUser("") @@ -45,7 +96,6 @@ func TestLookupUser(t *testing.T) { func TestAcceptItem(t *testing.T) { var mock ncipclient.NcipClient = new(ncipClientMock) ad := &LmsAdapterNcip{ - ncipInfo: map[string]any{}, ncipClient: mock, } err := ad.AcceptItem("item1", "req1", "testuser", "author", "title", "isbn", "callnum", "loc", "action") @@ -79,7 +129,6 @@ func TestAcceptItem(t *testing.T) { func TestDeleteItem(t *testing.T) { var mock ncipclient.NcipClient = new(ncipClientMock) ad := &LmsAdapterNcip{ - ncipInfo: map[string]any{}, ncipClient: mock, } res, err := ad.DeleteItem("item1") @@ -96,7 +145,6 @@ func TestDeleteItem(t *testing.T) { func TestRequestItem(t *testing.T) { var mock ncipclient.NcipClient = new(ncipClientMock) ad := &LmsAdapterNcip{ - ncipInfo: map[string]any{}, ncipClient: mock, } err := ad.RequestItem("req1", "item1", "testuser", "loc", "itemloc") @@ -113,7 +161,6 @@ func TestRequestItem(t *testing.T) { func TestCancelRequestItem(t *testing.T) { var mock ncipclient.NcipClient = new(ncipClientMock) ad := &LmsAdapterNcip{ - ncipInfo: map[string]any{}, ncipClient: mock, } err := ad.CancelRequestItem("req1", "testuser") @@ -126,7 +173,6 @@ func TestCancelRequestItem(t *testing.T) { func TestCheckInItem(t *testing.T) { var mock ncipclient.NcipClient = new(ncipClientMock) ad := &LmsAdapterNcip{ - ncipInfo: map[string]any{}, ncipClient: mock, } err := ad.CheckInItem("item1") @@ -140,7 +186,6 @@ func TestCheckInItem(t *testing.T) { func TestCheckOutItem(t *testing.T) { var mock ncipclient.NcipClient = new(ncipClientMock) ad := &LmsAdapterNcip{ - ncipInfo: map[string]any{}, ncipClient: mock, } err := ad.CheckOutItem("req1", "item1", "barcodeid", "extref") @@ -157,7 +202,6 @@ func TestCheckOutItem(t *testing.T) { func TestCreateUserFiscalTransaction(t *testing.T) { var mock ncipclient.NcipClient = new(ncipClientMock) ad := &LmsAdapterNcip{ - ncipInfo: map[string]any{}, ncipClient: mock, } err := ad.CreateUserFiscalTransaction("testuser", "item1") diff --git a/broker/lms/lms_creator_test.go b/broker/lms/lms_creator_test.go index 86d10606..1e21302b 100644 --- a/broker/lms/lms_creator_test.go +++ b/broker/lms/lms_creator_test.go @@ -44,12 +44,15 @@ func TestGetAdapterNoPeers(t *testing.T) { assert.IsType(t, &LmsAdapterMock{}, LmsAdapter) } -func TestGetAdapterNcip(t *testing.T) { +func TestGetAdapterNcipMissing(t *testing.T) { illRepo := &MockIllRepo{} peer := ill_db.Peer{ CustomData: map[string]any{ "ncip": map[string]any{ - "some_key": "some_value", + "address": "http://ncip.example.com", + "from_agency": "AGENCY1", + "to_agency": "AGENCY2", + "from_agency_authentication": "auth", }, }, } diff --git a/broker/ncipclient/ncipclient_impl.go b/broker/ncipclient/ncipclient_impl.go index 7f39e6bb..3ec4e7ba 100644 --- a/broker/ncipclient/ncipclient_impl.go +++ b/broker/ncipclient/ncipclient_impl.go @@ -10,22 +10,24 @@ import ( ) type NcipClientImpl struct { - client *http.Client - ncipInfo map[string]any + client *http.Client + address string + fromAgency string + toAgency string + fromAgencyAuthentication string } -func NewNcipClient(client *http.Client, ncipInfo map[string]any) NcipClient { +func NewNcipClient(client *http.Client, address string, fromAgency string, toAgency string, fromAgencyAuthentication string) NcipClient { return &NcipClientImpl{ - client: client, - ncipInfo: ncipInfo, + client: client, + address: address, + fromAgency: fromAgency, + toAgency: toAgency, + fromAgencyAuthentication: fromAgencyAuthentication, } } func (n *NcipClientImpl) LookupUser(lookup ncip.LookupUser) (*ncip.LookupUserResponse, error) { - handle, _, err := n.checkMode(LookupUserMode) - if handle { - return nil, err - } lookup.InitiationHeader = n.prepareHeader(lookup.InitiationHeader) ncipMessage := &ncip.NCIPMessage{ @@ -43,10 +45,6 @@ func (n *NcipClientImpl) LookupUser(lookup ncip.LookupUser) (*ncip.LookupUserRes } func (n *NcipClientImpl) AcceptItem(accept ncip.AcceptItem) (*ncip.AcceptItemResponse, error) { - handle, _, err := n.checkMode(AcceptItemMode) - if handle { - return nil, err - } accept.InitiationHeader = n.prepareHeader(accept.InitiationHeader) ncipMessage := &ncip.NCIPMessage{ AcceptItem: &accept, @@ -79,10 +77,6 @@ func (n *NcipClientImpl) DeleteItem(delete ncip.DeleteItem) (*ncip.DeleteItemRes } func (n *NcipClientImpl) RequestItem(request ncip.RequestItem) (*ncip.RequestItemResponse, error) { - handle, _, err := n.checkMode(RequestItemMode) - if handle { - return nil, err - } request.InitiationHeader = n.prepareHeader(request.InitiationHeader) ncipMessage := &ncip.NCIPMessage{ RequestItem: &request, @@ -147,10 +141,6 @@ func (n *NcipClientImpl) CheckOutItem(request ncip.CheckOutItem) (*ncip.CheckOut } func (n *NcipClientImpl) CreateUserFiscalTransaction(request ncip.CreateUserFiscalTransaction) (*ncip.CreateUserFiscalTransactionResponse, error) { - handle, _, err := n.checkMode(CreateUserFiscalTransactionMode) - if handle { - return nil, err - } request.InitiationHeader = n.prepareHeader(request.InitiationHeader) ncipMessage := &ncip.NCIPMessage{ @@ -177,58 +167,37 @@ func (n *NcipClientImpl) checkProblem(op string, responseProblems []ncip.Problem return nil } -func (n *NcipClientImpl) checkMode(key NcipProperty) (bool, bool, error) { - mode, ok := n.ncipInfo[string(key)].(string) - if !ok { - return true, false, fmt.Errorf("missing %s in NCIP configuration", key) - } - switch mode { - case string(ModeDisabled): - return true, true, nil - case string(ModeManual): - return true, false, nil - case string(ModeAuto): - break - default: - return true, false, fmt.Errorf("unknown value for %s: %s", key, mode) - } - return false, false, nil -} - func (n *NcipClientImpl) prepareHeader(header *ncip.InitiationHeader) *ncip.InitiationHeader { if header == nil { header = &ncip.InitiationHeader{} } - from_agency, ok := n.ncipInfo[string(FromAgency)].(string) - if !ok || from_agency == "" { + from_agency := n.fromAgency + if from_agency == "" { from_agency = "default-from-agency" } header.FromAgencyId.AgencyId = ncip.SchemeValuePair{ Text: from_agency, } - to_agency, ok := n.ncipInfo[string(ToAgency)].(string) - if !ok || to_agency == "" { + to_agency := n.toAgency + if to_agency == "" { to_agency = "default-to-agency" } header.ToAgencyId.AgencyId = ncip.SchemeValuePair{ Text: to_agency, } - if auth, ok := n.ncipInfo[string(FromAgencyAuthentication)].(string); ok { - header.FromAgencyAuthentication = auth - } + header.FromAgencyAuthentication = n.fromAgencyAuthentication return header } func (n *NcipClientImpl) sendReceiveMessage(message *ncip.NCIPMessage) (*ncip.NCIPMessage, error) { - url, ok := n.ncipInfo[string(Address)].(string) - if !ok { + if n.address == "" { return nil, fmt.Errorf("missing NCIP address in configuration") } message.Version = ncip.NCIP_V2_02_XSD var respMessage ncip.NCIPMessage err := httpclient.NewClient().RequestResponse(n.client, http.MethodPost, []string{httpclient.ContentTypeApplicationXml}, - url, message, &respMessage, xml.Marshal, xml.Unmarshal) + n.address, message, &respMessage, xml.Marshal, xml.Unmarshal) if err != nil { return nil, fmt.Errorf("NCIP message exchange failed: %s", err.Error()) } diff --git a/broker/ncipclient/ncipclient_test.go b/broker/ncipclient/ncipclient_test.go index 4ac0add2..578605a3 100644 --- a/broker/ncipclient/ncipclient_test.go +++ b/broker/ncipclient/ncipclient_test.go @@ -33,25 +33,20 @@ func TestMain(m *testing.M) { os.Exit(m.Run()) } -func TestPrepareHeaderEmpty(t *testing.T) { - ncipData := make(map[string]any) - ncipClient := NcipClientImpl{} - ncipClient.ncipInfo = ncipData - - header := ncipClient.prepareHeader(nil) - assert.Equal(t, "default-from-agency", header.FromAgencyId.AgencyId.Text) - assert.Equal(t, "default-to-agency", header.ToAgencyId.AgencyId.Text) - assert.Equal(t, "", header.FromAgencyAuthentication) +func createTestClient() NcipClient { + return NewNcipClient(http.DefaultClient, + "http://localhost:"+os.Getenv("HTTP_PORT")+"/ncip", + "ILL-MOCK", + "ILL-MOCK", + "pass").(*NcipClientImpl) } func TestPrepareHeaderValues(t *testing.T) { ncipClient := NcipClientImpl{} - ncipData := make(map[string]any) - ncipClient.ncipInfo = ncipData - - ncipData["to_agency"] = "ILL-MOCK1" - ncipData["from_agency"] = "ILL-MOCK2" - ncipData["from_agency_authentication"] = "pass" + ncipClient.client = http.DefaultClient + ncipClient.fromAgency = "ILL-MOCK2" + ncipClient.toAgency = "ILL-MOCK1" + ncipClient.fromAgencyAuthentication = "pass" header := ncipClient.prepareHeader(nil) assert.Equal(t, "ILL-MOCK1", header.ToAgencyId.AgencyId.Text) @@ -60,14 +55,7 @@ func TestPrepareHeaderValues(t *testing.T) { } func TestLookupUserAutoOK(t *testing.T) { - ncipData := make(map[string]any) - ncipData["lookup_user_mode"] = "auto" - ncipData["from_agency"] = "ILL-MOCK" - ncipData["from_agency_authentication"] = "pass" - ncipData["to_agency"] = "ILL-MOCK" - ncipData["address"] = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" - - ncipClient := NewNcipClient(http.DefaultClient, ncipData) + ncipClient := createTestClient() lookup := ncip.LookupUser{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", @@ -79,14 +67,7 @@ func TestLookupUserAutoOK(t *testing.T) { } func TestLookupUserAutoInvalidUser(t *testing.T) { - ncipData := make(map[string]any) - ncipData["lookup_user_mode"] = "auto" - ncipData["from_agency"] = "ILL-MOCK" - ncipData["from_agency_authentication"] = "pass" - ncipData["to_agency"] = "ILL-MOCK" - ncipData["address"] = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" - - ncipClient := NewNcipClient(http.DefaultClient, ncipData) + ncipClient := createTestClient() lookup := ncip.LookupUser{ UserId: &ncip.UserId{ UserIdentifierValue: "foo", @@ -97,43 +78,12 @@ func TestLookupUserAutoInvalidUser(t *testing.T) { assert.Equal(t, "NCIP user lookup failed: Unknown User: foo", err.Error()) } -func TestLookupUserModeManual(t *testing.T) { - ncipData := make(map[string]any) - ncipData["lookup_user_mode"] = "manual" - - ncipClient := NewNcipClient(http.DefaultClient, ncipData) - lookup := ncip.LookupUser{ - UserId: &ncip.UserId{ - UserIdentifierValue: "validuser", - }, - } - res, err := ncipClient.LookupUser(lookup) - assert.NoError(t, err) - assert.Nil(t, res) -} - -func TestLookupUserModeDisabled(t *testing.T) { - ncipData := make(map[string]any) - ncipData["lookup_user_mode"] = "disabled" - - ncipClient := NewNcipClient(http.DefaultClient, ncipData) - lookup := ncip.LookupUser{ - UserId: &ncip.UserId{ - UserIdentifierValue: "validuser", - }, - } - res, err := ncipClient.LookupUser(lookup) - assert.NoError(t, err) - assert.Nil(t, res) -} - func TestLookupUserMissingAddress(t *testing.T) { - ncipData := make(map[string]any) - ncipData["lookup_user_mode"] = "auto" - ncipData["from_agency"] = "ILL-MOCK" - ncipData["to_agency"] = "ILL-MOCK" + ncipClient := NcipClientImpl{} + ncipClient.client = http.DefaultClient + ncipClient.fromAgency = "ILL-MOCK" + ncipClient.toAgency = "ILL-MOCK" - ncipClient := NewNcipClient(http.DefaultClient, ncipData) lookup := ncip.LookupUser{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", @@ -145,39 +95,6 @@ func TestLookupUserMissingAddress(t *testing.T) { assert.Nil(t, res) } -func TestLookupUserMissingAuthUserInfo(t *testing.T) { - ncipData := make(map[string]any) - ncipData["from_agency"] = "ILL-MOCK" - ncipData["to_agency"] = "ILL-MOCK" - - ncipClient := NewNcipClient(http.DefaultClient, ncipData) - lookup := ncip.LookupUser{ - UserId: &ncip.UserId{ - UserIdentifierValue: "validuser", - }, - } - _, err := ncipClient.LookupUser(lookup) - assert.Error(t, err) - assert.Equal(t, "missing lookup_user_mode in NCIP configuration", err.Error()) -} - -func TestLookupUserBadMode(t *testing.T) { - ncipData := make(map[string]any) - ncipData["lookup_user_mode"] = "foo" - ncipData["from_agency"] = "ILL-MOCK" - ncipData["to_agency"] = "ILL-MOCK" - - ncipClient := NewNcipClient(http.DefaultClient, ncipData) - lookup := ncip.LookupUser{ - UserId: &ncip.UserId{ - UserIdentifierValue: "validuser", - }, - } - _, err := ncipClient.LookupUser(lookup) - assert.Error(t, err) - assert.Equal(t, "unknown value for lookup_user_mode: foo", err.Error()) -} - func TestBadNcipMessageResponse(t *testing.T) { handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/xml") @@ -188,17 +105,13 @@ func TestBadNcipMessageResponse(t *testing.T) { server := httptest.NewServer(handler) defer server.Close() - ncipData := make(map[string]any) - ncipData["lookup_user_mode"] = "auto" - ncipData["accept_item_mode"] = "auto" - ncipData["request_item_mode"] = "auto" - ncipData["create_user_fiscal_transaction_mode"] = "auto" - ncipData["from_agency"] = "ILL-MOCK" - ncipData["from_agency_authentication"] = "pass" - ncipData["to_agency"] = "ILL-MOCK" - ncipData["address"] = server.URL - - ncipClient := NewNcipClient(http.DefaultClient, ncipData) + ncipClient := NcipClientImpl{} + ncipClient.client = http.DefaultClient + ncipClient.fromAgency = "ILL-MOCK" + ncipClient.fromAgencyAuthentication = "pass" + ncipClient.toAgency = "ILL-MOCK" + ncipClient.address = server.URL + lookup := ncip.LookupUser{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", @@ -265,17 +178,12 @@ func TestEmptyNcipResponse(t *testing.T) { server := httptest.NewServer(handler) defer server.Close() - ncipData := make(map[string]any) - ncipData["lookup_user_mode"] = "auto" - ncipData["accept_item_mode"] = "auto" - ncipData["request_item_mode"] = "auto" - ncipData["create_user_fiscal_transaction_mode"] = "auto" - ncipData["from_agency"] = "ILL-MOCK" - ncipData["from_agency_authentication"] = "pass" - ncipData["to_agency"] = "ILL-MOCK" - ncipData["address"] = server.URL - - ncipClient := NewNcipClient(http.DefaultClient, ncipData) + ncipClient := NcipClientImpl{} + ncipClient.client = http.DefaultClient + ncipClient.fromAgency = "ILL-MOCK" + ncipClient.fromAgencyAuthentication = "pass" + ncipClient.toAgency = "ILL-MOCK" + ncipClient.address = server.URL lookup := ncip.LookupUser{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", @@ -353,14 +261,13 @@ func TestLookupUserProblemResponse(t *testing.T) { server := httptest.NewServer(handler) defer server.Close() - ncipData := make(map[string]any) - ncipData["lookup_user_mode"] = "auto" - ncipData["from_agency"] = "ILL-MOCK" - ncipData["from_agency_authentication"] = "pass" - ncipData["to_agency"] = "ILL-MOCK" - ncipData["address"] = server.URL + ncipClient := NcipClientImpl{} + ncipClient.client = http.DefaultClient + ncipClient.fromAgency = "ILL-MOCK" + ncipClient.fromAgencyAuthentication = "pass" + ncipClient.toAgency = "ILL-MOCK" + ncipClient.address = server.URL - ncipClient := NewNcipClient(http.DefaultClient, ncipData) lookup := ncip.LookupUser{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", @@ -372,14 +279,12 @@ func TestLookupUserProblemResponse(t *testing.T) { } func TestAcceptItemOK(t *testing.T) { - ncipData := make(map[string]any) - ncipData["accept_item_mode"] = "auto" - ncipData["from_agency"] = "ILL-MOCK" - ncipData["from_agency_authentication"] = "pass" - ncipData["to_agency"] = "ILL-MOCK" - ncipData["address"] = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" - - ncipClient := NewNcipClient(http.DefaultClient, ncipData) + ncipClient := NcipClientImpl{} + ncipClient.client = http.DefaultClient + ncipClient.fromAgency = "ILL-MOCK" + ncipClient.fromAgencyAuthentication = "pass" + ncipClient.toAgency = "ILL-MOCK" + ncipClient.address = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" accept := ncip.AcceptItem{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", @@ -394,14 +299,12 @@ func TestAcceptItemOK(t *testing.T) { } func TestAcceptItemInvalidUser(t *testing.T) { - ncipData := make(map[string]any) - ncipData["accept_item_mode"] = "auto" - ncipData["from_agency"] = "ILL-MOCK" - ncipData["from_agency_authentication"] = "pass" - ncipData["to_agency"] = "ILL-MOCK" - ncipData["address"] = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" - - ncipClient := NewNcipClient(http.DefaultClient, ncipData) + ncipClient := NcipClientImpl{} + ncipClient.client = http.DefaultClient + ncipClient.fromAgency = "ILL-MOCK" + ncipClient.fromAgencyAuthentication = "pass" + ncipClient.toAgency = "ILL-MOCK" + ncipClient.address = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" accept := ncip.AcceptItem{ UserId: &ncip.UserId{ UserIdentifierValue: "foo", @@ -415,63 +318,28 @@ func TestAcceptItemInvalidUser(t *testing.T) { assert.Equal(t, "NCIP accept item failed: Unknown User: foo", err.Error()) } -func TestAcceptItemModeManual(t *testing.T) { - ncipData := make(map[string]any) - ncipData["accept_item_mode"] = "manual" - - lookup := ncip.AcceptItem{ - UserId: &ncip.UserId{ - UserIdentifierValue: "validuser", - }, - RequestId: ncip.RequestId{ - RequestIdentifierValue: "validrequest", - }, - } - ncipClient := NewNcipClient(http.DefaultClient, ncipData) - res, err := ncipClient.AcceptItem(lookup) - assert.NoError(t, err) - assert.Nil(t, res) -} - -func TestAcceptItemMissingNcipInfo(t *testing.T) { - ncipClient := NewNcipClient(http.DefaultClient, nil) - accept := ncip.AcceptItem{} - _, err := ncipClient.AcceptItem(accept) - assert.Error(t, err) - assert.Equal(t, "missing accept_item_mode in NCIP configuration", err.Error()) -} - func TestDeleteItemOK(t *testing.T) { - ncipData := make(map[string]any) - ncipData["from_agency"] = "ILL-MOCK" - ncipData["from_agency_authentication"] = "pass" - ncipData["to_agency"] = "ILL-MOCK" - ncipData["address"] = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" + ncipClient := NcipClientImpl{} + ncipClient.client = http.DefaultClient + ncipClient.fromAgency = "ILL-MOCK" + ncipClient.fromAgencyAuthentication = "pass" + ncipClient.toAgency = "ILL-MOCK" + ncipClient.address = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" - ncipClient := NewNcipClient(http.DefaultClient, ncipData) delete := ncip.DeleteItem{} res, err := ncipClient.DeleteItem(delete) assert.NoError(t, err) assert.NotNil(t, res) } -func TestDeleteItemMissingNcipInfo(t *testing.T) { - ncipClient := NewNcipClient(http.DefaultClient, nil) - delete := ncip.DeleteItem{} - _, err := ncipClient.DeleteItem(delete) - assert.Error(t, err) - assert.Equal(t, "missing NCIP address in configuration", err.Error()) -} - func TestRequestItemOK(t *testing.T) { - ncipData := make(map[string]any) - ncipData["request_item_mode"] = "auto" - ncipData["from_agency"] = "ILL-MOCK" - ncipData["from_agency_authentication"] = "pass" - ncipData["to_agency"] = "ILL-MOCK" - ncipData["address"] = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" - - ncipClient := NewNcipClient(http.DefaultClient, ncipData) + ncipClient := NcipClientImpl{} + ncipClient.client = http.DefaultClient + ncipClient.fromAgency = "ILL-MOCK" + ncipClient.fromAgencyAuthentication = "pass" + ncipClient.toAgency = "ILL-MOCK" + ncipClient.address = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" + request := ncip.RequestItem{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", @@ -490,29 +358,14 @@ func TestRequestItemOK(t *testing.T) { assert.NoError(t, err) } -func TestRequestItemModeManual(t *testing.T) { - ncipData := make(map[string]any) - ncipData["request_item_mode"] = "manual" - - ncipClient := NewNcipClient(http.DefaultClient, ncipData) - lookup := ncip.RequestItem{ - UserId: &ncip.UserId{ - UserIdentifierValue: "validuser", - }, - } - res, err := ncipClient.RequestItem(lookup) - assert.NoError(t, err) - assert.Nil(t, res) -} - func TestCancelRequestItemOK(t *testing.T) { - ncipData := make(map[string]any) - ncipData["from_agency"] = "ILL-MOCK" - ncipData["from_agency_authentication"] = "pass" - ncipData["to_agency"] = "ILL-MOCK" - ncipData["address"] = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" + ncipClient := NcipClientImpl{} + ncipClient.client = http.DefaultClient + ncipClient.fromAgency = "ILL-MOCK" + ncipClient.fromAgencyAuthentication = "pass" + ncipClient.toAgency = "ILL-MOCK" + ncipClient.address = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" - ncipClient := NewNcipClient(http.DefaultClient, ncipData) request := ncip.CancelRequestItem{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", @@ -530,13 +383,12 @@ func TestCancelRequestItemOK(t *testing.T) { } func TestCheckInItemOK(t *testing.T) { - ncipData := make(map[string]any) - ncipData["from_agency"] = "ILL-MOCK" - ncipData["from_agency_authentication"] = "pass" - ncipData["to_agency"] = "ILL-MOCK" - ncipData["address"] = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" - - ncipClient := NewNcipClient(http.DefaultClient, ncipData) + ncipClient := NcipClientImpl{} + ncipClient.client = http.DefaultClient + ncipClient.fromAgency = "ILL-MOCK" + ncipClient.fromAgencyAuthentication = "pass" + ncipClient.toAgency = "ILL-MOCK" + ncipClient.address = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" request := ncip.CheckInItem{ ItemId: ncip.ItemId{ ItemIdentifierValue: "item-001", @@ -547,22 +399,14 @@ func TestCheckInItemOK(t *testing.T) { assert.NotNil(t, res) } -func TestCheckInItemMissingNcipInfo(t *testing.T) { - ncipClient := NewNcipClient(http.DefaultClient, nil) - request := ncip.CheckInItem{} - _, err := ncipClient.CheckInItem(request) - assert.Error(t, err) - assert.Equal(t, "missing NCIP address in configuration", err.Error()) -} - func TestCheckOutItemOK(t *testing.T) { - ncipData := make(map[string]any) - ncipData["from_agency"] = "ILL-MOCK" - ncipData["from_agency_authentication"] = "pass" - ncipData["to_agency"] = "ILL-MOCK" - ncipData["address"] = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" + ncipClient := NcipClientImpl{} + ncipClient.client = http.DefaultClient + ncipClient.fromAgency = "ILL-MOCK" + ncipClient.fromAgencyAuthentication = "pass" + ncipClient.toAgency = "ILL-MOCK" + ncipClient.address = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" - ncipClient := NewNcipClient(http.DefaultClient, ncipData) request := ncip.CheckOutItem{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", @@ -575,23 +419,14 @@ func TestCheckOutItemOK(t *testing.T) { assert.NoError(t, err) } -func TestCheckOutItemMissingNcipInfo(t *testing.T) { - ncipClient := NewNcipClient(http.DefaultClient, nil) - request := ncip.CheckOutItem{} - _, err := ncipClient.CheckOutItem(request) - assert.Error(t, err) - assert.Equal(t, "missing NCIP address in configuration", err.Error()) -} - func TestCreateUserFiscalTransactionOK(t *testing.T) { - ncipData := make(map[string]any) - ncipData["create_user_fiscal_transaction_mode"] = "auto" - ncipData["from_agency"] = "ILL-MOCK" - ncipData["from_agency_authentication"] = "pass" - ncipData["to_agency"] = "ILL-MOCK" - ncipData["address"] = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" - - ncipClient := NewNcipClient(http.DefaultClient, ncipData) + ncipClient := NcipClientImpl{} + ncipClient.client = http.DefaultClient + ncipClient.fromAgency = "ILL-MOCK" + ncipClient.fromAgencyAuthentication = "pass" + ncipClient.toAgency = "ILL-MOCK" + ncipClient.address = "http://localhost:" + os.Getenv("HTTP_PORT") + "/ncip" + lookup := ncip.CreateUserFiscalTransaction{ UserId: &ncip.UserId{ UserIdentifierValue: "validuser", @@ -607,28 +442,3 @@ func TestCreateUserFiscalTransactionOK(t *testing.T) { assert.NoError(t, err) assert.NotNil(t, res) } - -func TestCreateUserFiscalTransactionBadMode(t *testing.T) { - ncipData := make(map[string]any) - ncipData["create_user_fiscal_transaction_mode"] = "foo" - ncipData["from_agency"] = "ILL-MOCK" - ncipData["to_agency"] = "ILL-MOCK" - - ncipClient := NewNcipClient(http.DefaultClient, ncipData) - lookup := ncip.CreateUserFiscalTransaction{ - UserId: &ncip.UserId{ - UserIdentifierValue: "validuser", - }, - } - _, err := ncipClient.CreateUserFiscalTransaction(lookup) - assert.Error(t, err) - assert.Equal(t, "unknown value for create_user_fiscal_transaction_mode: foo", err.Error()) -} - -func TestCreateUserFiscalTransactionMissingNcipInfo(t *testing.T) { - ncipClient := NewNcipClient(http.DefaultClient, nil) - request := ncip.CreateUserFiscalTransaction{} - _, err := ncipClient.CreateUserFiscalTransaction(request) - assert.Error(t, err) - assert.Equal(t, "missing create_user_fiscal_transaction_mode in NCIP configuration", err.Error()) -} From 2819e894fa96526f8600f1db9bd3adf8b078c529 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Mon, 15 Dec 2025 15:02:55 +0100 Subject: [PATCH 06/12] Enable flags for LookupUser, ... --- broker/lms/lms_adapter.go | 2 +- broker/lms/lms_adapter_mock.go | 4 +- broker/lms/lms_adapter_ncip.go | 77 +++++++++++--- broker/lms/lms_adapter_test.go | 102 ++++++++++++++----- broker/lms/lms_creator_test.go | 18 +++- broker/ncipclient/ncipclient_impl.go | 12 +-- broker/patron_request/service/action.go | 2 +- broker/patron_request/service/action_test.go | 4 +- 8 files changed, 165 insertions(+), 56 deletions(-) diff --git a/broker/lms/lms_adapter.go b/broker/lms/lms_adapter.go index 81150a76..b62431ff 100644 --- a/broker/lms/lms_adapter.go +++ b/broker/lms/lms_adapter.go @@ -17,7 +17,7 @@ type LmsAdapter interface { requestedAction string, ) error - DeleteItem(itemId string) (string, error) + DeleteItem(itemId string) error RequestItem( requestId string, diff --git a/broker/lms/lms_adapter_mock.go b/broker/lms/lms_adapter_mock.go index 0ec69d3e..25ffecb3 100644 --- a/broker/lms/lms_adapter_mock.go +++ b/broker/lms/lms_adapter_mock.go @@ -21,8 +21,8 @@ func (l *LmsAdapterMock) AcceptItem( return nil } -func (l *LmsAdapterMock) DeleteItem(itemId string) (string, error) { - return itemId, nil +func (l *LmsAdapterMock) DeleteItem(itemId string) error { + return nil } func (l *LmsAdapterMock) RequestItem( diff --git a/broker/lms/lms_adapter_ncip.go b/broker/lms/lms_adapter_ncip.go index c135681a..eda3a4e6 100644 --- a/broker/lms/lms_adapter_ncip.go +++ b/broker/lms/lms_adapter_ncip.go @@ -16,6 +16,22 @@ const ( FromAgencyAuthentication NcipProperty = "from_agency_authentication" ToAgency NcipProperty = "to_agency" Address NcipProperty = "address" + LookupUserEnable NcipProperty = "lookup_user_enable" + AcceptItemEnable NcipProperty = "accept_item_enable" + CheckInItemEnable NcipProperty = "check_in_item_enable" + CheckOutItemEnable NcipProperty = "check_out_item_enable" +) + +type NcipUserElement string + +const ( + NCIPUserId NcipUserElement = "User Id" +) + +type NcipItemElement string + +const ( + NCIPBibliographicDescription NcipItemElement = "Bibliographic Description" ) // NCIP LMS Adapter, based on: @@ -28,19 +44,23 @@ type LmsAdapterNcip struct { toAgency string fromAgency string fromAgencyAuthentication string + lookupUserEnable bool + acceptItemEnable bool + checkInItemEnable bool + checkOutItemEnable bool } -func setField(m map[string]any, field NcipProperty, dst *string) error { - v, ok := m[string(field)].(string) +func setField(m map[string]any, key NcipProperty, dst *string) error { + v, ok := m[string(key)].(string) if !ok || v == "" { - return fmt.Errorf("missing required NCIP configuration field: %s", field) + return fmt.Errorf("missing required NCIP configuration field: %s", key) } *dst = v return nil } -func optField(m map[string]any, field NcipProperty, dst *string, def string) { - v, ok := m[string(field)].(string) +func optField(m map[string]any, key NcipProperty, dst *string, def string) { + v, ok := m[string(key)].(string) if !ok || v == "" { *dst = def } else { @@ -48,6 +68,15 @@ func optField(m map[string]any, field NcipProperty, dst *string, def string) { } } +func optBoolField(m map[string]any, key NcipProperty, dst *bool, def bool) { + v, ok := m[string(key)].(bool) + if !ok { + *dst = def + } else { + *dst = v + } +} + func (l *LmsAdapterNcip) parseConfig(ncipInfo map[string]any) error { err := setField(ncipInfo, Address, &l.address) if err != nil { @@ -59,6 +88,10 @@ func (l *LmsAdapterNcip) parseConfig(ncipInfo map[string]any) error { } optField(ncipInfo, FromAgencyAuthentication, &l.fromAgencyAuthentication, "") optField(ncipInfo, ToAgency, &l.toAgency, "default-to-agency") + optBoolField(ncipInfo, LookupUserEnable, &l.lookupUserEnable, true) + optBoolField(ncipInfo, AcceptItemEnable, &l.acceptItemEnable, true) + optBoolField(ncipInfo, CheckInItemEnable, &l.checkInItemEnable, true) + optBoolField(ncipInfo, CheckOutItemEnable, &l.checkOutItemEnable, true) return nil } @@ -73,6 +106,9 @@ func CreateLmsAdapterNcip(ncipInfo map[string]any) (LmsAdapter, error) { } func (l *LmsAdapterNcip) LookupUser(patron string) (string, error) { + if !l.lookupUserEnable { + return patron, nil // could even be empty + } if patron == "" { return "", fmt.Errorf("empty patron identifier") } @@ -92,7 +128,7 @@ func (l *LmsAdapterNcip) LookupUser(patron string) (string, error) { AuthenticationInputType: ncip.SchemeValuePair{Text: "username"}, AuthenticationInputData: patron, }) - userElements := []ncip.SchemeValuePair{{Text: "User Id"}} + userElements := []ncip.SchemeValuePair{{Text: string(NCIPUserId)}} arg = ncip.LookupUser{ AuthenticationInput: authenticationInput, UserElementType: userElements, @@ -101,10 +137,13 @@ func (l *LmsAdapterNcip) LookupUser(patron string) (string, error) { if err != nil { return "", err } - if response.UserId == nil { - return "", fmt.Errorf("missing User ID in LookupUser response") + if response.UserOptionalFields != nil && len(response.UserOptionalFields.UserId) != 0 { + return response.UserOptionalFields.UserId[0].UserIdentifierValue, nil + } + if response.UserId != nil { + return response.UserId.UserIdentifierValue, nil } - return response.UserId.UserIdentifierValue, nil + return "", fmt.Errorf("missing User ID in LookupUser response") } func (l *LmsAdapterNcip) AcceptItem( @@ -118,6 +157,9 @@ func (l *LmsAdapterNcip) AcceptItem( pickupLocation string, requestedAction string, ) error { + if !l.acceptItemEnable { + return nil + } var bibliographicItemId *ncip.BibliographicItemId if isbn != "" { bibliographicItemId = &ncip.BibliographicItemId{ @@ -157,15 +199,12 @@ func (l *LmsAdapterNcip) AcceptItem( return err } -func (l *LmsAdapterNcip) DeleteItem(itemId string) (string, error) { +func (l *LmsAdapterNcip) DeleteItem(itemId string) error { arg := ncip.DeleteItem{ ItemId: ncip.ItemId{ItemIdentifierValue: itemId}, } - res, err := l.ncipClient.DeleteItem(arg) - if err == nil && res != nil && res.ItemId != nil { - return res.ItemId.ItemIdentifierValue, nil - } - return itemId, err + _, err := l.ncipClient.DeleteItem(arg) + return err } func (l *LmsAdapterNcip) RequestItem( @@ -215,8 +254,11 @@ func (l *LmsAdapterNcip) CancelRequestItem(requestId string, userId string) erro } func (l *LmsAdapterNcip) CheckInItem(itemId string) error { + if !l.checkInItemEnable { + return nil + } itemElements := []ncip.SchemeValuePair{ - {Text: "Bibliographic Description"}, + {Text: string(NCIPBibliographicDescription)}, } arg := ncip.CheckInItem{ ItemId: ncip.ItemId{ItemIdentifierValue: itemId}, @@ -233,6 +275,9 @@ func (l *LmsAdapterNcip) CheckOutItem( borrowerBarcode string, externalReferenceValue string, ) error { + if !l.checkOutItemEnable { + return nil + } var ext *ncip.Ext if externalReferenceValue != "" { externalId := ncip.RequestId{RequestIdentifierValue: externalReferenceValue} diff --git a/broker/lms/lms_adapter_test.go b/broker/lms/lms_adapter_test.go index 1aa67096..fe25a927 100644 --- a/broker/lms/lms_adapter_test.go +++ b/broker/lms/lms_adapter_test.go @@ -17,6 +17,10 @@ func TestParseConfigFull(t *testing.T) { "from_agency": "AGENCY1", "to_agency": "AGENCY2", "from_agency_authentication": "auth", + "lookup_user_enable": false, + "accept_item_enable": false, + "check_in_item_enable": false, + "check_out_item_enable": false, } lmsAdapaterNcip := &LmsAdapterNcip{} err := lmsAdapaterNcip.parseConfig(ncipConfig) @@ -25,6 +29,28 @@ func TestParseConfigFull(t *testing.T) { assert.Equal(t, "AGENCY1", lmsAdapaterNcip.fromAgency) assert.Equal(t, "AGENCY2", lmsAdapaterNcip.toAgency) assert.Equal(t, "auth", lmsAdapaterNcip.fromAgencyAuthentication) + assert.False(t, lmsAdapaterNcip.lookupUserEnable) + assert.False(t, lmsAdapaterNcip.acceptItemEnable) + assert.False(t, lmsAdapaterNcip.checkInItemEnable) +} + +func TestParseConfigOptional(t *testing.T) { + // Missing optional to_agency and from_agency_authentication fields + ncipConfig := map[string]any{ + "address": "http://ncip.example.com", + "from_agency": "AGENCY1", + "from_agency_authentication": "auth", + } + lmsAdapaterNcip := &LmsAdapterNcip{} + err := lmsAdapaterNcip.parseConfig(ncipConfig) + assert.NoError(t, err) + assert.Equal(t, "http://ncip.example.com", lmsAdapaterNcip.address) + assert.Equal(t, "AGENCY1", lmsAdapaterNcip.fromAgency) + assert.Equal(t, "default-to-agency", lmsAdapaterNcip.toAgency) + assert.Equal(t, "auth", lmsAdapaterNcip.fromAgencyAuthentication) + assert.True(t, lmsAdapaterNcip.lookupUserEnable) + assert.True(t, lmsAdapaterNcip.acceptItemEnable) + assert.True(t, lmsAdapaterNcip.checkInItemEnable) } func TestParseConfigMissing(t *testing.T) { @@ -47,26 +73,11 @@ func TestParseConfigMissing(t *testing.T) { assert.Equal(t, "missing required NCIP configuration field: from_agency", err.Error()) } -func TestParseConfigOptional(t *testing.T) { - // Missing optional to_agency and from_agency_authentication fields - ncipConfig := map[string]any{ - "address": "http://ncip.example.com", - "from_agency": "AGENCY1", - "from_agency_authentication": "auth", - } - lmsAdapaterNcip := &LmsAdapterNcip{} - err := lmsAdapaterNcip.parseConfig(ncipConfig) - assert.NoError(t, err) - assert.Equal(t, "http://ncip.example.com", lmsAdapaterNcip.address) - assert.Equal(t, "AGENCY1", lmsAdapaterNcip.fromAgency) - assert.Equal(t, "default-to-agency", lmsAdapaterNcip.toAgency) - assert.Equal(t, "auth", lmsAdapaterNcip.fromAgencyAuthentication) -} - func TestLookupUser(t *testing.T) { var mock ncipclient.NcipClient = new(ncipClientMock) ad := &LmsAdapterNcip{ - ncipClient: mock, + ncipClient: mock, + lookupUserEnable: true, } _, err := ad.LookupUser("") assert.Error(t, err) @@ -90,13 +101,30 @@ func TestLookupUser(t *testing.T) { userId, err = ad.LookupUser("good user") assert.NoError(t, err) + assert.Equal(t, "user124", userId) + + userId, err = ad.LookupUser("other user") + assert.NoError(t, err) assert.Equal(t, "user123", userId) + + ad.lookupUserEnable = false + userId, err = ad.LookupUser("") + assert.NoError(t, err) + assert.Equal(t, "", userId) + + ad.lookupUserEnable = false + mock.(*ncipClientMock).lastRequest = nil + userId, err = ad.LookupUser("anyuser") + assert.NoError(t, err) + assert.Equal(t, "anyuser", userId) + assert.Nil(t, mock.(*ncipClientMock).lastRequest) // not called } func TestAcceptItem(t *testing.T) { var mock ncipclient.NcipClient = new(ncipClientMock) ad := &LmsAdapterNcip{ - ncipClient: mock, + acceptItemEnable: true, + ncipClient: mock, } err := ad.AcceptItem("item1", "req1", "testuser", "author", "title", "isbn", "callnum", "loc", "action") assert.NoError(t, err) @@ -124,6 +152,12 @@ func TestAcceptItem(t *testing.T) { assert.Nil(t, req.ItemOptionalFields.ItemDescription) assert.Nil(t, req.PickupLocation) assert.Equal(t, "Hold For Pickup", req.RequestedActionType.Text) + + ad.acceptItemEnable = false + mock.(*ncipClientMock).lastRequest = nil + err = ad.AcceptItem("", "", "", "", "", "", "", "", "") + assert.NoError(t, err) + assert.Nil(t, mock.(*ncipClientMock).lastRequest) } func TestDeleteItem(t *testing.T) { @@ -131,13 +165,12 @@ func TestDeleteItem(t *testing.T) { ad := &LmsAdapterNcip{ ncipClient: mock, } - res, err := ad.DeleteItem("item1") + err := ad.DeleteItem("item1") assert.NoError(t, err) - assert.Equal(t, "item1", res) req := mock.(*ncipClientMock).lastRequest.(ncip.DeleteItem) assert.Equal(t, "item1", req.ItemId.ItemIdentifierValue) - _, err = ad.DeleteItem("error") + err = ad.DeleteItem("error") assert.Error(t, err) assert.Equal(t, "deletion error", err.Error()) } @@ -173,7 +206,8 @@ func TestCancelRequestItem(t *testing.T) { func TestCheckInItem(t *testing.T) { var mock ncipclient.NcipClient = new(ncipClientMock) ad := &LmsAdapterNcip{ - ncipClient: mock, + ncipClient: mock, + checkInItemEnable: true, } err := ad.CheckInItem("item1") assert.NoError(t, err) @@ -181,12 +215,19 @@ func TestCheckInItem(t *testing.T) { assert.Equal(t, "item1", req.ItemId.ItemIdentifierValue) assert.Equal(t, 1, len(req.ItemElementType)) assert.Equal(t, "Bibliographic Description", req.ItemElementType[0].Text) + + ad.checkInItemEnable = false + mock.(*ncipClientMock).lastRequest = nil + err = ad.CheckInItem("item1") + assert.NoError(t, err) + assert.Nil(t, mock.(*ncipClientMock).lastRequest) } func TestCheckOutItem(t *testing.T) { var mock ncipclient.NcipClient = new(ncipClientMock) ad := &LmsAdapterNcip{ - ncipClient: mock, + ncipClient: mock, + checkOutItemEnable: true, } err := ad.CheckOutItem("req1", "item1", "barcodeid", "extref") assert.NoError(t, err) @@ -197,6 +238,12 @@ func TestCheckOutItem(t *testing.T) { bytes, err := xml.Marshal(ncip.RequestId{RequestIdentifierValue: "extref"}) assert.NoError(t, err) assert.Equal(t, bytes, req.Ext.XMLContent) + + ad.checkOutItemEnable = false + mock.(*ncipClientMock).lastRequest = nil + err = ad.CheckOutItem("req1", "item1", "barcodeid", "extref") + assert.NoError(t, err) + assert.Nil(t, mock.(*ncipClientMock).lastRequest) } func TestCreateUserFiscalTransaction(t *testing.T) { @@ -233,6 +280,15 @@ func (n *ncipClientMock) LookupUser(lookup ncip.LookupUser) (*ncip.LookupUserRes if lookup.AuthenticationInput[0].AuthenticationInputData == "missing data" { return &ncip.LookupUserResponse{}, nil } + if lookup.AuthenticationInput[0].AuthenticationInputData == "good user" { + return &ncip.LookupUserResponse{ + UserOptionalFields: &ncip.UserOptionalFields{ + UserId: []ncip.UserId{ + {UserIdentifierValue: "user124"}, + }, + }, + }, nil + } return &ncip.LookupUserResponse{ UserId: &ncip.UserId{UserIdentifierValue: "user123"}, }, nil diff --git a/broker/lms/lms_creator_test.go b/broker/lms/lms_creator_test.go index 1e21302b..5b8ac046 100644 --- a/broker/lms/lms_creator_test.go +++ b/broker/lms/lms_creator_test.go @@ -44,7 +44,7 @@ func TestGetAdapterNoPeers(t *testing.T) { assert.IsType(t, &LmsAdapterMock{}, LmsAdapter) } -func TestGetAdapterNcipMissing(t *testing.T) { +func TestGetAdapterNcipOK(t *testing.T) { illRepo := &MockIllRepo{} peer := ill_db.Peer{ CustomData: map[string]any{ @@ -65,6 +65,22 @@ func TestGetAdapterNcipMissing(t *testing.T) { assert.IsType(t, &LmsAdapterNcip{}, LmsAdapter) } +func TestGetAdapterNcipFail(t *testing.T) { + illRepo := &MockIllRepo{} + peer := ill_db.Peer{ + CustomData: map[string]any{ + "ncip": map[string]any{}, + }, + } + illRepo.On("GetCachedPeersBySymbols", mock.Anything).Return([]ill_db.Peer{peer}, "", nil) + creator := NewLmsCreator(illRepo, nil) + ctx := common.CreateExtCtxWithArgs(context.Background(), nil) + symbol := pgtype.Text{String: "TEST", Valid: true} + _, err := creator.GetAdapter(ctx, symbol) + assert.Error(t, err) + assert.Equal(t, "missing required NCIP configuration field: address", err.Error()) +} + type MockIllRepo struct { mock.Mock ill_db.PgIllRepo diff --git a/broker/ncipclient/ncipclient_impl.go b/broker/ncipclient/ncipclient_impl.go index 3ec4e7ba..a24ec915 100644 --- a/broker/ncipclient/ncipclient_impl.go +++ b/broker/ncipclient/ncipclient_impl.go @@ -171,19 +171,11 @@ func (n *NcipClientImpl) prepareHeader(header *ncip.InitiationHeader) *ncip.Init if header == nil { header = &ncip.InitiationHeader{} } - from_agency := n.fromAgency - if from_agency == "" { - from_agency = "default-from-agency" - } header.FromAgencyId.AgencyId = ncip.SchemeValuePair{ - Text: from_agency, - } - to_agency := n.toAgency - if to_agency == "" { - to_agency = "default-to-agency" + Text: n.fromAgency, } header.ToAgencyId.AgencyId = ncip.SchemeValuePair{ - Text: to_agency, + Text: n.toAgency, } header.FromAgencyAuthentication = n.fromAgencyAuthentication return header diff --git a/broker/patron_request/service/action.go b/broker/patron_request/service/action.go index 84a322fa..ff4089f4 100644 --- a/broker/patron_request/service/action.go +++ b/broker/patron_request/service/action.go @@ -253,7 +253,7 @@ func (a *PatronRequestActionService) shipReturnBorrowingRequest(ctx common.Exten return events.LogErrorAndReturnResult(ctx, "failed to create LMS adapter", err) } item := "" // TODO Get item identifier from the request - _, err = lmsAdapter.DeleteItem(item) + err = lmsAdapter.DeleteItem(item) if err != nil { return events.LogErrorAndReturnResult(ctx, "LMS DeleteItem failed", err) } diff --git a/broker/patron_request/service/action_test.go b/broker/patron_request/service/action_test.go index 8d2c7d99..ee64b3ff 100644 --- a/broker/patron_request/service/action_test.go +++ b/broker/patron_request/service/action_test.go @@ -533,8 +533,8 @@ func (l *MockLmsAdapterFail) AcceptItem( return errors.New("AcceptItem failed") } -func (l *MockLmsAdapterFail) DeleteItem(itemId string) (string, error) { - return "", errors.New("DeleteItem failed") +func (l *MockLmsAdapterFail) DeleteItem(itemId string) error { + return errors.New("DeleteItem failed") } func (l *MockLmsAdapterFail) RequestItem( From ee5f83b800ca31147ef5cfb44cda42e2e5d51751 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Mon, 15 Dec 2025 15:07:52 +0100 Subject: [PATCH 07/12] LmsAdapterManual --- ..._adapter_mock.go => lms_adapter_manual.go} | 20 +++++++++---------- broker/lms/lms_creator_test.go | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) rename broker/lms/{lms_adapter_mock.go => lms_adapter_manual.go} (51%) diff --git a/broker/lms/lms_adapter_mock.go b/broker/lms/lms_adapter_manual.go similarity index 51% rename from broker/lms/lms_adapter_mock.go rename to broker/lms/lms_adapter_manual.go index 25ffecb3..1dd16d6a 100644 --- a/broker/lms/lms_adapter_mock.go +++ b/broker/lms/lms_adapter_manual.go @@ -1,13 +1,13 @@ package lms -type LmsAdapterMock struct { +type LmsAdapterManual struct { } -func (l *LmsAdapterMock) LookupUser(patron string) (string, error) { +func (l *LmsAdapterManual) LookupUser(patron string) (string, error) { return patron, nil } -func (l *LmsAdapterMock) AcceptItem( +func (l *LmsAdapterManual) AcceptItem( itemId string, requestId string, userId string, @@ -21,11 +21,11 @@ func (l *LmsAdapterMock) AcceptItem( return nil } -func (l *LmsAdapterMock) DeleteItem(itemId string) error { +func (l *LmsAdapterManual) DeleteItem(itemId string) error { return nil } -func (l *LmsAdapterMock) RequestItem( +func (l *LmsAdapterManual) RequestItem( requestId string, itemId string, borrowerBarcode string, @@ -35,15 +35,15 @@ func (l *LmsAdapterMock) RequestItem( return nil } -func (l *LmsAdapterMock) CancelRequestItem(requestId string, userId string) error { +func (l *LmsAdapterManual) CancelRequestItem(requestId string, userId string) error { return nil } -func (l *LmsAdapterMock) CheckInItem(itemId string) error { +func (l *LmsAdapterManual) CheckInItem(itemId string) error { return nil } -func (l *LmsAdapterMock) CheckOutItem( +func (l *LmsAdapterManual) CheckOutItem( requestId string, itemBarcode string, borrowerBarcode string, @@ -52,10 +52,10 @@ func (l *LmsAdapterMock) CheckOutItem( return nil } -func (l *LmsAdapterMock) CreateUserFiscalTransaction(userId string, itemId string) error { +func (l *LmsAdapterManual) CreateUserFiscalTransaction(userId string, itemId string) error { return nil } func CreateLmsAdapterMockOK() LmsAdapter { - return &LmsAdapterMock{} + return &LmsAdapterManual{} } diff --git a/broker/lms/lms_creator_test.go b/broker/lms/lms_creator_test.go index 5b8ac046..5dfaaca3 100644 --- a/broker/lms/lms_creator_test.go +++ b/broker/lms/lms_creator_test.go @@ -41,7 +41,7 @@ func TestGetAdapterNoPeers(t *testing.T) { symbol := pgtype.Text{String: "TEST", Valid: true} LmsAdapter, err := creator.GetAdapter(ctx, symbol) assert.NoError(t, err) - assert.IsType(t, &LmsAdapterMock{}, LmsAdapter) + assert.IsType(t, &LmsAdapterManual{}, LmsAdapter) } func TestGetAdapterNcipOK(t *testing.T) { From f0ea2a6dda4af8be1d9c92c2c1dee4ec17f12a17 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Mon, 15 Dec 2025 16:09:40 +0100 Subject: [PATCH 08/12] RequestItem tune config --- broker/lms/lms_adapter_ncip.go | 83 +++++++++++++++++----------------- broker/lms/lms_adapter_test.go | 18 ++++++-- 2 files changed, 57 insertions(+), 44 deletions(-) diff --git a/broker/lms/lms_adapter_ncip.go b/broker/lms/lms_adapter_ncip.go index eda3a4e6..3fefbdb8 100644 --- a/broker/lms/lms_adapter_ncip.go +++ b/broker/lms/lms_adapter_ncip.go @@ -12,14 +12,18 @@ import ( type NcipProperty string const ( - FromAgency NcipProperty = "from_agency" - FromAgencyAuthentication NcipProperty = "from_agency_authentication" - ToAgency NcipProperty = "to_agency" - Address NcipProperty = "address" - LookupUserEnable NcipProperty = "lookup_user_enable" - AcceptItemEnable NcipProperty = "accept_item_enable" - CheckInItemEnable NcipProperty = "check_in_item_enable" - CheckOutItemEnable NcipProperty = "check_out_item_enable" + FromAgency NcipProperty = "from_agency" + FromAgencyAuthentication NcipProperty = "from_agency_authentication" + ToAgency NcipProperty = "to_agency" + Address NcipProperty = "address" + LookupUserEnable NcipProperty = "lookup_user_enable" + AcceptItemEnable NcipProperty = "accept_item_enable" + CheckInItemEnable NcipProperty = "check_in_item_enable" + CheckOutItemEnable NcipProperty = "check_out_item_enable" + RequestItemRequestType NcipProperty = "request_item_request_type" + RequestItemRequestScopeType NcipProperty = "request_item_request_scope_type" + RequestItemBibIdCode NcipProperty = "request_item_bib_id_code" + RequestItemPickupLocationEnable NcipProperty = "request_item_pickup_location_enable" ) type NcipUserElement string @@ -39,18 +43,22 @@ const ( // https://github.com/openlibraryenvironment/lib-ncip-client/tree/master/lib-ncip-client/src/main/java/org/olf/rs/circ/client type LmsAdapterNcip struct { - ncipClient ncipclient.NcipClient - address string - toAgency string - fromAgency string - fromAgencyAuthentication string - lookupUserEnable bool - acceptItemEnable bool - checkInItemEnable bool - checkOutItemEnable bool + ncipClient ncipclient.NcipClient + address string + toAgency string + fromAgency string + fromAgencyAuthentication string + lookupUserEnable bool + acceptItemEnable bool + checkInItemEnable bool + checkOutItemEnable bool + requestItemRequestType string + requestItemRequestScopeType string + requestItemBibIdCode string + requestItemPickupLocationEnable bool } -func setField(m map[string]any, key NcipProperty, dst *string) error { +func reqField(m map[string]any, key NcipProperty, dst *string) error { v, ok := m[string(key)].(string) if !ok || v == "" { return fmt.Errorf("missing required NCIP configuration field: %s", key) @@ -59,17 +67,8 @@ func setField(m map[string]any, key NcipProperty, dst *string) error { return nil } -func optField(m map[string]any, key NcipProperty, dst *string, def string) { - v, ok := m[string(key)].(string) - if !ok || v == "" { - *dst = def - } else { - *dst = v - } -} - -func optBoolField(m map[string]any, key NcipProperty, dst *bool, def bool) { - v, ok := m[string(key)].(bool) +func optField[T any](m map[string]any, key NcipProperty, dst *T, def T) { + v, ok := m[string(key)].(T) if !ok { *dst = def } else { @@ -78,20 +77,24 @@ func optBoolField(m map[string]any, key NcipProperty, dst *bool, def bool) { } func (l *LmsAdapterNcip) parseConfig(ncipInfo map[string]any) error { - err := setField(ncipInfo, Address, &l.address) + err := reqField(ncipInfo, Address, &l.address) if err != nil { return err } - err = setField(ncipInfo, FromAgency, &l.fromAgency) + err = reqField(ncipInfo, FromAgency, &l.fromAgency) if err != nil { return err } optField(ncipInfo, FromAgencyAuthentication, &l.fromAgencyAuthentication, "") optField(ncipInfo, ToAgency, &l.toAgency, "default-to-agency") - optBoolField(ncipInfo, LookupUserEnable, &l.lookupUserEnable, true) - optBoolField(ncipInfo, AcceptItemEnable, &l.acceptItemEnable, true) - optBoolField(ncipInfo, CheckInItemEnable, &l.checkInItemEnable, true) - optBoolField(ncipInfo, CheckOutItemEnable, &l.checkOutItemEnable, true) + optField(ncipInfo, LookupUserEnable, &l.lookupUserEnable, true) + optField(ncipInfo, AcceptItemEnable, &l.acceptItemEnable, true) + optField(ncipInfo, CheckInItemEnable, &l.checkInItemEnable, true) + optField(ncipInfo, CheckOutItemEnable, &l.checkOutItemEnable, true) + optField(ncipInfo, RequestItemRequestType, &l.requestItemRequestType, "Page") + optField(ncipInfo, RequestItemRequestScopeType, &l.requestItemRequestScopeType, "Item") + optField(ncipInfo, RequestItemBibIdCode, &l.requestItemBibIdCode, "SYSNUMBER") + optField(ncipInfo, RequestItemPickupLocationEnable, &l.requestItemPickupLocationEnable, true) return nil } @@ -214,9 +217,8 @@ func (l *LmsAdapterNcip) RequestItem( pickupLocation string, itemLocation string, ) error { - // mod-rs: see getRequestItemPickupLocation which in some cases overrides pickupLocation var pickupLocationField *ncip.SchemeValuePair - if pickupLocation != "" { + if l.requestItemPickupLocationEnable && pickupLocation != "" { pickupLocationField = &ncip.SchemeValuePair{Text: pickupLocation} } var userIdField *ncip.UserId @@ -226,12 +228,11 @@ func (l *LmsAdapterNcip) RequestItem( bibIdField := ncip.BibliographicId{ BibliographicRecordId: &ncip.BibliographicRecordId{ BibliographicRecordIdentifier: itemId, - BibliographicRecordIdentifierCode: &ncip.SchemeValuePair{Text: "SYSNUMBER"}, + BibliographicRecordIdentifierCode: &ncip.SchemeValuePair{Text: l.requestItemBibIdCode}, }} - requestScopeTypeField := ncip.SchemeValuePair{Text: "Item"} // or "Title" + requestScopeTypeField := ncip.SchemeValuePair{Text: l.requestItemRequestScopeType} - // mod-rs: getRequestItemRequestType() - requestTypeField := ncip.SchemeValuePair{Text: "Page"} // "Loan" in Base + requestTypeField := ncip.SchemeValuePair{Text: l.requestItemRequestType} arg := ncip.RequestItem{ RequestId: &ncip.RequestId{RequestIdentifierValue: requestId}, BibliographicId: []ncip.BibliographicId{bibIdField}, diff --git a/broker/lms/lms_adapter_test.go b/broker/lms/lms_adapter_test.go index fe25a927..1b293a7a 100644 --- a/broker/lms/lms_adapter_test.go +++ b/broker/lms/lms_adapter_test.go @@ -178,17 +178,29 @@ func TestDeleteItem(t *testing.T) { func TestRequestItem(t *testing.T) { var mock ncipclient.NcipClient = new(ncipClientMock) ad := &LmsAdapterNcip{ - ncipClient: mock, + ncipClient: mock, + requestItemPickupLocationEnable: true, + requestItemRequestType: "Loan", + requestItemRequestScopeType: "Title", + requestItemBibIdCode: "SYSNUMBER", } err := ad.RequestItem("req1", "item1", "testuser", "loc", "itemloc") assert.NoError(t, err) req := mock.(*ncipClientMock).lastRequest.(ncip.RequestItem) assert.Equal(t, "testuser", req.UserId.UserIdentifierValue) assert.Equal(t, "item1", req.BibliographicId[0].BibliographicRecordId.BibliographicRecordIdentifier) + assert.Equal(t, "SYSNUMBER", req.BibliographicId[0].BibliographicRecordId.BibliographicRecordIdentifierCode.Text) assert.Equal(t, "req1", req.RequestId.RequestIdentifierValue) assert.Equal(t, "loc", req.PickupLocation.Text) - assert.Equal(t, "Page", req.RequestType.Text) - assert.Equal(t, "Item", req.RequestScopeType.Text) + assert.Equal(t, "Loan", req.RequestType.Text) + assert.Equal(t, "Title", req.RequestScopeType.Text) + + ad.requestItemPickupLocationEnable = false + mock.(*ncipClientMock).lastRequest = nil + err = ad.RequestItem("req1", "item1", "testuser", "loc", "itemloc") + assert.NoError(t, err) + req = mock.(*ncipClientMock).lastRequest.(ncip.RequestItem) + assert.Nil(t, req.PickupLocation) } func TestCancelRequestItem(t *testing.T) { From 4fc5dd61bf36ad25aa404691f606b5d52e1596e9 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Mon, 15 Dec 2025 17:56:25 +0100 Subject: [PATCH 09/12] Naive attempt in mapping arguments for AcceptItem --- broker/patron_request/service/action.go | 47 +++++++++++++++++++- broker/patron_request/service/action_test.go | 21 +++++++-- 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/broker/patron_request/service/action.go b/broker/patron_request/service/action.go index ff4089f4..a53139e2 100644 --- a/broker/patron_request/service/action.go +++ b/broker/patron_request/service/action.go @@ -1,6 +1,7 @@ package prservice import ( + "encoding/json" "encoding/xml" "errors" "net/http" @@ -201,8 +202,52 @@ func (a *PatronRequestActionService) receiveBorrowingRequest(ctx common.Extended if err != nil { return events.LogErrorAndReturnResult(ctx, "failed to create LMS adapter", err) } + + var illRequest iso18626.Request + err = json.Unmarshal(pr.IllRequest, &illRequest) + if err != nil { + return events.LogErrorAndReturnResult(ctx, "failed to unmarshal ILL request", err) + } + itemId := illRequest.BibliographicInfo.SupplierUniqueRecordId + requestId := illRequest.Header.RequestingAgencyRequestId + author := illRequest.BibliographicInfo.Author + title := illRequest.BibliographicInfo.Title + isbn := "" + if len(illRequest.BibliographicInfo.BibliographicItemId) > 0 && + illRequest.BibliographicInfo.BibliographicItemId[0].BibliographicItemIdentifierCode.Text == "ISBN" { + isbn = illRequest.BibliographicInfo.BibliographicItemId[0].BibliographicItemIdentifier + } + callNumber := "" + if len(illRequest.SupplierInfo) > 0 { + callNumber = illRequest.SupplierInfo[0].CallNumber + } + pickupLocation := "" + if len(illRequest.RequestedDeliveryInfo) > 0 { + address := illRequest.RequestedDeliveryInfo[0].Address + if address != nil { + pa := address.PhysicalAddress + if pa != nil { + pickupLocation = pa.Line1 + if pa.Line2 != "" { + pickupLocation += " " + pa.Line2 + } + if pa.Locality != "" { + pickupLocation += " " + pa.Locality + } + if pa.PostalCode != "" { + pickupLocation += " " + pa.PostalCode + } + if pa.Region != nil { + pickupLocation += " " + pa.Region.Text + } + if pa.Country != nil { + pickupLocation += " " + pa.Country.Text + } + } + } + } // TODO: get all these parameters from the patron request - err = lmsAdapter.AcceptItem("itemId", "requestId", user, "author", "title", "isbn", "callNumber", "pickupLocation", "requestedAction") + err = lmsAdapter.AcceptItem(itemId, requestId, user, author, title, isbn, callNumber, pickupLocation, "requestedAction") if err != nil { return events.LogErrorAndReturnResult(ctx, "LMS AcceptItem failed", err) } diff --git a/broker/patron_request/service/action_test.go b/broker/patron_request/service/action_test.go index ee64b3ff..f5935fba 100644 --- a/broker/patron_request/service/action_test.go +++ b/broker/patron_request/service/action_test.go @@ -137,8 +137,8 @@ func TestHandleInvokeActionReceiveOK(t *testing.T) { lmsCreator := new(MockLmsCreator) lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), nil) prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) - mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateShipped, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}, nil) - + illRequest := []byte("{\"request\": {}}") + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{IllRequest: illRequest, State: BorrowerStateShipped, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}, nil) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionReceive}}}) assert.Equal(t, events.EventStatusSuccess, status) @@ -146,6 +146,20 @@ func TestHandleInvokeActionReceiveOK(t *testing.T) { assert.Equal(t, BorrowerStateReceived, mockPrRepo.savedPr.State) } +func TestHandleInvokeActionReceiveBadIllRequeset(t *testing.T) { + mockPrRepo := new(MockPrRepo) + mockIso18626Handler := new(MockIso18626Handler) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), nil) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) + illRequest := []byte("{\"bad\": {}") + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{IllRequest: illRequest, State: BorrowerStateShipped, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}, nil) + status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionReceive}}}) + + assert.Equal(t, events.EventStatusError, status) + assert.Equal(t, "failed to unmarshal ILL request", resultData.EventError.Message) +} + func TestHandleInvokeActionReceiveFailCreator(t *testing.T) { mockPrRepo := new(MockPrRepo) mockIso18626Handler := new(MockIso18626Handler) @@ -166,7 +180,8 @@ func TestHandleInvokeActionReceiveAcceptItemFailed(t *testing.T) { lmsCreator := new(MockLmsCreator) lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(createLmsAdapterMockFail(), nil) prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) - mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateShipped, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}, nil) + illRequest := []byte("{\"request\": {}}") + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{IllRequest: illRequest, State: BorrowerStateShipped, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}, nil) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionReceive}}}) From 807a1bca524381a4104d1b9b674ded6ec96f6650 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Tue, 16 Dec 2025 13:16:07 +0100 Subject: [PATCH 10/12] All arguments passed to LMS adapter from action --- broker/lms/lms_adapter.go | 2 +- broker/lms/lms_adapter_manual.go | 2 +- broker/lms/lms_adapter_ncip.go | 4 +- broker/patron_request/service/action.go | 113 ++++++++++++------- broker/patron_request/service/action_test.go | 61 ++++++++-- 5 files changed, 127 insertions(+), 55 deletions(-) diff --git a/broker/lms/lms_adapter.go b/broker/lms/lms_adapter.go index b62431ff..5d019988 100644 --- a/broker/lms/lms_adapter.go +++ b/broker/lms/lms_adapter.go @@ -33,7 +33,7 @@ type LmsAdapter interface { CheckOutItem( requestId string, - itemBarcode string, + itemId string, borrowerBarcode string, externalReferenceValue string, ) error diff --git a/broker/lms/lms_adapter_manual.go b/broker/lms/lms_adapter_manual.go index 1dd16d6a..b81fc773 100644 --- a/broker/lms/lms_adapter_manual.go +++ b/broker/lms/lms_adapter_manual.go @@ -45,7 +45,7 @@ func (l *LmsAdapterManual) CheckInItem(itemId string) error { func (l *LmsAdapterManual) CheckOutItem( requestId string, - itemBarcode string, + itemId string, borrowerBarcode string, externalReferenceValue string, ) error { diff --git a/broker/lms/lms_adapter_ncip.go b/broker/lms/lms_adapter_ncip.go index 3fefbdb8..d53e312d 100644 --- a/broker/lms/lms_adapter_ncip.go +++ b/broker/lms/lms_adapter_ncip.go @@ -272,7 +272,7 @@ func (l *LmsAdapterNcip) CheckInItem(itemId string) error { func (l *LmsAdapterNcip) CheckOutItem( requestId string, - itemBarcode string, + itemId string, borrowerBarcode string, externalReferenceValue string, ) error { @@ -291,7 +291,7 @@ func (l *LmsAdapterNcip) CheckOutItem( arg := ncip.CheckOutItem{ RequestId: &ncip.RequestId{RequestIdentifierValue: requestId}, UserId: &ncip.UserId{UserIdentifierValue: borrowerBarcode}, - ItemId: ncip.ItemId{ItemIdentifierValue: itemBarcode}, + ItemId: ncip.ItemId{ItemIdentifierValue: itemId}, Ext: ext, } _, err := l.ncipClient.CheckOutItem(arg) diff --git a/broker/patron_request/service/action.go b/broker/patron_request/service/action.go index a53139e2..f633fedd 100644 --- a/broker/patron_request/service/action.go +++ b/broker/patron_request/service/action.go @@ -138,15 +138,15 @@ func (a *PatronRequestActionService) updateStateAndReturnResult(ctx common.Exten } func (a *PatronRequestActionService) validateBorrowingRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest) (events.EventStatus, *events.EventResult) { - user := "" + patron := "" if pr.Patron.Valid { - user = pr.Patron.String + patron = pr.Patron.String } lmsAdapter, err := a.lmsCreator.GetAdapter(ctx, pr.RequesterSymbol) if err != nil { return events.LogErrorAndReturnResult(ctx, "failed to create LMS adapter", err) } - userId, err := lmsAdapter.LookupUser(user) + userId, err := lmsAdapter.LookupUser(patron) if err != nil { return events.LogErrorAndReturnResult(ctx, "LMS LookupUser failed", err) } @@ -193,34 +193,8 @@ func (a *PatronRequestActionService) sendBorrowingRequest(ctx common.ExtendedCon return a.updateStateAndReturnResult(ctx, pr, BorrowerStateSent, &result) } -func (a *PatronRequestActionService) receiveBorrowingRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest) (events.EventStatus, *events.EventResult) { - user := "" - if pr.Patron.Valid { - user = pr.Patron.String - } - lmsAdapter, err := a.lmsCreator.GetAdapter(ctx, pr.RequesterSymbol) - if err != nil { - return events.LogErrorAndReturnResult(ctx, "failed to create LMS adapter", err) - } - - var illRequest iso18626.Request - err = json.Unmarshal(pr.IllRequest, &illRequest) - if err != nil { - return events.LogErrorAndReturnResult(ctx, "failed to unmarshal ILL request", err) - } - itemId := illRequest.BibliographicInfo.SupplierUniqueRecordId - requestId := illRequest.Header.RequestingAgencyRequestId - author := illRequest.BibliographicInfo.Author - title := illRequest.BibliographicInfo.Title - isbn := "" - if len(illRequest.BibliographicInfo.BibliographicItemId) > 0 && - illRequest.BibliographicInfo.BibliographicItemId[0].BibliographicItemIdentifierCode.Text == "ISBN" { - isbn = illRequest.BibliographicInfo.BibliographicItemId[0].BibliographicItemIdentifier - } - callNumber := "" - if len(illRequest.SupplierInfo) > 0 { - callNumber = illRequest.SupplierInfo[0].CallNumber - } +// TODO : should be resolved pickup location +func pickupLocationFromIllRequest(illRequest iso18626.Request) string { pickupLocation := "" if len(illRequest.RequestedDeliveryInfo) > 0 { address := illRequest.RequestedDeliveryInfo[0].Address @@ -246,8 +220,49 @@ func (a *PatronRequestActionService) receiveBorrowingRequest(ctx common.Extended } } } - // TODO: get all these parameters from the patron request - err = lmsAdapter.AcceptItem(itemId, requestId, user, author, title, isbn, callNumber, pickupLocation, "requestedAction") + return pickupLocation +} + +func callNumberFromIllRequest(illRequest iso18626.Request) string { + callNumber := "" + if len(illRequest.SupplierInfo) > 0 { + callNumber = illRequest.SupplierInfo[0].CallNumber + } + return callNumber +} + +func isbnFromIllRequest(illRequest iso18626.Request) string { + isbn := "" + if len(illRequest.BibliographicInfo.BibliographicItemId) > 0 && + illRequest.BibliographicInfo.BibliographicItemId[0].BibliographicItemIdentifierCode.Text == "ISBN" { + isbn = illRequest.BibliographicInfo.BibliographicItemId[0].BibliographicItemIdentifier + } + return isbn +} + +func (a *PatronRequestActionService) receiveBorrowingRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest) (events.EventStatus, *events.EventResult) { + patron := "" + if pr.Patron.Valid { + patron = pr.Patron.String + } + lmsAdapter, err := a.lmsCreator.GetAdapter(ctx, pr.RequesterSymbol) + if err != nil { + return events.LogErrorAndReturnResult(ctx, "failed to create LMS adapter", err) + } + var illRequest iso18626.Request + err = json.Unmarshal(pr.IllRequest, &illRequest) + if err != nil { + return events.LogErrorAndReturnResult(ctx, "failed to unmarshal ILL request", err) + } + itemId := illRequest.BibliographicInfo.SupplierUniqueRecordId + requestId := illRequest.Header.RequestingAgencyRequestId + author := illRequest.BibliographicInfo.Author + title := illRequest.BibliographicInfo.Title + isbn := isbnFromIllRequest(illRequest) + callNumber := callNumberFromIllRequest(illRequest) + pickupLocation := pickupLocationFromIllRequest(illRequest) + requestedAction := "Hold For Pickup" + err = lmsAdapter.AcceptItem(itemId, requestId, patron, author, title, isbn, callNumber, pickupLocation, requestedAction) if err != nil { return events.LogErrorAndReturnResult(ctx, "LMS AcceptItem failed", err) } @@ -264,15 +279,23 @@ func (a *PatronRequestActionService) receiveBorrowingRequest(ctx common.Extended } func (a *PatronRequestActionService) checkoutBorrowingRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest) (events.EventStatus, *events.EventResult) { - user := "" + patron := "" if pr.Patron.Valid { - user = pr.Patron.String + patron = pr.Patron.String } lmsAdapter, err := a.lmsCreator.GetAdapter(ctx, pr.RequesterSymbol) if err != nil { return events.LogErrorAndReturnResult(ctx, "failed to create LMS adapter", err) } - err = lmsAdapter.CheckOutItem("requestId", "itemBarcode", user, "externalReferenceValue") + var illRequest iso18626.Request + err = json.Unmarshal(pr.IllRequest, &illRequest) + if err != nil { + return events.LogErrorAndReturnResult(ctx, "failed to unmarshal ILL request", err) + } + requestId := illRequest.Header.RequestingAgencyRequestId + itemId := illRequest.BibliographicInfo.SupplierUniqueRecordId + borrowerBarcode := patron + err = lmsAdapter.CheckOutItem(requestId, itemId, borrowerBarcode, "externalReferenceValue") if err != nil { return events.LogErrorAndReturnResult(ctx, "LMS CheckOutItem failed", err) } @@ -284,8 +307,13 @@ func (a *PatronRequestActionService) checkinBorrowingRequest(ctx common.Extended if err != nil { return events.LogErrorAndReturnResult(ctx, "failed to create LMS adapter", err) } - item := "" // TODO Get item identifier from the request - err = lmsAdapter.CheckInItem(item) + var illRequest iso18626.Request + err = json.Unmarshal(pr.IllRequest, &illRequest) + if err != nil { + return events.LogErrorAndReturnResult(ctx, "failed to unmarshal ILL request", err) + } + itemId := illRequest.BibliographicInfo.SupplierUniqueRecordId + err = lmsAdapter.CheckInItem(itemId) if err != nil { return events.LogErrorAndReturnResult(ctx, "LMS CheckInItem failed", err) } @@ -297,8 +325,13 @@ func (a *PatronRequestActionService) shipReturnBorrowingRequest(ctx common.Exten if err != nil { return events.LogErrorAndReturnResult(ctx, "failed to create LMS adapter", err) } - item := "" // TODO Get item identifier from the request - err = lmsAdapter.DeleteItem(item) + var illRequest iso18626.Request + err = json.Unmarshal(pr.IllRequest, &illRequest) + if err != nil { + return events.LogErrorAndReturnResult(ctx, "failed to unmarshal ILL request", err) + } + itemId := illRequest.BibliographicInfo.SupplierUniqueRecordId + err = lmsAdapter.DeleteItem(itemId) if err != nil { return events.LogErrorAndReturnResult(ctx, "LMS DeleteItem failed", err) } diff --git a/broker/patron_request/service/action_test.go b/broker/patron_request/service/action_test.go index f5935fba..4bd801dd 100644 --- a/broker/patron_request/service/action_test.go +++ b/broker/patron_request/service/action_test.go @@ -194,7 +194,8 @@ func TestHandleInvokeActionCheckOutOK(t *testing.T) { lmsCreator := new(MockLmsCreator) lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), nil) prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), new(handler.Iso18626Handler), lmsCreator) - mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{Patron: pgtype.Text{Valid: true, String: "patron1"}, State: BorrowerStateReceived, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}, nil) + illRequest := []byte("{\"request\": {}}") + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{IllRequest: illRequest, Patron: pgtype.Text{Valid: true, String: "patron1"}, State: BorrowerStateReceived, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}, nil) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionCheckOut}}}) @@ -208,7 +209,8 @@ func TestHandleInvokeActionCheckOutFailCreator(t *testing.T) { lmsCreator := new(MockLmsCreator) lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), assert.AnError) prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), new(handler.Iso18626Handler), lmsCreator) - mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateReceived, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}, nil) + illRequest := []byte("{\"request\": {}}") + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{IllRequest: illRequest, State: BorrowerStateReceived, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}, nil) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionCheckOut}}}) assert.Equal(t, events.EventStatusError, status) @@ -220,7 +222,8 @@ func TestHandleInvokeActionCheckOutFails(t *testing.T) { lmsCreator := new(MockLmsCreator) lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(createLmsAdapterMockFail(), nil) prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), new(handler.Iso18626Handler), lmsCreator) - mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateReceived, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}, nil) + illRequest := []byte("{\"request\": {}}") + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{IllRequest: illRequest, State: BorrowerStateReceived, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}, nil) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionCheckOut}}}) assert.Equal(t, events.EventStatusError, status) @@ -232,7 +235,8 @@ func TestHandleInvokeActionCheckInOK(t *testing.T) { lmsCreator := new(MockLmsCreator) lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), nil) prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), new(handler.Iso18626Handler), lmsCreator) - mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateCheckedOut, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}, nil) + illRequest := []byte("{\"request\": {}}") + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{IllRequest: illRequest, State: BorrowerStateCheckedOut, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}, nil) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionCheckIn}}}) @@ -258,7 +262,8 @@ func TestHandleInvokeActionCheckInFails(t *testing.T) { lmsCreator := new(MockLmsCreator) lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(createLmsAdapterMockFail(), nil) prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), new(handler.Iso18626Handler), lmsCreator) - mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateCheckedOut, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}, nil) + illRequest := []byte("{\"request\": {}}") + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{IllRequest: illRequest, State: BorrowerStateCheckedOut, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}, nil) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionCheckIn}}}) assert.Equal(t, events.EventStatusError, status) @@ -271,7 +276,8 @@ func TestHandleInvokeActionShipReturnOK(t *testing.T) { lmsCreator := new(MockLmsCreator) lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), nil) prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) - mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateCheckedIn, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}, nil) + illRequest := []byte("{\"request\": {}}") + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{IllRequest: illRequest, State: BorrowerStateCheckedIn, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}, nil) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionShipReturn}}}) @@ -300,7 +306,8 @@ func TestHandleInvokeActionShipReturnFails(t *testing.T) { lmsCreator := new(MockLmsCreator) lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(createLmsAdapterMockFail(), nil) prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) - mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateCheckedIn, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}, nil) + illRequest := []byte("{\"request\": {}}") + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{IllRequest: illRequest, State: BorrowerStateCheckedIn, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}, nil) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionShipReturn}}}) @@ -376,12 +383,41 @@ func TestShipReturnBorrowingRequestMissingSupplierSymbol(t *testing.T) { lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), nil) prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) - status, resultData := prAction.shipReturnBorrowingRequest(appCtx, pr_db.PatronRequest{Patron: pgtype.Text{Valid: true, String: "patron1"}, ID: "1", State: BorrowerStateValidated, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}) + illRequest := []byte("{\"request\": {}}") + status, resultData := prAction.shipReturnBorrowingRequest(appCtx, pr_db.PatronRequest{IllRequest: illRequest, Patron: pgtype.Text{Valid: true, String: "patron1"}, ID: "1", State: BorrowerStateValidated, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}) assert.Equal(t, events.EventStatusError, status) assert.Equal(t, "missing supplier symbol", resultData.EventError.Message) } +func TestShipReturnBorrowingRequestGetAdapterFailed(t *testing.T) { + mockPrRepo := new(MockPrRepo) + mockIso18626Handler := new(MockIso18626Handler) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), assert.AnError) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) + + illRequest := []byte("{\"request\": {}}") + status, resultData := prAction.shipReturnBorrowingRequest(appCtx, pr_db.PatronRequest{IllRequest: illRequest, Patron: pgtype.Text{Valid: true, String: "patron1"}, ID: "1", State: BorrowerStateValidated, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}) + + assert.Equal(t, events.EventStatusError, status) + assert.Equal(t, "failed to create LMS adapter", resultData.EventError.Message) +} + +func TestShipReturnBorrowingRequestBadIllRequest(t *testing.T) { + mockPrRepo := new(MockPrRepo) + mockIso18626Handler := new(MockIso18626Handler) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), nil) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) + + illRequest := []byte("{bad") + status, resultData := prAction.shipReturnBorrowingRequest(appCtx, pr_db.PatronRequest{IllRequest: illRequest, Patron: pgtype.Text{Valid: true, String: "patron1"}, ID: "1", State: BorrowerStateValidated, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}) + + assert.Equal(t, events.EventStatusError, status) + assert.Equal(t, "failed to unmarshal ILL request", resultData.EventError.Message) +} + func TestShipReturnBorrowingRequestMissingRequesterSymbol(t *testing.T) { mockPrRepo := new(MockPrRepo) mockIso18626Handler := new(MockIso18626Handler) @@ -389,7 +425,8 @@ func TestShipReturnBorrowingRequestMissingRequesterSymbol(t *testing.T) { lmsCreator.On("GetAdapter", pgtype.Text{}).Return(lms.CreateLmsAdapterMockOK(), nil) prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) - status, resultData := prAction.shipReturnBorrowingRequest(appCtx, pr_db.PatronRequest{ID: "1", State: BorrowerStateValidated, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}) + illRequest := []byte("{\"request\": {}}") + status, resultData := prAction.shipReturnBorrowingRequest(appCtx, pr_db.PatronRequest{IllRequest: illRequest, ID: "1", State: BorrowerStateValidated, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}) assert.Equal(t, events.EventStatusError, status) assert.Equal(t, "missing requester symbol", resultData.EventError.Message) @@ -402,7 +439,8 @@ func TestShipReturnBorrowingRequestInvalidSupplierSymbol(t *testing.T) { lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), nil) prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) - status, resultData := prAction.shipReturnBorrowingRequest(appCtx, pr_db.PatronRequest{ID: "1", State: BorrowerStateValidated, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "x"}}) + illRequest := []byte("{\"request\": {}}") + status, resultData := prAction.shipReturnBorrowingRequest(appCtx, pr_db.PatronRequest{IllRequest: illRequest, ID: "1", State: BorrowerStateValidated, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "x"}}) assert.Equal(t, events.EventStatusError, status) assert.Equal(t, "invalid supplier symbol", resultData.EventError.Message) @@ -415,7 +453,8 @@ func TestShipReturnBorrowingRequestInvalidRequesterSymbol(t *testing.T) { lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "x"}).Return(lms.CreateLmsAdapterMockOK(), nil) prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) - status, resultData := prAction.shipReturnBorrowingRequest(appCtx, pr_db.PatronRequest{ID: "1", State: BorrowerStateValidated, RequesterSymbol: pgtype.Text{Valid: true, String: "x"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}) + illRequest := []byte("{\"request\": {}}") + status, resultData := prAction.shipReturnBorrowingRequest(appCtx, pr_db.PatronRequest{IllRequest: illRequest, ID: "1", State: BorrowerStateValidated, RequesterSymbol: pgtype.Text{Valid: true, String: "x"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}) assert.Equal(t, events.EventStatusError, status) assert.Equal(t, "invalid requester symbol", resultData.EventError.Message) From 4d4bfef772a4f97500c815724955bc78ddd50889 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Tue, 16 Dec 2025 13:55:13 +0100 Subject: [PATCH 11/12] Further testing; etc --- broker/lms/lms_adapter_ncip.go | 4 +- broker/patron_request/service/action_test.go | 83 ++++++++++++++++++-- 2 files changed, 79 insertions(+), 8 deletions(-) diff --git a/broker/lms/lms_adapter_ncip.go b/broker/lms/lms_adapter_ncip.go index d53e312d..1cb0990c 100644 --- a/broker/lms/lms_adapter_ncip.go +++ b/broker/lms/lms_adapter_ncip.go @@ -123,9 +123,9 @@ func (l *LmsAdapterNcip) LookupUser(patron string) (string, error) { if err == nil { return patron, nil } - // then try by user user name + // then try by user username // a better solution would be that the LookupUser had type argument (eg barcode or PIN) - // but this is now mod-rs does it + // but this is how mod-rs does it var authenticationInput []ncip.AuthenticationInput authenticationInput = append(authenticationInput, ncip.AuthenticationInput{ AuthenticationInputType: ncip.SchemeValuePair{Text: "username"}, diff --git a/broker/patron_request/service/action_test.go b/broker/patron_request/service/action_test.go index 4bd801dd..7a314fd6 100644 --- a/broker/patron_request/service/action_test.go +++ b/broker/patron_request/service/action_test.go @@ -146,7 +146,7 @@ func TestHandleInvokeActionReceiveOK(t *testing.T) { assert.Equal(t, BorrowerStateReceived, mockPrRepo.savedPr.State) } -func TestHandleInvokeActionReceiveBadIllRequeset(t *testing.T) { +func TestHandleInvokeActionReceiveBadIllRequest(t *testing.T) { mockPrRepo := new(MockPrRepo) mockIso18626Handler := new(MockIso18626Handler) lmsCreator := new(MockLmsCreator) @@ -160,13 +160,14 @@ func TestHandleInvokeActionReceiveBadIllRequeset(t *testing.T) { assert.Equal(t, "failed to unmarshal ILL request", resultData.EventError.Message) } -func TestHandleInvokeActionReceiveFailCreator(t *testing.T) { +func TestHandleInvokeActionReceiveGetAdapterFail(t *testing.T) { mockPrRepo := new(MockPrRepo) mockIso18626Handler := new(MockIso18626Handler) lmsCreator := new(MockLmsCreator) lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), assert.AnError) prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) - mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateShipped, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}, nil) + illRequest := []byte("{\"request\": {}}") + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{IllRequest: illRequest, State: BorrowerStateShipped, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}, SupplierSymbol: pgtype.Text{Valid: true, String: "ISIL:SUP1"}}, nil) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionReceive}}}) @@ -204,7 +205,7 @@ func TestHandleInvokeActionCheckOutOK(t *testing.T) { assert.Equal(t, BorrowerStateCheckedOut, mockPrRepo.savedPr.State) } -func TestHandleInvokeActionCheckOutFailCreator(t *testing.T) { +func TestHandleInvokeActionCheckOutGetAdapterFail(t *testing.T) { mockPrRepo := new(MockPrRepo) lmsCreator := new(MockLmsCreator) lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), assert.AnError) @@ -217,6 +218,19 @@ func TestHandleInvokeActionCheckOutFailCreator(t *testing.T) { assert.Equal(t, "failed to create LMS adapter", resultData.EventError.Message) } +func TestHandleInvokeActionCheckOutBadIllRequest(t *testing.T) { + mockPrRepo := new(MockPrRepo) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), nil) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), new(handler.Iso18626Handler), lmsCreator) + illRequest := []byte("{\"bad\": {}") + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{IllRequest: illRequest, State: BorrowerStateReceived, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}, nil) + + status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionCheckOut}}}) + assert.Equal(t, events.EventStatusError, status) + assert.Equal(t, "failed to unmarshal ILL request", resultData.EventError.Message) +} + func TestHandleInvokeActionCheckOutFails(t *testing.T) { mockPrRepo := new(MockPrRepo) lmsCreator := new(MockLmsCreator) @@ -245,18 +259,32 @@ func TestHandleInvokeActionCheckInOK(t *testing.T) { assert.Equal(t, BorrowerStateCheckedIn, mockPrRepo.savedPr.State) } -func TestHandleInvokeActionCheckInFailCreator(t *testing.T) { +func TestHandleInvokeActionCheckInGetAdapterFail(t *testing.T) { mockPrRepo := new(MockPrRepo) lmsCreator := new(MockLmsCreator) lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), assert.AnError) prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), new(handler.Iso18626Handler), lmsCreator) - mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{State: BorrowerStateCheckedOut, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}, nil) + illRequest := []byte("{\"request\": {}}") + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{IllRequest: illRequest, State: BorrowerStateCheckedOut, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}, nil) status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionCheckIn}}}) assert.Equal(t, events.EventStatusError, status) assert.Equal(t, "failed to create LMS adapter", resultData.EventError.Message) } +func TestHandleInvokeActionCheckInBadIllRequest(t *testing.T) { + mockPrRepo := new(MockPrRepo) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", pgtype.Text{Valid: true, String: "ISIL:REC1"}).Return(lms.CreateLmsAdapterMockOK(), nil) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), new(handler.Iso18626Handler), lmsCreator) + illRequest := []byte("{\"bad\": {}") + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{IllRequest: illRequest, State: BorrowerStateCheckedOut, RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:REC1"}}, nil) + + status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &ActionCheckIn}}}) + assert.Equal(t, events.EventStatusError, status) + assert.Equal(t, "failed to unmarshal ILL request", resultData.EventError.Message) +} + func TestHandleInvokeActionCheckInFails(t *testing.T) { mockPrRepo := new(MockPrRepo) lmsCreator := new(MockLmsCreator) @@ -621,3 +649,46 @@ func (l *MockLmsAdapterFail) CheckOutItem( func (l *MockLmsAdapterFail) CreateUserFiscalTransaction(userId string, itemId string) error { return errors.New("CreateUserFiscalTransaction failed") } + +func TestPickupLocationFromIllRequest(t *testing.T) { + var r iso18626.Request + r.RequestedDeliveryInfo = []iso18626.RequestedDeliveryInfo{{ + Address: &iso18626.Address{ + PhysicalAddress: &iso18626.PhysicalAddress{ + Line1: "Main Library", + Line2: "123 Library St", + Locality: "Booktown", + PostalCode: "12345", + Region: &iso18626.TypeSchemeValuePair{Text: "State"}, + Country: &iso18626.TypeSchemeValuePair{Text: "Country"}, + }, + }, + }} + location := pickupLocationFromIllRequest(r) + assert.Equal(t, "Main Library 123 Library St Booktown 12345 State Country", location) +} + +func TestIsbnFromIllRequest(t *testing.T) { + var r iso18626.Request + r.BibliographicInfo = iso18626.BibliographicInfo{ + BibliographicItemId: []iso18626.BibliographicItemId{ + { + BibliographicItemIdentifier: "978-3-16-148410-0", + BibliographicItemIdentifierCode: iso18626.TypeSchemeValuePair{ + Text: "ISBN", + }, + }, + }, + } + isbn := isbnFromIllRequest(r) + assert.Equal(t, "978-3-16-148410-0", isbn) +} + +func TestCallNumberFromIllRequest(t *testing.T) { + var r iso18626.Request + r.SupplierInfo = []iso18626.SupplierInfo{{ + CallNumber: "QA76.73.G63 D37 2020", + }} + callNumber := callNumberFromIllRequest(r) + assert.Equal(t, "QA76.73.G63 D37 2020", callNumber) +} From 80cdad5724aae36b92031dcd107218e841c8a45f Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Mon, 29 Dec 2025 09:40:28 +0100 Subject: [PATCH 12/12] rename tenant utils --- broker/api/api-handler.go | 34 +++++++++---------- broker/app/app.go | 6 ++-- broker/common/tenant.go | 21 ++++++++++++ broker/common/tenant_to_symbol.go | 21 ------------ broker/patron_request/api/api-handler.go | 14 ++++---- broker/patron_request/api/api-handler_test.go | 16 ++++----- broker/test/api/api-handler_test.go | 2 +- 7 files changed, 57 insertions(+), 57 deletions(-) create mode 100644 broker/common/tenant.go delete mode 100644 broker/common/tenant_to_symbol.go diff --git a/broker/api/api-handler.go b/broker/api/api-handler.go index 529629a4..c86389a7 100644 --- a/broker/api/api-handler.go +++ b/broker/api/api-handler.go @@ -38,18 +38,18 @@ var LIMIT_DEFAULT int32 = 10 var ARCHIVE_PROCESS_STARTED = "Archive process started" type ApiHandler struct { - limitDefault int32 - eventRepo events.EventRepo - illRepo ill_db.IllRepo - tenantToSymbol common.TenantToSymbol + limitDefault int32 + eventRepo events.EventRepo + illRepo ill_db.IllRepo + tenant common.Tenant } -func NewApiHandler(eventRepo events.EventRepo, illRepo ill_db.IllRepo, tenentToSymbol common.TenantToSymbol, limitDefault int32) ApiHandler { +func NewApiHandler(eventRepo events.EventRepo, illRepo ill_db.IllRepo, tenant common.Tenant, limitDefault int32) ApiHandler { return ApiHandler{ - eventRepo: eventRepo, - illRepo: illRepo, - tenantToSymbol: tenentToSymbol, - limitDefault: limitDefault, + eventRepo: eventRepo, + illRepo: illRepo, + tenant: tenant, + limitDefault: limitDefault, } } @@ -57,13 +57,13 @@ func (a *ApiHandler) isOwner(trans *ill_db.IllTransaction, tenant *string, reque if tenant == nil && requesterSymbol != nil { return trans.RequesterSymbol.String == *requesterSymbol } - if !a.tenantToSymbol.TenantMode() { + if !a.tenant.IsSpecified() { return true } if tenant == nil { return false } - return trans.RequesterSymbol.String == a.tenantToSymbol.GetSymbolFromTenant(*tenant) + return trans.RequesterSymbol.String == a.tenant.GetSymbol(*tenant) } func (a *ApiHandler) getIllTranFromParams(ctx common.ExtendedContext, w http.ResponseWriter, @@ -168,14 +168,14 @@ func (a *ApiHandler) GetIllTransactions(w http.ResponseWriter, r *http.Request, fullCount = 1 resp.Items = append(resp.Items, toApiIllTransaction(r, *tran)) } - } else if a.tenantToSymbol.TenantMode() { - var tenantSymbol string + } else if a.tenant.IsSpecified() { + var symbol string if params.XOkapiTenant != nil { - tenantSymbol = a.tenantToSymbol.GetSymbolFromTenant(*params.XOkapiTenant) + symbol = a.tenant.GetSymbol(*params.XOkapiTenant) } else if params.RequesterSymbol != nil { - tenantSymbol = *params.RequesterSymbol + symbol = *params.RequesterSymbol } - if tenantSymbol == "" { + if symbol == "" { writeJsonResponse(w, resp) return } @@ -183,7 +183,7 @@ func (a *ApiHandler) GetIllTransactions(w http.ResponseWriter, r *http.Request, Limit: limit, Offset: offset, RequesterSymbol: pgtype.Text{ - String: tenantSymbol, + String: symbol, Valid: true, }, } diff --git a/broker/app/app.go b/broker/app/app.go index f5b559cc..0510a59f 100644 --- a/broker/app/app.go +++ b/broker/app/app.go @@ -161,7 +161,7 @@ func Init(ctx context.Context) (Context, error) { workflowManager := service.CreateWorkflowManager(eventBus, illRepo, service.WorkflowConfig{}) lmsCreator := lms.NewLmsCreator(illRepo, dirAdapter) prActionService := prservice.CreatePatronRequestActionService(prRepo, eventBus, &iso18626Handler, lmsCreator) - prApiHandler := prapi.NewApiHandler(prRepo, eventBus, common.NewTenantToSymbol(TENANT_TO_SYMBOL)) + prApiHandler := prapi.NewApiHandler(prRepo, eventBus, common.NewTenant(TENANT_TO_SYMBOL)) AddDefaultHandlers(eventBus, iso18626Client, supplierLocator, workflowManager, iso18626Handler, prActionService, prApiHandler, prMessageHandler) err = StartEventBus(ctx, eventBus) @@ -199,10 +199,10 @@ func StartServer(ctx Context) error { http.ServeFile(w, r, "handler/open-api.yaml") }) - apiHandler := api.NewApiHandler(ctx.EventRepo, ctx.IllRepo, common.NewTenantToSymbol(""), API_PAGE_SIZE) + apiHandler := api.NewApiHandler(ctx.EventRepo, ctx.IllRepo, common.NewTenant(""), API_PAGE_SIZE) oapi.HandlerFromMux(&apiHandler, ServeMux) if TENANT_TO_SYMBOL != "" { - apiHandler := api.NewApiHandler(ctx.EventRepo, ctx.IllRepo, common.NewTenantToSymbol(TENANT_TO_SYMBOL), API_PAGE_SIZE) + apiHandler := api.NewApiHandler(ctx.EventRepo, ctx.IllRepo, common.NewTenant(TENANT_TO_SYMBOL), API_PAGE_SIZE) oapi.HandlerFromMuxWithBaseURL(&apiHandler, ServeMux, "/broker") } diff --git a/broker/common/tenant.go b/broker/common/tenant.go new file mode 100644 index 00000000..9e17acc9 --- /dev/null +++ b/broker/common/tenant.go @@ -0,0 +1,21 @@ +package common + +import ( + "strings" +) + +type Tenant struct { + mapping string +} + +func NewTenant(tenantSymbol string) Tenant { + return Tenant{mapping: tenantSymbol} +} + +func (t *Tenant) IsSpecified() bool { + return t.mapping != "" +} + +func (t *Tenant) GetSymbol(tenant string) string { + return strings.ReplaceAll(t.mapping, "{tenant}", strings.ToUpper(tenant)) +} diff --git a/broker/common/tenant_to_symbol.go b/broker/common/tenant_to_symbol.go deleted file mode 100644 index 50c0a6f7..00000000 --- a/broker/common/tenant_to_symbol.go +++ /dev/null @@ -1,21 +0,0 @@ -package common - -import ( - "strings" -) - -type TenantToSymbol struct { - mapping string -} - -func NewTenantToSymbol(tenantSymbol string) TenantToSymbol { - return TenantToSymbol{mapping: tenantSymbol} -} - -func (t *TenantToSymbol) TenantMode() bool { - return t.mapping != "" -} - -func (t *TenantToSymbol) GetSymbolFromTenant(tenant string) string { - return strings.ReplaceAll(t.mapping, "{tenant}", strings.ToUpper(tenant)) -} diff --git a/broker/patron_request/api/api-handler.go b/broker/patron_request/api/api-handler.go index 5b4a2399..39a4049d 100644 --- a/broker/patron_request/api/api-handler.go +++ b/broker/patron_request/api/api-handler.go @@ -23,15 +23,15 @@ type PatronRequestApiHandler struct { prRepo pr_db.PrRepo eventBus events.EventBus actionMappingService prservice.ActionMappingService - tenantToSymbol common.TenantToSymbol + tenant common.Tenant } -func NewApiHandler(prRepo pr_db.PrRepo, eventBus events.EventBus, tenantToSymbol common.TenantToSymbol) PatronRequestApiHandler { +func NewApiHandler(prRepo pr_db.PrRepo, eventBus events.EventBus, tenant common.Tenant) PatronRequestApiHandler { return PatronRequestApiHandler{ prRepo: prRepo, eventBus: eventBus, actionMappingService: prservice.ActionMappingService{}, - tenantToSymbol: tenantToSymbol, + tenant: tenant, } } @@ -62,7 +62,7 @@ func (a *PatronRequestApiHandler) PostPatronRequests(w http.ResponseWriter, r *h addBadRequestError(ctx, w, err) return } - dbreq, err := toDbPatronRequest(a.tenantToSymbol, newPr, params.XOkapiTenant) + dbreq, err := toDbPatronRequest(a.tenant, newPr, params.XOkapiTenant) if err != nil { addBadRequestError(ctx, w, err) return @@ -264,12 +264,12 @@ func toStringFromBytes(bytes []byte) *string { return value } -func toDbPatronRequest(tenantToSymbol common.TenantToSymbol, request proapi.CreatePatronRequest, tenant *string) (pr_db.PatronRequest, error) { +func toDbPatronRequest(tenantToSymbol common.Tenant, request proapi.CreatePatronRequest, tenant *string) (pr_db.PatronRequest, error) { if tenant == nil { return pr_db.PatronRequest{}, errors.New("X-Okapi-Tenant header is required") } - if tenantToSymbol.TenantMode() { - symbol := tenantToSymbol.GetSymbolFromTenant(*tenant) + if tenantToSymbol.IsSpecified() { + symbol := tenantToSymbol.GetSymbol(*tenant) request.RequesterSymbol = &symbol } var illRequest []byte diff --git a/broker/patron_request/api/api-handler_test.go b/broker/patron_request/api/api-handler_test.go index 834ea1ee..7cb37807 100644 --- a/broker/patron_request/api/api-handler_test.go +++ b/broker/patron_request/api/api-handler_test.go @@ -36,7 +36,7 @@ func TestGetDbText(t *testing.T) { } func TestGetPatronRequests(t *testing.T) { - handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenantToSymbol("")) + handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenant("")) req, _ := http.NewRequest("GET", "/", nil) rr := httptest.NewRecorder() handler.GetPatronRequests(rr, req) @@ -47,7 +47,7 @@ func TestGetPatronRequests(t *testing.T) { } func TestPostPatronRequests(t *testing.T) { - handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenantToSymbol("")) + handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenant("")) toCreate := proapi.PatronRequest{ID: "1"} jsonBytes, err := json.Marshal(toCreate) assert.NoError(t, err, "failed to marshal patron request") @@ -61,7 +61,7 @@ func TestPostPatronRequests(t *testing.T) { } func TestPostPatronRequestsMissingTenant(t *testing.T) { - handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenantToSymbol("")) + handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenant("")) toCreate := proapi.PatronRequest{ID: "1"} jsonBytes, err := json.Marshal(toCreate) assert.NoError(t, err, "failed to marshal patron request") @@ -74,7 +74,7 @@ func TestPostPatronRequestsMissingTenant(t *testing.T) { } func TestPostPatronRequestsInvalidJson(t *testing.T) { - handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenantToSymbol("")) + handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenant("")) req, _ := http.NewRequest("POST", "/", bytes.NewBuffer([]byte("a\": v\""))) rr := httptest.NewRecorder() tenant := proapi.Tenant("test-lib") @@ -84,7 +84,7 @@ func TestPostPatronRequestsInvalidJson(t *testing.T) { } func TestDeletePatronRequestsIdNotFound(t *testing.T) { - handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenantToSymbol("")) + handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenant("")) req, _ := http.NewRequest("POST", "/", nil) rr := httptest.NewRecorder() handler.DeletePatronRequestsId(rr, req, "2", proapi.DeletePatronRequestsIdParams{}) @@ -92,7 +92,7 @@ func TestDeletePatronRequestsIdNotFound(t *testing.T) { } func TestDeletePatronRequestsId(t *testing.T) { - handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenantToSymbol("")) + handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenant("")) req, _ := http.NewRequest("POST", "/", nil) rr := httptest.NewRecorder() handler.DeletePatronRequestsId(rr, req, "3", proapi.DeletePatronRequestsIdParams{}) @@ -101,7 +101,7 @@ func TestDeletePatronRequestsId(t *testing.T) { } func TestGetPatronRequestsIdNotFound(t *testing.T) { - handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenantToSymbol("")) + handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenant("")) req, _ := http.NewRequest("POST", "/", nil) rr := httptest.NewRecorder() handler.GetPatronRequestsId(rr, req, "2", proapi.GetPatronRequestsIdParams{}) @@ -109,7 +109,7 @@ func TestGetPatronRequestsIdNotFound(t *testing.T) { } func TestGetPatronRequestsId(t *testing.T) { - handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenantToSymbol("")) + handler := NewApiHandler(new(PrRepoError), mockEventBus, common.NewTenant("")) req, _ := http.NewRequest("POST", "/", nil) rr := httptest.NewRecorder() handler.GetPatronRequestsId(rr, req, "1", proapi.GetPatronRequestsIdParams{}) diff --git a/broker/test/api/api-handler_test.go b/broker/test/api/api-handler_test.go index 1c1f95b6..c042c8f6 100644 --- a/broker/test/api/api-handler_test.go +++ b/broker/test/api/api-handler_test.go @@ -43,7 +43,7 @@ var illRepo ill_db.IllRepo var eventRepo events.EventRepo var mockIllRepoError = new(mocks.MockIllRepositoryError) var mockEventRepoError = new(mocks.MockEventRepositoryError) -var handlerMock = api.NewApiHandler(mockEventRepoError, mockIllRepoError, common.NewTenantToSymbol(""), api.LIMIT_DEFAULT) +var handlerMock = api.NewApiHandler(mockEventRepoError, mockIllRepoError, common.NewTenant(""), api.LIMIT_DEFAULT) func TestMain(m *testing.M) { app.TENANT_TO_SYMBOL = "ISIL:DK-{tenant}"