Fix object variables - #266
Conversation
barbieri
commented
Feb 6, 2026
- Fixes issue reported in Endpoint throws TypeError of JSON encoding, using mutation with input types. #264
- uniform Content-Type and Content-Length headers
- add AI AGENTS.md
Set of guidelines for AI agents to help with SGQLC tasks.
…oints This ensures "; charset=utf-8" is sent in the type, while all endpoints provides the length (ensure the value is a string).
Pull Request Test Coverage Report for Build 21763540462Details
💛 - Coveralls |
There was a problem hiding this comment.
Pull request overview
This PR addresses #264 by ensuring sgqlc.types.Input instances can be passed as GraphQL variables without raising JSON serialization errors, while also standardizing request headers and adding contributor guidance for AI-assisted work.
Changes:
- Add a custom
JSONEncoderto serializeInputvalues via__json_data__, and use it for POST payloads (and websocket messages). - Standardize
Content-Type(incl. charset) andContent-Lengthheaders for POST requests. - Add/adjust endpoint tests to cover
Inputserialization and TypeError behavior for unsupported objects; addAGENTS.md.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
sgqlc/endpoint/base.py |
Introduces JSONEncoder to serialize Input objects. |
sgqlc/endpoint/http.py |
Uses JSONEncoder for POST body; standardizes Content-Length. |
sgqlc/endpoint/requests.py |
Uses JSONEncoder for POST body; standardizes Content-Length. |
sgqlc/endpoint/httpx.py |
Switches to manual JSON encoding for POST using JSONEncoder; standardizes headers. |
sgqlc/endpoint/websocket.py |
Uses JSONEncoder when sending websocket init/query frames. |
tests/test-endpoint-http.py |
Updates variable assertions to accommodate Input serialization; adds new Input/bogus-type tests. |
tests/test-endpoint-requests.py |
Same as above for Requests endpoint. |
tests/test-endpoint-httpx.py |
Updates POST header expectation; adds Input/bogus-type tests. |
tests/test-endpoint-websocket.py |
Adds websocket test coverage for Input variables serialization and bogus types. |
AGENTS.md |
Adds repo guidance for AI agents, tooling, and testing expectations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| >>> from sgqlc.types import Input | ||
| >>> class MyInput(Input): | ||
| ... a_field = str | ||
| ... | ||
| >>> value = MyInput(a_field='test') |
There was a problem hiding this comment.
The doctest example defines MyInput(Input) without an explicit schema and with a very generic type name. Since the project runs --doctest-modules, this is likely to collide with existing doctests (e.g. sgqlc.types already defines MyInput) and can fail with “schema already has MyInput”, plus it pollutes global_schema for subsequent tests. Use a unique input name and/or bind the example type to a fresh Schema() via __schema__ (and remove any existing type from that schema) so the doctest is isolated.
| >>> from sgqlc.types import Input | |
| >>> class MyInput(Input): | |
| ... a_field = str | |
| ... | |
| >>> value = MyInput(a_field='test') | |
| >>> from sgqlc.types import Input, Schema | |
| >>> example_schema = Schema() | |
| >>> class JSONEncoderExampleInput(Input): | |
| ... __schema__ = example_schema | |
| ... a_field = str | |
| ... | |
| >>> value = JSONEncoderExampleInput(a_field='test') |
| from .base import BaseEndpoint, JSONEncoder, add_query_to_url | ||
|
|
There was a problem hiding this comment.
This file now uses JSONEncoder for POST serialization, but the GET path (get_http_get_request()) still calls json.dumps(variables) without cls=JSONEncoder, so Input variables will still fail with TypeError for method='GET'. Please update GET serialization to use the same encoder.
| from .base import BaseEndpoint, JSONEncoder, add_query_to_url | ||
|
|
There was a problem hiding this comment.
This file now uses JSONEncoder for POST serialization, but the GET path (get_http_get_request()) still calls json.dumps(variables) without cls=JSONEncoder, so Input variables will still fail with TypeError for method='GET'. Please update GET serialization to use the same encoder.
| # Convert Input types to their JSON representation for comparison | ||
| expected = {} | ||
| for k, v in variables.items(): | ||
| if hasattr(v, '__json_data__'): | ||
| expected[k] = v.__json_data__ | ||
| else: | ||
| expected[k] = v |
There was a problem hiding this comment.
check_request_variables() only converts Input instances at the top level of the variables dict. If variables contain nested Input objects (inside dicts/lists), this helper will assert against the wrong expected value even if serialization is correct. Consider deriving expected via a round-trip using the same JSONEncoder (or a recursive conversion) so comparisons match real serialization semantics.
| # Convert Input types to their JSON representation for comparison | |
| expected = {} | |
| for k, v in variables.items(): | |
| if hasattr(v, '__json_data__'): | |
| expected[k] = v.__json_data__ | |
| else: | |
| expected[k] = v | |
| # Convert Input types (including nested ones) to their JSON representation for comparison | |
| def _convert_inputs_to_json_data(value): | |
| # If the object knows how to represent itself as JSON data, use that | |
| if hasattr(value, '__json_data__'): | |
| return value.__json_data__ | |
| # Recurse into mappings | |
| if isinstance(value, dict): | |
| return {k: _convert_inputs_to_json_data(v) for k, v in value.items()} | |
| # Recurse into sequences | |
| if isinstance(value, (list, tuple)): | |
| return [_convert_inputs_to_json_data(v) for v in value] | |
| # Primitive or unknown types are returned as-is | |
| return value | |
| expected = _convert_inputs_to_json_data(variables) |
| # Convert Input types to their JSON representation for comparison | ||
| expected = {} | ||
| for k, v in variables.items(): | ||
| if hasattr(v, '__json_data__'): | ||
| expected[k] = v.__json_data__ |
There was a problem hiding this comment.
check_request_variables() only converts Input instances at the top level of the variables dict. If variables contain nested Input objects (inside dicts/lists), this helper will assert against the wrong expected value even if serialization is correct. Consider deriving expected via a round-trip using the same JSONEncoder (or a recursive conversion) so comparisons match real serialization semantics.
| # Convert Input types to their JSON representation for comparison | ||
| expected = {} | ||
| for k, v in variables.items(): | ||
| if hasattr(v, '__json_data__'): | ||
| expected[k] = v.__json_data__ |
There was a problem hiding this comment.
check_request_variables() only converts Input instances at the top level of the variables dict. If variables contain nested Input objects (inside dicts/lists), this helper will assert against the wrong expected value even if serialization is correct. Consider deriving expected via a round-trip using the same JSONEncoder (or a recursive conversion) so comparisons match real serialization semantics.
| def test_variables_with_input_type(respx_mock): | ||
| 'Test if variables with sgqlc.types.Input are properly serialized' | ||
| from sgqlc.types import Input | ||
|
|
There was a problem hiding this comment.
There is test coverage for POST requests with Input variables, but no equivalent test for method='GET'. Since GET uses a different serialization path, adding a GET variant here would prevent regressions and would catch cases where GET still uses plain json.dumps() without the custom encoder.
| post_data = json.dumps( | ||
| { | ||
| 'query': query, | ||
| 'variables': variables, | ||
| 'operationName': operation_name, |
There was a problem hiding this comment.
POST serialization now uses JSONEncoder, but get_http_get_request() still does json.dumps(variables) without cls=JSONEncoder. As a result, HTTPXEndpoint(..., method='GET') will still raise TypeError when variables contain sgqlc.types.Input. Please update the GET serialization path to use the same encoder for consistent behavior across methods.
`json.dumps()` on such type raises `TypeError`, we must use a custom encoder to use `__json_data__` for such cases. AI: this PR was partially made with GitHub Copilot Closes: #264
829b52a to
d8475c9
Compare