tenancy icon indicating copy to clipboard operation
tenancy copied to clipboard

[4.x] This adds support for tenancy aware Storage::url() method

Open dontfreakout opened this issue 4 years ago • 21 comments

This fixes #78 and #201 and enables using Storage::url('file.jpg') with local driver in tenants (see Laravel docs).

With this, you can use for example Spatie media library and local disk without any modifications. You only need to create symlinks in the public directory which points to the tenants' disks root.

For creating symlinks you can use the new command tenants:link

dontfreakout avatar Jul 19 '21 23:07 dontfreakout

Thanks a lot for the PR! The code looks very high quality. I'll review & merge this in a few weeks when I start work on v4 and will be making all changes at once. Thanks again!

stancl avatar Jul 19 '21 23:07 stancl

Thanks :-) Tomorrow I'll try to write a better description of all changes.

dontfreakout avatar Jul 19 '21 23:07 dontfreakout

@stancl any plans of merging this? idk but seems like tests failed

aneeskhan47 avatar Sep 08 '21 14:09 aneeskhan47

@stancl any plans of merging this? IDK but seems like tests failed

Hi @aneeskhan47 , tests failed not because of code, but because of an error in building one of the docker containers in workflow. Otherwise, all tests should pass.

You can test it yourself by cloning this pull request (e.g. to Gitpod) and run provided docker (see CONTRIBUTING.md)

If you want to use this in v3 you can extract my changes to custom TenancyBootstrapper.

I would help you more, but right now I'm at the hospital recovering from an accident and I'm limited to my phone screen.

dontfreakout avatar Sep 09 '21 09:09 dontfreakout

@dontfreakout first of all thank you for commenting and thank you for the pr as well. Yes I'm thinking to use your pr code to my custom tenancy boostrapper And I hope @stancl will merge this to currentl v3 or upcoming v4 🤗

aneeskhan47 avatar Sep 09 '21 09:09 aneeskhan47

@aneeskhan47 you are very welcome. It should be pretty straightforward. This PR is extracted (with some modifications) from the bootstrapper I'm using in v3.

If all goes well, next week I'll be back home and can create a gist, if it helps.

dontfreakout avatar Sep 09 '21 10:09 dontfreakout

@dontfreakout i have extracted your pr code to mine. i have added my own FilesystemTenancyBootstrapper (with your code) + the link command to my App\Console\Commands directory and modified the tenancy config to use my FilesystemTenancyBootstrapper plus added the url_override option. i also ran the tenant:link command and it succesfully linked, but now the global_asset helper got kinda broken:

image

aneeskhan47 avatar Sep 09 '21 10:09 aneeskhan47

@dontfreakout actually the reason of the global asset broke is that in tenancy original TenancyServiceProvider.php in /src the globalUrl was bounding to the Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper::class i have copied that and added to my TenancyServiceProvider in app/providers boot method and now it works :).

for the time being i have copied your PR code and i hope this PR to get merged @stancl so then i can use the package normally :)

Thanks @dontfreakout and @stancl

image

aneeskhan47 avatar Sep 09 '21 11:09 aneeskhan47

@aneeskhan47 did you disabled original FilesystemBootrapper in the config?

I can't do much right now. The phone screen is not very comfortable for anything about code 😒

About the symlinks... Can you check if created tenant dir in the public dir is a symlink or just a plain directory?

dontfreakout avatar Sep 09 '21 11:09 dontfreakout

@aneeskhan47 you are faster than me 🙂 Glad you got it working.

dontfreakout avatar Sep 09 '21 11:09 dontfreakout

Couldn't find the button to re-run the build, so I pushed a small change. The Docker issue was probably a random thing that'll be fixed by re-running the build.

Thanks for the help here @dontfreakout

stancl avatar Sep 09 '21 11:09 stancl

Ok, so @aneeskhan47 were right about the bug in the link command (at least for L6).

For the link command, I'm extending Laravel StorageLinkCommand, but L6 can create links only from "public/storage" to "storage/app/public".

I'll fix this once I get to my PC (next week, I hope).

dontfreakout avatar Sep 09 '21 12:09 dontfreakout

Ok, so @aneeskhan47 were right about the bug in the link command (at least for L6).

For the link command, I'm extending Laravel StorageLinkCommand, but L6 can create links only from "public/storage" to "storage/app/public".

I'll fix this once I get to my PC (next week, I hope).

@dontfreakout Hi, i hope you are doing well. when you will make the fix ? if you are unable for some reason tell me the changes and i'll add them 😄

aneeskhan47 avatar Sep 14 '21 02:09 aneeskhan47

@aneeskhan47 Hi, I'm back home after almost three weeks in hospital, so I'm doing definitely better. Thanks for asking :-)

The fix is committed. I've also added a few more features: Events, RemoveSymlink Job. And added Jobs to TenancyServiceProvider stub (to create and remove symlink when created/deleted Tenancy).

@stancl to manually trigger a workflow, the workflow must be configured to run on the workflow_dispatch, see the docs.

