From 3929fb367a7829a5de2e81e5312e72fcda1a200c Mon Sep 17 00:00:00 2001 From: Mike Rylander Date: Thu, 7 Nov 2019 11:05:00 -0500 Subject: [PATCH] LP#1798910: Update bib visibility whenever necessary Before this commit, we avoid a trigger loop related to updating a bib record's visibility attributes in the heavy-handed way of refusing to do so if the 'ingest.reingest.force_on_same_marc' flag is enabled. However, this keeps us from seeing Located URI changes. This commit, inspired by Josh Stompro, checks to see if the bib's visibility attributes have actually changed due to a located URI change, and only updates the row in that case. It also forces a stable order for LURI-based visibility to avoid any possible flapping due to returned row order when LURIs are updated or regenerated. Signed-off-by: Mike Rylander --- Open-ILS/src/sql/Pg/300.schema.staged_search.sql | 38 +++-- .../upgrade/XXXX.function.cache_bib_vis_attrs.sql | 188 +++++++++++++++++++++ 2 files changed, 213 insertions(+), 13 deletions(-) create mode 100644 Open-ILS/src/sql/Pg/upgrade/XXXX.function.cache_bib_vis_attrs.sql 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 ee911788ae..cfe30abbe2 100644 --- a/Open-ILS/src/sql/Pg/300.schema.staged_search.sql +++ b/Open-ILS/src/sql/Pg/300.schema.staged_search.sql @@ -569,6 +569,7 @@ BEGIN WHERE record = bib_id AND label = '##URI##' AND NOT deleted + ORDER BY owning_lib LOOP attr_set := attr_set || search.calculate_visibility_attribute(cn_row.owning_lib, 'luri_org'); END LOOP; @@ -582,10 +583,17 @@ DECLARE ocn asset.call_number%ROWTYPE; ncn asset.call_number%ROWTYPE; cid BIGINT; - dobib BOOL; + old_vav INT[]; + new_vav INT[]; BEGIN - SELECT enabled = FALSE INTO dobib FROM config.internal_flag WHERE name = 'ingest.reingest.force_on_same_marc'; + IF TG_OP IN ('INSERT','UPDATE') AND TG_TABLE_NAME = 'call_number' THEN + new_vav := biblio.calculate_bib_visibility_attribute_set(NEW.record); + END IF; + + IF TG_OP IN ('UPDATE','DELETE') AND TG_TABLE_NAME = 'call_number' THEN + old_vav := biblio.calculate_bib_visibility_attribute_set(OLD.record); + END IF; IF TG_TABLE_NAME = 'peer_bib_copy_map' THEN -- Only needs ON INSERT OR DELETE, so handle separately IF TG_OP = 'INSERT' THEN @@ -614,10 +622,11 @@ BEGIN ); ELSIF TG_TABLE_NAME = 'record_entry' THEN NEW.vis_attr_vector := biblio.calculate_bib_visibility_attribute_set(NEW.id, NEW.source, TRUE); - ELSIF TG_TABLE_NAME = 'call_number' AND NEW.label = '##URI##' AND dobib THEN -- New located URI + ELSIF TG_TABLE_NAME = 'call_number' AND NEW.label = '##URI##' THEN -- New located URI UPDATE biblio.record_entry - SET vis_attr_vector = biblio.calculate_bib_visibility_attribute_set(NEW.record) - WHERE id = NEW.record; + SET vis_attr_vector = new_vav + WHERE id = NEW.record + AND vis_attr_vector IS DISTINCT FROM new_vav; END IF; @@ -680,23 +689,26 @@ BEGIN ELSIF TG_TABLE_NAME = 'call_number' THEN - IF TG_OP = 'DELETE' AND OLD.label = '##URI##' AND dobib THEN -- really deleted located URI, if the delete protection rule is disabled... + IF TG_OP = 'DELETE' AND OLD.label = '##URI##' THEN -- really deleted located URI, if the delete protection rule is disabled... UPDATE biblio.record_entry - SET vis_attr_vector = biblio.calculate_bib_visibility_attribute_set(OLD.record) - WHERE id = OLD.record; + SET vis_attr_vector = old_vav + WHERE id = OLD.record + AND vis_attr_vector IS DISTINCT FROM old_vav; RETURN OLD; END IF; - IF OLD.label = '##URI##' AND dobib THEN -- Located URI + IF OLD.label = '##URI##' THEN -- Located URI IF OLD.deleted <> NEW.deleted OR OLD.record <> NEW.record OR OLD.owning_lib <> NEW.owning_lib THEN UPDATE biblio.record_entry - SET vis_attr_vector = biblio.calculate_bib_visibility_attribute_set(NEW.record) - WHERE id = NEW.record; + SET vis_attr_vector = new_vav + WHERE id = NEW.record + AND vis_attr_vector IS DISTINCT FROM new_vav; IF OLD.record <> NEW.record THEN -- maybe on merge? UPDATE biblio.record_entry - SET vis_attr_vector = biblio.calculate_bib_visibility_attribute_set(OLD.record) - WHERE id = OLD.record; + SET vis_attr_vector = old_vav + WHERE id = OLD.record + AND vis_attr_vector IS DISTINCT FROM old_vav; END IF; END IF; diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.function.cache_bib_vis_attrs.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.function.cache_bib_vis_attrs.sql new file mode 100644 index 0000000000..aac01ac597 --- /dev/null +++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.function.cache_bib_vis_attrs.sql @@ -0,0 +1,188 @@ +BEGIN; + +SELECT evergreen.upgrade_deps_block_check('XXXX', :eg_version); + +CREATE OR REPLACE FUNCTION biblio.calculate_bib_visibility_attribute_set ( bib_id BIGINT, new_source INT DEFAULT NULL, force_source BOOL DEFAULT FALSE ) RETURNS INT[] AS $f$ +DECLARE + bib_row biblio.record_entry%ROWTYPE; + cn_row asset.call_number%ROWTYPE; + attr_set INT[] := '{}'::INT[]; +BEGIN + SELECT * INTO bib_row FROM biblio.record_entry WHERE id = bib_id; + + IF force_source THEN + IF new_source IS NOT NULL THEN + attr_set := attr_set || search.calculate_visibility_attribute(new_source, 'bib_source'); + END IF; + ELSIF bib_row.source IS NOT NULL THEN + attr_set := attr_set || search.calculate_visibility_attribute(bib_row.source, 'bib_source'); + END IF; + + FOR cn_row IN + SELECT * + FROM asset.call_number + WHERE record = bib_id + AND label = '##URI##' + AND NOT deleted + ORDER BY owning_lib + LOOP + attr_set := attr_set || search.calculate_visibility_attribute(cn_row.owning_lib, 'luri_org'); + END LOOP; + + RETURN attr_set; +END; +$f$ LANGUAGE PLPGSQL; + +CREATE OR REPLACE FUNCTION asset.cache_copy_visibility () RETURNS TRIGGER as $func$ +DECLARE + ocn asset.call_number%ROWTYPE; + ncn asset.call_number%ROWTYPE; + cid BIGINT; + old_vav INT[]; + new_vav INT[]; +BEGIN + + IF TG_OP IN ('INSERT','UPDATE') AND TG_TABLE_NAME = 'call_number' THEN + new_vav := biblio.calculate_bib_visibility_attribute_set(NEW.record); + END IF; + + IF TG_OP IN ('UPDATE','DELETE') AND TG_TABLE_NAME = 'call_number' THEN + old_vav := biblio.calculate_bib_visibility_attribute_set(OLD.record); + END IF; + + IF TG_TABLE_NAME = 'peer_bib_copy_map' THEN -- Only needs ON INSERT OR DELETE, so handle separately + IF TG_OP = 'INSERT' THEN + INSERT INTO asset.copy_vis_attr_cache (record, target_copy, vis_attr_vector) VALUES ( + NEW.peer_record, + NEW.target_copy, + asset.calculate_copy_visibility_attribute_set(NEW.target_copy) + ); + + RETURN NEW; + ELSIF TG_OP = 'DELETE' THEN + DELETE FROM asset.copy_vis_attr_cache + WHERE record = OLD.peer_record AND target_copy = OLD.target_copy; + + RETURN OLD; + END IF; + END IF; + + IF TG_OP = 'INSERT' THEN -- Handles ON INSERT. ON UPDATE is below. + IF TG_TABLE_NAME IN ('copy', 'unit') THEN + SELECT * INTO ncn FROM asset.call_number cn WHERE id = NEW.call_number; + INSERT INTO asset.copy_vis_attr_cache (record, target_copy, vis_attr_vector) VALUES ( + ncn.record, + NEW.id, + asset.calculate_copy_visibility_attribute_set(NEW.id) + ); + ELSIF TG_TABLE_NAME = 'record_entry' THEN + NEW.vis_attr_vector := biblio.calculate_bib_visibility_attribute_set(NEW.id, NEW.source, TRUE); + ELSIF TG_TABLE_NAME = 'call_number' AND NEW.label = '##URI##' THEN -- New located URI + UPDATE biblio.record_entry + SET vis_attr_vector = new_vav + WHERE id = NEW.record + AND vis_attr_vector IS DISTINCT FROM new_vav; + + END IF; + + RETURN NEW; + END IF; + + -- handle items first, since with circulation activity + -- their statuses change frequently + IF TG_TABLE_NAME IN ('copy', 'unit') THEN -- This handles ON UPDATE OR DELETE. ON INSERT above + + IF TG_OP = 'DELETE' THEN -- Shouldn't get here, normally + DELETE FROM asset.copy_vis_attr_cache WHERE target_copy = OLD.id; + RETURN OLD; + END IF; + + SELECT * INTO ncn FROM asset.call_number cn WHERE id = NEW.call_number; + + IF OLD.deleted <> NEW.deleted THEN + IF NEW.deleted THEN + DELETE FROM asset.copy_vis_attr_cache WHERE target_copy = OLD.id; + ELSE + INSERT INTO asset.copy_vis_attr_cache (record, target_copy, vis_attr_vector) VALUES ( + ncn.record, + NEW.id, + asset.calculate_copy_visibility_attribute_set(NEW.id) + ); + END IF; + + RETURN NEW; + ELSIF OLD.location <> NEW.location OR + OLD.status <> NEW.status OR + OLD.opac_visible <> NEW.opac_visible OR + OLD.circ_lib <> NEW.circ_lib OR + OLD.call_number <> NEW.call_number + THEN + IF OLD.call_number <> NEW.call_number THEN -- Special check since it's more expensive than the next branch + SELECT * INTO ocn FROM asset.call_number cn WHERE id = OLD.call_number; + + IF ncn.record <> ocn.record THEN + -- We have to use a record-specific WHERE clause + -- to avoid modifying the entries for peer-bib copies. + UPDATE asset.copy_vis_attr_cache + SET target_copy = NEW.id, + record = ncn.record + WHERE target_copy = OLD.id + AND record = ocn.record; + + END IF; + ELSE + -- Any of these could change visibility, but + -- we'll save some queries and not try to calculate + -- the change directly. We want to update peer-bib + -- entries in this case, unlike above. + UPDATE asset.copy_vis_attr_cache + SET target_copy = NEW.id, + vis_attr_vector = asset.calculate_copy_visibility_attribute_set(NEW.id) + WHERE target_copy = OLD.id; + END IF; + END IF; + + ELSIF TG_TABLE_NAME = 'call_number' THEN + + IF TG_OP = 'DELETE' AND OLD.label = '##URI##' THEN -- really deleted located URI, if the delete protection rule is disabled... + UPDATE biblio.record_entry + SET vis_attr_vector = old_vav + WHERE id = OLD.record + AND vis_attr_vector IS DISTINCT FROM old_vav; + RETURN OLD; + END IF; + + IF OLD.label = '##URI##' THEN -- Located URI + IF OLD.deleted <> NEW.deleted OR OLD.record <> NEW.record OR OLD.owning_lib <> NEW.owning_lib THEN + UPDATE biblio.record_entry + SET vis_attr_vector = new_vav + WHERE id = NEW.record + AND vis_attr_vector IS DISTINCT FROM new_vav; + + IF OLD.record <> NEW.record THEN -- maybe on merge? + UPDATE biblio.record_entry + SET vis_attr_vector = old_vav + WHERE id = OLD.record + AND vis_attr_vector IS DISTINCT FROM old_vav; + END IF; + END IF; + + ELSIF OLD.record <> NEW.record OR OLD.owning_lib <> NEW.owning_lib THEN + UPDATE asset.copy_vis_attr_cache + SET record = NEW.record, + vis_attr_vector = asset.calculate_copy_visibility_attribute_set(target_copy) + WHERE target_copy IN (SELECT id FROM asset.copy WHERE call_number = NEW.id) + AND record = OLD.record; + + END IF; + + ELSIF TG_TABLE_NAME = 'record_entry' AND OLD.source IS DISTINCT FROM NEW.source THEN -- Only handles ON UPDATE, INSERT above + NEW.vis_attr_vector := biblio.calculate_bib_visibility_attribute_set(NEW.id, NEW.source, TRUE); + END IF; + + RETURN NEW; +END; +$func$ LANGUAGE PLPGSQL; + +COMMIT; + -- 2.11.0