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

GraphQL: Re-add extension and dependencies (2024) #554

Merged
merged 1 commit into from
Oct 31, 2024
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
7 changes: 5 additions & 2 deletions docs/source/tour.rst
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,17 @@ Here, you can spawn off a background thread to run any function, out-of-request:

GraphQL
-------
responder supports GraphQL::
Responder supports GraphQL, a query language for APIs that enables clients to
request exactly the data they need::

pip install 'responder[graphql]'

For more information about GraphQL, visit https://graphql.org/.

Serve a GraphQL API::

import graphene
from responder.ext.graphql import GraphQLView

class Query(graphene.ObjectType):
hello = graphene.String(name=graphene.String(default_value="stranger"))
Expand All @@ -50,7 +53,7 @@ Serve a GraphQL API::
return f"Hello {name}"

schema = graphene.Schema(query=Query)
view = responder.ext.GraphQLView(api=api, schema=schema)
view = GraphQLView(api=api, schema=schema)

api.add_route("/graph", view)

Expand Down
79 changes: 79 additions & 0 deletions responder/ext/graphql/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import json
from functools import partial

from graphql_server import default_format_error, encode_execution_results, json_encode

from .templates import GRAPHIQL


class GraphQLView:
def __init__(self, *, api, schema):
self.api = api
self.schema = schema

@staticmethod
async def _resolve_graphql_query(req, resp):
# TODO: Get variables and operation_name from form data, params, request text?
Copy link

@coderabbitai coderabbitai bot Oct 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Address the pending TODO: Extract variables and operation_name from additional sources

At line 16, there's a TODO comment indicating that variables and operation_name should be retrieved from form data, query parameters, or request text. Implementing this functionality will enhance compatibility with various GraphQL clients that send these fields in different ways.

Would you like assistance in implementing this feature?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for spotting. Deferred to...

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amotl, understood. Thank you for updating me that this will be addressed in GH-571.


if "json" in req.mimetype:
json_media = await req.media("json")
if "query" not in json_media:
resp.status_code = 400
resp.media = {"errors": ["'query' key is required in the JSON payload"]}
return None, None, None
return (
json_media["query"],
json_media.get("variables"),
json_media.get("operationName"),
)
amotl marked this conversation as resolved.
Show resolved Hide resolved

# Support query/q in form data.
# Form data is awaiting https://github.com/encode/starlette/pull/102
"""
if "query" in req.media("form"):
return req.media("form")["query"], None, None
if "q" in req.media("form"):
return req.media("form")["q"], None, None
"""
Comment on lines +32 to +37
Copy link

@coderabbitai coderabbitai bot Oct 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Remove or refactor the commented-out code within the triple-quoted string

Lines 28-33 contain code that is commented out inside a triple-quoted string. This code appears to handle extracting the GraphQL query from form data. To improve code clarity, consider one of the following:

  • Uncomment and integrate the code if form data support is required and the dependent feature in Starlette is now available.
  • Remove the commented code if it's obsolete or no longer needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added to GH-571.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amotl, understood. Thank you for adding this to GH-571.


# Support query/q in params.
if "query" in req.params:
return req.params["query"], None, None
if "q" in req.params:
return req.params["q"], None, None

# Otherwise, the request text is used (typical).
# TODO: Make some assertions about content-type here.
Copy link

@coderabbitai coderabbitai bot Oct 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Implement content-type assertions before processing req.text

At line 42, there's a TODO comment suggesting that assertions about the content-type should be made before using req.text. This is important to ensure that the server only processes requests with appropriate content types, enhancing security and robustness.

Would you like assistance in implementing content-type validation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dito.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amotl, schön zu hören! Wenn Sie weitere Unterstützung benötigen, lassen Sie es mich gerne wissen.

(_/)
(^.^)
(")(")

return req.text, None, None

async def graphql_response(self, req, resp):
show_graphiql = req.method == "get" and req.accepts("text/html")
Copy link

@coderabbitai coderabbitai bot Oct 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Ensure case-insensitive HTTP method comparison for reliability

At line 46, the code compares req.method == "get" to determine if the GraphiQL interface should be displayed. HTTP methods are typically uppercase ("GET"), so this comparison may fail. Consider modifying the comparison to account for this.

Apply this diff to fix the comparison:

-        show_graphiql = req.method == "get" and req.accepts("text/html")
+        show_graphiql = req.method.upper() == "GET" and req.accepts("text/html")

Alternatively, for a case-insensitive comparison:

