Pkp-lib: API: Issue with Slim router

[jonasraoni]

Describe the bug
AFAICT looks like a static route (api/users/reviewers) defined after a dynamic one (api/users/{userId}) is somehow shadowed by the latter, as long as the pattern matches.

The fix/discussion should probably be handled upstream, but as I don't know if it's an expected behavior (I've never used Slim), I'm leaving it here =]

I think it's better to avoid such collisions, so in our side we could decrease the problems by making the routes stricter whenever possible (e.g. /{userId:[0-9]+}).

What application are you using?
OJS/master

[NateWr]

Cool! I missed that in the Slim docs, but that's a great addition. We should certainly add it to any route with an int parameter. :+1:

Feel free to add the restriction in anywhere you see it (or everywhere if you have the time).

[jonasraoni]

I've linked a PR for pkp-lib (but couldn't link the other for OJS: pkp/ojs#2850) -_-

Also, I've left a comment in the Slim repository, let's see if we can get a fix from upstream =]

[NateWr]

Thanks @jonasraoni, it looks good! Can you add the submodule commit to the OJS pull request? That will ensure the tests are run against the changed code just to catch anything unexpected.

[jonasraoni]

Hi @NateWr, out of curiosity... Are the tests trustful?
Two envs have failed: pgsql/7.2 and mysql/7.3. But that's an odd combination considering that pgsql/7.3 and mysql/7.2 passed o_O

[NateWr]

You'll get occasional test failures like that due to race conditions. I'll
restart them on Monday.

On Sat, 19 Sep 2020, 09:52 Jonas Raoni Soares da Silva, <
[email protected]> wrote:

Hi @NateWr https://github.com/NateWr, out of curiosity... Are the tests
trustful?
Two envs have failed: pgsql/7.2 and mysql/7.3. But that's an odd
combination considering that pgsql/7.3 and mysql/7.2 passed o_O

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/pkp/pkp-lib/issues/6142#issuecomment-695186207, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AARTERJOSUGVHTRU6PRM4XDSGRWLLANCNFSM4PSVEDTA
.

[NateWr]

@jonasraoni I've restarted those tests. In order to make sure that the tests run against the correct repo/branch of pkp-lib, you'll need to create an appropriate submodule commit. You can learn more about that here:

https://docs.pkp.sfu.ca/dev/testing/en/continuous-integration