dontfreakout avatar Sep 14 '21 12:09 dontfreakout

This adds a lot of new stuff, so I'll add it in v4, but I'll also have to make some changes when I'll be merging this. As I mentioned in #654, Laravel 9 uses flysystem 2, which forces me to rewrite how the FilesystemTenancyBootstrapper works.

So once I have the v4 filesystem bootstrapper done, I'll update the code of this and merge 👍🏻

stancl avatar Dec 25 '21 21:12 stancl

Sorry, if i upgrade package to v3.5 can i use php artisan tenants:link ? or will it only be available in version 4?

serkor avatar Jan 05 '22 18:01 serkor

It will be available in v4.

stancl avatar Jan 05 '22 20:01 stancl

FWIW the filesystem logic (that uses adapters) will need to be adjusted after #802 is merged

stancl avatar Feb 19 '22 20:02 stancl

Add this code to your AppServiceProvider.php and add storage/tenant-xxx directory to the public/public-xxx

config/tenancy.php -> filesystem

/*
 * Use this to support Storage url method on local driver disks.
 * You should create a symbolic link which points to the public directory using command: artisan tenants:link
 * Then you can use tenant aware Storage url: Storage::disk('public')->url('file.jpg')
 */
'url_override' => [
    // The array key is local disk (must exist in root_override) and value is public directory (%tenant_id% will be replaced with actual tenant id).
    'public' => 'public-%tenant_id%',
],

AppServiceProvider.php

\Illuminate\Support\Facades\URL::macro('tenantFile', function(?string $file = '') {    
    if (is_null($file)) {
        return null;
    }

    // file is the url information, which is returned directly. No tenant domain name splicing.
    if (str_contains($file, '://')) {
        return $file;
    }
    
    $prefix = str_replace(
        '%tenant_id%', 
        tenant()->getKey(), 
        config('tenancy.filesystem.url_override.public', 'public-%tenant_id%')
    );

    return url($prefix.'/'.$file);
});

TenancyServiceProvider.php

Events\TenantCreated::class => [
    JobPipeline::make([
        Jobs\CreateDatabase::class,
        Jobs\MigrateDatabase::class,
        Jobs\SeedDatabase::class,

        // Your own jobs to prepare the tenant.
        // Provision API keys, create S3 buckets, anything you want!

    ])->send(function (Events\TenantCreated $event) {
        $tenant = $event->tenant;
        $target = base_path(sprintf("storage/%s%s/app/public", 
            config('tenancy.filesystem.suffix_base'),
            $tenant->id));
        $link = str_replace('%tenant_id%', $tenant->id, config('tenancy.filesystem.url_override.public', 'public-%tenant_id%'));

        chdir(public_path());
        \Illuminate\Support\Facades\File::ensureDirectoryExists(dirname($target));
        \Illuminate\Support\Facades\File::link($target, $link);
        return $event->tenant;
    })->shouldBeQueued(false), // `false` by default, but you probably want to make this `true` for production.
],

Events\TenantDeleted::class => [
    JobPipeline::make([
        Jobs\DeleteDatabase::class,
    ])->send(function (Events\TenantDeleted $event) {
        $tenant = $event->tenant;
        $target = base_path(sprintf("storage/%s%s/app/public", 
            config('tenancy.filesystem.suffix_base'),
            $tenant->id));
        $link = str_replace('%tenant_id%', $tenant->id, config('tenancy.filesystem.url_override.public', 'public-%tenant_id%'));

        chdir(public_path());
        \Illuminate\Support\Facades\File::delete($link);
        \Illuminate\Support\Facades\File::deleteDirectory(dirname($target));
        return $event->tenant;
    })->shouldBeQueued(false), // `false` by default, but you probably want to make this `true` for production.
],

usage

$path = str_replace('public/', '', $path);
\Illuminate\Support\Facades\URL::tenantFile($path);

mouyong avatar May 02 '22 09:05 mouyong

Seems like I didn't resolve the conflicts correctly. Adding a note to add back some of the removed logic from here https://github.com/archtechx/tenancy/pull/689/commits/accbf5b3afacd48619f8b497716bf60026901c1e#diff-a7131a9582938c1b642f3d517a9064cbacb7b83d75fb52fe2c6e52dbca26a584

stancl avatar Jun 01 '22 14:06 stancl

The conflict resolution is a bit of a mess now. I'm thinking about forcibly reverting to the point before I merged master into this branch, and you'd try to resolve the conflicts there from scratch. Now it's a mix of my changes with your changes, and your changes still need to be modified (eg the review above). And there are more conflicts to resolve in other files.

Perhaps you could create a branch based on 5ed5aea6d602c84f14d646f242948ac5dcad7574 and try to resolve the conflicts there.

I'd like to avoid having excessive back & forth about the conflict resolution in this specific thread.

stancl avatar Jul 25 '22 16:07 stancl

I'll close this now in favor of #909

stancl avatar Aug 25 '22 18:08 stancl