upgrade to laravel 8

[tschallacka]

Based on issue:
wintercms/winter#23

All conflicts are resolved, all tests seem to pass without issue.
Some care is needed to review the wintercms implementation for the TransportAdapter as this has changed in Laravel 7 to MailAdapter. The tests pass, but integration may not, but I'm not familiar enough with the mail adapter to make an integration test.

[tschallacka]

Concerning #21 laravel 8 needs yaml 5.1 so the other branch referenced here should aim to optimize for 5.1.

[tschallacka]

@mjauvin a QueuedClosure is, https://github.com/laravel/framework/blob/29b019774b746c90534465f1b949767707d49245/src/Illuminate/Events/QueuedClosure.php#L6

[mjauvin]

@tschallacka ok, but I am refering to regular closures provided as an event listener. If the Object which handle the event gets cached (serialized), any closure used as the event handler will currently cause an error when trying to serialize the object.

[tschallacka]

Can you provide a unit test code that mimicks that? I'll see if I can do something about that.

[mjauvin]

@tschallacka you can try this test CMS page:

title = Closure Serialization Test Page 
url = /cache 
== 
function onInit()
{
    \Cms\Classes\Page::extend(function ($model) {
        $model->bindEvent('cms.page.display', function () {});
    });

    $page = new \Cms\Classes\Page([
        'fileName' => 'test-page',
        'title' => 'My Test Page',
        'url' => '/test-page',
    ]);

    Cache::put('my-cached-page-key', $page);
}
==
Hello World!

[tschallacka]

What a strange place to extend a model. It should be extended in the boot method of the plugin...

I'll have a look at it though.

[mjauvin]

You can apply this wherever you want, just a quick way to demonstrate

[mjauvin]

You can also test this in artisan tinker like this (easiest way):

# php artisan tinker
Psy Shell v0.10.8 (PHP 8.0.5 — cli) by Justin Hileman
>>> class MyTest extends \Model
... {
... }
>>> 
>>> MyTest::extend(function ($model) {
...     $model->bindEvent('cms.page.display', function () {});
... });
=> null
>>> $obj = new MyTest();
=> MyTest {#3787}
>>> Cache::put('my-obj-key', $obj);
Exception with message 'Serialization of 'Closure' is not allowed'
>>> 

If you comment out $model->bindEvent('cms.page.display', function () {}); you won't get the error

[tschallacka]

@mjauvin that's a good example I can put in a unit test. Thanks.

[tschallacka]

@mjauvin I've added some serialisation protection. Referenced variable connections via use (&$var) get severed during serialisation, but use ($var) persists.

@LukeTowers I've added a lot in the latest commits, see the commit message for a broad overview, if you have any questions just ask.

I still need to check Dispatcher for closures. Are there any other classes that work via closures that need to be serializeable?

[tschallacka]

Should I squash the commits? And apologies for forgetting to run the code quality check.

[LukeTowers]

Don't worry about squashing the commits, we take care of the history when we merge.

[github-actions]

This pull request will be closed and archived in 3 days, as there has been no activity in the last 60 days.
If this is still being worked on, please respond and we will re-open this pull request.
If this pull request is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

[LukeTowers]

Thanks bot for doing the job you're meant to do :)

I still need to take another look at this, but @bennothommo have you had a look yet?

[bennothommo]

@LukeTowers not really - was going to take a look when Laravel 9 was out.

[jumbophp]

ok luke ,
where do you think is the problem ? regading this PR ?

[LukeTowers]

@lzomedia our main list for tracking progress on this is available here: wintercms/winter#148. This PR is good enough to merge into our WIP branch at the moment though. Thanks @tschallacka for your work on this so far!