https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40811 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40811 --- Comment #2 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 186858 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186858&action=edit Bug 40811: Implement simplified Transport API with connection management This commit introduces a simplified API for the Transport classes that reduces the traditional 5-step workflow to a 2-step workflow while maintaining backward compatibility. Key changes: - Centralized connection and directory management in base Transport class - Added _ensure_connected() method for automatic connection handling - Added _auto_change_directory() for intelligent directory management - Refactored all transport methods to use private implementations (_upload_file, etc.) - Added state tracking to respect explicit change_directory() calls - Support for both traditional explicit API and simplified auto-managing API Traditional API (unchanged): $transport->connect(); $transport->change_directory($path); $transport->upload_file($local, $remote); $transport->disconnect(); Simplified API (new): $transport->upload_file($local, $remote, { path => '/custom/path' }); # OR using default upload directory: $transport->upload_file($local, $remote); The implementation intelligently handles three scenarios: 1. Simplified API with custom path - uses provided path 2. Traditional API with explicit directory management - respects user's choice 3. Traditional API without explicit directories - uses configured defaults -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40811 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40811 --- Comment #4 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- It looks better. I don't love `_ensure_connected` but I get where the terminology comes from. I'd say it is pretty close to what I proposed. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40811 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #186858|0 |1 is obsolete| | --- Comment #5 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 188035 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=188035&action=edit Bug 40811: Implement simplified Transport API with connection management This commit introduces a simplified API for the Transport classes that reduces the traditional 5-step workflow to a 2-step workflow while maintaining backward compatibility. Key changes: - Centralized connection and directory management in base Transport class - Added _ensure_connected() method for automatic connection handling - Added _auto_change_directory() for intelligent directory management - Refactored all transport methods to use private implementations (_upload_file, etc.) - Added state tracking to respect explicit change_directory() calls - Support for both traditional explicit API and simplified auto-managing API Traditional API (unchanged): $transport->connect(); $transport->change_directory($path); $transport->upload_file($local, $remote); $transport->disconnect(); Simplified API (new): $transport->upload_file($local, $remote, { path => '/custom/path' }); # OR using default upload directory: $transport->upload_file($local, $remote); The implementation intelligently handles three scenarios: 1. Simplified API with custom path - uses provided path 2. Traditional API with explicit directory management - respects user's choice 3. Traditional API without explicit directories - uses configured defaults Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40811 --- Comment #6 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 188152 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=188152&action=edit Bug 40811: Implement dual API for file transports with simplified auto-management This patch implements a dual API design for Koha::File::Transport, providing both a simplified auto-managing API and a traditional explicit API for maximum flexibility and ease of use. DUAL API DESIGN =============== Simplified API (Recommended): - Automatic connection management (no connect/disconnect needed) - Flexible per-operation directory control via options hashref - Stateless operations safe for concurrent usage Example: $transport->upload_file($local, $remote, { path => '/custom/' }); $transport->download_file($remote, $local); # Uses download_directory $transport->list_files({ path => '/incoming/' }); Traditional API (Explicit Control): - Manual connection/directory management when needed - Stateful directory operations via change_directory() - Ideal for multiple operations in the same directory Example: $transport->change_directory('/work/'); $transport->upload_file($local, 'file1.txt'); $transport->upload_file($local, 'file2.txt'); my $files = $transport->list_files(); The APIs can be mixed - calling change_directory() explicitly switches to manual mode and disables auto-management for subsequent operations. STANDARDIZED TEMPLATE METHOD PATTERN ===================================== All transport methods now follow a consistent pattern with clear separation between public API (parent class) and protocol implementation (subclasses). Public methods in Koha::File::Transport: - connect() → Resets directory state, calls _connect() - disconnect() → Resets directory state, calls _disconnect() - change_directory() → Sets manual mode flag, calls _change_directory() - upload_file() → Manages connection/directory, calls _upload_file() - download_file() → Manages connection/directory, calls _download_file() - list_files() → Manages connection/directory, calls _list_files() - rename_file() → Ensures connection, calls _rename_file() Subclass responsibilities (SFTP/FTP/Local): - Implement _connect() - Protocol-specific connection logic - Implement _disconnect() - Protocol-specific disconnection logic - Implement _change_directory() - Protocol-specific directory change - Implement _upload_file() - Protocol-specific upload logic - Implement _download_file() - Protocol-specific download logic - Implement _list_files() - Protocol-specific listing logic - Implement _rename_file() - Protocol-specific rename logic - Implement _is_connected() - Protocol-specific connection check Test plan: prove t/db_dependent/Koha/File/Transports.t Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40811 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |tomascohen@gmail.com |y.org | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40811 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |38629 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38629 [Bug 38629] Update sftp_file.pl to support the new SFTP configurations page -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40811 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Medium patch Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40811 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #188313|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40811 --- Comment #9 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 190461 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190461&action=edit Bug 40811: (follow-up) Fix list context bug and add comprehensive tests This patch fixes a critical bug where plain_text_password() was called in list context during hash construction. When a transport has no password, the method's bare 'return' statement returns an empty list () in list context rather than undef, causing all subsequent key-value pairs to shift positions. This resulted in malformed connection parameters being passed to Net::SFTP::Foreign and Net::FTP. Example of the bug: my %hash = ( user => 'john', password => $self->plain_text_password, # Returns () when no password timeout => 10 ); # Results in: (user => 'john', timeout => 10) instead of # (user => 'john', password => undef, timeout => 10) Changes to code: - Wrap plain_text_password calls with scalar() to force scalar context in both FTP.pm and SFTP.pm - Make SFTP key and password authentication mutually exclusive using ternary operator - Prefer key-based auth over password when both are available (SFTP) Test plan: 1. Run t/db_dependent/Koha/File/Transport/FTP.t - should pass 2. Run t/db_dependent/Koha/File/Transport/SFTP.t - should pass 3. Verify tests cover: - Scalar context behavior with no password - FTP authentication with proper parameter passing - SFTP password-only authentication - SFTP key-only authentication - SFTP key preference when both key and password exist Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40811 --- Comment #10 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 26.05 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40811 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This enhancement introduces release notes| |a modernized dual API | |design for Koha's file | |transport system, making it | |easier for developers to | |work with FTP, SFTP, and | |local file operations. | | | | Key Improvements | | | | Simplified API | |(Recommended) | | - Automatic connection | |management - no need for | |manual connect() / | |disconnect() calls | | - Per-operation directory | |control via options hashref | | - Stateless operations | |safe for concurrent usage | | | | # Simple one-line | |operations | | | |$transport->upload_file($lo | |cal, $remote, { path => | |'/custom/' }); | | | |$transport->download_file($ | |remote, $local); | | $transport->list_files({ | |path => '/incoming/' }); | | | | Traditional API (Still | |Supported) | | - Manual connection and | |directory management when | |needed | | - Ideal for multiple | |operations in the same | |directory | | - Full backward | |compatibility maintained | | | | # Explicit control when | |needed | | | |$transport->change_director | |y('/work/'); | | | |$transport->upload_file($lo | |cal, 'file1.txt'); | | | |$transport->upload_file($lo | |cal, 'file2.txt'); | | | | Standardized Architecture | | - Consistent template | |method pattern across all | |transport types (SFTP, FTP, | |Local) | | - Clear separation | |between public API and | |protocol-specific | |implementation | | - Improved authentication | |handling with fixes for | |password-less connections | | | | This is an internal API | |enhancement that improves | |code maintainability and | |developer experience | |without affecting end-user | |functionality. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40811 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Needs documenting --- Comment #12 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- This bug won't be backported to the 25.11.x branch as it has been deemed an enhancement or new feature. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40811 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|42656 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42656 [Bug 42656] File transport test responses can be unclear -- You are receiving this mail because: You are watching all bug changes.