Skip to content

feat: resolve and link resource cross-references - #3459

Merged
LiteSun merged 1 commit into
apache:masterfrom
lxbme:feat/navigable-cross-references
Aug 3, 2026
Merged

LiteSun merged 1 commit into
apache:masterfrom
lxbme:feat/navigable-cross-references

Conversation

@lxbme

@lxbme lxbme commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Please answer these questions before submitting a pull request, or your PR will get closed.

Why submit this pull request?

  • Bugfix
  • New feature provided
  • Improve performance
  • Backport patches

What changes will this PR take into?

upstream_id, service_id, plugin_config_id and group_id rendered as plain text inputs holding a bare id. There was no name resolution and nothing to click, so answering "which upstream is this route actually using?" meant copying the id, navigating to Upstreams, and searching by hand.

Each field now resolves its id and offers a link to the referenced resource. Four fields in two files cover the four referenceable resource types — FormPartRoute/index.tsx (upstream_id, plugin_config_id, service_id) and FormPartConsumer.tsx (group_id) — because the form sections that hold these fields are shared between routes, stream routes, services and consumers. The change is far smaller than "every resource with an id field": it targets exactly the four fields that reference another top-level resource.

Why the control is an anchor, not RouteLinkBtn

The repo's existing RouteLinkBtn is a Mantine Button. FormSection renders a <fieldset> and passes disabled when the section is read-only — most visibly in the nested service → route view, where the whole form is disabled while browsing. A disabled <fieldset> disables every descendant form control, including a <button>-based link, which would make it dead exactly where a route's inline upstream is being reviewed. A real <a> (via createLink wrapping a Mantine Anchor) is left untouched by disabled on an ancestor <fieldset>, so it stays clickable in both read-only and edit views. RouteLinkBtn's own call sites are deliberately left untouched by this change.

Dangling references and non-404 failures

A reference that resolves to nothing gets a warning icon rather than a link — sending the user to a page that only reports the resource missing is a wasted round trip, and a broken reference is worth stating explicitly rather than silently. Anything that is not a 404 — in flight, a network error, a 500 — renders nothing at all: the dashboard could not ask whether the resource exists, which is not the same as the reference being broken, so it makes no claim either way.

The query carries its own retry rule

Resolving a reference means a GET that 404s whenever the reference is dangling, and the app's global policy retries anything that is not a 401 three times. A 404 here is not a failure — it is the answer "no such resource" — so inheriting that policy meant ~8s of backoff before the warning appeared, repeated on every typing pause in edit mode and every window refocus. The query therefore carries its own predicate, in the same shape as the one already in usePluginMetadataList, which brings that to ~1.1s on load and ~0.4s while typing. Those are measured, and the e2e timeouts were tightened from 15s to 3s so a return of the backoff fails the spec instead of passing slowly.

Interaction with #3458

#3458 (open) makes the same judgement globally: it stops retrying Admin API 404s and silences their toast for reads. Once it lands, the local predicate here becomes redundant and should collapse into it — the code says so at the point where it would be removed. Until then, one thing this branch does not fix on its own: a dangling reference still raises the gateway's Key not found toast, because suppressing that would mean either threading a skip header through the shared detail-query factory or importing #3458's global rule. That is left to #3458 rather than duplicated here.

Compatibility with the other in-flight PRs

This branch was verified against a local octopus merge of all five branches currently queued against master: #3454, #3456, #3457, #3458 and this one. The merge is clean on top of the other four — the only conflict in that merge is between #3456 and #3458 themselves (both rewrite the route detail page's PageHeader), and is unrelated to this change. A maintainer merging all five in one pass will hit that conflict once, in src/routes/routes/detail.$id.tsx; the resolution is to keep both sides (the test-request button from #3456 and the id-bearing title from #3458).

Related issues

Part of #3453

Checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

Tests:

  • e2e/tests/regression/form.cross-references.spec.ts — four tests: a resolved reference renders as a real link (asserted via getByRole('link'), which fails if it ever regresses to a button) to the referenced resource's detail page; a dangling reference shows a warning and no link; the resolved state follows what is currently typed into the field, not the saved value; and a consumer group reference resolves the same way on the consumer page, covering the second call-site shape and the no-name fallback (consumer groups have no name field). The seeded upstream is deliberately named with an ampersand so the accessible name is pinned unescaped.

    Note on the dangling fixture: it is created by force-deleting a referenced upstream, not by writing a route that points at nothing — APISIX 3.17.0 rejects the latter with a 400. Force-deletion is also how real deployments acquire dangling references.

  • src/components/form/ResourceRef.test.ts — pins each of the four resources to its own detail query and its own route. The table that pairs them is behind a cast that TanStack's queryOptions() forces, so the type system checks the route strings but not the pairing; swapping two factories compiles clean and this test is what catches it.

Verified: pnpm test, pnpm exec tsc -b, pnpm lint and a production pnpm build all clean, on this branch alone and again on the five-branch octopus merge described above. The full Playwright suite run on that merged tree: 232 passed, 2 failed, both environmental and unrelated to this change — a Monaco editor render race in plugin_metadata.crud-all-fields (finishes suspiciously fast on the failing run, 5.6s vs its usual ~7-9s, consistent with a race rather than a real failure) and stream_routes.show-disabled-error, which restarts a compose project that does not exist on this machine. Neither failure is specific to the merged tree or to this branch.

`upstream_id`, `service_id`, `plugin_config_id` and `group_id` rendered as
plain text inputs holding a bare id. There was no name resolution and
nothing to click, so answering "which upstream is this route actually
using?" meant copying the id, opening Upstreams, and searching.

Each field now resolves its id and offers a link to the referenced
resource. A reference that resolves to nothing gets a warning instead of
a link: sending the user to a page that only reports the resource missing
is a wasted round trip, and a broken reference is worth stating. Anything
that is not a 404 renders nothing at all — the dashboard could not ask,
which is not the same as the reference being broken.

The link is a real anchor rather than the existing RouteLinkBtn, which is
a Mantine Button. Detail sections render as a fieldset and pass `disabled`
in the nested service view, and a disabled fieldset kills every descendant
form control but leaves anchors alone.

The query carries its own retry rule. It inherits a global policy that
retries anything but a 401 three times, and a 404 here is not a failure —
it is the answer "no such resource" — so a dangling reference took ~8s of
backoff before the warning appeared, again on every typing pause and every
window refocus. It now settles in ~1s.

Four fields in two files cover routes, stream routes, services and
consumers, because the form sections are shared. The target route and the
detail query live beside the resource key in one table rather than being
passed in alongside it; they were redundant degrees of freedom over a 1:1
key, and a unit test pins each pairing since the type system cannot.

Part of apache#3453
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.

3 participants