Fix issues with too long sets of work keys in queries and URLs #3087
Conversation
The terms query should also provide better performance.
Work keys query is now a separate query type.
There was a problem hiding this comment.
I haven't tried this hands-on yet, but it looks logical to me. I've left a handful of minor comments.
This might also be a good time to review the Mink RecordVersionsTest and make sure it's covering everything that it should, if you have a moment... want to be sure we can reliably use that as a before-and-after to be confident that the refactoring hasn't obviously changed/broken anything.
| } | ||
| $params->set('q', implode(' OR ', $query)); | ||
| $params->set('q', '{!terms f=work_keys_str_mv}' . implode(',', $terms)); |
There was a problem hiding this comment.
Can we guarantee that no term will ever contain a comma?
There was a problem hiding this comment.
No. I thought escaping the delimiter above would be enough, but now that I checked it, that doesn't work. I'll think about an alternative.
There was a problem hiding this comment.
I changed the delimiter to chr(1F) and removed the useless escaping (verified manually with Solr that all the characters can be in a term as is).
There was a problem hiding this comment.
This is looking good to me now, and all tests are passing. Thanks for the fix, @EreMaijala!
No description provided.