Request (2.x): Make private properties/methods protected instead.

[BusterNeece]

I've only just now had a chance to evaluate moving to Flysystem 2.x for my application, and I'm noticing that in Flysystem 2.x, a lot of the methods and properties that were previously protected in 1.x are now private, preventing any descendent classes from using them at all.

In our application, we extend both Filesystem and MountManager to add additional functionality that suits our application's purposes. This has been made exceptionally difficult to do by the privatizing of the methods, instead resulting in a ton of code being unnecessarily re-implemented in our own classes as copy/pasted versions of what's implemented in the library itself.

I realize it's "the new hotness" to make every class in a library like this final and every non-public method and property private and to require users who want to change anything to have to rewrite or wrap everything, but I've never been on board with the kind of implications this has on making real-world developers' lives harder for little if any benefit.

If nothing else, there are two places it would be immensely helpful to have access to the parent class via a protected method:

• getAdapter on the Filesystem class, which existed in v1 and was removed in v2, and
• determineFilesystemAndPath on the MountManager, which was previously getPrefixAndPath in v1.

Also, in the LocalFilesystemAdapter any way of getting the actual local path of something was removed. While I can imagine the rationale for this being that the filesystem abstraction API itself shouldn't reveal or care whether a file is local or not (which I understand and am fine with), for our purposes we have to optimize the application code to check whether the adapter is a local one and handle things differently in those cases, to avoid needlessly copying local files to a temporary local directory every single time they're used. Currently, in 2.x, doing this would require completely reimplementing the entire local adapter interface from scratch.

[frankdejonge]

Hi @SlvrEagle23,

The path prefixing has been extracted into a class you can use yourself as well (League\Flysystem\PathPrefixer). The adapter is something you construct and supply to the Filesystem instance, so you might as well pass it to something else that needs it too.In regards to motivation, my use of private has very little to do with following "the new hotness". It has everything to do with the maintenance burden that my previous decision to have a public internal API has inflicted on me.

The thing is, having a public internal interface is a slippery slope. The argument for having one thing exposed is always used for having the next thing exposed as well, and so on. With every publication of an internal concept a door is closed to refactor is when new needs or edge-cases come forward. I experienced this many times over and do not wish to spend my time like that again. Besides this, the answer to your troubles is already in your own statement. You can copy the class and modify what is needed. You can even automate this to be copied from the original source and have the visibility rewritten. This would allow you to modify it to your needs, although when my private API now changes, it's your burden to bear instead of mine. And this is a benefit that you can only see if you maintain a project for a number of years.

Some things you can still do:

1. Extend the FIleystem class and overwrite the constructor

This allows you to get the adapter if you so desire.

<?php

namespace League\Flysystem;

class ExtendedFilesystem extends Filesystem
{
    /**
     * @var FilesystemAdapter
     */
    private $internalAdapter;

    public function __construct(FilesystemAdapter $adapter, array $config = [], PathNormalizer $pathNormalizer = null)
    {
        $this->internalAdapter = $adapter;
        parent::__construct($adapter, $config, $pathNormalizer);
    }

    /**
     * @return FilesystemAdapter
     */
    public function getInternalAdapter(): FilesystemAdapter
    {
        return $this->internalAdapter;
    }
}

2. Extend the local filesystem to expose a path prefixer method:

<?php

namespace League\Flysystem\Local;

use League\Flysystem\PathPrefixer;
use League\Flysystem\UnixVisibility\VisibilityConverter;
use League\MimeTypeDetection\MimeTypeDetector;

class ExtendedLocalAdapter extends LocalFilesystemAdapter
{
    /**
     * @var PathPrefixer
     */
    private $pathPrefixer;

    public function __construct(
        string $location,
        VisibilityConverter $visibility = null,
        int $writeFlags = LOCK_EX,
        int $linkHandling = self::DISALLOW_LINKS,
        MimeTypeDetector $mimeTypeDetector = null
    ) {
        $this->pathPrefixer = new PathPrefixer($location);
        parent::__construct($location, $visibility, $writeFlags, $linkHandling, $mimeTypeDetector);
    }

    public function prefixPath(string $path): string
    {
        return $this->pathPrefixer->prefixPath($path);
    }
}

And finally:

3. Extracting a mounted filesystem

<?php

namespace League\Flysystem;

use function array_key_exists;
use function explode;
use function strpos;

class ExtendedMountManager extends MountManager
{
    /**
     * @var array
     */
    private $mountedFilesystem;

    public function __construct(array $filesystems = [])
    {
        $this->mountedFilesystem = $filesystems;
        parent::__construct($filesystems);
    }

    /**
     * @param string $path
     *
     * @return array{0:FilesystemOperator, 1:string}
     */
    public function getFilesystemAndPath(string $path): array
    {
        if (strpos($path, '://') < 1) {
            throw UnableToResolveFilesystemMount::becauseTheSeparatorIsMissing($path);
        }

        /** @var string $mountIdentifier */
        /** @var string $mountPath */
        [$mountIdentifier, $mountPath] = explode('://', $path, 2);

        if ( ! array_key_exists($mountIdentifier, $this->mountedFilesystem)) {
            throw UnableToResolveFilesystemMount::becauseTheMountWasNotRegistered($mountIdentifier);
        }

        return [$this->mountedFilesystem[$mountIdentifier], $mountPath];
    }
}

---

If you ask me, the limitations of the visibility are (in my opinion) not very limiting at all. Apart from this all, I do not share the perspective of the current visibility having value. It has a lot of value, albeit for me and not for you. From my perspective, the cost of having a public internal interface is far greater than the return, especially when the limitations are so easily overcome.