From a11023d339458a7daf89e05334a16dc34789a756 Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Date: Wed, 15 Apr 2020 11:52:26 +0000
Subject: [PATCH] Bug 24715: Improve code of _check_split
Content-Type: text/plain; charset=utf-8

This routine may check repeatability in a specific framework, and
return the framework hash. The handling of $mss2 to store that
hash is improved here.

Adding a test in TransformKohaToMarc.t to see if we hit the cache.

Test plan:
Run t/db_dependent/Biblio/TransformKohaToMarc.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
---
 C4/Biblio.pm                                | 21 +++++++++++++--------
 t/db_dependent/Biblio/TransformKohaToMarc.t |  4 +++-
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/C4/Biblio.pm b/C4/Biblio.pm
index 051e44528c..01f6d20e50 100644
--- a/C4/Biblio.pm
+++ b/C4/Biblio.pm
@@ -1993,6 +1993,7 @@ sub TransformKohaToMarc {
     my $mss = GetMarcSubfieldStructure( '', { unsafe => 1 } ); # do not change framework
     my $mss2; # belongs to $params->{framework} only filled when needed
     my $tag_hr = {};
+    my $need_split;
     while ( my ($kohafield, $value) = each %$hash ) {
         foreach my $fld ( @{ $mss->{$kohafield} } ) {
             my $tagfield    = $fld->{tagfield};
@@ -2000,7 +2001,8 @@ sub TransformKohaToMarc {
             next if !$tagfield;
 
             # BZ 21800: split value if field is repeatable.
-            my @values = _check_split($params, $mss2, $fld, $value)
+            ( $need_split, $mss2 ) = _check_split($params, $mss2, $fld, $value);
+            my @values = $need_split
                 ? split(/\s?\|\s?/, $value, -1)
                 : ( $value );
             foreach my $value ( @values ) {
@@ -2024,22 +2026,25 @@ sub TransformKohaToMarc {
 }
 
 sub _check_split {
+# Checks if $value must be split; may consult passed framework
+# Returns 0|1, and framework hash if consulted
     my ($params, $mss2, $fld, $value) = @_;
-    return if index($value,'|') == -1; # nothing to worry about
-    return if $params->{no_split};
+    return (0, $mss2) if index($value,'|') == -1; # nothing to worry about
+    return (0, $mss2) if $params->{no_split};
 
     # if we did not get a specific framework, check default in $mss
-    return $fld->{repeatable} if !$params->{framework};
+    return ( $fld->{repeatable}, $mss2) if !$params->{framework};
 
     # here we need to check the specific framework
-    $mss2 = GetMarcSubfieldStructure($params->{framework}, { unsafe => 1 })
-        if !$mss2;
+    if(!$mss2) {
+        $mss2 = GetMarcSubfieldStructure($params->{framework}, { unsafe => 1 });
+    }
     foreach my $fld2 ( @{ $mss2->{ $fld->{kohafield} } } ) {
         next if $fld2->{tagfield} ne $fld->{tagfield};
         next if $fld2->{tagsubfield} ne $fld->{tagsubfield};
-        return 1 if $fld2->{repeatable};
+        return (1, $mss2) if $fld2->{repeatable};
     }
-    return;
+    return (0, $mss2);
 }
 
 =head2 PrepHostMarcField
diff --git a/t/db_dependent/Biblio/TransformKohaToMarc.t b/t/db_dependent/Biblio/TransformKohaToMarc.t
index f22297959b..377dbab6a5 100644
--- a/t/db_dependent/Biblio/TransformKohaToMarc.t
+++ b/t/db_dependent/Biblio/TransformKohaToMarc.t
@@ -79,7 +79,7 @@ subtest "Working with control fields" => sub {
 };
 
 subtest "Add tests for _check_split" => sub {
-    plan tests => 7;
+    plan tests => 8;
 
     Koha::MarcSubfieldStructures->search({ frameworkcode => '', tagfield => '952', tagsubfield => 'a' })->delete;
     Koha::MarcSubfieldStructure->new({ frameworkcode => '', tagfield => '952', tagsubfield => 'a', kohafield => 'items.fld1' })->store;
@@ -105,9 +105,11 @@ subtest "Add tests for _check_split" => sub {
     $record = C4::Biblio::TransformKohaToMarc( { @cols } );
     is( $record->subfield( '952', 'a' ), '01 | 02', 'Check composite in 952a' );
     # Test with other framework (repeatable)
+    Koha::Caches->get_instance->clear_from_cache( "MarcSubfieldStructure-". $fw );
     $record =  C4::Biblio::TransformKohaToMarc( { @cols }, { framework => $fw } );
     is( ($record->subfield( '952', 'a' ))[0], '01', "Framework $fw first 952a" );
     is( ($record->subfield( '952', 'a' ))[1], '02', "Framework $fw second 952a" );
+    is( ref(Koha::Caches->get_instance->get_from_cache( "MarcSubfieldStructure-". $fw )), 'HASH', 'We did hit the cache' );
 };
 
 # Cleanup
-- 
2.11.0