Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 31013: Quote branchcodes in Elasticsearch limits

Browse source 
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>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(cherry picked from commit a4c5bd5836)

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Nick Clemens 2022-06-22 13:19:03 +00:00 committed by Lucas Gass
parent 4018461d49
commit 584c49961c
2 changed files with 18 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
View file

@ -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 );
}
}
}

26
t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t
View file

@ -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");
};