Commit Graph
8 Commits
Author SHA1 Message Date
Jan-Hendrik BollandGitHub b2422452a1 Add GraphQL Extensions (#184)
This enables accessing the Extensions field, as defined in the response
format: https://spec.graphql.org/October2021/#sec-Response-Format

Extensions can be enabled using configuration option use_extensions.
This will change the return parameters of the generated client
functions. Making it a breaking change if enabled.

Since extensions are untyped as defined in the spec, the Client will
return an interface of type map[string]interface{}.
2022-03-30 13:48:01 -07:00
Adam BabikandGitHub 9fbb6b87aa Avoid capitalized error strings in generated code (#162)
Error strings should follow established guidelines
to ensure good composability and uniformity.
A mention of this particular guideline can be found
in Go Code Review Comments:
https://github.com/golang/go/wiki/CodeReviewComments#error-strings
2022-01-13 11:01:33 -08:00
Adam BabikandGitHub f0c2ac17a9 Move 'Code generated by' disclaimer up (#161)
`Code generated by` disclaimer should be at the top of the file, as it is demonstrated in [this article](https://go.dev/blog/generate) on The Go Blog.

The current placing breaks integration with some code formatters and linters which do no skip files generated by `genqlient`.
2021-12-20 10:04:13 -05:00
Ben KraftandGitHub 3a5bf46da4 Add getter methods to all fields, not just where needed (#126)
## Summary:
If your type implements an interface, we add getter methods for the
shared fields, so that those may be accessed via the interface.  But it
turns out occasionally it's useful to have these getter methods when
they don't implement a GraphQL interface, so you can use two
genqlient-generated types in the same function if they have the same
fields.  (This comes up most often when you have a GraphQL union that
maybe should really be an interface, or if you don't yet support
interfaces implementing other interfaces (indeed our parser doesn't
either).  But one can imagine other use cases.)

We can't predict how you want to do that, so we can't generate the
interface, but we can generate the methods, so you can define the
interface and do a type assertion from there.  Since these methods are
pretty simple to generate, we just do it always.  (As with #120, if
binary size becomes an issue we could later add an option to only
generate methods that are truly needed but including them seems like the
better default.)

This also fixes a subtle and rare bug, which would have become much more
common (indeed existing tests caught it).  Specifically, if you have a
query like
```graphql
fragment FragmentOne on T { id }
fragment FragmentTwo on T { id }
query Q {
    f {   # interface type T
        ...FragmentOne
        ...FragmentTwo
    }
}
```
since both `FragmentOne` and `FragmentTwo` request some common field,
say `id`, we generate a method `GetId` on each one.  But since
`FragmentOne` and `FragmentTwo` are both on `T`, we also include their
interfaces in the interface we generate for the type of `f`, `QFT`.  So
`QFT` includes a method `GetId`.  But on the implementations, the two
methods conflict, and neither gets promoted; this causes various code to
fail to compile.  With this change, this would have happened much more
frequently -- even if only one of the two fragments is on `T`, as long
as both request the field.  Anyway, we now generate explicit methods on
each struct for all of its recursively emebedded fields -- using the
logic from #120 to compute them -- so that we don't need to rely on
method-promotion.

## Test plan:
make tesc


Author: benjaminjkraft

Reviewers: csilvers, dnerdy, aberkan, jvoll, mahtabsabet, MiguelCastillo, StevenACoffman

Required Reviewers: 

Approved By: csilvers, dnerdy

Checks:  Test (1.17),  Test (1.16),  Test (1.15),  Test (1.14),  Lint,  Test (1.17),  Test (1.16),  Test (1.15),  Test (1.14),  Lint

Pull Request URL: https://github.com/Khan/genqlient/pull/126
2021-10-01 15:02:07 -07:00
Ben KraftandGitHub f4c981031e Allow genqlient types to be marshaled safely (#120)
## Summary:
When genqlient generates output types, it generates whatever code is
necessary to unmarshal them.  Conversely, when it generates input types,
it generates whatever code is necessary to marshal.  This is all that's
needed for genqlient itself: it never needs to marshal output types or
unmarshal input types.

But maybe you do!  (For example, to put the responses in a cache, which
is the use case that @csilvers hit at Khan, although there are others
one can imagine.)  While we can't support every serialization format you
might want (at least not without adding plugins or some such), it's not
unreasonable to expect that since genqlient can read JSON, it can write
it too.  Sadly, in the past this was not true for types requiring custom
unmarshaling logic, for several reasons.

In this commit I implement logic to always write both marshalers and
unmarshalers whenever they're needed to be able to correctly round-trip
the types, even though genqlient doesn't do so.  I wasn't starting from
scratch, since of course we already write both marshalers and
unmarshalers in some cases.  But this ended up requiring surprisingly
large changes on the marshaling side, mostly to correctly support
embedding (which we use for named fragments).

Specifically, as the comments in `types.go` discuss, the most difficult
issue is spreads with duplicate fields, which translate to Go embedded
fields which end up hidden from the json-marshaler.  Ultimately, I had
to do things quite differently from unmarshaling, and essentially
flatten the type when we write marshaler.  But in the end it's not so
ugly -- indeed arguably it's cleaner!  Mainly it's just different.

One thing to note is that we do marshal `__typename` based on
what we know about the types; users need not fill it in (and if they
do we'll ignore it).  This seemed to me to be a better UX, and
didn't add much complexity.

In general, I begin to wonder whether using `encoding/json` at all is
really right for genqlient: we're doing a lot of work to appease it,
despite knowing what our types look like.  I think it would still be a
significant increase in lines of code to roll our own, but that code
would perhaps be simpler, and would surely be faster (although if we
just want the speed gains we could use another JSON-generator library,
see also #47).  Anyway, something to think about in the future.

## Test plan:
make tesc


Author: benjaminjkraft

Reviewers: csilvers, StevenACoffman, benjaminjkraft, dnerdy, aberkan, jvoll, mahtabsabet, MiguelCastillo

Required Reviewers: 

Approved By: StevenACoffman, dnerdy

Checks:  Test (1.17),  Test (1.16),  Test (1.15),  Test (1.14),  Lint,  Test (1.17),  Test (1.16),  Test (1.15),  Test (1.14),  Lint

Pull Request URL: https://github.com/Khan/genqlient/pull/120
2021-09-29 10:30:43 -07:00
Ben KraftandGitHub 65c3e20ee6 Fix bugs relating to optional fields with custom (un)marshalers (#116)
## Summary:
There were a few bugs here, one of which Craig came across when pulling
the custom-unmarshaler change into webapp:
1. If you have an optional field with a custom unmarshaler, and the
   server omits the field from the response entirely (i.e. does not
   write `"myField": null`), we would still call your unmarshaler with
   an input of `[]byte(nil)`.  This is just wrong; it's our job to do
   the nil-check.  (This is the one Craig found; in practice gqlgen
   servers do not do this and I think the spec says not to although it's
   a bit fuzzy on the matter of serialization.  But in practice we have
   mocks that do it -- for required fields even! -- and it seems better
   to handle it than pass you data on which you'll probably err or even
   panic.)
2. If you have an optional field with a custom unmarshaler, and the
   server returns an explicit null (i.e. `"myField": null`), we would
   call your unmarshaler with `[]byte("null")`.  In principle the intent
   was you're supposed to implement that, as [`json.Unmarshaler`
   advises][1].  But (a) I forgot to document that, and (b) in practice
   `json.Unmarshal` [does *not* call you in that case][2], i.e. its
   advice is unnecessary.  So I think it's better for us to just match
   it, and not call you.  (And in that case I see no reason to bother
   documenting the advice.)
3. If you have an optional, `pointer: true` field with a custom
   marshaler, the reverse of (2) applies: if the pointer is nil, we
   shouldn't really call you.  (Indeed if you were a real
   `json.Marshaler` with a value-method rather than a pointer-method,
   trying to call you might panic!)  Note we don't need to explicitly
   write "null"; we just leave the `json.RawMessage` as nil, and
   `json.Marshal` [handles that][3].
4. We handle interface types effectively the same as custom
   unmarshalers, just we generate the unmarshaler.  So if you have an
   optional field with interface type, (1) would also apply there; our
   generated unmarshaler returns an error in this case.
5. While (2) doesn't apply to such optional interface fields (because we
   do the customary `if string(b) == "null"` check -- this I at least
   thought to test), if you set `pointer: true` on the field, we would
   still call the unmarshaler on the value, and it would no-op, but only
   *after* we initialized the pointer.  Put more simply, we'd return a
   non-nil pointer to nil interface, rather than a nil pointer; this is
   wrong since the whole point of `pointer: true` is you only get a
   non-nil pointer if your value is nil!  Of course, in practice there's
   little reason to use `pointer: true` on interface fields, and indeed
   this stuff gets so confusing my test was even wrong.

In this commit I fix all the bugs, by adding appropriate nil-checks to
wrap the unmarshaler-calls.  The templates are, as always, a bit
confusing, but the generated code makes it clear what changed.

Note we'll want to land this before cutting a release with custom
marshaler/unmarshaler support, because the first three bugs are
potentially quite noticeable.  (The latter two are in `v0.1.0`, but
presumably quite rare.)

[1]: https://pkg.go.dev/encoding/json#Unmarshaler
[2]: https://play.golang.org/p/Pw6zNN8trGO
[3]: https://play.golang.org/p/crTfnT7ePte

Issue: https://phabricator.khanacademy.org/D74453#inline-558571

## Test plan:
make tesc


Author: benjaminjkraft

Reviewers: csilvers, StevenACoffman, benjaminjkraft, aberkan, dnerdy, jvoll, mahtabsabet, MiguelCastillo

Required Reviewers: 

Approved By: csilvers, StevenACoffman

Checks:  Test (1.17),  Test (1.16),  Test (1.15),  Test (1.14),  Lint,  Test (1.17),  Test (1.16),  Test (1.15),  Test (1.14),  Lint

Pull Request URL: https://github.com/Khan/genqlient/pull/116
2021-09-27 20:35:39 -07:00
Ben KraftandGitHub 8de55d352e Add support for binding with a custom marshal/unmarshal function (#104)
## Summary:
This is useful if you want to bind to a type you don't control (or use
for other things) but need different serialization than its default.
This is a feature gqlgen has and we've found it very useful.  For
example, in webapp we want to bind `DateTime` to `time.Time`, but its
default serialization is not compatible with Python, so currently we
have to bind to a wrapper type and cast all over the place, which is
exactly the sort of boilerplate genqlient is supposed to avoid.

For unmarshaling, the implementation basically just follows the existing
support for abstract types; instead of calling our own generated
helper, we now call your specified function.  This required some
refactoring to abstract the handling of custom unmarshalers generally
from abstract types specifically, and to wire in not only the
unmarshaler-name but also the `generator` (in order to compute the right
import alias).

For marshaling, I had to implement all that stuff over again; it's
mostly parallel to unmarshaling (and I made a few minor changes to
unmarshaling to make the two more parallel).  Luckily, after #103 I at
least only had to do it once, rather than implementing the same
functionality for arguments and for input-type fields.  It was still
quite a bit of code; I didn't try to be quite as completionist about the
tests as with unmarshal but still had to add a few.

Issue: https://github.com/Khan/genqlient/issues/38

## Test plan:
make check


Author: benjaminjkraft

Reviewers: StevenACoffman, dnerdy, benjaminjkraft, aberkan, jvoll, mahtabsabet, MiguelCastillo

Required Reviewers: 

Approved By: StevenACoffman, dnerdy

Checks:  Test (1.17),  Test (1.16),  Test (1.15),  Test (1.14),  Lint,  Test (1.17),  Test (1.16),  Test (1.15),  Test (1.14),  Lint

Pull Request URL: https://github.com/Khan/genqlient/pull/104
2021-09-24 11:16:01 -07:00
Ben KraftandGitHub 5211442843 Add a new option to treat an interface like an object (#97)
## Summary:
The basic idea here is if you only request interface fields (no
fragments) you may not care about the concrete type, and so we could
just generate a struct as if it were an object.  I don't think it's a
good idea to do that by default, because then if you later add a
fragment all your code totally changes, but it's quite reasonable as an
option!

Most of the code involved is just wiring and validation; the
core implementation is literally just: treat it like an object.

Issue: https://github.com/Khan/genqlient/issues/85

## Test plan:
make check


Author: benjaminjkraft

Reviewers: csilvers, StevenACoffman, benjaminjkraft, aberkan, dnerdy, jvoll, mahtabsabet, MiguelCastillo

Required Reviewers: 

Approved By: StevenACoffman

Checks:  Test (1.17),  Test (1.16),  Test (1.15),  Test (1.14),  Lint,  Test (1.17),  Test (1.16),  Test (1.15),  Test (1.14),  Lint

Pull Request URL: https://github.com/Khan/genqlient/pull/97
2021-09-15 18:00:22 -07:00