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

graphql: ObjectOwner::Parent exposed as Owner #19785

Merged
merged 3 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
359 changes: 332 additions & 27 deletions crates/sui-graphql-e2e-tests/tests/stable/call/dynamic_fields.exp

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -60,23 +60,56 @@ module Test::m {
//# create-checkpoint

//# run-graphql
{
{ # Initially, the parent object should be accessible directly and as an
# "Owner".
object(address: "@{obj_2_0}") {
dynamicFields {
nodes {
name {
type {
repr
}
type { repr }
data
bcs
}
value {
__typename
... on MoveValue {
type { repr }
bcs
data
}
... on MoveObject {
__typename
contents {
type { repr }
bcs
data
}
}
}
}
}
}

owner(address: "@{obj_2_0}") {
dynamicFields {
nodes {
name {
type { repr }
data
bcs
}
value {
__typename
... on MoveValue {
__typename
type { repr }
bcs
data
}
... on MoveObject {
contents {
type { repr }
bcs
data
}
}
}
}
Expand All @@ -89,50 +122,56 @@ module Test::m {
//# create-checkpoint

//# run-graphql
{
{ # After it is wrapped, we can no longer fetch its latest version via `object`
# but we can still refer to it as an "Owner" and fetch its dynamic fields.
object(address: "@{obj_2_0}") {
dynamicFields {
nodes {
name {
type {
repr
}
type { repr }
data
bcs
}
value {
... on MoveObject {
__typename
}
__typename
... on MoveValue {
__typename
type { repr }
bcs
data
}
... on MoveObject {
contents {
type { repr }
bcs
data
}
}
}
}
}
}
}

//# run-graphql
{
owner(address: "@{obj_2_0}") {
dynamicFields {
nodes {
name {
type {
repr
}
type { repr }
data
bcs
}
value {
... on MoveObject {
__typename
}
__typename
... on MoveValue {
type { repr }
bcs
data
__typename
}
... on MoveObject {
contents {
type { repr }
bcs
data
}
}
}
}
Expand All @@ -145,15 +184,14 @@ module Test::m {
owner(address: "@{obj_2_0}") {
dynamicField(name: {type: "u64", bcs: "AAAAAAAAAAA="}) {
name {
type {
repr
}
type { repr }
data
bcs
}
value {
... on MoveValue {
__typename
type { repr }
bcs
data
}
Expand All @@ -169,6 +207,11 @@ module Test::m {
value {
... on MoveObject {
__typename
contents {
type { repr }
bcs
data
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,9 @@ Response: {
"data": {
"object": {
"owner": {
"parent": null
"parent": {
"address": "0x9958869e2da5c5af828a334520a81de2bec0861e171b575db12fb4987cb27a78"
}
},
"dynamicFields": {
"nodes": []
Expand Down
8 changes: 5 additions & 3 deletions crates/sui-graphql-rpc/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -3076,11 +3076,13 @@ type PageInfo {

"""
If the object's owner is a Parent, this object is part of a dynamic field (it is the value of
the dynamic field, or the intermediate Field object itself). Also note that if the owner
is a parent, then it's guaranteed to be an object.
the dynamic field, or the intermediate Field object itself), and it is owned by another object.

Although its owner is guaranteed to be an object, it is exposed as an Owner, as the parent
object could be wrapped and therefore not directly accessible.
"""
type Parent {
parent: Object
parent: Owner
}

"""
Expand Down
7 changes: 3 additions & 4 deletions crates/sui-graphql-rpc/src/consistency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ pub(crate) fn build_objects_query(
// Similar to the snapshot query, construct the filtered inner query for the history table.
let mut history_objs_inner = query!("SELECT * FROM objects_history");
history_objs_inner = filter_fn(history_objs_inner);
history_objs_inner = filter!(history_objs_inner, "object_status = 0");
Copy link
Contributor

Choose a reason for hiding this comment

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

sry that i missed it here and thx for the fix, is there an easy way to generate a sample query here so that I can verify it on psql?

Copy link
Member Author

Choose a reason for hiding this comment

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

You didn't miss it in this case -- I've just moved the filter up into a central place, rather than applying at the end to each query, because it was ambiguous whether the object_status query would apply just to the historical candidates, the newer values or both (and we only want it to apply to the historical candidates).

If you run the GraphQL service with debug logs enabled, you can see every query that gets run. I ran the tests as follows for example:

sui$ RUST_LOG="info,sui_graphql_rpc::data::pg=debug" \
  cargo nextest run -p sui-graphql-e2e-tests --no-capture \
  -- call/dynamic_fields.move


let mut history_objs = match view {
View::Consistent => {
Expand All @@ -167,16 +168,14 @@ pub(crate) fn build_objects_query(
newer
);
history_objs = filter!(history_objs, "newer.object_version IS NULL");
history_objs = filter!(history_objs, "object_status = 0");
history_objs
}
View::Historical => {
// The cursor pagination logic refers to the table with the `candidates` alias
let query = query!(
query!(
"SELECT candidates.* FROM ({}) candidates",
history_objs_inner
);
filter!(query, "object_status = 0")
)
}
};

Expand Down
4 changes: 2 additions & 2 deletions crates/sui-graphql-rpc/src/types/coin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@ impl Coin {
}

/// The owner type of this object: Immutable, Shared, Parent, Address
pub(crate) async fn owner(&self, ctx: &Context<'_>) -> Option<ObjectOwner> {
ObjectImpl(&self.super_.super_).owner(ctx).await
pub(crate) async fn owner(&self) -> Option<ObjectOwner> {
ObjectImpl(&self.super_.super_).owner().await
}

/// The transaction block that created this version of the object.
Expand Down
4 changes: 2 additions & 2 deletions crates/sui-graphql-rpc/src/types/coin_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,8 @@ impl CoinMetadata {
}

/// The owner type of this object: Immutable, Shared, Parent, Address
pub(crate) async fn owner(&self, ctx: &Context<'_>) -> Option<ObjectOwner> {
ObjectImpl(&self.super_.super_).owner(ctx).await
pub(crate) async fn owner(&self) -> Option<ObjectOwner> {
ObjectImpl(&self.super_.super_).owner().await
}

/// The transaction block that created this version of the object.
Expand Down
4 changes: 2 additions & 2 deletions crates/sui-graphql-rpc/src/types/move_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,8 @@ impl MoveObject {
}

/// The owner type of this object: Immutable, Shared, Parent, Address
pub(crate) async fn owner(&self, ctx: &Context<'_>) -> Option<ObjectOwner> {
ObjectImpl(&self.super_).owner(ctx).await
pub(crate) async fn owner(&self) -> Option<ObjectOwner> {
ObjectImpl(&self.super_).owner().await
}

/// The transaction block that created this version of the object.
Expand Down
4 changes: 2 additions & 2 deletions crates/sui-graphql-rpc/src/types/move_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,8 +308,8 @@ impl MovePackage {

/// The owner type of this object: Immutable, Shared, Parent, Address
/// Packages are always Immutable.
pub(crate) async fn owner(&self, ctx: &Context<'_>) -> Option<ObjectOwner> {
ObjectImpl(&self.super_).owner(ctx).await
pub(crate) async fn owner(&self) -> Option<ObjectOwner> {
ObjectImpl(&self.super_).owner().await
}

/// The transaction block that published or upgraded this package.
Expand Down
32 changes: 16 additions & 16 deletions crates/sui-graphql-rpc/src/types/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,13 @@ pub(crate) struct Shared {
}

/// If the object's owner is a Parent, this object is part of a dynamic field (it is the value of
/// the dynamic field, or the intermediate Field object itself). Also note that if the owner
/// is a parent, then it's guaranteed to be an object.
/// the dynamic field, or the intermediate Field object itself), and it is owned by another object.
///
/// Although its owner is guaranteed to be an object, it is exposed as an Owner, as the parent
/// object could be wrapped and therefore not directly accessible.
#[derive(SimpleObject, Clone)]
pub(crate) struct Parent {
parent: Option<Object>,
parent: Option<Owner>,
}

/// An address-owned object is owned by a specific 32-byte address that is
Expand Down Expand Up @@ -439,8 +441,8 @@ impl Object {

/// The owner type of this object: Immutable, Shared, Parent, Address
/// Immutable and Shared Objects do not have owners.
pub(crate) async fn owner(&self, ctx: &Context<'_>) -> Option<ObjectOwner> {
ObjectImpl(self).owner(ctx).await
pub(crate) async fn owner(&self) -> Option<ObjectOwner> {
ObjectImpl(self).owner().await
}

/// The transaction block that created this version of the object.
Expand Down Expand Up @@ -580,7 +582,7 @@ impl ObjectImpl<'_> {
.map(|native| native.digest().base58_encode())
}

pub(crate) async fn owner(&self, ctx: &Context<'_>) -> Option<ObjectOwner> {
pub(crate) async fn owner(&self) -> Option<ObjectOwner> {
use NativeOwner as O;

let native = self.0.native_impl()?;
Expand All @@ -598,16 +600,14 @@ impl ObjectImpl<'_> {
}
O::Immutable => Some(ObjectOwner::Immutable(Immutable { dummy: None })),
O::ObjectOwner(address) => {
let parent = Object::query(
ctx,
address.into(),
Object::latest_at(self.0.checkpoint_viewed_at),
)
.await
.ok()
.flatten();

Some(ObjectOwner::Parent(Parent { parent }))
let address = SuiAddress::from(address);
Some(ObjectOwner::Parent(Parent {
parent: Some(Owner {
address,
checkpoint_viewed_at: self.0.checkpoint_viewed_at,
root_version: Some(self.0.root_version()),
}),
}))
}
O::Shared {
initial_shared_version,
Expand Down
4 changes: 2 additions & 2 deletions crates/sui-graphql-rpc/src/types/stake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,8 @@ impl StakedSui {
}

/// The owner type of this object: Immutable, Shared, Parent, Address
pub(crate) async fn owner(&self, ctx: &Context<'_>) -> Option<ObjectOwner> {
ObjectImpl(&self.super_.super_).owner(ctx).await
pub(crate) async fn owner(&self) -> Option<ObjectOwner> {
ObjectImpl(&self.super_.super_).owner().await
}

/// The transaction block that created this version of the object.
Expand Down
4 changes: 2 additions & 2 deletions crates/sui-graphql-rpc/src/types/suins_registration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,8 @@ impl SuinsRegistration {
}

/// The owner type of this object: Immutable, Shared, Parent, Address
pub(crate) async fn owner(&self, ctx: &Context<'_>) -> Option<ObjectOwner> {
ObjectImpl(&self.super_.super_).owner(ctx).await
pub(crate) async fn owner(&self) -> Option<ObjectOwner> {
ObjectImpl(&self.super_.super_).owner().await
}

/// The transaction block that created this version of the object.
Expand Down
8 changes: 5 additions & 3 deletions crates/sui-graphql-rpc/staging.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -3081,11 +3081,13 @@ type PageInfo {

"""
If the object's owner is a Parent, this object is part of a dynamic field (it is the value of
the dynamic field, or the intermediate Field object itself). Also note that if the owner
is a parent, then it's guaranteed to be an object.
the dynamic field, or the intermediate Field object itself), and it is owned by another object.

Although its owner is guaranteed to be an object, it is exposed as an Owner, as the parent
object could be wrapped and therefore not directly accessible.
"""
type Parent {
parent: Object
parent: Owner
}

"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3080,11 +3080,13 @@ type PageInfo {

"""
If the object's owner is a Parent, this object is part of a dynamic field (it is the value of
the dynamic field, or the intermediate Field object itself). Also note that if the owner
is a parent, then it's guaranteed to be an object.
the dynamic field, or the intermediate Field object itself), and it is owned by another object.

Although its owner is guaranteed to be an object, it is exposed as an Owner, as the parent
object could be wrapped and therefore not directly accessible.
"""
type Parent {
parent: Object
parent: Owner
}

"""
Expand Down Expand Up @@ -4801,3 +4803,4 @@ schema {
query: Query
mutation: Mutation
}

Loading
Loading