Allow scoped and local extensions

[bennothommo]

This allows one to create either a scoped global extension through static::extend(), which will apply to all newly-constructed instances of a class going forward, or they can also use $object->extend() to apply an extension to the single instance at run-time.

This allows access to protected and private methods within an instance, allowing full customisation of a given instance if it's allowed to be extendable.

[what-the-diff]

• Added support for scoped extensions.
• Fixed a bug where the extend() method was not being called on parent classes when extending from an extended class, which caused it to be ignored in some cases (e.g., Model).

[LukeTowers]

Could you add some usage examples?

[bennothommo]

@LukeTowers sure, so one example, which spurned this PR, was me trying to emulate the run method in the Backend\Classes\Controller method - specifically, this block: (https://github.com/wintercms/winter/blob/develop/modules/backend/classes/Controller.php#L219-L243) - within a behaviour.

Since the behavior is provided the controller as a $controller property, I can emulate anything that uses public methods and properties, but I can't use the protected $publicActions and $requiredPermissions properties of that controller.

For example - this won't work:

    public function apiRun(string $action, array $params = [])
    {
        $this->controller->action = $action;
        $this->controller->params = $params;
        $this->controller->suppressView = true;

        // Determine if the action is public
        $isPublicAction = in_array($action, $this->controller->publicActions);

        if (!$isPublicAction) {
            if ($this->controller->requiredPermissions && !$this->controller->user->hasAnyAccess($this->controller->requiredPermissions)) {
                return $this->errorResponse('Unable to access this API endpoint', 403);
            }
        }

With this PR in place, I can use a local extension and return the callback to determine authorisation:

    public function apiRun(string $action, array $params = [])
    {
        $this->controller->action = $action;
        $this->controller->params = $params;
        $this->controller->suppressView = true;

        // Determine if the action is public
        $allowed = $this->controller->extend(function () {
            $isPublicAction = in_array($action, $this->publicActions);

            if (!$isPublicAction) {
                if ($this->requiredPermissions && !$this->user->hasAnyAccess($this->requiredPermissions)) {
                    return false;
                }
            }
           
            return true;
        });

        if (!$allowed) {
              return $this->errorResponse('Unable to access this API endpoint', 403);
        } 

[bennothommo]

Another example, say I wanted to make a global extension, but needed access to some protected methods or properties, this would now be possible:

FormWidgetBase::extend(function () {
    // $valueFrom is protected, and has no getter in this class.
    if ($this->valueFrom === 'my_field') {
        $this->addDynamicMethod('someMethod', function () {
            // ....
        });
    }
}, true);

[bennothommo]

Just realised in the most recent tests that this also means that we can now define dynamic methods that can use protected or private methods and properties within.

[LukeTowers]

Hmm, with that example I'm not so sure that extend is the right name for that method, at least in the context of the use case of injecting the provided closure into the context of the object instance that extend() is being called on. Is that the only use case for that method or would there be other reasons to use it like that? Also what would ->extend() inside of an ::extend() look like / be used for / actually do?

[bennothommo]

@LukeTowers the name can certainly be worked on if there's any better ideas, although extend does hew close to class extends, which is what this functionality does on a smaller scale IMO.

My first example is an example of a global extension (ie. Class:extend()) moving through to a local extension (ie. $class->extend), since it's a snippet of a controller behavior that is adding an apiRun method that then needs to extend further into the controller that the behavior is attached to. Effectively, the local extension is just acting on the specific controller as opposed to acting on any controller.

[bennothommo]

@LukeTowers any further thoughts on the above?

[mjauvin]

Tested this with my use case where I need to have access to methods added by behavior and it works beautifully:

MyModel::extend(function () {
    $this->extend(function () {
        $this->behaviorMethodAvailable();
    });
}, true);

And it's amazingly nice to be able to use $this for both static and local closures!

[mjauvin]

This is so freaking powerful!!! This changes everything.

Amazing work @bennothommo!

[mjauvin]

What's also very nice is behavior is unchanged if Class::extend() is called only with a $callback.
Also, the error is much more clear when no $callback is provided:

before:

\Cms\Classes\Page::extend();

   ArgumentCountError  Too few arguments to function Winter\Storm\Extension\Extendable::extend(), 0 passed in /home/w2c/w2c.studioazura.comeval()'d code on line 1 and exactly 1 expected.

after:

\Cms\Classes\Page::extend();

   InvalidArgumentException  The extend() method requires a callback parameter or closure.

[LukeTowers]

Let's get the conflicts resolved and a quick docs PR up and then I can review

[bennothommo]

Ready to go @LukeTowers

[mjauvin]

Looks good, can't wait to use this!

[mjauvin]

Tested this with my use case where I need to have access to methods added by behavior and it works beautifully:

MyModel::extend(function () {
    $this->extend(function () {
        $this->behaviorMethodAvailable();
    });
}, true);

@bennothommo I successfully tested this before the PR got merged, but now it doesn't see the behavior methods...

[bennothommo]

@mjauvin I'd need to see the context of how you've implemented the extension.

[mjauvin]

@bennothommo here is an example:

public function boot(): void
{
    \StudioAzura\Aerocom\Models\Service::extend(function () {
        $this->extend(function () {
            $this->addTranslatableAttributes('myTranslatableField');
        });
    }, true);
}

The Service model implements the TranslatableModel behavior and should have the addTranslatableAttributes() method available at this point, but I get error:

Call to undefined method StudioAzura\Aerocom\Models\Service::addTranslatableAttributes()

I tested this exact case before the PR got merged, so I know it was working.

What am I missing ?

[bennothommo]

@mjauvin I'm not sure how it worked before 😕. Basically, the extension callbacks (ie. the static extend call) are called before the behaviors are loaded. All this PR did was enable the ability to use protected and private methods - it didn't change the ordering of initialisation.

[bennothommo]

Wait - I saw my comment in your PR that was to introduce the after option to run callbacks after the behaviors are loaded - your way above should work. I'll investigate this further.

[bennothommo]

@mjauvin This commit should fix it (a8ba4ee), however, I'm still curious as to how it worked before, because it should never have. Static extend calls are always fired before behaviors are loaded, because they provide the ability to dynamically add behaviors. Local extensions (ie $object->extend) fire immediately on the given object, which means when used in a static extend call, it would've fired before the behaviors too.

The commit above will delay local extensions until after the construction is complete, because that was always the intention of them to be available on "constructed" objects (like when used in run-time).

[mjauvin]

I don't know, but I'm 99% sure I tested this when I wrote the comment in this PR. Maybe I did something stupid thinking it worked

[mjauvin]

@bennothommo the problem is still there, behavior method is still not found.

[bennothommo]

@mjauvin are you sure? The new test case I introduced in that commit is testing your exact scenario?

[mjauvin]

@bennothommo pretty sure. Where is getFoo() defined ? Is it in a behavior implemented by the model ?

[mjauvin]

@bennothommo problem is your test class extends the Extendable class, mine extends the Model class (which uses the ExtendableTrait)