Plugins about httplug HOT 19 CLOSED

Does a thing like validating a Request should also come as a plugin (i.e. protocol version is 1.0 and transfer encoding is set to chunked, which is not valid) ?

from httplug.

Absolutely. Plugins can stop executing the chain and return with an error/exception.

from httplug.

so we should add to this list: Convert error Responses to Exceptions.

from httplug.

Go ahead. 😀

David Buchmann [email protected] ezt írta (2015. szeptember 25.,
péntek):

so we should add to this list: Convert error Responses to Exceptions.

—
Reply to this email directly or view it on GitHub
#37 (comment).

from httplug.

following the discussion in #49, how do plugins work? do they wrap the client with an additional layer? then i would call them middleware, not plugin, to avoid wrong perception.

and how do we work with them? if my library expects exceptions for http errors, would i wrap the client into the exception converter middleware? will there be a way to discover what middlewares are in place? something like getWrappedClient or such?

from httplug.

Well, it is not fully planned yet.

One possibility would be to use the decorator pattern. In that case, you cannot know the original client as it might be way down deep in the object tree. Other downsides of this solution: Plugins would have implement sendRequest and sendRequests, in case of paralell execution, each plugin would have loop through the requests AND responses.

Ahother solution would be a PluginClient which accepts an HttpPsrClient and a set of Plugin instances. In this case the HttpPsrClient instance itself would be pushed as a plugin to the end of the stack.

Yet another solution: Have a separate PluginStack where you can push your plugins and at the and using a getClient method (and passig the actual client in it or in constructor?) you can build the middleware chain.

The advantage of the upper two solution is that plugins themselves can be used for both sendRequest and sendRequests as well.

Instead of middleware chaining we can also use a pipeline. In this case we have to separate RequestPlugins and ResponsePlugins and process them separately:
input request(s) -> process request(s) -> send request(s) -> process response(s) -> return response(s)

It's also an interesting question how we should proceed with sendRequests. What to do if a BatchException is thrown? How to process a BatchResult object?

from httplug.

I'm more in favor of the first solution with a middleware chaining (like stack) than a pipeline.

To help with the sendRequests methods we can provide an abstract plugin which handle the conventions need for the plugins and an implementation for the sendRequests / sendRequest methods like the following :

<?php

abstract class AbstractPlugin implements HttpPsrClient
{
    private $client;

    public function __construct(HttpPsrClient $client)
    {
        $this->client = $client;
    }

    public function sendRequest(RequestInterface $request, $options)
    {
        $request = $this->beforeSend($request, $options);
        $response = $this->client->sendRequest($request, $options);

        return $this->afterSend($response, $options);
    }

    public function sendRequests($requests, $options)
    {
        $newRequests = [];

        foreach ($requests as $request) {
            $newRequests[] = $this->beforeSend($request, $options);
        }

        $batchResult = $this->client->sendRequests($newRequests, $options);
        $newBatchResult = new BatchResult();

        foreach ($newRequests as $request) {
            $response = $this->afterSend($batchResult->getResponseFor($request), $options);
            $newBatchResult->addResponse($request, $response);
        }

        return $newBatchResult;
    }

    public function beforeSend(RequestInterface $request, $options)
    {
        return $request;
    }

    public function afterSend(ResponseInterface $response, $options)
    {
        return $response;
    }
}

from httplug.

👍

Actually I haven't thought about passing the options array to the plugin. Is it really necessary.

Also, I would modify the upper code a little: this shouldn't be a plugin abstraction, but a plugin client implementation which holds the two chains: a request processor and a response processor. Before send and after send hooks should call these chains.

from httplug.

Hum don't know about the options, it would be useful if we want different behavior for each request

(i.e. for a error to exception this can be a list of errors codes to transform, but this can be set in the constructor as well)

Also, I would modify the upper code a little: this shouldn't be a plugin abstraction, but a plugin client implementation which holds the two chains: a request processor and a response processor. Before send and after send hooks should call these chains.

