fixup: package paginate by version filter

MystenLabs · Aug 16, 2024 · 8d5c7e8 · 8d5c7e8
1 parent 056fdd4
commit 8d5c7e8
Show file tree

Hide file tree

Showing 6 changed files with 82 additions and 43 deletions.
diff --git a/crates/sui-graphql-e2e-tests/tests/packages/versioning.exp b/crates/sui-graphql-e2e-tests/tests/packages/versioning.exp
@@ -733,7 +733,7 @@ Response: {
   }
 }
 
-task 15, lines 345-378:
+task 15, lines 345-380:
 //# run-graphql
 Response: {
   "data": {
@@ -788,7 +788,7 @@ Response: {
   }
 }
 
-task 16, lines 380-398:
+task 16, lines 382-400:
 //# run-graphql
 Response: {
   "data": {

diff --git a/crates/sui-graphql-e2e-tests/tests/packages/versioning.move b/crates/sui-graphql-e2e-tests/tests/packages/versioning.move
@@ -351,14 +351,14 @@ module P2::m {
         }
     }
 
-    after: packageVersions(address: "@{P0}", afterVersion: 1) {
+    after: packageVersions(address: "@{P0}", filter: { afterVersion: 1 }) {
         nodes {
             address
             version
         }
     }
 
-    before: packageVersions(address: "@{P0}", beforeVersion: 3) {
+    before: packageVersions(address: "@{P0}", filter: { beforeVersion: 3 }) {
         nodes {
             address
             version
@@ -367,8 +367,10 @@ module P2::m {
 
     between: packageVersions(
         address: "@{P0}",
-        afterVersion: 1,
-        beforeVersion: 3,
+        filter: {
+          afterVersion: 1,
+          beforeVersion: 3,
+        },
     ) {
         nodes {
             address

diff --git a/crates/sui-graphql-rpc/schema/current_progress_schema.graphql b/crates/sui-graphql-rpc/schema/current_progress_schema.graphql
@@ -2177,7 +2177,7 @@ type MovePackage implements IObject & IOwner {
 	optionally bounding the versions exclusively from below with `afterVersion`, or from above
 	with `beforeVersion`.
 	"""
-	packageVersions(first: Int, after: String, last: Int, before: String, afterVersion: Int, beforeVersion: Int): MovePackageConnection!
+	packageVersions(first: Int, after: String, last: Int, before: String, filter: MovePackageVersionFilter): MovePackageConnection!
 	"""
 	Fetch the latest version of this package (the package with the highest `version` that shares
 	this packages's original ID)
@@ -2252,6 +2252,22 @@ type MovePackageEdge {
 	cursor: String!
 }
 
+"""
+Filter for paginating versions of a given `MovePackage`.
+"""
+input MovePackageVersionFilter {
+	"""
+	Fetch versions of this package that are strictly newer than this version. Omitting this
+	fetches versions since the original version.
+	"""
+	afterVersion: UInt53
+	"""
+	Fetch versions of this package that are strictly older than this version. Omitting this
+	fetches versions up to the latest version (inclusive).
+	"""
+	beforeVersion: UInt53
+}
+
 """
 Description of a struct type, defined in a Move module.
 """
@@ -3149,7 +3165,7 @@ type Query {
 	optionally bounding the versions exclusively from below with `afterVersion`, or from above
 	with `beforeVersion`.
 	"""
-	packageVersions(first: Int, after: String, last: Int, before: String, address: SuiAddress!, afterVersion: UInt53, beforeVersion: UInt53): MovePackageConnection!
+	packageVersions(first: Int, after: String, last: Int, before: String, address: SuiAddress!, filter: MovePackageVersionFilter): MovePackageConnection!
 	"""
 	Fetch the protocol config by protocol version (defaults to the latest protocol
 	version known to the GraphQL service).

diff --git a/crates/sui-graphql-rpc/src/types/move_package.rs b/crates/sui-graphql-rpc/src/types/move_package.rs
@@ -58,6 +58,18 @@ pub(crate) struct MovePackageCheckpointFilter {
     pub before_checkpoint: Option<UInt53>,
 }
 
+/// Filter for paginating versions of a given `MovePackage`.
+#[derive(InputObject, Debug, Default, Clone)]
+pub(crate) struct MovePackageVersionFilter {
+    /// Fetch versions of this package that are strictly newer than this version. Omitting this
+    /// fetches versions since the original version.
+    pub after_version: Option<UInt53>,
+
+    /// Fetch versions of this package that are strictly older than this version. Omitting this
+    /// fetches versions up to the latest version (inclusive).
+    pub before_version: Option<UInt53>,
+}
+
 /// Filter for a point query of a MovePackage, supporting querying different versions of a package
 /// by their version. Note that different versions of the same user package exist at different IDs
 /// to each other, so this is different from looking up the historical version of an object.
@@ -365,17 +377,15 @@ impl MovePackage {
         after: Option<Cursor>,
         last: Option<u64>,
         before: Option<Cursor>,
-        after_version: Option<u64>,
-        before_version: Option<u64>,
+        filter: Option<MovePackageVersionFilter>,
     ) -> Result<Connection<String, MovePackage>> {
         let page = Page::from_params(ctx.data_unchecked(), first, after, last, before)?;
 
         MovePackage::paginate_by_version(
             ctx.data_unchecked(),
             page,
             self.super_.address,
-            after_version,
-            before_version,
+            filter,
             self.checkpoint_viewed_at_impl(),
         )
         .await
@@ -734,8 +744,7 @@ impl MovePackage {
         db: &Db,
         page: Page<Cursor>,
         package: SuiAddress,
-        after_version: Option<u64>,
-        before_version: Option<u64>,
+        filter: Option<MovePackageVersionFilter>,
         checkpoint_viewed_at: u64,
     ) -> Result<Connection<String, MovePackage>, Error> {
         let cursor_viewed_at = page.validate_cursor_consistency()?;
@@ -746,9 +755,9 @@ impl MovePackage {
                     conn,
                     checkpoint_viewed_at,
                     if is_system_package(package) {
-                        system_package_version_query(package, after_version, before_version)
+                        system_package_version_query(package, filter)
                     } else {
-                        user_package_version_query(package, after_version, before_version)
+                        user_package_version_query(package, filter)
                     },
                 )
             })
@@ -1000,8 +1009,7 @@ impl TryFrom<&Object> for MovePackage {
 /// verified as a system package). This is an `objects_history` query disguised as a package query.
 fn system_package_version_query(
     package: SuiAddress,
-    after_version: Option<u64>,
-    before_version: Option<u64>,
+    filter: Option<MovePackageVersionFilter>,
 ) -> RawQuery {
     // Query uses a left join to force the query planner to use `objects_version` in the outer loop.
     let mut q = query!(
@@ -1028,12 +1036,14 @@ fn system_package_version_query(
         )
     );
 
-    if let Some(after) = after_version {
-        q = filter!(q, format!("v.object_version > {after}"));
+    if let Some(after) = filter.as_ref().and_then(|f| f.after_version) {
+        let a: u64 = after.into();
+        q = filter!(q, format!("v.object_version > {a}"));
     }
 
-    if let Some(before) = before_version {
-        q = filter!(q, format!("v.object_version < {before}"));
+    if let Some(before) = filter.as_ref().and_then(|f| f.before_version) {
+        let b: u64 = before.into();
+        q = filter!(q, format!("v.object_version < {b}"));
     }
 
     q
@@ -1043,8 +1053,7 @@ fn system_package_version_query(
 /// been verified as a system package)
 fn user_package_version_query(
     package: SuiAddress,
-    after_version: Option<u64>,
-    before_version: Option<u64>,
+    filter: Option<MovePackageVersionFilter>,
 ) -> RawQuery {
     let mut q = query!(
         r#"
@@ -1074,12 +1083,14 @@ fn user_package_version_query(
         )
     );
 
-    if let Some(after) = after_version {
-        q = filter!(q, format!("p.package_version > {after}"));
+    if let Some(after) = filter.as_ref().and_then(|f| f.after_version) {
+        let a: u64 = after.into();
+        q = filter!(q, format!("p.package_version > {a}"));
     }
 
-    if let Some(before) = before_version {
-        q = filter!(q, format!("p.package_version < {before}"));
+    if let Some(before) = filter.as_ref().and_then(|f| f.before_version) {
+        let b: u64 = before.into();
+        q = filter!(q, format!("p.package_version < {b}"));
     }
 
     q

diff --git a/crates/sui-graphql-rpc/src/types/query.rs b/crates/sui-graphql-rpc/src/types/query.rs
@@ -12,7 +12,9 @@ use sui_sdk::SuiClient;
 use sui_types::transaction::{TransactionData, TransactionKind};
 use sui_types::{gas_coin::GAS, transaction::TransactionDataAPI, TypeTag};
 
-use super::move_package::{self, MovePackage, MovePackageCheckpointFilter};
+use super::move_package::{
+    self, MovePackage, MovePackageCheckpointFilter, MovePackageVersionFilter,
+};
 use super::suins_registration::NameService;
 use super::uint53::UInt53;
 use super::{
@@ -468,22 +470,14 @@ impl Query {
         last: Option<u64>,
         before: Option<move_package::Cursor>,
         address: SuiAddress,
-        after_version: Option<UInt53>,
-        before_version: Option<UInt53>,
+        filter: Option<MovePackageVersionFilter>,
     ) -> Result<Connection<String, MovePackage>> {
         let Watermark { checkpoint, .. } = *ctx.data()?;
 
         let page = Page::from_params(ctx.data_unchecked(), first, after, last, before)?;
-        MovePackage::paginate_by_version(
-            ctx.data_unchecked(),
-            page,
-            address,
-            after_version.map(|v| v.into()),
-            before_version.map(|v| v.into()),
-            checkpoint,
-        )
-        .await
-        .extend()
+        MovePackage::paginate_by_version(ctx.data_unchecked(), page, address, filter, checkpoint)
+            .await
+            .extend()
     }
 
     /// Fetch the protocol config by protocol version (defaults to the latest protocol

diff --git a/crates/sui-graphql-rpc/tests/snapshots/snapshot_tests__schema_sdl_export.snap b/crates/sui-graphql-rpc/tests/snapshots/snapshot_tests__schema_sdl_export.snap
@@ -2181,7 +2181,7 @@ type MovePackage implements IObject & IOwner {
 	optionally bounding the versions exclusively from below with `afterVersion`, or from above
 	with `beforeVersion`.
 	"""
-	packageVersions(first: Int, after: String, last: Int, before: String, afterVersion: Int, beforeVersion: Int): MovePackageConnection!
+	packageVersions(first: Int, after: String, last: Int, before: String, filter: MovePackageVersionFilter): MovePackageConnection!
 	"""
 	Fetch the latest version of this package (the package with the highest `version` that shares
 	this packages's original ID)
@@ -2256,6 +2256,22 @@ type MovePackageEdge {
 	cursor: String!
 }
 
+"""
+Filter for paginating versions of a given `MovePackage`.
+"""
+input MovePackageVersionFilter {
+	"""
+	Fetch versions of this package that are strictly newer than this version. Omitting this
+	fetches versions since the original version.
+	"""
+	afterVersion: UInt53
+	"""
+	Fetch versions of this package that are strictly older than this version. Omitting this
+	fetches versions up to the latest version (inclusive).
+	"""
+	beforeVersion: UInt53
+}
+
 """
 Description of a struct type, defined in a Move module.
 """
@@ -3153,7 +3169,7 @@ type Query {
 	optionally bounding the versions exclusively from below with `afterVersion`, or from above
 	with `beforeVersion`.
 	"""
-	packageVersions(first: Int, after: String, last: Int, before: String, address: SuiAddress!, afterVersion: UInt53, beforeVersion: UInt53): MovePackageConnection!
+	packageVersions(first: Int, after: String, last: Int, before: String, address: SuiAddress!, filter: MovePackageVersionFilter): MovePackageConnection!
 	"""
 	Fetch the protocol config by protocol version (defaults to the latest protocol
 	version known to the GraphQL service).