{"action":"created","issue":{"url":"https://api.github.com/repos/http-interop/http-middleware/issues/16","repository_url":"https://api.github.com/repos/http-interop/http-middleware","labels_url":"https://api.github.com/repos/http-interop/http-middleware/issues/16/labels{/name}","comments_url":"https://api.github.com/repos/http-interop/http-middleware/issues/16/comments","events_url":"https://api.github.com/repos/http-interop/http-middleware/issues/16/events","html_url":"https://github.com/http-interop/http-middleware/issues/16","id":173643174,"number":16,"title":"Type-checking","user":{"login":"mindplay-dk","id":103348,"avatar_url":"https://avatars.githubusercontent.com/u/103348?v=3","gravatar_id":"","url":"https://api.github.com/users/mindplay-dk","html_url":"https://github.com/mindplay-dk","followers_url":"https://api.github.com/users/mindplay-dk/followers","following_url":"https://api.github.com/users/mindplay-dk/following{/other_user}","gists_url":"https://api.github.com/users/mindplay-dk/gists{/gist_id}","starred_url":"https://api.github.com/users/mindplay-dk/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mindplay-dk/subscriptions","organizations_url":"https://api.github.com/users/mindplay-dk/orgs","repos_url":"https://api.github.com/users/mindplay-dk/repos","events_url":"https://api.github.com/users/mindplay-dk/events{/privacy}","received_events_url":"https://api.github.com/users/mindplay-dk/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2016-08-28T12:02:55Z","updated_at":"2016-08-28T22:04:22Z","closed_at":null,"body":"I'm porting middleman to PSR-15, and I think we still have a type issue.\r\n\r\nHow do you check if a given value is middleware?\r\n\r\nCurrently, I have to do this:\r\n\r\n```php\r\nif ($value instanceof MiddlewareInterface || $value instanceof ServerMiddlewareInterface)\r\n```\r\n\r\nThat's ugly.\r\n\r\nI mean, strictly speaking, server-middleware is a specialized type of middleware - but our interfaces do not express this type-relationship, so you have to write two type-checks to find out if a value is middleware.\r\n\r\nThis indicates a pretty serious problem to me, communication-wise, semantically, etc. - if a type-check like that is necessary to check for what is actually a single pseudo-type, we're not using the type-system correctly.\r\n\r\nAlso, this is highly inconsistent with PSR-7, where `ServerRequestInterface extends RequestInterface` - it would be natural to expect a symmetry of these types, which means that `ServerMiddlewareInterface` should extend `MiddlewareInterface`.\r\n\r\nNow, this may not be ideal, but given the constraints of the type-system, I think that the following is technically the only correct way to model this type relationship with the PHP type system:\r\n\r\n```php\r\ninterface MiddlewareInterface {\r\n    public function process(RequestInterface $request, DelegateInterface $delegate);\r\n}\r\n\r\ninterface ServerMiddlewareInterface extends MiddlewareInterface {\r\n    public function processServer(ServerRequestInterface $request, DelegateInterface $delegate);\r\n}\r\n```\r\n\r\nWe can quibble about the method-names, but I don't think there's any good way to get around this - server middleware *is* a specialization of middleware, and it should be modeled that way.\r\n\r\nAnd yes, this means that server-middleware must implement both methods. This is not necessarily a bad thing - while many implementations of `process` will simply `return $this->processServer($request, $next)`, it removes the need to type-check, as this call will simply fail for server-only middleware:\r\n\r\n```php\r\nclass MyMiddleware implements ServerMiddlewareInterface {\r\n    public function process(RequestInterface $request, DelegateInterface $delegate) {\r\n        return $this->processServer($request, $delegate); // may error!\r\n    }\r\n\r\n    public function processServer(ServerRequestInterface $request, DelegateInterface $delegate) {\r\n        // ...\r\n    }\r\n}\r\n```\r\n\r\nNote that the marker interface does *not* solve this problem - if we have both types extend an empty interface, as we did previously, this still requires the exact same run-time type-checks; only now, if you also type-hint against the marker-interface, that makes the type-relationship even more illogical:\r\n\r\n```php\r\nfunction run(CommonMiddlewareInterface $middleware) {\r\n    if ($middleware instanceof MiddlewareInterface || $middleware instanceof ServerMiddlewareInterface) {\r\n        // ...\r\n    } else throw new RuntimeException(...);\r\n}\r\n```\r\n\r\nAs you can see, in practice, the marker interface changes nothing - and also, it still fails to provide symmetry with PSR-7, it just does so in a different way.\r\n\r\nThe problem is, what we have here is a generic type relationship, and there is no way to express that with the PHP type-system.\r\n\r\nI'm beginning to think we'd be better off with less type-constraints and a simpler middleware signature, e.g. something closer to the de-facto standard.\r\n\r\nI mean, we're struggling for type-safety here, and I am hugely in favor of type-safety, but it really seem like the PHP type-system cannot express this type-relationship, and whatever work-arounds we come up with, one way or the other, will inevitably be wrong. I think we're trying to do something that can't be done with PHP. Maybe we'd be better off accepting the fact that PHP is a dynamic language?\r\n\r\nWe could of course still type-hint statically with php-doc, but I'm no longer convinced we're going to get anywhere (good) by warping the PHP type-system concepts trying to bend it to our will.\r\n\r\nIt sucks, but, I tried to work with this for just a couple of hours, and, I already hate it. It obstructs rather than enables me. I never hated working with the de-facto standard and it never caused much trouble, even if it's very dynamic.\r\n\r\nAs a footnote, I also found that the existence of `DelegateInterface` creates obstruction - as I predicted in a comment earlier, it requires me to wrap callables in a meaningless object, which, internally, isn't type-safe anyhow:\r\n\r\n```php\r\nclass Delegate implements DelegateInterface\r\n{\r\n    /**\r\n     * @var callable\r\n     */\r\n    private $callable;\r\n\r\n    /**\r\n     * @param callable $callable\r\n     */\r\n    public function __construct($callable)\r\n    {\r\n        $this->callable = $callable;\r\n    }\r\n\r\n    public function next(RequestInterface $request)\r\n    {\r\n        return call_user_func($this->callable, $request);\r\n    }\r\n}\r\n```\r\n\r\nThere's no way around doing this, and I expect every middleware stack will need to do the same. (My first impulse was to have a delegate implementation that takes a reference to the stack, and an index - but without a `callable` somewhere in between, what you end up with in the middleware dispatcher, is immediate recursive creation of the entire middleware stack, which is not what you want, so there's no way around the callable, and therefore no way around the meaningless wrapper, as far as I can figure...)\r\n"},"comment":{"url":"https://api.github.com/repos/http-interop/http-middleware/issues/comments/243002380","html_url":"https://github.com/http-interop/http-middleware/issues/16#issuecomment-243002380","issue_url":"https://api.github.com/repos/http-interop/http-middleware/issues/16","id":243002380,"user":{"login":"shadowhand","id":38203,"avatar_url":"https://avatars.githubusercontent.com/u/38203?v=3","gravatar_id":"","url":"https://api.github.com/users/shadowhand","html_url":"https://github.com/shadowhand","followers_url":"https://api.github.com/users/shadowhand/followers","following_url":"https://api.github.com/users/shadowhand/following{/other_user}","gists_url":"https://api.github.com/users/shadowhand/gists{/gist_id}","starred_url":"https://api.github.com/users/shadowhand/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/shadowhand/subscriptions","organizations_url":"https://api.github.com/users/shadowhand/orgs","repos_url":"https://api.github.com/users/shadowhand/repos","events_url":"https://api.github.com/users/shadowhand/events{/privacy}","received_events_url":"https://api.github.com/users/shadowhand/received_events","type":"User","site_admin":false},"created_at":"2016-08-28T22:04:22Z","updated_at":"2016-08-28T22:04:22Z","body":"> How do you check if a given value is middleware?\r\n\r\nThis type check will be made simpler by union types.\r\n\r\n> I think that the following is technically the only correct way\r\n\r\nAnd also a nightmare to use. Having for fork in the dispatching based on the type of middleware _and_ in the delegate is just moving the `instanceof` check to a different place, it doesn't actually reduce the code required or make it easier to understand in any meaningful way.\r\n\r\nIt's also technically incorrect, in so far as the only reasonable implementation of `ServerMiddlewareInterface::process()` will be:\r\n\r\n```php\r\nthrow new RuntimeException('This middleware can only be used with server requests.');\r\n```\r\n\r\n> I also found that the existence of `DelegateInterface` creates obstruction - as I predicted in a comment earlier, it requires me to wrap `callables` in a meaningless object\r\n\r\nWhy would you use a callable? I expect the \"standard\" implementation of delegate to be something like the [Tari implementation](https://github.com/ircmaxell/Tari-PHP/blob/9d9512c767db557febc2a264f8390daf6b3c6517/lib/Server.php#L55-L74), where the delegate is an immutable object that takes the stack and clones itself as the it increments through the stack."}}
{"actor":{"display_login":"shadowhand"}}