From 2b808b2e7b493a9483f238551ae3c38f5d7703ad Mon Sep 17 00:00:00 2001 From: Mike Rylander Date: Thu, 25 Aug 2016 17:48:02 -0400 Subject: [PATCH] LP#1005040: implement business logic This patch gut most of the top level Search/Biblio.pm wrapper, inlines opensearch search params, uses the new dispach method, for OpenSRF subrequests, and return the abstract query when requested. It also adds CDBI classes for asset.copy_location_group which is needed for looking them up at search time. Signed-off-by: Mike Rylander Signed-off-by: Galen Charlton --- .../lib/OpenILS/Application/Search/Biblio.pm | 173 ++++-------------- .../Application/Storage/Publisher/metabib.pm | 29 ++- .../Application/Storage/QueryParser.pm | 3 +- .../src/perlmods/lib/OpenILS/WWW/SuperCat.pm | 15 +- 4 files changed, 61 insertions(+), 159 deletions(-) diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Search/Biblio.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Search/Biblio.pm index 5c077903f4..f595c28b9c 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Search/Biblio.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Search/Biblio.pm @@ -829,141 +829,21 @@ __PACKAGE__->register_method( } sub multiclass_query { + # arghash only really supports limit/offset anymore my($self, $conn, $arghash, $query, $docache) = @_; - $logger->debug("initial search query => $query"); - my $orig_query = $query; - - $query =~ s/\+/ /go; - $query =~ s/^\s+//go; - - # convert convenience classes (e.g. kw for keyword) to the full class name - # ensure that the convenience class isn't part of a word (e.g. 'playhouse') - $query =~ s/(^|\s)kw(:|\|)/$1keyword$2/go; - $query =~ s/(^|\s)ti(:|\|)/$1title$2/go; - $query =~ s/(^|\s)au(:|\|)/$1author$2/go; - $query =~ s/(^|\s)su(:|\|)/$1subject$2/go; - $query =~ s/(^|\s)se(:|\|)/$1series$2/go; - $query =~ s/(^|\s)name(:|\|)/$1author$2/og; - - $logger->debug("cleansed query string => $query"); - my $search = {}; - - my $simple_class_re = qr/((?:\w+(?:\|\w+)?):[^:]+?)$/; - my $class_list_re = qr/(?:keyword|title|author|subject|series)/; - my $modifier_list_re = qr/(?:site|dir|sort|lang|available|preflib)/; - - my $tmp_value = ''; - while ($query =~ s/$simple_class_re//so) { - - my $qpart = $1; - my $where = index($qpart,':'); - my $type = substr($qpart, 0, $where++); - my $value = substr($qpart, $where); - - if ($type !~ /^(?:$class_list_re|$modifier_list_re)/o) { - $tmp_value = "$qpart $tmp_value"; - next; - } - - if ($type =~ /$class_list_re/o ) { - $value .= $tmp_value; - $tmp_value = ''; - } - - next unless $type and $value; - - $value =~ s/^\s*//og; - $value =~ s/\s*$//og; - $type = 'sort_dir' if $type eq 'dir'; - - if($type eq 'site') { - # 'site' is the org shortname. when using this, we also want - # to search at the requested org's depth - my $e = new_editor(); - if(my $org = $e->search_actor_org_unit({shortname => $value})->[0]) { - $arghash->{org_unit} = $org->id if $org; - $arghash->{depth} = $e->retrieve_actor_org_unit_type($org->ou_type)->depth; - } else { - $logger->warn("'site:' query used on invalid org shortname: $value ... ignoring"); - } - } elsif($type eq 'pref_ou') { - # 'pref_ou' is the preferred org shortname. - my $e = new_editor(); - if(my $org = $e->search_actor_org_unit({shortname => $value})->[0]) { - $arghash->{pref_ou} = $org->id if $org; - } else { - $logger->warn("'pref_ou:' query used on invalid org shortname: $value ... ignoring"); - } - - } elsif($type eq 'available') { - # limit to available - $arghash->{available} = 1 unless $value eq 'false' or $value eq '0'; - - } elsif($type eq 'lang') { - # collect languages into an array of languages - $arghash->{language} = [] unless $arghash->{language}; - push(@{$arghash->{language}}, $value); - - } elsif($type =~ /^sort/o) { - # sort and sort_dir modifiers - $arghash->{$type} = $value; - - } else { - # append the search term to the term under construction - $search->{$type} = {} unless $search->{$type}; - $search->{$type}->{term} = - ($search->{$type}->{term}) ? $search->{$type}->{term} . " $value" : $value; - } - } - - $query .= " $tmp_value"; - $query =~ s/\s+/ /go; - $query =~ s/^\s+//go; - $query =~ s/\s+$//go; - - my $type = $arghash->{default_class} || 'keyword'; - $type = ($type eq '-') ? 'keyword' : $type; - $type = ($type !~ /^(title|author|keyword|subject|series)(?:\|\w+)?$/o) ? 'keyword' : $type; - - if($query) { - # This is the front part of the string before any special tokens were - # parsed OR colon-separated strings that do not denote a class. - # Add this data to the default search class - $search->{$type} = {} unless $search->{$type}; - $search->{$type}->{term} = - ($search->{$type}->{term}) ? $search->{$type}->{term} . " $query" : $query; + if ($query) { + $query =~ s/\+/ /go; + $query =~ s/^\s+//go; + $query =~ s/\s+/ /go; + $arghash->{query} = $query } - my $real_search = $arghash->{searches} = { $type => { term => $orig_query } }; - - # capture the original limit because the search method alters the limit internally - my $ol = $arghash->{limit}; - - my $sclient = OpenSRF::Utils::SettingsClient->new; - - (my $method = $self->api_name) =~ s/\.query//o; - $method =~ s/multiclass/multiclass.staged/ - if $sclient->config_value(apps => 'open-ils.search', - app_settings => 'use_staged_search') =~ /true/i; + $logger->debug("initial search query => $query") if $query; - # XXX This stops the session locale from doing the right thing. - # XXX Revisit this and have it translate to a lang instead of a locale. - #$arghash->{preferred_language} = $U->get_org_locale($arghash->{org_unit}) - # unless $arghash->{preferred_language}; + (my $method = $self->api_name) =~ s/\.query/.staged/o; + return $self->method_lookup($method)->dispatch($arghash, $docache); - $method = $self->method_lookup($method); - my ($data) = $method->run($arghash, $docache); - - $arghash->{searches} = $search if (!$data->{complex_query}); - - $arghash->{limit} = $ol if $ol; - $data->{compiled_search} = $arghash; - $data->{query} = $orig_query; - - $logger->info("compiled search is " . OpenSRF::Utils::JSON->perl2JSON($arghash)); - - return $data; } __PACKAGE__->register_method( @@ -1249,6 +1129,7 @@ __PACKAGE__->register_method( signature => q/The .staff search includes hidden bibs, hidden items and bibs with no items. Otherwise, @see open-ils.search.biblio.multiclass.staged/ ); +my $estimation_strategy; sub staged_search { my($self, $conn, $search_hash, $docache) = @_; @@ -1261,10 +1142,12 @@ sub staged_search { $method .= '.staff' if $self->api_name =~ /staff$/; $method .= '.atomic'; - return {count => 0} unless ( - $search_hash and - $search_hash->{searches} and - scalar( keys %{$search_hash->{searches}} )); + if (!$search_hash{query}) { + return {count => 0} unless ( + $search_hash and + $search_hash->{searches} and + scalar( keys %{$search_hash->{searches}} )); + } my $search_duration; my $user_offset = $search_hash->{offset} || 0; # user-specified offset @@ -1285,11 +1168,13 @@ sub staged_search { $search_hash->{core_limit} = $superpage_size * $max_superpages; # Set the configured estimation strategy, defaults to 'inclusion'. - my $estimation_strategy = OpenSRF::Utils::SettingsClient - ->new - ->config_value( - apps => 'open-ils.search', app_settings => 'estimation_strategy' - ) || 'inclusion'; + unless ($estimation_strategy) { + $estimation_strategy = OpenSRF::Utils::SettingsClient + ->new + ->config_value( + apps => 'open-ils.search', app_settings => 'estimation_strategy' + ) || 'inclusion'; + } $search_hash->{estimation_strategy} = $estimation_strategy; # pull any existing results from the cache @@ -1343,6 +1228,7 @@ sub staged_search { # retrieve the window of results from the database $logger->debug("staged search: fetching results from the database"); $search_hash->{skip_check} = $page * $superpage_size; + $search_hash->{return_query} = $page == 0 ? 1 : 0; my $start = time; $results = $U->storagereq($method, %$search_hash); $search_duration = time - $start; @@ -1384,6 +1270,12 @@ sub staged_search { my $current_count = scalar(@$all_results); + if ($page == 0) { + foreach (qw/ query_struct canonicalized_query /) { + $global_summary->{$_} = $summary->{$_}; + } + } + $est_hit_count = $summary->{estimated_hit_count} || $summary->{visible} if $page == 0; @@ -1444,6 +1336,7 @@ sub staged_search { $conn->respond_complete( { + global_summary => $global_summary, count => $est_hit_count, core_limit => $search_hash->{core_limit}, superpage_size => $search_hash->{check_limit}, @@ -1453,9 +1346,9 @@ sub staged_search { } ); - cache_facets($facet_key, $new_ids, $IAmMetabib, $ignore_facet_classes) if $docache; + $logger->info("Completed canonicalized search is: $$global_summary{canonicalized_query}"); - return undef; + return cache_facets($facet_key, $new_ids, $IAmMetabib, $ignore_facet_classes) if $docache; } sub tag_circulated_records { diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/metabib.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/metabib.pm index 7aec5f6f62..df5d03b41e 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/metabib.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/metabib.pm @@ -2792,7 +2792,7 @@ __PACKAGE__->register_method( sub abstract_query2str { my ($self, $conn, $query) = @_; - return QueryParser::Canonicalize::abstract_query2str_impl($query, 0); + return QueryParser::Canonicalize::abstract_query2str_impl($query, 0, $OpenILS::Application::Storage::QParser); } __PACKAGE__->register_method( @@ -3134,6 +3134,11 @@ sub query_parser_fts { $$summary_row{complex_query} = $query->simple_plan ? 0 : 1; } + if ($args{return_query}) { + $$summary_row{query_struct} = $query->parse_tree->to_abstract_query(); + $$summary_row{canonicalized_query} = QueryParser::Canonicalize::abstract_query2str_impl($$summary_row{query_struct}, 0, $parser); + } + $client->respond( $summary_row ); $log->debug("Search yielded ".scalar(@$recs)." checked, visible results with an approximate visible total of $estimate.",DEBUG); @@ -3173,17 +3178,20 @@ sub query_parser_fts_wrapper { _initialize_parser($parser) unless $parser->initialization_complete; - if (! scalar( keys %{$args{searches}} )) { + $args{searches} ||= {}; + if (!scalar(keys(%{$args{searches}})) && !$args{query}) { die "No search arguments were passed to ".$self->api_name; } $top_org ||= actor::org_unit->search( { parent_ou => undef } )->next; - $log->debug("Constructing QueryParser query from staged search hash ...", DEBUG); - my $base_query = ''; - for my $sclass ( keys %{$args{searches}} ) { - $log->debug(" --> staged search key: $sclass --> term: $args{searches}{$sclass}{term}", DEBUG); - $base_query .= " $sclass: $args{searches}{$sclass}{term}"; + my $base_query = $args{query} || ''; + if (scalar(keys($args{searches}))) { + $log->debug("Constructing QueryParser query from staged search hash ...", DEBUG); + for my $sclass ( keys %{$args{searches}} ) { + $log->debug(" --> staged search key: $sclass --> term: $args{searches}{$sclass}{term}", DEBUG); + $base_query .= " $sclass: $args{searches}{$sclass}{term}"; + } } my $query = $base_query; @@ -3262,6 +3270,8 @@ sub query_parser_fts_wrapper { $query = "estimation_strategy($args{estimation_strategy}) $query" if ($args{estimation_strategy}); $query = "badge_orgs($borgs) $query" if ($borgs); + + # XXX All of the following, down to the 'return' is basically dead code. someone higher up should handle it $query = "site($args{org_unit}) $query" if ($args{org_unit}); $query = "depth($args{depth}) $query" if (defined($args{depth})); $query = "sort($args{sort}) $query" if ($args{sort}); @@ -3290,13 +3300,12 @@ sub query_parser_fts_wrapper { $args{item_form} = [ split '', $f ]; } - for my $filter ( qw/locations location_groups statuses between audience language lit_form item_form item_type bib_level vr_format badges/ ) { + for my $filter ( qw/locations location_groups statuses audience language lit_form item_form item_type bib_level vr_format badges/ ) { if (my $s = $args{$filter}) { $s = [$s] if (!ref($s)); my @filter_list = @$s; - next if ($filter eq 'between' and scalar(@filter_list) != 2); next if (@filter_list == 0); my $filter_string = join ',', @filter_list; @@ -3306,7 +3315,7 @@ sub query_parser_fts_wrapper { $log->debug("Full QueryParser query: $query", DEBUG); - return query_parser_fts($self, $client, query => $query, _simple_plan => $base_plan->simple_plan ); + return query_parser_fts($self, $client, query => $query, _simple_plan => $base_plan->simple_plan, return_query => $args{return_query} ); } __PACKAGE__->register_method( api_name => "open-ils.storage.biblio.multiclass.staged.search_fts", diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/QueryParser.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/QueryParser.pm index 7f1f2356c6..a4c74d1e19 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/QueryParser.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/QueryParser.pm @@ -1459,10 +1459,9 @@ sub abstract_query2str_impl { ($abstract_query->{content} || '') . ($abstract_query->{suffix} || ''); } elsif ($abstract_query->{type} eq 'facet') { - # facet syntax [ # ] is hardcoded I guess? my $prefix = $abstract_query->{negate} ? $qpconfig->{operators}{disallowed} : ''; $q .= ($q ? ' ' : '') . $prefix . $abstract_query->{name} . "[" . - join(" # ", @{$abstract_query->{values}}) . "]"; + join("][", @{$abstract_query->{values}}) . "]"; } } diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/SuperCat.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/SuperCat.pm index a338ebe12c..b4ba2d75fc 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/SuperCat.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/SuperCat.pm @@ -1438,19 +1438,20 @@ sub opensearch_feed { my $org_unit = get_ou($org); # Apostrophes break search and get indexed as spaces anyway + # XXX ^that's kinda a lie ... my $safe_terms = $terms; $safe_terms =~ s{'}{ }go; + + my $query_terms = 'site('.$org_unit->[0]->shortname.") $safe_terms"; + $query_tems = "sort($sort) $query_terms" if ($sort); + $query_tems = "language($lang) $query_terms" if ($lang); + $query_tems = "#$sortdir $query_terms" if ($sortdir); my $recs = $search->request( 'open-ils.search.biblio.multiclass.query' => { - org_unit => $org_unit->[0]->id, offset => $offset, - limit => $limit, - sort => $sort, - sort_dir => $sortdir, - default_class => $class, - ($lang ? ( 'language' => $lang ) : ()), - } => $safe_terms => 1 + limit => $limit + } => $query_terms => 1 )->gather(1); $log->debug("Hits for [$terms]: $recs->{count}"); -- 2.43.2