camunda / camunda/api-test-generator

OffsetPagination limit has no maximum in offset based pagination

Open
#541 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

OCA
Dominant language
TypeScript
Stars
0
Forks
3
Avg merge
13h 41m
Merged PRs (30d)
23

Description

Parent: #538

82 failures, and the largest single group. Every __paginationLimit__aboveMaximum and
__paginationLimit__wayAboveMaximum test in the suite fails. 94 emitted, 0 passing, on every
deployment I tried.

The tests are wrong. The server is right.

What happens

{"page":{"limit":10001}} and {"page":{"limit":10100}} assert 400. They get 200 on rdbms and 500
on Elasticsearch.

Why

SearchQueryPageRequest is a oneOf over four branches.
request-validation/src/analysis/paginationLimit.ts:23-32 calls findLimitOnlyBranch, which returns
LimitPagination with minimum: 1 and maximum: 10000, then derives all four mutations from that
one branch at :34-45:

out.push({ kind: 'belowMinimum',    value: minimum - 1 });
out.push({ kind: 'wayBelowMinimum', value: minimum - 100 });
out.push({ kind: 'aboveMaximum',    value: maximum + 1 });
out.push({ kind: 'wayAboveMaximum', value: maximum + 100 });

It never looks at the siblings. OffsetPagination.limit (search-models.yaml:37-51) declares
minimum: 1 and no maximum. So {limit: 10001} still satisfies a branch, the payload is
contract-valid, and the generator is asserting a constraint the schema never imposed.

The below-minimum pair is fine, because 0 and -99 violate all four branches.

Evidence

High confidence. Reproduced live on both deployments: 10001 and 10100 return 200 (rdbms) or 500
(Elasticsearch), never 400, while -99 returns 400 on both. The suite-wide pass rate matches the
schema exactly. wayBelowMinimum passes 45 of 47. aboveMaximum and wayAboveMaximum pass 0 of 47
each.

Fix

In planLimitMutations, keep a mutation only when it violates every branch of the oneOf, rather
than only the limit-only branch.

This is smaller than it sounds, because the plumbing already exists. findPaginationPage returns
{ pageProp, branches }, so page.branches is the full oneOf array and it is already in scope at
paginationLimit.ts:52. Today the code narrows it to one branch through findLimitOnlyBranch and
throws the rest away.

paginationShape.ts:73 also already exports findOffsetBranch, added for paginationOffset.ts:38.
It is not directly reusable here, since it finds one specific branch rather than testing a value
against all of them. What it does show is that the file already knows the branches carry different
constraints, and that consulting a branch other than the limit-only one is an established pattern in
the same module. So this is an addition to existing structure, not new infrastructure.

Roughly what is needed:

function violatesEveryBranch(branches: SchemaFragment[], value: number): boolean {
  return branches.every((b) => {
    const lim = b.properties?.limit;
    if (!lim) return false;                          // branch imposes nothing, so it accepts
    if (typeof lim.minimum === 'number' && value < lim.minimum) return true;
    if (typeof lim.maximum === 'number' && value > lim.maximum) return true;
    return false;
  });
}

Filter the four planned mutations through it. That drops both above-maximum variants, because
OffsetPagination accepts them, and keeps both below-minimum ones, because every branch declares
minimum: 1. It also generalises to any future oneOf envelope instead of hard-coding a
Camunda-specific carve-out.

Tests

tests/request-validation/pagination-limit-invalid.test.ts asserts toHaveLength(4) and
values === [-99, 0, 10001, 10100] at :200, :213, :225 and :257. Those become length 2 and
[-99, 0].

Add a case with a oneOf where one branch declares no maximum, asserting the above-maximum
mutations get dropped.

Acceptance

  • Regenerate and diff. Exactly 94 tests removed, nothing else changed.
  • npx vitest run tests/request-validation/ green.

Dependencies

None. Shares no files with any other sub-issue. Good first merge.

Related upstream filings

Two, tracked in the parent. OffsetPagination.limit looks like it is simply missing
maximum: 10000, since the other three branches all have it. If upstream adds it, these scenarios
become correct and can come back on their own. Separately, the 500 on Elasticsearch is worth filing:
it is the max_result_window limit answering a request the contract currently permits.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in request-validation/src/analysis/paginationLimit.ts, especially planLimitMutations and the branches returned by findPaginationPage. Run tests/request-validation/pagination-limit-invalid.test.ts and inspect the oneOf cases in search-models.yaml. Done means invalid pagination mutations respect every branch, the affected assertions expect only [-99, 0], one missing-maximum case is covered, and npx vitest run tests/request-validation/ passes.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
testing-qa, tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
84/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.