Fixes #34689: record every Assets tab edit on the asset as its own PATCH

[manerow]

Fixes #34689

Problem

Adding or removing a tag, glossary term, domain or data product from an Assets tab changed the assets but recorded nothing on them: no new version, no "updated by", no change event for the activity feed, alerts or audit log. The tab endpoints wrote relationship rows directly instead of saving the asset.

PR #34690 did this through a second save pipeline. This PR starts over so an entity has one way to be saved.

What this PR changes

An Assets tab edit is now exactly the PATCH the user could make on each asset's page. EntityPatchBatch takes the selected assets in groups of 100 and loads each group in one query. For each asset it applies the edit to a copy, turns the difference into a JSON patch, authorizes the asset as that PATCH, and saves it through the EntityRepository.patch code a single PATCH runs, with the same rules and versioning. A selected column is saved through its table. An asset an approval workflow gates is held as that PATCH would be, and reported as pending with its change request. The tag tab's background job keeps the request's impersonation, so isImpersonated() rules apply there too.

Two related fixes:

• A PATCH now sends the entity to search after its inherited fields are filled, so removing an asset's own domain shows the inherited one in search at once.
• A table's classification tags and data products are no longer copied onto its column search entries. Its glossary terms still reach them as inherited terms, and test cases still inherit table tags.

The principles are recorded in docs/decisions/.

API change

The tag and glossary term assets/add and assets/remove endpoints no longer answer 403 for the whole request. An asset the caller may not edit comes back as a failed row with the reason, in a 200 whose status is failure or partialSuccess (for tags, in the job's outcome). Clients must read status, not only the HTTP code. Domain and data product endpoints still answer 403 to a caller who may not edit that domain or data product.

What users will notice

• Every Assets tab edit creates a version, sets "updated by", and shows up in the activity feed and alerts.
• An asset the user may not edit is reported with its reason, and the rest goes through.
• Removing a label from a schema or service keeps it on assets inside that carry it themselves.

How it was tested

New AssetsTabVersioningIT (versions, events, columns, partial failure, 251 assets, parity with the PATCH), AssetsTabAuthorizationIT (owner-only and deny rules, a domain editor, an impersonating bot) and AssetsTabSearchEntriesIT, plus Jest tests for refused removals. With the related suites, approval holds included, 1,218 tests pass on MySQL; the most affected also on Postgres.

Median of 5 rounds: a glossary term on 100 tables takes 2.7 s (3.4 s on main).

Not in this PR

Sending each group's search writes as one bulk request (#35023).

Confidence Score: 4/5

The cache must have an explicit size cap before merging to satisfy the repository’s requirement.

Findings

1.  Job outcomes grow without bounds ▶

Summary

Assets tab edits now use each asset’s normal PATCH path.

• Assets tab edits save each selected asset through its normal PATCH path.
• Bulk edits report each asset the caller cannot edit.
• Search entries now reflect inherited fields without copying every table label to columns.

Acknowledged limitations:

• Concurrent overwrites without a version check are explicitly documented as a known limitation.
• manerow explicitly deferred search conflict retries and grouped search writes to separate work.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Assets tab edit] --> B[Resolve selected assets and columns]
  B --> C[Group by saved entity]
  C --> D[Load up to 100 entities]
  D --> E[Apply edit to a copy and build PATCH]
  E --> F[Check permission for each asset]
  F -->|Refused| G[Failed row]
  F -->|Allowed| H[Normal entity PATCH]
  H -->|Approval required| I[Pending change request]
  H -->|Saved| J[Version, search, and change event]
  G --> K[Bulk result]
  I --> K
  J --> K

Loading

