From 052049160f60a346273eb5ae0b08133a9f0cba55 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 26 Sep 2013 15:35:42 +0200 Subject: [PATCH] Bug 8015: Get rid of the eval for substitution Before this patch, the regex substitution was contain into only one variable (e.g. my $regex = "/foo/bar/i"). Now each member of the regex is stored into a field in the marc_modification_template_actions sql table. In order to avoid a complex code, only modifiers i and g are take into account. Note: If you already add the mmta table, you have to drop it. This patch also adds a foreign key from mmta to mmt tables. --- C4/MarcModificationTemplates.pm | 44 +++++++++++++------- Koha/SimpleMARC.pm | 37 +++++++++++----- installer/data/mysql/kohastructure.sql | 7 +++- installer/data/mysql/updatedatabase.pl | 7 +++- .../modules/tools/marc_modification_templates.tt | 24 +++++++---- t/SimpleMARC.t | 30 +++++++++---- tools/marc_modification_templates.pl | 12 ++++-- 7 files changed, 114 insertions(+), 47 deletions(-) diff --git a/C4/MarcModificationTemplates.pm b/C4/MarcModificationTemplates.pm index ef48c3d..3e8e522 100644 --- a/C4/MarcModificationTemplates.pm +++ b/C4/MarcModificationTemplates.pm @@ -127,7 +127,9 @@ sub AddModificationTemplate { $action->{'field_value'}, $action->{'to_field'}, $action->{'to_subfield'}, - $action->{'to_regex'}, + $action->{'to_regex_search'}, + $action->{'to_regex_replace'}, + $action->{'to_regex_modifiers'}, $action->{'conditional'}, $action->{'conditional_field'}, $action->{'conditional_subfield'}, @@ -210,7 +212,7 @@ sub GetModificationTemplateActions { AddModificationTemplateAction( $template_id, $action, $field_number, $from_field, $from_subfield, $field_value, - $to_field, $to_subfield, $to_regex, + $to_field, $to_subfield, $to_regex_search, $to_regex_replace, $to_regex_modifiers $conditional, $conditional_field, $conditional_subfield, $conditional_comparison, $conditional_value, $conditional_regex, $description @@ -230,7 +232,9 @@ sub AddModificationTemplateAction { $field_value, $to_field, $to_subfield, - $to_regex, + $to_regex_search, + $to_regex_replace, + $to_regex_modifiers, $conditional, $conditional_field, $conditional_subfield, @@ -242,11 +246,11 @@ sub AddModificationTemplateAction { C4::Koha::Log( "C4::MarcModificationTemplates::AddModificationTemplateAction( $template_id, $action, $field_number, $from_field, $from_subfield, $field_value, $to_field, $to_subfield, - $to_regex, $conditional, $conditional_field, $conditional_subfield, $conditional_comparison, + $to_regex_search, $to_regex_replace, $to_regex_modifiers, $conditional, $conditional_field, $conditional_subfield, $conditional_comparison, $conditional_value, $conditional_regex, $description )" ) if DEBUG; warn( "C4::MarcModificationTemplates::AddModificationTemplateAction( $template_id, $action, $field_number, $from_field, $from_subfield, $field_value, $to_field, $to_subfield, - $to_regex, $conditional, $conditional_field, $conditional_subfield, $conditional_comparison, + $to_regex_search, $to_regex_replace, $to_regex_modifiers, $conditional, $conditional_field, $conditional_subfield, $conditional_comparison, $conditional_value, $conditional_regex, $description )" ) if DEBUG; $conditional_regex ||= '0'; @@ -269,7 +273,9 @@ sub AddModificationTemplateAction { field_value, to_field, to_subfield, - to_regex, + to_regex_search, + to_regex_replace, + to_regex_modifiers, conditional, conditional_field, conditional_subfield, @@ -278,7 +284,7 @@ sub AddModificationTemplateAction { conditional_regex, description ) - VALUES ( NULL, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? )"; + VALUES ( NULL, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? )"; $sth = $dbh->prepare( $query ); @@ -292,7 +298,9 @@ sub AddModificationTemplateAction { $field_value, $to_field, $to_subfield, - $to_regex, + $to_regex_search, + $to_regex_replace, + $to_regex_modifiers, $conditional, $conditional_field, $conditional_subfield, @@ -309,7 +317,7 @@ sub AddModificationTemplateAction { ModModificationTemplateAction( $mmta_id, $action, $field_number, $from_field, $from_subfield, $field_value, $to_field, - $to_subfield, $to_regex, $conditional, + $to_subfield, $to_regex_search, $to_regex_replace, $to_regex_modifiers, $conditional, $conditional_field, $conditional_subfield, $conditional_comparison, $conditional_value, $conditional_regex, $description @@ -329,7 +337,9 @@ sub ModModificationTemplateAction { $field_value, $to_field, $to_subfield, - $to_regex, + $to_regex_search, + $to_regex_replace, + $to_regex_modifiers, $conditional, $conditional_field, $conditional_subfield, @@ -350,7 +360,9 @@ sub ModModificationTemplateAction { field_value = ?, to_field = ?, to_subfield = ?, - to_regex = ?, + to_regex_search = ?, + to_regex_replace = ?, + to_regex_modifiers = ?, conditional = ?, conditional_field = ?, conditional_subfield = ?, @@ -370,7 +382,9 @@ sub ModModificationTemplateAction { $field_value, $to_field, $to_subfield, - $to_regex, + $to_regex_search, + $to_regex_replace, + $to_regex_modifiers, $conditional, $conditional_field, $conditional_subfield, @@ -511,7 +525,9 @@ sub ModifyRecordWithTemplate { 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 $to_regex_search = $a->{'to_regex_search'}; + my $to_regex_replace = $a->{'to_regex_replace'}; + my $to_regex_modifiers = $a->{'to_regex_modifiers'}; my $conditional = $a->{'conditional'}; my $conditional_field = $a->{'conditional_field'}; my $conditional_subfield = $a->{'conditional_subfield'}; @@ -540,7 +556,7 @@ sub ModifyRecordWithTemplate { } push @params, ( ( ( not $field_value and $to_field ) - ? ( $to_field, $to_subfield, $to_regex ) + ? ( $to_field, $to_subfield, { search => $to_regex_search, replace => $to_regex_replace, modifiers => $to_regex_modifiers} ) : () ), ( $field_number ? $field_number diff --git a/Koha/SimpleMARC.pm b/Koha/SimpleMARC.pm index 94f6ab1..bd4c35e 100644 --- a/Koha/SimpleMARC.pm +++ b/Koha/SimpleMARC.pm @@ -2,8 +2,7 @@ package Koha::SimpleMARC; # Copyright 2009 Kyle M. Hall -use strict; -use warnings; +use Modern::Perl; #use MARC::Record; @@ -66,7 +65,7 @@ at your option, any later version of Perl 5 you may have available. Copies a value from one field to another. If a regular expression ( $regex ) is supplied, the value will be transformed by the given regex before being copied into the new field. - Example: $regex = 's/Old Text/Replacement Text/' + Example: $regex = { search => 'Old Text', replace => 'Replacement Text', modifiers => 'g' }; If $n is passed, copy_field will only copy the Nth field of the list of fields. E.g. $n = 1 will only use the first field's value, $n = 2 will use only the 2nd field's value. @@ -83,15 +82,33 @@ sub copy_field { @values = ( $values[$n-1] ) if ( $n ); C4::Koha::Log( "@values = read_field( $record, $fromFieldName, $fromSubfieldName )" ) if $debug >= 3; - if ( $regex ) { + if ( $regex and $regex->{search} ) { + $regex->{modifiers} //= q||; + my @available_modifiers = qw( i g ); + my $modifiers = q||; + for my $modifier ( split //, $regex->{modifiers} ) { + $modifiers .= $modifier + if grep {/$modifier/} @available_modifiers; + } foreach my $value ( @values ) { - C4::Koha::Log( "\$value =~ s$regex" ) if ( $debug >= 3 ); - eval "\$value =~ s$regex"; + C4::Koha::Log( "\$value =~ s/$regex->{search}/$regex->{replace}/$modifiers" ) if ( $debug >= 3 ); + for ( $modifiers ) { + when ( /^(ig|gi)$/ ) { + $value =~ s/$regex->{search}/$regex->{replace}/ig; + } + when ( /^i$/ ) { + $value =~ s/$regex->{search}/$regex->{replace}/i; + } + when ( /^g$/ ) { + $value =~ s/$regex->{search}/$regex->{replace}/g; + } + default { + $value =~ s/$regex->{search}/$regex->{replace}/; + } + } } } - update_field( $record, $toFieldName, $toSubfieldName, $dont_erase, @values ); - } =head2 update_field @@ -220,7 +237,7 @@ sub field_exists { Returns true if the field equals the given value, false otherwise. If a regular expression ( $regex ) is supplied, the value will be compared using - the given regex. Example: $regex = 'm/sought_text/' + the given regex. Example: $regex = 'sought_text' If $n is passed, the Nth field of a repeatable series will be used for comparison. Set $n to 1 or leave empty for a non-repeatable field. @@ -238,7 +255,7 @@ sub field_equals { my $field_value = $field_values[$n-1]; if ( $regex ) { - C4::Koha::Log( "Testing '$field_value' =~ m$value" ) if $debug >= 3; + C4::Koha::Log( "Testing '$field_value' =~ m/$value/" ) if $debug >= 3; return $field_value =~ m/$value/; } else { return $field_value eq $value; diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index e41850d..9667dae 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -3256,7 +3256,9 @@ CREATE TABLE IF NOT EXISTS marc_modification_template_actions ( field_value varchar(100) DEFAULT NULL, to_field varchar(3) DEFAULT NULL, to_subfield varchar(1) DEFAULT NULL, - to_regex text, + to_regex_search text, + to_regex_replace text, + to_regex_modifiers varchar(8) DEFAULT '', conditional enum('if','unless') DEFAULT NULL, conditional_field varchar(3) DEFAULT NULL, conditional_subfield varchar(1) DEFAULT NULL, @@ -3264,7 +3266,8 @@ CREATE TABLE IF NOT EXISTS marc_modification_template_actions ( conditional_value text, conditional_regex tinyint(1) NOT NULL DEFAULT '0', description text, - PRIMARY KEY (mmta_id) + PRIMARY KEY (mmta_id), + CONSTRAINT `mmta_ibfk_1` FOREIGN KEY (`template_id`) REFERENCES `marc_modification_templates` (`template_id`) ON DELETE CASCADE ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8; /*!40103 SET TIME_ZONE=@OLD_TIME_ZONE */; diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index b7551d0..b433308 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -7176,7 +7176,9 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) { field_value varchar(100) default NULL, to_field varchar(3) default NULL, to_subfield varchar(1) default NULL, - to_regex text, + to_regex_search text, + to_regex_replace text, + to_regex_modifiers varchar(8) default '', conditional enum('if','unless') default NULL, conditional_field varchar(3) default NULL, conditional_subfield varchar(1) default NULL, @@ -7184,7 +7186,8 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) { conditional_value text, conditional_regex tinyint(1) NOT NULL default '0', description text, - PRIMARY KEY (mmta_id) + PRIMARY KEY (mmta_id), + CONSTRAINT `mmta_ibfk_1` FOREIGN KEY (`template_id`) REFERENCES `marc_modification_templates` (`template_id`) ON DELETE CASCADE ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8; "); diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/marc_modification_templates.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/marc_modification_templates.tt index f3538ca..cf49e6a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/marc_modification_templates.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/marc_modification_templates.tt @@ -149,7 +149,7 @@ var modaction_legend_innerhtml; var action_submit_value; function editAction( mmta_id, ordering, action, field_number, from_field, from_subfield, field_value, to_field, - to_subfield, to_regex, conditional, conditional_field, conditional_subfield, + to_subfield, to_regex_search, to_regex_replace, to_regex_modifiers, conditional, conditional_field, conditional_subfield, conditional_comparison, conditional_value, conditional_regex, description ) { document.getElementById('mmta_id').value = mmta_id; @@ -164,9 +164,11 @@ function editAction( mmta_id, ordering, action, field_number, from_field, from_s document.getElementById('field_value').value = field_value; document.getElementById('to_field').value = to_field; document.getElementById('to_subfield').value = to_subfield; - document.getElementById('to_regex').value = to_regex; + $("#to_regex_search").val(to_regex_search); + $("#to_regex_replace").val(to_regex_replace); + $("#to_regex_modifiers").val(to_regex_modifiers); - document.getElementById('to_field_regex').checked = to_regex.length; + document.getElementById('to_field_regex').checked = conditional_regex.length; document.getElementById('to_field_regex').onchange(); setSelectByValue( 'conditional', conditional ); @@ -204,7 +206,9 @@ function cancelEditAction() { document.getElementById('field_value').value = ''; document.getElementById('to_field').value = ''; document.getElementById('to_subfield').value = ''; - document.getElementById('to_regex').value = ''; + $("#to_regex_search").val(""); + $("#to_regex_replace").val(""); + $("#to_regex_modifiers").val(""); document.getElementById('to_field_regex').checked = false; document.getElementById('to_field_regex').onchange(); @@ -306,7 +310,7 @@ function setSelectByValue( selectId, value ) { - Go down + Go down @@ -334,8 +338,8 @@ function setSelectByValue( selectId, value ) { [% IF ( ActionsLoo.to_field ) %] to [% ActionsLoo.to_field %][% IF ( ActionsLoo.to_subfield ) %]$[% ActionsLoo.to_subfield %][% END %] - [% IF ( ActionsLoo.to_regex ) %] - using RegEx s[% ActionsLoo.to_regex %] + [% IF ( ActionsLoo.to_regex_search ) %] + using RegEx s/[% ActionsLoo.to_regex_search %]/[% ActionsLoo.to_regex_replace %]/[% ActionsLoo.to_regex_modifiers %] [% END %] [% END %] @@ -364,7 +368,9 @@ function setSelectByValue( selectId, value ) { "[% ActionsLoo.field_value |replace('\\\\', '\\\\') |replace("'", "\'") |replace('"', '\"') |replace('\n', '\\n') |replace('\r', '\\r') %]", "[% ActionsLoo.to_field |replace('\\\\', '\\\\') |replace("'", "\'") |replace('"', '\"') |replace('\n', '\\n') |replace('\r', '\\r') %]", "[% ActionsLoo.to_subfield |replace('\\\\', '\\\\') |replace("'", "\'") |replace('"', '\"') |replace('\n', '\\n') |replace('\r', '\\r') %]", - "[% ActionsLoo.to_regex |replace('\\\\', '\\\\') |replace('\\\\', '\\\\') |replace("'", "\'") |replace('"', '\"') |replace('\n', '\\n') |replace('\r', '\\r') %]", + "[% ActionsLoo.to_regex_search |replace('\\\\', '\\\\') |replace('\\\\', '\\\\') |replace("'", "\'") |replace('"', '\"') |replace('\n', '\\n') |replace('\r', '\\r') %]", + "[% ActionsLoo.to_regex_replace |replace('\\\\', '\\\\') |replace('\\\\', '\\\\') |replace("'", "\'") |replace('"', '\"') |replace('\n', '\\n') |replace('\r', '\\r') %]", + "[% ActionsLoo.to_regex_modifiers |replace('\\\\', '\\\\') |replace('\\\\', '\\\\') |replace("'", "\'") |replace('"', '\"') |replace('\n', '\\n') |replace('\r', '\\r') %]", "[% ActionsLoo.conditional |replace('\\\\', '\\\\') |replace("'", "\'") |replace('"', '\"') |replace('\n', '\\n') |replace('\r', '\\r') %]", "[% ActionsLoo.conditional_field |replace('\\\\', '\\\\') |replace("'", "\'") |replace('"', '\"') |replace('\n', '\\n') |replace('\r', '\\r') %]", "[% ActionsLoo.conditional_subfield |replace('\\\\', '\\\\') |replace("'", "\'") |replace('"', '\"') |replace('\n', '\\n') |replace('\r', '\\r') %]", @@ -415,7 +421,7 @@ function setSelectByValue( selectId, value ) { diff --git a/t/SimpleMARC.t b/t/SimpleMARC.t index def2edb..2969bd2 100644 --- a/t/SimpleMARC.t +++ b/t/SimpleMARC.t @@ -1,6 +1,6 @@ use Modern::Perl; -use Test::More tests => 33; +use Test::More tests => 37; use_ok("MARC::Field"); use_ok("MARC::Record"); @@ -83,17 +83,17 @@ copy_field( $record, '650', 'a', '651', 'a', undef, 2 ); is_deeply( read_field( $record, '651', 'a' ), 'Computer algorithms.', 'Copy second field 650$a' ); delete_field( $record, '651' ); -copy_field( $record, '650', 'a', '651', 'a', '/Computer/The art of/' ); +copy_field( $record, '650', 'a', '651', 'a', { search => 'Computer', replace => 'The art of' } ); @fields_651a = read_field( $record, '651', 'a' ); is_deeply( \@fields_651a, ['The art of programming.', 'The art of algorithms.'], 'Copy field using regex' ); -copy_field( $record, '650', 'a', '651', 'a', '/Computer/The mistake of/' ); +copy_field( $record, '650', 'a', '651', 'a', { search => 'Computer', replace => 'The mistake of' } ); @fields_651a = read_field( $record, '651', 'a' ); is_deeply( \@fields_651a, ['The mistake of programming.', 'The mistake of algorithms.'], 'Copy fields using regex on existing fields' ); delete_field( $record, '651' ); -copy_field( $record, '650', 'a', '651', 'a', '/Computer/The art of/' ); -copy_field( $record, '650', 'a', '651', 'a', '/Computer/The mistake of/', 1, "dont_erase" ); +copy_field( $record, '650', 'a', '651', 'a', { search => 'Computer', replace => 'The art of' } ); +copy_field( $record, '650', 'a', '651', 'a', { search => 'Computer', replace => 'The mistake of' }, 1, "dont_erase" ); @fields_651a = read_field( $record, '651', 'a' ); is_deeply( \@fields_651a, [ 'The art of programming.', @@ -103,8 +103,8 @@ is_deeply( \@fields_651a, [ ], 'Copy first field using regex on existing fields without erase existing values' ); delete_field( $record, '651' ); -copy_field( $record, '650', 'a', '651', 'a', '/Computer/The art of/' ); -copy_field( $record, '650', 'a', '651', 'a', '/Computer/The mistake of/', undef , "dont_erase" ); +copy_field( $record, '650', 'a', '651', 'a', { search => 'Computer', replace => 'The art of' } ); +copy_field( $record, '650', 'a', '651', 'a', { search => 'Computer', replace => 'The mistake of' }, undef , "dont_erase" ); @fields_651a = read_field( $record, '651', 'a' ); is_deeply( \@fields_651a, [ 'The art of programming.', @@ -116,6 +116,22 @@ is_deeply( \@fields_651a, [ ], 'Copy fields using regex on existing fields without erase existing values' ); delete_field( $record, '651' ); +# Copy with regex modifiers +copy_field( $record, '650', 'a', '652', 'a', { search => 'o', replace => 'foo' } ); +my @fields_652a = read_field( $record, '652', 'a' ); +is_deeply( \@fields_652a, ['Cfoomputer programming.', 'Cfoomputer algorithms.'], 'Copy field using regex' ); + +copy_field( $record, '650', 'a', '653', 'a', { search => 'o', replace => 'foo', modifiers => 'g' } ); +my @fields_653a = read_field( $record, '653', 'a' ); +is_deeply( \@fields_653a, ['Cfoomputer prfoogramming.', 'Cfoomputer algfoorithms.'], 'Copy field using regex' ); + +copy_field( $record, '650', 'a', '654', 'a', { search => 'O', replace => 'foo', modifiers => 'i' } ); +my @fields_654a = read_field( $record, '654', 'a' ); +is_deeply( \@fields_654a, ['Cfoomputer programming.', 'Cfoomputer algorithms.'], 'Copy field using regex' ); + +copy_field( $record, '650', 'a', '655', 'a', { search => 'O', replace => 'foo', modifiers => 'gi' } ); +my @fields_655a = read_field( $record, '655', 'a' ); +is_deeply( \@fields_655a, ['Cfoomputer prfoogramming.', 'Cfoomputer algfoorithms.'], 'Copy field using regex' ); # update_field update_field( $record, '952', 'p', undef, '3010023918' ); diff --git a/tools/marc_modification_templates.pl b/tools/marc_modification_templates.pl index f02aa74..e815f60 100755 --- a/tools/marc_modification_templates.pl +++ b/tools/marc_modification_templates.pl @@ -59,7 +59,9 @@ if ( $op eq "create_template" ) { my $field_value = $cgi->param('field_value'); my $to_field = $cgi->param('to_field'); my $to_subfield = $cgi->param('to_subfield'); - my $to_regex = $cgi->param('to_regex'); + my $to_regex_search = $cgi->param('to_regex_search'); + my $to_regex_replace = $cgi->param('to_regex_replace'); + my $to_regex_modifiers = $cgi->param('to_regex_modifiers'); my $conditional = $cgi->param('conditional'); my $conditional_field = $cgi->param('conditional_field'); my $conditional_subfield = $cgi->param('conditional_subfield'); @@ -78,7 +80,9 @@ if ( $op eq "create_template" ) { $field_value, $to_field, $to_subfield, - $to_regex, + $to_regex_search, + $to_regex_replace, + $to_regex_modifiers, $conditional, $conditional_field, $conditional_subfield, @@ -97,7 +101,9 @@ if ( $op eq "create_template" ) { $field_value, $to_field, $to_subfield, - $to_regex, + $to_regex_search, + $to_regex_replace, + $to_regex_modifiers, $conditional, $conditional_field, $conditional_subfield, -- 1.7.10.4