Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
58 changes: 56 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
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,13 @@ components:
lastLink:
type: string
description: Link to the last page of results
AboutWithFacets:
allOf:
- $ref: '#/components/schemas/About'
- type: object
properties:
facets:
$ref: '#/components/schemas/FacetsResult'
Event:
type: object
properties:
Expand Down Expand Up @@ -546,7 +599,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 +1506,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
}
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)
Comment thread
adamdickmeiss marked this conversation as resolved.

Comment thread
adamdickmeiss marked this conversation as resolved.
idx := strings.Index(sql, "GROUP BY")
if idx == -1 {
return nil, fmt.Errorf("base SQL query missing GROUP BY clause")
}
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)
}
}
return facets, nil
}

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
WHERE ill_request IS NOT NULL
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