refactor: snapshot listing based on query

Signed-off-by: Abhinandan Purkait <[email protected]>
openebs · Aug 22, 2023 · 3358087 · 3358087
1 parent 0ddaae3
commit 3358087
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 54 deletions.
diff --git a/io-engine-tests/src/snapshot.rs b/io-engine-tests/src/snapshot.rs
@@ -10,7 +10,6 @@ use mayastor_api::v1::snapshot::{
     // ListSnapshotsResponse,
     Replica,
     SnapshotInfo,
-    SnapshotQueryType,
 };
 use tonic::Status;
 
@@ -109,7 +108,7 @@ pub async fn list_snapshot(
         .list_snapshot(ListSnapshotsRequest {
             source_uuid: None,
             snapshot_uuid: None,
-            snapshot_query_type: SnapshotQueryType::AllSnapshots as i32,
+            query: None,
         })
         .await
         .map(|r| r.into_inner().snapshots)

diff --git a/io-engine/src/bin/io-engine-client/v1/snapshot_cli.rs b/io-engine/src/bin/io-engine-client/v1/snapshot_cli.rs
@@ -411,8 +411,7 @@ async fn list(mut ctx: Context, matches: &ArgMatches<'_>) -> crate::Result<()> {
     let request = v1_rpc::snapshot::ListSnapshotsRequest {
         source_uuid,
         snapshot_uuid,
-        snapshot_query_type: v1_rpc::snapshot::SnapshotQueryType::AllSnapshots
-            as i32,
+        query: None,
     };
 
     let response = ctx

diff --git a/io-engine/src/grpc/v1/snapshot.rs b/io-engine/src/grpc/v1/snapshot.rs
@@ -314,54 +314,38 @@ impl SnapshotService {
     }
 }
 
-/// Filter snapshots based on snapshot_query_type came in gRPC request.
+/// Filter snapshots based on query came in gRPC request.
 fn filter_snapshots_by_snapshot_query_type(
     snapshot_list: Vec<SnapshotInfo>,
-    snap_query_type: Option<SnapshotQueryType>,
+    query: Option<list_snapshots_request::Query>,
 ) -> Vec<SnapshotInfo> {
-    let Some(snap_query_type) = snap_query_type else {
-        return snapshot_list
+    let query = match query {
+        None => return snapshot_list,
+        Some(query) => query,
     };
+
     snapshot_list
         .into_iter()
-        .filter_map(|s| match snap_query_type {
-            // AllSnapshots
-            SnapshotQueryType::AllSnapshots => Some(s),
-            // AllSnapshotsExceptDiscardedSnapshots
-            SnapshotQueryType::AllSnapshotsExceptDiscardedSnapshots => {
-                if !s.discarded_snapshot {
-                    Some(s)
-                } else {
-                    None
-                }
-            }
-            // OnlyDiscardedSnapshots
-            SnapshotQueryType::OnlyDiscardedSnapshots => {
-                if s.discarded_snapshot {
-                    Some(s)
-                } else {
-                    None
-                }
-            }
-            // OnlyInvalidSnapshots
-            SnapshotQueryType::OnlyInvalidSnapshots => {
-                if !s.valid_snapshot {
-                    Some(s)
-                } else {
-                    None
-                }
-            }
-            // OnlyUsableSnapshots
-            SnapshotQueryType::OnlyUsableSnapshots => {
-                if !s.discarded_snapshot && s.valid_snapshot {
-                    Some(s)
-                } else {
-                    None
+        .filter(|snapshot| {
+            let query = &query;
+
+            let query_fields = vec![
+                (query.invalid, snapshot.valid_snapshot),
+                (query.discarded, snapshot.discarded_snapshot),
+                // ... add other fields here as needed
+            ];
+
+            query_fields.iter().all(|(query_field, snapshot_field)| {
+                match query_field {
+                    Some(true) => *snapshot_field,
+                    Some(false) => !(*snapshot_field),
+                    None => true,
                 }
-            }
+            })
         })
         .collect()
 }
+
 #[tonic::async_trait]
 impl SnapshotRpc for SnapshotService {
     #[named]
@@ -563,8 +547,7 @@ impl SnapshotRpc for SnapshotService {
                             .collect();
                     }
                     let snapshots = filter_snapshots_by_snapshot_query_type(
-                        snapshots,
-                        SnapshotQueryType::from_i32(args.snapshot_query_type),
+                        snapshots, args.query,
                     );
                     Ok(ListSnapshotsResponse {
                         snapshots,

diff --git a/io-engine/tests/snapshot_nexus.rs b/io-engine/tests/snapshot_nexus.rs
@@ -11,7 +11,7 @@ use common::{
             bdev::ListBdevOptions,
             pool::CreatePoolRequest,
             replica::{CreateReplicaRequest, ListReplicaOptions},
-            snapshot::{ListSnapshotsRequest, SnapshotInfo, SnapshotQueryType},
+            snapshot::{ListSnapshotsRequest, SnapshotInfo},
             GrpcConnect,
         },
         Builder,
@@ -250,7 +250,7 @@ async fn test_replica_handle_snapshot() {
         .list_snapshot(ListSnapshotsRequest {
             source_uuid: None,
             snapshot_uuid: None,
-            snapshot_query_type: SnapshotQueryType::AllSnapshots as i32,
+            query: None,
         })
         .await
         .expect("Failed to list snapshots on replica node")
@@ -293,7 +293,7 @@ async fn test_replica_handle_snapshot() {
         .list_snapshot(ListSnapshotsRequest {
             source_uuid: None,
             snapshot_uuid: None,
-            snapshot_query_type: SnapshotQueryType::AllSnapshots as i32,
+            query: None,
         })
         .await
         .expect("Failed to list snapshots on replica node")
@@ -375,7 +375,7 @@ async fn test_list_no_snapshots() {
         .list_snapshot(ListSnapshotsRequest {
             source_uuid: None,
             snapshot_uuid: None,
-            snapshot_query_type: SnapshotQueryType::AllSnapshots as i32,
+            query: None,
         })
         .await
         .expect("Failed to list snapshots on replica node")
@@ -458,7 +458,7 @@ async fn test_nexus_snapshot() {
         .list_snapshot(ListSnapshotsRequest {
             source_uuid: None,
             snapshot_uuid: None,
-            snapshot_query_type: SnapshotQueryType::AllSnapshots as i32,
+            query: None,
         })
         .await
         .expect("Failed to list snapshots on replica node")
@@ -501,7 +501,7 @@ async fn test_nexus_snapshot() {
         .list_snapshot(ListSnapshotsRequest {
             source_uuid: None,
             snapshot_uuid: None,
-            snapshot_query_type: SnapshotQueryType::AllSnapshots as i32,
+            query: None,
         })
         .await
         .expect("Failed to list snapshots on replica node")
@@ -589,7 +589,7 @@ async fn test_duplicated_snapshot_uuid_name() {
         .list_snapshot(ListSnapshotsRequest {
             source_uuid: None,
             snapshot_uuid: None,
-            snapshot_query_type: SnapshotQueryType::AllSnapshots as i32,
+            query: None,
         })
         .await
         .expect("Failed to list snapshots on replica node")
@@ -829,7 +829,7 @@ async fn test_snapshot_ancestor_usage() {
         .list_snapshot(ListSnapshotsRequest {
             source_uuid: None,
             snapshot_uuid: None,
-            snapshot_query_type: SnapshotQueryType::AllSnapshots as i32,
+            query: None,
         })
         .await
         .expect("Failed to list snapshots on replica node")
@@ -897,7 +897,7 @@ async fn test_snapshot_ancestor_usage() {
         .list_snapshot(ListSnapshotsRequest {
             source_uuid: None,
             snapshot_uuid: None,
-            snapshot_query_type: SnapshotQueryType::AllSnapshots as i32,
+            query: None,
         })
         .await
         .expect("Failed to list snapshots on replica node")

diff --git a/rpc/mayastor-api b/rpc/mayastor-api