Bug 12478 Increasing test Coverage for Koha::SearchEngine::Elasticsearch::Search
[koha-ffzg.git] / Koha / SearchEngine / Elasticsearch / Search.pm
index dd84cd7..cfdad81 100644 (file)
@@ -23,8 +23,10 @@ Koha::SearchEngine::ElasticSearch::Search - search functions for Elasticsearch
 
 =head1 SYNOPSIS
 
-    my $searcher = Koha::SearchEngine::ElasticSearch::Search->new();
-    my $builder = Koha::SearchEngine::Elasticsearch::QueryBuilder->new();
+    my $searcher =
+      Koha::SearchEngine::ElasticSearch::Search->new( { index => $index } );
+    my $builder = Koha::SearchEngine::Elasticsearch::QueryBuilder->new(
+        { index => $index } );
     my $query = $builder->build_query('perl');
     my $results = $searcher->search($query);
     print "There were " . $results->total . " results.\n";
@@ -37,8 +39,11 @@ Koha::SearchEngine::ElasticSearch::Search - search functions for Elasticsearch
 =cut
 
 use base qw(Koha::ElasticSearch);
+use C4::Context;
 use Koha::ItemTypes;
-
+use Koha::AuthorisedValues;
+use Koha::SearchEngine::QueryBuilder;
+use MARC::Record;
 use Catmandu::Store::ElasticSearch;
 
 use Data::Dumper; #TODO remove
@@ -86,10 +91,15 @@ sub search {
     $self->store(
         Catmandu::Store::ElasticSearch->new(
             %$params,
-            trace_calls => 1,
         )
-    );
-    my $results = $self->store->bag->search( %$query, %paging );
+    ) unless $self->store;
+    my $error;
+    my $results = eval {
+        $self->store->bag->search( %$query, %paging );
+    };
+    if ($@) {
+        die $self->process_error($@);
+    }
     return $results;
 }
 
@@ -103,14 +113,15 @@ faster than pulling all the data in, ususally.
 =cut
 
 sub count {
-    my ($self, $query) = @_;
+    my ( $self, $query ) = @_;
 
     my $params = $self->get_elasticsearch_params();
     $self->store(
-        Catmandu::Store::ElasticSearch->new( %$params, trace_calls => 1, ) );
-#    TODO something like this should work, but doesn't seem to just yet.
-#    my $count = $self->store->bag->count($query);
-    my $count = $self->store->bag->search(%$query)->total;
+        Catmandu::Store::ElasticSearch->new( %$params, trace_calls => 0, ) )
+      unless $self->store;
+
+    my $search = $self->store->bag->search( %$query);
+    my $count = $search->total() || 0;
     return $count;
 }
 
