Skip to content

Commit

Permalink
Fix abc.GitHubAPI.graphql() to accept response content types lacking …
Browse files Browse the repository at this point in the history
…spaces (#122)
  • Loading branch information
caseyduquettesc committed May 27, 2020
1 parent 7899dd8 commit d9eae33
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 6 deletions.
14 changes: 14 additions & 0 deletions docs/__init__.rst
Original file line number Diff line number Diff line change
Expand Up @@ -138,3 +138,17 @@ GraphQL-specific
An exception representing an error relating to the GraphQL query itself.

Inherits from :exc:`GraphQLException`.

.. exception:: GraphQLResponseTypeError(content_type, response)

An exception raised when a GraphQL query response is not JSON.

Inherits from :exc:`GraphQLException`.

.. attribute:: content_type

The value of the content-type header in the response from GitHub.

.. attribute:: response

The decoded response value from GitHub.
2 changes: 1 addition & 1 deletion docs/sansio.rst
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ without requiring the use of the :class:`Event` class.

.. attribute:: data

The `payload <https://developer.github.com/webhooks/#payloads>`_ of the
The `payload <https://developer.github.com/webhooks/event-payloads/>`_ of the
event.


Expand Down
12 changes: 11 additions & 1 deletion gidgethub/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
__version__ = "4.1.0"

import http
from typing import Any
from typing import Any, Optional


class GitHubException(Exception):
Expand Down Expand Up @@ -133,3 +133,13 @@ class QueryError(GraphQLException):

def __init__(self, response: Any) -> None:
super().__init__(response["errors"][0]["message"], response)


class GraphQLResponseTypeError(GraphQLException):

"""The GraphQL response has an unexpected content type."""

def __init__(self, content_type: Optional[str], response: Any) -> None:
super().__init__(
f"Response had an unexpected content-type: '{content_type!r}'", response
)
16 changes: 14 additions & 2 deletions gidgethub/abc.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
GraphQLAuthorizationFailure,
GraphQLException,
QueryError,
GraphQLResponseTypeError,
)
from . import sansio

Expand Down Expand Up @@ -239,8 +240,19 @@ async def graphql(
status_code, response_headers, response_data = await self._request(
"POST", endpoint, request_headers, request_data
)
assert response_headers["content-type"] == _json_content_type
response = json.loads(response_data.decode("utf-8"))

if not response_data:
raise GraphQLException("Response contained no data", response_data)

# Decode content.
resp_content_type = response_headers.get("content-type")
type_, encoding = sansio._parse_content_type(resp_content_type)
response_str = response_data.decode(encoding)
if type_ == "application/json":
response: Dict[str, Any] = json.loads(response_str)
else:
raise GraphQLResponseTypeError(resp_content_type, response_str)

if status_code >= 500:
raise GitHubBroken(http.HTTPStatus(status_code))
elif status_code == 401:
Expand Down
8 changes: 7 additions & 1 deletion gidgethub/httpx.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,13 @@ async def _request(
) -> Tuple[int, Mapping[str, str], bytes]:
"""Make an HTTP request."""
response = await self._client.request(
method, url, headers=dict(headers), data=body
method,
url,
# Reason: see discussion at https://github.com/brettcannon/gidgethub/pull/122#issuecomment-633738024.
# httpx 0.13.1 updated the HeaderTypes and it is acting invariant, causing our Mapping[str, str] to be
# technically incompatible with their Mapping[Union[str, bytes], Union[str, bytes]].
headers=dict(headers), # type: ignore
data=body,
)
return response.status_code, response.headers, response.content

Expand Down
41 changes: 41 additions & 0 deletions tests/test_abc.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
GraphQLException,
QueryError,
RedirectionException,
GraphQLResponseTypeError,
)
from gidgethub import abc as gh_abc
from gidgethub import sansio
Expand Down Expand Up @@ -804,3 +805,43 @@ async def test_unexpected_status_code(self):
with pytest.raises(GraphQLException) as exc:
await gh.graphql("does not matter")
assert exc.value.response == response_data

@pytest.mark.asyncio
async def test_response_content_type_parsing_gh121(self):
gh, response_data = self.gh_and_response("success-200.json")
# Test that a JSON content type still works if formatted without spaces.
gh.response_headers["content-type"] = "application/json;charset=utf-8"
# Should not fail.
resp = await gh.graphql("does not matter")
assert resp is not None
# Spaces in the content-type header should not be a problem.
gh.response_headers["content-type"] = "application/json; charset=utf-8"
# Should not fail.
resp = await gh.graphql("does not matter")
assert resp is not None

@pytest.mark.asyncio
async def test_unknown_response_content_type_gh121(self):
gh, response_data = self.gh_and_response("success-200.json")
# A non-JSON response should raise an exception.
gh.response_headers["content-type"] = "application/gidget;charset=utf-8"
with pytest.raises(GraphQLResponseTypeError):
await gh.graphql("does not matter")

@pytest.mark.asyncio
async def test_no_response_content_type_gh121(self):
gh, response_data = self.gh_and_response("success-200.json")
# An empty content type should raise an exception.
gh.response_headers["content-type"] = ""
with pytest.raises(GraphQLException):
await gh.graphql("does not matter")
del gh.response_headers["content-type"]
with pytest.raises(GraphQLException):
await gh.graphql("does not matter")

@pytest.mark.asyncio
async def test_no_response_data(self):
# An empty response should raise an exception.
gh = MockGitHubAPI(200, body=b"", oauth_token="oauth-token",)
with pytest.raises(GraphQLException):
await gh.graphql("does not matter")
2 changes: 1 addition & 1 deletion tests/test_tornado.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ async def test_sleep(self):
gh = gh_tornado.GitHubAPI("gidgethub")
await gh.sleep(delay)
stop = datetime.datetime.now()
assert (stop - start) > datetime.timedelta(seconds=delay)
assert (stop - start) >= datetime.timedelta(seconds=delay)

@tornado.testing.gen_test
async def test__request(self):
Expand Down

0 comments on commit d9eae33

Please sign in to comment.