WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

Last modified 2 years ago

#10667 closed enhancement (fixed)

Integration of Search API into core

Reported by: jshreve Owned by: westi
Milestone: 3.0 Priority: normal
Severity: normal Version: 2.9
Component: General Keywords: close
Focuses: Cc:

Description

The Search API plugin (http://wordpress.org/extend/plugins/search/) should be converted to a patch for the next release. The MySQL fulltext plugin should also be included in the distro to replace the default search.

Attachments (8)

searchapi.diff (3.4 KB) - added by jshreve 5 years ago.
Makes the neccessary changes to convert the WPSearchAPI plugin to a working part of the core
search.php (34.3 KB) - added by jshreve 5 years ago.
SearchAPI Core file. (to be placed in wp-includes/)
mysql.php (9.6 KB) - added by jshreve 5 years ago.
MySQL FullText Search Plugin (wp-plugins/). Changes to be compatible with the SearchAPI Core.
schema.txt (553 bytes) - added by jshreve 5 years ago.
The schema is included in the installer code but I'm providing it for reference
search.minimal.diff (359 bytes) - added by andy 4 years ago.
Minimalistic approach to making search pluggable.
plug-objects.diff (667 bytes) - added by andy 4 years ago.
Allow core classes WP_Query and WP to be replaced before instantiating.
search-pagination-1.diff (2.6 KB) - added by andy 4 years ago.
search-pagination-2.diff (3.0 KB) - added by andy 4 years ago.

Download all attachments as: .zip

Change History (36)

comment:1 scribu5 years ago

  • Keywords needs-patch added; search search-api removed
  • Version set to 2.9

jshreve5 years ago

Makes the neccessary changes to convert the WPSearchAPI plugin to a working part of the core

jshreve5 years ago

SearchAPI Core file. (to be placed in wp-includes/)

jshreve5 years ago

MySQL FullText Search Plugin (wp-plugins/). Changes to be compatible with the SearchAPI Core.

jshreve5 years ago

The schema is included in the installer code but I'm providing it for reference

comment:2 andy5 years ago

Excellent first draft!

What happens in WP_Query::get_posts when is_search? It looks like it will waste a database query and ignore the incorrect results. There are also some problems with execution order. Populating $wp_query->posts during template_redirect will expose any number of plugins to the incorrect results from above.

I think the search query should actually happen during WP_Query::get_posts, populate the appropriate object vars, run the appropriate filters on the result set, and skip everything else in that method. If possible, run the usual filters (for caching, etc.) on the query parts and then let the query happen as usual.

I expect some search plugins will return nothing more than an array of post_id's. The posts query would then look like SELECT * FROM $wpdb->posts WHERE ID IN(78,8,26).

comment:3 ryan4 years ago

  • Milestone changed from 2.9 to 3.0

comment:4 mattrude4 years ago

  • Cc m@… added

comment:5 hakre4 years ago

Can you compile a single patchfile so that it can be tested and reviewed better?

andy4 years ago

Minimalistic approach to making search pluggable.

comment:7 beaulebens4 years ago

  • Cc beau@… added

comment:8 jshreve4 years ago

  • Cc jshreve added

comment:9 follow-up: ryan4 years ago

(In [13037]) posts_search filter. Props skeltoac. see #10667

andy4 years ago

Allow core classes WP_Query and WP to be replaced before instantiating.

comment:10 filosofo4 years ago

plug-objects.diff really needs its own ticket.

But while it's here, wouldn't it be better handled by a filter?

$WP_Query = apply_filters('wp_query_class', 'WP_Query');
$wp_the_query =& new $WP_Query()

That way you have a decent chance through priority of deciding among competing plugins.

And don't forget about the half-dozen or so other instantiations of WP_Query in core.

comment:11 follow-up: ryan4 years ago

If the intent is to replace the class only for The Query, the filter could be called the_wp_query_class.

comment:12 in reply to: ↑ 11 ; follow-up: filosofo4 years ago

Replying to ryan:

If the intent is to replace the class only for The Query, the filter could be called the_wp_query_class.

Or maybe pass the context as the second argument to the filter?

I don't know why you'd want to replace the class in only one place, particularly since many themes call query_posts() directly.

comment:13 in reply to: ↑ 12 andy4 years ago

I think I like it better as a filter. I wrote it that way first but I'm not sure why I didn't keep it that way.

Replying to filosofo:

I don't know why you'd want to replace the class in only one place, particularly since many themes call query_posts() directly.

For example, a search plugin could use this. That would probably only want to replace/extend the core instance and only when isset($_GETs?).

Sometimes you're going to want to ignore the core posts query results so you might as well bypass all of WP_Query::get_posts. Heck, why not just stub it out in WP::main. That's why I showed up wanting to be able to replace or extend these classes.

While I'm at it, I'm thinking of how to fix wpdb so it can be extended. HyperDB is a PITA to keep synced with wpdb. It would be better if it extended wpdb.

So this should be two new tickets then.

comment:14 andy4 years ago

Twenty Ten uses "Older posts" and "Newer posts" labels for pagination links in search.php. The labels are only appropriate for chronological lists of posts and they are not filtered. A search plugin that supplies results in non-chronological order has easy way to correct the labels.

There should be $label filters in get_(next|previous)_posts_link(). Also, "Older posts" and "Newer posts" are unfortunate pagination labels for search.php. "Next page" or, better, "Next results" makes more sense here.

The Next and Previous links should be reversed on search result pages. It's going to become more common to see search results sorted other than reverse-chronological, and to include things other than posts.

Current (okay for browsing chronological archives):

<-- Older ..... Newer -->

Better for search results in general:

<-- Previous ... Next -->

Eventually I'd like to move all pagination into an abstract and filterable template tag. For now, let's set a better example for search results and apply filters to the the labels.

Attaching a patch, search-pagination-1.diff:

  • Reverses direction of search pagination links
  • Changes labels of same
  • Adds filters for pagination link labels

andy4 years ago

comment:15 nacin4 years ago

Related to your thoughts on abstracting that: #10219 - "Older Entries" and "Newer Entries" links are wrong when entries displayed in ascending order.

andy4 years ago

comment:16 in reply to: ↑ 9 ; follow-up: westi4 years ago

Replying to ryan:

(In [13037]) posts_search filter. Props skeltoac. see #10667

Is this PHP4 compatible?

Should we not be passing by reference so we don't end up with a copy - is this another reason we need an apply_filters_ref_array()? = #9886

comment:17 nacin4 years ago

  • Owner set to westi
  • Status changed from new to assigned

comment:18 in reply to: ↑ 16 ; follow-up: westi4 years ago

Replying to westi:

Replying to ryan:

(In [13037]) posts_search filter. Props skeltoac. see #10667

Is this PHP4 compatible?

Should we not be passing by reference so we don't end up with a copy - is this another reason we need an apply_filters_ref_array()? = #9886

This was done when we introduced apply_filters_ref_array()

comment:19 nacin4 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

comment:20 brentes4 years ago

  • Cc thenbrent@… added
  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Type changed from task (blessed) to defect (bug)

I've reopened this as the 'posts_search' filter is being applied on every query, rather than only those queries where a search pattern is specified.

I'm guessing that's a bug and not by design?

If it is a bug, it's easily fixed, the filter just needs to moved up one line above the curly brace. :)

comment:21 scribu4 years ago

  • Keywords needs-patch removed

I'm not so sure about that. A plugin could want to do some searching even when there isn't a search pattern defined.

Anyway, I don't really see the problem. Just bail if the search pattern is empty.

comment:22 scribu4 years ago

  • Keywords close added
  • Type changed from defect (bug) to enhancement

comment:23 brentes4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Yeah that's fair enough and as you say, it's not a problem to test within the hooked function if a search pattern is defined.

I guess the hook's documentation will just need to show it's not actually called only on searches, as the name implies, to avoid that trap for young players (like myself).

comment:24 westi4 years ago

(In [14603]) Add commentary about the posts_search filter. See #10667.

comment:25 sussane3 years ago

[spam comment removed by dd32]

Last edited 3 years ago by dd32 (previous) (diff)

comment:26 dmitrihughes3 years ago

[spam comment removed by dd32]

Last edited 3 years ago by dd32 (previous) (diff)

comment:27 sussane3 years ago

this api was causing me big headache, lucky i found this ticket and i look like so funny quotes to see it fixed. lovely... from hilarious quotes

Version 0, edited 3 years ago by sussane (next)

comment:28 in reply to: ↑ 18 Testuser12 years ago

Nothing.

Last edited 2 years ago by SergeyBiryukov (previous) (diff)
Note: See TracTickets for help on using tickets.