LP#1501471: add DB function for batch retrieval of OU settings
authorGalen Charlton <gmc@esilibrary.com>
Wed, 30 Sep 2015 22:11:55 +0000 (22:11 +0000)
committerBill Erickson <berickxx@gmail.com>
Thu, 5 Nov 2015 14:23:34 +0000 (09:23 -0500)
This adds a new stored function and a utility routine that uses it
for retrieving a set of OU settings in one fell swoop.  This offers
a significant speed boost for the Dojo patron editor, which loads
about 70 OU settings when it initalizes itself.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Bill Erickson <berickxx@gmail.com>
Open-ILS/src/perlmods/lib/OpenILS/Application/Actor.pm
Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm
Open-ILS/src/sql/Pg/020.schema.functions.sql
Open-ILS/src/sql/Pg/upgrade/XXXX.schema.batch_settings_retrieve_function.sql [new file with mode: 0644]

index 122b32d..050dfbb 100644 (file)
@@ -314,22 +314,38 @@ __PACKAGE__->register_method(
 sub ou_ancestor_setting_batch {
     my( $self, $client, $orgid, $name_list, $auth ) = @_;
 
-    my %must_check_perm = ();
-    unless (defined $auth) {
-        # figure out which OU settings *require* view permission
-        # checks
-        my $e = new_editor();
-        my $res = $e->search_config_org_unit_setting_type({
-            name      => $name_list,
-            view_perm => { "!=" => undef },
-        });
-        $must_check_perm{ $_->name() } = -1 for @$res;
+    # splitting the list of settings to fetch values
+    # so that ones that *don't* require view_perm checks
+    # can be fetched in one fell swoop, which is
+    # significantly faster in cases where a large
+    # number of settings need to be fetched.
+    my %perm_check_required = ();
+    my @perm_check_not_required = ();
+
+    # Note that ->ou_ancestor_setting also can check
+    # to see if the setting has a view_perm, but testing
+    # suggests that the redundant checks do not significantly
+    # increase the time it takes to fetch the values of
+    # permission-controlled settings.
+    my $e = new_editor();
+    my $res = $e->search_config_org_unit_setting_type({
+        name      => $name_list,
+        view_perm => { "!=" => undef },
+    });
+    %perm_check_required = map { $_->name() => 1 } @$res;
+    foreach my $setting (@$name_list) {
+        push @perm_check_not_required, $setting
+            unless exists($perm_check_required{$setting});
     }
+
     my %values;
+    if (@perm_check_not_required) {
+        %values = $U->ou_ancestor_setting_batch_insecure($orgid, \@perm_check_not_required);
+    }
     $values{$_} = $U->ou_ancestor_setting(
         $orgid, $_, undef,
-        ($auth ? $auth : $must_check_perm{$_})
-    ) for @$name_list;
+        ($auth ? $auth : -1)
+    ) for keys(%perm_check_required);
     return \%values;
 }
 
index 90c1463..a1336e6 100644 (file)
@@ -1304,7 +1304,28 @@ sub ou_ancestor_setting {
     return undef unless $setting;
     return {org => $setting->{org_unit}, value => OpenSRF::Utils::JSON->JSON2perl($setting->{value})};
 }   
-        
+
+# This fetches a set of OU settings in one fell swoop,
+# which can be significantly faster than invoking
+# $U->ou_ancestor_setting() one setting at a time.
+# As the "_insecure" implies, however, callers are
+# responsible for ensuring that the settings to be
+# fetch do not need view permission checks.
+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 $e = OpenILS::Utils::CStoreEditor->new();
+    my $settings = $e->json_query($query);
+    foreach my $setting (@$settings) {
+        $result{$setting->{name}} = {
+            org => $setting->{org_unit},
+            value => OpenSRF::Utils::JSON->JSON2perl($setting->{value})
+        };
+    }
+    return %result;
+}
 
 # returns the ISO8601 string representation of the requested epoch in GMT
 sub epoch2ISO8601 {
index c0eb8af..71b5116 100644 (file)
@@ -293,6 +293,34 @@ 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 $$
+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, VARIADIC 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.
+$$;
+
 CREATE OR REPLACE FUNCTION evergreen.get_barcodes(select_ou INT, type TEXT, in_barcode TEXT) RETURNS SETOF evergreen.barcode_set AS $$
 DECLARE
     cur_barcode TEXT;
diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.batch_settings_retrieve_function.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.batch_settings_retrieve_function.sql
new file mode 100644 (file)
index 0000000..63e3234
--- /dev/null
@@ -0,0 +1,33 @@
+BEGIN;
+
+--SELECT evergreen.upgrade_deps_block_check('XXXX', :eg_version);
+
+CREATE OR REPLACE FUNCTION actor.org_unit_ancestor_setting_batch( org_id INT, VARIADIC 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, VARIADIC 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;