Skip to content

Enterprise Search: Prevent site's index version leaking into cross-site ES queries - #6973

Open
rebeccahum wants to merge 2 commits into
developfrom
VIPOPS-5293
Open

rebeccahum wants to merge 2 commits into
developfrom
VIPOPS-5293

Conversation

@rebeccahum

@rebeccahum rebeccahum commented May 19, 2026

Copy link
Copy Markdown
Contributor

Description

This pull request addresses a bug in multisite environments where the search index name version could be incorrectly resolved from the calling site's context rather than the target site. The update ensures that cross-site queries use the correct version data for each site, and adds a test to verify this behavior.

Changelog Description

Fixed

  • Enterprise Search: Prevent site's index version leaking into cross-site ES queries

Pre-review checklist

Please make sure the items below have been covered before requesting a review:

  • This change works and has been tested locally or in Codespaces (or has an appropriate fallback).
  • This change works and has been tested on a sandbox.
  • This change has relevant unit tests (if applicable).
  • This change uses a rollout method to ease with deployment (if applicable - especially for large scale actions that require writes).
  • This change has relevant documentation additions / updates (if applicable).
  • I've created a changelog description that aligns with the provided examples.

Pre-deploy checklist

  • VIP staff: Ensure any alerts added/updated conform to internal standards (see internal documentation).

Steps to Test

  1. On a MS, create 2 more subsites
  2. On site 2, create an index version 2 and activate it
  3. Add:
$query = new WP_Query(
	array(
		's' => 'carrot',
		'sites' => array( 2, 3 ), // Search only in site IDs 2 and 3 
		'ep_integrate' => true,
	)
);
  1. In site 2 context, you should see in the QM API call list:
http://elasticsearch:9200/vip-200508-post-2-v2,vip-200508-post-3/_search

@codecov

codecov Bot commented May 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 35.66%. Comparing base (f9c0d59) to head (dd2f2aa).
⚠️ Report is 205 commits behind head on develop.

Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #6973      +/-   ##
=============================================
+ Coverage      35.62%   35.66%   +0.03%     
- Complexity      5210     5214       +4     
=============================================
  Files            300      300              
  Lines          21370    21375       +5     
=============================================
+ Hits            7614     7624      +10     
+ Misses         13756    13751       -5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rebeccahum
rebeccahum marked this pull request as ready for review May 19, 2026 23:01
@rebeccahum
rebeccahum requested a review from a team as a code owner May 19, 2026 23:01
Copilot AI review requested due to automatic review settings May 19, 2026 23:01

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

Fixes a multisite bug in VIP Enterprise Search where ep_index_name could resolve the index version using the calling site’s options instead of the target site’s options during cross-site queries.

Changes:

  • Switch blog context inside Search::filter__ep_index_name() when resolving version data for a different target blog.
  • Update/extend PHPUnit coverage to validate cross-site index version resolution behavior.

Reviewed changes

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

File Description
search/includes/classes/class-search.php Switches blog context when building versioned index names for cross-site queries.
tests/search/includes/classes/test-class-search.php Updates existing data provider expectations and adds a multisite test covering the cross-site version-resolution regression.

Comment thread search/includes/classes/class-search.php
Comment thread tests/search/includes/classes/test-class-search.php Outdated
@rinatkhaziev

Copy link
Copy Markdown
Contributor

Is there a way to do this without switch_to_blog? probably no bueno for very large sites.

rebeccahum and others added 2 commits June 3, 2026 10:03
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@sonarqubecloud

sonarqubecloud Bot commented Jun 3, 2026

Copy link
Copy Markdown

@rebeccahum

Copy link
Copy Markdown
Contributor Author

I'm not sure how to retrieve the blog context better w/o switch_to_blog() :/

@rinatkhaziev

Copy link
Copy Markdown
Contributor

@rebeccahum how about get_site_option?

@rebeccahum

Copy link
Copy Markdown
Contributor Author

@rinatkhaziev We can't do get_site_option because that reads from the overall network-wide options table, not the actual blog....so we'd need get_blog_option() which basically does the blog switch...

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

This pull request has been marked stale because it has been open for 60 days with no activity. If there is no activity within 7 days, it will be closed.

This is an automation to keep pull requests manageable and actionable and is not a comment on the quality of this pull request nor on the work done so far. Closed PRs are still valuable to the project and their branches are preserved.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants