Fixes 29542: make the testDefinitions entityType filter case-insensitive on both the REST and MCP doors

[TeddyCr]

Describe your changes:

Fixes #29542

I worked on the testDefinitions list endpoint because its entityType query parameter was compared with a raw, unnormalized SQL string bind, so ?entityType=Column returned 0 results while ?entityType=COLUMN returned the full set.

Root cause. TestDefinitionResource forwarded the raw parameter into a ListFilter, and CollectionDAO.TestDefinitionDAO compares it with = against the test_definition.entityType generated column in three places — listBefore (:9000-9003), listAfter (:9069-9072) and listCount (:9138-9141).

It is DB-flavour dependent, which is worth knowing when reproducing: PostgreSQL string equality is byte-exact under the deterministic collations it ships with, so it always reproduces; MySQL ships COLLATE=utf8mb4_0900_ai_ci (case-insensitive), so it typically does not. The fix deliberately does not rely on DB collation, so behaviour is now identical on both engines. Both are covered by tests, run on both dialects.

Approach — normalize at the REST boundary, not in SQL. A new TestDefinitionRepository.addEntityTypeFilter(ListFilter, String) trims the value, matches it case-insensitively against the generated TestDefinitionEntityType enum, and stores the canonical .value(); an unrecognised value throws IllegalArgumentException → HTTP 400 via CatalogGenericExceptionMapper:51. This mirrors the existing convention one file over — TestCaseResolutionStatusResource.parseIncidentStatus/parseIncidentGroupBy (:846-885) coerce query params through generated enums with a 400.

Rejected: UPPER(entityType) = UPPER(:entityType) in SQL. It would be six duplicated edits, it would still return a silent empty page for a typo like ?entityType=Banana, and it would preclude an index on the PostgreSQL STORED generated column. Note also that equalsIgnoreCase is locale-independent by specification, so this avoids the Turkish-İ hazard that a toUpperCase-keyed map only mitigates by convention — there is no toUpperCase/toLowerCase anywhere in this change.

The MCP door is fixed by the same code. openmetadata-mcp/.../tools/TestDefinitionsTool.java:95 wrote the same raw parameter into a ListFilter for the same DAO, so an LLM calling list_test_definitions(entityType="Column") got 0 rows silently. Both doors now call the one shared helper, so they cannot drift. tools.json also gains "enum": ["TABLE", "COLUMN"] on that parameter so a model is less likely to emit a bad value at all.

Two claims in the issue report are not supported by the code, and no change was made on their account:

1. "0 results for non-admin users." EntityResource.listInternal authorizes once as a pass/fail gate and then runs the identical ListFilter for every caller; DefaultAuthorizer.authorize only throws or returns and never mutates the query. Confirmed empirically — in the pre-fix run, admin and a role-less non-admin behaved identically, and the failure appeared on the admin client. A regression test now pins that parity.
2. "The frontend sends Column." EntityType.Column is the TypeScript enum member name; its value is "COLUMN" (generated/tests/testDefinition.ts:254-257), unchanged since 1.12.6-release, and playwright/.../TestDefinitionFilters.spec.ts pins the wire values. No UI change is needed.

Type of change:

• Bug fix

High-level design:

N/A — small change (2 production files, ~+40 lines).

Tests:

Use cases covered

• entityType=COLUMN, Column, column and " Column " all return the same non-empty set, and every entry is a COLUMN definition.
• A TABLE definition is never returned by a COLUMN-filtered query.
• entityType=Banana returns 400 with a message naming the rejected value, instead of a silent empty page.
• entityType= (blank) and entityType=%20 are treated as an absent filter → 200 with both TABLE and COLUMN definitions.
• An admin and a role-less non-admin get identical results for the same query.
• The MCP get_test_definitions tool returns COLUMN definitions for entityType="Column".

Unit tests

• Covered by integration tests instead — the defect is in query construction against a real database, and the case-sensitivity only manifests dialect-dependently.

Backend integration tests

