From 6a6a6e40c32cd60c7fc8844c2d138285be20dd94 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 3 May 2016 11:08:47 +0100 Subject: [PATCH] Bug 13074: Use Koha::Cache to cache the defaults values of a MARC record MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit With the global %default_values_for_mod_from_marc variable, the changes made to the marc_subfield_structure table and especially the links between MARC and DB fields are not safe and might be outdated (if a field is linked/unlinked) Test plan: Under Plack: - Link the barcode field, edit a record and set a barcode. - Remove the mapping for the barcode field and then update again the barcode of the record. The items.barcode DB field must not have been updated. Without this patch, the field should have been updated. Signed-off-by: Tomas Cohen Arazi Signed-off-by: Nick Clemens Signed-off-by: Kyle M Hall (cherry picked from commit 8c14c25521eac92a2cb62ab5330deac2e2d5343d) Signed-off-by: Frédéric Demians (cherry picked from commit a50e5f141b14c1ba6f9c491a40db1270a8c7be99) Signed-off-by: Julian Maurice --- C4/Items.pm | 17 +++-- admin/biblio_framework.pl | 2 + admin/marc_subfields_structure.pl | 2 + admin/marctagstructure.pl | 3 + t/db_dependent/Items.t | 116 +++++++++++++++++++++++++++++- 5 files changed, 132 insertions(+), 8 deletions(-) diff --git a/C4/Items.pm b/C4/Items.pm index f7165e59bb..9f46046063 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -447,12 +447,14 @@ Returns item record =cut -our %default_values_for_mod_from_marc; - sub _build_default_values_for_mod_marc { my ($frameworkcode) = @_; - return $default_values_for_mod_from_marc{$frameworkcode} - if exists $default_values_for_mod_from_marc{$frameworkcode}; + + my $cache = Koha::Cache->get_instance(); + my $cache_key = "default_value_for_mod_marc-$frameworkcode"; + my $cached = $cache->get_from_cache($cache_key); + return $cached if $cached; + my $default_values = { barcode => undef, booksellerid => undef, @@ -483,15 +485,18 @@ sub _build_default_values_for_mod_marc { uri => undef, withdrawn => 0, }; + my %default_values_for_mod_from_marc; while ( my ( $field, $default_value ) = each %$default_values ) { my $kohafield = $field; $kohafield =~ s|^([^\.]+)$|items.$1|; - $default_values_for_mod_from_marc{$frameworkcode}{$field} = + $default_values_for_mod_from_marc{$field} = $default_value if C4::Koha::IsKohaFieldLinked( { kohafield => $kohafield, frameworkcode => $frameworkcode } ); } - return $default_values_for_mod_from_marc{$frameworkcode}; + + $cache->set_in_cache($cache_key, \%default_values_for_mod_from_marc); + return \%default_values_for_mod_from_marc; } sub ModItemFromMarc { diff --git a/admin/biblio_framework.pl b/admin/biblio_framework.pl index a6fc24c72a..2a81f88ef8 100755 --- a/admin/biblio_framework.pl +++ b/admin/biblio_framework.pl @@ -87,6 +87,7 @@ if ($op eq 'add_form') { } $cache->clear_from_cache("MarcStructure-0-$frameworkcode"); $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); + $cache->clear_from_cache("default_value_for_mod_marc-$frameworkcode"); } print $input->redirect($script_name); # FIXME: unnecessary redirect exit; @@ -121,6 +122,7 @@ if ($op eq 'add_form') { $sth->execute($frameworkcode); $cache->clear_from_cache("MarcStructure-0-$frameworkcode"); $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); + $cache->clear_from_cache("default_value_for_mod_marc-$frameworkcode"); } print $input->redirect($script_name); # FIXME: unnecessary redirect exit; diff --git a/admin/marc_subfields_structure.pl b/admin/marc_subfields_structure.pl index 5e525232fc..b6fe25e8cd 100755 --- a/admin/marc_subfields_structure.pl +++ b/admin/marc_subfields_structure.pl @@ -345,6 +345,7 @@ elsif ( $op eq 'add_validate' ) { $sth_update->finish; $cache->clear_from_cache("MarcStructure-0-$frameworkcode"); $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); + $cache->clear_from_cache("default_value_for_mod_marc-$frameworkcode"); print $input->redirect("/cgi-bin/koha/admin/marc_subfields_structure.pl?tagfield=$tagfield&frameworkcode=$frameworkcode"); exit; @@ -388,6 +389,7 @@ elsif ( $op eq 'delete_confirmed' ) { } $cache->clear_from_cache("MarcStructure-0-$frameworkcode"); $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); + $cache->clear_from_cache("default_value_for_mod_marc-$frameworkcode"); print $input->redirect("/cgi-bin/koha/admin/marc_subfields_structure.pl?tagfield=$tagfield&frameworkcode=$frameworkcode"); exit; diff --git a/admin/marctagstructure.pl b/admin/marctagstructure.pl index 77286871fd..7298d6aca9 100755 --- a/admin/marctagstructure.pl +++ b/admin/marctagstructure.pl @@ -164,6 +164,7 @@ if ($op eq 'add_form') { } $cache->clear_from_cache("MarcStructure-0-$frameworkcode"); $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); + $cache->clear_from_cache("default_value_for_mod_marc-$frameworkcode"); } print $input->redirect("/cgi-bin/koha/admin/marctagstructure.pl?searchfield=$tagfield&frameworkcode=$frameworkcode"); exit; @@ -190,6 +191,7 @@ if ($op eq 'add_form') { $sth2->execute($searchfield, $frameworkcode); $cache->clear_from_cache("MarcStructure-0-$frameworkcode"); $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); + $cache->clear_from_cache("default_value_for_mod_marc-$frameworkcode"); } $template->param( searchfield => $searchfield, @@ -355,5 +357,6 @@ sub duplicate_framework { } $cache->clear_from_cache("MarcStructure-0-$newframeworkcode"); $cache->clear_from_cache("MarcStructure-1-$newframeworkcode"); + $cache->clear_from_cache("default_value_for_mod_marc-$frameworkcode"); } diff --git a/t/db_dependent/Items.t b/t/db_dependent/Items.t index 9dcd5f79dc..6bf63573af 100755 --- a/t/db_dependent/Items.t +++ b/t/db_dependent/Items.t @@ -26,7 +26,7 @@ use Koha::Database; use t::lib::Mocks; use t::lib::TestBuilder; -use Test::More tests => 9; +use Test::More tests => 10; use Test::Warn; @@ -553,13 +553,125 @@ subtest 'C4::Biblio::EmbedItemsInMarcBiblio' => sub { $schema->storage->txn_rollback; }; + +subtest 'C4::Items::_build_default_values_for_mod_marc' => sub { + plan tests => 4; + + $schema->storage->txn_begin(); + + # Clear cache + $C4::Context::context->{marcfromkohafield} = undef; + $C4::Biblio::inverted_field_map = undef; + + my $builder = t::lib::TestBuilder->new; + my $framework = $builder->build({ + source => 'BiblioFramework', + }); + # Link biblio.biblionumber and biblioitems.biblioitemnumber to avoid _koha_marc_update_bib_ids to fail with 'no biblio[item]number tag for framework" + $builder->build({ + source => 'MarcSubfieldStructure', + value => { + frameworkcode => $framework->{frameworkcode}, + kohafield => 'biblio.biblionumber', + tagfield => '999', + tagsubfield => 'c', + } + }); + $builder->build({ + source => 'MarcSubfieldStructure', + value => { + frameworkcode => $framework->{frameworkcode}, + kohafield => 'biblioitems.biblioitemnumber', + tagfield => '999', + tagsubfield => 'd', + } + }); + my $mss_itemnumber = $builder->build({ + source => 'MarcSubfieldStructure', + value => { + frameworkcode => $framework->{frameworkcode}, + kohafield => 'items.itemnumber', + tagfield => '952', + tagsubfield => '9', + } + }); + + my $mss_barcode = $builder->build({ + source => 'MarcSubfieldStructure', + value => { + frameworkcode => $framework->{frameworkcode}, + kohafield => 'items.barcode', + tagfield => '952', + tagsubfield => 'p', + } + }); + + # Create a record with a barcode + my ($biblionumber) = get_biblio( $framework->{frameworkcode} ); + my $item_record = new MARC::Record; + my $a_barcode = 'a barcode'; + my $barcode_field = MARC::Field->new( + '952', ' ', ' ', + p => $a_barcode, + ); + $item_record->append_fields( $barcode_field ); + my (undef, undef, $item_itemnumber) = AddItemFromMarc($item_record, $biblionumber); + + # Make sure everything has been set up + my $item = GetItem($item_itemnumber); + is( $item->{barcode}, $a_barcode, 'Everything has been set up correctly, the barcode is defined as expected' ); + + # Delete the barcode field and save the record + $item_record->delete_fields( $barcode_field ); + ModItemFromMarc($item_record, $biblionumber, $item_itemnumber); + $item = GetItem($item_itemnumber); + is( $item->{barcode}, undef, 'The default value should have been set to the barcode, the field is mapped to a kohafield' ); + + # Re-add the barcode field and save the record + $item_record->append_fields( $barcode_field ); + ModItemFromMarc($item_record, $biblionumber, $item_itemnumber); + $item = GetItem($item_itemnumber); + is( $item->{barcode}, $a_barcode, 'Everything has been set up correctly, the barcode is defined as expected' ); + + # Remove the mapping + my $dbh = C4::Context->dbh; + $dbh->do(q| + DELETE FROM marc_subfield_structure + WHERE kohafield = 'items.barcode' + AND frameworkcode = ? + |, undef, $framework->{frameworkcode} ); + + # And make sure the caches are cleared + $C4::Context::context->{marcfromkohafield} = undef; + $C4::Biblio::inverted_field_map = undef; + my $cache = Koha::Cache->get_instance(); + $cache->clear_from_cache("default_value_for_mod_marc-" . $framework->{frameworkcode} ); + + # Update the MARC field with another value + $item_record->delete_fields( $barcode_field ); + my $another_barcode = 'another_barcode'; + my $another_barcode_field = MARC::Field->new( + '952', ' ', ' ', + p => $another_barcode, + ); + $item_record->append_fields( $another_barcode_field ); + # The DB value should not have been updated + ModItemFromMarc($item_record, $biblionumber, $item_itemnumber); + $item = GetItem($item_itemnumber); + is ( $item->{barcode}, $a_barcode, 'items.barcode is not mapped anymore, so the DB column has not been updated' ); + + $schema->storage->txn_rollback; +}; + # Helper method to set up a Biblio. sub get_biblio { + my ( $frameworkcode ) = @_; + $frameworkcode //= ''; my $bib = MARC::Record->new(); $bib->append_fields( MARC::Field->new('100', ' ', ' ', a => 'Moffat, Steven'), MARC::Field->new('245', ' ', ' ', a => 'Silence in the library'), ); - my ($bibnum, $bibitemnum) = AddBiblio($bib, ''); + my ($bibnum, $bibitemnum) = AddBiblio($bib, $frameworkcode); return ($bibnum, $bibitemnum); } -- 2.39.5