-        show_graphiql = req.method == "get" and req.accepts("text/html")
+        show_graphiql = req.method.lower() == "get" and req.accepts("text/html")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
show_graphiql = req.method == "get" and req.accepts("text/html")
show_graphiql = req.method.upper() == "GET" and req.accepts("text/html")

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume Starlette will always relay them lower-case, otherwise this code would never have worked?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amotl, you are correct. In Starlette, the req.method attribute is always in lowercase, so the comparison req.method == "get" works as expected. Thank you for pointing that out.

(_?_)/


✏️ Learnings added
Learnt from: amotl
PR: kennethreitz/responder#554
File: responder/ext/graphql/__init__.py:46-46
Timestamp: 2024-10-31T04:36:00.272Z
Learning: In Starlette, `req.method` is always a lowercase string representing the HTTP method, so comparing it directly to a lowercase method name like `"get"` is valid.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


if show_graphiql:
resp.content = self.api.templates.render_string(
GRAPHIQL, endpoint=req.url.path
)
return None

query, variables, operation_name = await self._resolve_graphql_query(req, resp)
if query is None:
return None

context = {"request": req, "response": resp}
result = self.schema.execute(
query, variables=variables, operation_name=operation_name, context=context
)
Comment on lines +62 to +65
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add error handling for schema execution

The schema execution could raise exceptions that should be caught and handled gracefully.

     context = {"request": req, "response": resp}
-    result = self.schema.execute(
-        query, variables=variables, operation_name=operation_name, context=context
-    )
+    try:
+        result = self.schema.execute(
+            query, variables=variables, operation_name=operation_name, context=context
+        )
+    except Exception as e:
+        resp.status_code = 400
+        resp.media = {"errors": [{"message": str(e)}]}
+        return (query, None, 400)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
context = {"request": req, "response": resp}
result = self.schema.execute(
query, variables=variables, operation_name=operation_name, context=context
)
context = {"request": req, "response": resp}
try:
result = self.schema.execute(
query, variables=variables, operation_name=operation_name, context=context
)
except Exception as e:
resp.status_code = 400
resp.media = {"errors": [{"message": str(e)}]}
return (query, None, 400)

result, status_code = encode_execution_results(
[result],
is_batch=False,
format_error=default_format_error,
encode=partial(json_encode, pretty=False),
)
resp.media = json.loads(result)
Copy link

@coderabbitai coderabbitai bot Oct 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Avoid unnecessary JSON parsing

At line 65, result is already a JSON-formatted string after encoding. Parsing it back into a Python object with json.loads(result) only to have it serialized again by resp.media introduces redundant processing. Consider assigning the original result directly to resp.text and setting the appropriate content type.

Apply this diff to eliminate unnecessary parsing:

-        resp.media = json.loads(result)
+        resp.text = result
+        resp.headers["Content-Type"] = "application/json"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
resp.media = json.loads(result)
resp.text = result
resp.headers["Content-Type"] = "application/json"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deferred to #571 (comment).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amotl, understood. Thank you for letting me know that this suggestion has been deferred.

(_/)
( •_•)
💧<

return (query, result, status_code)

async def on_request(self, req, resp):
await self.graphql_response(req, resp)

async def __call__(self, req, resp):
await self.on_request(req, resp)
Comment on lines +78 to +79
Copy link

@coderabbitai coderabbitai bot Oct 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider removing the redundant on_request method

The __call__ method simply calls on_request, which in turn calls graphql_response. Since on_request does not add additional logic, you can streamline the code by having __call__ invoke graphql_response directly.

Apply this diff to simplify the class structure:

     async def __call__(self, req, resp):
-        await self.on_request(req, resp)
+        await self.graphql_response(req, resp)

Optionally, you may remove the on_request method entirely if it's not used elsewhere.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async def __call__(self, req, resp):
await self.on_request(req, resp)
async def __call__(self, req, resp):
await self.graphql_response(req, resp)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it is needed for other circumstances to have this method called on_request. I will not refactor this without consulting back with subject matter experts.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amotl, understood. I'll note that the on_request method may be needed for other circumstances.

🐰😞


✏️ Learnings added
Learnt from: amotl
PR: kennethreitz/responder#554
File: responder/ext/graphql/__init__.py:71-72
Timestamp: 2024-10-31T04:41:52.393Z
Learning: The `on_request` method in the `GraphQLView` class may be needed for other circumstances; avoid suggesting its removal without consulting subject matter experts.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

