Soften the zend-stratigility dependency
jschreuder opened this issue · 14 comments
Currently zendframework/zend-stratigility is only necessary for the Middleware, which I'm not using personally. This could be softened to a suggested package and only in require-dev probably?
Props on the lib otherwise! Just checking it out and loving how it works.
Not removing the dependency unless another middleware specification/interface is provided (for which I'd add a different dependency).
Sorry, I do absolutely not do duck typing.
I wasn't suggesting duck typing, I was suggesting that you can use the lib perfectly without pulling in Stratigility. Which I'm doing right now. As such I am now pulling in something I don't use and is thus (in my humble opinion) a soft dependency. It is suggested when used together with it, but not required for the central lib classes to function.
So you could keep SessionMiddleware based on Stratigility while allowing others not to depend on it. And it really wouldn't matter as either (1) someone is already using Stratigility, or (2) someone won't use Stratigility and won't run into the dependency.
(though for those of us not using Stratigility it would also be nice if the Middleware was only a utility Trait with Stratigility specific implementation)
If I depend on the interface and it isn't included, the entire thing would
just crash
On 12 Sep 2016 15:12, "Jelmer Schreuder" notifications@github.com wrote:
I wasn't suggesting duck typing, I was suggesting that you can use the lib
perfectly without pulling in Stratigility. Which I'm doing right now. As
such I am now pulling in something I don't use and is thus (in my humble
opinion) a soft dependency. It is suggested when used together with it, but
not required for the central lib classes to function.So you could keep SessionMiddleware based on Stratigility while allowing
others not to depend on it. And it really wouldn't matter as either (1)
someone is already using Stratigility, or (2) someone won't use
Stratigility and won't run into the dependency.(though for those of us not using Stratigility it would also be nice if
the Middleware was only a utility Trait with Stratigility specific
implementation)—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#53 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAJakLzYcK9ibDJGa8mZELQYmRZiV-Hfks5qpU_QgaJpZM4J6jQq
.
Not if you don't use it? Or am I wrong and is it used in other classes than the SessionMiddleware class? I don't think I see it anywhere else but I may have missed something...
Nevermind, I've pretty much copy pasted the whole thing by now. I may code up a pull-request which makes this suggestion a bit more feasible. Refactoring most of the Middleware into a trait that works with PSR7/SetCookie. And then one implementation that links it to Stratigility but keeps other options open by using the trait.
This is security-sensitive code: do not copy-paste it unless you understand
all the details of the package, and are prepared to rewrite the copy-pasted
code every time we do a release.
Also, yes, the interface is used as simple type safety and BC boundary
enforcement.
If I didn't include stratigility, I would have had to provide my own
middleware interface, and then this would defeat the BC boundary, since the
purpose of requiring an external interface is to lock onto a specific
signature defined by @MWOP, who is the most knowledgeable person when it
comes to PSR-7.
Should stratigility change in major version bump, then we'd have to adapt
via a BC break, and that is intentional again.
On 12 Sep 2016 15:21, "Jelmer Schreuder" notifications@github.com wrote:
Nevermind, I've pretty much copy pasted the whole thing by now. I may code
up a pull-request which makes this suggestion a bit more feasible.
Refactoring most of the Middleware into a trait that works with
PSR7/SetCookie. And then one implementation that links it to Stratigility
but keeps other options open by using the trait.—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#53 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAJakNfLp-KDtgZtVDjNjdbHC_HTvoVBks5qpVHxgaJpZM4J6jQq
.
@Ocramius Is there any specific reason why Stratigility does not provide package with just the interface?
@jschreuder Maybe you should go to Stratigility folks and ask them to make separate Middelware interface package that Stratigility and PSR7Session can depend on.
@Isinlor I'm currently working on something which is pretty much copy-pasted from PHP-FIG's working document for PSR-15 which I prefer over the Stratigility interface. So I'm already preparing to move to an outside interface with multi-vendor support.
@jschreuder If I remember well you should be able to create adaptor for Stratigility interface to PSR-15.
Nevertheless I don't see a reason for PSR7Session and all other like-minded middelwares to depend on concrete implementation of Stratigility and not on the interface only. I will create an issue on Stratigility repo as they should be able to do this without BC.
@Ocramius Is there any specific reason why Stratigility does not provide package with just the interface?
not sure, but that's a good suggestion indeed! Thanks for opening an issue there, if you do :)
Meanwhile, closed/dead-end here: come back to this suggestion once a more widespread middleware specification is ready.
I will NOT allow duck-typing on my code.
I added it here: zendframework/zend-stratigility#64 ;)
I will NOT allow duck-typing on my code.
Maybe I'm not being clear in what I mean, but I never suggested duck-typing. I just suggested that Stratigility not being available to those who don't use it isn't bad. You don't have to always load all dependencies, some may be optional but fail when not loaded. That's what I meant and that is very much and in every way NOT duck typing.
My suggestion was and remains:
Extract everything from SessionMiddleware into a utility/service class that knows how to handle a ServerRequestInterface instance and how to post-process a ResponseInterface instance.
Then you create a SessionMiddleware implementation that has the Stratigility interface (optional for those who need it) and expects the utility object/trait as a dependency. Once you have that others also have the means to write other types of Middlewares without pulling in Stratigility.
That's what I meant and that is very much and in every way NOT duck typing.
That still makes it for a non-interfaced final class, which is just trouble.
Extract everything from SessionMiddleware into a utility/service class that knows how to handle a ServerRequestInterface instance and how to post-process a ResponseInterface instance.
Then you create a SessionMiddleware implementation that has the Stratigility interface (optional for those who need it) and expects the utility object/trait as a dependency. Once you have that others also have the means to write other types of Middlewares without pulling in Stratigility.
That's the path we went towards when designing the package last year, but in the end:
- it caused more autoloading
- it was actually slower (not by much, but we benchmarked the package)
- it ended up being bloated (we tried to separate all responsibilities, but some of them are coupled by temporal/sequential constraints)
- it was more buggy (removing abstraction locked some behavior to the current
finalimplementation) - it made the package more complicated to use (when security is involved, you don't want to give your users more rope)
Requiring the interface and then wrapping the middleware in your own implementation is brutally simple, let's not go down the overengineering way (introducing a lot of mistakes) just to remove an external interface, ending up adding a few internal ones.