Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: Caching on the server #125

Closed
pascalwacker opened this issue Aug 26, 2019 · 8 comments
Closed

Feature Request: Caching on the server #125

pascalwacker opened this issue Aug 26, 2019 · 8 comments
Labels
help wanted Extra attention is needed

Comments

@pascalwacker
Copy link

Hi, I was wondering, if it would be possible to add caching on the server side using i.e. Redis.

As far as I can tell it should be possible to implement your own caching in a controller, that's tagged with a @Query annotation, however you'll have to be careful, to get all the parameters for the cache key.
What I was thinking about, was to be able to register a cache pool with graphqlite and then telling it by a @Cache annotation, to cache the queries (or possible something like @Query(cache=true)). If nothing else is specified, I'd suggest to simply use the request url along with all parameters as cache key and a default cache time of 1 hour.
Through a config file you should be able to adjust the default cache time plus by setting a @Cache(time="xxx") parameter in the annotation. Additionally I'd add a parameter to that annotation to either include or exclude certain parameters from the cache key, as for example the preference for the UI (list view/table view) for a list gets sent along but doesn't influence the query results, so devs should be able to exclude these parameters.

Do you think this is doable and if so do you see any gains from implementing such a system?

@moufmouf
Copy link
Member

Hey @pascalwacker ,

Indeed, a special annotation like @Cache is something that could be done.
We added the ability to add custom annotations in GraphQLite 4: https://graphqlite.thecodingmachine.io/docs/next/field-middlewares

It should therefore be relatively easy to cache the query (and compute automatically the cache key from the method's parameters as you suggest).

That being said, there are a few caveats.
The main problem is that if your query returns objects, those objects must be cachable. If you put these objects in cache, you have to put the dependencies of the objects too (and the whole object graph that could be queried through GraphQL). I soon realized that serializing the objects to put in cache is the real hard part.

In my project, I found out it was actually way easier to add caching at the HTTP middleware level. Basically, I'm parsing the GraphQL query before GraphQLite is even started and I'm caching the query and the result. The result if a JSON string so I'm 100% sure it is cachable.

Of course, your mileage may vary though. I'd be curious to know what you think? Don't you think caching at the middleware level might be easier in your case?

@oojacoboo
Copy link
Collaborator

@moufmouf is this something that still needs to be considered? Are there any real advantages to adding this to GraphQLite over just manually handling your caching in your query logic?

@oojacoboo oojacoboo added the additional info needed Additional information is needed to proceed label Mar 29, 2021
@cuchac
Copy link

cuchac commented Aug 6, 2021

Hello,
from my point of view, there should be support inside the library. May be I'm wrong, but I think it is quite common use-case to have different caching policies for different queries in combined query. I can ask in one query for both "static" and "dynamic" content. For example: Give me currently logged-in user (dynamic) and main menu items (static).
Caching inside controller is unable to cope with this. Could middleware be able to do this caching of one query and avoid caching of another?

@oojacoboo
Copy link
Collaborator

What's the overall object here for caching? I'm assuming it's mostly around eliminating unnecessary database queries? If so, this is something you should handle with your ORM. Is the building out of the output types/fields for a query really that intensive for you to need to cache those?

@cuchac
Copy link

cuchac commented Aug 13, 2021

OK I have another use-case. I have web app and want to load all data from GraphQL endpoint. With decent traffic I have to use some caching technique. To initialize graphqlite library, parse query and serialize let's say 50 objects in each request is too much work to handle the load. I can cache on controller, I can cache on CDN (my prefered solution) I can cache on proxy.
I need a way to set caching policy (whether to cache or not, expiration time) based on parsed query - different queries have different expiration and caching. I'm unable to do this inside controller.
Maybe some caching middleware would be ideal for this task?

@oojacoboo
Copy link
Collaborator

oojacoboo commented Aug 14, 2021

Here is a request body parser that you can use. We use this for parsing requests in order to determine some of our authz. We use this as part of the middleware. But, you could use it in a controller via another service and the request object.

    /**
     * Parses the request body JSON and populates the request object's parsedBody property
     *
     * We're checking to see if the body has already been pre-parsed, as is happening with the upload
     * handler middleware.  It needs to add the uploadedFiles to the request.  The funny thing is that
     * it's using an "operations" key.  So, we're just getting rid of it essentially and using the "query"
     * key, since that's all we really need out of this right now anyway.
     *
     * @see https://github.com/Ecodev/graphql-upload/issues/7
     */
    public static function parseRequestBody(ServerRequestInterface $request): DocumentNode
    {
        $parsedBody = $request->getParsedBody();
        if (!$parsedBody) {
            $contents = $request->getBody()->getContents();
            $parsedBody = json_decode($contents, true);
            if ($parsedBody === false || json_last_error() !== \JSON_ERROR_NONE) {
                throw new BadRequest(json_last_error_msg() . ' in body: "' . $contents . '"');
            }
        }

        $operations = $parsedBody['operations'] ?? null;

        if ($operations) {
            $decoded = json_decode($operations, true);
            $query = $decoded['query'] ?? null;
        } else {
            $query = $parsedBody['query'] ?? null;
        }

        if (!$operations && !$query) {
            throw new UnprocessableEntity('GraphQL request must include query param');
        }

        try {
            $document = GraphQLParser::parse($query, [
                'noLocation' => true,
            ]);
        } catch (\Throwable $e) {
            throw ExceptionConverter::convert($e);
        }

        return $document;
    }

    $document = Helper::parseRequestBody($request);

    // If any field is not in our nonSecuredOpreations array, require Auth
    foreach ($document->definitions as $definition) {
        if (strtolower($definition->kind) !== 'operationdefinition') {
            continue;
        }

        foreach ($definition->selectionSet->selections as $field) {
            if (!in_array($field->name->value, $this->nonSecuredOperations)) {
                return true;
            }
        }
    }

@oojacoboo
Copy link
Collaborator

@cuchac I think we should add a property to the Query and Mutation annotations, similar to Symfony's Route annotation with stateless routes:

https://symfony.com/doc/current/routing.html#stateless-routes

@oojacoboo oojacoboo added help wanted Extra attention is needed and removed additional info needed Additional information is needed to proceed labels Jun 12, 2022
@oojacoboo oojacoboo pinned this issue Jun 12, 2022
@oojacoboo oojacoboo unpinned this issue Oct 7, 2023
@oojacoboo
Copy link
Collaborator

So, I think a better approach here is persisted queries, which has been implemented: https://graphqlite.thecodingmachine.io/docs/automatic-persisted-queries

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants