From a2e0678b2899603e51b28321c459b4597c28a2ce Mon Sep 17 00:00:00 2001 From: Mike Rylander Date: Thu, 6 Apr 2023 16:18:56 -0400 Subject: [PATCH] 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 Signed-off-by: Galen Charlton Signed-off-by: Jason Boyer --- .../eg2/src/app/share/catalog/catalog.service.ts | 2 +- .../lib/OpenILS/Application/Search/Biblio.pm | 13 +----- .../Application/Storage/Driver/Pg/QueryParser.pm | 27 +++++++++++- .../Application/Storage/Publisher/metabib.pm | 1 + Open-ILS/src/sql/Pg/300.schema.staged_search.sql | 10 +---- .../Pg/upgrade/XXXX.function.safer_highlight.sql | 48 ++++++++++++++++++++++ 6 files changed, 79 insertions(+), 22 deletions(-) create mode 100644 Open-ILS/src/sql/Pg/upgrade/XXXX.function.safer_highlight.sql 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 99ed080ac3..990b7e2619 100644 --- 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 a47e52cc2a..f2191500af 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Search/Biblio.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Search/Biblio.pm @@ -1442,22 +1442,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 c13c0216c9..cec9c6914c 100644 --- 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; @@ -1926,7 +1934,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} @@ -1983,10 +1993,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 5766ccddfe..aa4102e472 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 @@ -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 c9656cf199..87c8970cf4 100644 --- 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 index 0000000000..f61e516394 --- /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; + -- 2.11.0