[thomasturrell]

Statement requests currently accept unknown or incorrectly capitalised parameters, and invalid single-statement/filter combinations can fall through to the general list handler and return 200. Reject these requests with 400 before calling the service.

• Keep the controller focused on endpoint handlers. Validate parameters in a dedicated Spring MVC HandlerInterceptor, registered for /xapi/statements through WebMvcConfigurer. No @ModelAttribute validation hook is used.
• Validate GET/HEAD names and permit only format and attachments alongside statementId or voidedStatementId; reject combining the two identifiers.
• Restrict PUT parameters to statementId and statement POST requests to no query parameters.
• Correct the GET routing exclusions. Valid voidedStatementId requests explicitly return 501 until voiding support is implemented, rather than returning an unrelated statement list.
• Add parameterised MVC regression tests for the 35 failing name/combination cases, both identifiers together, permitted representation/list parameters, HEAD and write rejection. Write rejection tests use valid bodies and verify no service interactions.
• Document the scope and remaining filtering/formatting limitations.

Reviewed xAPI 1.0.3 Communication requirements 2.1.3.s2.b1–b2 and 3.2.s3.b7–b8.

Validation on revision 494c5b5:

• Java 25 CI passed the full Maven build/tests, Checkstyle, documentation checks and Sonar scan: https://github.com/BerryCloud/xapi-java/actions/runs/38799944694
• All 48 statement controller tests and all 85 sample-server tests passed, without failures/errors/skips.
• SonarCloud Code Analysis passed with zero new issues.
• Conformance: 1,198 passed, 162 failed, 5 not run, compared with main's 1,165/195/5: https://github.com/BerryCloud/xapi-java/actions/runs/38799944854
• Detailed report comparison: 36 failures now pass (the 35 targeted cases plus an invalid-GET consistency-header test). Three previous passes now fail because valid voidedStatementId requests correctly reach an explicit 501 instead of an unrelated 200 statement list. These tests only checked the status and masked the existing missing voiding functionality. Net improvement: 33 passes.
• Changed Java files formatted with Google Java Format; git diff --check passed.

Local clean verify was blocked by Maven Central name resolution and the available Java 17 runtime; executable validation was completed in Java 25 CI.

[Copilot]

🔵 Needs a closer look

The change alters HTTP request routing and introduces validation that depends on subtle Spring MVC @ModelAttribute/handler-matching ordering, which I verified by reasoning but could not execute, so a human should confirm before approval.

0 open findings

What changed in this PR

This PR hardens query-parameter handling for the sample xAPI Learning Record Store's statements endpoint. Previously, unknown or incorrectly capitalised parameters and invalid single-statement/filter combinations could fall through to the general list handler and incorrectly return 200. The change adds an @ModelAttribute pre-handler validator that rejects such requests with 400 before the service is invoked, corrects the @GetMapping routing exclusions, and introduces a dedicated voidedStatementId handler returning 501 instead of an unrelated statement list.

Changes:

• Added validateParameters to allow-list parameter names per HTTP method and reject combining statementId/voidedStatementId with anything other than format/attachments (and reject combining the two identifiers).
• Fixed the GET routing conditions (getStatement, getStatementsSince, getStatements) and added a getVoidedStatement handler that explicitly returns 501.
• Added parameterised MVC regression tests (35 name/combination cases plus representation/list/HEAD/write cases) asserting no service interactions on rejection, and documented scope/limitations in the README.

File	Description
samples/​xapi-server/​src/​main/​java/​dev/​learning/​xapi/​samples/​xapiserver/​StatementController.java	Adds method-aware parameter validation, a voidedStatementId 501 handler, and corrected GET routing conditions.
samples/​xapi-server/​src/​test/​java/​dev/​learning/​xapi/​samples/​xapiserver/​StatementsControllerTest.java	Adds parameterised and single-case regression tests for name/combination rejection and permitted parameters.
samples/​xapi-server/​README.md	Documents the parameter validation rules and remaining filtering/formatting limitations.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud