Skip to content

Fixes #29542#29680

Draft
Al-Zaytoun wants to merge 4 commits into
open-metadata:mainfrom
Al-Zaytoun:fix/29542-case-insensitive-entitytype
Draft

Fixes #29542#29680
Al-Zaytoun wants to merge 4 commits into
open-metadata:mainfrom
Al-Zaytoun:fix/29542-case-insensitive-entitytype

Conversation

@Al-Zaytoun

@Al-Zaytoun Al-Zaytoun commented Jul 1, 2026

Copy link
Copy Markdown

Describe your changes:

Fixes #29542

I worked on ... because ...

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

High-level design:

N/A — small change.

Tests:

Use cases covered

Unit tests

Backend integration tests

Ingestion integration tests

Playwright (UI) tests

Manual testing performed

UI screen recording / screenshots:

Not applicable.

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: I updated the migration scripts or explained why it is not needed.
  • For UI changes: I attached a screen recording and/or screenshots above.
  • I have added tests (unit / integration / Playwright as applicable) and listed them above.

Summary by Gitar

  • Bug fixes:
    • Ensure entityType query parameter is handled case-insensitively in TestDefinitionResource by forcing it to uppercase using Locale.ROOT.
  • Test additions:
    • Added comprehensive integration test list_testDefinitionsByEntityType_caseInsensitive in TestDefinitionResourceIT to verify that mixed-case and lowercase entityType inputs correctly retrieve the expected data.

This will update automatically on new commits.

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@gitar-bot

gitar-bot Bot commented Jul 1, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 2 resolved / 2 findings

Enforces case-insensitive entityType handling using Locale.ROOT and adds a regression test to verify consistent behavior, addressing potential locale-dependent bugs and missing test coverage.

✅ 2 resolved
Bug: toUpperCase() without Locale.ROOT can break in some locales

📄 openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestDefinitionResource.java:167
entityType.toUpperCase() uses the default JVM locale. In the Turkish locale (tr-TR), lowercase i uppercases to dotted İ (U+0130), not I. An input such as column would become COLUMN correctly, but any entity type containing an i (or if the enum set later includes such values) could uppercase to a value that does not match the stored uppercase enum (TABLE, COLUMN), silently breaking the case-insensitive filter on servers running in those locales. Since the whole point of this change is normalization, it should be locale-independent. Use entityType.toUpperCase(Locale.ROOT) — this also matches the project's Java guideline requiring Locale.ROOT with case conversions.

Quality: Regression test is an empty placeholder with unresolved TODO

📄 openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestDefinitionResourceIT.java:259-265
The new list_testDefinitionsByEntityType_caseInsensitive test contains only a // #TODO comment and no assertions, so it passes without exercising the fix. Per the testing guidelines, bug fixes must include a regression test that actually covers the fixed scenario, and TODOs require a ticket reference. Before this PR leaves draft, implement the test: create a TestDefinition and assert that listing with a mixed/lower-case entityType query param (e.g. table, Column) returns the definition, proving the case-insensitive normalization works end-to-end.

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

1 similar comment
@gitar-bot

gitar-bot Bot commented Jul 1, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 2 resolved / 2 findings

Enforces case-insensitive entityType handling using Locale.ROOT and adds a regression test to verify consistent behavior, addressing potential locale-dependent bugs and missing test coverage.

✅ 2 resolved
Bug: toUpperCase() without Locale.ROOT can break in some locales

📄 openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestDefinitionResource.java:167
entityType.toUpperCase() uses the default JVM locale. In the Turkish locale (tr-TR), lowercase i uppercases to dotted İ (U+0130), not I. An input such as column would become COLUMN correctly, but any entity type containing an i (or if the enum set later includes such values) could uppercase to a value that does not match the stored uppercase enum (TABLE, COLUMN), silently breaking the case-insensitive filter on servers running in those locales. Since the whole point of this change is normalization, it should be locale-independent. Use entityType.toUpperCase(Locale.ROOT) — this also matches the project's Java guideline requiring Locale.ROOT with case conversions.

Quality: Regression test is an empty placeholder with unresolved TODO

📄 openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestDefinitionResourceIT.java:259-265
The new list_testDefinitionsByEntityType_caseInsensitive test contains only a // #TODO comment and no assertions, so it passes without exercising the fix. Per the testing guidelines, bug fixes must include a regression test that actually covers the fixed scenario, and TODOs require a ticket reference. Before this PR leaves draft, implement the test: create a TestDefinition and assert that listing with a mixed/lower-case entityType query param (e.g. table, Column) returns the definition, proving the case-insensitive normalization works end-to-end.

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

@Al-Zaytoun

Copy link
Copy Markdown
Author

Could a maintainer please add the safe to test label so CI can run.

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.

testDefinitions API: entityType query param is case-sensitive, frontend sends mixed case → 0 results for non-admin users

1 participant