Skip to content

Topic field fix: Added "messageSchema"#26722

Closed
Khairajani wants to merge 3 commits intomainfrom
topic_field_fix
Closed

Topic field fix: Added "messageSchema"#26722
Khairajani wants to merge 3 commits intomainfrom
topic_field_fix

Conversation

@Khairajani
Copy link
Copy Markdown
Contributor

@Khairajani Khairajani commented Mar 24, 2026

This pull request updates the logic for generating schema field names for topics to include the messageSchema prefix, improving clarity and consistency in fully qualified names.

Schema field naming improvements:

  • Updated EntityUtil.getSchemaField(Topic, Field, String) to prepend "messageSchema" to the schema field's fully qualified name, changing the format from "schemaFields".fieldName.fieldName to "messageSchema"."schemaFields".fieldName.fieldName. This clarifies that the schema fields belong to the message schema context.

Summary by Gitar

  • Fixed schema field naming:
    • Updated getSchemaField() method for Topic to include messageSchema prefix in FQN path construction

This will update automatically on new commits.

@github-actions
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!

@Khairajani Khairajani added the safe to test Add this label to run secure Github workflows on PRs label Mar 24, 2026
@gitar-bot
Copy link
Copy Markdown

gitar-bot bot commented Mar 24, 2026

Code Review ✅ Approved 2 resolved / 2 findings

Adds "messageSchema" prefix support to the topic field with corresponding API endpoint and unit test updates, resolving missing prefix bugs in schema field handling.

✅ 2 resolved
Bug: Tests not updated for new "messageSchema" prefix

📄 openmetadata-service/src/main/java/org/openmetadata/service/util/EntityUtil.java:703-705
The test assertions in EntityUtilTest.java still expect the old format without the "messageSchema" prefix (e.g., "schemaFields."user.id".description"). These tests will fail with the new code that now returns "messageSchema"."schemaFields"."user.id".description". The tests need to be updated to match the new field path format.

Bug: APIEndpoint's getSchemaField likely has the same missing prefix bug

📄 openmetadata-service/src/main/java/org/openmetadata/service/util/EntityUtil.java:708
The getSchemaField(APIEndpoint, ...) method at line 708 still uses FullyQualifiedName.build("schemaFields", ...) without a parent schema prefix. The APIEndpoint entity has requestSchema and responseSchema wrapper objects (per apiEndpoint.json schema), similar to how Topic has messageSchema. If the Topic fix is correct, the APIEndpoint variant likely needs the same treatment. However, since APIEndpoint has two possible parent wrappers (request vs response), the fix may require passing the schema type as a parameter.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@github-actions
Copy link
Copy Markdown
Contributor

OpenMetadata Service New-Code Coverage

PASS. Required changed-line coverage: 90.00% overall and per touched production file.

  • Overall executable changed lines: 9/9 covered (100.00%)
  • Missed executable changed lines: 0
  • Non-executable changed lines ignored by JaCoCo: 3
  • Changed production files: 1
File Covered Missed Executable Non-exec Coverage Uncovered lines
openmetadata-service/src/main/java/org/openmetadata/service/util/EntityUtil.java 9 0 9 3 100.00% -

Only changed executable lines under openmetadata-service/src/main/java are counted. Test files, comments, imports, and non-executable lines are excluded.

@sonarqubecloud
Copy link
Copy Markdown

yan-3005
yan-3005 previously approved these changes Mar 24, 2026
@yan-3005 yan-3005 dismissed their stale review March 24, 2026 14:33

Need to review further

@github-actions
Copy link
Copy Markdown
Contributor

🔴 Playwright Results — 1 failure(s), 25 flaky

✅ 3406 passed · ❌ 1 failed · 🟡 25 flaky · ⏭️ 208 skipped

Shard Passed Failed Flaky Skipped
🟡 Shard 1 452 0 3 2
🔴 Shard 2 597 1 7 32
🟡 Shard 3 610 0 6 27
🟡 Shard 4 599 0 4 47
✅ Shard 5 587 0 0 67
🟡 Shard 6 561 0 5 33

Genuine Failures (failed on all attempts)

