Bugzilla – Attachment 76382 Details for
Bug 20930
Validate and cache parsed YAML/JSON type system preferences
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20930: Dump and load system preferences of YAML/JSON type
Bug-20930-Dump-and-load-system-preferences-of-YAML.patch (text/plain), 8.65 KB, created by
Lari Taskula
on 2018-06-25 14:58:45 UTC
(
hide
)
Description:
Bug 20930: Dump and load system preferences of YAML/JSON type
Filename:
MIME Type:
Creator:
Lari Taskula
Created:
2018-06-25 14:58:45 UTC
Size:
8.65 KB
patch
obsolete
>From 26902fc30ec55f2a1d699ad42d4a916c0de33fcd Mon Sep 17 00:00:00 2001 >From: Lari Taskula <lari.taskula@jns.fi> >Date: Wed, 13 Jun 2018 15:35:20 +0300 >Subject: [PATCH] Bug 20930: Dump and load system preferences of YAML/JSON type > >Currently, C4::Context->preference('YAMLpref') returns a string that needs to be >parsed by any code using the preference. > >My suggestion is to return a parsed version instead (a hash/array ref or a string) >and using the syspref cache with the parsed value. Like this, we could: >- centralize parsing of YAML system preferences >- optimize use of system preference cache by caching the parsed version instead. > Parse once and store in cache; no need to re-parse on each call! > >We can centralize this functionality into Koha::Config::SysPref->value() (used by >C4::Context->preference). Use database column systempreferences.type with value of >"YAML" to trigger this functionality in "value()"-method. > >To test: >1. prove t/db_dependent/Context.t >--- > C4/Context.pm | 2 +- > Koha/Config/SysPref.pm | 107 +++++++++++++++++++++++++++++++++++++++++++++- > Koha/Exceptions/Config.pm | 5 +++ > t/db_dependent/Context.t | 79 ++++++++++++++++++++++++++++++++++ > 4 files changed, 190 insertions(+), 3 deletions(-) > >diff --git a/C4/Context.pm b/C4/Context.pm >index 41bce3b..bed381a 100644 >--- a/C4/Context.pm >+++ b/C4/Context.pm >@@ -533,7 +533,7 @@ sub set_preference { > } > > if ( $use_syspref_cache ) { >- $syspref_cache->set_in_cache( "syspref_$variable", $value ); >+ $syspref_cache->set_in_cache( "syspref_$variable", $syspref->value ); > } > > return $syspref; >diff --git a/Koha/Config/SysPref.pm b/Koha/Config/SysPref.pm >index 1e71b8c..bca2303 100644 >--- a/Koha/Config/SysPref.pm >+++ b/Koha/Config/SysPref.pm >@@ -20,8 +20,10 @@ package Koha::Config::SysPref; > use Modern::Perl; > > use Carp; >+use YAML::Syck; > > use Koha::Database; >+use Koha::Exceptions::Config; > > use C4::Log; > >@@ -46,9 +48,17 @@ sub store { > > my $action = $self->in_storage ? 'MODIFY' : 'ADD'; > >- C4::Log::logaction( 'SYSTEMPREFERENCE', $action, undef, $self->variable . ' | ' . $self->value ); >+ my $value = $self->value; >+ if (ref($value)) { >+ $value = $self->_dump_datatypes($value); >+ } > >- return $self->SUPER::store($self); >+ C4::Log::logaction( 'SYSTEMPREFERENCE', $action, undef, $self->variable . ' | ' . $value ); >+ >+ $self->SUPER::store($self); >+ $self->set({ value => $self->_load_datatypes($value) }) unless ref($value); >+ >+ return $self; > } > > =head3 delete >@@ -67,6 +77,99 @@ sub delete { > return $deleted; > } > >+=head3 value >+ >+=cut >+ >+sub value { >+ my $self = shift; >+ >+ my $value = $self->SUPER::value(@_); >+ return $value unless $value; >+ >+ $self->_load_datatypes($value); >+ >+ return $value; >+} >+ >+=head3 _dump_datatypes >+ >+=cut >+ >+sub _dump_datatypes { >+ my ($self, $value) = @_; >+ >+ $value = $self->_dump_yaml_or_json($value); >+ >+ return $value; >+} >+ >+=head3 _load_datatypes >+ >+=cut >+ >+sub _load_datatypes { >+ my ($self, $value) = @_; >+ >+ $value = $self->_load_yaml_or_json($value); >+ >+ return $value; >+} >+ >+=head3 _load_yaml_or_json >+ >+Loads system preference of YAML / JSON type. >+ >+Throws Koha::Exceptions::Config::InvalidSyntax if not valid YAML / JSON. >+ >+=cut >+ >+sub _load_yaml_or_json { >+ my ($self, $value) = @_; >+ >+ my $type = defined $self->type ? lc($self->type) : ''; >+ return $value if $type ne 'yaml' and $type ne 'json'; >+ if (!ref($value)) { >+ eval { $value = Load($value); }; >+ } else { >+ return $value; >+ } >+ if ($@) { >+ Koha::Exceptions::Config::InvalidSyntax->throw( >+ error => $@, >+ value => $value, >+ ); >+ } >+ return $value; >+} >+ >+=head3 _dump_yaml_or_json >+ >+Dumps system preference of YAML / JSON type. >+ >+Throws Koha::Exceptions::Config::InvalidSyntax if not valid YAML / JSON. >+ >+=cut >+ >+sub _dump_yaml_or_json { >+ my ($self, $value) = @_; >+ >+ my $type = defined $self->type ? lc($self->type) : ''; >+ return $value if $type ne 'yaml' and $type ne 'json'; >+ if (ref($value)) { >+ eval { $value = Dump($value); }; >+ } else { >+ return $value; >+ } >+ if ($@) { >+ Koha::Exceptions::Config::InvalidSyntax->throw( >+ error => $@, >+ value => $value, >+ ); >+ } >+ return $value; >+} >+ > =head3 type > > =cut >diff --git a/Koha/Exceptions/Config.pm b/Koha/Exceptions/Config.pm >index 14ae315..5785935 100644 >--- a/Koha/Exceptions/Config.pm >+++ b/Koha/Exceptions/Config.pm >@@ -7,6 +7,11 @@ use Exception::Class ( > 'Koha::Exceptions::Config' => { > description => 'Something went wrong!', > }, >+ 'Koha::Exceptions::Config::InvalidSyntax' => { >+ isa => 'Koha::Exceptions::Config', >+ description => 'The configuration value has invalid syntax', >+ fields => ['value'], >+ }, > 'Koha::Exceptions::Config::MissingEntry' => { > isa => 'Koha::Exceptions::Config', > description => 'The required entry is missing in the configuration file' >diff --git a/t/db_dependent/Context.t b/t/db_dependent/Context.t >index cb7c94f..c31aa01 100755 >--- a/t/db_dependent/Context.t >+++ b/t/db_dependent/Context.t >@@ -8,6 +8,7 @@ use vars qw($debug $koha $dbh $config $ret); > use t::lib::Mocks; > > use Koha::Database; >+use Koha::Caches; > > BEGIN { > $debug = $ENV{DEBUG} || 0; >@@ -48,6 +49,84 @@ my $SillyPeference = C4::Context->preference('SillyPreference'); > is($SillyPeference,'random','SillyPreference saved as specified'); > C4::Context->clear_syspref_cache(); > C4::Context->enable_syspref_cache(); >+ >+subtest 'test system preference of type YAML or JSON' => sub { >+ my ($hashref, $arrayref, $string, $pref); >+ my $cache = Koha::Caches->get_instance('syspref'); >+ >+ subtest 'Invalid syntax' => sub { >+ my $notyaml = ': - asd'; >+ my $notjson = '{"test"": { "fail\' ]}'; >+ eval { >+ C4::Context->set_preference('YAMLpref', $notyaml, '', 'yaml', ''); >+ }; >+ is(ref($@), 'Koha::Exceptions::Config::InvalidSyntax', >+ 'Invalid YAML syntax'); >+ eval { >+ C4::Context->set_preference('YAMLpref', $notjson, '', 'json', ''); >+ }; >+ is(ref($@), 'Koha::Exceptions::Config::InvalidSyntax', >+ 'Invalid JSON syntax'); >+ C4::Context->set_preference('Normalpref', $notyaml, '', 'Free', ''); >+ is(C4::Context->preference('Normalpref'), $notyaml, 'Does not affect ' >+ .'preferences with type != json ||Â yaml'); >+ }; >+ >+ subtest 'test YAML' => sub { >+ $hashref = qq/--- >+ test: >+ test: 1 >+ /; >+ $arrayref = qq/--- >+ [much, elements, in, this, very, dogearray] >+ /; >+ $string = qq/--- >+ wat >+ /; >+ >+ # Check HASHref >+ C4::Context->set_preference('YAMLpref', $hashref, '', 'yaml', ''); >+ $pref = C4::Context->preference('YAMLpref'); >+ is($pref->{test}->{test}, 1, 'Got hashref'); >+ is_deeply($cache->get_from_cache("syspref_yamlpref"), $pref, 'is_deeply'); >+ >+ # Check ARRAYref >+ C4::Context->set_preference('YAMLpref', $arrayref, '', 'yaml', ''); >+ $pref = C4::Context->preference('YAMLpref'); >+ is($pref->[5], 'dogearray', 'Got arrayref'); >+ is_deeply($cache->get_from_cache("syspref_yamlpref"), $pref, 'is_deeply'); >+ >+ # Check string >+ C4::Context->set_preference('YAMLpref', $string, '', 'yaml', ''); >+ $pref = C4::Context->preference('YAMLpref'); >+ is($pref, 'wat', 'Got string'); >+ is_deeply($cache->get_from_cache("syspref_yamlpref"), $pref, 'is_deeply'); >+ }; >+ subtest 'test JSON' => sub { >+ $hashref = qq/{"test": {"test": 1}}/; >+ $arrayref = qq/["much", "elements", "in", "this", "very", "dogearray"]/; >+ $string = qq/"wat"/; >+ >+ # Check HASHref >+ C4::Context->set_preference('JSONpref', $hashref, '', 'json', ''); >+ $pref = C4::Context->preference('JSONpref'); >+ is($pref->{test}->{test}, 1, 'Got hashref'); >+ is_deeply($cache->get_from_cache("syspref_jsonpref"), $pref, 'is_deeply'); >+ >+ # Check ARRAYref >+ C4::Context->set_preference('JSONpref', $arrayref, '', 'json', ''); >+ $pref = C4::Context->preference('JSONpref'); >+ is($pref->[5], 'dogearray', 'Got arrayref'); >+ is_deeply($cache->get_from_cache("syspref_jsonpref"), $pref, 'is_deeply'); >+ >+ # Check string >+ C4::Context->set_preference('JSONpref', $string, '', 'json', ''); >+ $pref = C4::Context->preference('JSONpref'); >+ is($pref, 'wat', 'Got string'); >+ is_deeply($cache->get_from_cache("syspref_jsonpref"), $pref, 'is_deeply'); >+ }; >+}; >+ > $dbh->rollback; > > ok($koha = C4::Context->new, 'C4::Context->new'); >-- >2.7.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 20930
:
76382
|
76383
|
76384
|
91861
|
91862
|
91863
|
94412
|
94413
|
96108
|
96109
|
96110
|
96111
|
96112
|
96817
|
96818
|
96819
|
96820
|
96821