EDS: Allow limiter-only queries #4976

maccabeelevine · 2025-12-18T17:45:27Z

This partially reverts #4364 , based on discussion afterwards in #vufind-help. @cwolfebsco shared

The Search PM told me this: "Limiter-only queries are still supported and will continue to be supported. They have not caused the problems that ‘FT Y OR FT N’ have." - so I think it is intentional that they work

So this allows searches that have either a query string or a limiter (or both).

maccabeelevine · 2025-12-18T17:46:43Z

module/VuFind/src/VuFind/Search/EDS/Results.php

        $limit  = $this->getParams()->getLimit();
        $offset = $this->getStartRecord() - 1;
        $params = $this->getParams()->getBackendParameters();
+        if ($allTerms === '') {


I could do both conditional checks together, but I thought there might be a performance impact to the array_filter call...though probably not significant?

It might be worth adding a paramsIncludeLimiter($params) method so you can check if ($allTerms === '' && !$this->paramsIncludeLimiter($params)) {. Due to the way the && operator works, the second clause will only be evaluated if the first clause evaluates to true, so there should be no performance hit for that approach.

maccabeelevine · 2025-12-18T17:56:02Z

This works best when an institution has a least one limiter enabled by default (this being done in the search profile in EBSCOadmin, I think). Our institution has "Available in Library Collection" (FT1) on by default. I would imagine a lot of institutions have the same.

maccabeelevine · 2025-12-18T17:59:25Z

@cwolfebsco would appreciate your review as well! Can't tag you above.

maccabeelevine · 2025-12-18T18:00:35Z

module/VuFind/src/VuFind/Search/EDS/Results.php

+        if ($allTerms === '') {
+            $hasLimiters = (bool)array_filter(
+                $params->get('filters') ?? [],
+                fn ($filter) => str_starts_with($filter, 'LIMIT')


This filter is necessary because some EDS "filters" are actually "expanders", and presumably those don't count in this analysis.

demiankatz

See below for a suggestion related to your comment; beyond that, I await input from @cwolfebsco before testing further. :-)

demiankatz · 2025-12-20T13:43:20Z

module/VuFind/src/VuFind/Search/EDS/Results.php

        $limit  = $this->getParams()->getLimit();
        $offset = $this->getStartRecord() - 1;
        $params = $this->getParams()->getBackendParameters();
+        if ($allTerms === '') {


It might be worth adding a paramsIncludeLimiter($params) method so you can check if ($allTerms === '' && !$this->paramsIncludeLimiter($params)) {. Due to the way the && operator works, the second clause will only be evaluated if the first clause evaluates to true, so there should be no performance hit for that approach.

cwolfebsco · 2025-12-22T10:58:06Z

@cwolfebsco would appreciate your review as well! Can't tag you above.

I had a look and I think this is ok, I trust you all way more than I trust myself when it comes to these implementations.

Allow me to quickly comment on the expense of the query though. While limiter only searches are technically supported they are quite "expensive" - I often see them take 8 to 12 times longer than other searches, as this short table might be able to confirm (I did run many more tests, of course)

Term	Limiter	TotalSearchTime
platinum		272
platinum	FT:Y	394
	FT:Y	3146
uranium		321
uranium	FT:Y	250
	FT:Y	2916

While it likely is ok to move forward, I believe the user is helped more by a clear guiding error message, rather than a result list with potentially limited real world benefit.

maccabeelevine · 2025-12-22T22:25:08Z

I respect @cwolfebsco 's analysis that it slows down the API call considerably. But there are valid use cases for the facets-only search, as evidenced from the discussion a few months back, as well as the fact that the EDS UI still supports it.

I don't mind adding a warning-style message and still performing the search, if folks think that's helpful. I.e. "Limiter-only queries can be slow. Add search term(s) to improve performance." In theory we could also put up a speed bump ("are you sure you want to do this limiter-only query"), but I would argue against that as it's a valid search.

If we think that level of performance is a problem, I think the solution is caching. In VuFind, the EDS Backend already caches various other interactions besides actual searches, and I could see adding caching for limiter-only queries (if not caching for all queries). Alternatively, if the real concern is server load (not UX), it seems like it's an easy kind of thing to cache on EBSCO's end.

demiankatz · 2025-12-23T12:07:12Z

@maccabeelevine, I don't think that displaying a warning about limiter-only searches slowing things down would be very meaningful in the user interface, since in at least some situations this would be telling end-users about the consequences of an administrator's decision. If we want to warn about that, maybe a better approach would be to make limiter-based searching support "opt in" and discuss the trade-offs in the .ini file comments. We could retain the current 11.0 behavior but have a switch that could be flipped to enable the new behavior.

Regarding caching, I know that @EreMaijala has implemented search result caching for Solr, but I can't remember all the implementation details. Maybe that mechanism could be extended to cover EDS (if it doesn't already; or otherwise it might at least be used as a model).

maccabeelevine · 2025-12-23T14:11:45Z

Regarding caching, I know that @EreMaijala has implemented search result caching for Solr, but I can't remember all the implementation details. Maybe that mechanism could be extended to cover EDS (if it doesn't already; or otherwise it might at least be used as a model).

Yes, thank you, I missed that @EreMaijala had done this in #2406, caching at the Connector level. I tested it on limiter-only quries and it works just fine.

(The Backend also does its own caching to store the getInfo method response, the auth token, and autocomplete data, although its phpdoc mentions only the auth token. And for fun while I was searching, there is also

The RecordCache
A cache for rendered record templates (Cache results of class based template resolution. #1747)
We have a lot of caching...)

maccabeelevine · 2025-12-23T14:12:53Z

If we want to warn about that, maybe a better approach would be to make limiter-based searching support "opt in" and discuss the trade-offs in the .ini file comments. We could retain the current 11.0 behavior but have a switch that could be flipped to enable the new behavior.

I'll implement this, and suggest enabling the cache if they're going to enable limiter-only queries.

maccabeelevine · 2025-12-23T14:53:38Z

If we want to warn about that, maybe a better approach would be to make limiter-based searching support "opt in" and discuss the trade-offs in the .ini file comments. We could retain the current 11.0 behavior but have a switch that could be flipped to enable the new behavior.

I'll implement this, and suggest enabling the cache if they're going to enable limiter-only queries.

I quoted @cwolfebsco 's performance stats and suggested caching. I didn't mention the UX reasons to enable it; it's off by default so if someone bothers to look for the config, they will already have a reason to do so.

maccabeelevine · 2025-12-23T15:00:39Z

module/VuFind/src/VuFind/Search/EDS/ResultsFactory.php

+     *
+     * @var string
+     */
+    protected string $configName = 'EDS';


This is just future-proofing, from lessons learned on Blender2, in case we want to extend it in the future for EPF or whatever else. Currently EPF has no checks on the search query, limiters, etc. @cwolfebsco if that should change, please let me know and we can address it separately.

@maccabeelevine - In /edsapi/publication/search submitting a limiter only query would yield an error 103. Best of my knowledge query is a required parameter for publication search

"DetailedErrorDescription": "Required parameter: query is missing", "ErrorDescription": "Missing Parameter", "ErrorNumber": "103"

I might have misunderstood the question though :) Happy New Year!

Thanks @cwolfebsco . Looks like you're correct on the API but VuFind is still translating empty EPF queries into (FT yes) OR (FT no). I'll deal with that separately: https://openlibraryfoundation.atlassian.net/browse/VUFIND-1816

demiankatz · 2026-01-07T14:32:19Z

Thanks for the progress and clarification here, @maccabeelevine and @cwolfebsco. It sounds to me like we may need to address the EPF issues first in a separate PR and then circle back to this one. Would you agree? In any case, please let me know if/when you need me to do more reviewing here or elsewhere!

maccabeelevine · 2026-01-15T15:53:00Z

Thanks for the progress and clarification here, @maccabeelevine and @cwolfebsco. It sounds to me like we may need to address the EPF issues first in a separate PR and then circle back to this one. Would you agree? In any case, please let me know if/when you need me to do more reviewing here or elsewhere!

@cwolfebsco and I are discussing EPF in https://openlibraryfoundation.atlassian.net/browse/VUFIND-1816

I don't think this EDS PR depends on the outcome of the EPF discussion as the default-limiting logic is EDS-only.

I've added a unit test, but some more hands-on testing would not hurt. Otherwise I just defer to @cwolfebsco on whether this is ready.

cwolfebsco · 2026-01-16T10:49:05Z

I had a look, looked at the PR on a virtual machine and found it to work as expected. I think this is ready @maccabeelevine and @demiankatz . Thank you so much for being considered, I really appreciate the comments in the EDS.ini for that feature making its impact clear.

ThoWagen · 2026-01-16T15:36:28Z

I just tested this and the code looks good to me.

But I had to perform a search with a non-empty query string, select a filter and then remove the query string. Did I miss an easier way to get to this? If no, I guess, normal users probably will not know, how to do it.

cwolfebsco · 2026-01-16T15:44:35Z

@ThoWagen - EBSCOadmin enables you to define limiters that are applied by default. If that's your setup, this approach would work just fine.

EDS: Allow limiter-only queries

1a878d9

maccabeelevine commented Dec 18, 2025

View reviewed changes

Use a more specific error message

6cc4eea

Fix alphabetical order

9534569

maccabeelevine requested review from ThoWagen and demiankatz December 18, 2025 17:58

maccabeelevine marked this pull request as ready for review December 18, 2025 17:59

maccabeelevine commented Dec 18, 2025

View reviewed changes

demiankatz requested changes Dec 20, 2025

View reviewed changes

Add paramsIncludeLimiter function

84ca859

Enable based on config

dbc89db

maccabeelevine commented Dec 23, 2025

View reviewed changes

maccabeelevine requested a review from demiankatz December 23, 2025 15:01

maccabeelevine added 2 commits January 15, 2026 15:26

Fix limiter logic to respect regular facet filters as limiters

6a90d3b

Add tests

3270878

Let Rector have its fun

0198728

EDS: Allow limiter-only queries #4976

Are you sure you want to change the base?

EDS: Allow limiter-only queries #4976

Conversation

maccabeelevine commented Dec 18, 2025

Uh oh!

maccabeelevine Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

demiankatz Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

maccabeelevine commented Dec 18, 2025

Uh oh!

maccabeelevine commented Dec 18, 2025

Uh oh!

maccabeelevine Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

demiankatz left a comment

Choose a reason for hiding this comment

Uh oh!

demiankatz Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

cwolfebsco commented Dec 22, 2025

Uh oh!

maccabeelevine commented Dec 22, 2025

Uh oh!

demiankatz commented Dec 23, 2025

Uh oh!

maccabeelevine commented Dec 23, 2025

Uh oh!

maccabeelevine commented Dec 23, 2025

Uh oh!

maccabeelevine commented Dec 23, 2025

Uh oh!

maccabeelevine Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

cwolfebsco Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

maccabeelevine Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

demiankatz commented Jan 7, 2026

Uh oh!

maccabeelevine commented Jan 15, 2026

Uh oh!

cwolfebsco commented Jan 16, 2026

Uh oh!

ThoWagen commented Jan 16, 2026

Uh oh!

cwolfebsco commented Jan 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

maccabeelevine Dec 29, 2025 •

edited

Loading