Bug 20727 - Replace usage of File::Spec->tmpdir with C4::Context->temporary_directory
Summary: Replace usage of File::Spec->tmpdir with C4::Context->temporary_directory
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
Depends on: 20428
Blocks:
  Show dependency treegraph
 
Reported: 2018-05-08 14:51 UTC by Kyle M Hall
Modified: 2019-10-14 19:58 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 20727: Replace usage of File::Spec->tmpdir with Koha::UploadedFile->temporary_directory (7.08 KB, patch)
2018-05-09 10:48 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20727: Move temporary_directory() to C4::Context (7.54 KB, patch)
2018-05-29 12:04 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20727: Replace usage of File::Spec->tmpdir with Koha::UploadedFile->temporary_directory (7.11 KB, patch)
2018-05-30 14:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 20727: Move temporary_directory() to C4::Context (7.55 KB, patch)
2018-05-30 14:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 20727: (QA follow-up) Fix Upload.t and Plugins.t (2.82 KB, patch)
2018-05-30 14:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 20727: (QA follow-up) Remove a few use statements again (2.49 KB, patch)
2018-05-30 14:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 20727: Replace use of tmpdir in updatedatabase.pl (1.14 KB, patch)
2018-06-05 18:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20727: Add use line for File::Spec in C4::Context, sort used modules (843 bytes, patch)
2018-06-05 18:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20727: Add use line for File::Spec in C4::Context, sort used modules (1.00 KB, patch)
2018-06-05 18:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20727: Replace usage of File::Spec->tmpdir with Koha::UploadedFile->temporary_directory (7.20 KB, patch)
2018-06-08 15:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20727: Move temporary_directory() to C4::Context (7.66 KB, patch)
2018-06-08 15:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20727: (QA follow-up) Fix Upload.t and Plugins.t (2.87 KB, patch)
2018-06-08 15:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20727: (QA follow-up) Remove a few use statements again (2.57 KB, patch)
2018-06-08 15:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20727: Replace use of tmpdir in updatedatabase.pl (1.20 KB, patch)
2018-06-08 15:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20727: Add use line for File::Spec in C4::Context, sort used modules (1.06 KB, patch)
2018-06-08 15:36 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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)