neomerx / json-api

Framework agnostic JSON API (jsonapi.org) implementation
Apache License 2.0
743 stars 66 forks source link

Error while using BaseQueryParser and Encoder together #243

Closed efinder2 closed 4 years ago

efinder2 commented 4 years ago

I'm currently implementing an REST-API with your lib. I've tried to use the BaseQueryParser and the Encoder together. I've got an error in this line. Explode got as parameter an iterator.

I've struggled quite a bit. After some intense digging I've found the solution in the BasseQueryParserTest.

Is it possible to get these components compatible? In a first step it would be great if the phpdoc explains the issue. In a second step some one may alter the code so the components can work with each other without extending the BaseQueryParser.

Thanks in advance.

neomerx commented 4 years ago

Can you please post some code to illustrate how to get the error? I do not quite get how you succeed with crashing it :smile: Thanks

efinder2 commented 4 years ago

I've modified the existing testIntegrationWithEncodingParameters. The crashed will be caused if the result of get Includes isn't "escaped" by iterableToArray.

Attention the assert in the withIncludedPaths prevents it with tests. It only throws when the assert is removed.:

 public function testCombination()
    {
        $profileUrl1 = 'http://example1.com/foo';
        $profileUrl2 = 'http://example2.com/boo';

        $queryParameters = [
            BaseQueryParser::PARAM_FIELDS  => [
                'comments' => Comment::LINK_AUTHOR.',     '.Comment::ATTRIBUTE_BODY.'      ',
                'people'   => Author::ATTRIBUTE_FIRST_NAME,
            ],
            BaseQueryParser::PARAM_SORT    => '-created,title,+updated',
            BaseQueryParser::PARAM_INCLUDE => Comment::LINK_AUTHOR.',   '.
                Comment::LINK_AUTHOR.'.'.Author::LINK_COMMENTS,
            BaseQueryParser::PARAM_PROFILE => urlencode(implode(' ', [$profileUrl1, $profileUrl2])),
        ];
        $parser = new BaseQueryParser($queryParameters);

        //
        // Now the main purpose of the test. Will it work with EncodingParameters?
        //

        // firstly setup some data
        $author = Author::instance(9, 'Dan', 'Gebhardt');
        $comments = [
            Comment::instance(5, 'First!', $author),
            Comment::instance(12, 'I like XML better', $author),
        ];
        $author->{Author::LINK_COMMENTS} = $comments;

        // and encode with params taken from the parser
        $actual = Encoder::instance(
            [
                Author::class  => AuthorSchema::class,
                Comment::class => CommentSchema::class,
            ]
        )
            ->withIncludedPaths($parser->getIncludes())
            ->encodeData($comments);
    }
neomerx commented 4 years ago

Thanks. I'll have a look this weekend.

neomerx commented 4 years ago

I am in doubt. The parser actually parses inputs paths and returns them 'parsed'. However, withIncludedPaths expects a list of dot-separated paths. And each of them is right and some sort of adapter is needed. Technically, the solution is simple and instead of

    ->withIncludedPaths($parser->getIncludes())

it is required to

    ->withIncludedPaths($this->iterableKeys($parser->getIncludes()))

where

    private function iterableKeys(iterable $iterable): iterable {
        foreach ($iterable as $key => $value) {
            yield $key;
        }
    }

What are you thoughts on this?

efinder2 commented 4 years ago

Thanks for your response. Now I'm confused. I don't understand why you put iterableToArray() as parameter in the test, but above you explain it should be iterableKeys?

Also isn't it the sence of a parser to prepare the parameter for later usage in the fetching and rendering of the output? Where do someone need additional data of the getInclude method?

In general it would be helpful to have more documentation. PhpDoc and wiki entries. Is it ok for you, if I write some PhpDoc? How to help you with the wiki? It couldn't be cloned

neomerx commented 4 years ago

Sorry for confusion

I needed paths like post.comments.author to be split into ['post', 'comments', 'author'] so I could build database/ORM quires and load those relationships. Thus, the parser seemed to be a perfect candidate to do such parsing and have the parsed result as an array/Generator. However, I bumped the same problem as you, the Encoder wanted paths dot-separated because it needed to know which paths to follow and in what order (1 post, 2 post.comments and 3 post.comments.author in this case). So I decided to return both forms.

return [
    'post.comments.author' => ['post', 'comments', 'author'],
];

Though in a form of a Generator. Sorry, for being a bit complicated.

That is why taking keys works in this case. Technically, they are the same values as in the parser input though they are validated and guaranteed to have expected format, no trailing spaces and etc.

AFAIK, the Wiki can be edited by any logged-in Github user. Git repo is also available https://github.com/neomerx/json-api.wiki.git If you have any issues with the edit please post changes here and I will move them.

neomerx commented 4 years ago

Thanks. I think it would be better to move the compatibility note to https://github.com/neomerx/json-api/wiki/Parsing-API-Parameters#query-parameters and rather than referring to 'tests' and provide a usage sample.

neomerx commented 4 years ago

Released in 4.0.1