Bug 17501: Additional polishing (POD, unit tests)
authorMarcel de Rooy <m.de.rooy@rijksmuseum.nl>
Thu, 24 Nov 2016 13:13:22 +0000 (14:13 +0100)
committerKyle M Hall <kyle@bywatersolutions.com>
Fri, 20 Jan 2017 14:20:06 +0000 (14:20 +0000)
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 <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Koha/UploadedFile.pm
Koha/UploadedFiles.pm
t/db_dependent/Upload.t

index 085037d..de696f3 100644 (file)
@@ -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
 
index 7587dfa..e970fbc 100644 (file)
@@ -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
 
index 49ec239..6f60b22 100644 (file)
@@ -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;