Features/BulkEditEntity.spec.ts › Glossary Term (Nested) (shard 2)
Error: �[2mexpect(�[22m�[31mlocator�[39m�[2m).�[22mtoContainText�[2m(�[22m�[32mexpected�[39m�[2m)�[22m failed

Locator: locator('.rdg-cell[aria-selected="true"][aria-readonly="true"]')
Expected substring: �[32m"PW.e1d648b0%Panther7c1823e7"�[39m
Timeout: 15000ms
Error: element(s) not found

Call log:
�[2m  - Expect "toContainText" with timeout 15000ms�[22m
�[2m  - waiting for locator('.rdg-cell[aria-selected="true"][aria-readonly="true"]')�[22m

🟡 25 flaky test(s) (passed on retry)
  • Features/DataAssetRulesDisabled.spec.ts › Database Schema (shard 1, 1 retry)
  • Pages/AuditLogs.spec.ts › should apply both User and EntityType filters simultaneously (shard 1, 1 retry)
  • Pages/UserCreationWithPersona.spec.ts › Create user with persona and verify on profile (shard 1, 1 retry)
  • Features/BulkEditEntity.spec.ts › Glossary (shard 2, 1 retry)
  • Features/BulkImport.spec.ts › Keyboard Delete selection (shard 2, 1 retry)
  • Features/DataQuality/TestCaseImportExportE2eFlow.spec.ts › Admin: Complete export-import-validate flow (shard 2, 1 retry)
  • Features/DataQuality/TestCaseImportExportE2eFlow.spec.ts › EditAll User: Complete export-import-validate flow (shard 2, 1 retry)
  • Features/DataQuality/TestCaseIncidentPermissions.spec.ts › User with TEST_CASE.EDIT_ALL can see edit icon on incidents (shard 2, 1 retry)
  • Features/DataQuality/TestCaseIncidentPermissions.spec.ts › User with only VIEW cannot PATCH incidents (shard 2, 1 retry)
  • Features/DataQuality/TestCaseResultPermissions.spec.ts › User with TEST_CASE.EDIT_ALL can see edit action on test case (shard 2, 1 retry)
  • Features/IncidentManager.spec.ts › Resolving incident & re-run pipeline (shard 3, 1 retry)
  • Features/Permissions/GlossaryPermissions.spec.ts › Team-based permissions work correctly (shard 3, 1 retry)
  • Features/TestSuiteMultiPipeline.spec.ts › TestSuite multi pipeline support (shard 3, 1 retry)
  • Flow/CustomizeWidgets.spec.ts › Following Assets Widget (shard 3, 1 retry)
  • Flow/ExploreDiscovery.spec.ts › Should display deleted assets when showDeleted is checked and deleted is not present in queryFilter (shard 3, 1 retry)
  • Flow/ExploreDiscovery.spec.ts › Should not display soft deleted assets in search suggestions (shard 3, 1 retry)
  • Pages/Customproperties-part2.spec.ts › entityReferenceList shows item count, scrollable list, no expand toggle (shard 4, 1 retry)
  • Pages/Domains.spec.ts › Rename domain with tags and glossary terms preserves associations (shard 4, 1 retry)
  • Pages/Domains.spec.ts › Rename domain with owners and experts preserves assignments (shard 4, 1 retry)
  • Pages/Entity.spec.ts › User as Owner Add, Update and Remove (shard 4, 1 retry)
  • Pages/Users.spec.ts › Create and Delete user (shard 6, 1 retry)
  • Pages/Users.spec.ts › Permissions for table details page for Data Consumer (shard 6, 1 retry)
  • Pages/Users.spec.ts › Check permissions for Data Steward (shard 6, 1 retry)
  • VersionPages/EntityVersionPages.spec.ts › Directory (shard 6, 1 retry)
  • VersionPages/TestCaseVersionPage.spec.ts › should show the test case version page (shard 6, 1 retry)

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

@SumanMaharana SumanMaharana self-requested a review March 24, 2026 14:58
@Khairajani
Copy link
Copy Markdown
Contributor Author

Closing this PR as this fix requires running migrations across the db

@Khairajani Khairajani closed this Mar 24, 2026
@Khairajani
Copy link
Copy Markdown
Contributor Author

Possibly right Quick fix: #26735

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants