Graylog2 / Graylog2/graylog2-server
PageListResponse.create — total vs grandTotal investigation
- Dominant language
- Java
- Stars
- 8.1k
- Forks
- 1.1k
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 217
Description
# `PageListResponse.create` — `total` vs `grandTotal` investigation
## Problem
`PageListResponse` (`graylog2-server/src/main/java/org/graylog2/rest/models/tools/responses/PageListResponse.java`) exposes two related but distinct counts:
- `total` — a top-level `long` field, meant to represent the number of results matching the current search/filter query.
- `paginationInfo` — a `PaginatedList.PaginationInfo`, which *also* carries its own `total()` (an `int`), representing the same thing: the filtered count, ignoring pagination.
The two 8-arg `create(...)` overloads accept `total` as an explicit, independent parameter instead of deriving it from `paginationInfo`. This means a caller can pass any value it wants for `total` — including one that has nothing to do with `paginationInfo.total()`. In practice, several call sites do exactly that, passing `result.grandTotal().orElse(0L)` instead.
`PaginatedList.grandTotal()` is a separate, optional concept: the count of *all* entries ignoring both pagination **and** query filters (e.g. total nodes in a cluster, regardless of any search query the user typed). It's `Optional.empty()` unless a caller explicitly populates it via the 5-arg `PaginatedList` constructor or a `grandTotalFilter(...)` step in `MongoPaginationHelper`.
**The bug:** for every buggy call site examined, `grandTotal()` is never actually populated (`Optional.empty()`), so `result.grandTotal().orElse(0L)` silently evaluates to **`0`** — the response reports `"total": 0` regardless of how many results actually matched the query. This breaks pagination UIs (e.g. "Showing 1-10 of 0").
A third `create(...)` overload takes a `PaginatedList` directly (no explicit `total` param) and *always* derives `total` from `paginatedList.pagination().total()` internally — this overload is immune to the bug by construction.
## Call site audit
25 call sites of `PageListResponse.create(...)` were audited across the codebase.
### ✅ Correct — uses filtered `total` (16 sites)
| File:line | Notes |
|---|---|
| `org/graylog/plugins/views/search/rest/DashboardsResource.java:167` | `result.pagination().total()` |
| `org/graylog/plugins/views/search/rest/SavedSearchesResource.java:126` | `result.pagination().total()` |
| `org/graylog/collectors/rest/FleetResource.java:130` | `result.pagination().total()` |
| `org/graylog/collectors/rest/CollectorInstancesResource.java:256` | `list.pagination().total()` |
| `org/graylog/collectors/rest/SourceResource.java:114` | `result.pagination().total()` |
| `org/graylog/collectors/opamp/rest/EnrollmentTokenResource.java:196` | `list.pagination().total()` |
| `org/graylog/events/rest/EventDefinitionsResource.java:281` | `definitionDtos.pagination().total()` |
| `org/graylog2/rest/resources/opensearch/OpensearchClusterResource.java` | fixed in commit `a34f5f45e6` |
| `org/graylog2/rest/resources/system/inputs/InputsResource.java:414` | `mappedResult.pagination().total()` |
| `org/graylog2/rest/resources/tokenusage/TokenUsageResource.java:128` | `pagination.total()` |
| `org/graylog2/rest/resources/system/indexer/IndexSetTemplateResource.java:230` | passthrough of an already-correct total |
| `org/graylog2/indexer/indexset/IndexSetFieldTypeSummaryService.java:112` | in-memory filtered total |
| `org/graylog2/indexer/indexset/profile/IndexFieldTypeProfileService.java:182` | 6-arg `PaginatedList` overload — always correct |
| `org/graylog2/indexer/indexset/template/IndexSetTemplateService.java:133` | 6-arg overload — always correct |
| `org/graylog2/indexer/fieldtypes/IndexFieldTypesListService.java:90` | in-memory filtered total |
| `org/graylog2/rest/resources/system/NotificationsResource.java:216` | 6-arg overload — always correct |
### ❌ Bug — uses `grandTotal()`, always reports `0` (8 sites)
| File:line | Expression | `grandTotal` actually populated upstream? |
|---|---|---|
| `org/graylog2/rest/resources/mongodb/MongodbClusterResource.java:154` | `result.grandTotal().orElse(0L)` | No |
| `org/graylog2/rest/resources/system/indexer/OutdatedIndexResource.java:165` | `result.grandTotal().orElse(0L)` | No |
| `org/graylog/events/rest/EventNotificationsResource.java:161` | `result.grandTotal().orElse(0L)` | No |
| `org/graylog2/rest/resources/streams/StreamPipelineRulesResource.java:129` | `result.grandTotal().orElse(0L)` | No |
| `org/graylog2/rest/resources/system/ClusterResource.java:171` | `result.grandTotal().orElse(0L)` | No |
| `org/graylog2/rest/resources/datanodes/DatanodeResource.java:114` | `result.grandTotal().orElse(0L)` | No |
| `org/graylog2/inputs/diagnosis/InputRoutingRulesService.java:183` (`getPipelineRulesPage`) | `paginatedList.grandTotal().orElse(0L)` | No — discards an already-correctly-computed count |
| `org/graylog2/inputs/diagnosis/InputRoutingRulesService.java:239` (`getStreamRulesPage`) | `paginatedList.grandTotal().orElse(0L)` | No — same pattern |
Note: `InputRoutingRulesService.java:116` also calls `.grandTotal().orElse(0L)`, but on an explicit `PaginatedList.emptyList(...)` where `grandTotal` is deliberately `0L` and the list is genuinely empty — not a real bug, just stylistically inconsistent with the fix below.
`org/graylog2/rest/resources/opensearch/OpensearchClusterResource.java` had the identical bug and was already fixed in commit `a34f5f45e6` ("fixed total count of results") by swapping `result.grandTotal().orElse(0L)` for `result.pagination().total()` — the same pattern applies to all 8 sites above.
### ⚠️ Different root cause, same symptom (1 site)
| File:line | Issue |
|---|---|
| `org/graylog2/rest/resources/streams/StreamResource.java:291` | Passes `total = paginatedStreamService.count()` — a separate, unfiltered, unpermissioned count query — instead of `result.pagination().total()`. Same visible bug (wrong total shown to the user), different code path; not a `grandTotal()` call, so it isn't fixed by the general rule below on its own and needs a one-off look. |
## Suggested fix
Rather than fixing the 8 broken call sites one at a time (which only fixes today's bugs and leaves the footgun in place for the next caller), remove the possibility of the bug entirely:
**Drop the explicit `total` parameter from the two 8-arg `create(...)` overloads and derive it internally from `paginationInfo.total()`**, the same way the existing 6-arg `PaginatedList`-based overload already does.
```java
// Before
public static PageListResponse create(
@Nullable String query,
PaginatedList.PaginationInfo paginationInfo,
long total,
@Nullable String sort,
@Nullable SortOrder order,
List elements,
List attributes,
EntityDefaults defaults) {
return new AutoValue_PageListResponse<>(query, paginationInfo, total, sort, order, elements, attributes, defaults);
}
// After
public static PageListResponse create(
@Nullable String query,
PaginatedList.PaginationInfo paginationInfo,
@Nullable String sort,
@Nullable SortOrder order,
List elements,
List attributes,
EntityDefaults defaults) {
return new AutoValue_PageListResponse<>(query, paginationInfo, paginationInfo.total(), sort, order, elements, attributes, defaults);
}
```
(Mirror the change for the `String order` overload.)
### Why this is the better fix
- It makes the invalid state unrepresentable: there is no longer any parameter through which a caller can supply a `total` that disagrees with `paginationInfo.total()`.
- Every one of the 16 "correct" call sites already passes `paginationInfo.total()` (or an equivalent) as `total` today — so no legitimate use case is lost by removing the parameter.
- It fixes all 8 `grandTotal()`-based bugs at once, without touching each call site's logic individually.
- The public JSON contract is unaffected — `total` remains a serialized field on `PageListResponse` (via the AutoValue accessor), it's just no longer an input.
### What still needs separate attention
- **All ~20 call sites of the two 8-arg overloads** need their invocations updated to drop the now-removed `total` argument (mechanical, but touches many files).
- **`StreamResource.java:291`** is not a `grandTotal()` bug — it needs its own fix (stop calling `paginatedStreamService.count()` and pass `result.pagination().total()` / switch to the 6-arg overload), independent of the signature change above.
- **`@JsonCreator`-annotated overload**: confirm nothing actually round-trips a `PageListResponse` through JSON deserialization relying on an explicit `"total"` field being fed back into the constructor — if so, verify dropping it as a constructor arg (while keeping it as a serialized accessor) doesn't break that path.
- **`PageListResponseTest.java`**: existing tests should be revisited once the signature changes, since some currently call the 8-arg overload directly with an explicit `total`.
Contributor guide
Research direction
Start in graylog2-server/src/main/java/org/graylog2/rest/models/tools/responses/PageListResponse.java and inspect both 8-arg create overloads and the existing PaginatedList-based overload. Audit the listed create call sites, including StreamResource.java, then revisit PageListResponseTest.java and the @JsonCreator path. Done means filtered totals are used consistently, signatures and callers compile, tests pass, and JSON behavior remains valid.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100