Bug 19318 - Add ability to use custom file upload paths
Summary: Add ability to use custom file upload paths
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Julian Maurice
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
: 20208 (view as bug list)
Depends on:
Blocks:
 
Reported: 2017-09-14 15:29 UTC by Kyle M Hall
Modified: 2022-04-30 16:16 UTC (History)
7 users (show)

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


Attachments
Bug 19318: Allow multiple storage spaces for file uploads (68.07 KB, patch)
2018-01-29 09:08 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 19318: Allow multiple storage spaces for file uploads (70.13 KB, patch)
2018-09-13 10:24 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 19318: Allow multiple storage spaces for file uploads (70.13 KB, patch)
2019-05-02 10:47 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 19318: Allow multiple storage spaces for file uploads (69.73 KB, patch)
2021-03-17 16:35 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19318: Allow multiple storage spaces for file uploads (68.82 KB, patch)
2021-04-23 13:40 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19318: DO NOT PUSH - SCHEMA CHANGES (2.01 KB, patch)
2021-04-23 13:40 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19318: Allow multiple storage spaces for file uploads (69.24 KB, patch)
2021-11-12 13:39 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 19318: Normalize uploaded file name (1.95 KB, patch)
2021-11-12 13:39 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 19318: DO NOT PUSH - SCHEMA CHANGES (2.06 KB, patch)
2021-12-21 16:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19318: Allow multiple storage spaces for file uploads (69.29 KB, patch)
2021-12-21 16:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19318: Normalize uploaded file name (2.01 KB, patch)
2021-12-21 16:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19318: (follow-up) remove uploadcategory column from tests (1.30 KB, patch)
2021-12-21 16:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19318: DO NOT PUSH - SCHEMA CHANGES (2.06 KB, patch)
2022-04-25 12:59 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 19318: Allow multiple storage spaces for file uploads (69.22 KB, patch)
2022-04-25 12:59 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 19318: Normalize uploaded file name (2.01 KB, patch)
2022-04-25 12:59 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 19318: (follow-up) remove uploadcategory column from tests (1.30 KB, patch)
2022-04-25 12:59 UTC, Julian Maurice
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 2017-09-14 15:29:04 UTC
Some libraries don't like the hard coded way uploaded files are all put in the same directory with the hash prefixing the file.

We should allow uploaded files to retain their original name, and optionally specify a path to upload to

I'm imagining we could accomplish this with something along these lines:
1) Add option on file upload page to not add file checksum to the uploaded filename, modify Koha::Upload
2) Modify Koha::Upload to support this option
3) Create new Authorised Value UPLOAD_PATH to define a set of upload paths
4) Add new 'upload path' option to file upload form as a pulldown of defined upload paths
5) Add new column 'file_path' to uploaded_files table
6) Modify Koha::Upload to support the automatic creation and use of custom file upload paths
Comment 1 Julian Maurice 2018-01-25 09:28:24 UTC
Hi Kyle,

Do you know if someone is working on that ? Because I will be working on something similar, that is a way to choose between several "storage spaces", eventually a subdirectory in that storage space, and other options like keeping the original name and using another URL to access those files.

But I think that the configuration of those storage spaces should not happen in authorised values, but on the server ($KOHA_CONF probably)
Comment 2 Julian Maurice 2018-01-29 09:08:29 UTC Comment hidden (obsolete)
Comment 3 Mirko Tietgen 2018-06-07 06:12:00 UTC
I tried to test, but the patch needs a rebase.
Comment 4 Julian Maurice 2018-09-13 10:24:30 UTC Comment hidden (obsolete)
Comment 5 Julian Maurice 2018-09-13 10:27:18 UTC
Patch rebased on master
Comment 6 Kyle M Hall 2019-03-01 15:04:50 UTC
Pretty sure this is a duplicate of bug 20208 which is a bit fresher than mine. Closing as a duplicate on that assumption.

