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

Fix complex default value InputType introspection #537

Conversation

binaryseed
Copy link
Contributor

WIP

This PR is an attempt to fix the bug that exists when introspecting a "complex" input value, ie: one that doesn't respond nicely to to_string...

closes #490

@binaryseed binaryseed changed the title Test that demonstrates the bug Fix complex default value InputType introspection Apr 4, 2018
@binaryseed
Copy link
Contributor Author

The code I have now does a hacky thing that gets close...

The problem is that I don't have a really solid way to turn the input %{fancy_value: "qwerty"} into the GraphQL representation: {fancyValue: "qwerty"}

@binaryseed binaryseed force-pushed the fix-complex-input-introspection branch from 1b3004b to 5571ff7 Compare April 4, 2018 18:07
@binaryseed
Copy link
Contributor Author

I added a naive encoder to get the example test passing, but I don't think this is the right solution as-is

@binaryseed
Copy link
Contributor Author

Any thoughts on this @bruce or @benwilson512 ?

@bruce
Copy link
Contributor

bruce commented Apr 7, 2018

I’ll give this a look this weekend. Thanks, @binaryseed!

Copy link
Contributor

@benwilson512 benwilson512 left a comment

Choose a reason for hiding this comment

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

Thanks for taking a crack at this! Unfortunately it's a fair bit more complicated than this, since any custom scalars need to be serialized correctly, as do enums. Consequently it isn't enough to just walk through the Elixir value itself, you need to walk through the type information on the input simultaneously so that you can have serializers and so forth on hand to properly encode leaf nodes.

@binaryseed
Copy link
Contributor Author

Yeah, I thought something like that might be the case. This type of capability is essentially a code formatter.. rendering an AST.

Might be useful for future schema stitching work, to auto-generate queries for a backend graphql server. I may try to take a further crack at it soon..

@binaryseed
Copy link
Contributor Author

OK I found something interesting... There is already a function in absinthe that can totally render this stuff, it's Absinthe.Blueprint.Input.inspect.

The only problem is that it takes Absinthe.Blueprint.Input.Object type structs, whereas here we have Absinthe.Type.InputObject structs...

Any idea how I could leverage that function to do the rendering? Or should I basically clone that module for Absinthe.Type.*?

@binaryseed binaryseed force-pushed the fix-complex-input-introspection branch 2 times, most recently from a29feae to 0471f13 Compare April 30, 2018 05:59
@binaryseed
Copy link
Contributor Author

I found a simple way to re-use most of the code that existed, just calling it recursively to render input objects!

There may be other cases like lists that I might need to check out

@ghost
Copy link

ghost commented Aug 16, 2018

Any progress on that?

@binaryseed binaryseed force-pushed the fix-complex-input-introspection branch from 792fee5 to 284145d Compare August 17, 2018 03:02
@binaryseed
Copy link
Contributor Author

I updated this, it was rendering all the things properly, so I validated that in a test & cleaned up the code.

At this point I suspect that master has moved quite far ahead into 1.5, if we were to merge this would you make sure it wound up in the 1.4 branch @benwilson512 ?

"{#{object_values}}"

%Absinthe.Type.Field{type: type, name: name, identifier: identifier} ->
key = Absinthe.Utils.camelize(name, lower: true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Final note here and then we can merge. Instead of doing the camelize call here, pull the adapter out of the resolution struct on line 279: _, %{schema: schema, source: %{default_value: value, type: type}, adapter: adapter} -> and then pass it in to this function. With that you should be able to do:

key = adapter.to_external_name(name, :field)

This way it will honor adapter settings if people are using custom ones.

@binaryseed
Copy link
Contributor Author

Sweet, I used adapter.to_external_name.

Also I found the unwrap: option and used that explicitly so we can ensure that we properly handle any type of value inside a list...

@benwilson512 any other possible Absinthe.Type.* structs that I might need to handle?

@ghost
Copy link

ghost commented Sep 10, 2018

Is this for the next version?

@benwilson512
Copy link
Contributor

This looks great, thank you!

@benwilson512 benwilson512 merged commit 46e0c4d into absinthe-graphql:master Sep 25, 2018
@binaryseed binaryseed deleted the fix-complex-input-introspection branch June 25, 2019 19:12
binaryseed added a commit to binaryseed/absinthe that referenced this pull request Aug 9, 2019
* Test that demonstrates the bug

* Dummy implementation of a fix

* Add naive GraphQL encoder function

* Recursively render the default value

* Ensure we render all possible types & cleanup

* Use adapter to determine key

* Don't un-wrap, handle list / non_nul explicitly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introspecting complex default values raises Protocol.UndefinedError
3 participants