Attachment 112957 Details for Bug 17385 – Bug 17385: (QA follow-up) Do not needlessly scan biblioexport folder

[patch] Bug 17385: (QA follow-up) Do not needlessly scan biblioexport folder

Bug-17385-QA-follow-up-Do-not-needlessly-scan-bibl.patch (text/plain), 4.65 KB, created by Alexis Ripetti on 2020-11-03 21:57:31 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Alexis Ripetti

Created: 2020-11-03 21:57:31 UTC

Size: 4.65 KB

patch

obsolete

>From 6631b2aa5890ceae05651696f95b997a554e1b28 Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Date: Fri, 22 Mar 2019 08:03:16 +0000
>Subject: [PATCH] Bug 17385: (QA follow-up) Do not needlessly scan biblioexport
> folder
>
>Performance: We need to return an empty list if OpacExportOptions does not contain Custom.  No need to scan the directory for custom files, if we are not using them at all.
>
>Note: The test only pertains to OPAC now, since the pref should not control intranet behavior. We have no intranet counterpart. See further next follow-up.
>
>Test plan:
>Run t/db_dependent/XSLT.t
>---
> C4/XSLT.pm            | 48 +++++++++++++++++++++++++++++++++++++++++++
> t/db_dependent/XSLT.t | 44 +++++++++++++--------------------------
> 2 files changed, 62 insertions(+), 30 deletions(-)
>
>diff --git a/C4/XSLT.pm b/C4/XSLT.pm
>index 90f67bf307..689e855efc 100644
>--- a/C4/XSLT.pm
>+++ b/C4/XSLT.pm
>@@ -410,6 +410,54 @@ sub engine {
>     return $engine;
> }
> 
>+=head2 CustomXSLTExportList
>+
>+ Returns list of file for custom xslt conversion
>+
>+=cut
>+
>+sub CustomXSLTExportList {
>+    my $opac = shift; # opac (1) vs intranet (0)
>+    return [] if $opac && C4::Context->preference('OpacExportOptions') !~ /custom/;
>+
>+    my @tabFiles;
>+
>+    my $dir = C4::Context->config( $opac ? 'opachtdocs' : 'intrahtdocs') .
>+                      '/' . C4::Context->preference( $opac ? "opacthemes" : "template") .
>+                      '/' . C4::Languages::getlanguage() .
>+                      '/xslt/biblioexport';
>+    my @files = glob qq("$dir/*.xsl");
>+    foreach my $file (@files) {
>+        if ( -f "$file" ) {
>+            (my $text = $file) =~ s/.*\///g;
>+
>+            ## Get title of the option
>+            my $dom;
>+            eval { $dom = XML::LibXML->load_xml( location => $file ); };
>+            next unless $dom;
>+
>+            my $node = $dom->documentElement();
>+            my $title = $node->{"title"};
>+            ($title = $text) =~ s/\.xsl// unless defined $title;
>+
>+            # Get output format
>+            # There should only be one xsl:output node, so taking the first one only is OK
>+            $node = @{$node->findnodes("xsl:output")}[0];
>+            my $outputformat= "";
>+            $outputformat = $node->{"method"} if $node;
>+            $outputformat = "txt" if ($outputformat eq "" || $outputformat eq "text");
>+
>+            my %row = (
>+                value       => $text,
>+                filename    => $title,
>+                format      => $outputformat,
>+            );
>+
>+            push @tabFiles, \%row;
>+        }
>+    }
>+    return \@tabFiles;
>+}
> 1;
> 
> __END__
>diff --git a/t/db_dependent/XSLT.t b/t/db_dependent/XSLT.t
>index f8a24e39d8..519a3a7a9a 100755
>--- a/t/db_dependent/XSLT.t
>+++ b/t/db_dependent/XSLT.t
>@@ -1,37 +1,23 @@
>-#!/usr/bin/perl
>-
>-# 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 <http://www.gnu.org/licenses>.
>-
> use Modern::Perl;
>-
> use Test::More tests => 2;
>-use Test::Warn;
>-use t::lib::TestBuilder;
>-use t::lib::Mocks;
>-
>-use Koha::ItemTypes;
> 
>-BEGIN {
>-    use_ok('C4::XSLT');
>-}
>+use t::lib::Mocks;
>+use C4::XSLT;
>+use Koha::Database;
> 
>-my $schema  = Koha::Database->new->schema;
>-my $builder = t::lib::TestBuilder->new;
>+our $schema = Koha::Database->new->schema;
> 
>+# Here we go
> $schema->storage->txn_begin;
>+subtest 'CustomXSLTExportList: Check export options' => sub {
>+    plan tests => 2;
>+    t::lib::Mocks::mock_preference('OpacExportOptions', 'custom');
>+        my $list = C4::XSLT::CustomXSLTExportList(1);
>+    is( @$list>0, 1, 'We expect at least one result: simple export' );
>+    t::lib::Mocks::mock_preference('OpacExportOptions', 'dc');
>+    $list = C4::XSLT::CustomXSLTExportList(1);
>+    is( @$list, 0, 'We expect an empty list now' );
>+};
> 
> subtest 'buildKohaItemsNamespace status tests' => sub {
>     plan tests => 13;
>@@ -111,8 +97,6 @@ subtest 'buildKohaItemsNamespace status tests' => sub {
>     });
>     $xml = C4::XSLT::buildKohaItemsNamespace( $item->biblionumber,[]);
>     like($xml,qr{<status>Pending hold</status>},"Pending status takes precedence over all");
>-
>-
> };
> subtest  'CustomXSLTExportList: Caching' => sub {
>     plan tests => 1;
>-- 
>2.25.1

Actions: View | Diff | Splinter Review

Attachments on bug 17385: 55961 | 58308 | 64033 | 64034 | 67357 | 67579 | 74660 | 74661 | 79055 | 79056 | 80675 | 81385 | 81386 | 84388 | 84389 | 84390 | 86875 | 86876 | 86877 | 86878 | 86879 | 86880 | 97076 | 97101 | 97102 | 97103 | 97104 | 97105 | 100289 | 100290 | 100291 | 100292 | 100293 | 100663 | 100670 | 100671 | 100672 | 100673 | 100674 | 100675 | 112671 | 112672 | 112673 | 112674 | 112675 | 112676 | 112677 | 112678 | 112679 | 112680 | 112681 | 112682 | 112956 | 112957 | 112958 | 112959 | 130283 | 130284 | 130285 | 130286 | 130287 | 130691 | 130692 | 130693 | 130694 | 130695 | 131492 | 132488 | 132489 | 132490 | 132491 | 132492 | 132493 | 132581 | 133498 | 133499 | 133500 | 133501 | 133502 | 133503 | 133504 | 138031 | 138032 | 138033 | 138034 | 138035 | 138036 | 138037 | 138082 | 138083 | 138084 | 138085 | 138086 | 138087 | 138088 | 138132 | 139277 | 139278 | 139279 | 139280 | 139281 | 139282 | 139283 | 139284 | 141157 | 141158 | 141159 | 141160 | 141161 | 141162 | 141163 | 141164 | 144335 | 144336 | 144380 | 144384 | 151095 | 151096 | 151097 | 151098 | 151099 | 151102 | 151103 | 151104 | 151105 | 151106 | 151377 | 151378 | 151379 | 151380 | 151381 | 154379 | 154380 | 154381 | 154382 | 154383 | 156815 | 156816 | 156817 | 156818 | 156819 | 159142 | 159143 | 159144 | 159145 | 159146 | 159147 | 159148 | 163623 | 170014 | 170015 | 170016 | 170017 | 170018 | 170019 | 170026 | 170027 | 170028 | 170029 | 170030 | 170031 | 170983 | 170984 | 170985 | 170986 | 170987 | 170988