Skip to content

Fixes #31214: fluent API for glossary relation types, removal, and graph - #31215

Open
harshach wants to merge 1 commit into
mainfrom
harshach/sdk-fluent-relation-types
Open

Fixes #31214: fluent API for glossary relation types, removal, and graph#31215
harshach wants to merge 1 commit into
mainfrom
harshach/sdk-fluent-relation-types

Conversation

@harshach

@harshach harshach commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Describe your changes:

Fixes #31214

The Java fluent layer could set a glossary relation type but not discover one — GlossaryTerms.find(id).relateTo(x).as("prescribes") existed, while listing the configured types, removing a relation, and reading the relation graph were reachable only by dropping to the service layer (client.settings() / client.glossaryTerms()). I added the missing half because #31070 makes the relation-type read available to non-admins, so the one call those users just gained is the one the fluent layer skipped.

Python needs no equivalent — metadata.sdk already exposes relation reads and writes at the same level as the rest of its API (Settings.glossary_relation_types(), GlossaryTerms.add_relation / remove_relation / relations_graph / relation_type_usage).

Type of change:

  • Improvement

High-level design:

N/A — small change (4 files, no new HTTP surface).

Every addition delegates to service-layer methods that already exist; no new endpoints, request shapes, or error handling. GlossaryRelationTypes is a new fluent class because relation types are a distinct resource from GlossaryTerms and the fluent package is one class per resource (SearchAPI, LineageAPI are the precedent for non-entity fluent classes). Category filtering is client-side, since the settings endpoint returns the whole vocabulary in one document. The identifier-resolution helpers (UUID-or-FQN) that GlossaryTermRelator held privately are now shared statics, so the three builders don't carry three copies.

New surface

GlossaryRelationTypes.list().names();                                  // populate a picker
GlossaryRelationTypes.list().inCategory(RelationCategory.ASSOCIATIVE).fetch();
GlossaryRelationTypes.find("prescribes");                              // Optional<...>
GlossaryRelationTypes.exists("prescribes");
GlossaryRelationTypes.settings();                                      // whole relation config
GlossaryRelationTypes.usage();                                         // per-type counts
GlossaryRelationTypes.define(relationType);                            // admin only

GlossaryTerms.find(hcpId).unrelateFrom(drugFqn).as("prescribes").apply();
GlossaryTerms.find(hcpId).unrelateFrom(drugFqn).apply();               // all relation types
GlossaryTerms.find(hcpId).relations().depth(2).ofTypes("prescribes").fetch();

Tests:

Use cases covered

  • List every configured relation type, or just the names, or only one category
  • Look a relation type up by name before using it in relateTo(...).as(...)
  • Read per-relation-type usage counts and the full relation configuration
  • Register a relation type through the fluent layer (delegates to the idempotent service call)
  • Remove one relation type between two terms, or every relation between them
  • Fetch a term's relation graph with an explicit depth and type filter, and with the defaults
  • Both new builders resolve either a UUID or a fully-qualified name for each endpoint

Unit tests

  • Added openmetadata-sdk/src/test/java/org/openmetadata/sdk/fluent/GlossaryRelationsFluentAPITest.java (11 tests). SystemSettingsService and GlossaryTermService are mocked at the service boundary, so each test asserts the call the fluent builder actually makes — including that an unset .as(...) passes null (remove all types) and that relations() defaults to depth 1 with no type filter.
  • mvn test -pl openmetadata-sdk -Dtest=GlossaryRelationsFluentAPITestTests run: 11, Failures: 0, Errors: 0
  • Full module: mvn test -pl openmetadata-sdkTests run: 244, Failures: 0, Errors: 0 (no regression from the GlossaryTerms/OM edits)

Backend integration tests

  • Not applicable — no server-side change. Every method routes to an existing endpoint already covered by GlossaryTermRelationsIT / GlossaryTermRelationSettingsIT.

Ingestion integration tests

  • Not applicable (no ingestion changes).

