Skip to content

fix(bricks): avoid multiplying postmeta joins during class cleanup - #46

Open
Jan-CoreBunch wants to merge 2 commits into
CoreBunch:mainfrom
Jan-CoreBunch:fix/bricks-class-reference-query
Open

Jan-CoreBunch wants to merge 2 commits into
CoreBunch:mainfrom
Jan-CoreBunch:fix/bricks-class-reference-query

Conversation

@Jan-CoreBunch

@Jan-CoreBunch Jan-CoreBunch commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

What

Removing Core Framework classes can hang a Bricks site because reference discovery generates five postmeta joins. Replace the five OR/EXISTS clauses with one EXISTS clause using compare_key => IN, so WordPress generates a single join across the same five metadata keys.

Keep the existing get_posts() post type/status handling, metadata batching, and class-reference cleanup. Add coverage for all five keys and a real WordPress SQL regression check that short-circuits execution before a potentially explosive query can run.

Why

The five joins multiply intermediate rows on posts with lots of unrelated metadata. A customer reported a six-hour query that stalled their site. Batching the later cleanup does not bound this initial lookup.

The new query preserves WordPress query behavior while removing the join multiplication; it does not require a post type allowlist or custom SQL.

The PR also reuses the dependency-only fix from #45 to unblock the existing critical security audit: pin proxy-addr to 2.0.8 and shell-quote to 1.11.0. The lockfile synchronizes existing workspace version metadata; no other dependencies are upgraded and no Figma application changes are included.

Verification

php packages/wp/vendor/bin/phpunit --do-not-cache-result packages/wp/Tests
OK (46 tests, 103 assertions)

php -l packages/wp/wp/App/Bricks/Functions.php
php -l packages/wp/Tests/BricksSynchronizationTest.php
No syntax errors detected

bash -n scripts/test-wp-e2e.sh
git diff --check
Passed
  • Confirmed the SQL regression check fails with the original implementation and passes with the fix.
  • Docker runtime checks passed with WordPress 7.1.2, PHP 8.2.34, MariaDB 11.4.13, and Bricks 2.4.2. Covered plugin activation/reactivation, stylesheet generation, admin/assets/fonts, REST authorization, and Figma connection-key lifecycle.
  • An isolated fixture with 50 posts and 3,651 metadata rows selected the same 18 post IDs and cleaned 19 metadata rows. Checked templates, custom post types, post statuses, duplicate keys, empty/scalar values, orphan exclusion, unrelated metadata, and repeated cleanup.
  • On that fixture, the original five-join query hit the deliberate one-second database timeout (1,000.426 ms); the fixed lookup took 0.755 ms and complete cleanup took 71.296 ms. These are local fixture measurements, not customer-site timings.

The Docker run used a temporary Windows-compatible copy of the E2E runner with the fixture checks appended:

$env:MSYS_NO_PATHCONV='1'
$env:WP_E2E_WORDPRESS_IMAGE='wordpress:7.1.2-php8.2-apache'
$env:WP_E2E_DATABASE_IMAGE='mariadb:11.4'
& 'C:\Program Files\Git\bin\bash.exe' .tmp/bricks-mariadb-validation/source/scripts/test-wp-e2e-windows.sh /c/Projects/Core-Framework-Public/.tmp/bricks-mariadb-validation/artifacts/core-framework-2.0.2.zip

The GitHub Actions WordPress E2E job built the release ZIP from a fresh checkout, including Gutenberg assets, and passed bun run e2e:wp on WordPress 7.1.3 / PHP 8.2.34. This supplies fresh-build validation in addition to the local fixture run, which used existing built assets. Temporary runners, fixtures, and build artifacts are not included in this PR.

The dependency update also passed bun install --frozen-lockfile --lockfile-only --ignore-scripts and bun audit --audit-level=critical in an isolated Bun 1.3.11 container (both exit 0).

All checks passed on commit 1d4a33b: Build & Typecheck, web tests, PHP tests, and WordPress release E2E, plus both Socket Security checks.

This branch has not been deployed

No deployments
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.

1 participant