LP#1568195: fix retrieving big OUS batches
authorGalen Charlton <gmc@esilibrary.com>
Sat, 9 Apr 2016 01:46:35 +0000 (21:46 -0400)
committerMike Rylander <mrylander@gmail.com>
Thu, 28 Apr 2016 15:52:33 +0000 (11:52 -0400)
This patch fixes a regression introduced in LP#1501471
where the Library Settings Editor could fail to retrieve
the values of org unit settings if more than 99 were
requested at a time.

To test
-------
[1] Open the XUL library settings editor and ensure that
    no search filters are in effect. Note that values
    are not displayed for any of the OU settings, and that
    the Pg log contains error messages like this:

    "ERROR: cannot pass more than 100 arguments to a function"

[2] Apply the patch.
[3] Repeat step one, and verify that values are now retrieved
    for all of the OU settings that have values set.
[4] Verify that the pgTAP tests in live_t/aous_batch.pg pass.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Mike Rylander <mrylander@gmail.com>
Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm
Open-ILS/src/sql/Pg/020.schema.functions.sql
Open-ILS/src/sql/Pg/live_t/aous_batch.pg
Open-ILS/src/sql/Pg/upgrade/XXXX.schema.make_ous_batch_retrieve_func_nonvariadic.sql [new file with mode: 0644]

index 1378a47..5f4714b 100644 (file)
@@ -1315,7 +1315,13 @@ sub ou_ancestor_setting_batch_insecure {
     my( $self, $orgid, $names ) = @_;
 
     my %result = map { $_ => undef } @$names;
-    my $query = {from => ['actor.org_unit_ancestor_setting_batch', $orgid, @$names]};
+    my $query = {
+        from => [
+            'actor.org_unit_ancestor_setting_batch',
+            $orgid,
+            '{' . join(',', @$names) . '}'
+        ]
+    };
     my $e = OpenILS::Utils::CStoreEditor->new();
     my $settings = $e->json_query($query);
     foreach my $setting (@$settings) {
index 71b5116..7cc5be0 100644 (file)
@@ -293,7 +293,7 @@ Search "up" the org_unit tree until we find the first occurrence of an
 org_unit_setting with the given name.
 $$;
 
-CREATE OR REPLACE FUNCTION actor.org_unit_ancestor_setting_batch( org_id INT, VARIADIC setting_names TEXT[] ) RETURNS SETOF actor.org_unit_setting AS $$
+CREATE OR REPLACE FUNCTION actor.org_unit_ancestor_setting_batch( org_id INT, setting_names TEXT[] ) RETURNS SETOF actor.org_unit_setting AS $$
 DECLARE
     setting RECORD;
     setting_name TEXT;
@@ -316,7 +316,7 @@ BEGIN
 END;
 $$ LANGUAGE plpgsql STABLE;
 
-COMMENT ON FUNCTION actor.org_unit_ancestor_setting_batch( INT, VARIADIC TEXT[] ) IS $$
+COMMENT ON FUNCTION actor.org_unit_ancestor_setting_batch( INT,  TEXT[] ) IS $$
 For each setting name passed, search "up" the org_unit tree until
 we find the first occurrence of an org_unit_setting with the given name.
 $$;
index 4a0dabd..87c05e5 100644 (file)
@@ -10,20 +10,20 @@ INSERT INTO actor.org_unit_setting (org_unit, name, value) VALUES (2, 'foo', '"f
 INSERT INTO actor.org_unit_setting (org_unit, name, value) VALUES (2, 'bar', '"bar 2"');
 
 SELECT results_eq(
-    $$ SELECT name, value FROM actor.org_unit_ancestor_setting_batch(1, 'foo', 'bar') $$,
+    $$ SELECT name, value FROM actor.org_unit_ancestor_setting_batch(1, '{foo,bar}') $$,
     $$ VALUES ('foo', '"foo 1"') $$,
     'can retrieve batch of org unit settings'
 );
 
 SELECT results_eq(
-    $$ SELECT name, value FROM actor.org_unit_ancestor_setting_batch(2, 'foo', 'bar') $$,
+    $$ SELECT name, value FROM actor.org_unit_ancestor_setting_batch(2, '{foo,bar}') $$,
     $$ VALUES ('foo', '"foo 2"'), ('bar', '"bar 2"') $$,
     'can retrieve batch of org unit settings at lower level'
 );
 
 DELETE FROM actor.org_unit_setting WHERE name = 'foo' AND org_unit = 2;
 SELECT results_eq(
-    $$ SELECT name, value FROM actor.org_unit_ancestor_setting_batch(2, 'foo', 'bar') $$,
+    $$ SELECT name, value FROM actor.org_unit_ancestor_setting_batch(2, '{foo,bar}') $$,
     $$ VALUES ('foo', '"foo 1"'), ('bar', '"bar 2"') $$,
     'can retrieve batch of org unit settings with fallback'
 );
diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.make_ous_batch_retrieve_func_nonvariadic.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.make_ous_batch_retrieve_func_nonvariadic.sql
new file mode 100644 (file)
index 0000000..f933345
--- /dev/null
@@ -0,0 +1,36 @@
+BEGIN;
+
+--SELECT evergreen.upgrade_deps_block_check('XXXX', :eg_version);
+
+-- note: it is not necessary to explicitly drop the previous VARIADIC
+-- version of this stored procedure; create or replace function...
+-- suffices.
+CREATE OR REPLACE FUNCTION actor.org_unit_ancestor_setting_batch( org_id INT, setting_names TEXT[] ) RETURNS SETOF actor.org_unit_setting AS $$
+DECLARE
+    setting RECORD;
+    setting_name TEXT;
+    cur_org INT;
+BEGIN
+    FOREACH setting_name IN ARRAY setting_names
+    LOOP
+        cur_org := org_id;
+        LOOP
+            SELECT INTO setting * FROM actor.org_unit_setting WHERE org_unit = cur_org AND name = setting_name;
+            IF FOUND THEN
+                RETURN NEXT setting;
+                EXIT;
+            END IF;
+            SELECT INTO cur_org parent_ou FROM actor.org_unit WHERE id = cur_org;
+            EXIT WHEN cur_org IS NULL;
+        END LOOP;
+    END LOOP;
+    RETURN;
+END;
+$$ LANGUAGE plpgsql STABLE;
+
+COMMENT ON FUNCTION actor.org_unit_ancestor_setting_batch( INT,  TEXT[] ) IS $$
+For each setting name passed, search "up" the org_unit tree until
+we find the first occurrence of an org_unit_setting with the given name.
+$$;
+
+COMMIT;