From 94666e2bfaeac14131ccc1686160665e77051d4a 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 1c861fa40c..e310fb588c 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 f7a0c8b1fa..8ea3324874 100644 --- 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 0a6f7ba748..874d0f4bba 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; @@ -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 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 9533dcc80c..88b8c037dc 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