Bug 20254 - Forbid the download of duplicate EDI messages
Summary: Forbid the download of duplicate EDI messages
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Colin Campbell
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-02-20 14:22 UTC by Colin Campbell
Modified: 2020-06-04 20:32 UTC (History)
2 users (show)

See Also:
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 (1.22 KB, patch)
2018-04-11 15:42 UTC, Colin Campbell
Details | Diff | Splinter Review
Patch forbidding ingest of duplicate filename (1.41 KB, patch)
2018-12-10 13:05 UTC, Colin Campbell
Details | Diff | Splinter Review
Patch working_directory method (942 bytes, patch)
2018-12-10 13:08 UTC, Colin Campbell
Details | Diff | Splinter Review
Add tests for patch 20254 (2.02 KB, patch)
2018-12-10 13:12 UTC, Colin Campbell
Details | Diff | Splinter Review
Bug 20254: Do not ingest files with duplicate filenames (1.45 KB, patch)
2019-07-08 18:15 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 20254: Set correct member in working_directory (990 bytes, patch)
2019-07-08 18:15 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 20254: Add tests for transport ingest (2.07 KB, patch)
2019-07-08 18:15 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 20254: Do not ingest files with duplicate filenames (1.52 KB, patch)
2019-08-19 18:42 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20254: Set correct member in working_directory (1.04 KB, patch)
2019-08-19 18:42 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20254: Add tests for transport ingest (2.14 KB, patch)
2019-08-19 18:42 UTC, Kyle M Hall
Details | Diff | Splinter Review

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