From 6f9fb7c575c1b1fca820ae0481df5d3cdcdd0d4e Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@biblibre.com>
Date: Thu, 26 Sep 2013 12:39:30 +0200
Subject: [PATCH] Bug 8015: Get rid of the eval in ModifyRecordWithTemplate
This patch removes the use of eval in the
C4::MarcModificationTemplates::ModifyRecordWithTemplate routine.
Now this routine call the wanted modification routine with the list of
parameters.
This call is done only if the condition is respected.
---
C4/MarcModificationTemplates.pm | 179 ++++++++++++++++------------
Koha/SimpleMARC.pm | 2 +-
t/SimpleMARC.t | 7 +-
t/db_dependent/MarcModificationTemplates.t | 140 +++++++++++++++++++++-
4 files changed, 245 insertions(+), 83 deletions(-)
diff --git a/C4/MarcModificationTemplates.pm b/C4/MarcModificationTemplates.pm
index 2c7bb97..ef48c3d 100644
--- a/C4/MarcModificationTemplates.pm
+++ b/C4/MarcModificationTemplates.pm
@@ -492,82 +492,111 @@ sub ModifyRecordsWithTemplate {
=cut
sub ModifyRecordWithTemplate {
- my ( $template_id, $record ) = @_;
- C4::Koha::Log( "C4::MarcModificationTemplates::ModifyRecordWithTemplate( $template_id, $record )" ) if DEBUG;
- warn( "C4::MarcModificationTemplates::ModifyRecordWithTemplate( $template_id, $record )" ) if DEBUG;
- C4::Koha::Log( "Unmodified Record:\n" . $record->as_formatted() ) if DEBUG >= 10;
- warn( "Unmodified Record:\n" . $record->as_formatted() ) if DEBUG >= 10;
-
- my $current_date = DateTime->now()->ymd();
- my $branchcode = C4::Context->userenv->{branch};
-
- my @actions = GetModificationTemplateActions( $template_id );
-
- foreach my $a ( @actions ) {
- my $action = $a->{'action'};
- my $field_number = $a->{'field_number'};
- my $from_field = $a->{'from_field'};
- my $from_subfield = $a->{'from_subfield'};
- my $field_value = $a->{'field_value'};
- my $to_field = $a->{'to_field'};
- my $to_subfield = $a->{'to_subfield'};
- my $to_regex = $a->{'to_regex'};
- my $conditional = $a->{'conditional'};
- my $conditional_field = $a->{'conditional_field'};
- my $conditional_subfield = $a->{'conditional_subfield'};
- my $conditional_comparison = $a->{'conditional_comparison'};
- my $conditional_value = $a->{'conditional_value'};
- my $conditional_regex = $a->{'conditional_regex'};
-
- my $eval = "$action( \$record, '$from_field', '$from_subfield', ";
-
- if ( $field_value ) {
- C4::Koha::Log( "Field value before replacements: $field_value" ) if ( DEBUG >= 3 );
- warn( "Field value before replacements: $field_value" ) if ( DEBUG >= 3 );
-
- $field_value =~ s/__CURRENTDATE__/$current_date/g;
- $field_value =~ s/__BRANCHCODE__/$branchcode/g;
-
- $eval .= " undef, " if ( $action eq 'update_field' );
- $eval .= " '$field_value' ";
-
- C4::Koha::Log( "Field value after replacements: $field_value" ) if ( DEBUG >= 3 );
- warn( "Field value after replacements: $field_value" ) if ( DEBUG >= 3 );
- } elsif ( $to_field ) {
- $eval .= " '$to_field', '$to_subfield', '$to_regex' ";
- }
-
- $eval .= ", '$field_number' " if ( $field_number );
- $eval .= ') ';
-
- if ( $conditional ) {
- $eval .= " $conditional ( ";
-
- if ( $conditional_comparison eq 'exists' ) {
- $eval .= "field_exists( \$record, '$conditional_field', '$conditional_subfield' )";
-
- } elsif ( $conditional_comparison eq 'not_exists' ) {
- $eval .= "!field_exists( \$record, '$conditional_field', '$conditional_subfield' )";
-
- } elsif ( $conditional_comparison eq 'equals' ) {
- $eval .= "field_equals( \$record, '$conditional_value', '$conditional_field', '$conditional_subfield', '$conditional_regex' ) ";
-
- } elsif ( $conditional_comparison eq 'not_equals' ) {
- $eval .= "!field_equals( \$record, '$conditional_value', '$conditional_field', '$conditional_subfield', '$conditional_regex' ) ";
- }
-
- $eval .= " )";
+ my ( $template_id, $record ) = @_;
+ C4::Koha::Log( "C4::MarcModificationTemplates::ModifyRecordWithTemplate( $template_id, $record )" ) if DEBUG;
+ warn( "C4::MarcModificationTemplates::ModifyRecordWithTemplate( $template_id, $record )" ) if DEBUG;
+ C4::Koha::Log( "Unmodified Record:\n" . $record->as_formatted() ) if DEBUG >= 10;
+ warn( "Unmodified Record:\n" . $record->as_formatted() ) if DEBUG >= 10;
+
+ my $current_date = DateTime->now()->ymd();
+ my $branchcode = C4::Context->userenv->{branch};
+
+ my @actions = GetModificationTemplateActions( $template_id );
+
+ foreach my $a ( @actions ) {
+ my $action = $a->{'action'};
+ my $field_number = $a->{'field_number'};
+ my $from_field = $a->{'from_field'};
+ my $from_subfield = $a->{'from_subfield'};
+ my $field_value = $a->{'field_value'};
+ my $to_field = $a->{'to_field'};
+ my $to_subfield = $a->{'to_subfield'};
+ my $to_regex = $a->{'to_regex'};
+ my $conditional = $a->{'conditional'};
+ my $conditional_field = $a->{'conditional_field'};
+ my $conditional_subfield = $a->{'conditional_subfield'};
+ my $conditional_comparison = $a->{'conditional_comparison'};
+ my $conditional_value = $a->{'conditional_value'};
+ my $conditional_regex = $a->{'conditional_regex'};
+
+ if ( $field_value ) {
+ $field_value =~ s/__CURRENTDATE__/$current_date/g;
+ $field_value =~ s/__BRANCHCODE__/$branchcode/g;
+ }
+
+ my @params = ( $record, $from_field, $from_subfield );
+ if ( $action eq 'update_field' ) {
+ push @params,
+ ( $field_value
+ ? ( undef, $field_value )
+ : ()
+ );
+ } else {
+ push @params,
+ ( $field_value
+ ? $field_value
+ : ()
+ );
+ }
+ push @params, (
+ ( ( not $field_value and $to_field )
+ ? ( $to_field, $to_subfield, $to_regex )
+ : () ),
+ ( $field_number
+ ? $field_number
+ : () )
+ );
+
+ my $do = 1;
+ if ( $conditional ) {
+ for ( $conditional_comparison ) {
+ when ( /^exists$/ ) {
+ my $exists = field_exists( $record, $conditional_field, $conditional_subfield );
+ $do = $conditional eq 'if'
+ ? $exists
+ : not $exists;
+ }
+ when ( /^not_exists$/ ) {
+ my $exists = field_exists( $record, $conditional_field, $conditional_subfield );
+ $do = $conditional eq 'if'
+ ? not $exists
+ : $exists;
+ }
+ when ( /^equals$/ ) {
+ my $equals = field_equals( $record, $conditional_value, $conditional_field, $conditional_subfield, $conditional_regex );
+ $do = $conditional eq 'if'
+ ? $equals
+ : not $equals;
+ }
+ when ( /^not_equals$/ ) {
+ my $equals = field_equals( $record, $conditional_value, $conditional_field, $conditional_subfield, $conditional_regex );
+ $do = $conditional eq 'if'
+ ? not $equals
+ : $equals;
+ }
+ }
+ }
+
+ if ( $do ) {
+ for ( $action ) {
+ when ( /^copy_field$/ ) {
+ copy_field( @params );
+ }
+ when ( /^update_field$/ ) {
+ update_field( @params );
+ }
+ when ( /^move_field$/ ) {
+ move_field( @params );
+ }
+ when ( /^delete_field$/ ) {
+ delete_field( @params );
+ }
+ }
+ }
+
+ C4::Koha::Log( $record->as_formatted() ) if DEBUG >= 10;
+ warn( $record->as_formatted() ) if DEBUG >= 10;
}
-
- $eval .= ";";
-
- C4::Koha::Log("eval $eval") if DEBUG >= 2;
- warn("eval $eval") if DEBUG >= 2;
- eval {$eval};
- C4::Koha::Log( $record->as_formatted() ) if DEBUG >= 10;
- warn( $record->as_formatted() ) if DEBUG >= 10;
-
- }
}
1;
__END__
diff --git a/Koha/SimpleMARC.pm b/Koha/SimpleMARC.pm
index ef40694..e78fc92 100644
--- a/Koha/SimpleMARC.pm
+++ b/Koha/SimpleMARC.pm
@@ -125,7 +125,7 @@ sub update_field {
$field->update( "$subfieldName" => $values[$i++] );
}
}
- if ( $i <= scalar @values - 1 ) {
+ if ( $i <= scalar ( @values ) - 1 ) {
foreach my $field ( @fields ) {
foreach my $j ( $i .. scalar( @values ) - 1) {
$field->add_subfields( "$subfieldName" => $values[$j] );
diff --git a/t/SimpleMARC.t b/t/SimpleMARC.t
index cfba615..def2edb 100644
--- a/t/SimpleMARC.t
+++ b/t/SimpleMARC.t
@@ -1,14 +1,11 @@
use Modern::Perl;
-use Test::More;#tests => 1;
-
-use Data::Dumper;
+use Test::More tests => 33;
use_ok("MARC::Field");
use_ok("MARC::Record");
use_ok("Koha::SimpleMARC");
-
sub new_record {
my $record = MARC::Record->new;
$record->leader('03174nam a2200445 a 4500');
@@ -184,5 +181,3 @@ move_field( $record, '952', 'p', '954', 'p' ); # Move all field
is_deeply( \@fields_952p, [], 'All 952$p have moved' );
is_deeply( \@fields_954p, ['3010023917', '3010023917'], 'Now 2 954$p exist' );
-
-done_testing;
diff --git a/t/db_dependent/MarcModificationTemplates.t b/t/db_dependent/MarcModificationTemplates.t
index d253b82..c31983e 100644
--- a/t/db_dependent/MarcModificationTemplates.t
+++ b/t/db_dependent/MarcModificationTemplates.t
@@ -1,9 +1,18 @@
use Modern::Perl;
-use Test::More tests => 66;
+use Test::More tests => 74;
+use_ok("MARC::Field");
+use_ok("MARC::Record");
use_ok("C4::MarcModificationTemplates");
+my $dbh = C4::Context->dbh;
+$dbh->{AutoCommit} = 0;
+$dbh->{RaiseError} = 1;
+
+$dbh->do(q|DELETE FROM marc_modification_template_actions|);
+$dbh->do(q|DELETE FROM marc_modification_templates|);
+
# Creation
my $template_id = AddModificationTemplate("template_name");
like( $template_id, qr|^\d+$|, "new template returns an id" );
@@ -123,3 +132,132 @@ is( GetModificationTemplateAction( $actions[1]->{mmta_id} ), undef, "second acti
is( GetModificationTemplateAction( $actions[2]->{mmta_id} ), undef, "third action does not exist anymore" );
is( GetModificationTemplateActions( $template_id ), 0, "There is no action for deleted template" );
+
+# ModifyRecordWithTemplate
+my @USERENV = (
+ 1,
+ 'test',
+ 'MASTERTEST',
+ 'Test',
+ 'Test',
+ 't',
+ 'Test',
+ 0,
+);
+C4::Context->_new_userenv ('DUMMY_SESSION_ID');
+C4::Context->set_userenv ( @USERENV );
+
+$template_id = AddModificationTemplate("template_name");
+like( $template_id, qr|^\d+$|, "new template returns an id" );
+
+is( AddModificationTemplateAction(
+ $template_id, 'copy_field', 0,
+ '245', 'a', '',
+ '246', 'a', '',
+ '', '', '', '', '', '', '',
+ 'copy field 245$a to 246$a'
+), 1, 'Add first action: copy 245$a to 246$a');
+
+is( AddModificationTemplateAction(
+ $template_id, 'delete_field', 0,
+ '650', 'a', '',
+ '', '', '',
+ 'if', '650', '9', 'equals', '462', '',
+ 'Delete field 650$a if 650$9=462'
+), 1, 'Add second action: delete field 650$a if 650$9=462');
+
+is( AddModificationTemplateAction(
+ $template_id, 'update_field', 0,
+ '952', 'p', '3010023917_updated',
+ '', '', '',
+ 'unless', '650', '9', 'equals', '42', '',
+ 'Update field 952$p with "3010023917_updated" if 650$9 != 42'
+), 1, 'Add third action: update field 952$p with "3010023917_updated" if 650$9 != 42');
+
+is( AddModificationTemplateAction(
+ $template_id, 'move_field', 0,
+ '952', 'd', '',
+ '952', 'e', '',
+ 'if', '952', 'c', 'equals', '/^GEN/', '1',
+ 'Move field 952$d to 952$e if 952$c =~ /^GE/'
+), 1, 'Add fourth action: move field 952$d to 952$e if 952$c =~ /^GE/');
+
+my $record = new_record();
+
+ModifyRecordWithTemplate( $template_id, $record );
+
+my $expected_record = expected_record();
+is_deeply( $record, $expected_record );
+
+done_testing;
+
+sub new_record {
+ my $record = MARC::Record->new;
+ $record->leader('03174nam a2200445 a 4500');
+ my @fields = (
+ MARC::Field->new(
+ 100, '1', ' ',
+ a => 'Knuth, Donald Ervin',
+ d => '1938',
+ ),
+ MARC::Field->new(
+ 245, '1', '4',
+ a => 'The art of computer programming',
+ c => 'Donald E. Knuth.',
+ ),
+ MARC::Field->new(
+ 650, ' ', '0',
+ a => 'Computer programming.',
+ 9 => '462',
+ ),
+ MARC::Field->new(
+ 952, ' ', ' ',
+ p => '3010023917',
+ y => 'BK',
+ c => 'GEN',
+ d => '2001-06-25',
+ ),
+ );
+ $record->append_fields(@fields);
+ return $record;
+}
+
+sub expected_record {
+ my $record = MARC::Record->new;
+ $record->leader('03174nam a2200445 a 4500');
+ my @fields = (
+ MARC::Field->new(
+ 100, '1', ' ',
+ a => 'Knuth, Donald Ervin',
+ d => '1938',
+ ),
+ MARC::Field->new(
+ 245, '1', '4',
+ a => 'The art of computer programming',
+ c => 'Donald E. Knuth.',
+ ),
+ MARC::Field->new(
+ 650, ' ', '0',
+ 9 => '462',
+ ),
+ MARC::Field->new(
+ 952, ' ', ' ',
+ p => '3010023917_updated',
+ y => 'BK',
+ c => 'GEN',
+ e => '2001-06-25',
+ ),
+ MARC::Field->new(
+ 246, '', ' ',
+ a => 'The art of computer programming',
+ ),
+ );
+ $record->append_fields(@fields);
+ return $record;
+}
+
+
+# C4::Context->userenv
+sub Mock_userenv {
+ return { branchcode => 'CPL' };
+}
--
1.7.10.4