https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com --- Comment #1 from Julian Maurice <julian.maurice@biblibre.com> --- 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) -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 --- Comment #2 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 71017 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=71017&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 Mirko Tietgen <mirko@abunchofthings.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply CC| |mirko@abunchofthings.net --- Comment #3 from Mirko Tietgen <mirko@abunchofthings.net> --- I tried to test, but the patch needs a rebase. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #71017|0 |1 is obsolete| | --- Comment #4 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 78620 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=78620&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |DUPLICATE Status|Needs Signoff |RESOLVED --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- 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 *** -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #78620|0 |1 is obsolete| | --- Comment #8 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 89219 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=89219&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=20208 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |andrew@bywatersolutions.com --- Comment #9 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- 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 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- 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. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 --- Comment #13 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 120102 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120102&action=edit Bug 19318: DO NOT PUSH - SCHEMA CHANGES -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 --- Comment #15 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 127585 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127585&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 --- Comment #17 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 128845 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128845&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 --- Comment #19 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 128847 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128847&action=edit Bug 19318: (follow-up) remove uploadcategory column from tests Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |tomascohen@gmail.com |y.org | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 --- Comment #22 from Nick Clemens <nick@bywatersolutions.com> --- *** Bug 20208 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128844|0 |1 is obsolete| | --- Comment #24 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 133772 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=133772&action=edit Bug 19318: DO NOT PUSH - SCHEMA CHANGES Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128846|0 |1 is obsolete| | --- Comment #26 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 133774 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=133774&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff --- Comment #28 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to Tomás Cohen Arazi from comment #23)

Can I have a quick rebase please?

Here you go ;) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Large patch -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 --- Comment #30 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to Martin Renvoize from comment #29)

why do we remove the category browser as part of this development? I thought it was not useful anymore, since the main purpose of the category was to indicate a sub-directory where to put uploaded files, and with this patch it's possible without it. Can you describe a use case that is possible today with the category that is not possible anymore with these patches ?

-- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 --- Comment #32 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Stepping out of this one, I'm not close enough to the feature set to properly review. Thanks for offering to take it on later this week Marcel :) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 --- Comment #34 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to Katrin Fischer from comment #33)

How do we transition from categories to the new way of handling things? We can assume that a lot of libraries are using the upload file feature.

There may be something missing here. I'd say we should prepend the category name to the file path or something like that (patches don't do that yet). But first we must decide if we need to keep upload category or not. -- You are receiving this mail because: You are watching all bug changes.

hash_filename: Whether to prepend hashvalue to the filename or not. I wonder if some of these features would be better suited as follow-ups to keep

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19318 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |In Discussion --- Comment #36 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I have decided to move this to 'In discussion' for now. My reasoning: The patch set does appear to do much more than is written on the 'tin': It claims to allow for custom upload paths, but it does a lot more than that. There are unresolved questions about some of the design decisions and how we can deal with the consequences: * While Kyle had originally proposed using an AV in the initial description, this patch set moves everything away from the GUI into the koha-conf.xml. In doing so, it removes the UPLOAD AV. If this is proven to be the right path, it would require more work to remove the AV category properly and ensure a clean upgrade path (see comment#34, comment#29). But we also should discuss if this is a required step as it takes away control from libraries that often don't have easy access to server side configuration. Personally this feels contrary to what I see in other work (like making a GUI for SMTP servers). * Adds other extra features, like: the initial bug smaller. (comment#35) My feeling is that these conceptual questions cannot be resolved in QA, but need to be resolved before it reaches the QA step with a clear test plan/upgrade path/reasoning of changes. -- You are receiving this mail because: You are watching all bug changes.