From 6ea0cbca858f6e92548523cd46ac5927bc370826 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Thu, 24 Nov 2016 14:13:22 +0100 Subject: [PATCH] Bug 17501: Additional polishing (POD, unit tests) This patch adds some documentation lines. And mainly rearrangs the tests in Upload.t. The 'basic CRUD testing' is not needed separately any more. A new test catches the "file missing" warn. Test plan: [1] Run perldoc on UploadedFile[s].pm [2] Run t/db_dependent/Upload.t Signed-off-by: Marcel de Rooy Signed-off-by: Tomas Cohen Arazi Signed-off-by: Jonathan Druart --- Koha/UploadedFile.pm | 34 ++++++++++--- Koha/UploadedFiles.pm | 17 +++++-- t/db_dependent/Upload.t | 129 ++++++++++++++++++++++-------------------------- 3 files changed, 100 insertions(+), 80 deletions(-) diff --git a/Koha/UploadedFile.pm b/Koha/UploadedFile.pm index 085037d..de696f3 100644 --- a/Koha/UploadedFile.pm +++ b/Koha/UploadedFile.pm @@ -20,8 +20,6 @@ package Koha::UploadedFile; use Modern::Perl; use File::Spec; -#use Koha::Database; - use parent qw(Koha::Object); =head1 NAME @@ -30,11 +28,28 @@ Koha::UploadedFile - Koha::Object class for single uploaded file =head1 SYNOPSIS -use Koha::UploadedFile; + use Koha::UploadedFile; + + # store record in uploaded_files + my $upload = Koha::UploadedFile->new({ [columns and values] }); + + # 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; =head1 DESCRIPTION -Description +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. + +Additional methods include: file_handle, httpheaders. =head1 METHODS @@ -103,8 +118,9 @@ sub file_handle { =head3 httpheaders - httpheaders returns http headers for a retrievable upload - Will be extended by report 14282 +httpheaders returns http headers for a retrievable upload. + +Will be extended by report 14282 =cut @@ -120,6 +136,8 @@ sub httpheaders { =head3 permanent_directory +Returns root directory for permanent storage + =cut sub permanent_directory { @@ -129,6 +147,8 @@ sub permanent_directory { =head3 tmp_directory +Returns root directory for temporary storage + =cut sub temporary_directory { @@ -138,7 +158,7 @@ sub temporary_directory { =head3 getCategories - getCategories returns a list of upload category codes and names +getCategories returns a list of upload category codes and names =cut diff --git a/Koha/UploadedFiles.pm b/Koha/UploadedFiles.pm index 7587dfa..e970fbc 100644 --- a/Koha/UploadedFiles.pm +++ b/Koha/UploadedFiles.pm @@ -19,7 +19,6 @@ package Koha::UploadedFiles; use Modern::Perl; -#use Koha::Database; use Koha::UploadedFile; use parent qw(Koha::Objects); @@ -30,11 +29,23 @@ Koha::UploadedFiles - Koha::Objects class for uploaded files =head1 SYNOPSIS -use Koha::UploadedFiles; + use Koha::UploadedFiles; + + # get one upload + my $upload01 = Koha::UploadedFiles->find( $id ); + + # get some uploads + my @uploads = Koha::UploadedFiles->search_term({ term => '.mrc' }); + + # delete all uploads + Koha::UploadedFiles->new->delete; =head1 DESCRIPTION -Description +Allows regular CRUD operations on uploaded_files via Koha::Objects / DBIx. + +The delete method also takes care of deleting files. The search_term method +provides a wrapper around search to look for a term in multiple columns. =head1 METHODS diff --git a/t/db_dependent/Upload.t b/t/db_dependent/Upload.t index 49ec239..6f60b22 100644 --- a/t/db_dependent/Upload.t +++ b/t/db_dependent/Upload.t @@ -2,7 +2,8 @@ use Modern::Perl; use File::Temp qw/ tempdir /; -use Test::More tests => 9; +use Test::More tests => 10; +use Test::Warn; use Test::MockModule; use t::lib::Mocks; @@ -16,6 +17,7 @@ use Koha::Uploader; my $schema = Koha::Database->new->schema; $schema->storage->txn_begin; +my $builder = t::lib::TestBuilder->new; our $current_upload = 0; our $uploads = [ @@ -50,50 +52,29 @@ my $cgimod = Test::MockModule->new( 'CGI' ); $cgimod->mock( 'new' => \&newCGI ); # Start testing -subtest 'Test01' => sub { - plan tests => 11; - test01(); -}; -subtest 'Test02' => sub { - plan tests => 5; - test02(); -}; -subtest 'Test03' => sub { - plan tests => 2; - test03(); -}; -subtest 'Test04' => sub { - plan tests => 3; - test04(); -}; -subtest 'Test05' => sub { - plan tests => 6; - test05(); -}; -subtest 'Test06' => sub { - plan tests => 3; - test06(); -}; -subtest 'Test07' => sub { - plan tests => 2; - test07(); -}; -subtest 'Test08: allows_add_by' => sub { - plan tests => 4; - test08(); -}; -$schema->storage->txn_rollback; +subtest 'Make a fresh start' => sub { + plan tests => 1; -sub test01 { # Delete existing records (for later tests) - # Passing keep_file suppresses warnings + # Passing keep_file suppresses warnings (and does not delete files) + # Note that your files are not in danger, since we redirected + # all files to a new empty temp folder Koha::UploadedFiles->new->delete({ keep_file => 1 }); + 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( Koha::UploadedFile->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}, @@ -115,9 +96,11 @@ sub test01 { is( $rec->filename, 'file2', 'Check file name 2' ); is( $rec->filesize, 8000, 'Check size of file2' ); is( $rec->public, undef, 'Check public undefined' ); -} +}; + +subtest 'Add another upload, check file_handle' => sub { + plan tests => 5; -sub test02 { my $upl = Koha::Uploader->new({ category => $uploads->[$current_upload]->[0]->{cat}, public => 1, @@ -135,17 +118,21 @@ sub test02 { $rec->filename( 'doesprobablynotexist' )->store; is( $rec->file_handle, undef, 'Sabotage with file handle' ); $rec->filename( $orgname )->store; -} +}; + +subtest 'Add temporary upload' => sub { + plan tests => 2; -sub test03 { my $upl = Koha::Uploader->new({ tmp => 1 }); #temporary 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' ); -} +}; + +subtest 'Add same file in same category' => sub { + plan tests => 3; -sub test04 { # Fail on a file already there my $upl = Koha::Uploader->new({ category => $uploads->[$current_upload]->[0]->{cat}, }); @@ -154,12 +141,15 @@ sub test04 { # Fail on a file already there is( $upl->result, undef, 'Result is undefined' ); my $e = $upl->err; is( $e->{file2}, 1, "Errcode 1 [already exists] reported" ); -} +}; + +subtest 'Test delete via UploadedFile as well as UploadedFiles' => sub { + plan tests => 8; -sub test05 { # add temporary file with same name and contents, delete it + # add temporary file with same name and contents (file4) my $upl = Koha::Uploader->new({ tmp => 1 }); my $cgi= $upl->cgi; - is( $upl->count, 1, 'Upload 5 adds duplicate temporary file' ); + is( $upl->count, 1, 'Add duplicate temporary file (file4)' ); my $id = $upl->result; my $path = Koha::UploadedFiles->find( $id )->full_path; @@ -179,9 +169,19 @@ sub test05 { # add temporary file with same name and contents, delete it $delete = $kohaobj->delete; is( $delete, $name, 'Delete successful' ); isnt( -e $path, 1, 'File no longer found after delete' ); -} -sub test06 { #search_term with[out] private flag + # add another record with TestBuilder, so file does not exist + # catch warning + my $upload01 = $builder->build({ source => 'UploadedFile' }); + warning_like { Koha::UploadedFiles->find( $upload01->{id} )->delete; } + qr/file was missing/, + 'delete warns when file is missing'; + is( Koha::UploadedFiles->count, 4, 'Back to four uploads now' ); +}; + +subtest 'Call search_term with[out] private flag' => sub { + plan tests => 3; + my @recs = Koha::UploadedFiles->search_term({ term => 'file' }); is( @recs, 1, 'Returns only one public result' ); is( $recs[0]->filename, 'file3', 'Should be file3' ); @@ -189,20 +189,22 @@ sub test06 { #search_term with[out] private flag is( Koha::UploadedFiles->search_term({ term => 'file', include_private => 1, })->count, 4, 'Returns now four results' ); -} +}; + +subtest 'Simple tests for httpheaders and getCategories' => sub { + plan tests => 2; -sub test07 { #simple test for httpheaders and getCategories 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'); - my $builder = t::lib::TestBuilder->new; $builder->build({ source => 'AuthorisedValue', value => { category => 'UPLOAD', authorised_value => 'HAVE_AT_LEAST_ONE', lib => 'Hi there' } }); my $cat = Koha::UploadedFile->getCategories; is( @$cat >= 1, 1, 'getCategories returned at least one category' ); -} +}; + +subtest 'Testing allows_add_by' => sub { + plan tests => 4; -sub test08 { # allows_add_by - my $builder = t::lib::TestBuilder->new; my $patron = $builder->build({ source => 'Borrower', value => { flags => 0 }, #no permissions @@ -236,25 +238,12 @@ sub test08 { # allows_add_by }); is( Koha::Uploader->allows_add_by( $patron->{userid} ), 1, 'Patron is still allowed to add uploaded files' ); -} - -# Additional tests for Koha::UploadedFiles -# TODO Rearrange the tests after this migration -subtest 'Some basic CRUD testing' => sub { - plan tests => 2; - - # Test find and attribute id, delete and search - my $builder = t::lib::TestBuilder->new; - my $upload01 = $builder->build({ source => 'UploadedFile' }); - my $found = Koha::UploadedFiles->find( $upload01->{id} ); - is( $found->id, $upload01->{id}, 'Koha::Object returns id' ); - $found->delete({ keep_file => 1 }); #note that it does not exist - $found = Koha::UploadedFiles->search( - { id => $upload01->{id} }, - ); - is( $found->count, 0, 'Delete seems successful' ); }; +# The end +$schema->storage->txn_rollback; + +# Helper routine sub newCGI { my ( $class, $hook ) = @_; my $read = 0; -- 2.1.4