Skip to content

Support alternate xAPI request syntax with a servlet filter - #591

Open
thomasturrell wants to merge 1 commit into
mainfrom
feat/alternate-request-filter
Open

thomasturrell wants to merge 1 commit into
mainfrom
feat/alternate-request-filter

Conversation

@thomasturrell

Copy link
Copy Markdown
Contributor

The example LRS previously treated POST requests such as /xapi/statements?method=PUT as ordinary POSTs. Add AlternateRequestSyntaxFilter to translate the intended method, form parameters, supported header fields and UTF-8 content into a servlet request handled by the existing controllers.

Reject invalid transport methods, additional query parameters, malformed URL encoding, repeated content/header fields, missing write content and inconsistent inner content lengths. Order the response-header filter before translation so filter errors retain xAPI response headers. Document the syntax with a curl example.

Validation:

  • Maven reactor build and install passed: 493 tests across the model, starter and example server, including all 50 server tests and 8 new tests. Local execution required loading the Mockito Java agent explicitly through JAVA_TOOL_OPTIONS.
  • ADL suite revision 5bc232d349c60faded8240da698f195106091638, xAPI 1.0.3, alternate request syntax group against the running server: 7 passed, 1 failed.
  • The remaining ADL test submits X-Experience-API-Version=0.8 as a form header and expects HTTP 400. The sample does not yet validate request versions, so it returns 204. Header forwarding is separately covered by the new filter tests; request version validation remains a separate change.
  • Checkstyle and git diff --check passed.

Copilot AI balanced review requested due to automatic review settings October 15, 2026 21:03
@sonarqubecloud

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It introduces a servlet filter that rewrites request methods, headers, and bodies, which is security- and correctness-sensitive and warrants final human review despite only a minor style nit being found.

1 open finding
What changed in this PR

This PR adds xAPI 1.0 "alternate request syntax" support to the sample Learning Record Store. Clients that cannot issue arbitrary HTTP methods can now POST to /xapi/...?method=PUT|GET|... with the real parameters, selected header fields, and UTF‑8 content in a URL‑encoded form body. A new AlternateRequestSyntaxFilter validates the envelope and rewrites it into an AlternateRequestWrapper that the existing controllers handle transparently. The existing XapiResponseHeadersFilter is pinned to run first so that xAPI response headers are retained even when the translation rejects a malformed request.

Changes:

  • Add AlternateRequestSyntaxFilter (envelope validation, query/form decoding, error mapping to 400) and AlternateRequestWrapper (translates method, form headers, parameters, and decoded UTF‑8 content).
  • Order XapiResponseHeadersFilter at HIGHEST_PRECEDENCE ahead of the translation filter so filter-level 400s keep xAPI headers.
  • Add unit tests (filter), an integration test (end‑to‑end through controllers), and README documentation with a curl example.
File Description
AlternateRequestSyntaxFilter.java New servlet filter validating the alternate envelope and delegating to the wrapper or a 400 response.
AlternateRequestWrapper.java New request wrapper exposing the decoded form as headers/parameters/content to controllers.
XapiResponseHeadersFilter.java Adds @Order(HIGHEST_PRECEDENCE) so response headers are set before translation runs.
AlternateRequestSyntaxFilterTest.java Unit tests for translation, empty GET forms, rejection cases, pass-through, and context paths.
AlternateRequestSyntaxIntegrationTest.java End-to-end MockMvc tests covering statements/state flows and header retention on errors.
README.md Documents the alternate syntax with a curl example and header/limitation notes.

🧠 Review effort: Balanced


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

.forEach(name -> headers.put(name, Collections.list(request.getHeaders(name))));
form.forEach(
(name, values) -> {
if (FORM_HEADERS.contains(name.toLowerCase(java.util.Locale.ROOT))) {
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants