LP#2004055: Simpler Display Field highlighting

This commit uses database functions to precompute the normalized and
tokenized tsquery required for highlighting before it is returned to the
user, and disallows highlight-time compilation of the highlight map.
The primary purpose of this is to avoid the chance for user input to
find its way directly into SQL statements, but an additional benefit is
that it becomes much simpler for high level application code to make use
of Display Field highlighting in non-search contexts.

Signed-off-by: Mike Rylander <mrylander@gmail.com>
Signed-off-by: Galen Charlton <gmc@equinoxOLI.org>
Signed-off-by: Jason Boyer <JBoyer@equinoxOLI.org>

diff --git a/Open-ILS/src/eg2/src/app/share/catalog/catalog.service.ts b/Open-ILS/src/eg2/src/app/share/catalog/catalog.service.ts
index 1c861fa..e310fb5 100644 (file)
--- a/Open-ILS/src/eg2/src/app/share/catalog/catalog.service.ts
+++ b/Open-ILS/src/eg2/src/app/share/catalog/catalog.service.ts
@@ -259,7 +259,7 @@ export class CatalogService {
             (hlMap = hlMap.query_struct)    &&
             (hlMap = hlMap.additional_data) &&
             (hlMap = hlMap.highlight_map)   &&
-            (Object.keys(hlMap).length > 0)) {
+            (hlMap.length > 0)) {
         } else { return Promise.resolve(); }
 
         let ids;

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 f7a0c8b..8ea3324 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Search/Biblio.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Search/Biblio.pm
@@ -1415,22 +1415,13 @@ sub fetch_display_fields {
         return;
     }
 
-    my $hl_map_string = "";
-    if (ref($highlight_map) =~ /HASH/) {
-        for my $tsq (keys %$highlight_map) {
-            my $field_list = join(',', @{$$highlight_map{$tsq}});
-            $hl_map_string .= ' || ' if $hl_map_string;
-            $hl_map_string .= "hstore(($tsq)\:\:TEXT,'$field_list')";
-        }
-    }
-
     my $e = new_editor();
 
     for my $record ( @records ) {
-        next unless ($record && $hl_map_string);
+        next unless ($record && $highlight_map);
         $conn->respond(
             $e->json_query(
-                {from => ['search.highlight_display_fields', $record, $hl_map_string]}
+                {from => ['search.highlight_display_fields', $record, $highlight_map]}
             )
         );
     }

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Driver/Pg/QueryParser.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Driver/Pg/QueryParser.pm
index 0a6f7ba..874d0f4 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Driver/Pg/QueryParser.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Driver/Pg/QueryParser.pm
@@ -136,6 +136,14 @@ sub max_popularity_importance_multiplier {
     return $self->custom_data->{max_popularity_importance_multiplier};
 }
 
