Skip to content

[ZEPPELIN-6581] Match interpreter search queries literally instead of as regex - #5503

Open
kimyenac wants to merge 1 commit into
apache:masterfrom
kimyenac:ZEPPELIN-6581
Open

kimyenac wants to merge 1 commit into
apache:masterfrom
kimyenac:ZEPPELIN-6581

Conversation

@kimyenac

Copy link
Copy Markdown
Contributor

What is this PR for?

The New UI interpreter page filters settings by name as the user types, but filterInterpreters() compiled the raw query with new RegExp(value, 'i'). So the search box took regular-expression syntax:

  • An incomplete metacharacter such as (, [, * or a trailing \ threw an uncaught SyntaxError: Invalid regular expression. The list was not updated, so it kept showing the results of the previous query.
  • A valid metacharacter changed what matched. . matched every interpreter, and sp.rk matched spark.

This PR filters with a case-insensitive literal substring match (name.toLowerCase().includes(query.toLowerCase())). That keeps the case-insensitive behavior ZEPPELIN-6147 added, which was the only reason the RegExp was introduced. ZEPPELIN-6535 (#5320) fixed the same coercion in the Table visualization column filter. This is a separate code path.

Checked in the browser before and after the change (/#/interpreter, 22 settings, anonymous mode):

Query Before After
( SyntaxError: ... /(/i: Unterminated group, list unchanged 0 items, no error
[, *, \ SyntaxError, list left on the previous query's results 0 items, no error
. all 22 0 items
sp.rk 2 0 items
spark / SPARK 2 2
cleared 22 22

A note on scope. InterpreterSettingManager rejects new setting names outside ^[-_a-zA-Z0-9]+$, and every bundled interpreter group fits that set. So in practice a metacharacter query now matches nothing, where before it threw or matched too much. The test fixtures stay inside that character set, so they do not rely on names the server would reject.

What type of PR is it?

Bug Fix

Todos

  • Match interpreter search queries literally instead of compiling them as a RegExp
  • Add unit tests for InterpreterComponent.filterInterpreters

What is the Jira issue?

ZEPPELIN-6581

How should this be tested?

Done:

  • cd zeppelin-web-angular && npm run test:shell -- src/app/pages/workspace/interpreter/: 8 passed. The new spec covers every acceptance criterion: (, [, *, . and \ without throwing, literal matching, case-insensitive matching, and restoring the full list when the query is cleared.
  • Ran the same spec against the unfixed component. The 6 tests for this bug fail for the reason the issue describes: 4 with SyntaxError: Invalid regular expression, and 2 because . and sp.rk still match as patterns. The 2 tests for existing behavior (case-insensitive matching, clearing) pass both before and after.
  • npm run test:shell: 21 files, 139 tests passed.
  • npm run lint: exit 0. interpreter.component.ts has one member-ordering warning about the constructor's position, and master has the same warning.
  • Manual reproduction from the issue: ran a local Zeppelin server with ng serve and drove /#/interpreter with Playwright, before and after. Results are in the table above. Typing sp(ar[k* one key at a time also produces no console errors. The other console errors and warnings on page load (codicon.ttf load failure, NG8107, NG0912) appear identically on master.

Not done:

  • No Playwright e2e spec was added. The behavior is a pure filter function, so per zeppelin-web-angular/AGENTS.md it belongs in a unit test. The template and debounce wiring are unchanged.

Screenshots (if appropriate)

N/A. The before/after table above records the browser run.

Questions:

  • Does the license files need to update? No. The new spec file has the ASF header.
  • Is there breaking changes for older versions? Only for anyone who relied on regular-expression syntax in the interpreter search box, which was never documented. ZEPPELIN-6147 introduced the RegExp only to make the search case-insensitive.
  • Does this needs documentation? No.

🤖 Generated with Claude Code

… as regex

The New UI interpreter search compiled the raw query with `new RegExp(value, 'i')`.
Incomplete metacharacters such as `(` or `[` threw an uncaught SyntaxError and left
the list showing the previous results, and `.` matched every interpreter.

Filter with a case-insensitive literal substring match instead, keeping the
case-insensitive behavior from ZEPPELIN-6147.
@kimyenac

Copy link
Copy Markdown
Contributor Author

The test-selenium-with-spark-module-for-spark-3-5 failure is unrelated to this change. This PR only touches the New UI interpreter search, and that job drives the classic UI. It is the modal-backdrop interception in AuthenticationIT that has failed on every master push since #5497 and is fixed by #5502.

This branch has not been deployed

No deployments
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