_{Reviews (1) · Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." · Reviewed by Greptile}

[sonika-shah]

Since each asset is now authorized inside the background job, impersonation-based policy conditions stop working on the tag endpoints. isImpersonated() and impersonatedBy(...) read only the ImpersonationContext thread-local (RuleEvaluator.java:614, :634), which is set by JwtFilter on the request thread and isn't copied into the job (RequestLatencyContext.wrapWithContext only carries the latency context).

So a Deny rule like isImpersonated() && matchAnyTag('Sensitivity.Confidential') evaluates to false here, and a bot acting for a user (e.g. an MCP agent) could tag or untag assets that rule is meant to protect. On main the check ran on the request thread, so it applied. Glossary, domain and data product are synchronous, so they're fine.

Setting ImpersonationContext from requester.actor().impersonatedBy() at the start of the job (and clearing it after) should fix it. A test with an impersonated request against such a deny rule would be good to have.

[manerow]

Confirmed, thanks. The tag tab's job ran without the request's impersonation, so isImpersonated() and impersonatedBy(...) rules were skipped there. d0d9895 sets ImpersonationContext from requester.actor().impersonatedBy() when the job starts and clears it when it ends.

New IT AssetsTabAuthorizationIT.aBotActingForAUser_isRefusedWhatAnImpersonationRuleDenies: a bot impersonating a user adds a tag to two tables, and the user has a deny rule isImpersonated() && matchAnyTag(...) covering one of them. Without the fix the protected table got the tag; with it, only the other table changes.

[sonika-shah]

A refused removal now fails silently in the UI. Before, a user without permission got a 403 for the whole request and the UI showed an error. Now it's a 200 with failed rows; the add dialog shows those, but the remove handler in AssetsTabs.component.tsx (onAssetRemove) only awaits the call and never looks at the result. So the dialog closes as if it worked, the asset stays, and no error appears. Same for rule failures, and on all four tabs.

Could we handle the BulkOperationResult on remove the same way the add dialog does?

[manerow]

Agreed, that was a regression. 61e5d64 and c3989f9 make the remove path read the result the way the add dialog does, on all four tabs. For the synchronous tabs it reads the BulkOperationResult. For the tag tab, whose removal runs as a job, it waits up to 2 minutes for the job's outcome on the websocket. When an asset is refused, an error toast shows the reason. If the outcome doesn't arrive in time, or the socket is disconnected, it asks the user to refresh the page to see the result. Tests are in AssetsTabs.utils.test.ts.

[sonika-shah]

This changes the API response for a user who isn't allowed: before it was a 403 for the whole request, now it's a 200 with status: FAILURE and a failed row (the updated GlossaryTermResourceIT cases assert exactly that). Scripts, SDK users or integrations that only check the HTTP status will now read a refusal as success.

It makes sense with per-asset authorization, but since it's a behaviour change for API clients, could we call it out in the PR description and release notes?

[manerow]

Agreed. The description now has an "API change" section. It says the tag and glossary term endpoints answer 200, with status failure or partialSuccess and failed rows, where they used to answer 403, so clients must read status. I also added the Backward-Incompatible-Change label so it gets into the 2.1 release notes.

[github-actions]

🚦 Removed from the merge queue — merge_conflict (2026-10-15T13:20:54Z)

The entry left the queue before it was built, so no checks ran against it.

Conflicts with the latest main — rebase, then re-queue.

[github-actions]

🚦 Removed from the merge queue — merge_conflict (2026-10-15T23:33:19Z)

The entry left the queue before it was built, so no checks ran against it.

Conflicts with the latest main — rebase, then re-queue.

[gitar-bot]

Code Review ✅ Approved 2 closed / 2 findings

🔴 High risk · Changing tag/glossary bulk endpoints from whole-request 403s to per-asset 200 failures may mislead clients that rely on HTTP status alone.

Assets tab tag, glossary term, domain and data product edits now save each asset through the same PATCH path as the asset page, so every edit creates a version, sets "updated by", and emits change events. Resolved that one unresolvable asset no longer fails every asset of its type, and that tag-tab job refusals are now reported instead of going unrecorded after a 120 s wait. No open issues found.

✅ 2 closed

✅ Bug: One unresolvable asset fails every asset of its type, including saved ones

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/AssetEditService.java:136-150 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityPatchBatch.java:83-97
EntityUtil.populateEntityReferences skips a reference given only by FQN when that FQN doesn't resolve, and leaves its id null. resolve() still builds a Selection with entityId == null. When EntityPatchBatch.load reaches that group, findEntitiesByIds calls UUID::toString on the null id and throws an NPE. The NPE leaves EntityPatchBatch.apply, and AssetEditService.editType catches it and runs failAll over every selection of that entity type. The same catch-all handles any group-level error, such as a failed DB load. Example: 250 tables are selected and the bad reference lands in the third group of 100. The first two groups (200 tables) are already committed with new versions and events, yet the response lists them as failed, and recordBulkAssetsChange leaves them out of the domain or data product's change event. Fix: reject a reference with no id in resolve() so it fails as its own row, and handle errors per group inside applyGroup so a group failure marks only that group's ids as failed.

✅ Quality: Unrecorded 120 s wait on tag-tab job after which refusals go unreported

📄 openmetadata-ui/src/main/resources/ui/src/components/Glossary/GlossaryTerms/tabs/AssetsTabs.utils.ts:30 📄 openmetadata-ui/src/main/resources/ui/src/components/Glossary/GlossaryTerms/tabs/AssetsTabs.utils.ts:81-95 📄 openmetadata-ui/src/main/resources/ui/src/components/Glossary/GlossaryTerms/tabs/AssetsTabs.utils.ts:109-123
BULK_ASSETS_JOB_WAIT_MS = 120000 sets how long a tag Assets tab removal keeps the spinner up waiting for the job's outcome. If the job takes longer, or the socket isn't connected, outcomeOf resolves undefined and removeAndReportRefusals shows no toast. So a refused asset looks removed, which is the problem this commit is meant to fix. The 120 s figure is a judgment call, and nothing in docs/decisions/ records it (the bulk-edit records cover authorization and composition, not a client wait budget).

The record would state: "A tag Assets tab removal waits up to 120 s for its job outcome on the bulk assets channel. After that, or with no socket connection, it stops waiting and reports nothing." Either add that record, or decide something else on timeout, for example show a toast saying the outcome is still pending. You could also reuse the existing TWO_MINUTE_IN_MILLISECOND constant.

Verification

Rules No rules evaluated

Functional validation Not enabled

Set up

Auto-approval Not enabled

Set up

_{For more verification details, comment gitar display:verbose.}

Options

Display: compact → Showing compact verification results.

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}

[greptile-apps]

Job outcomes grow without bounds

finished caches every completed job received on the shared channel, including unrelated jobs, without a size limit. It keeps collecting while the removal waits for its response or job outcome. The repository requires every cache to have an explicit size cap, so this must be bounded before merging. Cap the stored entries and discard them when consumed; the timeout does not limit how many outcomes the cache can hold.

Context Used: CLAUDE.md (source)

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ui'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ingestion'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud