Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
bd3eb9f
CROSSLINK-274 openapi changes
adamdickmeiss May 26, 2026
18e1a8a
Functional
adamdickmeiss May 26, 2026
7adb3ab
Lint
adamdickmeiss May 26, 2026
6f8f9bb
Merge remote-tracking branch 'origin/main' into CROSSLINK-274-patron-…
adamdickmeiss May 26, 2026
e443966
About confusion
adamdickmeiss May 26, 2026
8c8771e
Includes ill_request IS NOT NULL
adamdickmeiss May 26, 2026
53cbba1
Ensure base where clause is applied to whole CQL where clause
adamdickmeiss May 26, 2026
db35bae
trimspace
adamdickmeiss May 26, 2026
741f620
Rename
adamdickmeiss May 26, 2026
8272b29
Potential fix for pull request finding
adamdickmeiss May 26, 2026
29478d3
trimmedField
adamdickmeiss May 26, 2026
0abe8e2
Limit facet results to patron requests in test
adamdickmeiss May 26, 2026
fe3775f
Fix description
adamdickmeiss May 26, 2026
e2c2c2b
FacetValue, limit 100
adamdickmeiss May 26, 2026
3b99463
Fixes, test with supplier_symbol facet
adamdickmeiss May 26, 2026
c9f82fb
Merge remote-tracking branch 'origin/main' into CROSSLINK-274-patron-…
adamdickmeiss May 26, 2026
4292f47
Cover case facets=
adamdickmeiss May 26, 2026
69a7a50
lint
adamdickmeiss May 26, 2026
55d9bf0
Ignore empty facet components
adamdickmeiss May 26, 2026
0f99e23
sort 2nd on value
adamdickmeiss May 26, 2026
99e63f5
Yet another rename
adamdickmeiss May 26, 2026
48e88cd
Merge remote-tracking branch 'origin/main' into CROSSLINK-274-patron-…
adamdickmeiss May 26, 2026
6941da3
Rename
adamdickmeiss May 27, 2026
89350b9
CQL query always present for facets
adamdickmeiss May 27, 2026
08fca4c
Use OpenAPI param schema
jakub-id May 27, 2026
372bc57
Use typed errors
jakub-id May 27, 2026
632cd52
Comments and cosmetics
jakub-id May 27, 2026
6630a6e
Simplify, test case without user-supplied cql
adamdickmeiss May 27, 2026
d22b007
AboutWithFacets
adamdickmeiss May 27, 2026
f8b34e0
Potential fix for pull request finding
adamdickmeiss May 27, 2026
4f5bd89
Reformat, avoid :
adamdickmeiss May 27, 2026
da8e7b7
OpenAPI rejects blanks in facets spec
adamdickmeiss May 27, 2026
fc44e77
Update test for OpenAPI validation
adamdickmeiss May 27, 2026
31d5f47
Mention 100 entries for now
adamdickmeiss May 27, 2026
413e363
Make GetPatronRequestsFacets strict
adamdickmeiss May 27, 2026
5fe8682
Useless comment now
adamdickmeiss May 27, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 72 additions & 2 deletions broker/oapi/open-api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,22 @@ components:
description: Patron request ID
schema:
type: string
Facets:
name: facets
in: query
description: Comma separated list of facet names to include in the response.
Supported values are "requester_symbol" and "supplier_symbol";
unsupported facet names result in a 400 response.
For example "requester_symbol,supplier_symbol"
style: form
explode: false
schema:
type: array
items:
type: string
Comment thread
adamdickmeiss marked this conversation as resolved.
enum:
- requester_symbol
- supplier_symbol
schemas:
Index:
type: object
Expand Down Expand Up @@ -197,6 +213,36 @@ components:
description: List of peers
items:
$ref: '#/components/schemas/LocatedSupplier'
FacetsResult:
type: array
description: List of facets for the result
items:
type: object
required:
- name
- values
properties:
name:
type: string
description: Facet name
values:
type: array
description: List of facet values. At most 100 entries are returned.
items:
$ref: '#/components/schemas/FacetResultValue'
FacetResultValue:
type: object
required:
- value
- count
properties:
value:
type: string
description: Facet value
count:
type: integer
format: int64
description: Count of items for this facet value
About:
type: object
required:
Expand All @@ -218,6 +264,29 @@ components:
lastLink:
type: string
description: Link to the last page of results
AboutWithFacets:
type: object
required:
- count
properties:
count:
type: integer
format: int64
description: Total number of items in the result
nextLink:
type: string
description: Link to the next page of results
prevLink:
type: string
description: Link to the previous page of results
firstLink:
type: string
description: Link to the first page of results
lastLink:
type: string
description: Link to the last page of results
facets:
$ref: '#/components/schemas/FacetsResult'
Event:
Comment thread
adamdickmeiss marked this conversation as resolved.
type: object
properties:
Expand Down Expand Up @@ -546,7 +615,7 @@ components:
- about
properties:
about:
$ref: '#/components/schemas/About'
$ref: '#/components/schemas/AboutWithFacets'
items:
type: array
description: List of patron request
Expand Down Expand Up @@ -1453,9 +1522,10 @@ paths:
- $ref: '#/components/parameters/Offset'
- $ref: '#/components/parameters/Side'
- $ref: '#/components/parameters/Symbol'
- $ref: '#/components/parameters/Facets'
responses:
'200':
description: Successful retrieval of events
description: Successful retrieval of patron requests
content:
application/json:
schema:
Expand Down
38 changes: 37 additions & 1 deletion broker/patron_request/api/api-handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,46 @@ func (a *PatronRequestApiHandler) GetPatronRequests(w http.ResponseWriter, r *ht
}

