Skip to content

Fix: auto-flatten paged /users responses; slim User attributes#55

Merged
alexskr merged 10 commits intomasterfrom
fix/slow-users-endpoint
May 1, 2026
Merged

Fix: auto-flatten paged /users responses; slim User attributes#55
alexskr merged 10 commits intomasterfrom
fix/slow-users-endpoint

Conversation

@mdorf
Copy link
Copy Markdown
Member

@mdorf mdorf commented Apr 29, 2026

PR description

Summary

  • Collection#all detects paged API responses (collection/pageCount/nextPage shape) and walks nextPage links transparently, returning a flat Array. Pre-pagination Array responses pass through unchanged. Internal pagesize: 5_000 to minimize request count.
  • Models::User defaults to a slim @include_attrs allow-list (username, email, role, firstName, lastName, created) instead of "all", and overrides .all to inject display_context: false, display_links: false. Drops the heavy User payload (custom ontologies, tokens, hashes, JSON-LD context) for list/select use cases.
  • Adds a manual test/benchmark/users.rb harness for reproducing and quantifying the production timeout (not loaded by the test suite — invoked manually with UT_APIKEY=… bundle exec rubytest/benchmark/users.rb).

Why

Production users were unable to create ontologies, with Faraday::TimeoutError from bioportal_web_ui controllers calling LinkedData::Client::Models::User.all. The fix spans four repos:

  1. Fix /users perf with security: idempotent admin?, drop inverse attrs from auth load ontologies_linked_data#286 (merged) — idempotent User#admin?, drop inverse attrs from auth load
  2. ncbo/ontologies_api fix/slow-users-endpoint PR — mandatory pagination on GET /users (+ a related /search and /property_search 500 fix)
  3. This PR — client-side auto-flatten + slim User attrs
  4. ncbo/bioportal_web_ui — bump Gemfile.lock to the new client release (no controller code changes needed)

This client release is what makes the ontologies_api breaking change (mandatory pagination) transparent to existing in-process callers.

Backward / forward compatibility

API response Old client New client
Flat Array (current /users, /ontologies, etc.) returns Array returns Array (passthrough — paged_collection? false)
Paged Hash (new /users, future paginated endpoints) returns Hash unchanged → callers iterating it as Array break walks nextPage, returns flat Array
Caller passes :page explicitly returns response unchanged returns response unchanged (passthrough)

Test plan

  • UT_APIKEY=<key> bundle exec rake test TEST="./test/models/test_collection.rb" — three new tests pass: test_all_flattens_paged_collections, test_all_returns_page_when_page_requested,
    test_user_all_uses_lightweight_defaults. Existing test_all, test_class_for_type, test_where continue to pass.
  • Note: test_find and test_find_by will fail when run against a local API — they hard-code https://data.bioontology.org/... URIs while a local triplestore stores
    http://localhost:9393/.... Pre-existing issue (introduced 2023-07-19, not a regression from this PR). Worth a separate cleanup ticket.
  • Manual: UT_APIKEY=<key> bundle exec ruby test/benchmark/users.rb reports per-page and full-walk timings consistent with the table above.
  • Smoke test against staging (after API deploys): Models::User.all returns a flat Array; Models::User.all(page: 1) returns a paged response object; Models::Ontology.all continues to return
    a flat Array unchanged.

mdorf added 6 commits July 18, 2024 10:54
Pairs with the ontologies_api /users pagination change. Two changes
that together let existing callers of
`LinkedData::Client::Models::User.all` keep working without code
changes after the API switches to mandatory pagination.

Collection#all detects paged responses (collection/pageCount/nextPage
shape) and walks `nextPage` links transparently, returning a flat
Array. Falls through to existing behavior when the response is
already an Array (pre-pagination back-compat) or when the caller
explicitly requested a `:page` (passthrough). Internal pagesize:
5_000 to minimize request count.

Models::User defaults to a slim `@include_attrs` allow-list
(username, email, role, firstName, lastName, created) instead of
"all", and overrides `.all` to inject `display_context: false,
display_links: false`. Drops the heavy User payload (custom
ontologies, tokens, hashes, JSON-LD context, HATEOAS links) for
list/select use cases.

Adds Collection tests for paged-flatten, page-passthrough, and
User.all slim-defaults.
Manual benchmark harness for the /users endpoint. Times two paths
(raw Net::HTTP and Models::User.all) plus an optional
--page=N --pagesize=M single-page mode for isolating per-page cost
without the all-pages walk.

Used to verify the ontologies_api + ontologies_linked_data#286
fixes against the production timeout issue. Not loaded by the test
suite; invoked manually via:

  UT_APIKEY=<key> bundle exec ruby test/benchmark/users.rb
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 29, 2026

Codecov Report

❌ Patch coverage is 96.96970% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 68.34%. Comparing base (8549896) to head (dad23b9).

Files with missing lines Patch % Lines
lib/ontologies_api_client/collection.rb 96.55% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #55      +/-   ##
==========================================
+ Coverage   66.48%   68.34%   +1.86%     
==========================================
  Files          29       29              
  Lines         919      973      +54     
  Branches      174      188      +14     
==========================================
+ Hits          611      665      +54     
  Misses        308      308              
Flag Coverage Δ
3.2 68.34% <96.96%> (+1.86%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mdorf mdorf changed the base branch from develop to master April 30, 2026 20:16
mdorf added 4 commits April 30, 2026 13:18
Per PR review feedback: the back-compat invariant for non-paginated
endpoints (e.g. /ontologies, /groups, /categories today) is currently
only exercised implicitly by the network-dependent `test_all`. If we
ever paginate one of those endpoints, that test would silently switch
to exercising the paged path instead, leaving the Array passthrough
uncovered.

Adds test_all_passes_through_array_response — stubs entry_point to
return a flat Array and asserts Collection#all returns it unchanged.
Also asserts pagesize: 5_000 is still injected on the request (the
API simply ignores it on non-paginated endpoints).
@alexskr alexskr merged commit 91eb87d into master May 1, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants