Fixes #29510: parse glossary CSV entityStatus case-insensitively#29747
Open
yan-3005 wants to merge 3 commits into
Open
Fixes #29510: parse glossary CSV entityStatus case-insensitively#29747yan-3005 wants to merge 3 commits into
yan-3005 wants to merge 3 commits into
Conversation
Glossary and metric CSV imports rejected valid status values like `draft` or `APPROVED` because `EntityStatus.fromValue` matches the enum's canonical casing (`Draft`, `Approved`, ...). Users typing lower/upper/mixed case saw the import fail with an unhelpful "status is invalid" error. Add `EntityFieldUtils.parseEntityStatus` that first tries the exact enum lookup and falls back to a case-insensitive scan over `EntityStatus.values()`. Route the glossary and metric CSV importers, plus the shared `setEntityStatus` helper, through it so any casing (`draft`, `DRAFT`, `drAFT`, `in review`, ...) resolves to the canonical enum. Truly unknown values still throw and surface as a clear per-row validation failure. Tests: - EntityFieldUtilsTest: parse helper accepts canonical, lower, upper, and mixed casings for every EntityStatus, and throws on unknown values. - GlossaryResourceIT: imports terms with `draft`, `APPROVED`, `deprecated`, `drAFT` and asserts the import succeeds and terms are created; a second test asserts an invalid value still fails with a message referencing the bad input.
yan-3005
added
the
safe to test
Contributor
✅ PR checks passedThe linked issue has a description and all required Shipping project fields set. Thanks! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes CSV import failures for glossary terms and metrics by parsing EntityStatus case-insensitively, addressing issue #29510 where valid statuses like draft or APPROVED were rejected due to enum casing.
Changes:
- Added
EntityFieldUtils.parseEntityStatus(String)to parseEntityStatuswith a case-insensitive fallback. - Updated Glossary and Metric CSV import paths to use the new parsing helper.
- Added unit and integration tests to cover mixed/incorrect casing inputs and invalid-value failures.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| openmetadata-service/src/main/java/org/openmetadata/service/util/EntityFieldUtils.java | Adds a shared case-insensitive EntityStatus parsing helper and routes setEntityStatus through it. |
| openmetadata-service/src/test/java/org/openmetadata/service/util/EntityFieldUtilsTest.java | Adds unit tests validating mixed-case parsing and unknown-value behavior. |
| openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/MetricRepository.java | Uses the shared parsing helper for metric CSV entityStatus field parsing. |
| openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/GlossaryRepository.java | Uses the shared parsing helper for glossary term CSV glossaryStatus parsing. |
| openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/GlossaryResourceIT.java | Adds integration coverage for case-insensitive status import and invalid-value failure behavior. |
…ow parsing
- parseEntityStatus: single case-insensitive scan over EntityStatus.values()
after trimming, no more try/catch around EntityStatus.fromValue. Rejects
null/blank/unknown with a direct IllegalArgumentException.
- Unit tests: cover leading/trailing whitespace (" draft ", "Approved\t",
"\n In Review \n") plus a blank-string throw case.
- GlossaryResourceIT#test_importCsv_glossaryStatusIsCaseInsensitive: assert
each expected casing ("draft", "APPROVED", "deprecated", "drAFT") appears
on its own success row in importResult.getImportResultsCsv(). Guards
against a regression where the importer silently defaults every row to
the same status.
Contributor
🟡 Playwright Results — all passed (14 flaky)✅ 4053 passed · ❌ 0 failed · 🟡 14 flaky · ⏭️ 21 skipped
🟡 14 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
Drop the untyped Map<termName, casing> pair in test_importCsv_glossaryStatusIsCaseInsensitive in favour of a small private StatusCase record (termName, displayName, description, csvCasing). The CSV rows, per-row success assertions, and post-import lookups now iterate the same typed list, making the test's intent explicit and removing the Map.of import.
Code Review ✅ ApprovedEnables case-insensitive parsing for glossary and metric CSV status imports via a new shared utility, ensuring robust validation. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #29510.
Glossary and metric CSV imports rejected valid
glossaryStatusvalues whose casing didn't match the JSON-schema enum (Draft,Approved,Deprecated, ...). Users typingdraft,APPROVED, ordrAFTsaw the import fail withGlossary term status <value> is invalid.EntityFieldUtils.parseEntityStatus(String)— first triesEntityStatus.fromValue, then falls back to a case-insensitive scan ofEntityStatus.values(). Multi-word values (in review,IN REVIEW) match too.GlossaryRepository#getTermStatusandMetricRepository#getEntityStatus(both CSV import paths) plus the sharedEntityFieldUtils#setEntityStatusthrough the helper.Field 11 error - Glossary term status <value> is invalidfailure row — no silent acceptance.Test plan
EntityFieldUtilsTest— 15 tests pass. New cases coverDraft/draft/DRAFT/drAFT/DrAfT,approved/APPROVED/ApPrOvEd,Deprecated/deprecated/DEPRECATED,in review/IN REVIEW/In Review,rejected,ARCHIVED, and an unknown-value throws case.GlossaryResourceIT#test_importCsv_glossaryStatusIsCaseInsensitive— imports terms withdraft/APPROVED/deprecated/drAFT; assertsApiStatus.SUCCESS, 4 rows passed, 0 failed, and terms are created with a non-nullentityStatus.GlossaryResourceIT#test_importCsv_glossaryStatusInvalidValueFails— importsnot-a-status; asserts at least one failed row and the message referencesnot-a-status+is invalid.Greptile Summary
This PR makes glossary and metric CSV status imports accept valid status values regardless of case. The main changes are:
EntityStatusparser with trimming and case-insensitive matching.Confidence Score: 5/5
This looks safe to merge.
Important Files Changed
EntityStatusand uses it before the legacy status fallback.Reviews (3): Last reviewed commit: "test(glossary): replace ad-hoc Map with ..." | Re-trigger Greptile
Context used (3)