Bug 18300 - Delete missing upload records
Summary: Delete missing upload records
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: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on: 17669
Blocks:
  Show dependency treegraph
 
Reported: 2017-03-19 15:50 UTC by Marcel de Rooy
Modified: 2017-12-07 22:16 UTC (History)
3 users (show)

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


Attachments
Bug 18300: Delete missing upload records (6.80 KB, patch)
2017-03-20 08:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18300: Delete missing upload records (6.82 KB, patch)
2017-03-22 15:26 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 18300: Delete missing upload records (6.83 KB, patch)
2017-04-03 08:26 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18300: [QA Follow-up] Fix return value inconsistency (1.77 KB, patch)
2017-04-03 19:45 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18300: Delete missing upload records (6.83 KB, patch)
2017-04-04 13:11 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18300: [QA Follow-up] Fix return value inconsistency (3.77 KB, patch)
2017-04-04 13:11 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18300: Delete missing upload records (6.86 KB, patch)
2017-04-20 18:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18300: [QA Follow-up] Fix return value inconsistency (3.80 KB, patch)
2017-04-20 18:29 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 Marcel de Rooy 2017-03-19 15:50:00 UTC
On top of bug 17669 (deleting temporary files), we should be able to remove records too for missing upload files.
Comment 1 Marcel de Rooy 2017-03-20 08:39:56 UTC
Created attachment 61250 [details] [review]
Bug 18300: Delete missing upload records

If you deleted files from the upload directories manually, or you rebooted
with files in the temporary upload folder, or for some other reason have
records without a file, you may want to cleanup your records in the
uploaded_files table.

This patch adds the method delete_missing to Koha::UploadedFiles. It also
supports a keep_record parameter to do a dry run (count the missing files
first).

Also, we add an option --uploads-missing to cleanup_database. If you add
the flag 1 after this option, you will delete missing files. If you add the
flag 0 or only use the option, you will count missing files.

A subtest is added to Upload.t for delete_missing tests.

Test plan:
[1] Run t/db_dependent/Upload.t
[2] Upload a file and delete the file manually.
[3] Run cleanup_database.pl --uploads-missing
    It should report at least one missing file now.
    Check that the record has not been deleted.
[4] Run cleanup_database.pl --uploads-missing 1
    It should report that it removed at least one file.
    Check that the record is gone.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 Mirko Tietgen 2017-03-22 15:26:49 UTC
Created attachment 61470 [details] [review]
Bug 18300: Delete missing upload records

If you deleted files from the upload directories manually, or you rebooted
with files in the temporary upload folder, or for some other reason have
records without a file, you may want to cleanup your records in the
uploaded_files table.

This patch adds the method delete_missing to Koha::UploadedFiles. It also
supports a keep_record parameter to do a dry run (count the missing files
first).

Also, we add an option --uploads-missing to cleanup_database. If you add
the flag 1 after this option, you will delete missing files. If you add the
flag 0 or only use the option, you will count missing files.

A subtest is added to Upload.t for delete_missing tests.

Test plan:
[1] Run t/db_dependent/Upload.t
[2] Upload a file and delete the file manually.
[3] Run cleanup_database.pl --uploads-missing
    It should report at least one missing file now.
    Check that the record has not been deleted.
[4] Run cleanup_database.pl --uploads-missing 1
    It should report that it removed at least one file.
    Check that the record is gone.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Comment 3 Marcel de Rooy 2017-04-03 08:26:36 UTC
Created attachment 61778 [details] [review]
Bug 18300: Delete missing upload records

If you deleted files from the upload directories manually, or you rebooted
with files in the temporary upload folder, or for some other reason have
records without a file, you may want to cleanup your records in the
uploaded_files table.

This patch adds the method delete_missing to Koha::UploadedFiles. It also
supports a keep_record parameter to do a dry run (count the missing files
first).

Also, we add an option --uploads-missing to cleanup_database. If you add
the flag 1 after this option, you will delete missing files. If you add the
flag 0 or only use the option, you will count missing files.

A subtest is added to Upload.t for delete_missing tests.

