Middleware vs "server" middleware

[mindplay-dk]

I'd like to discuss options for resolving the weird situation with the empty interface.

An empty interface does not provides type-safety, since it does not define any behavior, capabilities or responsibilities. The empty interface is, in my view, a symptom that we're doing something wrong.

For example, type-hinting as MiddlewareInterface in a dispatcher does not provide auto-completion or inspections in an IDE; you still need if/instanceof-statements in dispatchers to determine the actual type of middleware - which is no different from the situation today - so the type-hint itself doesn't provide any type-safety at all.

In client/server-middleware, the proposed middleware-signature also does not provide type-safety for neither client nor server-requests, nor even for the dispatch-method, at all.

Here are some ideas.

OPTION 1: Single Interface

The obvious option is to accept the lack of type-hintability of server-specific middleware.

This isn't the same as ignoring it, and it's not necessary the wrong approach.

I've noticed, currently, my own server-middleware follows this pattern:

    public function __invoke(RequestInterface $request, callable $next = null)
    {
        if ($request instanceof ServerRequestInterface) {
            // ...
        }

        return $next($request);
    }

This doesn't seem wrong? I mean, in a world with a single middleware interface, for middleware that works both on the client and server, this seems the correct thing to do.

So I think one option, is to simply have only a single interface - consistent with what we have in the de-facto standard today.

OPTION 2: Separate Interfaces

Another option is to not have a third "general middleware" interface, at all.

The methods would need dedicated names:

interface ClientMiddlewareInterface {
    public function processClientRequest(RequestInterface $request, DelegateInterface $next);
}

interface ServerMiddlewareInterface {
    public function processServerRequest(ServerRequestInterface $request, DelegateInterface $next);
}

