feat(graphql): support flattened response fields - #300
Conversation
1b090b1 to
a83ee02
Compare
|
fragment Metadata on IObject {
storageRebate
previousTransaction { digest }
}#[derive(Response)]
#[graphql(root_type = "Object")]
struct Object {
#[field(flatten)]
metadata: Metadata
}
#[derive(Response)]
#[graphql(root_type = "DynamicField")]
struct DynamicField {
#[field(flatten)]
metadata: Metadata
}
#[derive(Response)]
#[graphql(root_type = "IObject")]
struct Metadata {
#[field(path = "storageRebate")]
storage_rebate: UInt53
#[field(path = "previousTransaction.digest")]
previous_transaction: Digest
} |
The `Response` derive previously required every struct field to specify a
path. This made it awkward to compose reusable response projections that
read different fields from the same GraphQL response root.
Add `#[field(flatten)]` for fields that should receive the complete response
value instead of a narrowed value:
```rust
#[derive(Response)]
struct ChainInfo {
#[field(path = "chainIdentifier")]
chain_id: String,
}
#[derive(Response)]
struct ResponseData {
#[field(flatten)]
chain: ChainInfo,
}
```
The derive now generates a borrowed `extract(&serde_json::Value)` method and
keeps `from_value(serde_json::Value)` as an owned convenience wrapper.
Flattened fields recursively call `extract`, so multiple projections can
share the same response without cloning the entire JSON value. GraphQL union
responses use the same borrowed extraction path for their inner variants.
`flatten` and `path` are mutually exclusive; combining them produces a
compile-time error at the `flatten` attribute.
## Test Plan
Adds extraction coverage for multiple flattened fields receiving the same
root value, an `extract`-only field type, and a flattened field named `value`.
Adds a compile-fail test for combining `flatten` with `path`.
```
$ cargo test -p sui-graphql-macros
$ cargo clippy -p sui-graphql-macros --all-features --all-targets -- -D warnings
$ cargo test -p sui-graphql --no-run
```
tpham-mysten
left a comment
There was a problem hiding this comment.
Thanks @amnn!
The fragment use case is a good motivation for it. Just a thing that flatten is now not compile-time safety. If client defines an incompatible object like metadata: RandomObject, it can be failed in runtime. It seems an intended behavior from your test with ExtractOnly object.
I'm thinking it would be ideal to enforce a stricter rule on top of this (maybe in a follow-up PR): the type used in flatten must declare a root_type, and that root_type must be compatible with the parent root_type.
The idea is that from the parent root_type we can compute the list of compatible types from the schema (the type itself, any interface it implements, and any union it belongs to). Then we check which root_type the flattened type declares and validate it against that list.
If users want to opt-out to define a custom ExtractOnly object, they can use skip_schema_validation
I just asked Claude to put up a draft in #303 to show what this would look like.
WDYT?
|
Nice, yes, the extra validation looks good, thanks for that @tpham-mysten -- I've commented directly on that PR as well. |
Description
The
Responsederive previously required every struct field to specify a path. This made it awkward to compose reusable response projections that read different fields from the same GraphQL response root.Add
#[field(flatten)]for fields that should receive the complete response value instead of a narrowed value:The derive now generates a borrowed
extract(&serde_json::Value)method and keepsfrom_value(serde_json::Value)as an owned convenience wrapper. Flattened fields recursively callextract, so multiple projections can share the same response without cloning the entire JSON value. GraphQL union responses use the same borrowed extraction path for their inner variants.flattenandpathare mutually exclusive; combining them produces a compile-time error at theflattenattribute.Test Plan
Adds extraction coverage for multiple flattened fields receiving the same root value, an
extract-only field type, and a flattened field namedvalue. Adds a compile-fail test for combiningflattenwithpath.