[RFC]: Remove laminas/laminas-eventmanager dependency

[Jurj-Bogdan]

RFC

Removal of the laminas/laminas-eventmanager dependecy or refactoring that still includes it?

Q	A
Proposed Version(s)	3.0.x
BC Break?	Yes

Description

Following the release of laminas-eventmanager:3.1.5, the EventManager class has been marked soft final. Currently it's being extended by ValidatorChain, which in turn is only used in the SessionManager and their respective tests. It doesn't seem to be used anywhere else.

In order not to extend the final class, what would you say the next step should be in version 3.0.x? Refactoring the ValidatorChain (maybe using EventManager as a dependecy, or moving its features elsewhere?) while keeping the laminas-eventmanager dependency, or removing it altogether?

[froschdesign]

I think it is completely irrelevant whether something is marked as final or not. Other questions need to be asked. Why is the event manager needed at all, and why is a chain for validators used with it? Why not rely on real validators instead and use the validator chain of laminas-validator?
Unfortunately, I don't know enough about this component to answer that question myself.

[Jurj-Bogdan]

Maybe i haven't worded the issue properly - the issue with "final" was just found when updating version 2 where it was baselined, that's why i referenced it.

I don't think the event manager is mandatory in this new version though, but I could also use some feedback.

[gsteel]

It seems that session validation is a core feature here, and, users can opt-in to a number of ready-made validators, and, create their own validators.

As far as I can tell, validation occurs at session startup, and there is a single validator chain that contains all the configured validators.

IMO there is no need to use EventManager to facilitate this setup. When SessionManager::isValid() is called, I'd just iterate over the composed validators and blow up as soon as one of them fails.

This may also provide an opportunity to simplify how the persistence and retrieval of the data necessary for session validation is handled, i.e. the before vs after comparisons that the validators rely on.

Considering validation failure is currently exceptional:

// SessionManager.php

if (! $this->isValid()) {
    throw new Exception\RuntimeException('Session validation failed');
}

I'd introduce a SessionValidationFailedException, and make it the responsibility of ValidatorInterface to throw it rather than returning bool.

Then your ValidatorInterface can be simplified to a single method:

/** @throws SessionValidationFailedException */
public function validate(Environment $previous, Environment $current): void;

and… maybe you don't need ValidatorChain at all, just:

// SessionManager.php

$previous = $this->getTheSerialisedEnvironmentFromTheSessionSomehow();
$current = $this->makeANewEnvironmentSomehow();

foreach ($this->validators as $validator) {
    $validator->validate($previous, $current);
}

The session manager already accepts a list<class-string<ValidatorInterface>> - instead of accepting class-string, require list<ValidatorInterface> and let service manager take care of fetching those instances.

No more state in validators - everything they need to do their job is given in the entry point validate method.

[froschdesign]

Is another validator interface in this component really needed? It is confusing when there is already a dependency on laminas-validator and then another interface for validators is defined. Or are other requirements needed for the session environment?

[gsteel]

Laminas\Validator is only used in the CSRF validator. There is already a Laminas\Session\ValidatorInterface here for session validation which is unrelated to Laminas\Validator

[froschdesign]

That is exactly what I mean. There are currently two validator interfaces. The question now is if there are plans to remove the dependency on laminas-validator and use a separate/unrelated interface in this component.
In the documentation will still point to laminas-validator: https://docs.laminas.dev/laminas-session/v2/validators/introduction/
And on the other side, in the documentation of laminas-validator: https://docs.laminas.dev/laminas-validator/v3/set/#additional-validators

[Jurj-Bogdan]

Currently the shipped interface Laminas\Session\ValidatorInterface is also used to force new validators to define initial and current Environment objects used to replace the previously hardcoded superglobals - this feature is not applicable to the Csrf validator though, which is why it still implements the interface from laminas-validator.

Looking over Csrf i don't seem to find any feature of laminas-validator being used that wouldn't be simple enough to implement directly. The main missing feature would be the "error message translation", but since that's not used for any of the other validators (neither in v2) i can assume it could be dropped, right?

This would be one of the two uses of laminas-validator in this package, the other being in StandardConfig.