Bug 43088: Add a consistent current_directory() accessor to Koha::File::Transport

FTP, Local and SFTP list_files() were unified to return the same flat
shape, but consumers still had no portable way to ask a transport for
its current remote directory - the underlying libraries' own methods
of that name don't agree on what "cwd" means. Net::SFTP::Foreign's
cwd() is a pure read-only accessor, but Net::FTP's cwd() is a mutator:
called with no argument it defaults to "/", issues a real CWD command,
and returns a boolean rather than a path.

The koha-plugin-transport-browser plugin discovered this the hard way:
it read $transport->{connection}->cwd directly to capture the initial
working directory, which happened to work for SFTP but silently reset
the FTP connection to root and captured "1" (a boolean) as the "path"
on every request, breaking directory navigation for FTP transports.

Add a public current_directory() method to the Koha::File::Transport
base class, following the same connect-on-demand pattern as
list_files()/upload_file()/etc, with a _current_directory() hook each
subclass implements using its own real read-only accessor: pwd() for
FTP, cwd() for SFTP, and the tracked {current_directory} (falling back
to download_directory) for Local. Consumers should use this instead of
reaching into transport internals.

Test plan:
1. prove t/db_dependent/Koha/File/Transport.t
        t/db_dependent/Koha/File/Transport/FTP.t
        t/db_dependent/Koha/File/Transport/Local.t
        t/db_dependent/Koha/File/Transport/SFTP.t
   => All tests successful, including the new current_directory()
      tests subtest in each transport test file, which mock the
      underlying libraries (Net::FTP / Net::SFTP::Foreign) to assert
      each backend reads its own real read-only accessor.
2. koha-qa.pl -v 2 --more-tests
   => [PASS] for all four changed modules, no critic/pod warnings.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>

Koha/File/Transport.pm

@@ -244,6 +244,45 @@ sub _is_connected {
     die "Subclass must implement _is_connected";
 }
 
+=head3 current_directory
+
+    my $path = $transport->current_directory();
+
+Returns the transport's current remote working directory as a plain string,
+without side effects. Automatically establishes a connection if needed.
+
+B<Note:> This is a read-only query. It must never navigate the connection -
+unlike some underlying libraries' own C<cwd()>/C<pwd()> methods, which differ
+in meaning between backends (see subclass implementations).
+
+B<Returns:> The current directory path on success, undef on failure. Check
+object_messages() for details.
+
+=cut
+
+sub current_directory {
+    my ($self) = @_;
+
+    return unless $self->_ensure_connected();
+
+    return $self->_current_directory();
+}
+
+=head3 _current_directory
+
+    my $path = $transport->_current_directory();
+
+Internal method that returns the protocol-specific current working
+directory. Must be implemented by subclasses. Called by current_directory()
+after connection verification.
+
+=cut
+
+sub _current_directory {
+    my ($self) = @_;
+    die "Subclass must implement _current_directory";
+}
+
 =head3 _auto_change_directory
 
     $transport->_auto_change_directory($dir_type, $custom_path);

Koha/File/Transport/FTP.pm

@@ -123,6 +123,26 @@ sub _download_file {
     return 1;
 }
 
+=head3 _current_directory
+
+    my $path = $self->_current_directory;
+
+Internal method that returns the FTP connection's current working directory.
+
+Net::FTP's own C<cwd()> method is a mutator: called with no argument it
+defaults to "/" and issues a real C<CWD /> command, changing the connection's
+directory as a side effect and returning a boolean rather than a path. The
+read-only equivalent is C<pwd()>, which sends PWD and parses the path out of
+the response without navigating anywhere.
+
+=cut
+
+sub _current_directory {
+    my ($self) = @_;
+
+    return $self->{connection}->pwd;
+}
+
 =head3 _change_directory
 
     my $success = $server->_change_directory($directory);

Koha/File/Transport/Local.pm

@@ -254,6 +254,22 @@ sub _change_directory {
     return 1;
 }
 
+=head3 _current_directory
+
+    my $path = $self->_current_directory;
+
+Internal method that returns the local transport's current working
+directory: the directory most recently set via change_directory(), falling
+back to the configured download_directory, then to '.'.
+
+=cut
+
+sub _current_directory {
+    my ($self) = @_;
+
+    return $self->{current_directory} || $self->download_directory || '.';
+}
+
 =head3 _list_files
 
 Internal method that performs the local file system file listing operation.

Koha/File/Transport/SFTP.pm

@@ -138,6 +138,24 @@ sub _download_file {
     return 1;
 }
 
+=head3 _current_directory
+
+    my $path = $self->_current_directory;
+
+Internal method that returns the SFTP connection's current working directory.
+
+Net::SFTP::Foreign's C<cwd()> is a pure read-only accessor - it reports the
+current remote directory without navigating - unlike FTP's C<cwd()> (see
+Koha::File::Transport::FTP::_current_directory).
+
+=cut
+
+sub _current_directory {
+    my ($self) = @_;
+
+    return $self->{connection}->cwd;
+}
+
 =head3 _change_directory
 
     my $success = $server->_change_directory($directory);

