Skip to content

🐛 Fix incomplete OPDS 2.0 catalog pagination - #1444

Open
eduardojvieira wants to merge 1 commit into
stumpapp:nightlyfrom
eduardojvieira:fix/opds2-complete-pagination
Open

eduardojvieira wants to merge 1 commit into
stumpapp:nightlyfrom
eduardojvieira:fix/opds2-complete-pagination

Conversation

@eduardojvieira

Copy link
Copy Markdown

Closes #1443

Description

Fix OPDS 2.0 feeds that stopped KOReader from reaching complete library, series, and book lists:

  • Give /libraries a real self URL and page-specific next/previous links; report the requested page in metadata.
  • Add a permission-scoped, paginated /libraries/{id}/series feed and an All Series navigation entry in each library. KOReader 2026.07.2 ignores group links when a group also has navigation, so the top-level entry makes the complete series list reachable while retaining previews.
  • Stop book feeds from linking past their final page, correct their start URL, and preserve pagination parameters in links.

OPDS 1.2 is intentionally unchanged: its libraries feed returned the full list in the reproduction, and I could not establish a matching 1.2 truncation bug.

LLM disclosure: I used Codex to help inspect KOReader/Stump, implement the patch, and generate tests. I reviewed the diff and ran the checks below myself.

Verification

  • cargo fmt --check — passed
  • cargo test -p stump_server --test api_tests — 77 passed, 0 failed
  • cargo clippy -p stump_server --test api_tests — passed with three warnings in unrelated existing files (apps/server/src/middleware/auth.rs and apps/server/tests/reading_progress/manual_progression_changes.rs)
  • git diff --check — passed

The JSON/navigation contract is covered by new integration tests, including access denial for an excluded library. I could not test on a physical Kobo; this PR is not deployed to the reported server.

Ready?

  • I read the contributing guidelines
  • I searched for existing issues or pull requests that may be related to my contribution
  • This PR is based into nightly and not main
  • I added tests and/or documentation for my changes if applicable
  • I disclosed any use of LLMs in the creation of this PR (if applicable)

Stump Contributor License Agreement

By contributing to Stump, I agree that my contributions will be licensed under the applicable licenses described in the contribution template.

@codecov

codecov Bot commented Sep 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.10638% with 14 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
apps/server/src/routers/opds/v2_0.rs 85.10% 14 Missing ⚠️
Files with missing lines Coverage Δ
apps/server/src/routers/opds/v2_0.rs 55.79% <85.10%> (+13.70%) ⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@aaronleopold aaronleopold left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this fix, there are a few items that should be addressed before merge but otherwise things look good to me

))
.rel(OPDSLinkRel::Previous.item())
.build()
.expect("valid OPDS previous link"),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error is able to bubble up and return a response, we should not panic here. You can see other instances where this is done, e.g. build()?

))
.rel(OPDSLinkRel::Next.item())
.build()
.expect("valid OPDS next link"),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

))
.rel(OPDSLinkRel::Next.item())
.build()
.expect("static OPDS link is valid"),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another panic here we can remove

.href(pagination_href(&base_url, &pagination, page))
.rel(OPDSLinkRel::Previous.item())
.build()
.expect("valid OPDS previous link"),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

))
.rel(OPDSLinkRel::Next.item())
.build()
.expect("valid OPDS next link"),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

This branch has not been deployed

No deployments
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.

2 participants