Better atom divider regex; remove incorrect local use

First, we didn't need to make $last_type local, and it broke explicit
grouping anyway.  That's removed.

Second, we are smarter about finding the boundary of atoms.  Previous
to this commit, and curly brace could send the parser into a tailspin
from which it would not recover.  Now we use alternation instead of
a character class, which is much safer with the default multi-character
float syntax specifier.

Third, as a catch-all, if we can't parse the remained of a query we
now simply say so (when in debug mode) and go away, instead of risking
an infinite loop.  We do this via a final, unqualified "else" clause
in decompose().

Signed-off-by: Mike Rylander <mrylander@gmail.com>

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 1aa5c76..79579f3 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/QueryParser.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/QueryParser.pm
@@ -911,6 +911,8 @@ sub decompose {
     my $float_end = $pkg->operator('float_end');
     my $float_end_re = qr/^\s*\Q$float_end\E/;
 
+    my $atom_re = qr/.+?(?=\Q$float_start\E|\Q$group_start\E|\Q$float_end\E|\Q$group_end\E|\s|"|$)/;
+
     my $modifier_tag = $pkg->operator('modifier');
     my $modifier_tag_re = qr/^\s*\Q$modifier_tag\E/;
 
@@ -944,7 +946,7 @@ sub decompose {
         }
 
         if (/^\s*$/) { # end of an explicit group
-            local $last_type = '';
+            $last_type = '';
             last;
         } elsif (/$float_end_re/) { # end of an explicit group
             warn '  'x$recursing."Encountered explicit float end, remainder: $'\n" if $self->debug;
@@ -961,7 +963,7 @@ sub decompose {
             $remainder = $';
             $_ = '';
 
-            local $last_type = '';
+            $last_type = '';
         } elsif ($self->filter_count && /$filter_re/) { # found a filter
             warn '  'x$recursing."Encountered search filter: $1$2 set to $3\n" if $self->debug;
 
@@ -979,7 +981,7 @@ sub decompose {
             }
 
 
-            local $last_type = '';
+            $last_type = '';
         } elsif ($self->filter_count && /$filter_as_class_re/) { # found a filter
             warn '  'x$recursing."Encountered search filter: $1$2 set to $3\n" if $self->debug;
 
@@ -996,7 +998,7 @@ sub decompose {
                 $struct->new_filter( $filter => $params, $negate );
             }
 
-            local $last_type = '';
+            $last_type = '';
         } elsif ($self->modifier_count && /$modifier_re/) { # found a modifier
             warn '  'x$recursing."Encountered search modifier: $1\n" if $self->debug;
 
@@ -1007,7 +1009,7 @@ sub decompose {
                 $struct->new_modifier($1);
             }
 
-            local $last_type = '';
+            $last_type = '';
         } elsif ($self->modifier_count && /$modifier_as_class_re/) { # found a modifier
             warn '  'x$recursing."Encountered search modifier: $1\n" if $self->debug;
 
@@ -1020,7 +1022,7 @@ sub decompose {
                 $struct->new_modifier($mod);
             }
 
-            local $last_type = '';
+            $last_type = '';
         } elsif (/$float_start_re/) { # start of an explicit float
             warn '  'x$recursing."Encountered explicit float start\n" if $self->debug;
             $floating = 1;
@@ -1048,14 +1050,14 @@ sub decompose {
             $_ = $subremainder;
             warn '  'x$recursing."Query remainder after bool group: $_\n" if $self->debug;
 
-            local $last_type = '';
+            $last_type = '';
 
         } elsif (/$and_re/) { # ANDed expression
             $_ = $';
             warn '  'x$recursing."Encountered AND\n" if $self->debug;
             do {warn '  'x$recursing."!!! Already doing the bool dance for AND\n" if $self->debug; next} if ($last_type eq 'AND');
             do {warn '  'x$recursing."!!! Already doing the bool dance for OR\n" if $self->debug; next} if ($last_type eq 'OR');
-            local $last_type = 'AND';
+            $last_type = 'AND';
 
             warn '  'x$recursing."Saving LHS, building RHS\n" if $self->debug;
             my $LHS = $struct;
@@ -1083,13 +1085,13 @@ sub decompose {
 
             $self->parse_tree( $struct ) if ($self->parse_tree == $LHS);
 
-            local $last_type = '';
+            $last_type = '';
         } elsif (/$or_re/) { # ORed expression
             $_ = $';
             warn '  'x$recursing."Encountered OR\n" if $self->debug;
             do {warn '  'x$recursing."!!! Already doing the bool dance for AND\n" if $self->debug; next} if ($last_type eq 'AND');
             do {warn '  'x$recursing."!!! Already doing the bool dance for OR\n" if $self->debug; next} if ($last_type eq 'OR');
-            local $last_type = 'OR';
+            $last_type = 'OR';
 
             warn '  'x$recursing."Saving LHS, building RHS\n" if $self->debug;
             my $LHS = $struct;
@@ -1117,7 +1119,7 @@ sub decompose {
 
             $self->parse_tree( $struct ) if ($self->parse_tree == $LHS);
 
-            local $last_type = '';
+            $last_type = '';
         } elsif ($self->facet_class_count && /$facet_re/) { # changing current class
             warn '  'x$recursing."Encountered facet: $1$2 => $3\n" if $self->debug;
 
@@ -1127,7 +1129,7 @@ sub decompose {
             $struct->new_facet( $facet => $facet_value, $negate );
             $_ = $';
 
-            local $last_type = '';
+            $last_type = '';
         } elsif ($self->search_class_count && /$search_class_re/) { # changing current class
 
             if ($last_type eq 'CLASS') {
@@ -1140,7 +1142,7 @@ sub decompose {
             $current_class = $struct->classed_node( $1 )->requested_class();
             $_ = $';
 
-            local $last_type = 'CLASS';
+            $last_type = 'CLASS';
         } elsif (/^\s*($required_re|$disallowed_re|$negated_re)?"([^"]+)"/) { # phrase, always anded
             warn '  'x$recursing.'Encountered' . ($1 ? " ['$1' modified]" : '') . " phrase: $2\n" if $self->debug;
 
@@ -1155,7 +1157,7 @@ sub decompose {
                 $struct->add_node( $substruct ) if ($substruct);
                 $_ = $after;
             } else {
-                warn '  'x$recursing."Directly parsing the phrase subquery\n" if $self->debug;
+                warn '  'x$recursing."Directly parsing the phrase [ $phrase ] subquery\n" if $self->debug;
                 $struct->joiner( '&' );
 
                 my $class_node = $struct->classed_node($current_class);
@@ -1175,15 +1177,15 @@ sub decompose {
 
             }
 
-            local $last_type = '';
+            $last_type = '';
 
-        } elsif (/^\s*($required_re|$disallowed_re)([^${group_end}${float_end}\s"]+)/) { # convert require/disallow word to {un}phrase
+        } elsif (/^\s*($required_re|$disallowed_re)($atom_re)/) { # convert require/disallow word to {un}phrase
             warn '  'x$recursing."Encountered required atom (mini phrase), transforming for phrase parse: $1\n" if $self->debug;
 
             $_ = $1 . '"' . $2 . '"' . $';
 
-            local $last_type = '';
-        } elsif (/^\s*([^${group_end}${float_end}\s]+)/o) { # atom
+            $last_type = '';
+        } elsif (/^\s*($atom_re)/) { # atom
             warn '  'x$recursing."Encountered atom: $1\n" if $self->debug;
             warn '  'x$recursing."Remainder: $'\n" if $self->debug;
 
@@ -1191,7 +1193,7 @@ sub decompose {
             my $after = $';
 
             $_ = $after;
-            local $last_type = '';
+            $last_type = '';
 
             my $class_node = $struct->classed_node($current_class);
 
@@ -1205,8 +1207,11 @@ sub decompose {
                 $struct->joiner( '&' );
             }
 
-            local $last_type = '';
-        } 
+            $last_type = '';
+        } else {
+            warn '  'x$recursing."Cannot parse: $_\n" if $self->debug;
+            $_ = '';
+        }
 
         last unless ($_);