Bug 20727

Summary: Replace usage of File::Spec->tmpdir with C4::Context->temporary_directory
Product: Koha Reporter: Kyle M Hall <kyle>
Component: Architecture, internals, and plumbingAssignee: Kyle M Hall <kyle>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: dcook, jonathan.druart, m.de.rooy, martin.renvoize, mirko, nick
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 20428    
Bug Blocks:    
Attachments: Bug 20727: Replace usage of File::Spec->tmpdir with Koha::UploadedFile->temporary_directory
Bug 20727: Move temporary_directory() to C4::Context
Bug 20727: Replace usage of File::Spec->tmpdir with Koha::UploadedFile->temporary_directory
Bug 20727: Move temporary_directory() to C4::Context
Bug 20727: (QA follow-up) Fix Upload.t and Plugins.t
Bug 20727: (QA follow-up) Remove a few use statements again
Bug 20727: Replace use of tmpdir in updatedatabase.pl
Bug 20727: Add use line for File::Spec in C4::Context, sort used modules
Bug 20727: Add use line for File::Spec in C4::Context, sort used modules
Bug 20727: Replace usage of File::Spec->tmpdir with Koha::UploadedFile->temporary_directory
Bug 20727: Move temporary_directory() to C4::Context
Bug 20727: (QA follow-up) Fix Upload.t and Plugins.t
Bug 20727: (QA follow-up) Remove a few use statements again
Bug 20727: Replace use of tmpdir in updatedatabase.pl
Bug 20727: Add use line for File::Spec in C4::Context, sort used modules

Description Kyle M Hall 2018-05-08 14:51:37 UTC
As explained in bug 20428 use tmpdir can cause issues and it just makes sense to standardize our temp directory in a universal way.
Comment 1 Kyle M Hall 2018-05-09 10:48:25 UTC
Created attachment 75200 [details] [review]
Bug 20727: Replace usage of File::Spec->tmpdir with Koha::UploadedFile->temporary_directory

As explained in bug 20428 use tmpdir can cause issues and it just makes sense to standardize our temp directory in a universal way.

Test Plan:
1) Apply this patch
2) Verify you can still log in and use Koha
3) Verify the web installer still works
4) Verify EDI module can still download files via FTP
5) Verify fines.pl still runs with -o option
6) prove t/db_dependent/Plugins.t
7) prove t/db_dependent/Sitemapper.t
8) prove t/db_dependent/Templates.t
Comment 2 Marcel de Rooy 2018-05-09 13:46:04 UTC
+my $dir = Koha::UploadedFile->temporary_directory;

I think it should be moved to some more general module?
Comment 3 Kyle M Hall 2018-05-09 14:03:40 UTC
I had thought about that as well. Where would you want to move it? Koha.pm?

(In reply to Marcel de Rooy from comment #2)
> +my $dir = Koha::UploadedFile->temporary_directory;
> 
> I think it should be moved to some more general module?
Comment 4 Marcel de Rooy 2018-05-11 06:16:20 UTC
(In reply to Kyle M Hall from comment #3)
> I had thought about that as well. Where would you want to move it? Koha.pm?

Yeah. Definitely better.. Note that I commented too on 20428 that imo we need a general koha-conf variable for an alternative tmp directory..
Comment 5 Kyle M Hall 2018-05-14 14:02:35 UTC
(In reply to Marcel de Rooy from comment #4)
> (In reply to Kyle M Hall from comment #3)
> > I had thought about that as well. Where would you want to move it? Koha.pm?
> 
> Yeah. Definitely better.. Note that I commented too on 20428 that imo we
> need a general koha-conf variable for an alternative tmp directory..

I'll update this patch after bug 20428 has been pushed.
Comment 6 Mirko Tietgen 2018-05-28 14:41:08 UTC
(In reply to Kyle M Hall from comment #5)
> (In reply to Marcel de Rooy from comment #4)
> > (In reply to Kyle M Hall from comment #3)
> > > I had thought about that as well. Where would you want to move it? Koha.pm?
> > 
> > Yeah. Definitely better.. Note that I commented too on 20428 that imo we
> > need a general koha-conf variable for an alternative tmp directory..
> 
> I'll update this patch after bug 20428 has been pushed.

Ping, 20428 has been pushed :)
Comment 7 Kyle M Hall 2018-05-29 12:04:17 UTC
Created attachment 75626 [details] [review]
Bug 20727: Move temporary_directory() to C4::Context
Comment 8 Marcel de Rooy 2018-05-30 14:33:22 UTC
Looking here now
Comment 9 Marcel de Rooy 2018-05-30 14:59:12 UTC
Created attachment 75667 [details] [review]
Bug 20727: Replace usage of File::Spec->tmpdir with Koha::UploadedFile->temporary_directory

As explained in bug 20428 use tmpdir can cause issues and it just makes sense to standardize our temp directory in a universal way.

Test Plan:
1) Apply this patch
2) Verify you can still log in and use Koha
3) Verify the web installer still works
4) Verify EDI module can still download files via FTP
5) Verify fines.pl still runs with -o option
6) prove t/db_dependent/Plugins.t
7) prove t/db_dependent/Sitemapper.t
8) prove t/db_dependent/Templates.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Marcel de Rooy 2018-05-30 14:59:17 UTC
Created attachment 75668 [details] [review]
Bug 20727: Move temporary_directory() to C4::Context

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Marcel de Rooy 2018-05-30 14:59:21 UTC
Created attachment 75669 [details] [review]
Bug 20727: (QA follow-up) Fix Upload.t and Plugins.t

