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

Add introspection caching #6871

Merged
merged 2 commits into from
Feb 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ All notable, unreleased changes to this project will be documented in this file.

## [Unreleased]
- Add query contains only schema validation - #6827 by @fowczarek
- Add introspection caching - #6871 by @fowczarek

### Breaking
- Multichannel MVP: Multicurrency - #6242 by @fowczarek @d-wysocki
Expand Down
64 changes: 64 additions & 0 deletions saleor/graphql/core/tests/test_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
import graphene
import pytest
from django.test import override_settings
from graphql.execution.base import ExecutionResult

from .... import __version__ as saleor_version
from ....demo.views import EXAMPLE_QUERY
from ...tests.fixtures import (
ACCESS_CONTROL_ALLOW_CREDENTIALS,
Expand All @@ -13,6 +15,7 @@
API_PATH,
)
from ...tests.utils import get_graphql_content, get_graphql_content_from_response
from ...views import generate_cache_key


def test_batch_queries(category, product, api_client, channel_USD):
Expand Down Expand Up @@ -333,3 +336,64 @@ def test_query_contains_not_only_schema_raise_error(
assert graphql_log_handler.messages == [
"saleor.graphql.errors.handled[INFO].GraphQLError"
]


INTROSPECTION_QUERY = """
query IntrospectionQuery {
__schema {
queryType {
name
}
}
}
"""

INTROSPECTION_RESULT = {"__schema": {"queryType": {"name": "Query"}}}


@mock.patch("saleor.graphql.views.cache.set")
@mock.patch("saleor.graphql.views.cache.get")
@override_settings(DEBUG=False)
def test_introspection_query_is_cached(cache_get_mock, cache_set_mock, api_client):
cache_get_mock.return_value = None
cache_key = generate_cache_key(INTROSPECTION_QUERY)
response = api_client.post_graphql(INTROSPECTION_QUERY)
content = get_graphql_content(response)
assert content["data"] == INTROSPECTION_RESULT
cache_get_mock.assert_called_once_with(cache_key)
cache_set_mock.assert_called_once_with(
cache_key, ExecutionResult(data=INTROSPECTION_RESULT)
)


@mock.patch("saleor.graphql.views.cache.set")
@mock.patch("saleor.graphql.views.cache.get")
@override_settings(DEBUG=False)
def test_introspection_query_is_cached_only_once(
cache_get_mock, cache_set_mock, api_client
):
cache_get_mock.return_value = ExecutionResult(data=INTROSPECTION_RESULT)
cache_key = generate_cache_key(INTROSPECTION_QUERY)
response = api_client.post_graphql(INTROSPECTION_QUERY)
content = get_graphql_content(response)
assert content["data"] == INTROSPECTION_RESULT
cache_get_mock.assert_called_once_with(cache_key)
cache_set_mock.assert_not_called()


@mock.patch("saleor.graphql.views.cache.set")
@mock.patch("saleor.graphql.views.cache.get")
@override_settings(DEBUG=True)
def test_introspection_query_is_not_cached_in_debug_mode(
cache_get_mock, cache_set_mock, api_client
):
response = api_client.post_graphql(INTROSPECTION_QUERY)
content = get_graphql_content(response)
assert content["data"] == INTROSPECTION_RESULT
cache_get_mock.assert_not_called()
cache_set_mock.assert_not_called()


def test_generate_cache_key_use_saleor_version():
cache_key = generate_cache_key(INTROSPECTION_QUERY)
assert saleor_version in cache_key
50 changes: 38 additions & 12 deletions saleor/graphql/views.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import fnmatch
import hashlib
import json
import logging
import traceback
Expand All @@ -7,6 +8,7 @@
import opentracing
import opentracing.tags
from django.conf import settings
from django.core.cache import cache
from django.db import connection
from django.db.backends.postgresql.base import DatabaseWrapper
from django.http import HttpRequest, HttpResponseNotAllowed, JsonResponse
Expand All @@ -22,6 +24,7 @@
from graphql.execution import ExecutionResult
from jwt.exceptions import PyJWTError

from .. import __version__ as saleor_version
from ..core.exceptions import PermissionDenied, ReadOnlyException
from ..core.utils import is_valid_ipv4, is_valid_ipv6

Expand Down Expand Up @@ -213,14 +216,18 @@ def parse_query(
return None, ExecutionResult(errors=[e], invalid=True)

def check_if_query_contains_only_schema(self, document: GraphQLDocument):
query_with_schema = False
for definition in document.document_ast.definitions:
selections = definition.selection_set.selections
if len(selections) > 1:
for selection in selections:
selection_name = str(selection.name.value)
if selection_name == "__schema":
selection_count = len(selections)
for selection in selections:
selection_name = str(selection.name.value)
if selection_name == "__schema":
query_with_schema = True
if selection_count > 1:
msg = "`__schema` must be fetched in separete query"
raise GraphQLError(msg)
return query_with_schema

def execute_graphql_request(self, request: HttpRequest, data: dict):
with opentracing.global_tracer().start_active_span("graphql_query") as scope:
Expand All @@ -237,7 +244,9 @@ def execute_graphql_request(self, request: HttpRequest, data: dict):
raw_query_string = document.document_string
span.set_tag("graphql.query", raw_query_string)
try:
self.check_if_query_contains_only_schema(document)
query_contains_schema = self.check_if_query_contains_only_schema(
document
)
except GraphQLError as e:
return ExecutionResult(errors=[e], invalid=True)

Expand All @@ -249,14 +258,26 @@ def execute_graphql_request(self, request: HttpRequest, data: dict):
extra_options["executor"] = self.executor
try:
with connection.execute_wrapper(tracing_wrapper):
return document.execute( # type: ignore
root=self.get_root_value(),
variables=variables,
operation_name=operation_name,
context=request,
middleware=self.middleware,
**extra_options,
response = None
should_use_cache_for_scheme = query_contains_schema & (
not settings.DEBUG
)
if should_use_cache_for_scheme:
key = generate_cache_key(raw_query_string)
response = cache.get(key)

if not response:
response = document.execute( # type: ignore
root=self.get_root_value(),
variables=variables,
operation_name=operation_name,
context=request,
middleware=self.middleware,
**extra_options,
)
if should_use_cache_for_scheme:
cache.set(key, response)
return response
except Exception as e:
span.set_tag(opentracing.tags.ERROR, True)
return ExecutionResult(errors=[e], invalid=True)
Expand Down Expand Up @@ -365,3 +386,8 @@ def obj_set(obj, path, value, do_not_replace):
except IndexError:
pass
return obj_set(obj[current_path], path[1:], value, do_not_replace)


def generate_cache_key(raw_query: str) -> str:
hashed_query = hashlib.sha256(str(raw_query).encode("utf-8")).hexdigest()
return f"{saleor_version}-{hashed_query}"
1 change: 1 addition & 0 deletions saleor/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,7 @@ def get_host():
if REDIS_URL:
CACHE_URL = os.environ.setdefault("CACHE_URL", REDIS_URL)
CACHES = {"default": django_cache_url.config()}
CACHES["default"]["TIMEOUT"] = parse(os.environ.get("CACHE_TIMEOUT", "7 days"))

# Default False because storefront and dashboard don't support expiration of token
JWT_EXPIRE = get_bool_from_env("JWT_EXPIRE", False)
Expand Down