Note that the interface method-names are different. (using long method-names here to clarify for discussion, but one could rationalize shorter names like process for client-middleware and dispatch for server-middleware, if you can accept the idea that client-middleware doesn't actually dispatch requests per se - as opposed to server-middleware which is actually dispatches requests.)

Client and server middleware, respectively, would implement the relevant interface, and client/server-middleware would implement both interfaces. This approach eliminates the use of if/else-statements in middleware that supports both client and server.

In dispatchers, we would need to type-hint with php-doc only, but as argued above, the empty middleware-interface requires exactly the same run-time type-checks in dispatchers now - and it's an isolated problem anyhow, by which I mean, there are much fewer dispatcher than middleware implementations; the two if-statements required to type-check incoming middleware are not going to be a big problem for dispatchers - any type-related issue is a much bigger burden to middleware authors.

Footnote: if we have support for union types at some point, this problem will melt away.

OPTION 3: Explicit Third Interface

One last option, perhaps the most technically correct, is to have completely separate interfaces and method-names for all three cases, requiring middleware implementers to explicitly indicate whether a middleware component is client, server, or client/server-compatible; and in client/server-middleware, requiring them to explicitly implement dispatch for all three (not two) cases.

So the interfaces (very wordy here to further understanding) would look like:

interface ClientMiddlewareInterface {
    public function processClientRequest(RequestInterface $request, DelegateInterface $next);
}

interface ServerMiddlewareInterface {
    public function processServerRequest(ServerRequestInterface $request, DelegateInterface $next);
}

interface ClientServerMiddlewareInterface
    extends ClientMiddlewareInterface, ServerMiddlewareInterface
{}

Note the key difference from what we have now, is that the client/server-middleware interface actually has two methods, and requires the middleware authors to implement both methods explicitly - eliminating the if/instanceof run-time type-check in middleware.

I don't want to point at one of these options as my favorite - I do have a favorite, but I'd like to check the pulse on @http-interop/http-middleware-contributors before coloring your opinions.

Thoughts?

[shadowhand]

1. Pushes all validation to run time and depends entirely on the developer. Depending on how code paths work out, might mean the error is not immediately noticed.
2. Doesn't allow for mixing client/server middleware in a stack, unless the stack dispatch includes instanceof checks. This is an improvement over (1) but still not ideal.
3. Doesn't make sense because it creates an artificial separation between client and server code. This seems like a huge mistake to me, because client middleware is already compatible with server stacks, without advertising itself as such.

I prefer the current setup. It's easy to understand and doesn't make developers jump through hoops. It allows for an explicit type hint and loose type hint. And most importantly, it keeps a consistent method signature regardless of request type.

[mindplay-dk]

1 . Pushes all validation to run time and depends entirely on the developer

So does the current setup - an empty interface means nothing.

This is valid code:

class Foo implements MiddlewareInterface {}

So you need run-time type-checks in dispatchers.

Not to mention other types of totally unrelated middleware:

interface ParameterMiddleware implements MiddlewareInterface {
    /** @return mixed */
    public function dispatch(ReflectionParameter $param, $next);
}

That's not fiction, and implementing a middleware host that supports this as well as HTTP middleware (and any other kind of middleware you can think of) is real too.

My main point is that an empty interface implies I can extend that with whatever I want. Only that's not the case here.

2 . Doesn't allow for mixing client/server middleware in a stack, unless the stack dispatch includes instanceof checks

As said, it does, by type-hinting with php-doc and a union-type.

And as demonstrated above, we already need instanceof checks.

3 . Doesn't make sense because it creates an artificial separation between client and server code

Good point - that one is off the table.

I prefer the current setup. It's easy to understand and doesn't make developers jump through hoops

I think it's okay if dispatcher-developers have to jump through an extra hoop, so that middleware-developers do not?

most importantly, it keeps a consistent method signature regardless of request type.

So a consistent method signature for different types of middleware is important?

Because, in my opinion, that's just confusing, or even somewhat misleading - I should not be able to look at the signature or implements clause of a middleware component and immediately know from what whether it supports server-only, client-only, or client/server? Why not?

An empty interface can have meaning: what you're asserting, is that the object has no specific capabilities or responsibilities - i.e. is not expected or required to do anything specific.

That's not the assertion we're using it to make here.

In our case, objects pass only meaningless type-checks, e.g. making the sole assertion that middleware has no specific capabilities or responsibilities, which is false, which is why subsequent run-time type-checks are required, in some form or another.

If we had generics, we'd be done, but we don't, so we have to solve this type relationship with old-fashioned OO techniques - at least, to me, that's preferable to having a false interface and mandating run-time type-checks of implementers. It's an ugly work-around.

Either 1 or 2 still seem like cleaner options to me. In my opinion, we should either enforce constraints with declarations, or not enforce them at all - false type-checks add no value or safety, and requiring implementers to enforce constraints with code, in my opinion, is laying a shaky foundation.

[shadowhand]

So does the current setup - an empty interface means nothing.

Not in the context of a stack. Which, as you example points out, the interface would actually be useful. Why would using stack/dispatcher be a problem in that use case?

My main point is that an empty interface implies I can extend that with whatever I want. Only that's not the case here.

Indeed you could extend it with whatever you want. Being able to use stack/dispatcher with any type of middleware seems like a benefit, not a problem...?

As said, it does, by type-hinting with php-doc and a union-type.

Union type will only be supported in PHP >= 7.1. In my opinion, these interfaces need to be usable in PHP <7.0.

And as demonstrated above, we already need instanceof checks.

In some dispatching contexts, it might be good to have instanceof checks. I wouldn't say it is absolutely necessary though, since the user will be composing the middleware and will have knowledge of the types involved. Using instanceof would provide better type safety but also prevent novel uses of the interfaces.

I should not be able to look at the signature or implements clause of a middleware component and immediately know from what whether it supports server-only, client-only, or client/server? Why not?

You already can. Each client/server middleware must extend either server or client interface.

Also, you keep referencing "client-only" middleware, which doesn't exist. All client middleware is usable in a server context, but not vice versa, as server request always extends from client request.

Either 1 or 2 still seem like cleaner options to me. In my opinion, we should either enforce constraints with declarations, or not enforce them at all - false type-checks add no value or safety, and requiring implementers to enforce constraints with code, in my opinion, is laying a shaky foundation.

There are "false type-checks" defined within the current interfaces. Nowhere is there a type check against MiddlewareInterface in this package or in PSR-15. It exists only for stack/dispatch implementations to type hint against if they so choose. These implementations could also do instanceof checks or use union types for better type safety.

From my perspective, the current interfaces are not a "shaky foundation" but a highly flexible set of interfaces that will provide greater interop between middleware and dispatchers. Adding more restrictions will only reduce flexibility and reduce the possibility of novel usage.

[mindplay-dk]

For an interface to actually be a marker interface, all of its direct or indirect super interfaces must also be markers. If you extend a marker interface, by definition, it's not a marker anymore. So you're using the middleware interface as a kind of "poor man's type union", but that's not going to work - it won't accomplish anything, other than setting up a kind of artificial or mental barrier, or possibly causing confusion and raising questions with no clear answer.

Marker interfaces are used by other code to test for permission to do something, such as permission to serialize or cache objects, etc. - things that do not depend on having any specific methods, but this isn't that: you're using a marker interface to try to assert that an object does implement the methods of one of the super interfaces, but that's an unsafe assertion, because you cannot know which super interfaces exist, and someone could implement the empty interface itself.

I don't know how you think an empty marker interface helps interoperability. Of what? Objects that don't have any capabilities or responsibilities? The absence of any type-hint (and type) does that just as well.