From d9b2f68037913c986f877a095bb03e5d39d8f58f Mon Sep 17 00:00:00 2001 From: John Doe Date: Wed, 15 Oct 2025 18:06:47 +0000 Subject: [PATCH] Bug 41020: Add delete_file() method to file transport classes This patch adds the ability to delete files from remote servers through the file transport API. This is needed for the MARC ordering feature to clean up processed files from FTP/SFTP servers. Changes: - Added abstract delete_file() method to Koha::File::Transport base class - Implemented delete_file() in Koha::File::Transport::FTP using Net::FTP->delete() - Implemented delete_file() in Koha::File::Transport::SFTP using Net::SFTP::Foreign->remove() - Implemented delete_file() in Koha::File::Transport::Local using unlink() - All implementations follow the standard message/error handling pattern - Updated polymorphic tests to verify delete_file() capability The delete_file() method: - Takes a filename as parameter (relative to current directory) - Returns true on success, undef on failure - Adds appropriate success/error messages to the transport message queue - Follows the same pattern as existing methods (download_file, rename_file) Test plan: 1. prove t/db_dependent/Koha/File/Transports.t 2. Verify all transport types support delete_file() 3. Test with MARC ordering cronjob using --dr flag (added in main patch) Signed-off-by: Martin Renvoize --- Koha/File/Transport.pm | 13 ++++++ Koha/File/Transport/FTP.pm | 29 ++++++++++++++ Koha/File/Transport/Local.pm | 57 +++++++++++++++++++++++++++ Koha/File/Transport/SFTP.pm | 30 ++++++++++++++ t/db_dependent/Koha/File/Transports.t | 4 +- 5 files changed, 132 insertions(+), 1 deletion(-) diff --git a/Koha/File/Transport.pm b/Koha/File/Transport.pm index 8f5e7d61a75..495c8d8ec0f 100644 --- a/Koha/File/Transport.pm +++ b/Koha/File/Transport.pm @@ -354,6 +354,19 @@ sub download_file { return $self->_download_file( $remote_file, $local_file ); } +=head3 delete_file + + $transport->delete_file($file); + +Method for deleting a file from the current file server + +=cut + +sub delete_file { + my ( $self, $file ) = @_; + die "Subclass must implement delete_file"; +} + =head3 _download_file $transport->_download_file($remote_file, $local_file); diff --git a/Koha/File/Transport/FTP.pm b/Koha/File/Transport/FTP.pm index 10484b30f50..cce88f5b3a7 100644 --- a/Koha/File/Transport/FTP.pm +++ b/Koha/File/Transport/FTP.pm @@ -122,6 +122,35 @@ sub _download_file { return 1; } +=head3 delete_file + + my $success = $server->delete_file($filename); + +Passed a filename, this will delete the file from the current directory of the server connection. + +Returns true on success or undefined on failure. + +=cut + +sub delete_file { + my ( $self, $remote_file ) = @_; + my $operation = "delete"; + + $self->{connection}->delete($remote_file) or return $self->_abort_operation( $operation, $remote_file ); + + $self->add_message( + { + message => $operation, + type => 'success', + payload => { + remote_file => $remote_file, + } + } + ); + + return 1; +} + =head3 change_directory my $success = $server->change_directory($directory); diff --git a/Koha/File/Transport/Local.pm b/Koha/File/Transport/Local.pm index e56f45318d0..adef6837fe6 100644 --- a/Koha/File/Transport/Local.pm +++ b/Koha/File/Transport/Local.pm @@ -403,6 +403,63 @@ sub _is_connected { return 1; # Local filesystem is always "connected" } +=head3 delete_file + + $server->delete_file(); + +Deletes a file in the current directory. + +Returns true on success or undefined on failure. + +=cut + +sub delete_file { + my ( $self, $remote_file ) = @_; + my $operation = "delete"; + + my $directory = $self->download_directory || $self->{current_directory} || '.'; + my $source = File::Spec->catfile( $directory, $remote_file ); + + # Check if the file exists + unless ( -e $source ) { + $self->add_message( + { + message => $operation, + type => 'error', + payload => { + error => "File not found: $source", + path => $source + } + } + ); + return; + } + + # Attempt to delete the file + if ( unlink $source ) { + $self->add_message( + { + message => $operation, + type => 'success', + payload => { path => $source } + } + ); + } else { + $self->add_message( + { + message => $operation, + type => 'error', + payload => { + error => "File could not be deleted: $source", + path => $source + } + } + ); + } + + return 1; +} + =head3 disconnect $server->disconnect(); diff --git a/Koha/File/Transport/SFTP.pm b/Koha/File/Transport/SFTP.pm index ad3cabe8ff7..b69fc181919 100644 --- a/Koha/File/Transport/SFTP.pm +++ b/Koha/File/Transport/SFTP.pm @@ -21,6 +21,7 @@ use Koha::Logger; use File::Spec; use IO::File; +use JSON qw( encode_json ); use Net::SFTP::Foreign; use Try::Tiny; @@ -134,6 +135,35 @@ sub _download_file { return 1; } +=head3 delete_file + + my $success = $server->delete_file($filename); + +Passed a filename, this will delete the file from the current directory of the server connection. + +Returns true on success or undefined on failure. + +=cut + +sub delete_file { + my ( $self, $remote_file ) = @_; + my $operation = "delete"; + + $self->{connection}->remove($remote_file) or return $self->_abort_operation( $operation, $remote_file ); + + $self->add_message( + { + message => $operation, + type => 'success', + payload => { + remote_file => $remote_file, + } + } + ); + + return 1; +} + =head3 change_directory my $success = $server->change_directory($directory); diff --git a/t/db_dependent/Koha/File/Transports.t b/t/db_dependent/Koha/File/Transports.t index 928c48423f0..597bd646bb4 100755 --- a/t/db_dependent/Koha/File/Transports.t +++ b/t/db_dependent/Koha/File/Transports.t @@ -29,7 +29,7 @@ my $schema = Koha::Database->new->schema; my $builder = t::lib::TestBuilder->new; subtest 'Polymorphic object creation' => sub { - plan tests => 6; + plan tests => 7; $schema->storage->txn_begin; @@ -90,6 +90,8 @@ subtest 'Polymorphic object creation' => sub { can_ok( $local_transport, 'rename_file' ); + can_ok( $local_transport, 'delete_file' ); + $schema->storage->txn_rollback; }; -- 2.51.0