Laravel 5.5 MailQueue Contract Change

wells commented 7 years ago

Hey, I just wanted to point out a breaking change in laravel v5.5 that causes this package to no longer function. The signatures for at least the queue(), onQueue(), and queueOn() functions has changed from laravel v5.4.

In laravel v5.5 only Mailables can be queued now:

https://github.com/illuminate/mail/blob/5.5/Mailer.php#L351-L358

I believe the solution will be rather simple:

Update your composer.json in the next release to require ^5.5 of the illuminate dependencies.

"require": {
    "illuminate/mail": "^5.5",
    "illuminate/view": "^5.5"
},

Update the method signatures on MailThief object to match the MailQueue contract changes
Update any associated tests for queues to only test queueing Mailables

If you need any assistance with a PR, I'd be happy to help. However, it is hard to tell how active this repo is until I post this. So for now, I'll just resort to logging test emails and disable usage of this package.

calebporzio commented 7 years ago

Hey @wells - a PR would be wonderful! Thanks for bringing this to our attention

calebporzio commented 7 years ago

@wells - I updated composer.json to not allow laravel 5.5 because of that breaking change. If you want to PR the 5.5 change, I can tag a new, breaking release. - thanks!

wells commented 7 years ago

@calebporzio Sounds good. I'll see about getting a PR to you soon.

wells commented 7 years ago

I've got a branch on my fork of this repo with what I believe will be the necessary changes to this package for 5.5. The only thing is that I need is to get a pull request through on laravel/framework in order to expose some methods on the Mailable class as public.

Here's the particular section that will need these public methods exposed.

https://github.com/wells/mailthief/blob/patch-laravel-5.5/src/MailThief.php#L163-L167

Here's the full commit so far for mailthief.

https://github.com/wells/mailthief/commit/a3aa432c9c07b27c38743841f4c2bf6534218efd

On laravel/framework, I am requesting to make buildView() public and also to move the function calls in the Mailable send function out to a public prepareMessage() function. You can see the details below.

laravel/framework#20927

wells commented 7 years ago

I had to get clever and use reflection to call the methods I needed from the Mailable class using a ReflectionMethod trick. This only was necessary so we could get the mailable built up when the later() methods are called.

$buildView = new ReflectionMethod(Mailable::class, 'buildView');
$buildView->setAccessible(true);
$buildView->invoke($view);

Pull request soon to come.

calebporzio commented 7 years ago

@wells - thanks for all the hard work, for now, I just want to get this package to a state that doesn't hinder people upgrading to 5.5 - I think we are going to continue to not support Mailables in MailThief because Laravel supports Mailable faking out of the box. Thanks for the effort in that PR!

I tagged a new release that should be compatible with 5.5

wells commented 7 years ago

@calebporzio no worries. Unfortunately your update doesn't appear to be functioning.

First off, the update runs into the error that @psaunders reported, which is caused by the breaking change in phpunit 6.

Error: Call to a member function hijack() on null
vendor/tightenco/mailthief/src/Testing/InteractsWithMail.php:59

You'll want to update the hijack() function on InteractsWithMail.php to match what I had in my pull request for that file.

https://github.com/wells/mailthief/blob/57cf52ac71066e6ba7f5a0f5158a3206adfbe878/src/Testing/InteractsWithMail.php#L56-L62

/** @before */
public function hijackMail()
{
    $this->afterApplicationCreated(function() {
        $this->getMailer()->hijack();
    });
}

With your update, I actually saw the following error without that change to the hijack() function.

Illuminate\Contracts\Container\BindingResolutionException: 
Target [Illuminate\Contracts\View\Factory] is not instantiable while building 
[MailThief\MailThiefFiveFiveCompatible, 
MailThief\MailThiefFiveFiveCompatible, 
MailThief\MailThiefFiveFiveCompatible, 
MailThief\MailThiefFiveFiveCompatible, 
MailThief\MailThiefFiveFiveCompatible, 
MailThief\MailThiefFiveFiveCompatible, 
MailThief\MailThiefFiveFiveCompatible]

Second, once I updated the hijack() function I'm now receiving an error of null being returned from the lastMessage() function when I try to assert with seeMessage().

Unable to find a generated email.
Failed asserting that null is not null.

vendor/tightenco/mailthief/src/Testing/InteractsWithMail.php:164
vendor/tightenco/mailthief/src/Testing/InteractsWithMail.php:66

As to the cause, I believe that MailThief is no longer being instantiated as a singleton in your update... Looks like you need to update the service provider to register both new implementations as singletons.

class MailThiefServiceProvider extends ServiceProvider
{
    public function register()
    {
        if((float) $this->app->version() >= 5.5) {
            $this->app->singleton(MailThiefFiveFiveCompatible::class);
        } else {
            $this->app->singleton(MailThiefFiveFourCompatible::class);
        }
    }
}

calebporzio commented 7 years ago

Hey, @wells - thanks for all the solid info. I deleted the release until we figure this out.

I will get to this as soon as I can. If you want to PR these fixes in the meantime that would be great! If not, I'll try my best to get to them later today. Thanks!

tighten / mailthief

Laravel 5.5 MailQueue Contract Change #76