Playwright (UI) tests

  • Not applicable (no UI changes).

Manual testing performed

None against a live server — this is a client-side wrapper over endpoints that already have server-side IT coverage, and the mock tests pin the exact URL, method, and arguments each builder produces. To verify by hand against a running instance:

  1. OM.init(client) (registers the new fluent class).
  2. GlossaryRelationTypes.list().names() → the configured relation types; works as a non-admin once Non-admin users cannot fetch full glossary term relation list when adding related terms #31070 is in.
  3. GlossaryTerms.find(a).relateTo(b).as(names.get(0)).apply(), then GlossaryTerms.find(a).relations().fetch() → the new edge appears.
  4. GlossaryTerms.find(a).unrelateFrom(b).apply() → edge gone.

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: not applicable — no schema changes.
  • For UI changes: not applicable — no UI changes.
  • I have added tests and listed them above.
  • I have added tests around the new logic.
  • For connector/ingestion changes: not applicable.

Depends on nothing, but pairs with #31146 (the authorization fix for #31070). Merge order doesn't matter — this compiles and tests green against main as-is.

🤖 Generated with Claude Code

…l/graph

The Java fluent layer could set a relation type but not discover one:
GlossaryTerms.find(id).relateTo(x).as("prescribes") existed, while listing
the configured types, removing a relation, and reading the relation graph
were reachable only by dropping to the service layer. That asymmetry is
sharper now that #31070 makes the relation-type read available to non-admins
— the one call they gained is the one the fluent layer skipped.

Adds GlossaryRelationTypes (list/names/inCategory, find/exists, settings,
usage, and an admin-only define) plus unrelateFrom(...) and relations() on
the GlossaryTerms finder. Identifier resolution shared by all three builders
moves to one place instead of being copied per builder.

Python needs no equivalent: metadata.sdk already exposes relation reads and
writes at the same level as the rest of its API.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 7, 2026 22:52
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

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

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.

@github-actions github-actions Bot added backend safe to test Add this label to run secure Github workflows on PRs labels Aug 7, 2026
Comment on lines +89 to +95
public static Optional<GlossaryTermRelationType> find(String name) {
return list().fetch().stream().filter(type -> matchesName(type, name)).findFirst();
}

public static boolean exists(String name) {
return find(name).isPresent();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Performance: find()/exists() each trigger a fresh HTTP fetch of all relation types

GlossaryRelationTypes.find(name) and exists(name) both call list().fetch(), which issues a full settings().glossaryRelationTypes() HTTP round trip every invocation. The documented pattern (e.g. if (exists(x)) { ... find(x) ... }, or calling find twice) causes several redundant network calls for what is a single lookup. Consider fetching once and reusing the list, or having exists() reuse a cached fetch, since the endpoint returns the whole vocabulary in one document.

Was this helpful? React with 👍 / 👎

@gitar-bot

gitar-bot Bot commented Aug 7, 2026

Copy link
Copy Markdown
Code Review 👍 Approved with suggestions 0 resolved / 1 findings

Adds a Java fluent API for glossary relation types, removal, and graphs with comprehensive test coverage. Consider avoiding fresh HTTP fetches for each find() or exists() call in GlossaryRelationTypes.

💡 Performance: find()/exists() each trigger a fresh HTTP fetch of all relation types

📄 openmetadata-sdk/src/main/java/org/openmetadata/sdk/fluent/GlossaryRelationTypes.java:89-95

GlossaryRelationTypes.find(name) and exists(name) both call list().fetch(), which issues a full settings().glossaryRelationTypes() HTTP round trip every invocation. The documented pattern (e.g. if (exists(x)) { ... find(x) ... }, or calling find twice) causes several redundant network calls for what is a single lookup. Consider fetching once and reusing the list, or having exists() reuse a cached fetch, since the endpoint returns the whole vocabulary in one document.

🤖 Prompt for agents
Code Review: Adds a Java fluent API for glossary relation types, removal, and graphs with comprehensive test coverage. Consider avoiding fresh HTTP fetches for each find() or exists() call in GlossaryRelationTypes.

1. 💡 Performance: find()/exists() each trigger a fresh HTTP fetch of all relation types
   Files: openmetadata-sdk/src/main/java/org/openmetadata/sdk/fluent/GlossaryRelationTypes.java:89-95

   GlossaryRelationTypes.find(name) and exists(name) both call list().fetch(), which issues a full settings().glossaryRelationTypes() HTTP round trip every invocation. The documented pattern (e.g. `if (exists(x)) { ... find(x) ... }`, or calling find twice) causes several redundant network calls for what is a single lookup. Consider fetching once and reusing the list, or having exists() reuse a cached fetch, since the endpoint returns the whole vocabulary in one document.

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 👍 / 👎 | Powered by Gitar — free for open source

@harshach harshach added the To release Will cherry-pick this PR into the release branch label Aug 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the Java SDK fluent layer (org.openmetadata.sdk.fluent) to cover the full glossary term relation surface: discovering configured relation types, removing relations, and fetching a term’s relations graph—without dropping down to the service-layer clients.

Changes:

  • Added GlossaryRelationTypes fluent resource to list/filter/find/define glossary relation types, fetch full settings, and retrieve per-type usage.
  • Extended GlossaryTerms fluent finder with unrelateFrom(...) and relations() builders, and centralized UUID/FQN identifier resolution helpers.
  • Added unit tests validating the new fluent calls delegate correctly to SystemSettingsService / GlossaryTermService, including defaults and “remove all types” behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
openmetadata-sdk/src/main/java/org/openmetadata/sdk/fluent/GlossaryRelationTypes.java New fluent resource for glossary relation type vocabulary (list/filter/find/settings/usage/define).
openmetadata-sdk/src/main/java/org/openmetadata/sdk/fluent/GlossaryTerms.java Adds fluent builders for removing relations and fetching relation graphs; shares identifier resolution.
openmetadata-sdk/src/main/java/org/openmetadata/sdk/OM.java Registers GlossaryRelationTypes in OM.init(client) for standard fluent initialization.
openmetadata-sdk/src/test/java/org/openmetadata/sdk/fluent/GlossaryRelationsFluentAPITest.java Unit coverage for new fluent surfaces and delegation/default semantics.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit 3f00fe39afbbae8ee240ec9c6d064a6f8cca843d in Playwright run 31225313977, attempt 1.

✅ 550 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky

Performance

Blocking targets: ✅ met · Optimization targets: 🟡 in progress

Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting.

🕒 Full workflow signal wall (to summary) 50m 38s

⏱️ Max setup 3m 41s · max shard execution 17m 23s · max shard-job elapsed before upload 22m 1s · reporting 4s

🌐 200.06 requests/attempt · 2.83 app boots/UI scenario · 2.01% common-shard skew

Optimization targets still in progress:

  • Browser traffic was 200.06 requests per attempt (convergence target: fewer than 200).
  • Application boot ratio was 2.83 per UI scenario (1618 boots / 571 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
✅ Shard chromium-01 127 0 0 0 0 0
✅ Shard chromium-02 132 0 0 0 0 0
✅ Shard chromium-03 140 0 0 0 0 0
✅ Shard data-asset-rules-01 61 0 0 0 0 0
✅ Shard domain-isolation-01 14 0 0 0 0 0
✅ Shard global-state-01 34 0 0 0 0 0
✅ Shard ingestion-01 1 0 0 0 0 0
✅ Shard reindex-01 2 0 0 0 0 0
✅ Shard search-01 10 0 0 0 0 0
✅ Shard search-rbac-01 29 0 0 0 0 0

📦 Download artifacts

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

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

Labels

backend safe to test Add this label to run secure Github workflows on PRs To release Will cherry-pick this PR into the release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Java SDK fluent API can set a glossary relation type but cannot list them

2 participants