profusion / sgqlc

Simple GraphQL Client
https://sgqlc.readthedocs.io/
ISC License
513 stars 85 forks source link

Potential fix for issue 164 #210

Closed dbrumley closed 2 years ago

dbrumley commented 2 years ago

sgqlc/types/init.py:1947 to_json_value() may lose information when the underlying type is generic (e.g., RepositoryOwner), but the values are specific (e.g., User)

Example from github looking at a list of forks for a repository:

(Pdb) p self.json_data__[field.graphql_name] [{'typename': 'Repository', 'url': 'https://github.com/ivg/bap', 'owner': {'__typename': 'User', 'url': 'https://github.com/ivg', 'id': 'MDQ6VXNlcjIzMzY2OTg=', 'login': 'ivg', 'company': 'Carnegie Mellon University, Cylab', 'email': 'ivg@ieee.org', 'twitterUsername': 'ivg_t'}}] (Pdb) p json_value [{'url': 'https://github.com/ivg/bap', 'owner': {'id': 'MDQ6VXNlcjIzMzY2OTg=', 'login': 'ivg', 'url': 'https://github.com/ivg', 'email': 'ivg@ieee.org', 'company': 'Carnegie Mellon University, Cylab', 'twitterUsername': 'ivg_t'}}, {'url': 'https://github.com/maverickwoo/bap', 'owner': {'id': 'MDQ6VXNlcjE3MjY5Mw==', 'login': 'maverickwoo', 'url': 'https://github.com/maverickwoo', 'email': '', 'company': None, 'twitterUsername': None}}]

Note that json_value lost the fact that we knew the owner was a User. The current code will change it to the generic RepositoryOwner class.

This code very well could be a hack/wrong, but demonstrates the expected value to get when adding together two repos.

I ran into this with my own code base. I tried to create a minimal example to demo the issue, and attached it here as well. With the fix, d['data']['repository']['forks']['nodes'][0].owner still returns a RepositoryOwner, but this allows us to look at results and see the full type of an owner, e.g., User.

dbrumley commented 2 years ago

This PR seems to fix the issue where the type is incorrect, but does not fix the value where the values are incorrect.

BTW, I am looking at this because I am unable to serialize and then deserialize a repo's forks because of information lost.