146 changes: 146 additions & 0 deletions responder/ext/graphql/templates.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
# ruff: noqa: E501
GRAPHIQL = """
{% set GRAPHIQL_VERSION = '0.12.0' %}
Copy link

@coderabbitai coderabbitai bot Oct 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Consider upgrading GraphiQL version.

The template uses GraphiQL v0.12.0 which is significantly outdated. The latest stable version is 3.0.1 which includes security fixes, performance improvements, and new features.

Apply this diff to update the version:

-{% set GRAPHIQL_VERSION = '0.12.0' %}
+{% set GRAPHIQL_VERSION = '3.0.1' %}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{% set GRAPHIQL_VERSION = '0.12.0' %}
{% set GRAPHIQL_VERSION = '3.0.1' %}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Deferred to....

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amotl, understood. We'll defer the GraphiQL version update to GH-569 as you've indicated. Let me know if there's anything else I can assist with.


<!--
* Copyright (c) Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the license found in the
* LICENSE file in the root directory of this source tree.
-->
<!DOCTYPE html>
<html>
<head>
<style>
body {
height: 100%;
margin: 0;
width: 100%;
overflow: hidden;
}
#graphiql {
height: 100vh;
}
</style>

<!--
This GraphiQL example depends on Promise and fetch, which are available in
modern browsers, but can be "polyfilled" for older browsers.
GraphiQL itself depends on React DOM.
If you do not want to rely on a CDN, you can host these files locally or
include them directly in your favored resource bunder.
-->
<link href="//cdn.jsdelivr.net/npm/graphiql@{{ GRAPHIQL_VERSION }}/graphiql.css" rel="stylesheet"/>
<script src="//cdn.jsdelivr.net/npm/[email protected]/fetch.min.js"></script>
<script src="//cdn.jsdelivr.net/npm/[email protected]/umd/react.production.min.js"></script>
<script src="//cdn.jsdelivr.net/npm/[email protected]/umd/react-dom.production.min.js"></script>
<script src="//cdn.jsdelivr.net/npm/graphiql@{{ GRAPHIQL_VERSION }}/graphiql.min.js"></script>
Comment on lines +34 to +38
Copy link

@coderabbitai coderabbitai bot Oct 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Security and compatibility improvements needed for external dependencies.

Several concerns with the current implementation:

  1. React/ReactDOM versions (16.2.0) are outdated
  2. CDN resources lack SRI (Subresource Integrity) hashes
  3. whatwg-fetch polyfill might be unnecessary for modern browsers

[security]
Apply these improvements:

-    <link href="//cdn.jsdelivr.net/npm/graphiql@{{ GRAPHIQL_VERSION }}/graphiql.css" rel="stylesheet"/>
-    <script src="//cdn.jsdelivr.net/npm/[email protected]/fetch.min.js"></script>
-    <script src="//cdn.jsdelivr.net/npm/[email protected]/umd/react.production.min.js"></script>
-    <script src="//cdn.jsdelivr.net/npm/[email protected]/umd/react-dom.production.min.js"></script>
-    <script src="//cdn.jsdelivr.net/npm/graphiql@{{ GRAPHIQL_VERSION }}/graphiql.min.js"></script>
+    <link 
+      href="https://cdn.jsdelivr.net/npm/graphiql@{{ GRAPHIQL_VERSION }}/graphiql.css" 
+      rel="stylesheet"
+      integrity="sha384-[GET-ACTUAL-HASH]"
+      crossorigin="anonymous"
+    />
+    <script 
+      src="https://cdn.jsdelivr.net/npm/[email protected]/umd/react.production.min.js"
+      integrity="sha384-[GET-ACTUAL-HASH]"
+      crossorigin="anonymous"
+    ></script>
+    <script 
+      src="https://cdn.jsdelivr.net/npm/[email protected]/umd/react-dom.production.min.js"
+      integrity="sha384-[GET-ACTUAL-HASH]"
+      crossorigin="anonymous"
+    ></script>
+    <script 
+      src="https://cdn.jsdelivr.net/npm/graphiql@{{ GRAPHIQL_VERSION }}/graphiql.min.js"
+      integrity="sha384-[GET-ACTUAL-HASH]"
+      crossorigin="anonymous"
+    ></script>

Note: Replace [GET-ACTUAL-HASH] with actual SRI hashes generated for each resource.

Committable suggestion was skipped due to low confidence.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deferred to #569 (comment), thanks again.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amotl, understood. Thank you for letting me know.

</head>
<body>
<div id="graphiql">Loading...</div>
<script>

/**
* This GraphiQL example illustrates how to use some of GraphiQL's props
* in order to enable reading and updating the URL parameters, making
* link sharing of queries a little bit easier.
*
* This is only one example of this kind of feature, GraphiQL exposes
* various React params to enable interesting integrations.
*/

// Parse the search string to get url parameters.
var search = window.location.search;
var parameters = {};
search.substr(1).split('&').forEach(function (entry) {
var eq = entry.indexOf('=');
if (eq >= 0) {
parameters[decodeURIComponent(entry.slice(0, eq))] =
decodeURIComponent(entry.slice(eq + 1));
}
});

// if variables was provided, try to format it.
if (parameters.variables) {
try {
parameters.variables =
JSON.stringify(JSON.parse(parameters.variables), null, 2);
} catch (e) {
// Do nothing, we want to display the invalid JSON as a string, rather
// than present an error.
}
}

// When the query and variables string is edited, update the URL bar so
// that it can be easily shared
function onEditQuery(newQuery) {
parameters.query = newQuery;
updateURL();
}

function onEditVariables(newVariables) {
parameters.variables = newVariables;
updateURL();
}

function onEditOperationName(newOperationName) {
parameters.operationName = newOperationName;
updateURL();
}

function updateURL() {
var newSearch = '?' + Object.keys(parameters).filter(function (key) {
return Boolean(parameters[key]);
}).map(function (key) {
return encodeURIComponent(key) + '=' +
encodeURIComponent(parameters[key]);
}).join('&');
history.replaceState(null, null, newSearch);
}

// Defines a GraphQL fetcher using the fetch API. You're not required to
// use fetch, and could instead implement graphQLFetcher however you like,
// as long as it returns a Promise or Observable.
function graphQLFetcher(graphQLParams) {
// This example expects a GraphQL server at the path /graphql.
// Change this to point wherever you host your GraphQL server.
return fetch('{{ endpoint }}', {
method: 'post',
headers: {
'Accept': 'application/json',
'Content-Type': 'application/json',
},
body: JSON.stringify(graphQLParams),
credentials: 'include',
}).then(function (response) {
return response.text();
}).then(function (responseBody) {
try {
return JSON.parse(responseBody);
} catch (error) {
return responseBody;
}
});
}

// Render <GraphiQL /> into the body.
// See the README in the top level of this module to learn more about
// how you can customize GraphiQL by providing different values or
// additional child elements.
ReactDOM.render(
React.createElement(GraphiQL, {
fetcher: graphQLFetcher,
query: parameters.query,
variables: parameters.variables,
operationName: parameters.operationName,
onEditQuery: onEditQuery,
onEditVariables: onEditVariables,
onEditOperationName: onEditOperationName
}),
document.getElementById('graphiql')
);
Comment on lines +105 to +142
Copy link

@coderabbitai coderabbitai bot Oct 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Improve error handling and update React rendering API.

Several improvements needed in the JavaScript implementation:

  1. ReactDOM.render is deprecated in React 18
  2. Fetch implementation lacks proper error handling
  3. Missing CSRF protection
  4. No request timeout handling

Apply these improvements:

       function graphQLFetcher(graphQLParams) {
-        return fetch('{{ endpoint }}', {
+        const controller = new AbortController();
+        const timeoutId = setTimeout(() => controller.abort(), 30000);
+        
+        return fetch('{{ endpoint }}', {
           method: 'post',
           headers: {
             'Accept': 'application/json',
             'Content-Type': 'application/json',
+            'X-CSRFToken': '{{ csrf_token }}',  // Add CSRF token
           },
           body: JSON.stringify(graphQLParams),
           credentials: 'include',
+          signal: controller.signal,
         }).then(function (response) {
+          if (!response.ok) {
+            throw new Error(`HTTP error! status: ${response.status}`);
+          }
           return response.text();
         }).then(function (responseBody) {
           try {
             return JSON.parse(responseBody);
           } catch (error) {
             return responseBody;
           }
+        }).catch(function (error) {
+          if (error.name === 'AbortError') {
+            throw new Error('Request timeout');
+          }
+          throw error;
+        }).finally(() => {
+          clearTimeout(timeoutId);
         });
       }

-      ReactDOM.render(
+      const root = ReactDOM.createRoot(document.getElementById('graphiql'));
+      root.render(
         React.createElement(GraphiQL, {
           fetcher: graphQLFetcher,
           query: parameters.query,
           variables: parameters.variables,
           operationName: parameters.operationName,
           onEditQuery: onEditQuery,
           onEditVariables: onEditVariables,
           onEditOperationName: onEditOperationName
-        }),
-        document.getElementById('graphiql')
+        })
       );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function graphQLFetcher(graphQLParams) {
// This example expects a GraphQL server at the path /graphql.
// Change this to point wherever you host your GraphQL server.
return fetch('{{ endpoint }}', {
method: 'post',
headers: {
'Accept': 'application/json',
'Content-Type': 'application/json',
},
body: JSON.stringify(graphQLParams),
credentials: 'include',
}).then(function (response) {
return response.text();
}).then(function (responseBody) {
try {
return JSON.parse(responseBody);
} catch (error) {
return responseBody;
}
});
}
// Render <GraphiQL /> into the body.
// See the README in the top level of this module to learn more about
// how you can customize GraphiQL by providing different values or
// additional child elements.
ReactDOM.render(
React.createElement(GraphiQL, {
fetcher: graphQLFetcher,
query: parameters.query,
variables: parameters.variables,
operationName: parameters.operationName,
onEditQuery: onEditQuery,
onEditVariables: onEditVariables,
onEditOperationName: onEditOperationName
}),
document.getElementById('graphiql')
);
function graphQLFetcher(graphQLParams) {
const controller = new AbortController();
const timeoutId = setTimeout(() => controller.abort(), 30000);
return fetch('{{ endpoint }}', {
method: 'post',
headers: {
'Accept': 'application/json',
'Content-Type': 'application/json',
'X-CSRFToken': '{{ csrf_token }}', // Add CSRF token
},
body: JSON.stringify(graphQLParams),
credentials: 'include',
signal: controller.signal,
}).then(function (response) {
if (!response.ok) {
throw new Error(`HTTP error! status: ${response.status}`);
}
return response.text();
}).then(function (responseBody) {
try {
return JSON.parse(responseBody);
} catch (error) {
return responseBody;
}
}).catch(function (error) {
if (error.name === 'AbortError') {
throw new Error('Request timeout');
}
throw error;
}).finally(() => {
clearTimeout(timeoutId);
});
}
const root = ReactDOM.createRoot(document.getElementById('graphiql'));
root.render(
React.createElement(GraphiQL, {
fetcher: graphQLFetcher,
query: parameters.query,
variables: parameters.variables,
operationName: parameters.operationName,
onEditQuery: onEditQuery,
onEditVariables: onEditVariables,
onEditOperationName: onEditOperationName
})
);

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't assess this evaluation, so I've deferred your admonition.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amotl, understood. Thank you for considering the suggestion and for deferring it to GH-570.

(•̥̥̥ _ •̥̥̥)

</script>
</body>
</html>
""".strip()
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def run(self):
"sphinxext.opengraph",
],
"full": ["responder[graphql,openapi]"],
"graphql": ["graphene"],
"graphql": ["graphene<3", "graphql-server-core>=1.2,<2"],
"openapi": ["apispec>=1.0.0"],
"release": ["build", "twine"],
"test": ["flask", "mypy", "pytest", "pytest-cov", "pytest-mock"],
Expand Down
38 changes: 38 additions & 0 deletions tests/test_graphql.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# ruff: noqa: E402
import pytest

graphene = pytest.importorskip("graphene")

from responder.ext.graphql import GraphQLView


@pytest.fixture
def schema():
class Query(graphene.ObjectType):
hello = graphene.String(name=graphene.String(default_value="stranger"))

def resolve_hello(self, info, name):
return f"Hello {name}"

return graphene.Schema(query=Query)


def test_graphql_schema_query_querying(api, schema):
api.add_route("/", GraphQLView(schema=schema, api=api))
r = api.requests.get("http://;/?q={ hello }", headers={"Accept": "json"})
assert r.status_code == 200
assert r.json() == {"data": {"hello": "Hello stranger"}}


def test_graphql_schema_json_query(api, schema):
api.add_route("/", GraphQLView(schema=schema, api=api))
r = api.requests.post("http://;/", json={"query": "{ hello }"})
assert r.status_code < 300
assert r.json() == {"data": {"hello": "Hello stranger"}}


def test_graphiql(api, schema):
api.add_route("/", GraphQLView(schema=schema, api=api))
r = api.requests.get("http://;/", headers={"Accept": "text/html"})
assert r.status_code < 300
assert "GraphiQL" in r.text
amotl marked this conversation as resolved.
Show resolved Hide resolved
Loading