resp := proapi.PatronRequests{Items: responseItems}
resp.About = proapi.About(api.CollectAboutData(count, offset, limit, r))
resp.About = toProAboutWithFacets(api.CollectAboutData(count, offset, limit, r))
var facets []pr_db.Facet
if params.Facets != nil {
facets, err = a.prRepo.GetPatronRequestsFacets(ctx, *params.Facets, cqlStr)
}
if err != nil {
if errors.Is(err, pr_db.ErrUnsupportedFacet) {
api.AddBadRequestError(ctx, w, err)
} else {
api.AddInternalError(ctx, w, err)
}
return
Comment thread
jakub-id marked this conversation as resolved.
}
Comment thread
adamdickmeiss marked this conversation as resolved.
Comment thread
adamdickmeiss marked this conversation as resolved.
if len(facets) > 0 {
facetResults := make(proapi.FacetsResult, len(facets))
for i, field := range facets {
facetResults[i].Name = field.Field
facetResults[i].Values = make([]proapi.FacetResultValue, len(field.Values))
for j, value := range field.Values {
facetResults[i].Values[j] = proapi.FacetResultValue{
Value: value.Value,
Count: value.Count,
}
}
}
resp.About.Facets = &facetResults
}
api.WriteJsonResponse(w, resp)
}

func toProAboutWithFacets(a oapi.About) proapi.AboutWithFacets {
return proapi.AboutWithFacets{
Count: a.Count,
FirstLink: a.FirstLink,
LastLink: a.LastLink,
NextLink: a.NextLink,
PrevLink: a.PrevLink,
}
}

func AddOwnerRestriction(queryBuilder *cqlbuilder.QueryBuilder, symbol string, side pr_db.PatronRequestSide) (*cqlbuilder.QueryBuilder, error) {
var err error
switch side {
Expand Down
39 changes: 39 additions & 0 deletions broker/patron_request/api/api-handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"context"
"encoding/json"
"errors"
"fmt"
"net/http"
"net/http/httptest"
"strconv"
Expand Down Expand Up @@ -168,6 +169,32 @@ func TestGetPatronRequests(t *testing.T) {
assert.Contains(t, rr.Body.String(), "DB error")
}

func TestGetPatronRequestsFacetsDBError(t *testing.T) {
facets := proapi.Facets{"requester_symbol"}
handler := NewPrApiHandler(new(PrRepoError), mockEventBus, mockEventRepo, tenant.NewResolver(), nil, 10)
req, _ := http.NewRequest("GET", "/", nil)
rr := httptest.NewRecorder()
params := proapi.GetPatronRequestsParams{
Facets: &facets,
}
handler.GetPatronRequests(rr, req, params)
assert.Equal(t, http.StatusInternalServerError, rr.Code)
assert.Contains(t, rr.Body.String(), "DB error")
}

func TestGetPatronRequestsFacetsUnsupported(t *testing.T) {
facets := proapi.Facets{"nosuch"}
handler := NewPrApiHandler(new(PrRepoFacetsUnsupported), mockEventBus, mockEventRepo, tenant.NewResolver(), nil, 10)
req, _ := http.NewRequest("GET", "/", nil)
rr := httptest.NewRecorder()
params := proapi.GetPatronRequestsParams{
Facets: &facets,
}
handler.GetPatronRequests(rr, req, params)
assert.Equal(t, http.StatusBadRequest, rr.Code)
assert.Contains(t, rr.Body.String(), "nosuch")
}

func TestGetPatronRequestsNoSymbol(t *testing.T) {
repo := new(PrRepoCapture)
handler := NewPrApiHandler(repo, mockEventBus, mockEventRepo, tenant.NewResolver(), nil, 10)
Expand Down Expand Up @@ -1017,6 +1044,18 @@ func (r *PrRepoError) GetNotificationById(ctx common.ExtendedContext, id string)
}
}

func (r *PrRepoError) GetPatronRequestsFacets(_ common.ExtendedContext, _ []string, _ string) ([]pr_db.Facet, error) {
return nil, errors.New("DB error")
}

type PrRepoFacetsUnsupported struct {
PrRepoCapture
}

func (r *PrRepoFacetsUnsupported) GetPatronRequestsFacets(_ common.ExtendedContext, _ []string, _ string) ([]pr_db.Facet, error) {
return nil, fmt.Errorf("%w: nosuch", pr_db.ErrUnsupportedFacet)
}

type MockIso18626Handler struct {
mock.Mock
handler.Iso18626Handler
Expand Down
39 changes: 39 additions & 0 deletions broker/patron_request/db/prcql.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,45 @@ func handlePatronRequestsQuery(cqlString string, noBaseArgs int) (pgcql.Query, e
return def.Parse(query, noBaseArgs+1)
}

// facetFieldPlaceholder is the column name used in the facetsPatronRequests SQL template.
// FacetsPatronRequestsCql substitutes it with the validated facet field at runtime.
const facetFieldPlaceholder = "requester_symbol"

func (q *Queries) FacetsPatronRequestsCql(ctx context.Context, db DBTX, facetField string, cqlString string) ([]FacetsPatronRequestsRow, error) {
// facetField is validated against an allowlist by the caller (GetPatronRequestsFacets),
// so it is safe to substitute directly as a column name.
sql := strings.Replace(facetsPatronRequests, facetFieldPlaceholder, facetField, 1)

idx := strings.Index(sql, "GROUP BY")
if idx == -1 {
return nil, fmt.Errorf("base SQL query missing GROUP BY clause")
}
Comment thread
adamdickmeiss marked this conversation as resolved.
res, err := handlePatronRequestsQuery(cqlString, 0)
if err != nil {
return nil, fmt.Errorf("failed to handle CQL query: %w", err)
}
if res.GetWhereClause() != "" {
sql = sql[:idx] + "AND (" + res.GetWhereClause() + ") " + sql[idx:]
}
rows, err := db.Query(ctx, sql, res.GetQueryArguments()...)
if err != nil {
return nil, fmt.Errorf("failed to execute facets query: %w", err)
}
defer rows.Close()
var items []FacetsPatronRequestsRow
for rows.Next() {
var i FacetsPatronRequestsRow
if err := rows.Scan(&i.Value, &i.Count); err != nil {
return nil, err
}
items = append(items, i)
}
if err := rows.Err(); err != nil {
return nil, err
}
return items, nil
}

func (q *Queries) ListPatronRequestsCql(ctx context.Context, db DBTX, arg ListPatronRequestsParams,
cqlString *string, explainAnalyze bool) ([]ListPatronRequestsRow, []string, error) {
if cqlString == nil {
Expand Down
44 changes: 44 additions & 0 deletions broker/patron_request/db/prrepo.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package pr_db

import (
"errors"
"fmt"
"strings"

"github.com/indexdata/crosslink/broker/common"
Expand All @@ -18,6 +20,7 @@ type PrRepo interface {
GetPatronRequestByIdAndSide(ctx common.ExtendedContext, id string, side PatronRequestSide) (PatronRequest, error)
ListPatronRequests(ctx common.ExtendedContext, args ListPatronRequestsParams, cql *string) ([]PatronRequest, int64, error)
ListPatronRequestsSearchView(ctx common.ExtendedContext, args ListPatronRequestsParams, cql *string) ([]PatronRequestSearchView, int64, error)
GetPatronRequestsFacets(ctx common.ExtendedContext, facetFields []string, cql string) ([]Facet, error)
UpdatePatronRequest(ctx common.ExtendedContext, params UpdatePatronRequestParams) (PatronRequest, error)
UpdatePatronRequestInternalNote(ctx common.ExtendedContext, id string, internalNote pgtype.Text) error
CreatePatronRequest(ctx common.ExtendedContext, params CreatePatronRequestParams) (PatronRequest, error)
Expand All @@ -35,6 +38,18 @@ type PrRepo interface {
DeleteItemById(ctx common.ExtendedContext, id string) error
}

var ErrUnsupportedFacet = errors.New("unsupported facet field")

type Facet struct {
Field string
Values []FacetValue
}

type FacetValue struct {
Value string
Count int64
}

type PgPrRepo struct {
repo.PgBaseRepo[PrRepo]
queries Queries
Expand Down Expand Up @@ -102,6 +117,35 @@ func (r *PgPrRepo) ListPatronRequests(ctx common.ExtendedContext, params ListPat
return list, fullCount, nil
}

func (r *PgPrRepo) GetPatronRequestsFacets(ctx common.ExtendedContext, facetFields []string, cql string) ([]Facet, error) {
var facets []Facet
for _, field := range facetFields {
switch field {
case "requester_symbol", "supplier_symbol":
rows, err := r.queries.FacetsPatronRequestsCql(ctx, r.GetConnOrTx(), field, cql)
if err != nil {
return nil, err
}
var values []FacetValue
for _, row := range rows {
if row.Value.Valid {
values = append(values, FacetValue{
Value: row.Value.String,
Count: row.Count,
})
}
}
facets = append(facets, Facet{
Field: field,
Values: values,
})
default:
return nil, fmt.Errorf("%w: %s", ErrUnsupportedFacet, field)
}
}
Comment thread
adamdickmeiss marked this conversation as resolved.
return facets, nil
Comment thread
jakub-id marked this conversation as resolved.
}

func (r *PgPrRepo) ListPatronRequestsSearchView(ctx common.ExtendedContext, params ListPatronRequestsParams, cql *string) ([]PatronRequestSearchView, int64, error) {
rows, fullCount, err := r.listPatronRequestRows(ctx, params, cql)
if err != nil {
Expand Down
11 changes: 11 additions & 0 deletions broker/sqlc/pr_query.sql
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,17 @@ WHERE ill_request IS NOT NULL
ORDER BY created_at
LIMIT $1 OFFSET $2;

-- name: FacetsPatronRequests :many
-- TEMPLATE: 'requester_symbol' is a placeholder column name. This query is not
-- meant to be called directly; use FacetsPatronRequestsCql in prcql.go, which
-- substitutes the column with the validated facet field at runtime.
SELECT requester_symbol AS value, COUNT(*) AS count
FROM patron_request_search_view
Comment thread
adamdickmeiss marked this conversation as resolved.
WHERE ill_request IS NOT NULL
Comment thread
adamdickmeiss marked this conversation as resolved.
GROUP BY 1
ORDER BY count DESC, value ASC
LIMIT 100;

-- name: UpdatePatronRequest :one
-- internal_note ($20) is a pass-through to keep PatronRequest <-> UpdatePatronRequestParams
-- convertible; edits go through UpdatePatronRequestInternalNote.
Expand Down
Loading
Loading