Test plan:
[1] Run t/db_dependent/Upload.t
[2] Upload a file and delete the file manually.
[3] Run cleanup_database.pl --uploads-missing
    It should report at least one missing file now.
    Check that the record has not been deleted.
[4] Run cleanup_database.pl --uploads-missing 1
    It should report that it removed at least one file.
    Check that the record is gone.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Comment 4 Marcel de Rooy 2017-04-03 08:27:14 UTC
Rebased on top of changes in report 17669 (cleanup_database.pl)
Comment 5 Marcel de Rooy 2017-04-03 19:42:20 UTC Comment hidden (obsolete)
Comment 6 Marcel de Rooy 2017-04-03 19:45:36 UTC Comment hidden (obsolete)
Comment 7 Marcel de Rooy 2017-04-04 13:11:01 UTC
Created attachment 61821 [details] [review]
Bug 18300: Delete missing upload records

If you deleted files from the upload directories manually, or you rebooted
with files in the temporary upload folder, or for some other reason have
records without a file, you may want to cleanup your records in the
uploaded_files table.

This patch adds the method delete_missing to Koha::UploadedFiles. It also
supports a keep_record parameter to do a dry run (count the missing files
first).

Also, we add an option --uploads-missing to cleanup_database. If you add
the flag 1 after this option, you will delete missing files. If you add the
flag 0 or only use the option, you will count missing files.

A subtest is added to Upload.t for delete_missing tests.

Test plan:
[1] Run t/db_dependent/Upload.t
[2] Upload a file and delete the file manually.
[3] Run cleanup_database.pl --uploads-missing
    It should report at least one missing file now.
    Check that the record has not been deleted.
[4] Run cleanup_database.pl --uploads-missing 1
    It should report that it removed at least one file.
    Check that the record is gone.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Comment 8 Marcel de Rooy 2017-04-04 13:11:05 UTC
Created attachment 61822 [details] [review]
Bug 18300: [QA Follow-up] Fix return value inconsistency

As noted on bug report 17669, the return values of delete (both singular
and plural), delete_missing and delete_temporary should be consistent.

Removed the if-construction around full_path. We do not need it; in the
very exceptional case that full_path would be empty, we should delete
the record since the file is missing.

Adjusted POD and unit tests accordingly.

Test plan:
Run t/db_dependent/Upload.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Jonathan Druart 2017-04-20 18:29:28 UTC
Created attachment 62496 [details] [review]
Bug 18300: Delete missing upload records

If you deleted files from the upload directories manually, or you rebooted
with files in the temporary upload folder, or for some other reason have
records without a file, you may want to cleanup your records in the
uploaded_files table.

This patch adds the method delete_missing to Koha::UploadedFiles. It also
supports a keep_record parameter to do a dry run (count the missing files
first).

Also, we add an option --uploads-missing to cleanup_database. If you add
the flag 1 after this option, you will delete missing files. If you add the
flag 0 or only use the option, you will count missing files.

A subtest is added to Upload.t for delete_missing tests.

Test plan:
[1] Run t/db_dependent/Upload.t
[2] Upload a file and delete the file manually.
[3] Run cleanup_database.pl --uploads-missing
    It should report at least one missing file now.
    Check that the record has not been deleted.
[4] Run cleanup_database.pl --uploads-missing 1
    It should report that it removed at least one file.
    Check that the record is gone.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 10 Jonathan Druart 2017-04-20 18:29:32 UTC
Created attachment 62497 [details] [review]
Bug 18300: [QA Follow-up] Fix return value inconsistency

As noted on bug report 17669, the return values of delete (both singular
and plural), delete_missing and delete_temporary should be consistent.

Removed the if-construction around full_path. We do not need it; in the
very exceptional case that full_path would be empty, we should delete
the record since the file is missing.

Adjusted POD and unit tests accordingly.

Test plan:
Run t/db_dependent/Upload.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 11 Kyle M Hall 2017-04-21 18:12:44 UTC
Pushed to master for 17.05, thanks Marcel!
Comment 12 Katrin Fischer 2017-04-21 23:21:36 UTC
This depends on bug 17669 which is not in 16.11.x.

This won't get ported back to 16.11.x as it is an enhancement.