Inconsistency in middleware docs

[pedzed]

A separate issue could be opened for roach-php/core, wherein code refactoring is suggested.

The documentation at https://roach-php.dev/docs/downloader-middleware states:

Downloader middleware that process outgoing requests before they get sent need to implement RequestMiddlewareInterface.

Then example code is provided,

use RoachPHP\Downloader\Middleware\RequestMiddlewareInterface;
use RoachPHP\Http\Request;
use RoachPHP\Support\Configurable;

class MyRequestMiddleware implements RequestMiddlewareInterface
{
    // ...
}

Note the namespace of RequestMiddlewareInterface.

Further down, the documentation states:

Downloader middleware that deal with responses need to implement ResponseMiddlewareInterface.

Then example code is provided,

use RoachPHP\Http\Response;
use RoachPHP\Spider\Middleware\ResponseMiddlewareInterface;
use RoachPHP\Support\Configurable;

class MyResponseMiddleware implements ResponseMiddlewareInterface
{
    // ...
}

Note again the namespace, this time of ResponseMiddlewareInterface.

Apparently there is:

use RoachPHP\Downloader\Middleware\RequestMiddlewareInterface;
use RoachPHP\Spider\Middleware\RequestMiddlewareInterface;

use RoachPHP\Downloader\Middleware\ResponseMiddlewareInterface;
use RoachPHP\Spider\Middleware\ResponseMiddlewareInterface;

Although there is the disclaimer that

Note that downloader request middleware gets run after spider request middleware. The exception to this are the initial requests, which don’t get sent through the spider middleware at all.

and that

Downloader response middleware gets run immediately after a response was received. This means it gets run before any spider response middleware.

(which I only saw upon writing this issue), I suggest making this less confusing. Perhaps a code refactoring should be done: why two interface namespaces if they could be simplified to one?

How I stumbled upon this, is my IDE suggesting that both namespaces could be used.

[ksassnowski]

I don't really see how these two namespaces can be consolidated into one. The respective middleware runs at a different part of the life cycle so they are definitely two separate things. This also would be a breaking change which I don't think is worth it.