Skip to content

Commit 3610b00

Browse files
adamdickmeissadamdickmeiss
authored
ILLDEV-451 state model selector (#671)
1 parent 8204eff commit 3610b00

19 files changed

Lines changed: 288 additions & 13 deletions

‎broker/migrations/050_add_patron_request_state_model.down.sql‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
DROP VIEW IF EXISTS patron_request_search_view;
2+
3+
ALTER TABLE patron_request
4+
DROP COLUMN state_model;
5+
6+
CREATE VIEW patron_request_search_view AS
7+
SELECT
8+
pr.*,
9+
EXISTS (
10+
SELECT 1
11+
FROM notification n
12+
WHERE n.pr_id = pr.id
13+
) AS has_notification,
14+
EXISTS (
15+
SELECT 1
16+
FROM notification n
17+
WHERE n.pr_id = pr.id and cost is not null
18+
) AS has_cost,
19+
(unread.unread_notifications_count > 0) AS has_unread_notification,
20+
(pr.internal_note IS NOT NULL AND btrim(pr.internal_note) <> '') AS has_internal_note,
21+
pr.ill_request -> 'serviceInfo' ->> 'serviceType' AS service_type,
22+
pr.ill_request -> 'serviceInfo' -> 'serviceLevel' ->> '#text' AS service_level,
23+
immutable_to_timestamp(pr.ill_request -> 'serviceInfo' ->> 'needBeforeDate') AS needed_at,
24+
unread.unread_notifications_count AS unread_notifications_count,
25+
req_peer.name AS requester_name,
26+
sup_peer.name AS supplier_name
27+
FROM patron_request pr
28+
LEFT JOIN LATERAL (
29+
SELECT COUNT(*) AS unread_notifications_count
30+
FROM notification n
31+
WHERE n.pr_id = pr.id and n.acknowledged_at is null
32+
) unread ON true
33+
LEFT JOIN symbol req_sym ON req_sym.symbol_value = pr.requester_symbol
34+
LEFT JOIN peer req_peer ON req_peer.id = req_sym.peer_id
35+
LEFT JOIN symbol sup_sym ON sup_sym.symbol_value = pr.supplier_symbol
36+
LEFT JOIN peer sup_peer ON sup_peer.id = sup_sym.peer_id;

‎broker/migrations/050_add_patron_request_state_model.up.sql‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
ALTER TABLE patron_request
2+
ADD COLUMN state_model VARCHAR NOT NULL DEFAULT 'returnables';
3+
4+
DROP VIEW IF EXISTS patron_request_search_view;
5+
6+
CREATE VIEW patron_request_search_view AS
7+
SELECT
8+
pr.*,
9+
EXISTS (
10+
SELECT 1
11+
FROM notification n
12+
WHERE n.pr_id = pr.id
13+
) AS has_notification,
14+
EXISTS (
15+
SELECT 1
16+
FROM notification n
17+
WHERE n.pr_id = pr.id and cost is not null
18+
) AS has_cost,
19+
(unread.unread_notifications_count > 0) AS has_unread_notification,
20+
(pr.internal_note IS NOT NULL AND btrim(pr.internal_note) <> '') AS has_internal_note,
21+
pr.ill_request -> 'serviceInfo' ->> 'serviceType' AS service_type,
22+
pr.ill_request -> 'serviceInfo' -> 'serviceLevel' ->> '#text' AS service_level,
23+
immutable_to_timestamp(pr.ill_request -> 'serviceInfo' ->> 'needBeforeDate') AS needed_at,
24+
unread.unread_notifications_count AS unread_notifications_count,
25+
req_peer.name AS requester_name,
26+
sup_peer.name AS supplier_name
27+
FROM patron_request pr
28+
LEFT JOIN LATERAL (
29+
SELECT COUNT(*) AS unread_notifications_count
30+
FROM notification n
31+
WHERE n.pr_id = pr.id and n.acknowledged_at is null
32+
) unread ON true
33+
LEFT JOIN symbol req_sym ON req_sym.symbol_value = pr.requester_symbol
34+
LEFT JOIN peer req_peer ON req_peer.id = req_sym.peer_id
35+
LEFT JOIN symbol sup_sym ON sup_sym.symbol_value = pr.supplier_symbol
36+
LEFT JOIN peer sup_peer ON sup_peer.id = sup_sym.peer_id;

‎broker/oapi/open-api.yaml‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,9 @@ components:
521521
state:
522522
type: string
523523
description: Patron request state
524+
stateModel:
525+
type: string
526+
description: State model configuration key governing this request
524527
side:
525528
type: string
526529
description: Patron request side - borrowing or lending
@@ -602,6 +605,7 @@ components:
602605
- id
603606
- createdAt
604607
- state
608+
- stateModel
605609
- side
606610
- illRequest
607611
- needsAttention
@@ -750,6 +754,8 @@ components:
750754
version:
751755
type: string
752756
description: Version of the state model in SemVer
757+
selector:
758+
$ref: '#/components/schemas/StateModelSelector'
753759
states:
754760
type: array
755761
description: A list of all allowed states
@@ -761,6 +767,26 @@ components:
761767
- type
762768
- version
763769

770+
StateModelSelector:
771+
title: StateModelSelector
772+
type: object
773+
description: Criteria used to select this state model for an ISO 18626 request
774+
additionalProperties: false
775+
properties:
776+
serviceType:
777+
type: array
778+
description: ISO 18626 service types handled by this state model
779+
minItems: 1
780+
uniqueItems: true
781+
items:
782+
type: string
783+
enum:
784+
- Copy
785+
- Loan
786+
- CopyOrLoan
787+
required:
788+
- serviceType
789+
764790
StateModelCapabilities:
765791
title: StateModelCapabilities
766792
type: object
@@ -3007,4 +3033,3 @@ paths:
30073033
application/json:
30083034
schema:
30093035
$ref: '#/components/schemas/Error'
3010-

‎broker/patron_request/api/api-handler.go‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -351,11 +351,17 @@ func (a *PatronRequestApiHandler) PostPatronRequests(w http.ResponseWriter, r *h
351351
api.AddInternalError(ctx, w, err)
352352
return
353353
}
354-
actionMapping, err := a.actionMappingService.GetActionMapping(illRequest)
354+
stateModelName, err := a.actionMappingService.GetStateModelNameForRequest(illRequest)
355355
if err != nil {
356356
api.AddInternalError(ctx, w, err)
357357
return
358358
}
359+
stateModel, err := a.actionMappingService.GetStateModel(stateModelName)
360+
if err != nil {
361+
api.AddInternalError(ctx, w, err)
362+
return
363+
}
364+
actionMapping := prservice.NewActionMapping(stateModel)
359365
borrowerInitialState, ok := actionMapping.GetInitialState(prservice.SideBorrowing)
360366
if !ok {
361367
api.AddInternalError(ctx, w, fmt.Errorf("no initial state defined for borrower side"))
@@ -385,7 +391,7 @@ func (a *PatronRequestApiHandler) PostPatronRequests(w http.ResponseWriter, r *h
385391
}
386392
}
387393

388-
dbreq := buildDbPatronRequest(&newPr, params.XOkapiTenant, creationTime, requesterReqId, illRequest, borrowerInitialState)
394+
dbreq := buildDbPatronRequest(&newPr, params.XOkapiTenant, creationTime, requesterReqId, illRequest, borrowerInitialState, stateModelName)
389395
pr, err := a.prRepo.CreatePatronRequest(ctx, pr_db.CreatePatronRequestParams(dbreq))
390396
if err != nil {
391397
var pgErr *pgconn.PgError
@@ -604,9 +610,15 @@ func (a *PatronRequestApiHandler) PutPatronRequestsId(w http.ResponseWriter, r *
604610
api.AddInternalError(ctx, w, err)
605611
return
606612
}
613+
stateModelName, err := a.actionMappingService.GetStateModelNameForRequest(illRequest)
614+
if err != nil {
615+
api.AddInternalError(ctx, w, err)
616+
return
617+
}
607618

608619
existingPr.RequesterReqID = getDbText(&requesterReqId)
609620
existingPr.IllRequest = illRequest
621+
existingPr.StateModel = stateModelName
610622
existingPr.Patron = getDbText(newPr.Patron)
611623
if newPr.InternalNote != nil {
612624
var note pgtype.Text
@@ -1351,6 +1363,7 @@ func toApiPatronRequest(r *http.Request, request pr_db.PatronRequestSearchView)
13511363
Id: request.ID,
13521364
CreatedAt: request.CreatedAt.Time,
13531365
State: string(request.State),
1366+
StateModel: request.StateModel,
13541367
Side: string(request.Side),
13551368
Patron: toString(request.Patron),
13561369
RequesterSymbol: toString(request.RequesterSymbol),
@@ -1501,6 +1514,7 @@ func buildDbPatronRequest(
15011514
requesterReqId string,
15021515
illRequest iso18626.Request,
15031516
initialState pr_db.PatronRequestState,
1517+
stateModel string,
15041518
) pr_db.PatronRequest {
15051519
return pr_db.PatronRequest{
15061520
ID: requesterReqId,
@@ -1518,6 +1532,7 @@ func buildDbPatronRequest(
15181532
Items: []pr_db.PrItem{},
15191533
TerminalState: false,
15201534
NeedsAttention: true,
1535+
StateModel: stateModel,
15211536
// LastAction, LastActionOutcome and LastActionResult are not set on creation
15221537
// they will be updated when the first action is executed.
15231538
}

‎broker/patron_request/api/api-handler_test.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,10 @@ func TestToApiPatronRequestSurfacesInternalNote(t *testing.T) {
128128
pr := pr_db.PatronRequest{
129129
ID: "pr-1",
130130
InternalNote: pgtype.Text{String: "staff note", Valid: true},
131+
StateModel: "returnables",
131132
}
132133
apiPr := toApiPatronRequest(req, patronRequestSearchViewFromPatronRequest(pr, false))
134+
assert.Equal(t, "returnables", apiPr.StateModel)
133135
if assert.NotNil(t, apiPr.InternalNote) {
134136
assert.Equal(t, "staff note", *apiPr.InternalNote)
135137
}
@@ -157,6 +159,7 @@ func patronRequestSearchViewFromPatronRequest(pr pr_db.PatronRequest, hasCost bo
157159
UpdatedAt: pr.UpdatedAt,
158160
IllResponse: pr.IllResponse,
159161
InternalNote: pr.InternalNote,
162+
StateModel: pr.StateModel,
160163
HasCost: hasCost,
161164
}
162165
}
@@ -760,14 +763,15 @@ func TestParseAndValidateIllRequestAndBuildDbPatronRequest(t *testing.T) {
760763
illRequest, requesterReqID, err := handler.parseAndValidateIllRequest(ctx, reqWithID, creationTime)
761764
assert.NoError(t, err)
762765
assert.Equal(t, id, requesterReqID)
763-
pr := buildDbPatronRequest(reqWithID, nil, pgtype.Timestamp{Valid: true, Time: creationTime}, requesterReqID, illRequest, prservice.BorrowerStateNew)
766+
pr := buildDbPatronRequest(reqWithID, nil, pgtype.Timestamp{Valid: true, Time: creationTime}, requesterReqID, illRequest, prservice.BorrowerStateNew, "returnables")
764767
assert.Equal(t, id, pr.ID)
765768
assert.True(t, pr.CreatedAt.Valid)
766769
assert.True(t, pr.RequesterReqID.Valid)
767770
assert.Equal(t, id, pr.RequesterReqID.String)
768771
assert.False(t, pr.SupplierSymbol.Valid)
769772
assert.Equal(t, patron, pr.Patron.String)
770773
assert.Equal(t, patron, pr.IllRequest.PatronInfo.PatronId)
774+
assert.Equal(t, "returnables", pr.StateModel)
771775

772776
reqWithoutID := &proapi.CreatePatronRequest{RequesterSymbol: &symbol}
773777
_, _, err = handler.parseAndValidateIllRequest(ctx, reqWithoutID, creationTime)
@@ -1633,6 +1637,7 @@ func TestPutPatronRequestsIdOK(t *testing.T) {
16331637
assert.Equal(t, patron, repo.lastUpdateParams.Patron.String)
16341638
assert.True(t, repo.lastUpdateParams.InternalNote.Valid)
16351639
assert.Equal(t, note, repo.lastUpdateParams.InternalNote.String)
1640+
assert.Equal(t, "returnables", repo.lastUpdateParams.StateModel)
16361641
}
16371642
var response proapi.PatronRequest
16381643
err := json.Unmarshal(rr.Body.Bytes(), &response)

‎broker/patron_request/db/prcql.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,7 @@ func (q *Queries) ListPatronRequestsCql(ctx context.Context, db DBTX, arg ListPa
302302
&i.PatronRequestSearchView.NextReqID,
303303
&i.PatronRequestSearchView.PrevReqID,
304304
&i.PatronRequestSearchView.RetryBibInfo,
305+
&i.PatronRequestSearchView.StateModel,
305306
&i.PatronRequestSearchView.HasNotification,
306307
&i.PatronRequestSearchView.HasCost,
307308
&i.PatronRequestSearchView.HasUnreadNotification,

‎broker/patron_request/db/prrepo.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,7 @@ func patronRequestFromSearchView(v PatronRequestSearchView) PatronRequest {
210210
UpdatedAt: v.UpdatedAt,
211211
IllResponse: v.IllResponse,
212212
InternalNote: v.InternalNote,
213+
StateModel: v.StateModel,
213214
}
214215
}
215216

‎broker/patron_request/service/action.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -671,6 +671,7 @@ func (a *PatronRequestActionService) acceptRetryBorrowingRequest(ctx common.Exte
671671
retryPr.Language = pr.Language
672672
retryPr.Items = []pr_db.PrItem{}
673673
retryPr.RetryBibInfo = nil
674+
retryPr.StateModel = pr.StateModel
674675
if pr.RetryBibInfo != nil {
675676
// only take selected fields from retry bib info to allow for corrections without affecting other fields
676677
if pr.RetryBibInfo.SupplierUniqueRecordId != "" {

‎broker/patron_request/service/action_mapping.go‎

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package prservice
22

33
import (
4+
"fmt"
45
"slices"
6+
"sort"
57
"strings"
68

79
"github.com/indexdata/crosslink/broker/events"
@@ -29,8 +31,45 @@ func (r *ActionMappingService) GetActionMapping(request iso18626.Request) (*Acti
2931
}
3032

3133
func (r *ActionMappingService) GetStateModelForRequest(request iso18626.Request) (*proapi.StateModel, error) {
32-
//TODO: check the ISO18626Request to decide what kind of state model/mapping to return
33-
return r.GetStateModel("returnables")
34+
modelName, err := r.GetStateModelNameForRequest(request)
35+
if err != nil {
36+
return nil, err
37+
}
38+
return r.GetStateModel(modelName)
39+
}
40+
41+
func (r *ActionMappingService) GetStateModelNameForRequest(request iso18626.Request) (string, error) {
42+
if request.ServiceInfo == nil {
43+
var selectableModels []string
44+
for name, stateModel := range stateModelsConfig.StateModels {
45+
if stateModel.Selector != nil {
46+
selectableModels = append(selectableModels, name)
47+
}
48+
}
49+
sort.Strings(selectableModels)
50+
if len(selectableModels) == 1 {
51+
return selectableModels[0], nil
52+
}
53+
return "", fmt.Errorf("cannot select state model without service info: found %d selectable models", len(selectableModels))
54+
}
55+
56+
serviceType := proapi.StateModelSelectorServiceType(request.ServiceInfo.ServiceType)
57+
var matches []string
58+
for name, stateModel := range stateModelsConfig.StateModels {
59+
if stateModel.Selector != nil && slices.Contains(stateModel.Selector.ServiceType, serviceType) {
60+
matches = append(matches, name)
61+
}
62+
}
63+
sort.Strings(matches)
64+
65+
switch len(matches) {
66+
case 0:
67+
return "", fmt.Errorf("no state model matches service type %q", serviceType)
68+
case 1:
69+
return matches[0], nil
70+
default:
71+
return "", fmt.Errorf("multiple state models match service type %q: %s", serviceType, strings.Join(matches, ", "))
72+
}
3473
}
3574

3675
func (r *ActionMappingService) GetStateModel(modelName string) (*proapi.StateModel, error) {

‎broker/patron_request/service/action_mapping_test.go‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,61 @@ var actionMappingService = ActionMappingService{}
5353

5454
func mustActionMapping(t *testing.T) *ActionMapping {
5555
t.Helper()
56-
mapping, err := actionMappingService.GetActionMapping(iso18626.Request{})
56+
mapping, err := actionMappingService.GetActionMapping(iso18626.Request{
57+
ServiceInfo: &iso18626.ServiceInfo{ServiceType: iso18626.TypeServiceTypeLoan},
58+
})
5759
assert.NoError(t, err)
5860
assert.NotNil(t, mapping)
5961
return mapping
6062
}
6163

64+
func TestGetStateModelForRequestUsesSelector(t *testing.T) {
65+
service := ActionMappingService{}
66+
67+
for _, serviceType := range []iso18626.TypeServiceType{
68+
iso18626.TypeServiceTypeCopy,
69+
iso18626.TypeServiceTypeLoan,
70+
iso18626.TypeServiceTypeCopyOrLoan,
71+
} {
72+
t.Run(string(serviceType), func(t *testing.T) {
73+
model, err := service.GetStateModelForRequest(iso18626.Request{
74+
ServiceInfo: &iso18626.ServiceInfo{ServiceType: serviceType},
75+
})
76+
assert.NoError(t, err)
77+
if assert.NotNil(t, model) {
78+
assert.Equal(t, "CrossLink Returnables State Model", model.Name)
79+
}
80+
})
81+
}
82+
}
83+
84+
func TestGetStateModelNameForRequestUsesSelector(t *testing.T) {
85+
name, err := (&ActionMappingService{}).GetStateModelNameForRequest(iso18626.Request{
86+
ServiceInfo: &iso18626.ServiceInfo{ServiceType: iso18626.TypeServiceTypeLoan},
87+
})
88+
89+
assert.NoError(t, err)
90+
assert.Equal(t, "returnables", name)
91+
}
92+
93+
func TestGetStateModelForRequestWithoutServiceInfoUsesOnlyConfiguredModel(t *testing.T) {
94+
model, err := (&ActionMappingService{}).GetStateModelForRequest(iso18626.Request{})
95+
96+
assert.NoError(t, err)
97+
if assert.NotNil(t, model) {
98+
assert.Equal(t, "CrossLink Returnables State Model", model.Name)
99+
}
100+
}
101+
102+
func TestGetStateModelForRequestWithoutMatch(t *testing.T) {
103+
model, err := (&ActionMappingService{}).GetStateModelForRequest(iso18626.Request{
104+
ServiceInfo: &iso18626.ServiceInfo{ServiceType: iso18626.TypeServiceType("Unsupported")},
105+
})
106+
107+
assert.Nil(t, model)
108+
assert.EqualError(t, err, `no state model matches service type "Unsupported"`)
109+
}
110+
62111
func TestIsActionAvailable(t *testing.T) {
63112
mapping := mustActionMapping(t)
64113
// Borrower

0 commit comments

Comments
 (0)