Event system ZfcUser 3 seriously broken

[roelvanduijnhoven]

The way ZfcUser 3 works with events is seriously broken:

• Not all objects register themselves correctly to the SharedEventManager. All services should implement EventManagerAwareInterface in order for Zend\Mvc to inject it propertly (plus: there is a bug in the EventProvider that comes bundled: Fix broken event system #677).
• A lot of services attach and trigger events in their constructor. That has two problems:
  • They are not usuable. See RegisterFilter. The only way to use it would normally be the shared manager. However that is in ZF3 only injected after construction using an initializer.
  • They are overwritten. There is some business logic dependent on it in AdapterChainServiceFactory. If we were to properly mark the AdapterClass as a EventManagerAwareInterface capable service, the event manager would be overwritten directly after construction (again via the bundled Initializer of ZF3 in Mvc). And thus the adapters will never receive the authenticate event.
• The way events are triggered in AdapterChain is super weird. And is different from how it worked under ZF2. The event is passed as the $target parameter! What we need to do is use the triggerEvent method of the EventManager.

I have a PR that fixes all of this. See #677

[gkzsolt]

Thank you, roelvanduijnhoven, your PR fixed one of our problems when we migrated an app to ZF3. We hooked to the 'authenticate' event of the ZfcUser AdapterChain, but our callback was never called. Now this works. We still have another problem related.

We attached to the 'init' event of the ZfcUser\Form\LoginFilter, in order to reset the validator chain (we don't need password length >= 6). This was done in our app's onBootstrap($e) function with this code:

$events = $e->getApplication()->getEventManager()->getSharedManager();
$events->attach('ZfcUser\Form\LoginFilter','init', function($e)  {
            $form = $e->getTarget();
            // we reset password validation, to avoid 6 characters limit
            $form->get("credential")->setValidatorChain( new \Zend\Validator\ValidatorChain);
        });

, but unfortunately it never got called. Now, that you removed $this->getEventManager()->trigger('init', $this) from LoginFilter constructor, I wander how to do this. Can you please advice?

[roelvanduijnhoven]

@gkzsolt I used a delegator to do this.

class AdaptLoginFormDelegator implements DelegatorFactoryInterface
{
    public function __invoke(ContainerInterface $container, $requestedName, callable $callback, array $options = null)
    {
        $form = $callback();

        $form->getInputFilter()->remove('credential');

        return $form;
    }
}

@Danielss89 Any thought on this PR? Would be great to merge it!

[gkzsolt]

@roelvanduijnhoven, excellent idea, thank you !

I agree that this PR would be great to merge in. Any thoughts from the main developers?

[BrunoSpy]

Sadly this project seems abandoned.... Any news @Danielss89 ?

[Danielss89]

Sorry, i haven't had the time to look into this yet.
How many of you are using this patch already?

[roelvanduijnhoven]

@Danielss89 We have been running it on our production environment for some time; but I created this PR: so not the best reference material ;).

[jroedel]

It'd be awesome to get this working. I'm starting to feel left behind in ZF2, but I can't upgrade without this package. I wish I knew ZF3 event manager well enough to be able to look this over.

[gkzsolt]

@Danielss89 We are also using it in production, and if it's not merged, risk to not keep with the master. Is there a reason to not merge it?

[kingharrison]

Solid working ZfcUser is the main reason we are holding out upgrading to ZF3 from ZF2.

[roelvanduijnhoven]

What about merging this PR to a zf3 branch for the time being? And mark the default branch EOL. When we agree that zf3 branch is stable we can bump a new 4.0 release that is ZF3-only.

@kingharrison Did you test this patch already? If not: please do so we are sure we are not missing anything.

Also tests need to be revised on my PR. Anybody that wants to help with that?

[jarrettj]

👍

[kingharrison]

It looks to work for me. On May 22, 2018 at 8:45:28 AM, Roel van Duijnhoven ([email protected]) wrote: What about merging this PR to a zf3 branch for the time being? And mark the default branch EOL. When we agree that zf3 branch is stable we can bump a new 4.0 release that is ZF3-only. @kingharrison <https://github.com/kingharrison> Did you test this patch already? If not: please do so we are sure we are not missing anything. Also tests need to be revised on my PR. Anybody that wants to help with that? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#678 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AF1OcJ2u1u0HX9SI8hsHpBg9QcO3UUU_ks5t1AhmgaJpZM4Qre3o> .

[stijnhau]

Maybe this can be merged in now?