Bugzilla – Attachment 136419 Details for
Bug 31013
Reserved words as branchcodes cause search error in Elasticsearch
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 31013: Quote branchcodes in Elasticsearch limits
Bug-31013-Quote-branchcodes-in-Elasticsearch-limit.patch (text/plain), 7.87 KB, created by
Martin Renvoize (ashimema)
on 2022-06-22 13:56:05 UTC
(
hide
)
Description:
Bug 31013: Quote branchcodes in Elasticsearch limits
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2022-06-22 13:56:05 UTC
Size:
7.87 KB
patch
obsolete
>From 31f4ba965229e6761e6fa4650984d21348d9b62e Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Wed, 22 Jun 2022 13:19:03 +0000 >Subject: [PATCH] Bug 31013: Quote branchcodes in Elasticsearch limits > >This patch adds quoting when handling branchcodes in searching in order >to prevent errors when branchcodes are reserved words in ES > >To test: >0 - Be using Elasticsearch with Koha >1 - Add a new branch code:OR name:Orly >2 - Add an item to this branch >3 - Use advanced search to limit search to only Orly >4 - Oh really bad, the search fails! >5 - Apply patch >6 - Repeat search >7 - Oh really good, the search succeeds >8 - prove t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t > >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >--- > .../Elasticsearch/QueryBuilder.pm | 9 ++++--- > .../SearchEngine/Elasticsearch/QueryBuilder.t | 26 +++++++++---------- > 2 files changed, 18 insertions(+), 17 deletions(-) > >diff --git a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm >index dba475eb3c..b6a6b9a786 100644 >--- a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm >+++ b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm >@@ -1080,16 +1080,17 @@ sub _fix_limit_special_cases { > } > > if (@branchcodes) { >+ # We quote the branchcodes here to prevent issues when codes are reserved words in ES, e.g. OR, AND, NOT, etc. > if ( $branchfield eq "homebranch" ) { >- push @new_lim, sprintf "(%s)", join " OR ", map { 'homebranch: ' . $_ } @branchcodes; >+ push @new_lim, sprintf "(%s)", join " OR ", map { 'homebranch: "' . $_ . '"' } @branchcodes; > } > elsif ( $branchfield eq "holdingbranch" ) { >- push @new_lim, sprintf "(%s)", join " OR ", map { 'holdingbranch: ' . $_ } @branchcodes; >+ push @new_lim, sprintf "(%s)", join " OR ", map { 'holdingbranch: "' . $_ . '"' } @branchcodes; > } > else { > push @new_lim, sprintf "(%s OR %s)", >- join( " OR ", map { 'homebranch: ' . $_ } @branchcodes ), >- join( " OR ", map { 'holdingbranch: ' . $_ } @branchcodes ); >+ join( " OR ", map { 'homebranch: "' . $_ . '"' } @branchcodes ), >+ join( " OR ", map { 'holdingbranch: "' . $_ . '"' } @branchcodes ); > } > } > } >diff --git a/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t b/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t >index b4e2de938c..c032bf8c91 100755 >--- a/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t >+++ b/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t >@@ -725,37 +725,37 @@ subtest 'build_query_compat() SearchLimitLibrary tests' => sub { > t::lib::Mocks::mock_preference('SearchLimitLibrary', 'both'); > my ( undef, undef, undef, undef, undef, $limit, $limit_cgi, $limit_desc, undef ) = > $query_builder->build_query_compat( undef, undef, undef, [ "branch:CPL" ], undef, undef, undef, undef ); >- is( $limit, "(homebranch: CPL OR holdingbranch: CPL)", "Branch limit expanded to home/holding branch"); >- is( $limit_desc, "(homebranch: CPL OR holdingbranch: CPL)", "Limit description correctly expanded"); >- is( $limit_cgi, "&limit=branch%3ACPL", "Limit cgi does not get expanded"); >+ is( $limit, '(homebranch: "CPL" OR holdingbranch: "CPL")', "Branch limit expanded to home/holding branch"); >+ is( $limit_desc, '(homebranch: "CPL" OR holdingbranch: "CPL")', "Limit description correctly expanded"); >+ is( $limit_cgi, '&limit=branch%3ACPL', "Limit cgi does not get expanded"); > ( undef, undef, undef, undef, undef, $limit, $limit_cgi, $limit_desc, undef ) = > $query_builder->build_query_compat( undef, undef, undef, [ "multibranchlimit:$groupid" ], undef, undef, undef, undef ); >- is( $limit, "(homebranch: $branchcodes[0] OR homebranch: $branchcodes[1] OR holdingbranch: $branchcodes[0] OR holdingbranch: $branchcodes[1])", "Multibranch limit expanded to home/holding branches"); >- is( $limit_desc, "(homebranch: $branchcodes[0] OR homebranch: $branchcodes[1] OR holdingbranch: $branchcodes[0] OR holdingbranch: $branchcodes[1])", "Multibranch limit description correctly expanded"); >+ is( $limit, "(homebranch: \"$branchcodes[0]\" OR homebranch: \"$branchcodes[1]\" OR holdingbranch: \"$branchcodes[0]\" OR holdingbranch: \"$branchcodes[1]\")", "Multibranch limit expanded to home/holding branches"); >+ is( $limit_desc, "(homebranch: \"$branchcodes[0]\" OR homebranch: \"$branchcodes[1]\" OR holdingbranch: \"$branchcodes[0]\" OR holdingbranch: \"$branchcodes[1]\")", "Multibranch limit description correctly expanded"); > is( $limit_cgi, "&limit=multibranchlimit%3A$groupid", "Multibranch limit cgi does not get expanded"); > > t::lib::Mocks::mock_preference('SearchLimitLibrary', 'homebranch'); > ( undef, undef, undef, undef, undef, $limit, $limit_cgi, $limit_desc, undef ) = > $query_builder->build_query_compat( undef, undef, undef, [ "branch:CPL" ], undef, undef, undef, undef ); >- is( $limit, "(homebranch: CPL)", "branch limit expanded to home branch"); >- is( $limit_desc, "(homebranch: CPL)", "limit description correctly expanded"); >+ is( $limit, "(homebranch: \"CPL\")", "branch limit expanded to home branch"); >+ is( $limit_desc, "(homebranch: \"CPL\")", "limit description correctly expanded"); > is( $limit_cgi, "&limit=branch%3ACPL", "limit cgi does not get expanded"); > ( undef, undef, undef, undef, undef, $limit, $limit_cgi, $limit_desc, undef ) = > $query_builder->build_query_compat( undef, undef, undef, [ "multibranchlimit:$groupid" ], undef, undef, undef, undef ); >- is( $limit, "(homebranch: $branchcodes[0] OR homebranch: $branchcodes[1])", "branch limit expanded to home branch"); >- is( $limit_desc, "(homebranch: $branchcodes[0] OR homebranch: $branchcodes[1])", "limit description correctly expanded"); >+ is( $limit, "(homebranch: \"$branchcodes[0]\" OR homebranch: \"$branchcodes[1]\")", "branch limit expanded to home branch"); >+ is( $limit_desc, "(homebranch: \"$branchcodes[0]\" OR homebranch: \"$branchcodes[1]\")", "limit description correctly expanded"); > is( $limit_cgi, "&limit=multibranchlimit%3A$groupid", "Limit cgi does not get expanded"); > > t::lib::Mocks::mock_preference('SearchLimitLibrary', 'holdingbranch'); > ( undef, undef, undef, undef, undef, $limit, $limit_cgi, $limit_desc, undef ) = > $query_builder->build_query_compat( undef, undef, undef, [ "branch:CPL" ], undef, undef, undef, undef ); >- is( $limit, "(holdingbranch: CPL)", "branch limit expanded to holding branch"); >- is( $limit_desc, "(holdingbranch: CPL)", "Limit description correctly expanded"); >+ is( $limit, "(holdingbranch: \"CPL\")", "branch limit expanded to holding branch"); >+ is( $limit_desc, "(holdingbranch: \"CPL\")", "Limit description correctly expanded"); > is( $limit_cgi, "&limit=branch%3ACPL", "Limit cgi does not get expanded"); > ( undef, undef, undef, undef, undef, $limit, $limit_cgi, $limit_desc, undef ) = > $query_builder->build_query_compat( undef, undef, undef, [ "multibranchlimit:$groupid" ], undef, undef, undef, undef ); >- is( $limit, "(holdingbranch: $branchcodes[0] OR holdingbranch: $branchcodes[1])", "branch limit expanded to holding branch"); >- is( $limit_desc, "(holdingbranch: $branchcodes[0] OR holdingbranch: $branchcodes[1])", "Limit description correctly expanded"); >+ is( $limit, "(holdingbranch: \"$branchcodes[0]\" OR holdingbranch: \"$branchcodes[1]\")", "branch limit expanded to holding branch"); >+ is( $limit_desc, "(holdingbranch: \"$branchcodes[0]\" OR holdingbranch: \"$branchcodes[1]\")", "Limit description correctly expanded"); > is( $limit_cgi, "&limit=multibranchlimit%3A$groupid", "Limit cgi does not get expanded"); > > }; >-- >2.20.1
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=136419">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 31013
:
136412
| 136419