search

10up / ElasticPress

A fast and flexible search and query engine for WordPress.
https://elasticpress.io
GNU General Public License v2.0
1.25k stars 311 forks source link

BUG: Removal from index on update may fail #3936

Open dtakken opened 3 months ago

dtakken commented 3 months ago

Describe the bug

Setting a password on a post and updating it from the admin page does not always remove it from the ES index. It may require hitting the update button a second time for the post to be removed. The behavior depends on details in the implementation of wp_insert_post() in WordPress, which varies between versions.

Explanation of why this might happen follows. Some versions of WordPress trigger the set_post_terms hook before the post is updated in the database, while the wp_insert_post hook is triggered after the database update. You can check this in the implementation of wp_insert_post() of WordPress 6.4.3 for example. In other WordPress versions (6.5.2) both hooks are invoked after the database update.

ElasticPress uses both of these hooks. As a result, SyncManager::should_reindex_post() is called twice while updating a post. Because it fetches the post from the database, this method may first see the old post, then the new post.

Currently, when any of the filters adds the post to the SyncManager update queue, the post is indexed even when other filters determine that the post should not be indexed. I think this is not correct. Regardless of whether or not this is a WordPress bug, I think the last filter that runs should have the final say about indexing the post or not.

Steps to Reproduce

  1. Create and publish a post
  2. Check that the post is in de index
  3. Change visibility, set a password
  4. Update the post
  5. Check that the post is no longer in the index

Screenshots, screen recording, code snippet

No response

Environment information

Ubuntu 22.04 , Chromium 125.0.6422.141

WordPress and ElasticPress information

No response

Code of Conduct

dtakken commented 3 months ago

I just identified another scenario where updating a post fails to exclude it from the index, also on newer WordPress versions (I found the issue using WP 6.5.3).

The scenario is as follows. We use a custom ACF field to enable authors to flag posts for exclusion from the index. This predates the similar feature offered by newer ElasticPress versions. During a post update, WordPress calls wp_set_post_terms() before the meta field is updated. The result is the same: ElasticPress has already added the post to the queue before the meta field which indicates exclusion is updated.

The linked pull requests will fix this scenario as well.