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 <mrylander@gmail.com>

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 ee91178..cfe30ab 100644 (file)
--- 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 (file)
index 0000000..aac01ac
--- /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;
+