t/db_dependent/Koha/File/Transport/FTP.t

@@ -17,7 +17,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 8;
+use Test::More tests => 9;
 use Test::Exception;
 use Test::NoWarnings;
 use Test::Warn;
@@ -135,6 +135,37 @@ subtest 'change_directory() tests' => sub {
     can_ok( $transport, 'change_directory' );
 };
 
+subtest 'current_directory() tests' => sub {
+    plan tests => 2;
+
+    $schema->storage->txn_begin;
+
+    my $transport = $builder->build_object(
+        {
+            class => 'Koha::File::Transports',
+            value => { transport => 'ftp', password => 'testpass' }
+        }
+    );
+
+    can_ok( $transport, 'current_directory' );
+
+    # Net::FTP's own cwd() is a mutator (no-arg means "change to /" and
+    # returns a boolean) - current_directory() must read pwd(), the real
+    # read-only accessor, never cwd().
+    my $mock_ftp = Test::MockModule->new('Net::FTP');
+    $mock_ftp->mock( 'pwd', sub { return '/incoming/'; } );
+    $mock_ftp->mock( 'cwd', sub { die 'cwd() must not be called by current_directory()'; } );
+
+    $transport->{connection}          = bless {}, 'Net::FTP';
+    $transport->{_user_set_directory} = 1;
+
+    is( $transport->current_directory, '/incoming/', 'current_directory() reads pwd(), not cwd()' );
+
+    $transport->{connection} = undef;
+
+    $schema->storage->txn_rollback;
+};
+
 subtest 'list_files() MLSD tests' => sub {
     plan tests => 16;
 

t/db_dependent/Koha/File/Transport/Local.t

@@ -17,7 +17,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 2;
+use Test::More tests => 3;
 use Test::NoWarnings;
 use File::Temp qw( tempdir );
 use File::Spec;
@@ -79,4 +79,42 @@ subtest 'list_files() tests' => sub {
     $schema->storage->txn_rollback;
 };
 
+subtest 'current_directory() tests' => sub {
+    plan tests => 3;
+
+    $schema->storage->txn_begin;
+
+    my $tempdir      = tempdir( CLEANUP => 1 );
+    my $download_dir = File::Spec->catdir( $tempdir, 'download' );
+    mkdir $download_dir or die "Cannot create download_dir: $!";
+
+    my $transport = $builder->build_object(
+        {
+            class => 'Koha::File::Transports',
+            value => {
+                transport          => 'local',
+                download_directory => $download_dir,
+            }
+        }
+    );
+
+    can_ok( $transport, 'current_directory' );
+
+    is(
+        $transport->current_directory, $download_dir,
+        'current_directory() falls back to the configured download_directory before any change_directory() call'
+    );
+
+    my $subdir_path = File::Spec->catdir( $download_dir, 'incoming' );
+    mkdir $subdir_path or die "Cannot create subdir: $!";
+
+    $transport->change_directory($subdir_path);
+    is(
+        $transport->current_directory, $subdir_path,
+        "current_directory() reflects the most recent change_directory() call"
+    );
+
+    $schema->storage->txn_rollback;
+};
+
 1;

t/db_dependent/Koha/File/Transport/SFTP.t

@@ -17,7 +17,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 9;
+use Test::More tests => 10;
 use Test::Exception;
 use Test::NoWarnings;
 use Test::Warn;
@@ -249,6 +249,37 @@ subtest 'change_directory() tests' => sub {
     can_ok( $transport, 'change_directory' );
 };
 
+subtest 'current_directory() tests' => sub {
+    plan tests => 2;
+
+    $schema->storage->txn_begin;
+
+    my $transport = $builder->build_object(
+        {
+            class => 'Koha::File::Transports',
+            value => { transport => 'sftp', password => 'testpass' }
+        }
+    );
+
+    can_ok( $transport, 'current_directory' );
+
+    # Net::SFTP::Foreign's cwd() is a pure read-only accessor - unlike FTP's
+    # cwd(), it never navigates - so current_directory() can read it directly.
+    my $undef;
+    my $mock_sftp = Test::MockModule->new('Net::SFTP::Foreign');
+    $mock_sftp->mock( 'error', sub { return $undef; } );
+    $mock_sftp->mock( 'cwd',   sub { return '/home/vendor'; } );
+
+    $transport->{connection}          = bless {}, 'Net::SFTP::Foreign';
+    $transport->{_user_set_directory} = 1;
+
+    is( $transport->current_directory, '/home/vendor', "current_directory() reads the SFTP connection's cwd()" );
+
+    $transport->{connection} = undef;
+
+    $schema->storage->txn_rollback;
+};
+
 subtest 'list_files() tests' => sub {
     plan tests => 12;