From 87f917ae564a0a956d46d79e514b7105da506f2e Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Tue, 23 Sep 2025 18:49:57 +0100 Subject: [PATCH] Bug 38489: Add disconnect method and proper connection cleanup to file transport system This patch adds proper connection cleanup functionality to the file transport system, addressing a gap identified during EDI migration. Changes: - Add disconnect() method to base Transport class interface - Implement SFTP disconnect using connection->disconnect() - Implement FTP disconnect using connection->quit() - Add DESTROY methods to ensure connections are cleaned up on object destruction - Local transport includes no-op disconnect for API consistency This ensures that FTP/SFTP connections are properly closed, preventing connection leaks and potential resource exhaustion issues. Sponsored-by: ByWater Solutions Signed-off-by: Hannah Dunne-Howrie Signed-off-by: Kyle M Hall --- Koha/File/Transport.pm | 13 +++++++++++++ Koha/File/Transport/FTP.pm | 34 ++++++++++++++++++++++++++++++++++ Koha/File/Transport/Local.pm | 15 +++++++++++++++ Koha/File/Transport/SFTP.pm | 24 ++++++++++++++++++++++++ 4 files changed, 86 insertions(+) diff --git a/Koha/File/Transport.pm b/Koha/File/Transport.pm index 3e9e847b809..188aaa1ec8e 100644 --- a/Koha/File/Transport.pm +++ b/Koha/File/Transport.pm @@ -264,6 +264,19 @@ sub rename_file { die "Subclass must implement rename_file"; } +=head3 disconnect + + $transport->disconnect(); + +Method for disconnecting from the current file server + +=cut + +sub disconnect { + my ($self) = @_; + die "Subclass must implement disconnect"; +} + =head3 _post_store_trigger $server->_post_store_trigger; diff --git a/Koha/File/Transport/FTP.pm b/Koha/File/Transport/FTP.pm index 0bc23eee278..cc215fc83c9 100644 --- a/Koha/File/Transport/FTP.pm +++ b/Koha/File/Transport/FTP.pm @@ -188,6 +188,25 @@ sub rename_file { return 1; } +=head3 disconnect + + $server->disconnect(); + +Disconnects from the FTP server. + +=cut + +sub disconnect { + my ($self) = @_; + + if ( $self->{connection} ) { + $self->{connection}->quit; + $self->{connection} = undef; + } + + return 1; +} + sub _abort_operation { my ( $self, $message ) = @_; @@ -206,4 +225,19 @@ sub _abort_operation { return; } +=head3 DESTROY + +Ensure proper cleanup of FTP connections + +=cut + +sub DESTROY { + my ($self) = @_; + + # Clean up the FTP connection + if ( $self->{connection} ) { + $self->{connection}->quit; + } +} + 1; diff --git a/Koha/File/Transport/Local.pm b/Koha/File/Transport/Local.pm index 058980f3db8..22ef93bc5b3 100644 --- a/Koha/File/Transport/Local.pm +++ b/Koha/File/Transport/Local.pm @@ -379,4 +379,19 @@ sub rename_file { return 1; } +=head3 disconnect + + $server->disconnect(); + +For local transport, this is a no-op as there are no connections to close. + +=cut + +sub disconnect { + my ($self) = @_; + + # No-op for local transport + return 1; +} + 1; diff --git a/Koha/File/Transport/SFTP.pm b/Koha/File/Transport/SFTP.pm index f7ffe19577e..a2a37dd0186 100644 --- a/Koha/File/Transport/SFTP.pm +++ b/Koha/File/Transport/SFTP.pm @@ -231,6 +231,25 @@ sub rename_file { return 1; } +=head3 disconnect + + $server->disconnect(); + +Disconnects from the SFTP server. + +=cut + +sub disconnect { + my ($self) = @_; + + if ( $self->{connection} ) { + $self->{connection}->disconnect; + $self->{connection} = undef; + } + + return 1; +} + =head2 Internal methods =head3 _post_store_trigger @@ -355,6 +374,11 @@ Ensure proper cleanup of open filehandles sub DESTROY { my ($self) = @_; + # Clean up the SFTP connection + if ( $self->{connection} ) { + $self->{connection}->disconnect; + } + # Ensure the filehandle is closed properly if ( $self->{stderr_fh} ) { close $self->{stderr_fh} or warn "Failed to close STDERR filehandle: $!"; -- 2.39.5 (Apple Git-154)