Skip to content

Conversation

driesvints
Copy link
Member

This will prevent dynamically adding routes when routes are cached.

@crynobone
Copy link
Member

crynobone commented Mar 6, 2020

Does this worked with registering route cache with routes only exists from an event?

https://github.com/laravel/nova/blob/3.0/src/PendingRouteRegistration.php#L82-L95

I don't see the two route available when you run php artisan route:list.

@deleugpn
Copy link
Contributor

deleugpn commented Mar 7, 2020

It's a shame to see this go away as it sacrifices a feature without any way around it.
I have an app A which sends email with signed URL. However the URL refers to routes on an app B.
We cannot sign routes without adding them to app A itself, but that route is invalid for A. So we dynamically add a Route and sign it before sending the email.
With this, we'll have to stop caching routes completley on app A, I guess?

@driesvints
Copy link
Member Author

@crynobone we're investigating that. It was solved by #31808 but Taylor wanted to look at a different approach. This will prevent things like in code snippet you linked to from happening.

@deleugpn but in that use case route caching isn't applicable to your app? You're adding the route dynamically so route caching can't help you there?

@deleugpn
Copy link
Contributor

deleugpn commented Mar 7, 2020

@driesvints the App A has all of it's routes cached. There's only one specific process which sends an email and when that process is executed, 1 route is 'dynamically added', but never actually called or used. To be honest, it would actually be easier if the URL Signed feature could be decoupled from the Router so that we could sign a route without actually having it on our app, but since that might be a lot more work, we get away with dynamically adding it.
We still take advantage of route cache.

@deleugpn
Copy link
Contributor

deleugpn commented Mar 7, 2020

    private function sendMail($name, $email, $username)
    {
        // We need to configure the exact same Route login has in order to sign the
        // request using the Laravel built-in temporary signed route feature.
        $this->router->domain($this->config->get('my-company.login-url'))->group(function (Router $router) {
            $router->name('sign-up')->get('/sign-up', null);
        });

        // UrlGenerator is a singleton on the Laravel container. We can clone it to avoid
        // modifying the original state of the framework to avoid conflicting with
        // changes that may happen at a later stage.
        $url = clone $this->url;

        $url->forceScheme('https');

        // Use the Login App key to sign the URL so that the login project
        // can validate whether the sign up URL is actually valid.
        $url->setKeyResolver(function () {
            return $this->config->get('my-company.login-app-key');
        });

        $signUpLink = $url->temporarySignedRoute(
            'sign-up',
            now()->addHours(24),
            ['username' => $username]
        );

        $mail = new Welcome($name, $email, $username, $signUpLink);

        Mail::send($mail);
    }

@driesvints driesvints deleted the throw-exception-for-add-method branch March 7, 2020 14:44
@driesvints
Copy link
Member Author

We're gonna try a different approach.

@driesvints
Copy link
Member Author

New attempt: #31829

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants