From be1b6aafe5e0f6ad0f6f6f5417aa09c86a620bfa Mon Sep 17 00:00:00 2001 From: Galen Charlton Date: Wed, 30 Sep 2015 22:11:55 +0000 Subject: [PATCH] LP#1501471: add DB function for batch retrieval of OU settings 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 Signed-off-by: Bill Erickson --- .../src/perlmods/lib/OpenILS/Application/Actor.pm | 40 +++++++++++++++------- .../perlmods/lib/OpenILS/Application/AppUtils.pm | 23 ++++++++++++- Open-ILS/src/sql/Pg/020.schema.functions.sql | 28 +++++++++++++++ ...XXX.schema.batch_settings_retrieve_function.sql | 33 ++++++++++++++++++ 4 files changed, 111 insertions(+), 13 deletions(-) create mode 100644 Open-ILS/src/sql/Pg/upgrade/XXXX.schema.batch_settings_retrieve_function.sql diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Actor.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Actor.pm index 122b32d7f0..050dfbb0cc 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Actor.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Actor.pm @@ -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; } diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm index 90c1463494..a1336e6eda 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm @@ -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 { diff --git a/Open-ILS/src/sql/Pg/020.schema.functions.sql b/Open-ILS/src/sql/Pg/020.schema.functions.sql index c0eb8af385..71b5116f14 100644 --- a/Open-ILS/src/sql/Pg/020.schema.functions.sql +++ b/Open-ILS/src/sql/Pg/020.schema.functions.sql @@ -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 index 0000000000..63e3234e2a --- /dev/null +++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.batch_settings_retrieve_function.sql @@ -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; -- 2.11.0