So we will need to add an interface for a plugin and set them for this PluginClient ? I'm not a fan of this, i think there will be too many interface and a new interface for this is not useful, with this AbstractPlugin a Client is a Plugin with no delegation.

With a plugin only requiring to implements a HttpPsrClient we can also have different implementations for handling the BatchException / BatchResult from sendRequests.

from httplug.

With the upper solution there are two main problems:

1. Complex/Deep object tree caused by wrapping. Decorator pattern is good, but it is not that good.
2. Duplication. Not code duplication, but "execution duplication". Using 5 plugins means five iteration through a BatchResult object. It is not very efficient.

A separate plugin interface allows to concentrate on one thing: process one request or response and then return one. It is also easier to build the stack this way: you need to populate the builder with plugins instead of wrapping a client and plugins in each other.

from httplug.

Also, this interface would not be part of the main contract. It is a completely separate thing.

from httplug.

Should we provide 2 interface then, one for the request and one for the response (As majority of plugins will only interact on one of them) ?

Also 2 plugins i would like to add:

• Validator (like discussed before)
• Sanitizer (which will set correct headers depending on the request, like setting the content-length header, or setting the Transfer encoding to chunked if we cannot have a size for the body)

Also on the socket adapter i'm working one i want to support the chunked transfer encoding, and i'm wondering where this implementation have to be done:

• Do we assume if the user have set Trasnfer-Encoding to chunked then the body stream is already prepared to handle this ?
• Should the stream be handle in a plugin ?
• Should the adapter handle this (the current implementation) ?

from httplug.

Actually I think we should add three: Plugin, RequestPlugin, ResponsePlugin. It allows to add plugins in a single method using typehinting.

👍 for the sanitizer

In the HttpPsrClient you can assume that everything is correctly set in the request. However I think that adding extra checks is also a good idea. What SHOULD NOT be in clients is sanitizing logic. I think that in this case checking headers is not necessary. It will result in a failed request I guess. The less logic in clients the better it is.

from httplug.

I guess the plugin system is a blocker for getting 1.0 out, right?

• where do we define the 3 interfaces? in utils?
• should we add plugin mechanics to the HttpClient interface? i think not, setting the plugins up should be adapter specific. this will however pose a problem for discovery - what do we do there?
• for the exception (status code plugin) would the exceptions also live in utils? that would mean that libraries that use httplug and want to have those exceptions need to depend on utils as well. i can't see a reason why this would be bad, but wanted to raise the topic. however, is there a way for a library to control what plugins are active? can it investigate and raise an error if a plugin it needs is not active?
• i think once we have agreed on the architecture we should split this ticket into one per plugin (in the right repo) and start building them 1 by 1, rather than in a large pile of plugins...

from httplug.

I would separate plugins from the main repo. I don't think it is blocker as plugins are not related to the main contract.

My idea is to have a separate plugins repo, where interfaces and some basic implementations can live. When I was thinking about this the util repo was not even planned, so I am not sure about that. Exceptions are not the easiest question, as Client and Server exception might be useful in other cases, not just with the plugin. One concern: utility might become too large that way.

I think plugins should be a configuration detail: you should configure the client and pass plugins. From a technical point of view: we should have a PluginClient or something which accepts and implements an HTTPClient and accepts plugins.

I agree with the rest.

from httplug.

you added "Convert errror responses..." . what is the "Status code" plugin? is this not the same thing?

lets create a plugin repository then, which holds the interface, a middleware to apply the plugins and all plugins that are of general interest. this means it can also hold the client/server exceptions, no point in adding them to the main repository if they are never thrown. a library would either have to depend on the plugins or look at the response status code to decide what to do.

from httplug.

Yeah, they are the same.

👍

from httplug.

lets create a plugin repository then, which holds the interface, a middleware to apply the plugins and all plugins that are of general interest. this means it can also hold the client/server exceptions, no point in adding them to the main repository if they are never thrown. a library would either have to depend on the plugins or look at the response status code to decide what to do.

👍

from httplug.

Closing this in favor of the new plugins repository.

from httplug.