Answer 400 for a non-integer limit on @users

[kunalKumar-13]

What

@users parses its limit with a bare int(), so a value that is not an integer raises ValueError and the request becomes a 500.

Run against a real Plone with the project's own test layer:

limit='10'    -> HTTP 200
limit=''      -> HTTP 200
limit='-1'    -> HTTP 200
limit='abc'   -> HTTP 500
limit='1.5'   -> HTTP 500

Why this is the odd one out

plone.restapi already has the right helper for this, and every other service uses it:

def parse_int(data, prop, default):
    try:
        return int(data.get(prop, default))
    except (ValueError, TypeError):
        raise BadRequest(f"Invalid {prop}: Not an integer")

batching.py (b_start, b_size), navigation/get.py (expand.navigation.depth), discussion/conversation.py (comment_id) and querystringsearch/get.py all go through it — and querystringsearch reads a limit of its own that way:

limit = parse_int(data, "limit", None)

So the same bad value is a 400 on @querystring-search and a 500 on @users. This makes @users behave like the rest.

The change

services/users/get.py now routes limit through parse_int. parse_qs returns a list per key, so the value is unwrapped before parse_int sees it — that is the only wrinkle, and there is a comment saying so.

Nothing else moves. limit=1, an empty limit, a missing limit and a negative limit all behave exactly as before.

Tests

Written first and run before the fix, where they failed with AssertionError: 400 != 500 for each of abc, 1.5 and 10,20.

• test_list_users_with_non_integer_limit — asserts 400 and that the message names the parameter
• test_list_users_with_valid_limit — pins 1, `` and -1 at 200, so a later change cannot quietly turn a working value into an error

zope-testrunner -m plone.restapi.tests.test_services_users — 95 tests, 0 failures, 0 errors.

The repository's own pre-commit hooks pass on commit: pyupgrade, isort, black, flake8, codespell, pyroma, check-python-versions.

[mister-roboto]

@kunalKumar-13 thanks for creating this Pull Request and helping to improve Plone!

TL;DR: Finish pushing changes, pass all other checks, then paste a comment:

@jenkins-plone-org please run jobs

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically.

Happy hacking!

[kunalKumar-13]

@jenkins-plone-org please run jobs

[kunalKumar-13]

Rebased onto current main — it was one commit behind, and nothing upstream had touched services/users/get.py or tests/test_services_users.py, so the change itself is unaltered.

@jenkins-plone-org please run jobs

[kunalKumar-13]

Jenkins has reported since the rebase, and two of the six jobs are red. They are not from this change — the acceptance layer is timing out on browser startup.

In the 6.2-3.10 run (build 838): 9,778 tests, 4 failures. All four are in Robot/Playwright suites:

Robot.Test Add Translation                                    | FAIL |
TimeoutError: page.goto: Timeout 10000ms exceeded.
Call log:
  - navigating to "http://localhost:35605/plone/ca/a-catalan-document", waiting until "load"
...
Error: Tried to take screenshot, but no page was open.

That is plone.app.multilingual's "As an editor I can add new translation". This PR changes @users' limit parsing in plone.restapi.

plone.restapi's own suites are clean — nine of them ran in that build, all zero failures, including the 2,030-test integration layer and the 33-test blocks layer.

It is not specific to this PR, or to me. Across the currently open pull requests here:

PR	author	Jenkins
#2024	erral	4 pass
#2045	me	4 pass
#2046	me	2 fail / 2 pass
#2021	sneridagh	4 fail

Same pattern on the other three I nudged today — zodbverify#18's log has "Tried to take screenshot, but no page was open" 298 times, with Test Add Translation and Test Schemaeditor failing. So it lands on core developers' branches too, and passes on some runs and not others.

One thing I should own: these four jobs had been sitting pending since 5–8 September, and I asked Jenkins to run them today. That turned "no signal" into "some red", which looks worse at a glance. The red is the browser layer rather than any of the changes, and I would rather it be visible and diagnosable than silently queued — but it is the direct consequence of my nudge, not a pre-existing state.

Nothing to do on this PR as far as I can tell. Happy to re-trigger if a fresh run would help.

[kunalKumar-13]

The two red Jenkins jobs are not this PR, and — unlike the browser-install problem I
reported for NodeKombinat in plone/plone.jenkins_node#21 — these ran on NodePy763
and failed a different way.

#147 has three robot
failures, all page timeouts rather than assertions:

AssertionError: TimeoutError: page.goto: Timeout 10000ms exceeded.
  - navigating to "http://localhost:39465/plone/doc/edit", waiting until "load"
Also teardown failed:
TimeoutError: page.screenshot: Timeout 10000ms exceeded.

in test_contentbrowser.robot (add/upload an image) and test_translate_content.robot.
The run also took 26m35s against the ~11m the same suite takes on a good build, so this
reads as the node being slow enough to blow a 10s navigation budget rather than anything
in the diff. This PR only changes how a non-integer limit is answered on @users.

[davisagli]

@jenkins-plone-org please run jobs

[davisagli]

Thanks for this fix.