Bug 20254

Summary: Forbid the download of duplicate EDI messages
Product: Koha Reporter: Colin Campbell <colin.campbell>
Component: AcquisitionsAssignee: Colin Campbell <colin.campbell>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: fridolin.somers, mtompset
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.11.00
Attachments: Block ingest of duplicate filenames
Patch forbidding ingest of duplicate filename
Patch working_directory method
Add tests for patch 20254
Bug 20254: Do not ingest files with duplicate filenames
Bug 20254: Set correct member in working_directory
Bug 20254: Add tests for transport ingest
Bug 20254: Do not ingest files with duplicate filenames
Bug 20254: Set correct member in working_directory
Bug 20254: Add tests for transport ingest

Description Colin Campbell 2018-02-20 14:22:07 UTC
In theory EDI messages should be downloaded only once, as we rename it according to Edifact conventions as soon as downloaded. However the case has occured when after an issue with a vendor's server duplicates of the days files have been uploaded to the download directory. 
As a result duplicate quotes are downloaded, creating new orders on the system,
as Koha cannot now rename the file (it would entail overwriting a file on the vendor's system) this can continue infinitely.

So do not allow duplicate filenames to be downloaded, add a unique constraint to edifact_messages to support this
Comment 1 Colin Campbell 2018-04-11 15:42:17 UTC
Created attachment 74002 [details] [review]
Block ingest of duplicate filenames

Protect against ingest of duplicate filenames
Comment 2 Mark Tompsett 2018-05-01 02:32:57 UTC
While the code eyeballs well, is it possible to write a test to trigger this case? We have no tests which trigger ingest, as far as I can tell. And this is a Koha library. Failing QA for lack of test(s).
Comment 3 Colin Campbell 2018-12-10 13:05:50 UTC
Created attachment 83013 [details] [review]
Patch forbidding ingest of duplicate filename
Comment 4 Colin Campbell 2018-12-10 13:08:31 UTC
Created attachment 83014 [details] [review]
Patch working_directory method

Was an unused method. Needs an update to set the correct member as is now required for tests
Comment 5 Colin Campbell 2018-12-10 13:12:19 UTC
Created attachment 83015 [details] [review]
Add tests for patch 20254

Run tests, confirms that file is added to edifact_messages and that a second attempt to re-add the same filename is rejected
Comment 6 Colin Campbell 2018-12-10 13:14:42 UTC
Tests added to confirm patch does stop duplicate messages are loaded, protecting the system from damage resulting from multiple ingests of duplicate files
Comment 7 Mark Tompsett 2019-07-08 18:13:55 UTC
Comment on attachment 83014 [details] [review]
Patch working_directory method

Review of attachment 83014 [details] [review]:
-----------------------------------------------------------------

technically beyond scope of this bug, I think, but hey, valid fix.
Comment 8 Mark Tompsett 2019-07-08 18:15:48 UTC
Created attachment 91412 [details] [review]
Bug 20254: Do not ingest files with duplicate filenames

If the supplier delivers the same file twice we are
unable to rename it on the second occurence causing us to download
and process it infinitely.

Check that downloaded filenames are unique against those
on file and reject processing if a duplicate.

This patch delays this check to the ingest stage so that the
same code is processed irrespective of transport

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 9 Mark Tompsett 2019-07-08 18:15:51 UTC
Created attachment 91413 [details] [review]
Bug 20254: Set correct member in working_directory

Koha::Edifact::Transport working_directory was setting the
wrong member name correct that
method is only used in testing

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 10 Mark Tompsett 2019-07-08 18:15:53 UTC
Created attachment 91414 [details] [review]
Bug 20254: Add tests for transport ingest

Add test framework for Koha::Edifact::Transport
Add specific test that ingest does not duplicate
an existing filename

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 11 Kyle M Hall 2019-08-19 18:42:39 UTC
Created attachment 92353 [details] [review]
Bug 20254: Do not ingest files with duplicate filenames

If the supplier delivers the same file twice we are
unable to rename it on the second occurence causing us to download
and process it infinitely.

Check that downloaded filenames are unique against those
on file and reject processing if a duplicate.

This patch delays this check to the ingest stage so that the
same code is processed irrespective of transport

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 12 Kyle M Hall 2019-08-19 18:42:53 UTC
Created attachment 92354 [details] [review]
Bug 20254: Set correct member in working_directory

Koha::Edifact::Transport working_directory was setting the
wrong member name correct that
method is only used in testing

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 13 Kyle M Hall 2019-08-19 18:42:56 UTC
Created attachment 92355 [details] [review]
Bug 20254: Add tests for transport ingest

Add test framework for Koha::Edifact::Transport
Add specific test that ingest does not duplicate
an existing filename

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 14 Martin Renvoize 2019-08-20 14:04:56 UTC
Nice work!

Pushed to master for 19.11.00
Comment 15 Fridolin Somers 2019-09-06 09:32:26 UTC
Enhancement not pushed to 19.05.x