From 28a8b519d3cd01445c63384efcb56fd85b5a9396 Mon Sep 17 00:00:00 2001 From: David Cook Date: Wed, 18 Jun 2014 16:26:31 +1000 Subject: [PATCH] Bug 12443 - Initial re-factoring of buildQuery This patch reduces three repeated code fragments into a single internal subroutine, which is easier to read, has comments, and should make it easier to refactor more buildQuery code in the future. _TEST PLAN_ Before applying 1) Run a bunch of different searches in the staff client and OPAC in separate tabs 2) Apply the patch 3) Run the same searches again (maybe in yet more tabs) and notice that the results are exactly the same. --- C4/Search.pm | 84 +++++++++++++++++++++++++++++++++------------------------- 1 files changed, 48 insertions(+), 36 deletions(-) diff --git a/C4/Search.pm b/C4/Search.pm index 748bb1c..c94d924 100644 --- a/C4/Search.pm +++ b/C4/Search.pm @@ -1486,43 +1486,19 @@ sub buildQuery { warn "FIELD WEIGHTED OPERAND: >$weighted_operand<" if $DEBUG; - # If there's a previous operand, we need to add an operator - if ($previous_operand) { - - # User-specified operator - if ( $operators[ $i - 1 ] ) { - $query .= " $operators[$i-1] "; - $query .= " $index_plus " unless $indexes_set; - $query .= " $operand"; - $query_cgi .= "&op=".uri_escape($operators[$i-1]); - $query_cgi .= "&idx=".uri_escape($index) if $index; - $query_cgi .= "&q=".uri_escape($operands[$i]) if $operands[$i]; - $query_desc .= - " $operators[$i-1] $index_plus $operands[$i]"; - } - - # Default operator is and - else { - $query .= " and "; - $query .= "$index_plus " unless $indexes_set; - $query .= "$operand"; - $query_cgi .= "&op=and&idx=".uri_escape($index) if $index; - $query_cgi .= "&q=".uri_escape($operands[$i]) if $operands[$i]; - $query_desc .= " and $index_plus $operands[$i]"; - } - } - - # There isn't a pervious operand, don't need an operator - else { + ($query,$query_cgi,$query_desc,$previous_operand) = _build_initial_query({ + query => $query, + query_cgi => $query_cgi, + query_desc => $query_desc, + operator => ($operators[ $i - 1 ]) ? $operators[ $i - 1 ] : '', + parsed_operand => $operand, + original_operand => ($operands[$i]) ? $operands[$i] : '', + index => $index, + index_plus => $index_plus, + indexes_set => $indexes_set, + previous_operand => $previous_operand, + }); - # Field-weighted queries already have indexes set - $query .= " $index_plus " unless $indexes_set; - $query .= $operand; - $query_desc .= " $index_plus $operands[$i]"; - $query_cgi .= "&idx=".uri_escape($index) if $index; - $query_cgi .= "&q=".uri_escape($operands[$i]) if $operands[$i]; - $previous_operand = 1; - } } #/if $operands } # /for } @@ -1627,6 +1603,42 @@ sub buildQuery { ); } +=head2 _build_initial_query + + ($query, $query_cgi, $query_desc, $previous_operand) = _build_initial_query($initial_query_params); + + Build a section of the initial query containing indexes, operators, and operands. + +=cut + +sub _build_initial_query { + my ($params) = @_; + + my $operator = ""; + if ($params->{previous_operand}){ + #If there is a previous operand, add a supplied operator or the default 'and' + $operator = ($params->{operator}) ? " ".($params->{operator})." " : ' and '; + } + + #NOTE: indexes_set is typically set when doing truncation or field weighting + my $operand = ($params->{indexes_set}) ? $params->{parsed_operand} : $params->{index_plus}.$params->{parsed_operand}; + + #e.g. "kw,wrdl:test" + #e.g. " and kw,wrdl:test" + $params->{query} .= $operator . $operand; + + $params->{query_cgi} .= "&op=".uri_escape($operator) if $operator; + $params->{query_cgi} .= "&idx=".uri_escape($params->{index}) if $params->{index}; + $params->{query_cgi} .= "&q=".uri_escape($params->{original_operand}) if $params->{original_operand}; + + #e.g. " and kw,wrdl: test" + $params->{query_desc} .= $operator . $params->{index_plus} . " " . $params->{original_operand}; + + $params->{previous_operand} = 1 unless $params->{previous_operand}; #If there is no previous operand, mark this as one + + return ($params->{query}, $params->{query_cgi}, $params->{query_desc}, $params->{previous_operand}); +} + =head2 searchResults my @search_results = searchResults($search_context, $searchdesc, $hits, -- 1.7.7.4