@@ -124,7 +135,7 @@ sub count {
 
 A search interface somewhat compatible with L<C4::Search->getRecords>. Anything
 that is returned in the query created by build_query_compat will probably
-get ignored here.
+get ignored here, along with some other things (like C<@servers>.)
 
 =cut
 
@@ -137,6 +148,7 @@ sub search_compat {
 
     my %options;
     $options{offset} = $offset;
+    $options{expanded_facet} = $expanded_facet;
     my $results = $self->search($query, undef, $results_per_page, %options);
 
     # Convert each result into a MARC::Record
@@ -154,7 +166,7 @@ sub search_compat {
     my %result;
     $result{biblioserver}{hits} = $results->total;
     $result{biblioserver}{RECORDS} = \@records;
-    return (undef, \%result, $self->_convert_facets($results->{facets}));
+    return (undef, \%result, $self->_convert_facets($results->{facets}, $expanded_facet));
 }
 
 =head2 search_auth_compat
@@ -186,7 +198,7 @@ sub search_auth_compat {
             # rather than hard-coded conversions.
             # Our results often come through as nested arrays, to fix this
             # requires changes in catmandu.
-            my $authid = $record->{ 'Local-Number' }[0][0];
+            my $authid = $record->{ 'Local-number' }[0][0];
             $result{authid} = $authid;
 
             # TODO put all this info into the record at index time so we
@@ -253,7 +265,80 @@ sub count_auth_use {
     $bib_searcher->count($query);
 }
 
+=head2 simple_search_compat
+
+    my ( $error, $marcresults, $total_hits ) =
+      $searcher->simple_search( $query, $offset, $max_results );
+
+This is a simpler interface to the searching, intended to be similar enough to
+L<C4::Search::SimpleSearch>.
+
+Arguments:
+
+=over 4
+
+=item C<$query>
+
+A thing to search for. It could be a simple string, or something constructed
+with the appropriate QueryBuilder module.
+
+=item C<$offset>
+
+How many results to skip from the start of the results.
+
+=item C<$max_results>
+
+The max number of results to return. The default is 100 (because unlimited
+is a pretty terrible thing to do.)
+
+=back
+
+Returns:
+
+=over 4
+
+=item C<$error>
+
+if something went wrong, this'll contain some kind of error
+message.
+
+=item C<$marcresults>
 
+an arrayref of MARC::Records (note that this is different from the
+L<C4::Search> version which will return plain XML, but too bad.)
+
+=item C<$total_hits>
+
+the total number of results that this search could have returned.
+
+=back
+
+=cut
+
+sub simple_search_compat {
+    my ($self, $query, $offset, $max_results) = @_;
+
+    return ('No query entered', undef, undef) unless $query;
+
+    my %options;
+    $options{offset} = $offset // 0;
+    $max_results //= 100;
+
+    unless (ref $query) {
+        # We'll push it through the query builder to sanitise everything.
+        my $qb = Koha::SearchEngine::QueryBuilder->new({index => $self->index});
+        (undef,$query) = $qb->build_query_compat(undef, [$query]);
+    }
+    my $results = $self->search($query, undef, $max_results, %options);
+    my @records;
+    $results->each(sub {
+            # The results come in an array for some reason
+            my $marc_json = @_[0]->{record};
+            my $marc = $self->json2marc($marc_json);
+            push @records, $marc;
+        });
+    return (undef, \@records, $results->total);
+}
 
 =head2 json2marc
 
@@ -288,49 +373,65 @@ sub json2marc {
 
 =head2 _convert_facets
 
-    my $koha_facets = _convert_facets($es_facets);
+    my $koha_facets = _convert_facets($es_facets, $expanded_facet);
 
 Converts elasticsearch facets types to the form that Koha expects.
 It expects the ES facet name to match the Koha type, for example C<itype>,
 C<au>, C<su-to>, etc.
 
+C<$expanded_facet> is the facet that we want to show FacetMaxCount entries for, rather
+than just 5 like normal.
+
 =cut
 
 sub _convert_facets {
-    my ( $self, $es ) = @_;
+    my ( $self, $es, $exp_facet ) = @_;
 
     return undef if !$es;
 
     # These should correspond to the ES field names, as opposed to the CCL
     # things that zebra uses.
+    # TODO let the library define the order using the interface.
     my %type_to_label = (
-        author   => 'Authors',
-        location => 'Location',
-        itype    => 'ItemTypes',
-        se       => 'Series',
-        subject  => 'Topics',
-        'su-geo' => 'Places',
+        author   => { order => 1, label => 'Authors', },
+        itype    => { order => 2, label => 'ItemTypes', },
+        location => { order => 3, label => 'Location', },
+        'su-geo' => { order => 4, label => 'Places', },
+        se       => { order => 5, label => 'Series', },
+        subject  => { order => 6, label => 'Topics', },
     );
 
     # We also have some special cases, e.g. itypes that need to show the
     # value rather than the code.
-    my $itypes = Koha::ItemTypes->new();
-    my %special = ( itype => sub { $itypes->get_description_for_code(@_) }, );
-    my @res;
+    my @itypes = Koha::ItemTypes->search;
+    my @locations = Koha::AuthorisedValues->search( { category => 'LOC' } );
+    my $opac = C4::Context->interface eq 'opac' ;
+    my %special = (
+        itype    => { map { $_->itemtype         => $_->description } @itypes },
+        location => { map { $_->authorised_value => ( $opac ? ( $_->lib_opac || $_->lib ) : $_->lib ) } @locations },
+    );
+    my @facets;
+    $exp_facet //= '';
     while ( ( $type, $data ) = each %$es ) {
         next if !exists( $type_to_label{$type} );
+
+        # We restrict to the most popular $limit !results
+        my $limit = ( $type eq $exp_facet ) ? C4::Context->preference('FacetMaxCount') : 5;
         my $facet = {
-            type_id => $type . '_id',
-            expand  => $type,
-            expandable => 1,    # TODO figure how that's supposed to work
-            "type_label_$type_to_label{$type}" => 1,
+            type_id    => $type . '_id',
+            expand     => $type,
+            expandable => ( $type ne $exp_facet )
+              && ( @{ $data->{terms} } > $limit ),
+            "type_label_$type_to_label{$type}{label}" => 1,
             type_link_value                    => $type,
+            order      => $type_to_label{$type}{order},
         };
-        foreach my $term ( @{ $data->{terms} } ) {
+        $limit = @{ $data->{terms} } if ( $limit > @{ $data->{terms} } );
+        foreach my $term ( @{ $data->{terms} }[ 0 .. $limit - 1 ] ) {
             my $t = $term->{term};
             my $c = $term->{count};
             if ( exists( $special{$type} ) ) {
-                $label = $special{$type}->($t);
+                $label = $special{$type}->{$t} // $t;
             }
             else {
                 $label = $t;
@@ -339,14 +440,16 @@ sub _convert_facets {
                 facet_count       => $c,
                 facet_link_value  => $t,
                 facet_title_value => $t . " ($c)",
-                facet_label_value => $label,    # TODO either truncate this,
+                facet_label_value => $label,        # TODO either truncate this,
                      # or make the template do it like it should anyway
                 type_link_value => $type,
             };
         }
-        push @res, $facet if exists $facet->{facets};
+        push @facets, $facet if exists $facet->{facets};
     }
-    return \@res;
+
+    @facets = sort { $a->{order} cmp $b->{order} } @facets;
+    return \@facets;
 }