Skip to content

BUGFIX: Reject suggestions without a context node - #82

Open
rolandschuetz wants to merge 1 commit into
Flowpack:5.3from
rolandschuetz:bugfix-kapsch-backend-t-missing-search-context
Open

BUGFIX: Reject suggestions without a context node#82
rolandschuetz wants to merge 1 commit into
Flowpack:5.3from
rolandschuetz:bugfix-kapsch-backend-t-missing-search-context

Conversation

@rolandschuetz

Copy link
Copy Markdown

Requests to /flowpack/searchplugin with an omitted or empty contextNodeIdentifier currently use the empty string to look up a live node and throw an unhandled exception. Return HTTP 400 with the existing JSON result shape and an explicit input error before looking at the query cache, content repository, or Elasticsearch.

This targets the 5.3 maintenance branch, matching version 5.3.4. Nonempty identifiers keep their existing behavior, including errors for references to nodes that cannot be found.

Sentry: KAPSCH-BACKEND-T.

Validation:

  • Reproduced the exact exception and code 1634467679 in the unmodified 5.3.4 controller and local HTTP endpoint with GET /flowpack/searchplugin?term=test&dimensionCombination=%7B%7D; explicitly empty identifiers behave identically. Before: HTTP 500. After: HTTP 400 with empty completions/suggestions and contextNodeIdentifier must not be empty in errors.
  • Seven focused PHPUnit tests pass (39 assertions), covering omitted/empty identifiers, a nonexistent identifier, and valid requests on cache-hit/cache-miss paths with different site and dimension arguments. The missing-context tests fail against the original source.
  • Valid English/German HTTP requests return identical JSON before and after, using existing local site nodes and temporary sanitized Elasticsearch fixtures. Suggestions belonging to another site are excluded. A nonexistent identifier retains its prior error behavior.
  • Consuming website unit suite passes: 122 tests, 224 assertions. Both changed PHP files pass syntax checks; the new test passes PSR-12 checking.
  • Invalid-input tests assert zero cache reads, content-context creation, and Elasticsearch index access. Valid requests retain their query/cache logic. Repeated local HTTP responses took roughly 53–55 ms for empty input after the fix and 59 ms before; this small sample is not a throughput benchmark. Compilation requests were excluded.

The existing mismatch between template-cache read/write keys is outside this change. Cache hits are exercised explicitly in unit tests; repeated HTTP requests are not claimed to prove actual template-cache hits. Frontend rendering and editor workspaces are unchanged because the fix only rejects an empty identifier at the suggestion endpoint.

Tests were run in DDEV with PHP 8.3/PHPUnit 9.6.34. HTTP verification used an isolated temporary Flow cache because the normal local cache had an unrelated SwiftMailer reflection error. The installed dependency was temporarily linked to the package checkout for integration checks, then restored; temporary local indices were removed. No production data or requests are required for reproduction.

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.

1 participant