From 2309aa93bcd828f2b166b7cb195d9f56b85d52e0 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Sat, 11 Sep 2021 11:51:38 +0000 Subject: [PATCH] Bug 28999: Additional check method, some parameter changes Content-Type: text/plain; charset=utf-8 License for module. Remove interface parameter in new. Add dry_run parameter to apply. Add arrayref as input for apply. Add check as shortcut for apply with dry_run. Add tests. --- Koha/AllowList.pm | 78 ++++++++++++++++++++++++++++++++++------------ t/Koha/AllowList.t | 27 +++++++++++++--- 2 files changed, 81 insertions(+), 24 deletions(-) diff --git a/Koha/AllowList.pm b/Koha/AllowList.pm index e563dcf3f3..551d09a401 100644 --- a/Koha/AllowList.pm +++ b/Koha/AllowList.pm @@ -1,10 +1,25 @@ -package Koha::Allowlist; +package Koha::AllowList; + +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . use Modern::Perl; =head1 NAME -Koha::Allowlist - Allowlist implementation base class +Koha::AllowList - Allow list implementation base class =head1 API @@ -12,22 +27,18 @@ Koha::Allowlist - Allowlist implementation base class =head3 new - my $allow_list = Koha::Allowlist->new({ interface => 'staff' }); + my $allow_list = Koha::AllowList->new; Constructor. - Interface must be 'staff' or 'opac' - defaults to 'opac' - =cut +our $defaults = []; + sub new { my ($class, $args) = @_; - $args = {} unless defined $args; - my $self = bless ($args, $class); - $self->{interface} = - ( $args->{interface} && $args->{interface} eq 'staff' ) - ? 'staff' - : 'opac'; + $args //= {}; + my $self = bless( $args, $class ); $self->{_entries} = {}; return $self; } @@ -61,29 +72,57 @@ sub remove { =head3 apply - my $blocked = $allowlist->apply({ input => $hashref, verbose => 1 }); + my $blocked = $allowlist->apply({ + input => $hashref | $arrayref, verbose => 1, dry_run => 0, + }); - Apply an allowlist to input data. Note that we modify the $hashref argument! + 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? + 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, .. } =cut 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 = {}; - foreach my $key ( keys %$input ){ - unless ( $self->{_entries}->{$key} ) { - $blocked->{$key} = delete $input->{ $key }; + if( ref($input) eq 'ARRAY' ) { + my ( @ok, @nok ); + map { $self->{_entries}->{$_} ? push @ok, $_ : push @nok, $_; } @$input; + @$input = @ok if !$dry; + $blocked = { map { ( $_ => 1 ) } @nok }; + } 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}; + } + } } } - warn 'Koha::Allowlist: apply blocked one or more keys' if keys %$blocked && $params->{verbose}; + warn 'Koha::AllowList: apply blocked one or more keys' if keys %$blocked && $params->{verbose}; return $blocked; } +=head3 check + + my $blocked = $allowlist->check({ input => $a, verbose => 0 }); + + This is just a shortcut for ->apply({ dry_run => 1 }) + +=cut + +sub check { + my ( $self, $params ) = @_; + return $self->apply({ %$params, dry_run => 1 }); +} + =head3 load my $fields = $self->load; @@ -96,8 +135,7 @@ sub apply { sub load { my ( $self ) = @_; - # You could do something here like: - # $self->add( 'myfield' ); + $self->add( @$defaults ); } =head3 unload diff --git a/t/Koha/AllowList.t b/t/Koha/AllowList.t index e488ab063a..c930709324 100755 --- a/t/Koha/AllowList.t +++ b/t/Koha/AllowList.t @@ -18,16 +18,17 @@ use Modern::Perl; use Test::More tests => 1; -use Koha::Allowlist; +use Koha::AllowList; -subtest 'Allowlist' => sub { - plan tests => 7; +subtest 'AllowList' => sub { + plan tests => 13; - my $allowlist = Koha::Allowlist->new; + my $allowlist = Koha::AllowList->new; $allowlist->load; is( $allowlist->apply, undef, 'No input returns undef' ); is( %{$allowlist->apply({ input => {} })}, 0, 'Empty hash returns nothing blocked' ); + # Test return from apply on hashrefs my @input = ( col1 => 1, col2 => 2, col3 => 3, col4 => 4 ); my $blocked = $allowlist->apply({ input => { @input } }); is( keys %$blocked, 4, 'Empty list blocks all' ); @@ -42,4 +43,22 @@ subtest 'Allowlist' => sub { $allowlist->unload; $blocked = $allowlist->apply({ input => { @input } }); is( keys %$blocked, 4, 'Same after unloading' ); + + # Test dry run + my $input2 = { 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' ); + + # Test array + @input = ( 'col1', 'col2', 'col3', 'col4', 'col5' ); + $blocked = $allowlist->check({ input => [ @input ] }); + 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' ); }; -- 2.20.1