- Await both tabs before asserting order in tab ordering tests
- Use findByText for icon assertions in context menu tests
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Fredrik Adelöw <freben@spotify.com>
The entity page tests render the full extension tester + entity layout
component tree, which involves multiple React.lazy boundaries,
useAsyncRetry for entity fetching, and Material-UI CSS-in-JS processing
in jsdom. This makes each test inherently slow (~300-600ms) due to
jsdom's CSS engine processing MUI stylesheets during React render
cycles. Under CI load, the default 5s Jest timeout can be exceeded.
- Add jest.setTimeout(30_000) to prevent CI timeouts
- Replace waitFor(expect(getByRole(...))) antipattern with
await expect(findByRole(...)).resolves pattern throughout
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Fredrik Adelöw <freben@spotify.com>
* draft: restructure entities() ordering to drive from search-by-key
When a sort field is specified, build the query so the search table
filtered by that key is the driving relation, instead of left-joining
search onto final_entities and sorting after. The planner can then walk
the (key, value, entity_id) index in already-sorted order and short
circuit on LIMIT.
Measured against a production replica, the catalog UI's default
"first page of components ordered by metadata.name" query goes from
~940 ms to ~8 ms. See PR description for the full numbers.
Known caveats this draft does not yet address:
- Entities lacking the order field are excluded; the previous shape
put them at the end with NULLS LAST. A UNION ALL pattern can
preserve the old semantics.
- Multi-field order falls back to the OLD shape (only the first field
is taken when present today; subsequent fields acted as
tie-breakers). Restoring tie-breakers needs additional joins or a
CTE.
- queryEntities (/entities/by-query) is left untouched; the same
optimization applies but the CTE/cursor structure makes the rewrite
more involved.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Fredrik Adelöw <freben@spotify.com>
* two-phase entities() ordering: fast path + NULLs-last fallback
When an order field is specified, run a fast path first that drives
from the search-by-key index (excluding entities without the field).
If that path doesn't produce enough rows to cover offset+limit+1, run
a fallback that picks up the no-field entities in entity_id order.
This preserves NULLs-LAST semantics while keeping the fast plan for
the common case where every entity has the order field.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Fredrik Adelöw <freben@spotify.com>
* fix: multi-field order fallback + MySQL quoting in entities()
Fall back to the original LEFT JOIN shape when multiple order fields
are specified, since tie-breaking on secondary fields inherently
requires materialization. The fast INNER-JOIN-driven path is used only
for single-field order (the typical UI case).
Fix the phase 2 NOT EXISTS clause to use knex's ?? identifier escaping
instead of hardcoded double-quotes, which broke on MySQL.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Fredrik Adelöw <freben@spotify.com>
* catalog-backend: fix phase 2 ordering and update changeset
Fix two issues raised in review:
- Phase 2 (entities lacking the sort field) now always sorts by
entity_id ASC regardless of the primary sort direction, matching the
original NULLS-LAST behaviour where the NULL group was always
entity_id ASC.
- Update changeset to describe the actual two-phase behaviour (NULLS
LAST preserved) instead of the stale description that said entities
without the field are excluded.
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* catalog-backend: apply offset when limit is undefined in runOrderedEntitiesQuery
Previously the fast-path returned the full combined array when no limit
was specified, silently ignoring any pagination offset. The old
implementation always pushed offset to SQL independently of limit.
Restore parity by slicing the combined array by the offset even when
limit is absent.
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* catalog-backend: add pagination and phase-boundary tests for entities() ordering
Add two new test cases that cover the parts of runOrderedEntitiesQuery not
exercised before:
- "paginates correctly through single-field ordering": exercises limit, offset,
and hasNextPage through the fast path (Phase 1 only) across all DB engines.
- "paginates across the Phase 1 / Phase 2 boundary": exercises the case where
the requested page straddles entities that have the sort field (Phase 1) and
those that do not (Phase 2), and verifies that Phase 2 entities are always
ordered ASC by entity_id regardless of the primary sort direction.
Also fixes a double-space caught by prettier in DefaultEntitiesCatalog.ts.
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* catalog-backend: treat null sort-field value the same as a missing sort field
buildEntitySearch stores value=NULL for entity fields that are explicitly
null or exceed MAX_VALUE_LENGTH. Previously, Phase 1 included those rows via
the INNER JOIN (key matches, value IS NULL), causing them to sort ahead of
entities that have no row for the key at all — changing semantics vs the old
LEFT JOIN shape where both cases landed in the same NULLS-LAST bucket.
Fix Phase 1 to require order_0.value IS NOT NULL, and fix Phase 2's NOT EXISTS
to check for no non-null value (value IS NOT NULL) so that both null-valued
and missing-key entities are collected in Phase 2 and ordered together by
entity_id ASC.
Adds a regression test covering an entity with spec.b=null alongside one with
no spec.b, asserting both appear after sorted entities regardless of direction.
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Signed-off-by: Fredrik Adelöw <freben@spotify.com>
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Add ORDER BY to guarantee stable result ordering across all database
backends (MySQL does not sort without it). Also consolidate double
catalog.facets() calls in tests into a single call with both content
and length assertions on the same result.
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Replace the LEFT OUTER JOIN + DISTINCT in the queryEntities CTE with
an INNER JOIN that drives from the search table for the sort field's
key. Entities lacking the sort field are excluded from both the result
and the count, aligning totalItems with navigable entities.
Removes DISTINCT (prerequisite: search table dedup migration).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Fredrik Adelöw <freben@spotify.com>
GROUP BY result ordering is non-deterministic across database engines.
The switch from COUNT(DISTINCT entity_id) to COUNT(*) changes MySQL's
aggregation plan, which surfaces a different row order. Use
arrayContaining + length check instead of exact array equality.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
The UNIQUE constraint on (entity_id, key, value) from the search
indices migration guarantees each entity appears at most once per
(key, original_value) group, making DISTINCT unnecessary. Removing
it lets the database use a simpler aggregation plan.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
* feat(scaffolder): add BUI theme for scaffolder forms
Add a Backstage UI (BUI) form theme as an alternative to the Material
UI theme. Toggled via formProps.theme or enableBackstageUi page config.
Includes BUI widgets, templates, field extension variants, and a ported
React Aria Autocomplete component.
Signed-off-by: benjdlambert <ben@blam.sh>
* refactor(scaffolder): use BUI Combobox and CheckboxGroup for form widgets
Signed-off-by: benjdlambert <ben@blam.sh>
* chore(scaffolder): enable BUI form flag and add kitchen sink demo template
Signed-off-by: benjdlambert <ben@blam.sh>
* fix(scaffolder): use outlined input style for BUI form widgets
Signed-off-by: benjdlambert <ben@blam.sh>
* fix(scaffolder): address BUI form PR feedback
Signed-off-by: benjdlambert <ben@blam.sh>
* fix(scaffolder): format CSS and regen API reports
Signed-off-by: benjdlambert <ben@blam.sh>
---------
Signed-off-by: benjdlambert <ben@blam.sh>
Move NULL_SENTINEL to util.ts and import it in buildEntitySearch and
syncSearchRows instead of hardcoding '\x01'. Keeps the dedup keys
consistent with filterSentinelValues and the SQL COALESCE(…, chr(1))
logic, avoiding drift if the sentinel ever changes.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
The 'silently rejects a direct duplicate insert' test inserted a row
via raw Knex onConflict().ignore(), which has no corresponding
production code path (syncSearchRows uses ON CONFLICT DO UPDATE, not
DO NOTHING). The idempotency behaviour it was intended to verify is
already covered by 'leaves unchanged rows untouched'.
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Replace the raw Knex onConflict().merge() insert in the
'overwrites original_value on conflict' test with a direct UPDATE to
corrupt the stored original_value, followed by a second syncSearchRows
call. This tests the actual application code path (ON CONFLICT DO UPDATE
inside syncSearchRows) rather than embedding the conflict logic in the
test itself.
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Tests two scenarios across all supported databases:
Preconditions NOT met (dedup runs): inserts five rows with two
duplicate (entity_id, key, value) pairs (one with null value),
runs the migration, and verifies that duplicates are removed, null
values are handled correctly, and the unique constraint is enforced
post-migration. The down migration is verified to drop the constraint
so duplicates can be inserted again.
Preconditions met (PostgreSQL only, dedup skipped): pre-creates the
unique index to simulate a user who ran the manual SQL before
deploying, re-runs the migration, and verifies the row count is
unchanged — confirming the fast path fires correctly.
Non-PostgreSQL databases skip the fast-path test with an early return
since the pg_index check is PostgreSQL-specific.
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Update the 'keeps one row when original_value casing differs' test to
expect original_value: 'V' (first occurrence) instead of 'v' (last
occurrence), matching the first-wins dedup semantics that were aligned
with buildEntitySearch in a prior commit.
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
The index-only GROUP BY scan in Phase 1 of the dedup requires
search_key_value_entity_idx (key, value, entity_id) to exist.
Previously it was created after dedup, meaning fresh installs that
had never manually run preparatory SQL would fall back to a full
sequential scan.
Move the ensurePgIndex call for search_key_value_entity_idx to before
the dedup step so the fast path is guaranteed for all users, not just
those who created the index manually in advance.
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Replace the window-function full-table scan with a two-phase approach
that leverages search_key_value_entity_idx (key, value, entity_id):
Phase 1: GROUP BY entity_id, key, value with HAVING COUNT(*) > 1
resolves as a pure index-only scan (Heap Fetches: 0).
Stores only the duplicate groups in a temp table (~8s for
a 14M-row table with 700k dupes).
Phase 2: CROSS JOIN LATERAL back into search using the same covering
index (Nested Loop + Index Scan). row_number() runs per-group
over the 2-3 matching rows, so there is no global external
sort. A single DELETE removes all extras in one statement
(~16s).
NULL values get a separate UNION ALL arm so the index equality
condition stays usable (value = NULL is always false in SQL).
Benchmarked on a 14.4M-row production-like staging master with
700k injected duplicates, post-VACUUM:
Old (seq scan + external merge sort): ~101s
New (index-only + index scan): ~25s (~4× faster)
Clean second run (no dupes, fast path skips dedup): <50ms.
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Use `=== null ? '\x01' : value` instead of `?? ''` in both buildEntitySearch
and syncSearchRows dedup maps, so that null and empty-string values are
treated as distinct keys. In theory an entity could produce both value=null
and value='' for the same key (e.g. spec.foo: [null, '']), and the old
encoding would silently drop one of the two distinct rows.
Also adds two focused unit tests to buildEntitySearch.test.ts: one covering
deduplication of duplicate array values (e.g. tags: ['java', 'java', 'Java']),
and one confirming that null and empty-string are kept as separate rows.
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
The original single-pass CTE window-function DELETE re-scanned every row
on each invocation — including the idempotent "nothing to do" case which
took over 4 minutes on a 14 M-row table, and timed out the DB proxy on
the dirty case (~217 s for 722 k duplicates).
Replace it with two improvements:
1. Fast path: if the UNIQUE index already exists and is valid, skip dedup
entirely. A valid unique index is a proof-of-no-duplicates. This makes
migration startup essentially free for installations that prepare the
index manually beforehand (as documented in the migration comment).
2. Two-phase dedup when dedup is needed: Phase 1 does one full-table scan
to collect all duplicate ctids into a temp table (~60 s on 14 M rows).
Phase 2 drains that temp table in 10 k-row batches via cheap ctid
lookups with no further full-table scans (~5 s for 722 k rows). Total
~65 s dirty vs >217 s+ before, and ~35 ms clean vs 4+ minutes before.
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Prevents a false match if any non-index object (table, sequence, view)
shares a name with one of the search indices.
Signed-off-by: Fredrik Adelöw <freben@spotify.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Verifies that a conflicting insert with a different original_value casing
updates the stored value, and documents that DO UPDATE requires explicit
conflict columns unlike DO NOTHING.
Signed-off-by: Fredrik Adelöw <freben@spotify.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Remove the explicit column list from .onConflict() calls in test setup —
ON CONFLICT DO NOTHING without a target is equivalent since (entity_id,
key, value) is the only unique constraint on the search table.
Add a dedicated test in syncSearchRows that directly inserts a duplicate
row and verifies it is silently rejected via the UNIQUE constraint.
Signed-off-by: Fredrik Adelöw <freben@spotify.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Avoids a window with no coverage on (key, value) during rollback.
Signed-off-by: Fredrik Adelöw <freben@spotify.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* feat(scaffolder): promote formDecorators out of experimental
Signed-off-by: benjdlambert <ben@blam.sh>
* fix(scaffolder): parse form decorator input through the configured zod schema
Signed-off-by: benjdlambert <ben@blam.sh>
* refactor(scaffolder-backend): emit single formDecorators field on the parameter-schema response
Signed-off-by: benjdlambert <ben@blam.sh>
* feat(scaffolder): promote form decorator blueprints to public API
Signed-off-by: benjdlambert <ben@blam.sh>
---------
Signed-off-by: benjdlambert <ben@blam.sh>
Be explicit that interrupted index builds do not leave partial progress —
each retry starts from scratch. On large tables with short liveness
probe timeouts this repeats indefinitely. Recommend running the SQL
commands manually before deploying.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Fredrik Adelöw <freben@spotify.com>
The down migration now restores the previous index state (drops new
indices, recreates the old search_key_value_idx and
search_key_original_value_idx). Updated the SQL report to reflect the
new index set.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Fredrik Adelöw <freben@spotify.com>
Single knex migration that cleans up duplicate search rows and creates
covering indices including a UNIQUE constraint on (entity_id, key, value).
Each step is idempotent and handles INVALID indices from interrupted runs.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Fredrik Adelöw <freben@spotify.com>
Hold a shared promise rather than just a resolved value. Concurrent
callers awaiting a fresh count get the same in-flight promise back, so
the underlying query is never overlapped by a duplicate. The TTL is now
the minimum gap between the resolution of one query and the start of
the next, rather than a hard bound on cache age.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Fredrik Adelöw <freben@spotify.com>
The legacy Prometheus and OpenTelemetry observable gauges previously each
ran the per-kind count query against the search table on every metrics
scrape. With multiple pods and short scrape intervals, identical
sequential scans piled up faster than they completed, contending for
buffers in the database.
Extract a shared helper that wraps a 30-second TTL cache around a single
query, and have both gauges read from it. The query itself moves from
the (large) search table to final_entities, parsing kind out of
entity_ref via per-engine substring functions. The emitted labels and
values are unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Fredrik Adelöw <freben@spotify.com>
Replace the `WHERE search.entity_id IN (...)` form with an INNER JOIN
against the filtered final_entities subquery in DefaultEntitiesCatalog#facets.
Results are unchanged; the planner gets more freedom to pick cheaper plans,
which on large catalogs leads to substantial speedups (1.2× to 7×+ in
adversarial testing on a ~13.8M-row search table) and avoids the
materialize-then-spill pattern of the IN form.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Fredrik Adelöw <freben@spotify.com>
Use a word-boundary regex (:name\b) instead of a plain string replace
so that a shorter param like :a doesn't corrupt a longer param :ab
when both are present in the route.
Signed-off-by: Patrik Oldsberg <rugvip@backstage.io>
Signed-off-by: Fredrik Adelöw <freben@spotify.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
The app/routes redirect config now performs the same :param and *
substitution that the legacy Redirect component did before navigating.
Named params captured by the `from` pattern are replaced in the `to`
string, enabling redirects like /users/:userId → /profile/:userId and
/old-docs → /docs/* (with splat forwarding).
Adds tests for both named-param and splat substitution.
Signed-off-by: Patrik Oldsberg <rugvip@backstage.io>
Signed-off-by: Fredrik Adelöw <freben@spotify.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Fredrik Adelöw