+sub dbh {
+    my $self = shift;
+    my $dbh = shift;
+
+    $self->custom_data->{dbh} = $dbh if defined($dbh);
+    return $self->custom_data->{dbh};
+}
+
 sub simple_plan {
     my $self = shift;
 
@@ -1932,7 +1940,9 @@ sub abstract_node_additions {
     my $hm = $self->plan
                 ->QueryParser
                 ->parse_tree
-                ->get_abstract_data('highlight_map') || {};
+                ->get_abstract_data('highlight_map') // {};
+
+    return unless ref($hm);
 
     my $field_set = $self->fields;
     $field_set = $self->plan->QueryParser->search_fields->{$self->classname}
@@ -1989,10 +1999,23 @@ sub abstract_node_additions {
         }
     }
 
+    # finally, ask the database to give us an hstore literal
+    my $hl_map_string = "";
+    for my $tsq (keys %$hm) {
+        my $field_list = join(',', @{$$hm{$tsq}});
+        $hl_map_string .= ' || ' if $hl_map_string;
+        $hl_map_string .= "hstore(($tsq)\:\:TEXT,'$field_list')";
+    }
+
+    my $calculated_hm = '';
+    $calculated_hm = $self->plan->QueryParser->dbh->selectcol_arrayref(
+        "SELECT $hl_map_string AS hm"
+    )->[0] if ($hl_map_string);
+
     $self->plan
         ->QueryParser
         ->parse_tree
-        ->set_abstract_data('highlight_map', $hm);
+        ->set_abstract_data('highlight_map', $calculated_hm);
 }
 
 sub add_vfield {

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 5766ccd..aa4102e 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/metabib.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/metabib.pm
@@ -88,6 +88,7 @@ sub _initialize_parser {
     $max_mult //= 2.0;
     $max_mult = 2.0 unless $max_mult =~ /^-?(?:\d+\.?|\.\d)\d*\z/; # just in case
     $parser->max_popularity_importance_multiplier($max_mult);
+    $parser->dbh(biblio::record_entry->db_Main);
 
     $cstore->disconnect;
     die("Cannot initialize $parser!") unless ($parser->initialization_complete);

diff --git a/Open-ILS/src/sql/Pg/300.schema.staged_search.sql b/Open-ILS/src/sql/Pg/300.schema.staged_search.sql
index 9533dcc..88b8c03 100644 (file)
--- a/Open-ILS/src/sql/Pg/300.schema.staged_search.sql
+++ b/Open-ILS/src/sql/Pg/300.schema.staged_search.sql
@@ -1048,7 +1048,7 @@ $$ LANGUAGE SQL IMMUTABLE LEAKPROOF STRICT COST 10;
 
 CREATE OR REPLACE FUNCTION search.highlight_display_fields(
     rid         BIGINT,
-    tsq_map     TEXT, -- { '(a | b) & c' => '1,2,3,4', ...}
+    tsq_map     TEXT, -- '(a | b) & c' => '1,2,3,4', ...
     css_class   TEXT DEFAULT 'oils_SH',
     hl_all      BOOL DEFAULT TRUE,
     minwords    INT DEFAULT 5,
@@ -1058,19 +1058,13 @@ CREATE OR REPLACE FUNCTION search.highlight_display_fields(
     delimiter   TEXT DEFAULT ' ... '
 ) RETURNS SETOF search.highlight_result AS $f$
 DECLARE
-    tsq_hstore  TEXT;
     tsq         TEXT;
     fields      TEXT;
     afields     INT[];
     seen        INT[];
 BEGIN
-    IF (tsq_map ILIKE 'hstore%') THEN
-        EXECUTE 'SELECT ' || tsq_map INTO tsq_hstore;
-    ELSE
-        tsq_hstore := tsq_map::HSTORE;
-    END IF;
 
-    FOR tsq, fields IN SELECT key, value FROM each(tsq_hstore::HSTORE) LOOP
+    FOR tsq, fields IN SELECT key, value FROM each(tsq_map::HSTORE) LOOP
         SELECT  ARRAY_AGG(unnest::INT) INTO afields
           FROM  unnest(regexp_split_to_array(fields,','));
         seen := seen || afields;

diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.function.safer_highlight.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.function.safer_highlight.sql
new file mode 100644 (file)
index 0000000..f61e516
--- /dev/null
+++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.function.safer_highlight.sql
@@ -0,0 +1,48 @@
+BEGIN;
+
+SELECT evergreen.upgrade_deps_block_check('XXXX', :eg_version);
+
+CREATE OR REPLACE FUNCTION search.highlight_display_fields(
+    rid         BIGINT,
+    tsq_map     TEXT, -- '(a | b) & c' => '1,2,3,4', ...
+    css_class   TEXT DEFAULT 'oils_SH',
+    hl_all      BOOL DEFAULT TRUE,
+    minwords    INT DEFAULT 5,
+    maxwords    INT DEFAULT 25,
+    shortwords  INT DEFAULT 0,
+    maxfrags    INT DEFAULT 0,
+    delimiter   TEXT DEFAULT ' ... '
+) RETURNS SETOF search.highlight_result AS $f$
+DECLARE
+    tsq         TEXT;
+    fields      TEXT;
+    afields     INT[];
+    seen        INT[];
+BEGIN
+
+    FOR tsq, fields IN SELECT key, value FROM each(tsq_map::HSTORE) LOOP
+        SELECT  ARRAY_AGG(unnest::INT) INTO afields
+          FROM  unnest(regexp_split_to_array(fields,','));
+        seen := seen || afields;
+
+        RETURN QUERY
+            SELECT * FROM search.highlight_display_fields_impl(
+                rid, tsq, afields, css_class, hl_all,minwords,
+                maxwords, shortwords, maxfrags, delimiter
+            );
+    END LOOP;
+
+    RETURN QUERY
+        SELECT  id,
+                source,
+                field,
+                evergreen.escape_for_html(value) AS value,
+                evergreen.escape_for_html(value) AS highlight
+          FROM  metabib.display_entry
+          WHERE source = rid
+                AND NOT (field = ANY (seen));
+END;
+$f$ LANGUAGE PLPGSQL ROWS 10;
+
+COMMIT;
+