• I added integration tests in openmetadata-integration-tests/.
• Files updated: .../it/tests/TestDefinitionResourceIT.java (4 tests), .../it/tests/mcp/McpToolsValidationIT.java (1 test)
• PostgreSQL: TestDefinitionResourceIT + McpToolsValidationIT → 221 run, 0 failures, 0 errors
• MySQL (-DdatabaseType=mysql): 221 run, 0 failures, 0 errors
• Verified RED before GREEN, per fix. Reverting only the canonicalization (PostgreSQL): the casing test and the Banana 400 test both fail. Reverting only the nullOrEmpty guard: only the blank test fails. Reverting only the trim: Invalid entityType ' ' and Invalid entityType ' Column '. Reverting only the MCP one-liner: McpToolsValidationIT.testGetTestDefinitionsForMixedCaseEntityType fails with Expecting actual not to be empty.
• On MySQL the same pre-fix code passes the casing tests — independent confirmation of the collation analysis above, and the reason the fix normalizes explicitly rather than relying on the database.
• Assertions are on response bodies and status codes with zero mocks. Because test_definition is a global collection and the class is @Execution(CONCURRENT), the tests assert membership of self-created FQNs plus allMatch(entityType == COLUMN) plus non-containment of a self-created TABLE definition — never an exact collection size, which would be flaky. assertColumnOnlyListing also asserts paging.total > 0, which exercises TestDefinitionDAO.listCount's own copy of the predicate.

Note: the pre-existing McpToolsValidationIT.testGetTestDefinitionsForColumn asserts only that data is an array, never that it is non-empty — so it would have passed against the broken MCP door. The new test asserts non-emptiness and that every element is COLUMN.

Ingestion integration tests

• Not applicable (no ingestion changes).

Playwright (UI) tests

• Not applicable — no UI change. The existing TestDefinitionFilters.spec.ts already pins the wire values the UI sends.

Manual testing performed

1. ./docker/run_local_docker.sh -m no-ui -d postgresql (PostgreSQL matters — see the collation note above).
2. export TOKEN=<admin jwt> and export BASE=http://localhost:8585/api.
3. Baseline: curl -s -H "Authorization: Bearer $TOKEN" "$BASE/v1/dataQuality/testDefinitions?entityType=COLUMN&limit=1000" | jq '.paging.total' → non-zero.
4. Mixed case: same with entityType=Column → after the fix, the same non-zero count. On main: 0.
5. Lower case: same with entityType=column → same result.
6. ... "?entityType=Column&limit=1000" | jq '[.data[].entityType] | unique' → ["COLUMN"].
7. Bogus value: curl -s -o /dev/stderr -w '%{http_code}\n' ... "?entityType=Banana" → 400, body Invalid entityType 'Banana'. Must be one of [TABLE, COLUMN]. On main: 200 with an empty data array.
8. Blank still accepted: curl -s -o /dev/null -w '%{http_code}\n' ... "?entityType=&limit=1000" → 200, with both TABLE and COLUMN definitions present.
9. Repeat 4–6 with a role-less non-admin JWT → identical results.
10. MCP door: call get_test_definitions with {"entityType": "Column"} → non-empty data, every element "entityType": "COLUMN".

UI screen recording / screenshots:

Not applicable — no UI changes.

Checklist:

• I have read the CONTRIBUTING document.
• My PR title is Fixes <issue-number>: <short explanation>
• My PR is linked to a GitHub issue via Fixes #<issue-number> above.
• I have commented on my code, particularly in hard-to-understand areas.
• For JSON Schema changes: N/A — no schema change, so no migration needed. (tools.json is an MCP tool-descriptor resource, not an entity schema.)
• For UI changes: N/A — no UI changes.
• I have added tests (unit / integration / Playwright as applicable) and listed them above.
• I have added a test that covers the exact scenario we are fixing.

⚠️ Known limitations — please read before closing #29542

This PR fixes a real, reproducible defect on this endpoint, but it may not be the whole of what the reporter hit.

1. The reporter's specific symptom is still unexplained. They reported that granting testDefinition: ViewAll shows "No data" while granting All works. That is a permission-level difference, and it is explained by neither this casing fix (which is privilege-independent — the pre-fix failure reproduces on the admin client) nor the domain defect below (which requires DomainOnlyAccessRole with no domains, not a ViewAll-vs-All distinction). A third mechanism, most likely in PolicyEvaluator/ResourceContext handling of VIEW_ALL versus ALL, is not investigated here. Please confirm with the reporter before treating testDefinitions API: entityType query param is case-sensitive, frontend sends mixed case → 0 results for non-admin users #29542 as fully resolved.

