Skip to content
This repository has been archived by the owner on Jul 15, 2021. It is now read-only.

Mock filename tokenizer #3

Open
Ciloe opened this issue Dec 6, 2017 · 3 comments
Open

Mock filename tokenizer #3

Ciloe opened this issue Dec 6, 2017 · 3 comments

Comments

@Ciloe
Copy link

Ciloe commented Dec 6, 2017

csarrazi/CsaGuzzleBundle#208

Hi, I saw a problem with the fixture file name generated by this function :

src/Cache/NamingStrategy/AbstractNamingStrategy.php

protected function getFingerprint(RequestInterface $request)
    {
        return md5(serialize([
            'method' => $request->getMethod(),
            'path' => $request->getUri()->getPath(),
            'query' => $request->getUri()->getQuery(),
            'user_info' => $request->getUri()->getUserInfo(),
            'port' => $request->getUri()->getPort(),
            'scheme' => $request->getUri()->getScheme(),
            'headers' => array_diff_key($request->getHeaders(), array_flip($this->blacklist)),
        ]));
    }

It's appear that if wee requested a graphQl API, we use the body to send the request and not the query. Is it possible to add 'body' => $request->getBody(), in this array ?

I think a new naming strategy is a good idea. But what can we use in the strategy ? I think this header is the minimum

       [
            'method' => $request->getMethod(),
            'user_info' => $request->getUri()->getUserInfo(),
            'port' => $request->getUri()->getPort(),
            'scheme' => $request->getUri()->getScheme(),
            'headers' => array_diff_key($request->getHeaders(), array_flip($this->blacklist)),
        ]

But, with my tests I can see some problems.

  • If we use the body : 'body' => $request->getBody(). If the request has generic fragments and we change the fragment signature. The naming will change, and we will loose the last name. We need then to delete and regenerate all fixtures...
  • If we use a personal hash in the query : 'query' => $request->getUri()->getQuery() // has a specific hash for exemple, the route who call the api. If they are differents parameters, for a same route, the fixture will be the same.
  • If we use a personal hash in the query with parameters, it will be the same problem if we add a parameter.

What is the good solution for you ?

@csarrazi
Copy link
Owner

Hi @Ciloe, and sorry for the late answer. It seems that when you create a new repository on GitHub, you no longer automatically watch the repository, so you miss all notifications. My bad on that one.

Can you provide some call examples, so it is more clear?

What I think is that we should use all possible inputs for generating the hash. Indeed, it seems that in your case, you need all information from the URI + the actual body.
This could be passed as an option to the naming strategy, or we could create a BodyAwareNamingStrategy, which would take all parts of the URI + the body's content.

Do you think that would do the trick?

@Ciloe
Copy link
Author

Ciloe commented Jan 17, 2018

Hi !

For example we can call https://api.host.com/v1/ with the body

{
  userList (someoptions) {
    edges { node {
      name
      login
    }}
  }
}

The url is always the same, but the body change. I saied that

If we use the body : 'body' => $request->getBody(). If the request has generic fragments and we change the fragment signature. The naming will change, and we will loose the last name. We need then to delete and regenerate all fixtures...

So at the time, in my client, witch have defined url, I hash my url dans pass them in the query with CSA guzzle to call my api. This will create a unique naming stategy for the cache and the fixture file. But I think it's not a good approche.

What do you think ?

@csarrazi
Copy link
Owner

When speaking about GraphQL, a different body means a different signature, and thus should be considered a different request. I think we should take all parts of the URI + the body. This will let you remove the hashed URI from the query string. But in any case, passing parameters through the URI is still a valid. So in your naming strategy, I think you should use all parts of the request:

[
            'method' => $request->getMethod(),
            'path' => $request->getUri()->getPath(),
            'query' => $request->getUri()->getQuery(),
            'user_info' => $request->getUri()->getUserInfo(),
            'port' => $request->getUri()->getPort(),
            'scheme' => $request->getUri()->getScheme(),
            'headers' => array_diff_key($request->getHeaders(), array_flip($this->blacklist)),
            'body' => (string) $request->getBody()
        ]

Generally speaking, fixtures / mocks are highly dependent on the request's actual signature. So if you change a fragment, this means that you change at least a part of the query, meaning that you should indeed regenerate the associated mocks. This is the expected behaviour.

If you want to prevent regenerating all fixtures in case you just change a generic fragment, in this case you might want to have a more specific naming strategy, which would filter the body's contents, which would calculate a hash based on the significant part of your query.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants