@@ -, +, @@ - Apply the patch - Apply the atomicupdate - Prove t/db_dependant/OAI/AndSets.t - Check that existing mappings still work - Try modifying existing mappings - Try creating new mappings - Check that the boolean operators precedence is correctly taken into account --- C4/OAI/Sets.pm | 100 ++++++++---- admin/oai_set_mappings.pl | 13 +- installer/data/mysql/atomicupdate/bug_21520.perl | 9 + .../prog/en/modules/admin/oai_set_mappings.tt | 32 +++- t/db_dependent/OAI/AndSets.t | 181 +++++++++++++++++++++ 5 files changed, 292 insertions(+), 43 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_21520.perl create mode 100644 t/db_dependent/OAI/AndSets.t --- a/C4/OAI/Sets.pm +++ a/C4/OAI/Sets.pm @@ -290,19 +290,19 @@ sub AddOAISet { GetOAISetsMappings returns mappings for all OAI Sets. Mappings define how biblios are categorized in sets. -A mapping is defined by four properties: +A mapping is defined by six properties: { - marcfield => 'XXX', # the MARC field to check - marcsubfield => 'Y', # the MARC subfield to check - operator => 'A', # the operator 'equal' or 'notequal'; 'equal' if '' - marcvalue => 'zzzz', # the value to check + marcfield => 'XXX', # the MARC field to check + marcsubfield => 'Y', # the MARC subfield to check + operator => 'A', # the operator 'equal' or 'notequal'; 'equal' if '' + marcvalue => 'zzzz', # the value to check + rule_operator => 'and|or|undef', # the operator between the rules + rule_order => 'n' # the order of the rule for the mapping } If defined in a set mapping, a biblio which have at least one 'Y' subfield of one 'XXX' field equal to 'zzzz' will belong to this set. -If multiple mappings are defined in a set, the biblio will belong to this set -if at least one condition is matched. GetOAISetsMappings returns a hashref of arrayrefs of hashrefs. The first hashref keys are the sets IDs, so it looks like this: @@ -313,7 +313,9 @@ The first hashref keys are the sets IDs, so it looks like this: marcfield => 'XXX', marcsubfield => 'Y', operator => 'A', - marcvalue => 'zzzz' + marcvalue => 'zzzz', + rule_operator => 'and|or|undef', + rule_order => 'n' }, { ... @@ -329,7 +331,7 @@ The first hashref keys are the sets IDs, so it looks like this: sub GetOAISetsMappings { my $dbh = C4::Context->dbh; my $query = qq{ - SELECT * FROM oai_sets_mappings + SELECT * FROM oai_sets_mappings ORDER BY set_id, rule_order }; my $sth = $dbh->prepare($query); $sth->execute; @@ -340,7 +342,9 @@ sub GetOAISetsMappings { marcfield => $result->{'marcfield'}, marcsubfield => $result->{'marcsubfield'}, operator => $result->{'operator'}, - marcvalue => $result->{'marcvalue'} + marcvalue => $result->{'marcvalue'}, + rule_operator => $result->{'rule_operator'}, + rule_order => $result->{'rule_order'} }; } @@ -365,6 +369,7 @@ sub GetOAISetMappings { SELECT * FROM oai_sets_mappings WHERE set_id = ? + ORDER BY rule_order }; my $sth = $dbh->prepare($query); $sth->execute($set_id); @@ -375,7 +380,9 @@ sub GetOAISetMappings { marcfield => $result->{'marcfield'}, marcsubfield => $result->{'marcsubfield'}, operator => $result->{'operator'}, - marcvalue => $result->{'marcvalue'} + marcvalue => $result->{'marcvalue'}, + rule_operator => $result->{'rule_operator'}, + rule_order => $result->{'rule_order'} }; } @@ -411,15 +418,14 @@ sub ModOAISetMappings { }; my $sth = $dbh->prepare($query); $sth->execute($set_id); - if(scalar @$mappings > 0) { $query = qq{ - INSERT INTO oai_sets_mappings (set_id, marcfield, marcsubfield, operator, marcvalue) - VALUES (?,?,?,?,?) + INSERT INTO oai_sets_mappings (set_id, marcfield, marcsubfield, operator, marcvalue, rule_operator, rule_order) + VALUES (?,?,?,?,?,?,?) }; $sth = $dbh->prepare($query); foreach (@$mappings) { - $sth->execute($set_id, $_->{'marcfield'}, $_->{'marcsubfield'}, $_->{'operator'}, $_->{'marcvalue'}); + $sth->execute($set_id, $_->{'marcfield'}, $_->{'marcsubfield'}, $_->{'operator'}, $_->{'marcvalue'}, $_->{'rule_operator'}, $_->{'rule_order'}); } } } @@ -492,30 +498,64 @@ sub CalcOAISetsBiblio { my @biblio_sets; foreach my $set_id (keys %$oai_sets_mappings) { + + my $rules = []; foreach my $mapping (@{ $oai_sets_mappings->{$set_id} }) { next if not $mapping; - my $field = $mapping->{'marcfield'}; - my $subfield = $mapping->{'marcsubfield'}; - my $operator = $mapping->{'operator'}; - my $value = $mapping->{'marcvalue'}; - my @subfield_values = $record->subfield($field, $subfield); - if ($operator eq 'notequal') { - if(0 == grep /^$value$/, @subfield_values) { - push @biblio_sets, $set_id; - last; - } + my $rule_operator = $mapping->{'rule_operator'}; + my $result = _evalRule($record, $mapping); + + # First rule or 'or' rule is always pushed + if (!@$rules || $rule_operator eq 'or') { + push @$rules, [$result]; + next; } - else { - if(0 < grep /^$value$/, @subfield_values) { - push @biblio_sets, $set_id; - last; - } + + # 'and' rule is pushed in the last 'or' rule + push @{$rules->[-1]}, $result; + } + + my @evaluated_and; + foreach my $ruleset (@$rules) { + if (0 < grep /0/, @{$ruleset}) { + push @evaluated_and, 0; + } else { + push @evaluated_and, 1; } } + + if (grep /1/, @evaluated_and) { + push @biblio_sets, $set_id; + } + } return @biblio_sets; } +# Does the record match a given mapping rule? +sub _evalRule { + my $record = shift; + my $mapping = shift; + + my $field = $mapping->{'marcfield'}; + my $subfield = $mapping->{'marcsubfield'}; + my $operator = $mapping->{'operator'}; + my $value = $mapping->{'marcvalue'}; + my @subfield_values = $record->subfield($field, $subfield); + if ($operator eq 'notequal') { + if(0 == grep /^$value$/, @subfield_values) { + return 1; + } + } + else { + if(0 < grep /^$value$/, @subfield_values) { + return 1; + } + } + return 0; +} + + =head2 ModOAISetsBiblios my $oai_sets_biblios = { --- a/admin/oai_set_mappings.pl +++ a/admin/oai_set_mappings.pl @@ -57,20 +57,25 @@ if($op && $op eq "save") { my @marcsubfields = $input->multi_param('marcsubfield'); my @operators = $input->multi_param('operator'); my @marcvalues = $input->multi_param('marcvalue'); + my @ruleoperators = $input->multi_param('rule_operator'); + my @ruleorders = $input->multi_param('rule_order'); my @mappings; my $i = 0; while($i < @marcfields and $i < @marcsubfields and $i < @marcvalues) { if($marcfields[$i] and $marcsubfields[$i]) { push @mappings, { - marcfield => $marcfields[$i], - marcsubfield => $marcsubfields[$i], - operator => $operators[$i], - marcvalue => $marcvalues[$i] + marcfield => $marcfields[$i], + marcsubfield => $marcsubfields[$i], + operator => $operators[$i], + marcvalue => $marcvalues[$i], + rule_operator => $ruleoperators[$i], + rule_order => $i }; } $i++; } + $mappings[0]{'rule_operator'} = undef if (@mappings); ModOAISetMappings($id, \@mappings); $template->param(mappings_saved => 1); } --- a/installer/data/mysql/atomicupdate/bug_21520.perl +++ a/installer/data/mysql/atomicupdate/bug_21520.perl @@ -0,0 +1,9 @@ +$DBversion = 'XXX'; # will be replaced by the RM +if( CheckVersion( $DBversion ) ) { + $dbh->do( "ALTER TABLE oai_sets_mappings ADD COLUMN rule_order INT AFTER set_id, ADD COLUMN rule_operator VARCHAR(3) AFTER rule_order" ); + $dbh->do( "UPDATE oai_sets_mappings SET rule_operator='or'" ); + + # Always end with this (adjust the bug info) + SetVersion( $DBversion ); + print "Upgrade to $DBversion done (Bug 21520 - Add rule_order and rule_operator fields to oai_sets_mappings table)\n"; +} --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/oai_set_mappings.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/oai_set_mappings.tt @@ -31,6 +31,7 @@ + @@ -43,6 +44,12 @@ [% IF ( mappings ) %] [% FOREACH mapping IN mappings %] + @@ -67,6 +72,12 @@ [% END %] [% ELSE %] + - + [% END %] @@ -104,7 +115,7 @@ $("#mappingform").submit(function(){ hideDialogBox(); }); - $("body").on("click","#ORbutton", function(e){ + $("body").on("click","#new_rule_button", function(e){ e.preventDefault(); newCondition(); }); @@ -112,22 +123,25 @@ e.preventDefault(); clearRow(e.target); }); + $("#mappings tbody tr:first-child td:first-child select").hide(); }); function newCondition() { - var tr = $('#ORbutton').parents('tr'); + var tr = $('#new_rule_button').parents('tr'); var clone = $(tr).clone(); - $("#ORbutton").parent('td').replaceWith(''); + $("#new_rule_button").parent('td').find("#new_rule_button").remove(); + $(clone).find("select").show(); $(tr).parent('tbody').append(clone); } function clearRow(link){ var tr = $(link).parent().parent(); - var found = tr.find('#ORbutton'); + var found = tr.find('#new_rule_button'); if( found.length ){ tr.find('input[type="text"]').attr("value",""); } else { - tr.find('input[type="text"]').attr("value","").end().hide(); + tr.remove(); } + $("#mappings tbody tr:first-child td:first-child select").hide(); } function hideDialogBox() { $('div.dialog').remove(); --- a/t/db_dependent/OAI/AndSets.t +++ a/t/db_dependent/OAI/AndSets.t @@ -0,0 +1,181 @@ +#!/usr/bin/perl + +# Copyright 2019 BibLibre +# +# 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; + +use Test::More tests => 4; +use Test::MockModule; +use Test::Warn; +use MARC::Record; +use Data::Dumper; + +use Koha::Database; +use C4::Biblio; +use C4::OAI::Sets; + +use t::lib::TestBuilder; + +my $schema = Koha::Database->new->schema; +$schema->storage->txn_begin; +my $dbh = C4::Context->dbh; + +$dbh->do('DELETE FROM oai_sets'); +$dbh->do('DELETE FROM oai_sets_descriptions'); +$dbh->do('DELETE FROM oai_sets_mappings'); +$dbh->do('DELETE FROM oai_sets_biblios'); + +my $builder = t::lib::TestBuilder->new; + +my $set1 = { + 'spec' => 'specSet1', + 'name' => 'nameSet1', +}; +my $set1_id = AddOAISet($set1); + +my $marcflavour = C4::Context->preference('marcflavour'); +my $mapping1; + +if ($marcflavour eq 'UNIMARC' ){ + $mapping1 = [ + { + rule_order => 1, + marcfield => '200', + marcsubfield => 'a', + operator => 'equal', + marcvalue => 'myTitle' + }, + { + rule_order => 2, + rule_operator => 'and', + marcfield => '200', + marcsubfield => 'f', + operator => 'equal', + marcvalue => 'myAuthor' + }, + ]; +} else { + $mapping1 = [ + { + rule_order => 1, + marcfield => '245', + marcsubfield => 'a', + operator => 'equal', + marcvalue => 'myTitle' + }, + { + rule_order => 2, + rule_operator => 'and', + marcfield => '100', + marcsubfield => 'a', + operator => 'equal', + marcvalue => 'myAuthor' + }, + ]; +} + +#Add 1st mapping for set1 +ModOAISetMappings($set1_id, $mapping1); + +my $biblio_1 = $builder->build_sample_biblio({ title => 'myTitle' }); +my $biblio_2 = $builder->build_sample_biblio({ title => 'myTitle', author => 'myAuthor' }); + +my $biblionumber1 = $biblio_1->biblionumber; +my $biblionumber2 = $biblio_2->biblionumber; + + +my $record = GetMarcBiblio({ biblionumber => $biblionumber1 }); +my @setsEq = CalcOAISetsBiblio($record); +ok(!@setsEq, 'If only one condition is true, the record does not belong to the set'); + +$record = GetMarcBiblio({ biblionumber => $biblionumber2 }); +@setsEq = CalcOAISetsBiblio($record); +is_deeply(@setsEq, $set1_id, 'If all conditions are true, the record belongs to the set'); + +if ($marcflavour eq 'UNIMARC' ){ + $mapping1 = [ + { + rule_order => 1, + marcfield => '200', + marcsubfield => 'a', + operator => 'equal', + marcvalue => 'myTitle' + }, + { + rule_order => 2, + rule_operator => 'or', + marcfield => '200', + marcsubfield => 'f', + operator => 'equal', + marcvalue => 'myAuthor' + }, + { + rule_order => 3, + rule_operator => 'and', + marcfield => '995', + marcsubfield => 'r', + operator => 'equal', + marcvalue => 'myItemType' + }, + + ]; +} else { + $mapping1 = [ + { + rule_order => 1, + marcfield => '245', + marcsubfield => 'a', + operator => 'equal', + marcvalue => 'myTitle' + }, + { + rule_order => 2, + rule_operator => 'or', + marcfield => '100', + marcsubfield => 'a', + operator => 'equal', + marcvalue => 'myAuthor' + }, + { + rule_order => 3, + rule_operator => 'and', + marcfield => '942', + marcsubfield => 'c', + operator => 'equal', + marcvalue => 'myItemType' + }, + ]; +} + +ModOAISetMappings($set1_id, $mapping1); + +$biblio_1 = $builder->build_sample_biblio({ title => 'myTitle' }); +$biblio_2 = $builder->build_sample_biblio({ author => 'myAuthor', itemtype => 'myItemType' }); + +$biblionumber1 = $biblio_1->biblionumber; +$biblionumber2 = $biblio_2->biblionumber; + +$record = GetMarcBiblio({ biblionumber => $biblionumber1 }); +@setsEq = CalcOAISetsBiblio($record); + +is_deeply(@setsEq, $set1_id, 'Boolean operators precedence is respected, the record with only the title belongs to the set'); + +$record = GetMarcBiblio({ biblionumber => $biblionumber2 }); +@setsEq = CalcOAISetsBiblio($record); +is_deeply(@setsEq, $set1_id, 'Boolean operators precedence is respected, the record with author and itemtype belongs to the set'); + +$schema->storage->txn_rollback; --
Rule operator Field Subfield Operator
+ + [% IF ( loop.last ) %] - - [% ELSE %] - OR + [% END %]
+ +
OR