guzzle / guzzle-services

Provides an implementation of the Guzzle Command library that uses Guzzle service descriptions to describe web services, serialize requests, and parse responses into easy to use model structures.
MIT License
253 stars 78 forks source link

Allow filters on response model #138

Closed danizord closed 7 years ago

danizord commented 7 years ago

Fixes https://github.com/guzzle/guzzle-services/issues/133

Konafets commented 7 years ago

@tmikaeld put the filters inside additionalProperties, your tests does not reflect this. Furthermore you can not just return a mixed value from $model->filter() when the expected return type is Result|ResultInterface|ResponseInterface.

tmikaeld commented 7 years ago

@danizord I guess you meant :)

Konafets commented 7 years ago

Nope, @tmikaeld in your example at #133 you put the filters in the additionalProperties section, but @danizord put it on level up. So the test is not testing your described issue.

tmikaeld commented 7 years ago

@Konafets Ah, ok, i tried both but none worked.

danizord commented 7 years ago

@Konafets not sure about @tmikaeld use case, but my use case is that I want to have a filter to "unwrap" the response result. If I place the filters in the additionalProperties section, it would run the filter for each scalar value, but I'd like to run the filter against the whole response model. Is it mergeable?

Konafets commented 7 years ago

IMO a filter is intended to apply to every value a filter is defined. Personally I never used filters so far, so I have no experience with it. What you try to do has to be done on a different way. Filters are the wrong way.

danizord commented 7 years ago

@Konafets but since the model is a Parameter itself, and the schema of models has a filters property, I think it makes sense to run these filters instead of simply ignoring them, no?

danizord commented 7 years ago

I may update the PR to pass and receive array instead of ResutInterface. What do you think?

tmikaeld commented 7 years ago

@danizord Why i wanted to run the filter in the first place was to convert the object to an array and in this case i think the most expected result is an array anyway.

Konafets commented 7 years ago

@danizord As I said before and the documentation + the doc header states, the filter is not intended to "unwrap" your response but "Run a value through the filters OR format attribute associated with parameter.".

Just because the filter method is there, it does not mean to use it in any possible way. Since Result uses the hasDataTrait you can get any data from your response by $result->offsetGet() or by implement an own Client and override the execute() method.

class MyCustomClient extends GuzzleClient
{
    /**
     * @param  CommandInterface $command
     * @return ResultInterface
     * @throws Exception
     */
    public function execute(CommandInterface $command) : ResultInterface
    {
        $name = $command->getName();
        $command->offsetSet('response', $this->getHttpClient()->getConfig('response'));
        try {
            $result = $this->executeAsync($command)->wait();
            $model = $this->getDescription()->getOperation($name)->getResponseModel();
        } catch (Exception $e) {
            // Catch all exceptions we did not covered before. These should be logged in any case.
            throw $e;
        }
        if (! empty($result) && $result->count() > 0) {
            $result = new Result($result->offsetGet($model));
        } else {
            $result = new Result();
        }
        return $result;
    }
}