2. A separate, unfixed admin/non-admin asymmetry exists on this exact endpoint and should be filed on its own. EntityUtil.addDomainQueryParam (EntityUtil.java:1093-1103) calls filter.addQueryParam("entityType", entityType) with the resource type, and Filter.addQueryParam is a put (Filter.java:14-17) — so it overwrites the caller's value. For a non-admin, non-bot user holding DomainOnlyAccessRole with no domains, a bare GET /v1/dataQuality/testDefinitions — with no entityType parameter at all — is flipped out of TestDefinitionDAO's all-null fast path into the custom branch, appending AND entityType='testDefinition' → 0 rows, always. DocStoreDAO (CollectionDAO.java:11607/11645/11683) and KnowledgePageDAO (:15997/16049/16105) read the same key and are clobbered identically. Fixing it means renaming the domain filter's parameter key across ListFilter and every caller, which is well outside this bug fix.

3. Behaviour change: ?entityType=<unrecognised> now returns 400 where it previously returned 200 with an empty page. Blank and whitespace-only values are exempt and still behave as an absent filter. One in-repo caller can reach the new 400 — useTestDefinitionData.ts:66 casts a URL query token unchecked, so a hand-edited or bookmarked /test-library?entityType=Tables now shows an error toast instead of an empty table. That loud failure is deliberate; the same path also gains correct behaviour, since ?entityType=column now works where it silently returned nothing.

4. before-cursor (reverse) paging under the entityType filter is not directly asserted; it shares the identical canonical bind value, so it is covered by construction.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR fixes a case-sensitivity bug in the testDefinitions listing flow by canonicalizing the entityType filter at the boundary (shared by both the REST resource and the MCP tool), ensuring consistent behavior across PostgreSQL/MySQL and preventing silent empty results for mixed-case inputs.

Changes:

• Centralized entityType canonicalization in TestDefinitionRepository.addEntityTypeFilter(...) and reused it from both REST and MCP entry points.
• Added REST OpenAPI documentation for a 400 response when entityType is invalid.
• Added integration tests covering case-insensitive filtering, blank handling, invalid values (400), admin vs role-less parity, and MCP mixed-case behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestDefinitionResource.java	Uses shared helper to canonicalize entityType and documents 400 response.
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestDefinitionRepository.java	Adds shared entityType parsing/canonicalization helper with 400-on-invalid behavior.
openmetadata-mcp/src/main/java/org/openmetadata/mcp/tools/TestDefinitionsTool.java	Reuses the shared helper so MCP and REST cannot drift.
openmetadata-mcp/src/main/resources/json/data/mcp/tools.json	Adds an enum constraint for entityType in the MCP tool descriptor.
openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestDefinitionResourceIT.java	Adds IT coverage for REST casing/blank/invalid/admin-vs-nonadmin behaviors.
openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/mcp/McpToolsValidationIT.java	Adds IT coverage for MCP mixed-case entityType returning COLUMN definitions.

[github-actions]

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

• Issue testDefinitions API: entityType query param is case-sensitive, frontend sends mixed case → 0 results for non-admin users #29542 is missing Shipping → Source.
• Issue testDefinitions API: entityType query param is case-sensitive, frontend sends mixed case → 0 results for non-admin users #29542 is missing Shipping → Priority.

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

[gitar-bot]

Code Review ✅ Approved 2 resolved / 2 findings

Makes the testDefinitions entityType filter case-insensitive across both REST and MCP endpoints by normalizing values against the enum. However, the changes to tools.json introduced a syntax error due to a missing comma.

✅ 2 resolved

✅ Quality: enum constraint dropped, weakening model guidance

📄 openmetadata-mcp/src/main/resources/json/data/mcp/tools.json:769-774
This commit replaced the "enum": ["TABLE", "COLUMN"] constraint with a free-form description/default/examples, contrary to the PR description which states tools.json "gains enum". Without the enum the schema no longer constrains the model to valid values, so an LLM can emit an arbitrary entityType that now triggers the new HTTP 400. If the intent was to keep the schema permissive because filtering is case-insensitive, that is reasonable, but consider retaining enum (or documenting why it was removed) to keep the model from producing rejected values.

✅ Bug: Missing comma makes tools.json invalid JSON

📄 openmetadata-mcp/src/main/resources/json/data/mcp/tools.json:773-775
The closing brace of the entityType property on line 774 is not followed by a comma before the testPlatform property on line 775, so the file is no longer valid JSON (verified: parse fails at line 775 with "Expected ',' or '}' after property value"). This breaks loading of all MCP tool descriptors, not just this tool. Add a comma after the closing brace: } → },.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar | Powered by Gitar — free for open source}

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.