Bugzilla – Attachment 124823 Details for
Bug 28999
Add Koha::AllowList and DenyList
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 28999: Remove (un)load, add reset; apply returns result now
Bug-28999-Remove-unload-add-reset-apply-returns-re.patch (text/plain), 9.33 KB, created by
Marcel de Rooy
on 2021-09-13 06:15:44 UTC
(
hide
)
Description:
Bug 28999: Remove (un)load, add reset; apply returns result now
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2021-09-13 06:15:44 UTC
Size:
9.33 KB
patch
obsolete
>From 10ed783e3686b8e17e4b78446384def5cae6e24f Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Sat, 11 Sep 2021 11:51:38 +0000 >Subject: [PATCH] Bug 28999: Remove (un)load, add reset; apply returns result > now >Content-Type: text/plain; charset=utf-8 > >Remove load but add $defaults in new. >Replace unload by reset, restoring defaults. >Replace warn en verbose by a dump variable in apply. >Do not return $blocked anymore but return the filtered input in apply. >New check method returns true when no entries were blocked. >New keys method to list current allow list entries. >--- > Koha/AllowList.pm | 86 ++++++++++++++++++++++------------------------ > t/Koha/AllowList.t | 69 +++++++++++++++++++++---------------- > 2 files changed, 81 insertions(+), 74 deletions(-) > >diff --git a/Koha/AllowList.pm b/Koha/AllowList.pm >index 551d09a401..6145e43060 100644 >--- a/Koha/AllowList.pm >+++ b/Koha/AllowList.pm >@@ -27,19 +27,21 @@ Koha::AllowList - Allow list implementation base class > > =head3 new > >- my $allow_list = Koha::AllowList->new; >+ my $allow_list = Koha::AllowList->new({ defaults => $list }); > > Constructor. > > =cut > >-our $defaults = []; >- > sub new { >- my ($class, $args) = @_; >- $args //= {}; >- my $self = bless( $args, $class ); >+ my ( $class, $args ) = @_; # args hash with keys: defaults >+ $args = {} if !$args or ref($args) ne 'HASH'; >+ my $defaults = ref($args->{defaults}) eq 'ARRAY' ? $args->{defaults} : []; >+ >+ my $self = bless( {}, $class ); >+ $self->{_defaults} = $defaults; > $self->{_entries} = {}; >+ $self->add( @$defaults ); > return $self; > } > >@@ -51,7 +53,6 @@ sub new { > > sub add { > my ( $self, @args ) = @_; >- # TODO Could be extended by passing things like column3 => { staff => 1 } > foreach my $arg ( @args ) { > $self->{_entries}->{$arg} = 1; > } >@@ -72,16 +73,13 @@ sub remove { > > =head3 apply > >- my $blocked = $allowlist->apply({ >- input => $hashref | $arrayref, verbose => 1, dry_run => 0, >+ my $result = $allowlist->apply({ >+ input => $hashref | $arrayref, dump => $dump, > }); > >- Apply an allowlist to input data. Note that we modify the passed >- reference unless the dry_run flag is true. >- The verbose flag controls the warn statement. >- >- Returns a hash of blocked entries. Might be useful for logging purposes. >- When you pass an arrayref, the return looks like: { blocked1 => 1, blocked2 => 1, .. } >+ Apply an allowlist to input data. Returns result as hash- or arrayref. >+ If you pass a dump hashref, blocked entries will be written. When you >+ pass an arrayref, the dump looks like: { blocked1 => 1, .. } > > =cut > >@@ -89,66 +87,64 @@ sub apply { > my ( $self, $params ) = @_; > my $input = $params->{input} or return; > return if ref($input) ne 'HASH' && ref($input) ne 'ARRAY'; >- my $dry = $params->{dry_run}; > >- my $blocked = {}; >+ my $dump = {}; >+ my $result; > if( ref($input) eq 'ARRAY' ) { >- my ( @ok, @nok ); >- map { $self->{_entries}->{$_} ? push @ok, $_ : push @nok, $_; } @$input; >- @$input = @ok if !$dry; >- $blocked = { map { ( $_ => 1 ) } @nok }; >+ $result = []; >+ map { $self->{_entries}->{$_} ? ( push @$result, $_ ) : ( $dump->{$_} = 1 ); } @$input; > } else { # Handle hashref >- foreach my $key ( keys %$input ) { >- unless ( $self->{_entries}->{$key} ) { >- if( ref($input) eq 'HASH' ) { >- $blocked->{$key} = $dry ? $input->{$key} : delete $input->{$key}; >- } >- } >- } >+ $result = {}; >+ map { $self->{_entries}->{$_} ? ( $result->{$_} = $input->{$_} ) : ( $dump->{$_} = $input->{$_} ); } keys %$input; > } >- warn 'Koha::AllowList: apply blocked one or more keys' if keys %$blocked && $params->{verbose}; >- return $blocked; >+ map { $params->{dump}->{$_} = $dump->{$_}; } keys %$dump if ref($params->{dump}) eq 'HASH'; >+ return $result; > } > > =head3 check > >- my $blocked = $allowlist->check({ input => $a, verbose => 0 }); >+ my $bool = $allowlist->check({ input => $a, dump => $dump }); > >- This is just a shortcut for ->apply({ dry_run => 1 }) >+ This is just a shortcut for running ->apply and checking if nothing was blocked. >+ Optional dump variable serves the same purpose. >+ Returns a boolean (1 or empty string). True means that all input is allowed, >+ but only if there was input. > > =cut > > sub check { > my ( $self, $params ) = @_; >- return $self->apply({ %$params, dry_run => 1 }); >+ my $dump = {}; >+ my $rv = $self->apply({ %$params, dump => $dump }); >+ map { $params->{dump}->{$_} = $dump->{$_}; } keys %$dump if ref($params->{dump}) eq 'HASH'; >+ return $rv ? keys %$dump == 0 : q{}; > } > >-=head3 load >+=head3 reset > >- my $fields = $self->load; >+ $allowlist->reset; > >- Loads default allow list entries. >- >- This routine should be overridden when subclassing. >+ Restore default entries. > > =cut > >-sub load { >+sub reset { > my ( $self ) = @_; >- $self->add( @$defaults ); >+ $self->{_entries} = {}; >+ $self->add( @{$self->{_defaults}} ); > } > >-=head3 unload >+=head3 keys > >- $allowlist->unload; >+ @keys = $allowlist->keys; > >- Clear all entries. >+ Return the allow list entries. > > =cut > >-sub unload { >+sub keys { > my ( $self ) = @_; >- $self->{_entries} = {}; >+ return sort keys %{$self->{_entries}}; > } > > 1; >diff --git a/t/Koha/AllowList.t b/t/Koha/AllowList.t >index c930709324..ace4d96404 100755 >--- a/t/Koha/AllowList.t >+++ b/t/Koha/AllowList.t >@@ -16,49 +16,60 @@ > # along with Koha; if not, see <http://www.gnu.org/licenses>. > > use Modern::Perl; >+use Data::Dumper qw( Dumper ); > use Test::More tests => 1; > > use Koha::AllowList; > > subtest 'AllowList' => sub { >- plan tests => 13; >+ plan tests => 17; > >- my $allowlist = Koha::AllowList->new; >- $allowlist->load; >+ my $defaults = [ 'col1' ]; >+ my $allowlist = Koha::AllowList->new({ defaults => $defaults }); > is( $allowlist->apply, undef, 'No input returns undef' ); >- is( %{$allowlist->apply({ input => {} })}, 0, 'Empty hash returns nothing blocked' ); >+ is( %{$allowlist->apply({ input => {} })}, 0, 'Empty hash returns empty hash' ); > >- # Test return from apply on hashrefs >- my @input = ( col1 => 1, col2 => 2, col3 => 3, col4 => 4 ); >- my $blocked = $allowlist->apply({ input => { @input } }); >+ # Test apply on hashrefs >+ my $input = { col1 => 1, col2 => 2, col3 => 3, col4 => 4 }; >+ $allowlist->remove('col1'); # results in empty allow list >+ my $blocked = {}; >+ $allowlist->apply({ input => $input, dump => $blocked }); > is( keys %$blocked, 4, 'Empty list blocks all' ); >- $allowlist->add( 'col1' ); >- $blocked = $allowlist->apply({ input => { @input } }); >+ $allowlist->reset; # Back to col1 >+ $blocked = {}; >+ $allowlist->apply({ input => $input, dump => $blocked }); > is( keys %$blocked, 3, 'One field allowed' ); > is( $blocked->{col1}, undef, 'And that is col1' ); >- $allowlist->remove( 'col1' ); >- $blocked = $allowlist->apply({ input => { @input } }); >- is( keys %$blocked, 4, 'Back to 4 after removing col1' ); > $allowlist->add( 'col2' ); >- $allowlist->unload; >- $blocked = $allowlist->apply({ input => { @input } }); >- is( keys %$blocked, 4, 'Same after unloading' ); >+ $blocked = {}; >+ my $result = $allowlist->apply({ input => $input, dump => $blocked }); >+ is( keys %$blocked, 2, 'Two fields blocked' ); >+ is( keys %$result, 2, 'Two fields allowed' ); >+ is( $result->{col2}, 2, 'Check a result' ); >+ $allowlist->reset; # Undo col2 >+ $blocked = {}; >+ $allowlist->apply({ input => $input, dump => $blocked }); >+ is( keys %$blocked, 3, 'Back to three blocked' ); > >- # Test dry run >- my $input2 = { col1 => 1, col2 => 2, col3 => 3, col4 => 4 }; >+ # Test check method >+ $blocked = {}; >+ $input = { col1 => 1, col2 => 2, col3 => 3, col4 => 4 }; > $allowlist->add( 'col1', 'col3' ); >- $blocked = $allowlist->check({ input => $input2 }); >- is( keys %$blocked, 2, 'Expected two entries' ); >- is( keys %$input2, 4, 'Expected no change in input2' ); >- $blocked = $allowlist->apply({ input => $input2 }); >- is( keys %$input2, 2, 'Apply removed entries now' ); >+ is( $allowlist->check({ input => $input, dump => $blocked }), q{}, 'Check returns false' ); >+ is( keys %$blocked, 2, 'Dumped 2 entries' ); >+ $allowlist->add( 'col2', 'col4' ); >+ is( $allowlist->check({ input => $input }), 1, 'Check returns true' ); > >- # Test array >- @input = ( 'col1', 'col2', 'col3', 'col4', 'col5' ); >- $blocked = $allowlist->check({ input => [ @input ] }); >+ # Test on arrays >+ $allowlist->remove( 'col2', 'col3' ); >+ $input = [ 'col1', 'col2', 'col3', 'col4', 'col5' ]; >+ $blocked = {}; >+ $result = $allowlist->apply({ input => $input, dump => $blocked }); >+ is( @$result, 2, 'Expected 2 allowed entries' ); >+ is( ( grep { $_ eq 'col1' } @$result ), 1, 'col1 allowed' ); > is( keys %$blocked, 3, 'Expected three blocked entries' ); >- $input2 = [ @input ]; >- $blocked = $allowlist->apply({ input => $input2 }); >- is( keys %$blocked, 3, 'Expected three blocked entries again' ); >- is( @$input2, 2, 'Expect two entries now in input2' ); >+ is( $blocked->{col2}, 1, 'col2 blocked' ); >+ >+ # Test keys method >+ is( join(',', $allowlist->keys), 'col1,col4', 'List current entries' ); > }; >-- >2.20.1
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 28999
:
124820
|
124821
|
124822
| 124823 |
124824
|
124828