Trivial changes.
Adding C4::Context->temporary_directory to subsequent tmpdir call in Plugins.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Marcel de Rooy 2018-05-30 14:59:25 UTC
Created attachment 75670 [details] [review]
Bug 20727: (QA follow-up) Remove a few use statements again

We do no longer need "use Koha::UploadedFile" in a few places.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 13 Jonathan Druart 2018-06-05 16:03:09 UTC
Do not we need to update the occurrence in updatedatabase.pl as well?

installer/data/mysql/updatedatabase.pl:my $tmp= File::Spec->tmpdir.'/koha_upload';
Comment 14 Kyle M Hall 2018-06-05 18:49:19 UTC
Created attachment 75833 [details] [review]
Bug 20727: Replace use of tmpdir in updatedatabase.pl
Comment 15 Kyle M Hall 2018-06-05 18:49:26 UTC
Created attachment 75834 [details] [review]
Bug 20727: Add use line for File::Spec in C4::Context, sort used modules
Comment 16 Kyle M Hall 2018-06-05 18:50:34 UTC
Created attachment 75835 [details] [review]
Bug 20727: Add use line for File::Spec in C4::Context, sort used modules
Comment 17 Jonathan Druart 2018-06-08 15:36:33 UTC
Created attachment 75923 [details] [review]
Bug 20727: Replace usage of File::Spec->tmpdir with Koha::UploadedFile->temporary_directory

As explained in bug 20428 use tmpdir can cause issues and it just makes sense to standardize our temp directory in a universal way.

Test Plan:
1) Apply this patch
2) Verify you can still log in and use Koha
3) Verify the web installer still works
4) Verify EDI module can still download files via FTP
5) Verify fines.pl still runs with -o option
6) prove t/db_dependent/Plugins.t
7) prove t/db_dependent/Sitemapper.t
8) prove t/db_dependent/Templates.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 18 Jonathan Druart 2018-06-08 15:36:38 UTC
Created attachment 75924 [details] [review]
Bug 20727: Move temporary_directory() to C4::Context

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 19 Jonathan Druart 2018-06-08 15:36:43 UTC
Created attachment 75925 [details] [review]
Bug 20727: (QA follow-up) Fix Upload.t and Plugins.t

Trivial changes.
Adding C4::Context->temporary_directory to subsequent tmpdir call in Plugins.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 20 Jonathan Druart 2018-06-08 15:36:48 UTC
Created attachment 75926 [details] [review]
Bug 20727: (QA follow-up) Remove a few use statements again

We do no longer need "use Koha::UploadedFile" in a few places.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 21 Jonathan Druart 2018-06-08 15:36:54 UTC
Created attachment 75927 [details] [review]
Bug 20727: Replace use of tmpdir in updatedatabase.pl

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 22 Jonathan Druart 2018-06-08 15:36:58 UTC
Created attachment 75928 [details] [review]
Bug 20727: Add use line for File::Spec in C4::Context, sort used modules

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 23 Nick Clemens 2018-06-22 18:12:15 UTC
Awesome work all!

Pushed to master for 18.11.x
Comment 24 Martin Renvoize 2018-06-27 10:04:54 UTC
Architectural enhancement, not backporting for 18.05.x series.
Comment 25 David Cook 2018-10-17 23:58:10 UTC Comment hidden (obsolete)
Comment 26 David Cook 2018-10-18 00:01:06 UTC Comment hidden (obsolete)
Comment 27 David Cook 2018-10-18 00:05:27 UTC Comment hidden (obsolete)
Comment 28 David Cook 2018-10-18 00:16:17 UTC Comment hidden (obsolete)
Comment 29 David Cook 2018-10-18 00:18:25 UTC Comment hidden (obsolete)
Comment 30 David Cook 2018-10-18 00:48:08 UTC Comment hidden (obsolete)