matrei commented on PR #16323:
URL: https://github.com/apache/grails-core/pull/16323#issuecomment-5630002373
# Round 2
Thanks, this round addresses everything from the first pass:
- The JSON API path now reads the total as a `Number` and widens with
`longValue()`, and the new `IterableRenderSpec` case pins the 64-bit `last`
offset.
- `PaginationSpec` declares `Long` results and has rows past
`Integer.MAX_VALUE` for all three offset helpers.
- `offset` is `Long` on every `paginate()` overload and is read back with
`Parameters.long()`, so the request path no longer caps at 2^31.
- The upgrade note now covers `.gson` `model {}` declarations and the
`HalViewHelper` signature change, and the section renumbered cleanly to 60
after the 8.0.x merge.
Locally: `grails-views-gson` (184 tests) and the `views-functional-tests`
integration suite (52 tests, including `ProductSpec` which drives the real
`index.gson` through `hal.paginate`) both pass.
Three documentation nits left, nothing in code.
### 1. Editing leftover in the upgrade note
```
`GormStaticApi.count()` narrowed the datastore result with `intValue()`,
which truncates silently. That second step truncates silently, so
a table with more than `Integer.MAX_VALUE` rows ...
```
"truncates silently" appears twice, and "That second step" refers to a step
the rewritten sentence no longer describes. Suggest:
```
`GormStaticApi.count()` narrowed the datastore result with `intValue()`,
which truncates silently, so
a table with more than `Integer.MAX_VALUE` rows ...
```
### 2. Comment wording in the `.gson` example
`Long bookCount // or Number, to accept either controller` reads oddly.
Perhaps `// or Number, to accept either width`.
### 3. The HAL guide still shows the old model type, and its argument order
is wrong
`grails-doc/src/en/guide/theWebLayer/gson/gsonHal.adoc` (around line 255) is
the user-facing reference for `hal.paginate()` and this PR changes that
signature, so it should move with it:
```groovy
model {
Iterable<Book> bookList
Integer bookCount // now contradicts the upgrade note
...
}
json {
hal.paginate(Book, bookCount, max, offset, sort, order)
}
```
Two things: `Integer bookCount` should become `Long`, and the call passes
`max, offset` while the signature is `(object, total, offset, max, sort,
order)`. The swap predates this PR, but it still compiles today only because
`Integer` widens into the new `Long offset`, and it produces links with `max`
and `offset` exchanged. Since the guide is being touched for the type anyway,
fixing the order in the same commit avoids shipping an example that silently
paginates wrong.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
