add getters to filesystem repository

[Gummibeer]

fixes #43

[Gummibeer]

👊 poke - this PR is blocking for Astrotomic/stancy#23
Could you please review it? 😊

[sebastiandedeyne]

Hmm this doesn't really feel right... This is unusable if someone uses a different repository. If you need this information, maybe you're better of extracting it from the configuration?

[Gummibeer]

It's not part of the interface. So a simple instance of check is enough.
And is MUCH simpler than extracting myself and doesn't hurt anyone. It's only an additional feature which isn't part of the defined public API for all repositories.

[brendt]

Can I suggest an alternative?

I want to be able to create a new file in the sheet collection filesystem.

So in essence, you need access to the same filesystem used by the sheets. I would rely on the dependency container to resolve the correct filesystem, eg. something like this:

class SheetsFilesystem extends Filesystem
{
    public function __construct()
    {
        parent::__construct('disk_name');
    }
}

(I haven't checked if this code is correct, but I'm sure you get what I mean)

This way the container is doing what it's supposed to do: managing class instances, and we don't need to add these kinds of shortcuts on the repository class.

I'd even go as far as manually configuring the sheet repository in your AppServiceProvider, manually passing it the SheetsFilesystem, just to be explicit.

Anyway, that's what I would do.

[Gummibeer]

Just to know: why don't you want the getters? 🤔

[brendt]

It's not the repository's responsibility to expose one of its dependencies to the outside world, that's the container's job 🙂

[Gummibeer]

Okay, but the container also doesn't expose these two things. Only the config does but this also just optional.
I would also be happy with any public option to get a full merged config. How about a config class that does what the service provider does in protected methods? I'm okay with resolving the service my own. But doing the whole config resolving my own isn't a good solution for me because I have to copy part of this package logic.

[Gummibeer]

I've solved it in my package.

[brendt]

How about a config class that does what the service provider does in protected methods?

I was actually about to say the same. Freek already does this in one or two packages, and I think it's a pattern we should use more. Maybe @sebastiandedeyne is open for that idea?

[Gummibeer]

Yes, the idea is from facade/ignition. Right now I've done it in two lines in my package. But would still like to adopt the config class or even provide a PR for it.

[Gummibeer]

Regarding the repository interface and the package implementation FilesystemRepository.
How am I supposed to configure my own repository? The package implementation is hardcoded in the service provider. No service binding, config value or anything else to inject my own repository.

[Gummibeer]

#45 is the follow up for custom repositories.