From 94e7bbe48bfd35bdd6d62c37e58642d4242ae29d Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 27 May 2024 23:37:01 +0000 Subject: [PATCH] Bug 36947: Do a locale-based sort for ES facet names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change uses a configurable locale-based collator to sort the ES facet names. Test plan: 0. Apply the patch 1. vi /etc/locale.gen 2. Uncomment the locale you want to generate (e.g. fi_FI.UTF-8 UTF-8) 3. locale-gen 4. vi "/etc/default/koha-common" 5. Add the following to the bottom of the file: export LC_ALL=fi_FI.UTF-8 6. koha-plack --restart koha-common 7. Setup some test records with authors with accented and unaccented names, and different cases for the lead letter e.g. Aa author, Åa author2, aa author, étienne 8. Switch to using Elasticsearch and reindex koha-elasticsearch -b -v --rebuild kohadev 9. Do a test search e.g. http://localhost:8081/cgi-bin/koha/catalogue/search.pl?q=test 10. Confirm the facet names are sorted in ascending order following Finnish collation rules e.g. aa author Aa author étienne Farley, David Humble, Jez Martin, Robert C. Åa author NOTE: Any collation and language can be used. Finnish is just an example of a Latin-based script which has a different alphabetical ordering than just A-Z --- Koha/SearchEngine/Elasticsearch/Search.pm | 40 ++++++++++++- t/Koha/SearchEngine/Elasticsearch/Search.t | 69 +++++++++++++++++++++- 2 files changed, 106 insertions(+), 3 deletions(-) diff --git a/Koha/SearchEngine/Elasticsearch/Search.pm b/Koha/SearchEngine/Elasticsearch/Search.pm index 42326e3573..b57d506575 100644 --- a/Koha/SearchEngine/Elasticsearch/Search.pm +++ b/Koha/SearchEngine/Elasticsearch/Search.pm @@ -54,6 +54,9 @@ use MARC::File::XML; use MIME::Base64 qw( decode_base64 ); use JSON; +use POSIX qw(setlocale LC_COLLATE); +use Unicode::Collate::Locale; + Koha::SearchEngine::Elasticsearch::Search->mk_accessors(qw( store )); =head2 search @@ -427,6 +430,37 @@ sub max_result_window { return $max_result_window; } + +=head2 _sort_facets + + my $facets = _sort_facets($facets); + +Sorts facets using a locale. + +=cut + +sub _sort_facets { + my ( $self, $args ) = @_; + my $facets = $args->{facets}; + my $locale = $args->{locale}; + if ( !$locale ) { + + #NOTE: When setlocale is run with only the 1st parameter, it is a "get" not a "set" function. + $locale = setlocale(LC_COLLATE); + } + my $collator = Unicode::Collate::Locale->new( locale => $locale ); + if ( $collator && $facets ) { + my @sorted_facets = sort { $collator->cmp( $a->{facet_label_value}, $b->{facet_label_value} ) } @{$facets}; + if (@sorted_facets) { + return \@sorted_facets; + } + } + + #NOTE: If there was a problem, at least return the not sorted facets + return $facets; +} + + =head2 _convert_facets my $koha_facets = _convert_facets($es_facets); @@ -504,8 +538,10 @@ sub _convert_facets { }; } if( C4::Context->preference('FacetOrder') eq 'Alphabetical' ){ - @{ $facet->{facets} } = - sort { $a->{facet_label_value} cmp $b->{facet_label_value} } @{ $facet->{facets} }; + my $sorted_facets = $self->_sort_facets( { facets => $facet->{facets} } ); + if ($sorted_facets) { + $facet->{facets} = $sorted_facets; + } } push @facets, $facet if exists $facet->{facets}; } diff --git a/t/Koha/SearchEngine/Elasticsearch/Search.t b/t/Koha/SearchEngine/Elasticsearch/Search.t index cae1b2a96e..1353f1ee1c 100755 --- a/t/Koha/SearchEngine/Elasticsearch/Search.t +++ b/t/Koha/SearchEngine/Elasticsearch/Search.t @@ -17,12 +17,79 @@ use Modern::Perl; -use Test::More tests => 2; +use Test::More tests => 3; use Test::MockModule; use t::lib::Mocks; +use utf8; + use_ok('Koha::SearchEngine::Elasticsearch::Search'); +subtest '_sort_facets' => sub { + plan tests => 2; + t::lib::Mocks::mock_preference( 'SearchEngine', 'Elasticsearch' ); + my $facets = [ + { facet_label_value => 'Mary' }, + { facet_label_value => 'Harry' }, + { facet_label_value => 'Fairy' }, + { facet_label_value => 'Ari' }, + { facet_label_value => 'mary' }, + { facet_label_value => 'harry' }, + { facet_label_value => 'fairy' }, + { facet_label_value => 'ari' }, + { facet_label_value => 'étienne' }, + { facet_label_value => 'Åuthor' }, + ]; + + my @normal_sort_facets = sort { $a->{facet_label_value} cmp $b->{facet_label_value} } @$facets; + my @normal_expected_facets = ( + { facet_label_value => 'Ari' }, + { facet_label_value => 'Fairy' }, + { facet_label_value => 'Harry' }, + { facet_label_value => 'Mary' }, + { facet_label_value => 'ari' }, + { facet_label_value => 'fairy' }, + { facet_label_value => 'harry' }, + { facet_label_value => 'mary' }, + { facet_label_value => 'Åuthor' }, + { facet_label_value => 'étienne' }, + ); + + #NOTE: stringwise/bytewise is not UTF-8 friendly + is_deeply( \@normal_sort_facets, \@normal_expected_facets, "Perl's built-in sort is stringwise/bytewise." ); + + my $search = Koha::SearchEngine::Elasticsearch::Search->new( + { index => $Koha::SearchEngine::Elasticsearch::AUTHORITIES_INDEX } ); + + #NOTE: The 'default' locale uses the Default Unicode Collation Element Table, which + #is used for the locales of English (en) and French (fr). + my $sorted_facets = $search->_sort_facets( { facets => $facets, locale => 'default' } ); + my $expected = [ + { facet_label_value => 'ari' }, + { facet_label_value => 'Ari' }, + { facet_label_value => 'Åuthor' }, + { facet_label_value => 'étienne' }, + { facet_label_value => 'fairy' }, + { facet_label_value => 'Fairy' }, + { facet_label_value => 'harry' }, + { facet_label_value => 'Harry' }, + { facet_label_value => 'mary' }, + { facet_label_value => 'Mary' }, + ]; + is_deeply( $sorted_facets, $expected, "Facets sorted correctly with default locale" ); + + #NOTE: If "locale" is not provided to _sort_facets, it will look up the LC_COLLATE + #for the local system. This is what allows this function to work well in production. + #However, since LC_COLLATE could vary from system to system running these unit tests, + #we can't test arbitrary locales here. + + #TODO: It would be great to explicitly use a fi_FI locale for another test, but we cannot guarantee + #that every system has that locale. That said, perhaps we could do some conditional unit + #testing if certain locales are available when running tests. This could allow for the Koha community + #to thoroughly test this functionality, while letting anyone run the unit tests regardless of their + #installed locales. +}; + subtest 'search_auth_compat' => sub { plan tests => 4; -- 2.39.2