From 110b937730fe07c71792254731a22a048a10c965 Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Thu, 25 Jan 2018 17:41:40 +0100 Subject: [PATCH] Bug 19318: Allow multiple storage spaces for file uploads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch allow to configure multiple storage spaces to be used when uploading a file. Configuration is done in $KOHA_CONF. Element 'upload_path' is replaced by 'storage', which is repeatable and can contain the following elements: - name: The storage's identifier (also used for display) - adapter: The storage's adapter is a Perl module who is responsible for actually writing in the storage (creating/deleting files/directories). There is actually only one adapter, 'directory', which writes directly on the filesystem. - adapter_params: Additional parameters required for the storage's adapter - hash_filename: Whether to prepend hashvalue to the filename or not - temporary: Whether the storage is temporary or not - baseurl: Base URL of the storage, if one doesn't want to (or can't) use opac-retrieve-file.pl This is all documented in Koha::Storage There is two built-in storages: - 'TMP', the default temporary storage (not configurable) - 'DEFAULT', the default persistent storage (configurable in $KOHA_CONF) Note that if $KOHA_CONF is not updated, uploads should continues to work (the 'DEFAULT' storage will use 'upload_path', if set) This patch affects the following pages: - Tools › Upload - Tools › Upload local cover image (ZIP file upload) - Tools › Stage MARC records for import - Tools › Upload patron images - Circulation › Offline circulation file upload - Cataloguing plugin upload.pl Test plan: 0. Before applying the patch, be sure to use the pages mentioned above and upload some files, those files should continue to be accessible after the patch 1. Apply the patch, run updatedatabase and update_dbix_class_files 2. Without modifying $KOHA_CONF, verify that files uploaded in step 0 are still accessible (you can find them in tools/upload.pl) 3. Verify that you can still upload files using the pages mentioned above 4. Edit your $KOHA_CONF, copy the element from etc/koha-conf.xml in the source, set the using the value of and remove 5. Verify that you can still upload files 6. Add another storage in $KOHA_CONF, set to 0, and a . Now try to use the cataloguing plugin and see how the generated URL change. Also note that the files keep their original filename (without the hashvalue prepended) 7. Create some subdirectories under the main storage's path (using a shell). Try to upload a new files and see that these directories appear in a dropdown list. Select one, finish the upload and verify that the file has been uploaded to the selected subdirectory. 8. Run prove t/ImportBatch.t t/Koha/Storage.t t/db_dependent/ImportBatch.t t/db_dependent/Upload.t --- C4/ImportBatch.pm | 27 +- Koha/Storage.pm | 365 ++++++++++++++++++ Koha/Storage/Adapter/Directory.pm | 173 +++++++++ Koha/UploadedFile.pm | 89 +++-- Koha/UploadedFiles.pm | 19 +- Koha/Uploader.pm | 124 +++--- cataloguing/value_builder/upload.pl | 3 + etc/koha-conf.xml | 11 +- .../data/mysql/atomicupdate/bug-19318.pl | 41 ++ installer/data/mysql/kohastructure.sql | 2 +- .../en/modules/offline_circ/process_koc.tt | 2 +- .../en/modules/tools/stage-marc-import.tt | 2 +- .../prog/en/modules/tools/upload-images.tt | 2 +- .../prog/en/modules/tools/upload.tt | 119 +++--- t/ImportBatch.t | 15 +- t/Koha/Storage.t | 182 +++++++++ t/db_dependent/ImportBatch.t | 7 +- t/db_dependent/Upload.t | 102 ++--- tools/stage-marc-import.pl | 9 +- tools/upload-cover-image.pl | 12 +- tools/upload-file.pl | 16 +- tools/upload.pl | 16 +- 22 files changed, 1077 insertions(+), 261 deletions(-) create mode 100644 Koha/Storage.pm create mode 100644 Koha/Storage/Adapter/Directory.pm create mode 100644 installer/data/mysql/atomicupdate/bug-19318.pl create mode 100755 t/Koha/Storage.t diff --git a/C4/ImportBatch.pm b/C4/ImportBatch.pm index 6ae148d6d2..434fa1941c 100644 --- a/C4/ImportBatch.pm +++ b/C4/ImportBatch.pm @@ -20,6 +20,8 @@ package C4::ImportBatch; use strict; use warnings; +use Scalar::Util qw(openhandle); + use C4::Context; use C4::Koha qw( GetNormalizedISBN ); use C4::Biblio qw( @@ -1531,7 +1533,7 @@ sub SetImportRecordMatches { Reads ISO2709 binary porridge from the given file and creates MARC::Record-objects out of it. -@PARAM1, String, absolute path to the ISO2709 file. +@PARAM1, String, absolute path to the ISO2709 file or an open filehandle. @PARAM2, String, see stage_file.pl @PARAM3, String, should be utf8 @@ -1546,7 +1548,13 @@ sub RecordsFromISO2709File { my $marc_type = C4::Context->preference('marcflavour'); $marc_type .= 'AUTH' if ($marc_type eq 'UNIMARC' && $record_type eq 'auth'); - open my $fh, '<', $input_file or die "$0: cannot open input file $input_file: $!\n"; + my $fh; + if (openhandle($input_file)) { + $fh = $input_file; + } else { + open $fh, '<', $input_file or die "$0: cannot open input file $input_file: $!\n"; + } + my @marc_records; $/ = "\035"; while (<$fh>) { @@ -1562,6 +1570,7 @@ sub RecordsFromISO2709File { } } close $fh; + return ( \@errors, \@marc_records ); } @@ -1571,7 +1580,7 @@ sub RecordsFromISO2709File { Creates MARC::Record-objects out of the given MARCXML-file. -@PARAM1, String, absolute path to the ISO2709 file. +@PARAM1, String, absolute path to the ISO2709 file or an open filehandle @PARAM2, String, should be utf8 Returns two array refs. @@ -1594,7 +1603,9 @@ sub RecordsFromMARCXMLFile { =head2 RecordsFromMarcPlugin - Converts text of input_file into array of MARC records with to_marc plugin +Converts text of C<$input_file> into array of MARC records with to_marc plugin + +C<$input_file> can be either a filename or an open filehandle. =cut @@ -1604,7 +1615,13 @@ sub RecordsFromMarcPlugin { return \@return if !$input_file || !$plugin_class; # Read input file - open my $fh, '<', $input_file or die "$0: cannot open input file $input_file: $!\n"; + my $fh; + if (openhandle($input_file)) { + $fh = $input_file; + } else { + open $fh, '<', $input_file or die "$0: cannot open input file $input_file: $!\n"; + } + $/ = "\035"; while (<$fh>) { s/^\s+//; diff --git a/Koha/Storage.pm b/Koha/Storage.pm new file mode 100644 index 0000000000..95c87d7ea0 --- /dev/null +++ b/Koha/Storage.pm @@ -0,0 +1,365 @@ +package Koha::Storage; + +# Copyright 2018 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 . + +=head1 NAME + +Koha::Storage - Manage file storages + +=head1 SYNOPSIS + + use Koha::Storage; + + # Get all available storages + my $config = Koha::Storage->config; + + # Get storage instance by name + # By default, 'TMP' and 'DEFAULT' are available + # Others can be added in $KOHA_CONF + my $storage = Koha::Storage->get_instance($name) + + my $directories = $storage->directories(); + + my $filepath = $storage->filepath({ + hashvalue => $hashvalue, + filename => $filename, + dir => $dir, + }); + + my $exists = $storage->exists($filepath); + my $deleted_count = $storage->delete($filepath); + + my $fh = $storage->fh($filepath, $mode); + + my $url = $storage->url($hashfile, $filepath); + +=cut + +use Modern::Perl; + +use File::Spec; +use List::Util qw( first ); + +use C4::Context; + +use constant KOHA_UPLOAD => 'koha_upload'; + +=head1 CLASS METHODS + +=head2 config + +Returns the configuration for all available storages. + + my $config = Koha::Storage->config + +Returns an arrayref containing hashrefs. Example: + + [ + { + name => 'TMP', + adapter => 'directory', + adapter_params => { + path => '/tmp', + }, + temporary => 1, + hash_filename => 1, + }, + { + name => 'DEFAULT', + ... + }, + ... + ] + +Storages can be configured in C<$KOHA_CONF> by adding elements: + + + + + + MyStorage + + + directory + + + + + + /mnt/mystorage + + + + + 1 + + + + 0 + + + + https://mystorage.example.com/ + + + + + +The 'TMP' storage is always available and cannot be configured. + +The 'DEFAULT' storage is available if: + +=over + +=item * C is set in C<$KOHA_CONF>, or + +=item * a storage named 'DEFAULT' is configured in C<$KOHA_CONF> + +=back + +=cut + +sub config { + my $storage = C4::Context->config('storage'); + + my $config; + if (ref $storage eq 'ARRAY') { + $config = [ @$storage ]; + } elsif ($storage) { + $config = [ $storage ]; + } else { + $config = []; + } + + my $default = first { $_->{name} eq 'DEFAULT' } @$config; + unless ($default) { + # Backward compatibility for those who haven't changed their $KOHA_CONF + warn "No 'DEFAULT' storage configured. Using upload_path as a fallback."; + + my $upload_path = C4::Context->config('upload_path'); + if ($upload_path) { + unshift @$config, { + name => 'DEFAULT', + adapter => 'directory', + adapter_params => { + path => C4::Context->config('upload_path'), + }, + hash_filename => 1, + }; + } else { + warn "No upload_path defined." + } + } + + my $database = C4::Context->config('database'); + my $subdir = KOHA_UPLOAD =~ s/koha/$database/r; + unshift @$config, { + name => 'TMP', + adapter => 'directory', + adapter_params => { + path => File::Spec->catfile(File::Spec->tmpdir, $subdir), + }, + temporary => 1, + hash_filename => 1, + }; + + return $config; +} + +=head2 get_instance + +Retrieves an instance of Koha::Storage + + my $storage = Koha::Storage->get_instance($name); + +Returns a Koha::Storage object + +=cut + +my $instances = {}; + +sub get_instance { + my ($class, $name) = @_; + + unless (exists $instances->{$name}) { + my $storages = $class->config; + my $storage = first { $_->{name} eq $name } @$storages; + + if ($storage) { + $instances->{$name} = $class->new($storage); + } else { + warn "There is no storage named $name"; + } + } + + return $instances->{$name}; +} + +=head2 new + +Creates a new Koha::Storage object + + my $storage = Koha::Storage->new(\%params); + +C<%params> can contain the same keys as the one returned by Cconfig> + +You shouldn't use this directly. Use Cget_instance> instead + +=cut + +sub new { + my ($class, $params) = @_; + + my $adapter_class = 'Koha::Storage::Adapter::' . ucfirst(lc($params->{adapter})); + my $adapter; + eval { + my $adapter_file = $adapter_class =~ s,::,/,gr . '.pm'; + require $adapter_file; + $adapter = $adapter_class->new($params->{adapter_params}); + }; + if ($@) { + warn "Unable to create an instance of $adapter_class : $@"; + + return; + } + + my $self = $params; + $self->{adapter} = $adapter; + + return bless $self, $class; +} + +=head1 INSTANCE METHODS + +=head2 filepath + +Returns relative filepath of a file according to storage's parameters and file's +properties (filename, hashvalue, dir) + + my $filepath = $storage->filepath({ + hashvalue => $hashvalue, + filename => $filename, + dir => $dir, + }) + +The return value is a required parameter for several other methods. + +=cut + +sub filepath { + my ($self, $params) = @_; + + my $filepath; + if ($params->{dir}) { + $filepath .= $params->{dir} . '/'; + } + if ($params->{hashvalue} && $self->{hash_filename}) { + $filepath .= $params->{hashvalue} . '_'; + } + $filepath .= $params->{filename}; + + return $filepath; +} + +=head2 exists + +Check file existence + + my $filepath = $storage->filepath(\%params); + my $exists = $storage->exists($filepath); + +Returns a true value if the file exists, and a false value otherwise. + +=cut + +sub exists { + my ($self, $filepath) = @_; + + return $self->{adapter}->exists($filepath); +} + +=head2 fh + +Returns a file handle for the given C<$filepath> + + my $filepath = $storage->filepath(\%params); + my $fh = $storage->fh($filepath, $mode); + +For possible values of C<$mode>, see L + +=cut + +sub fh { + my ($self, $filepath, $mode) = @_; + + return $self->{adapter}->fh($filepath, $mode); +} + +=head2 directories + +Returns a list of writable directories in storage + + my $directories = $storage->directories(); + +=cut + +sub directories { + my ($self) = @_; + + return $self->{adapter}->directories(); +} + +=head2 delete + +Deletes a file + + my $filepath = $storage->filepath(\%params); + $storage->delete($filepath); + +=cut + +sub delete { + my ($self, $filepath) = @_; + + return $self->{adapter}->delete($filepath); +} + +=head2 url + +Returns the URL to access the file + + my $filepath = $storage->filepath(\%params); + my $url = $storage->url($hashvalue, $filepath); + +=cut + +sub url { + my ($self, $hashvalue, $filepath) = @_; + + if ($self->{baseurl}) { + return $self->{baseurl} . $filepath; + } + + # Default to opac-retrieve-file.pl + my $url = C4::Context->preference('OPACBaseURL'); + $url =~ s/\/$//; + $url .= '/cgi-bin/koha/opac-retrieve-file.pl?id=' . $hashvalue; + + return $url; +} + +1; diff --git a/Koha/Storage/Adapter/Directory.pm b/Koha/Storage/Adapter/Directory.pm new file mode 100644 index 0000000000..f264ae72de --- /dev/null +++ b/Koha/Storage/Adapter/Directory.pm @@ -0,0 +1,173 @@ +package Koha::Storage::Adapter::Directory; + +# Copyright 2018 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, if not, see . + +=head1 NAME + +Koha::Storage::Adapter::Directory - Storage adapter for a filesystem directory + +=head1 DESCRIPTION + +This is the default storage adapter. It stores files in a directory on the +filesystem. + +You shouldn't use this directly. Use C instead. + +=cut + +use Modern::Perl; + +use File::Basename; +use File::Find; +use File::Path qw(make_path); +use File::Spec; +use IO::File; + +=head1 INSTANCE METHODS + +=head2 new + +Creates a new C object. + + my $adapter = Koha::Storage::Adapter::Directory->new(\%params): + +C<%params> contains the following keys: + +=over + +=item * C: Mandatory. Absolute path of storage + +=back + +=cut + +sub new { + my ($class, $params) = @_; + + unless ($params->{path}) { + die "Missing parameter 'path'"; + } + + my $self = { %$params }; + + return bless $self, $class; +} + +=head2 exists + +See L. + +=cut + +sub exists { + my ($self, $filepath) = @_; + + return -e $self->abspath($filepath); +} + +=head2 fh + +See L. + +=cut + +sub fh { + my ($self, $filepath, $mode) = @_; + + my $abspath = $self->abspath($filepath); + + my $dirname = dirname($abspath); + unless (-e $dirname) { + eval { + make_path($dirname); + }; + if ($@) { + warn "Unable to create path $dirname: $@"; + return; + } + } + + unless (-w $dirname) { + warn "Directory $dirname is not writable"; + return; + } + + my $fh = IO::File->new($abspath, $mode); + unless ($fh) { + warn "File handle creation failed for $abspath (mode $mode)"; + return; + } + + $fh->binmode; + + return $fh; +} + +=head2 directories + +See L. + +=cut + +sub directories { + my ($self) = @_; + + my @directories; + + if (-e $self->{path}) { + find(sub { + if (-d $File::Find::name) { + my $relpath = $File::Find::name =~ s/^\Q$self->{path}\E\/?//r; + push @directories, $relpath if $relpath; + } + }, $self->{path}); + } + + return \@directories; +} + +=head2 delete + +See L. + +=cut + +sub delete { + my ($self, $filepath) = @_; + + return unlink $self->abspath($filepath); +} + +=head1 INTERNAL METHODS + +=head2 abspath + +Returns the absolute path of a file + + my $abspath = $adapter->abspath($filepath); + +=cut + +sub abspath { + my ($self, $filepath) = @_; + + my $abspath = File::Spec->catfile($self->{path}, $filepath); + + return $abspath; +} + +1; diff --git a/Koha/UploadedFile.pm b/Koha/UploadedFile.pm index 009c333a8d..2c73f04bfd 100644 --- a/Koha/UploadedFile.pm +++ b/Koha/UploadedFile.pm @@ -18,9 +18,10 @@ package Koha::UploadedFile; # along with Koha; if not, see . use Modern::Perl; -use File::Spec; -use parent qw(Koha::Object); +use Koha::Storage; + +use base qw(Koha::Object); =head1 NAME @@ -36,9 +37,6 @@ Koha::UploadedFile - Koha::Object class for single uploaded file # get a file handle on an uploaded_file my $fh = $upload->file_handle; - # get full path - my $path = $upload->full_path; - # delete uploaded file $upload->delete; @@ -46,8 +44,7 @@ Koha::UploadedFile - Koha::Object class for single uploaded file Allows regular CRUD operations on uploaded_files via Koha::Object / DBIx. -The delete method also takes care of deleting files. The full_path method -returns a fully qualified path for an upload. +The delete method also takes care of deleting files. Additional methods include: file_handle, httpheaders. @@ -72,36 +69,20 @@ sub delete { my ( $self, $params ) = @_; my $name = $self->filename; - my $file = $self->full_path; my $retval = $self->SUPER::delete; return $retval if $params->{keep_file}; - if( ! -e $file ) { - warn "Removing record for $name within category ". - $self->uploadcategorycode. ", but file was missing."; - } elsif( ! unlink($file) ) { - warn "Problem while deleting: $file"; - } - return $retval; -} - -=head3 full_path - -Returns the fully qualified path name for an uploaded file. + my $storage = Koha::Storage->get_instance($self->storage); + my $filepath = $self->filepath; -=cut + if ( ! $storage->exists($filepath) ) { + warn "Removing record for $name within storage " . $self->storage . ", but file was missing."; + } elsif ( ! $storage->delete($filepath) ) { + warn "Problem while deleting: $filepath"; + } -sub full_path { - my ( $self ) = @_; - my $path = File::Spec->catfile( - $self->permanent - ? $self->permanent_directory - : C4::Context->temporary_directory, - $self->dir, - $self->hashvalue. '_'. $self->filename, - ); - return $path; + return $retval; } =head3 file_handle @@ -112,10 +93,10 @@ Returns a file handle for an uploaded file. sub file_handle { my ( $self ) = @_; - $self->{_file_handle} = IO::File->new( $self->full_path, "r" ); - return if !$self->{_file_handle}; - $self->{_file_handle}->binmode; - return $self->{_file_handle}; + + my $storage = Koha::Storage->get_instance($self->storage); + + return $storage->fh($self->filepath, 'r'); } =head3 httpheaders @@ -141,19 +122,45 @@ sub httpheaders { } } -=head2 CLASS METHODS +=head3 url -=head3 permanent_directory +Returns the URL to access the file -Returns root directory for permanent storage + my $url = $uploaded_file->url; =cut -sub permanent_directory { - my ( $class ) = @_; - return C4::Context->config('upload_path'); +sub url { + my ($self) = @_; + + my $storage = Koha::Storage->get_instance($self->storage); + + return $storage->url($self->hashvalue, $self->filepath); } +=head3 filepath + +Returns the filepath of a file relative to the storage root path + + my $filepath = $uploaded_file->filepath; + +=cut + +sub filepath { + my ($self) = @_; + + my $storage = Koha::Storage->get_instance($self->storage); + my $filepath = $storage->filepath({ + hashvalue => $self->hashvalue, + filename => $self->filename, + dir => $self->dir, + }); + + return $filepath; +} + +=head2 CLASS METHODS + =head3 _type Returns name of corresponding DBIC resultset diff --git a/Koha/UploadedFiles.pm b/Koha/UploadedFiles.pm index c453c6f977..5ccacf9caf 100644 --- a/Koha/UploadedFiles.pm +++ b/Koha/UploadedFiles.pm @@ -22,9 +22,10 @@ use Modern::Perl; use C4::Koha qw( GetAuthorisedValues ); use Koha::Database; use Koha::DateUtils qw( dt_from_string ); +use Koha::Storage; use Koha::UploadedFile; -use parent qw(Koha::Objects); +use base qw(Koha::Objects); =head1 NAME @@ -126,8 +127,8 @@ sub delete_missing { $self = Koha::UploadedFiles->new if !ref($self); # handle class call my $rv = 0; while( my $row = $self->next ) { - my $file = $row->full_path; - next if -e $file; + my $storage = Koha::Storage->get_instance($row->storage); + next if $storage->exists($row->filepath); if( $params->{keep_record} ) { $rv++; next; @@ -166,18 +167,6 @@ sub search_term { =head2 CLASS METHODS -=head3 getCategories - -getCategories returns a list of upload category codes and names - -=cut - -sub getCategories { - my ( $class ) = @_; - my $cats = C4::Koha::GetAuthorisedValues('UPLOAD'); - [ map {{ code => $_->{authorised_value}, name => $_->{lib} }} @$cats ]; -} - =head3 _type Returns name of corresponding DBIC resultset diff --git a/Koha/Uploader.pm b/Koha/Uploader.pm index 081bc3f078..58c2e39b99 100644 --- a/Koha/Uploader.pm +++ b/Koha/Uploader.pm @@ -31,7 +31,7 @@ Koha::Uploader - Facilitate file uploads (temporary and permanent) # add an upload (see tools/upload-file.pl) # the public flag allows retrieval via OPAC - my $upload = Koha::Uploader->new( public => 1, category => 'A' ); + my $upload = Koha::Uploader->new( public => 1, storage => 'DEFAULT' ); my $cgi = $upload->cgi; # Do something with $upload->count, $upload->result or $upload->err @@ -58,18 +58,14 @@ Koha::Uploader - Facilitate file uploads (temporary and permanent) =cut -use constant KOHA_UPLOAD => 'koha_upload'; use constant BYTES_DIGEST => 2048; use constant ERR_EXISTS => 'UPLERR_ALREADY_EXISTS'; use constant ERR_PERMS => 'UPLERR_CANNOT_WRITE'; -use constant ERR_ROOT => 'UPLERR_NO_ROOT_DIR'; -use constant ERR_TEMP => 'UPLERR_NO_TEMP_DIR'; use Modern::Perl; use CGI; # no utf8 flag, since it may interfere with binary uploads use Digest::MD5; use Encode; -use IO::File; use Time::HiRes; use base qw(Class::Accessor); @@ -78,6 +74,7 @@ use C4::Context; use C4::Koha; use Koha::UploadedFile; use Koha::UploadedFiles; +use Koha::Storage; __PACKAGE__->mk_ro_accessors( qw|| ); @@ -85,10 +82,21 @@ __PACKAGE__->mk_ro_accessors( qw|| ); =head2 new - Returns new object based on Class::Accessor. - Use tmp or temp flag for temporary storage. - Use public flag to mark uploads as available in OPAC. - The category parameter is only useful for permanent storage. +Returns new object based on Class::Accessor. + + my $uploader = Koha::Uploader->new(\%params); + +C<%params> contains the following keys: + +=over + +=item * C: Mandatory. Storage's name + +=item * C: Subdirectory in storage + +=item * C: Whether or not the uploaded files are public (available in OPAC). + +=back =cut @@ -191,74 +199,51 @@ sub allows_add_by { sub _init { my ( $self, $params ) = @_; - $self->{rootdir} = Koha::UploadedFile->permanent_directory; - $self->{tmpdir} = C4::Context::temporary_directory; - - $params->{tmp} = $params->{temp} if !exists $params->{tmp}; - $self->{temporary} = $params->{tmp}? 1: 0; #default false - if( $params->{tmp} ) { - my $db = C4::Context->config('database'); - $self->{category} = KOHA_UPLOAD; - $self->{category} =~ s/koha/$db/; - } else { - $self->{category} = $params->{category} || KOHA_UPLOAD; - } - + $self->{storage} = Koha::Storage->get_instance($params->{storage}); $self->{files} = {}; $self->{uid} = C4::Context->userenv->{number} if C4::Context->userenv; - $self->{public} = $params->{public}? 1: undef; + $self->{public} = $params->{public} ? 1 : 0; + $self->{dir} = $params->{dir} // ''; } sub _fh { my ( $self, $filename ) = @_; - if( $self->{files}->{$filename} ) { + + if ( $self->{files}->{$filename} ) { return $self->{files}->{$filename}->{fh}; } } sub _create_file { my ( $self, $filename ) = @_; - my $fh; - if( $self->{files}->{$filename} && - $self->{files}->{$filename}->{errcode} ) { - #skip - } elsif( !$self->{temporary} && !$self->{rootdir} ) { - $self->{files}->{$filename}->{errcode} = ERR_ROOT; #no rootdir - } elsif( $self->{temporary} && !$self->{tmpdir} ) { - $self->{files}->{$filename}->{errcode} = ERR_TEMP; #no tempdir + + return if ($self->{files}->{$filename} && $self->{files}->{$filename}->{errcode}); + my $hashval = $self->{files}->{$filename}->{hash}; + my $filepath = $self->{storage}->filepath({ + hashvalue => $hashval, + filename => $filename, + dir => $self->{dir}, + }); + + # if the file exists and it is registered, then set error + # if it exists, but is not in the database, we will overwrite + if ( $self->{storage}->exists($filepath) && + Koha::UploadedFiles->search({ + hashvalue => $hashval, + storage => $self->{storage}->{name}, + })->count ) { + $self->{files}->{$filename}->{errcode} = ERR_EXISTS; #already exists + return; + } + + my $fh = $self->{storage}->fh($filepath, 'w'); + if ($fh) { + $self->{files}->{$filename}->{fh} = $fh; } else { - my $dir = $self->_dir; - my $hashval = $self->{files}->{$filename}->{hash}; - my $fn = $hashval. '_'. $filename; - - # if the file exists and it is registered, then set error - # if it exists, but is not in the database, we will overwrite - if( -e "$dir/$fn" && - Koha::UploadedFiles->search({ - hashvalue => $hashval, - uploadcategorycode => $self->{category}, - })->count ) { - $self->{files}->{$filename}->{errcode} = ERR_EXISTS; - return; - } - - $fh = IO::File->new( "$dir/$fn", "w"); - if( $fh ) { - $fh->binmode; - $self->{files}->{$filename}->{fh}= $fh; - } else { - $self->{files}->{$filename}->{errcode} = ERR_PERMS; - } + $self->{files}->{$filename}->{errcode} = ERR_PERMS; #not writable } - return $fh; -} -sub _dir { - my ( $self ) = @_; - my $dir = $self->{temporary}? $self->{tmpdir}: $self->{rootdir}; - $dir.= '/'. $self->{category}; - mkdir $dir if !-d $dir; - return $dir; + return $fh; } sub _hook { @@ -283,14 +268,14 @@ sub _done { sub _register { my ( $self, $filename, $size ) = @_; my $rec = Koha::UploadedFile->new({ + storage => $self->{storage}->{name}, hashvalue => $self->{files}->{$filename}->{hash}, filename => $filename, - dir => $self->{category}, + dir => $self->{dir}, filesize => $size, owner => $self->{uid}, - uploadcategorycode => $self->{category}, public => $self->{public}, - permanent => $self->{temporary}? 0: 1, + permanent => $self->{storage}->{temporary} ? 0 : 1, })->store; $self->{files}->{$filename}->{id} = $rec->id if $rec; } @@ -300,11 +285,12 @@ sub _compute { # For temporary files, the id is made unique with time my ( $self, $name, $block ) = @_; if( !$self->{files}->{$name}->{hash} ) { - my $str = $name. ( $self->{uid} // '0' ). - ( $self->{temporary}? Time::HiRes::time(): '' ). - $self->{category}. substr( $block, 0, BYTES_DIGEST ); + my $str = $name . ( $self->{uid} // '0' ) . + ( $self->{storage}->{temporary} ? Time::HiRes::time() : '' ) . + $self->{storage}->{name} . $self->{dir} . + substr( $block, 0, BYTES_DIGEST ); # since Digest cannot handle wide chars, we need to encode here - # there could be a wide char in the filename or the category + # there could be a wide char in the filename my $h = Digest::MD5::md5_hex( Encode::encode_utf8( $str ) ); $self->{files}->{$name}->{hash} = $h; } diff --git a/cataloguing/value_builder/upload.pl b/cataloguing/value_builder/upload.pl index 82b9816f62..bcace84cbb 100755 --- a/cataloguing/value_builder/upload.pl +++ b/cataloguing/value_builder/upload.pl @@ -42,6 +42,9 @@ my $builder = sub { if( str && str.match(/id=([0-9a-f]+)/) ) { term = RegExp.\$1; myurl = '../tools/upload.pl?op=search&index='+index+'&term='+term+'&plugin=1'; + } else if (str && str.match(/\\/([^\\/]*)\$/)) { + term = RegExp.\$1; + myurl = '../tools/upload.pl?op=search&index='+index+'&term='+term+'&plugin=1'; } else { myurl = '../tools/upload.pl?op=new&index='+index+'&plugin=1'; } diff --git a/etc/koha-conf.xml b/etc/koha-conf.xml index 118b6b8207..3393a645e9 100644 --- a/etc/koha-conf.xml +++ b/etc/koha-conf.xml @@ -79,8 +79,15 @@ 1 __PLUGINS_DIR__ 0 - - + + DEFAULT + directory + + + + + 1 + __INTRANET_CGI_DIR__ __OPAC_CGI_DIR__/opac __OPAC_TMPL_DIR__ diff --git a/installer/data/mysql/atomicupdate/bug-19318.pl b/installer/data/mysql/atomicupdate/bug-19318.pl new file mode 100644 index 0000000000..2663304201 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug-19318.pl @@ -0,0 +1,41 @@ +use Modern::Perl; + +return { + bug_number => '19318', + description => 'Add uploaded_files.storage and remove uploaded_files.uploadcategorycode', + up => sub { + my ($args) = @_; + my ($dbh, $out) = @$args{qw(dbh out)}; + + unless (column_exists('uploaded_files', 'storage')) { + $dbh->do(q{ + ALTER TABLE uploaded_files + ADD COLUMN storage VARCHAR(255) NULL DEFAULT NULL AFTER id + }); + } + + if (column_exists('uploaded_files', 'uploadcategorycode')) { + $dbh->do(q{ + ALTER TABLE uploaded_files + DROP COLUMN uploadcategorycode + }); + } + + $dbh->do(q{ + UPDATE uploaded_files + SET storage = IF(permanent, 'DEFAULT', 'TMP') + WHERE storage IS NULL OR storage = '' + }); + + $dbh->do(q{ + UPDATE uploaded_files + SET dir = '' + WHERE storage = 'TMP' + }); + + $dbh->do(q{ + ALTER TABLE uploaded_files + MODIFY COLUMN storage VARCHAR(255) NOT NULL + }); + }, +} diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index ccf99df9e0..75b7ef20ee 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -5158,12 +5158,12 @@ DROP TABLE IF EXISTS `uploaded_files`; /*!40101 SET character_set_client = utf8 */; CREATE TABLE `uploaded_files` ( `id` int(11) NOT NULL AUTO_INCREMENT, + `storage` varchar(255) NOT NULL, `hashvalue` char(40) COLLATE utf8mb4_unicode_ci NOT NULL, `filename` mediumtext COLLATE utf8mb4_unicode_ci NOT NULL, `dir` mediumtext COLLATE utf8mb4_unicode_ci NOT NULL, `filesize` int(11) DEFAULT NULL, `dtcreated` timestamp NOT NULL DEFAULT current_timestamp() ON UPDATE current_timestamp(), - `uploadcategorycode` text COLLATE utf8mb4_unicode_ci DEFAULT NULL, `owner` int(11) DEFAULT NULL, `public` tinyint(4) DEFAULT NULL, `permanent` tinyint(4) DEFAULT NULL, diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/offline_circ/process_koc.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/offline_circ/process_koc.tt index 20e95365d4..bd5638addc 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/offline_circ/process_koc.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/offline_circ/process_koc.tt @@ -123,7 +123,7 @@ $("#fileuploadstatus").show(); $("form#processfile #uploadedfileid").val(''); $("form#enqueuefile #uploadedfileid").val(''); - xhr= AjaxUpload( $('#fileToUpload'), $('#fileuploadprogress'), 'temp=1', cbUpload ); + xhr= AjaxUpload( $('#fileToUpload'), $('#fileuploadprogress'), 'storage=TMP', cbUpload ); } function cbUpload( status, fileid, errors ) { diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/stage-marc-import.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/stage-marc-import.tt index 7877265d90..55840644a8 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/stage-marc-import.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/stage-marc-import.tt @@ -436,7 +436,7 @@ $('#profile_fieldset').hide(); $("#fileuploadstatus").show(); $("#uploadedfileid").val(''); - xhr= AjaxUpload( $('#fileToUpload'), $('#fileuploadprogress'), 'temp=1', cbUpload ); + xhr= AjaxUpload( $('#fileToUpload'), $('#fileuploadprogress'), 'storage=TMP', cbUpload ); $("#fileuploadcancel").show(); } function CancelUpload() { diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/upload-images.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/upload-images.tt index 506187271b..41d8063d10 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/upload-images.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/upload-images.tt @@ -213,7 +213,7 @@ $('#uploadform button.submit').prop('disabled',true); $("#fileuploadstatus").show(); $("#uploadedfileid").val(''); - xhr= AjaxUpload( $('#fileToUpload'), $('#fileuploadprogress'), 'temp=1', cbUpload ); + xhr= AjaxUpload( $('#fileToUpload'), $('#fileuploadprogress'), 'storage=TMP', cbUpload ); } function cbUpload( status, fileid, errors ) { if( status=='done' ) { diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/upload.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/upload.tt index 7a232de0ef..9ce52cbef0 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/upload.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/upload.tt @@ -1,9 +1,11 @@ [% USE raw %] +[% USE To %] [% USE Asset %] [% USE Koha %] [% USE KohaDates %] [% USE TablesSettings %] [% USE AuthorisedValues %] +[% USE JSON.Escape %] [% SET footerjs = 1 %] [% INCLUDE 'doc-head-open.inc' %] [% IF plugin %] @@ -13,6 +15,14 @@ [% END %] [% INCLUDE 'doc-head-close.inc' %] +[% BLOCK storage_label %] + [% SWITCH name %] + [% CASE 'TMP' %]Temporary + [% CASE 'DEFAULT' %]Default + [% CASE %][% name | html %] + [% END %] +[% END %] + [% BLOCK plugin_pars %] [% IF plugin %] @@ -57,35 +67,21 @@ - [% IF uploadcategories %] -
  • - - -
  • - [% END %] - [% IF !plugin %] -
  • - [% IF uploadcategories %] -
    Note: For temporary uploads do not select a category.
    - [% ELSE %] -
    - Note: No upload categories are defined. - [% IF ( CAN_user_parameters_manage_auth_values ) -%] - Add values to the UPLOAD authorized value category otherwise all uploads will be marked as temporary. - [% ELSE -%] - An administrator must add values to the UPLOAD authorized value category otherwise all uploads will be marked as temporary. +
  • + + +
  • +
  • + + +
  • [% IF plugin %] @@ -204,9 +200,12 @@ Size Hashvalue Date added - Category - [% IF !plugin %]Public[% END %] - [% IF !plugin %]Temporary[% END %] + Storage + Directory + [% IF !plugin %] + Public + Temporary + [% END %] Actions @@ -217,9 +216,8 @@ [% record.filesize | html %] [% record.hashvalue | html %] [% record.dtcreated | $KohaDates with_hours = 1 %] - - [% AuthorisedValues.GetByCode( 'UPLOAD', record.uploadcategorycode ) | html %] - + [% PROCESS storage_label name=record.storage %] + [% record.dir | html %] [% IF !plugin %] [% IF record.public %] @@ -232,7 +230,7 @@ [% END %] [% IF plugin %] - + [% END %] [% IF record.owner == owner || CAN_user_tools_upload_manage %] @@ -348,17 +346,16 @@ $("#searchfile").hide(); $("#lastbreadcrumb").text( _("Add a new upload") ); - var cat, xtra=''; - if( $("#uploadcategory").val() ) - cat = encodeURIComponent( $("#uploadcategory").val() ); - if( cat ) xtra= 'category=' + cat + '&'; + var xtra = 'storage=' + $('#storage').val(); + xtra = xtra + '&dir=' + $('#dir').val(); [% IF plugin %] - xtra = xtra + 'public=1&temp=0'; + xtra = xtra + '&public=1'; [% ELSE %] - if( !cat ) xtra = 'temp=1&'; - if( $('#public').prop('checked') ) xtra = xtra + 'public=1'; + if ( $('#public').prop('checked') ) { + xtra = xtra + '&public=1'; + } [% END %] - xhr= AjaxUpload( $('#fileToUpload'), $('#fileuploadprogress'), xtra, cbUpload ); + xhr = AjaxUpload( $('#fileToUpload'), $('#fileuploadprogress'), xtra, cbUpload ); } function CancelUpload() { if( xhr ) xhr.abort(); @@ -401,7 +398,7 @@ var rv; switch(code) { case 'UPLERR_ALREADY_EXISTS': - rv = _("This file already exists (in this category)."); + rv = _("This file already exists (in this storage)."); break; case 'UPLERR_CANNOT_WRITE': rv = _("File could not be created. Check permissions."); @@ -450,12 +447,9 @@ $(window.opener.document).find('#[% index | html %]').val( '' ); [% END %] } - function Choose(hashval) { - var res = '[% Koha.Preference('OPACBaseURL') | html %]'; - res = res.replace( /\/$/, ''); - res = res + '/cgi-bin/koha/opac-retrieve-file.pl?id=' + hashval; + function Choose(url) { [% IF index %] - $(window.opener.document).find('#[% index | html %]').val( res ); + $(window.opener.document).find('#[% index | html %]').val( url ); [% END %] window.close(); } @@ -484,8 +478,8 @@ }); $("#uploadresults tbody").on("click",".choose_entry",function(e){ e.preventDefault(); - var record_hashvalue = $(this).data("record-hashvalue"); - Choose( record_hashvalue ); + var record_url = $(this).data("record-url"); + Choose( record_url ); }); $("#uploadresults tbody").on("click",".download_entry",function(e){ e.preventDefault(); @@ -519,6 +513,29 @@ } }); + [% END %] [% INCLUDE 'intranet-bottom.inc' %] diff --git a/t/ImportBatch.t b/t/ImportBatch.t index 7274879a3a..f64245fba3 100755 --- a/t/ImportBatch.t +++ b/t/ImportBatch.t @@ -31,7 +31,7 @@ BEGIN { t::lib::Mocks::mock_preference('marcflavour', 'MARC21'); subtest 'RecordsFromISO2709File' => sub { - plan tests => 4; + plan tests => 5; my ( $errors, $recs ); my $file = create_file({ whitespace => 1, format => 'marc' }); @@ -48,10 +48,16 @@ subtest 'RecordsFromISO2709File' => sub { ( $errors, $recs ) = C4::ImportBatch::RecordsFromISO2709File( $file, 'biblio', 'UTF-8' ); is( @$recs, 2, 'File contains 2 records' ); + $file = create_file({ two => 1, format => 'marc' }); + open my $fh, '<', $file; + ( $errors, $recs ) = C4::ImportBatch::RecordsFromISO2709File( $fh, 'biblio', 'UTF-8' ); + close $fh; + is( @$recs, 2, 'Can take a file handle as parameter' ); + }; subtest 'RecordsFromMARCXMLFile' => sub { - plan tests => 3; + plan tests => 4; my ( $errors, $recs ); my $file = create_file({ whitespace => 1, format => 'marcxml' }); @@ -66,6 +72,11 @@ subtest 'RecordsFromMARCXMLFile' => sub { ( $errors, $recs ) = C4::ImportBatch::RecordsFromMARCXMLFile( $file, 'UTF-8' ); is( @$recs, 2, 'File has two records' ); + $file = create_file({ two => 1, format => 'marcxml' }); + open my $fh, '<', $file; + ( $errors, $recs ) = C4::ImportBatch::RecordsFromMARCXMLFile( $fh, 'UTF-8' ); + close $fh; + is( @$recs, 2, 'Can take a filehandle as parameter' ); }; sub create_file { diff --git a/t/Koha/Storage.t b/t/Koha/Storage.t new file mode 100755 index 0000000000..a87c626f91 --- /dev/null +++ b/t/Koha/Storage.t @@ -0,0 +1,182 @@ +# Copyright 2018 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 File::Path qw(remove_tree make_path); +use File::Spec; +use File::Temp qw(tempdir); +use Test::More tests => 5; +use Test::MockModule; + +use C4::Context; + +BEGIN { use_ok('Koha::Storage') } + +my $storage_config = [ + { + name => 'DEFAULT', + adapter => 'directory', + adapter_params => { + path => tempdir('koha-storage-DEFAULT-XXXXXX', TMPDIR => 1, CLEANUP => 1), + }, + hash_filename => 1, + }, + { + name => 'external', + adapter => 'directory', + adapter_params => { + path => tempdir('koha-storage-external-XXXXXX', TMPDIR => 1, CLEANUP => 1), + }, + baseurl => 'https://external.example.com/', + }, +]; + +my $c4_context = Test::MockModule->new('C4::Context'); +$c4_context->mock('config', sub { + my ($class, $name) = @_; + + if ($name eq 'storage') { + return $storage_config; + } + + return C4::Context::_common_config($name, 'config') +}); + +my $config = Koha::Storage->config; + +my $expected = [ + { + name => 'TMP', + adapter => 'directory', + adapter_params => { + path => File::Spec->catfile(File::Spec->tmpdir, C4::Context->config('database') . '_upload'), + }, + temporary => 1, + hash_filename => 1, + }, + @$storage_config, +]; + +is_deeply($config, $expected, 'Koha::Storage->config return value is as expected'); + +subtest 'TMP' => sub { + plan tests => 8; + + # Clean temporary storage + my $tmpdir = $config->[0]->{adapter_params}->{path}; + if (-e $tmpdir) { + remove_tree($tmpdir); + } + + my $tmp_storage = Koha::Storage->get_instance('TMP'); + isa_ok($tmp_storage, 'Koha::Storage', 'Koha::Storage->get_instance return value'); + is($tmp_storage->{name}, 'TMP', 'Koha::Storage->get_instance returns the correct instance'); + + my $filepath = $tmp_storage->filepath({ + dir => 'one/two', + hashvalue => 'abcdef', + filename => 'foo.bar', + }); + is($filepath, 'one/two/abcdef_foo.bar', 'filepath is correct'); + + ok(!$tmp_storage->exists($filepath), "$filepath doesn't exist yet"); + + my $fh = $tmp_storage->fh($filepath, 'w'); + print $fh 'foo.bar content'; + close $fh; + + ok($tmp_storage->exists($filepath), "$filepath now exists"); + + $fh = $tmp_storage->fh($filepath, 'r'); + my $content = <$fh>; + is($content, 'foo.bar content', "$filepath content is as expected"); + + my $directories = $tmp_storage->directories; + is_deeply($directories, ['one', 'one/two'], 'directories() return value is as expected'); + + my $url = $tmp_storage->url('abcdef', $filepath); + my $expected_url = C4::Context->preference('OPACBaseURL') . '/cgi-bin/koha/opac-retrieve-file.pl?id=abcdef'; + is($url, $expected_url, 'url() return value is as expected'); +}; + +subtest 'DEFAULT' => sub { + plan tests => 8; + + my $storage = Koha::Storage->get_instance('DEFAULT'); + isa_ok($storage, 'Koha::Storage', 'Koha::Storage->get_instance return value'); + is($storage->{name}, 'DEFAULT', 'Koha::Storage->get_instance returns the correct instance'); + + my $filepath = $storage->filepath({ + dir => 'one/two', + hashvalue => 'abcdef', + filename => 'foo.bar', + }); + is($filepath, 'one/two/abcdef_foo.bar', 'filepath is correct'); + + ok(!$storage->exists($filepath), "$filepath doesn't exist yet"); + + my $fh = $storage->fh($filepath, 'w'); + print $fh 'foo.bar content'; + close $fh; + + ok($storage->exists($filepath), "$filepath now exists"); + + $fh = $storage->fh($filepath, 'r'); + my $content = <$fh>; + is($content, 'foo.bar content', "$filepath content is as expected"); + + my $directories = $storage->directories; + is_deeply($directories, ['one', 'one/two'], 'directories() return value is as expected'); + + my $url = $storage->url('abcdef', $filepath); + my $expected_url = C4::Context->preference('OPACBaseURL') . '/cgi-bin/koha/opac-retrieve-file.pl?id=abcdef'; + is($url, $expected_url, 'url() return value is as expected'); +}; + +subtest 'external' => sub { + plan tests => 8; + + my $storage = Koha::Storage->get_instance('external'); + isa_ok($storage, 'Koha::Storage', 'Koha::Storage->get_instance return value'); + is($storage->{name}, 'external', 'Koha::Storage->get_instance returns the correct instance'); + + my $filepath = $storage->filepath({ + dir => 'one/two', + hashvalue => 'abcdef', + filename => 'foo.bar', + }); + is($filepath, 'one/two/foo.bar', 'filepath is correct'); + + ok(!$storage->exists($filepath), "$filepath doesn't exist yet"); + + my $fh = $storage->fh($filepath, 'w'); + print $fh 'foo.bar content'; + close $fh; + + ok($storage->exists($filepath), "$filepath now exists"); + + $fh = $storage->fh($filepath, 'r'); + my $content = <$fh>; + is($content, 'foo.bar content', "$filepath content is as expected"); + + my $directories = $storage->directories; + is_deeply($directories, ['one', 'one/two'], 'directories() return value is as expected'); + + my $url = $storage->url('abcdef', $filepath); + my $expected_url = 'https://external.example.com/one/two/foo.bar'; + is($url, $expected_url, 'url() return value is as expected'); +}; diff --git a/t/db_dependent/ImportBatch.t b/t/db_dependent/ImportBatch.t index c96222de5e..6ac963b443 100755 --- a/t/db_dependent/ImportBatch.t +++ b/t/db_dependent/ImportBatch.t @@ -183,7 +183,7 @@ my $batch3_results = $dbh->do('SELECT * FROM import_batches WHERE import_batch_i is( $batch3_results, "0E0", "Batch 3 has been deleted"); subtest "RecordsFromMarcPlugin" => sub { - plan tests => 5; + plan tests => 6; # Create a test file my ( $fh, $name ) = tempfile(); @@ -211,6 +211,11 @@ subtest "RecordsFromMarcPlugin" => sub { 'Checked one field in first record' ); is( $records->[1]->subfield('100', 'a'), 'Another', 'Checked one field in second record' ); + + open my $fh2, '<', $name; + $records = C4::ImportBatch::RecordsFromMarcPlugin( $fh2, ref $plugin, 'UTF-8' ); + close $fh2; + is( @$records, 2, 'Can take a filehandle as parameter' ); }; $schema->storage->txn_rollback; diff --git a/t/db_dependent/Upload.t b/t/db_dependent/Upload.t index 8c672ea199..093dbe20e5 100755 --- a/t/db_dependent/Upload.t +++ b/t/db_dependent/Upload.t @@ -2,7 +2,7 @@ use Modern::Perl; use File::Temp qw/ tempdir /; -use Test::More tests => 13; +use Test::More tests => 12; use Test::Warn; use Try::Tiny; @@ -23,34 +23,44 @@ our $builder = t::lib::TestBuilder->new; our $current_upload = 0; our $uploads = [ [ - { name => 'file1', cat => 'A', size => 6000 }, - { name => 'file2', cat => 'A', size => 8000 }, + { name => 'file1', storage => 'DEFAULT', dir => 'A', size => 6000 }, + { name => 'file2', storage => 'DEFAULT', dir => 'A', size => 8000 }, ], [ - { name => 'file3', cat => 'B', size => 1000 }, + { name => 'file3', storage => 'DEFAULT', dir => 'B', size => 1000 }, ], [ - { name => 'file4', cat => undef, size => 5000 }, # temporary + { name => 'file4', storage => 'TMP', dir => undef, size => 5000 }, ], [ - { name => 'file2', cat => 'A', size => 8000 }, - # uploading a duplicate in cat A should fail + { name => 'file2', storage => 'DEFAULT', dir => 'A', size => 8000 }, + # uploading a duplicate in dir A should fail ], [ - { name => 'file4', cat => undef, size => 5000 }, # temp duplicate + { name => 'file4', storage => 'TMP', dir => undef, size => 5000 }, # temp duplicate ], [ - { name => 'file5', cat => undef, size => 7000 }, + { name => 'file5', storage => 'DEFAULT', dir => undef, size => 7000 }, ], [ - { name => 'file6', cat => undef, size => 6500 }, - { name => 'file7', cat => undef, size => 6501 }, + { name => 'file6', storage => 'TMP', dir => undef, size => 6500 }, + { name => 'file7', storage => 'TMP', dir => undef, size => 6501 }, ], ]; # Redirect upload dir structure and mock C4::Context and CGI my $tempdir = tempdir( CLEANUP => 1 ); -t::lib::Mocks::mock_config('upload_path', $tempdir); +my $storage_config = [ + { + name => 'DEFAULT', + adapter => 'directory', + adapter_params => { + path => $tempdir, + }, + hash_filename => 1, + }, +]; +t::lib::Mocks::mock_config('storage', $storage_config); my $specmod = Test::MockModule->new( 'C4::Context' ); $specmod->mock( 'temporary_directory' => sub { return $tempdir; } ); my $cgimod = Test::MockModule->new( 'CGI' ); @@ -68,21 +78,12 @@ subtest 'Make a fresh start' => sub { is( Koha::UploadedFiles->count, 0, 'No records left' ); }; -subtest 'permanent_directory and temporary_directory' => sub { - plan tests => 2; - - # Check mocked directories - is( Koha::UploadedFile->permanent_directory, $tempdir, - 'Check permanent directory' ); - is( C4::Context::temporary_directory, $tempdir, - 'Check temporary directory' ); -}; - subtest 'Add two uploads in category A' => sub { plan tests => 9; my $upl = Koha::Uploader->new({ - category => $uploads->[$current_upload]->[0]->{cat}, + storage => $uploads->[$current_upload]->[0]->{storage}, + dir => $uploads->[$current_upload]->[0]->{dir}, }); my $cgi= $upl->cgi; my $res= $upl->result; @@ -95,26 +96,27 @@ subtest 'Add two uploads in category A' => sub { }, { order_by => { -asc => 'filename' }}); my $rec = $rs->next; is( $rec->filename, 'file1', 'Check file name' ); - is( $rec->uploadcategorycode, 'A', 'Check category A' ); + is( $rec->dir, 'A', 'Check dir A' ); is( $rec->filesize, 6000, 'Check size of file1' ); $rec = $rs->next; is( $rec->filename, 'file2', 'Check file name 2' ); is( $rec->filesize, 8000, 'Check size of file2' ); - is( $rec->public, undef, 'Check public undefined' ); + is( $rec->public, 0, 'Check public 0' ); }; subtest 'Add another upload, check file_handle' => sub { plan tests => 5; my $upl = Koha::Uploader->new({ - category => $uploads->[$current_upload]->[0]->{cat}, + storage => $uploads->[$current_upload]->[0]->{storage}, + dir => $uploads->[$current_upload]->[0]->{dir}, public => 1, }); my $cgi= $upl->cgi; is( $upl->count, 1, 'Upload 2 includes one file' ); my $res= $upl->result; my $rec = Koha::UploadedFiles->find( $res ); - is( $rec->uploadcategorycode, 'B', 'Check category B' ); + is( $rec->dir, 'B', 'Check dir B' ); is( $rec->public, 1, 'Check public == 1' ); my $fh = $rec->file_handle; is( ref($fh) eq 'IO::File' && $fh->opened, 1, 'Get returns a file handle' ); @@ -128,18 +130,22 @@ subtest 'Add another upload, check file_handle' => sub { subtest 'Add temporary upload' => sub { plan tests => 2; - my $upl = Koha::Uploader->new({ tmp => 1 }); #temporary + my $upl = Koha::Uploader->new({ + storage => $uploads->[$current_upload]->[0]->{storage}, + dir => $uploads->[$current_upload]->[0]->{dir}, + }); my $cgi= $upl->cgi; is( $upl->count, 1, 'Upload 3 includes one temporary file' ); my $rec = Koha::UploadedFiles->find( $upl->result ); - is( $rec->uploadcategorycode =~ /_upload$/, 1, 'Check category temp file' ); + is( $rec->dir, '', 'Check dir is empty' ); }; subtest 'Add same file in same category' => sub { plan tests => 3; my $upl = Koha::Uploader->new({ - category => $uploads->[$current_upload]->[0]->{cat}, + storage => $uploads->[$current_upload]->[0]->{storage}, + dir => $uploads->[$current_upload]->[0]->{dir}, }); my $cgi= $upl->cgi; is( $upl->count, 0, 'Upload 4 failed as expected' ); @@ -152,31 +158,38 @@ subtest 'Test delete via UploadedFile as well as UploadedFiles' => sub { plan tests => 10; # add temporary file with same name and contents (file4) - my $upl = Koha::Uploader->new({ tmp => 1 }); + my $upl = Koha::Uploader->new({ + storage => $uploads->[$current_upload]->[0]->{storage}, + dir => $uploads->[$current_upload]->[0]->{dir}, + }); my $cgi= $upl->cgi; is( $upl->count, 1, 'Add duplicate temporary file (file4)' ); my $id = $upl->result; - my $path = Koha::UploadedFiles->find( $id )->full_path; + my $uploaded_file = Koha::UploadedFiles->find( $id ); + my $storage = Koha::Storage->get_instance($uploaded_file->storage); # testing delete via UploadedFiles (plural) my $delete = Koha::UploadedFiles->search({ id => $id })->delete; isnt( $delete, "0E0", 'Delete successful' ); - isnt( -e $path, 1, 'File no longer found after delete' ); + isnt( $storage->exists($uploaded_file->filepath), 1, 'File no longer found after delete' ); is( Koha::UploadedFiles->find( $id ), undef, 'Record also gone' ); # testing delete via UploadedFile (singular) # Note that find returns a Koha::Object - $upl = Koha::Uploader->new({ tmp => 1 }); + $upl = Koha::Uploader->new({ + storage => $uploads->[$current_upload]->[0]->{storage}, + dir => $uploads->[$current_upload]->[0]->{dir}, + }); $upl->cgi; my $kohaobj = Koha::UploadedFiles->find( $upl->result ); - $path = $kohaobj->full_path; + $storage = Koha::Storage->get_instance($kohaobj->storage); $delete = $kohaobj->delete; ok( $delete, 'Delete successful' ); - isnt( -e $path, 1, 'File no longer found after delete' ); + isnt($storage->exists($kohaobj->filepath), 1, 'File no longer found after delete' ); # add another record with TestBuilder, so file does not exist # catch warning - my $upload01 = $builder->build({ source => 'UploadedFile' }); + my $upload01 = $builder->build({ source => 'UploadedFile', value => {storage => 'TMP'} }); warning_like { $delete = Koha::UploadedFiles->find( $upload01->{id} )->delete; } qr/file was missing/, 'delete warns when file is missing'; @@ -198,8 +211,8 @@ subtest 'Test delete_missing' => sub { plan tests => 5; # If we add files via TestBuilder, they do not exist - my $upload01 = $builder->build({ source => 'UploadedFile' }); - my $upload02 = $builder->build({ source => 'UploadedFile' }); + my $upload01 = $builder->build({ source => 'UploadedFile', value => {storage => 'TMP'} }); + my $upload02 = $builder->build({ source => 'UploadedFile', value => {storage => 'TMP'} }); # dry run first my $deleted = Koha::UploadedFiles->delete_missing({ keep_record => 1 }); is( $deleted, 2, 'Expect two records with missing files' ); @@ -226,15 +239,13 @@ subtest 'Call search_term with[out] private flag' => sub { })->count, 4, 'Returns now four results' ); }; -subtest 'Simple tests for httpheaders and getCategories' => sub { - plan tests => 2; +subtest 'Simple tests for httpheaders' => sub { + plan tests => 1; my $rec = Koha::UploadedFiles->search_term({ term => 'file' })->next; my @hdrs = $rec->httpheaders; is( @hdrs == 4 && $hdrs[1] =~ /application\/octet-stream/, 1, 'Simple test for httpheaders'); $builder->build({ source => 'AuthorisedValue', value => { category => 'UPLOAD', authorised_value => 'HAVE_AT_LEAST_ONE', lib => 'Hi there' } }); - my $cat = Koha::UploadedFiles->getCategories; - is( @$cat >= 1, 1, 'getCategories returned at least one category' ); }; subtest 'Testing allows_add_by' => sub { @@ -279,7 +290,10 @@ subtest 'Testing delete_temporary' => sub { plan tests => 9; # Add two temporary files: result should be 3 + 3 - Koha::Uploader->new({ tmp => 1 })->cgi; # add file6 and file7 + Koha::Uploader->new({ + storage => $uploads->[$current_upload]->[0]->{storage}, + dir => $uploads->[$current_upload]->[0]->{dir}, + })->cgi; # add file6 and file7 is( Koha::UploadedFiles->search->count, 6, 'Test starting count' ); is( Koha::UploadedFiles->search({ permanent => 1 })->count, 3, 'Includes 3 permanent' ); diff --git a/tools/stage-marc-import.pl b/tools/stage-marc-import.pl index 7653bc6893..c0b733a1ce 100755 --- a/tools/stage-marc-import.pl +++ b/tools/stage-marc-import.pl @@ -87,17 +87,18 @@ if ($completedJobID) { $template->param(map { $_ => $results->{$_} } keys %{ $results }); } elsif ($fileID) { my $upload = Koha::UploadedFiles->find( $fileID ); - my $file = $upload->full_path; + my $storage = Koha::Storage->get_instance($upload->storage); + my $fh = $storage->fh($upload->filepath, 'r'); my $filename = $upload->filename; my ( $errors, $marcrecords ); if( $format eq 'MARCXML' ) { - ( $errors, $marcrecords ) = C4::ImportBatch::RecordsFromMARCXMLFile( $file, $encoding); + ( $errors, $marcrecords ) = C4::ImportBatch::RecordsFromMARCXMLFile( $fh, $encoding); } elsif( $format eq 'ISO2709' ) { - ( $errors, $marcrecords ) = C4::ImportBatch::RecordsFromISO2709File( $file, $record_type, $encoding ); + ( $errors, $marcrecords ) = C4::ImportBatch::RecordsFromISO2709File( $fh, $record_type, $encoding ); } else { # plugin based $errors = []; - $marcrecords = C4::ImportBatch::RecordsFromMarcPlugin( $file, $format, $encoding ); + $marcrecords = C4::ImportBatch::RecordsFromMarcPlugin( $fh, $format, $encoding ); } warn "$filename: " . ( join ',', @$errors ) if @$errors; # no need to exit if we have no records (or only errors) here diff --git a/tools/upload-cover-image.pl b/tools/upload-cover-image.pl index f55d8f128d..5ace0af3f8 100755 --- a/tools/upload-cover-image.pl +++ b/tools/upload-cover-image.pl @@ -39,6 +39,7 @@ resized, maintaining aspect ratio. use Modern::Perl; +use Archive::Zip qw(:ERROR_CODES); use File::Temp; use CGI qw ( -utf8 ); use GD; @@ -122,11 +123,12 @@ if ($fileID) { undef $srcimage; } else { - my $filename = $upload->full_path; + my $storage = Koha::Storage->get_instance($upload->storage); + my $fh = $storage->fh($upload->filepath, 'r'); my $dirname = File::Temp::tempdir( CLEANUP => 1 ); - qx/unzip $filename -d $dirname/; - my $exit_code = $?; - unless ( $exit_code == 0 ) { + my $zip = Archive::Zip->new(); + $zip->readFromFileHandle($fh); + unless (AZ_OK == $zip->extractTree(undef, $dirname)) { $error = 'UZIPFAIL'; } else { @@ -165,6 +167,7 @@ if ($fileID) { $error = 'DELERR'; } else { + my $filename; ( $biblionumber, $filename ) = split $delim, $line, 2; $biblionumber =~ s/[\"\r\n]//g; # remove offensive characters @@ -212,6 +215,7 @@ if ($fileID) { } } } + close $fh; } $template->param( diff --git a/tools/upload-file.pl b/tools/upload-file.pl index 125e7cb611..c8733c65f4 100755 --- a/tools/upload-file.pl +++ b/tools/upload-file.pl @@ -48,7 +48,7 @@ if( $auth_status ne 'ok' || !$allowed ) { exit 0; } -my $upload = Koha::Uploader->new( upload_pars($ENV{QUERY_STRING}) ); +my $upload = Koha::Uploader->new( { CGI->new($ENV{QUERY_STRING})->Vars } ); if( !$upload || !$upload->cgi || !$upload->count ) { # not one upload succeeded send_reply( 'failed', undef, $upload? $upload->err: undef ); @@ -68,17 +68,3 @@ sub send_reply { # response will be sent back as JSON errors => $error, }); } - -sub upload_pars { # this sub parses QUERY_STRING in order to build the - # parameter hash for Koha::Uploader - my ( $qstr ) = @_; - $qstr = Encode::decode_utf8( uri_unescape( $qstr ) ); - # category could include a utf8 character - my $rv = {}; - foreach my $p ( qw[public category temp] ) { - if( $qstr =~ /(^|&)$p=(\w+)(&|$)/ ) { - $rv->{$p} = $2; - } - } - return $rv; -} diff --git a/tools/upload.pl b/tools/upload.pl index 509e99bb00..57ad47c307 100755 --- a/tools/upload.pl +++ b/tools/upload.pl @@ -22,7 +22,9 @@ use CGI qw/-utf8/; use JSON; use C4::Auth qw( get_template_and_user ); +use C4::Context; use C4::Output qw( output_html_with_http_headers ); +use Koha::Storage; use Koha::UploadedFiles; use constant ERR_READING => 'UPLERR_FILE_NOT_READ'; @@ -46,11 +48,17 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( } ); +my @storages; +foreach my $config (@{ Koha::Storage->config }) { + my $storage = Koha::Storage->get_instance($config->{name}); + push @storages, $storage if $storage; +} + $template->param( index => $index, owner => $loggedinuser, plugin => $plugin, - uploadcategories => Koha::UploadedFiles->getCategories, + storages => \@storages, ); if ( $op eq 'new' ) { @@ -82,15 +90,15 @@ if ( $op eq 'new' ) { my @id = split /,/, $id; foreach my $recid (@id) { my $rec = Koha::UploadedFiles->find( $recid ); - push @$uploads, $rec->unblessed + push @$uploads, $rec if $rec && ( $rec->public || !$plugin ); # Do not show private uploads in the plugin mode (:editor) } } else { - $uploads = Koha::UploadedFiles->search_term({ + $uploads = [ Koha::UploadedFiles->search_term({ term => $term, $plugin? (): ( include_private => 1 ), - })->unblessed; + }) ]; } $template->param( -- 2.20.1