From 444ef3a020d99b58c324c5d53653af10050d2c82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nazl=C4=B1=20=C3=87etin?= Date: Wed, 30 Jan 2019 13:41:02 +0000 Subject: [PATCH] Bug 20208: Unit tests for checksum and dir --- Koha/Schema/Result/UploadedFile.pm | 16 ++- Koha/UploadedFile.pm | 5 +- Koha/Uploader.pm | 13 ++- installer/data/mysql/atomicupdate/bug_20208.perl | 14 +++ t/db_dependent/Upload.t | 120 ++++++++++++++++++++++- 5 files changed, 158 insertions(+), 10 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_20208.perl diff --git a/Koha/Schema/Result/UploadedFile.pm b/Koha/Schema/Result/UploadedFile.pm index 5a4397db69..8a64954c9a 100644 --- a/Koha/Schema/Result/UploadedFile.pm +++ b/Koha/Schema/Result/UploadedFile.pm @@ -35,6 +35,12 @@ __PACKAGE__->table("uploaded_files"); is_nullable: 0 size: 40 +=head2 checksum + + data_type: 'tinyint' + default_value: 1 + is_nullable: 1 + =head2 filename data_type: 'mediumtext' @@ -43,7 +49,7 @@ __PACKAGE__->table("uploaded_files"); =head2 dir data_type: 'mediumtext' - is_nullable: 0 + is_nullable: 1 =head2 filesize @@ -84,10 +90,12 @@ __PACKAGE__->add_columns( { data_type => "integer", is_auto_increment => 1, is_nullable => 0 }, "hashvalue", { data_type => "char", is_nullable => 0, size => 40 }, + "checksum", + { data_type => "tinyint", default_value => 1, is_nullable => 1 }, "filename", { data_type => "mediumtext", is_nullable => 0 }, "dir", - { data_type => "mediumtext", is_nullable => 0 }, + { data_type => "mediumtext", is_nullable => 1 }, "filesize", { data_type => "integer", is_nullable => 1 }, "dtcreated", @@ -120,8 +128,8 @@ __PACKAGE__->add_columns( __PACKAGE__->set_primary_key("id"); -# Created by DBIx::Class::Schema::Loader v0.07042 @ 2018-02-16 17:54:54 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:kJUbIULQMBo3t51HvWC8cg +# Created by DBIx::Class::Schema::Loader v0.07046 @ 2019-01-30 13:21:07 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:4f2CTtOjsy/t/1/u9Am1hQ # You can replace this text with custom code or comments, and it will be preserved on regeneration diff --git a/Koha/UploadedFile.pm b/Koha/UploadedFile.pm index 20fc608c68..8fcaf08722 100644 --- a/Koha/UploadedFile.pm +++ b/Koha/UploadedFile.pm @@ -101,12 +101,13 @@ Returns the fully qualified path name for an uploaded file. sub full_path { my ( $self ) = @_; + my $filename = $self->checksum ? $self->hashvalue. '_'. $self->filename: $self->filename; my $path = File::Spec->catfile( $self->permanent ? $self->permanent_directory : C4::Context->temporary_directory, - $self->dir, - $self->hashvalue. '_'. $self->filename, + $self->dir .'/'. $self->uploadcategorycode, + $filename, ); return $path; } diff --git a/Koha/Uploader.pm b/Koha/Uploader.pm index a4baa6a30b..fdbeb2fd0a 100644 --- a/Koha/Uploader.pm +++ b/Koha/Uploader.pm @@ -69,6 +69,7 @@ use Modern::Perl; use CGI; # no utf8 flag, since it may interfere with binary uploads use Digest::MD5; use Encode; +use File::Path; use File::Spec; use IO::File; use Time::HiRes; @@ -197,6 +198,8 @@ sub _init { $params->{tmp} = $params->{temp} if !exists $params->{tmp}; $self->{temporary} = $params->{tmp}? 1: 0; #default false + $params->{checksum} = 1 if !exists $params->{checksum}; + $self->{checksum} = $params->{checksum} ; if( $params->{tmp} ) { my $db = C4::Context->config('database'); $self->{category} = KOHA_UPLOAD; @@ -205,6 +208,7 @@ sub _init { $self->{category} = $params->{category} || KOHA_UPLOAD; } + $self->{dir} = $params->{dir} || ""; $self->{files} = {}; $self->{uid} = C4::Context->userenv->{number} if C4::Context->userenv; $self->{public} = $params->{public}? 1: undef; @@ -230,7 +234,7 @@ sub _create_file { } else { my $dir = $self->_dir; my $hashval = $self->{files}->{$filename}->{hash}; - my $fn = $hashval. '_'. $filename; + my $fn = $self->{checksum} ? $hashval. '_'. $filename: $filename; # if the file exists and it is registered, then set error # if it exists, but is not in the database, we will overwrite @@ -238,6 +242,7 @@ sub _create_file { Koha::UploadedFiles->search({ hashvalue => $hashval, uploadcategorycode => $self->{category}, + dir => $self->{dir}, })->count ) { $self->{files}->{$filename}->{errcode} = ERR_EXISTS; return; @@ -257,8 +262,9 @@ sub _create_file { sub _dir { my ( $self ) = @_; my $dir = $self->{temporary}? $self->{tmpdir}: $self->{rootdir}; + $dir = $self->{rootdir}.'/'. $self->{dir} if( $self->{dir} ); $dir.= '/'. $self->{category}; - mkdir $dir if !-d $dir; + mkpath $dir if !-d $dir; return $dir; } @@ -285,8 +291,9 @@ sub _register { my ( $self, $filename, $size ) = @_; my $rec = Koha::UploadedFile->new({ hashvalue => $self->{files}->{$filename}->{hash}, + checksum => $self->{checksum}, filename => $filename, - dir => $self->{category}, + dir => $self->{dir}, filesize => $size, owner => $self->{uid}, uploadcategorycode => $self->{category}, diff --git a/installer/data/mysql/atomicupdate/bug_20208.perl b/installer/data/mysql/atomicupdate/bug_20208.perl new file mode 100644 index 0000000000..00e750ef41 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_20208.perl @@ -0,0 +1,14 @@ +$DBversion = '18.12.00.XXX'; # will be replaced by the RM +if( CheckVersion( $DBversion ) ) { + unless ( column_exists('uploaded_files', 'checksum') ) { + $dbh->do( q{ + ALTER TABLE uploaded_files ADD checksum tinyint(1) DEFAULT 1 AFTER hashvalue + }); + $dbh->do( q{ + UPDATE uploaded_files SET checksum=1 + }); + $dbh->do("ALTER TABLE `uploaded_files` MODIFY COLUMN `dir` mediumtext DEFAULT NULL"); + } + SetVersion( $DBversion ); + print "Upgrade to $DBversion done (Bug 20208 - Development - Peddie School - Custom file upload paths)\n"; +} diff --git a/t/db_dependent/Upload.t b/t/db_dependent/Upload.t index 802dc58da0..3fa852f796 100644 --- 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 => 18; use Test::Warn; use Test::MockModule; @@ -46,6 +46,23 @@ our $uploads = [ { name => 'file6', cat => undef, size => 6500 }, { name => 'file7', cat => undef, size => 6501 }, ], + #cheksum tests + [ + { name => 'file8', cat => 'A', checksum => 0, size => 2000 }, + ], + [ + { name => 'file9', cat => undef, checksum => 0, size => 5000 }, # temp without checksum + ], + [ + { name => 'file3', cat => 'B', checksum => 0, size => 1000 }, #duplicate file without checksum + ], + #path tests + [ + { name => 'file10', dir => 'A/B', cat => 'A', size => 2000 }, + ], + [ + { name => 'file11', dir => 'A/B', cat => undef, size => 3000 }, + ], ]; # Redirect upload dir structure and mock C4::Context and CGI @@ -327,6 +344,107 @@ subtest 'Testing download headers' => sub { is_deeply(\@pdf_head, \@pdf_expect,"Get inline pdf headers for pdf"); is_deeply(\@not_head, \@not_expect,"Get download headers for non pdf"); }; + +subtest 'Not add file checksum to the uploaded filename' => sub { + plan tests => 7; + + my $upl = Koha::Uploader->new({ + category => $uploads->[$current_upload]->[0]->{cat}, + checksum => 0, + }); #add file8 + my $cgi = $upl->cgi; + my $res = $upl->result; + is( $upl->err, undef, 'No errors reported' ); + + my $rec = Koha::UploadedFiles->find( $res ); + is( $rec->filename, 'file8', 'Check file name' ); + is( $rec->checksum, 0, 'Checksum false' ); + my $path = $rec->full_path; + is( -e $path, 1, 'File is found' ); + my $fh = $rec->file_handle; + is( ref($fh) eq 'IO::File' && $fh->opened, 1, 'Get returns a file handle' ); + my $delete = $rec->delete; + ok( $delete=~/^-?1$/, 'Delete successful' ); + isnt( -e $path, 1, 'File no longer found after delete' ); +}; + +subtest 'Temporary upload without checksum' => sub { + plan tests => 5; + + my $upl = Koha::Uploader->new({ + tmp => 1, + checksum => 0, + }); #add file9 + my $cgi = $upl->cgi; + is( $upl->count, 1, 'Upload 9 includes one temporary file' ); + is( $upl->err, undef, 'No errors reported' ); + + my $rec = Koha::UploadedFiles->find( $upl->result ); + is( $rec->checksum, 0, 'Checksum false' ); + is( $rec->uploadcategorycode =~ /_upload$/, 1, 'Check category temp file' ); + my $path = $rec->full_path; + is( -e $path, 1, 'File is found' ); +}; + +subtest 'Duplicate file without checksum' => sub { + plan tests => 5; + + my $upl = Koha::Uploader->new({ + category => $uploads->[$current_upload]->[0]->{cat}, + checksum => 0, + }); + my $cgi = $upl->cgi; + is( $upl->count, 1, 'Upload 10 successful' ); + is( $upl->err, undef, 'No errors reported' ); + + my $rec = Koha::UploadedFiles->find( $upl->result ); + is( $rec->filename, 'file3', 'Check file name' ); + is( $rec->checksum, 0, 'Checksum false' ); + my $e = $upl->err; + isnt( $e->{file2}->{code}, Koha::Uploader::ERR_EXISTS, "Already exists error reported" ); +}; + +subtest 'Add upload in dir' => sub { + plan tests => 8; + + my $upl = Koha::Uploader->new({ + category => $uploads->[$current_upload]->[0]->{cat}, + dir => $uploads->[$current_upload]->[0]->{dir}, + }); + my $cgi = $upl->cgi; + is( $upl->count, 1, 'Upload 10 successful' ); + isnt( $upl->result, undef, 'Result is undefined' ); + is( $upl->err, undef, 'No errors reported' ); + + my $rec = Koha::UploadedFiles->find( $upl->result ); + is( $rec->dir, 'A/B', 'Check dir A/B' ); + my $path = $rec->full_path; + is( -e $path, 1, 'File is found' ); + my $fh = $rec->file_handle; + is( ref($fh) eq 'IO::File' && $fh->opened, 1, 'Get returns a file handle' ); + my $delete = $rec->delete; + ok( $delete=~/^-?1$/, 'Delete successful' ); + isnt( -e $path, 1, 'File no longer found after delete' ); + +}; + + +subtest 'Temporary upload in dir' => sub { + plan tests => 4; + + my $upl = Koha::Uploader->new({ + tmp => 1, + dir => $uploads->[$current_upload]->[0]->{dir}, + }); #add file9 + my $cgi = $upl->cgi; + is( $upl->err, undef, 'No errors reported' ); + + my $rec = Koha::UploadedFiles->find( $upl->result ); + is( $rec->dir, 'A/B', 'Check dir' ); + is( $rec->uploadcategorycode =~ /_upload$/, 1, 'Check category temp file' ); + my $path = $rec->full_path; + is( -e $path, 1, 'File is found' ); +}; # The end $schema->storage->txn_rollback; -- 2.11.0