*** This bug has been marked as a duplicate of bug 20208 ***
Comment 7 Julian Maurice 2019-05-02 10:40:38 UTC
(In reply to Kyle M Hall from comment #6)
> Pretty sure this is a duplicate of bug 20208 which is a bit fresher than
> mine. Closing as a duplicate on that assumption.
> 
> *** This bug has been marked as a duplicate of bug 20208 ***

Allow me to reopen it. They are both duplicates, that is true, but both have patches that solve the problem in different ways. I think at least someone from the QA team must look at both solutions and pick one.
Comment 8 Julian Maurice 2019-05-02 10:47:28 UTC Comment hidden (obsolete)
Comment 9 Andrew Fuerste-Henry 2021-01-21 15:12:49 UTC
Doesn't apply, to start. But, after clearing conflicts we still saw a few issues:
- when opening the Upload plugin popup, we always get a "no results found" message to start, even though we haven't yet done a search
- we're always given the option of [root] as a directory
- setting hash filename to 0 in the config did not prevent the file name from being hashed in the url
Comment 10 Nick Clemens 2021-03-17 16:35:41 UTC Comment hidden (obsolete)
Comment 11 Marcel de Rooy 2021-03-19 07:31:28 UTC
Processing files before patches
Constant subroutine Koha::Storage::KOHA_UPLOAD redefined at /usr/share/perl/5.24/constant.pm line 171.


An error occurred : Inconsistent hierarchy during C3 merge of class 'Koha::UploadedFiles':
        current merge results [
                Koha::UploadedFiles,
        ]
        merging failed on 'Koha::Objects' at /app/qatools/koha-qa.pl line 105.
Comment 12 Nick Clemens 2021-04-23 13:40:30 UTC Comment hidden (obsolete)
Comment 13 Nick Clemens 2021-04-23 13:40:34 UTC Comment hidden (obsolete)
Comment 14 Julian Maurice 2021-11-12 13:39:22 UTC Comment hidden (obsolete)
Comment 15 Julian Maurice 2021-11-12 13:39:26 UTC Comment hidden (obsolete)
Comment 16 Nick Clemens 2021-12-21 16:11:10 UTC Comment hidden (obsolete)
Comment 17 Nick Clemens 2021-12-21 16:11:14 UTC Comment hidden (obsolete)
Comment 18 Nick Clemens 2021-12-21 16:11:18 UTC Comment hidden (obsolete)
Comment 19 Nick Clemens 2021-12-21 16:11:22 UTC Comment hidden (obsolete)
Comment 20 Tomás Cohen Arazi 2022-01-07 18:35:08 UTC
Is there room for different storage backends here?
Comment 21 Nick Clemens 2022-01-14 14:24:47 UTC
(In reply to Tomás Cohen Arazi from comment #20)
> Is there room for different storage backends here?

Yes, this patch provides only 'Directory' but see Koha::Storage for documentation on defining in koha conf and provide another Adapter
Comment 22 Nick Clemens 2022-02-22 14:35:53 UTC
*** Bug 20208 has been marked as a duplicate of this bug. ***
Comment 23 Tomás Cohen Arazi 2022-04-25 12:09:16 UTC
Can I have a quick rebase please?
Comment 24 Julian Maurice 2022-04-25 12:59:13 UTC
Created attachment 133772 [details] [review]
Bug 19318: DO NOT PUSH - SCHEMA CHANGES

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 25 Julian Maurice 2022-04-25 12:59:20 UTC
Created attachment 133773 [details] [review]
Bug 19318: Allow multiple storage spaces for file uploads

This patch allow to configure multiple storage spaces to be used when
uploading a file.

Configuration is done in $KOHA_CONF. Element 'upload_path' is replaced
by 'storage', which is repeatable and can contain the following
elements:
- name: The storage's identifier (also used for display)
- adapter: The storage's adapter is a Perl module who is responsible for
  actually writing in the storage (creating/deleting files/directories).
  There is actually only one adapter, 'directory', which writes directly
  on the filesystem.
- adapter_params: Additional parameters required for the storage's
  adapter
- hash_filename: Whether to prepend hashvalue to the filename or not
- temporary: Whether the storage is temporary or not
- baseurl: Base URL of the storage, if one doesn't want to (or can't)
  use opac-retrieve-file.pl
This is all documented in Koha::Storage

There is two built-in storages:
- 'TMP', the default temporary storage (not configurable)
- 'DEFAULT', the default persistent storage (configurable in $KOHA_CONF)

Note that if $KOHA_CONF is not updated, uploads should continues to work
(the 'DEFAULT' storage will use 'upload_path', if set)

This patch affects the following pages:
- Tools › Upload
- Tools › Upload local cover image (ZIP file upload)
- Tools › Stage MARC records for import
- Tools › Upload patron images
- Circulation › Offline circulation file upload
- Cataloguing plugin upload.pl

Test plan:
0. Before applying the patch, be sure to use the pages mentioned above
   and upload some files, those files should continue to be accessible
   after the patch
1. Apply the patch, run updatedatabase and update_dbix_class_files
2. Without modifying $KOHA_CONF, verify that files uploaded in step 0
   are still accessible (you can find them in tools/upload.pl)
3. Verify that you can still upload files using the pages mentioned
   above
4. Edit your $KOHA_CONF, copy the <storage> element from
   etc/koha-conf.xml in the source, set the <path> using the value of
   <upload_path> and remove <upload_path>
5. Verify that you can still upload files
6. Add another storage in $KOHA_CONF, set <hash_filename> to 0,
   and a <baseurl>. Now try to use the cataloguing plugin and see how
   the generated URL change. Also note that the files keep their
   original filename (without the hashvalue prepended)
7. Create some subdirectories under the main storage's path (using a
   shell). Try to upload a new files and see that these directories
   appear in a dropdown list. Select one, finish the upload and verify
   that the file has been uploaded to the selected subdirectory.
8. Run prove t/ImportBatch.t t/Koha/Storage.t
   t/db_dependent/ImportBatch.t t/db_dependent/Upload.t

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 26 Julian Maurice 2022-04-25 12:59:26 UTC
Created attachment 133774 [details] [review]
Bug 19318: Normalize uploaded file name

The uploaded file name can appear in the MARC record (if using the
upload plugin), and all the strings in the MARC record are normalized
using Unicode::Normalize::NFC.
So the file name must be normalized everywhere (in the filesystem and in
the uploaded_files table too).

Test plan:
1. Create a file whose name contains combining characters. For instance:
   echo 'contents' > $(perl -e 'print "cine\x{cc}\x{81}ma"')

   0xcc 0x81 is utf8 for "Combining acute accent" (́ )
   https://unicode-table.com/fr/0301/
   Once normalized it should be 0xc3 0xa9 (é)

2. Use the upload tool to upload this file
3. Verify on your filesystem that the filename is normalized
   % ls cin* | xxd
   00000000: 6369 6ec3 a96d 610a                      cin..ma.
                    ^^ ^^
4. If you have configured the storage with a `baseurl`, use the upload
   cataloguing plugin, and verify that the generated URL works correctly

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 27 Julian Maurice 2022-04-25 12:59:31 UTC
Created attachment 133775 [details] [review]
Bug 19318: (follow-up) remove uploadcategory column from tests

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 28 Julian Maurice 2022-04-25 13:00:24 UTC
(In reply to Tomás Cohen Arazi from comment #23)
> Can I have a quick rebase please?

Here you go ;)