diff --git a/graph/src/components/store/err.rs b/graph/src/components/store/err.rs index dcf92f6f461..cbf500884df 100644 --- a/graph/src/components/store/err.rs +++ b/graph/src/components/store/err.rs @@ -56,8 +56,6 @@ pub enum StoreError { ForkFailure(String), #[error("subgraph writer poisoned by previous error")] Poisoned, - #[error("subgraph not found: {0}")] - SubgraphNotFound(String), #[error("panic in subgraph writer: {0}")] WriterPanic(JoinError), #[error( @@ -121,7 +119,6 @@ impl Clone for StoreError { Self::DatabaseUnavailable => Self::DatabaseUnavailable, Self::ForkFailure(arg0) => Self::ForkFailure(arg0.clone()), Self::Poisoned => Self::Poisoned, - Self::SubgraphNotFound(arg0) => Self::SubgraphNotFound(arg0.clone()), Self::WriterPanic(arg0) => Self::Unknown(anyhow!("writer panic: {}", arg0)), Self::UnsupportedDeploymentSchemaVersion(arg0) => { Self::UnsupportedDeploymentSchemaVersion(*arg0) @@ -205,7 +202,6 @@ impl StoreError { | Canceled | DatabaseUnavailable | ForkFailure(_) - | SubgraphNotFound(_) | Poisoned | WriterPanic(_) | UnsupportedDeploymentSchemaVersion(_) diff --git a/node/src/bin/manager.rs b/node/src/bin/manager.rs index 8894f18ff56..792df8853c9 100644 --- a/node/src/bin/manager.rs +++ b/node/src/bin/manager.rs @@ -165,7 +165,7 @@ pub enum Command { /// Remove a named subgraph Remove { /// The name of the subgraph to remove - name: DeploymentSearch, + name: String, }, /// Create a subgraph name Create { @@ -1745,7 +1745,7 @@ fn make_deployment_selector( use graphman::deployment::DeploymentSelector::*; match deployment { - DeploymentSearch::Name { name } => Name(name.to_string()), + DeploymentSearch::Name { name } => Name(name), DeploymentSearch::Hash { hash, shard } => Subgraph { hash, shard }, DeploymentSearch::All => All, DeploymentSearch::Deployment { namespace } => Schema(namespace), diff --git a/node/src/manager/commands/remove.rs b/node/src/manager/commands/remove.rs index 4311e824cbb..bcf9417569a 100644 --- a/node/src/manager/commands/remove.rs +++ b/node/src/manager/commands/remove.rs @@ -3,24 +3,11 @@ use std::sync::Arc; use graph::prelude::{anyhow, Error, SubgraphName, SubgraphStore as _}; use graph_store_postgres::SubgraphStore; -use crate::manager::deployment::DeploymentSearch; +pub async fn run(store: Arc, name: &str) -> Result<(), Error> { + let name = SubgraphName::new(name).map_err(|name| anyhow!("illegal subgraph name `{name}`"))?; -pub async fn run(store: Arc, name: &DeploymentSearch) -> Result<(), Error> { - match name { - DeploymentSearch::Name { name } => { - let subgraph_name = SubgraphName::new(name) - .map_err(|name| anyhow!("illegal subgraph name `{name}`"))?; - println!("Removing subgraph {}", name); - store.remove_subgraph(subgraph_name).await?; - println!("Subgraph {} removed", name); - } - _ => { - return Err(anyhow!( - "Remove command expects a subgraph name, but got either hash or namespace: {}", - name - )) - } - } + println!("Removing subgraph {}", name); + store.remove_subgraph(name).await?; Ok(()) } diff --git a/server/graphman/src/resolvers/deployment_mutation/remove.rs b/server/graphman/src/resolvers/deployment_mutation/remove.rs index ba1b50a2adb..c7997b6885f 100644 --- a/server/graphman/src/resolvers/deployment_mutation/remove.rs +++ b/server/graphman/src/resolvers/deployment_mutation/remove.rs @@ -24,10 +24,7 @@ pub async fn run(ctx: &GraphmanContext, name: &String) -> Result<()> { } }; - let changes = catalog_conn - .remove_subgraph(name) - .await - .map_err(GraphmanError::from)?; + let changes = catalog_conn.remove_subgraph(name).await?; catalog_conn .send_store_event(&ctx.notification_sender, &StoreEvent::new(changes)) .await?; diff --git a/server/graphman/tests/deployment_mutation.rs b/server/graphman/tests/deployment_mutation.rs index e4d89633976..fd2020ee740 100644 --- a/server/graphman/tests/deployment_mutation.rs +++ b/server/graphman/tests/deployment_mutation.rs @@ -6,8 +6,8 @@ use graph::components::store::SubgraphStore; use graph::prelude::DeploymentHash; use serde::Deserialize; use serde_json::json; +use test_store::create_test_subgraph; use test_store::SUBGRAPH_STORE; -use test_store::{create_subgraph_name, create_test_subgraph}; use tokio::time::sleep; use self::util::client::send_graphql_request; @@ -358,8 +358,6 @@ fn graphql_cannot_create_new_subgraph_with_invalid_name() { #[test] fn graphql_can_remove_subgraph() { run_test(|| async { - create_subgraph_name("subgraph_1").await.unwrap(); - let resp = send_graphql_request( json!({ "query": r#"mutation RemoveSubgraph { @@ -405,44 +403,17 @@ fn graphql_cannot_remove_subgraph_with_invalid_name() { ) .await; - let data = &resp["data"]["deployment"]; - let errors = resp["errors"].as_array().unwrap(); - - assert!(data.is_null()); - assert_eq!(errors.len(), 1); - assert_eq!( - errors[0]["message"].as_str().unwrap(), - "store error: Subgraph name must contain only a-z, A-Z, 0-9, '-' and '_'" - ); - }); -} - -#[test] -fn graphql_remove_returns_error_for_non_existing_subgraph() { - run_test(|| async { - let resp = send_graphql_request( - json!({ - "query": r#"mutation RemoveNonExistingSubgraph { - deployment { - remove(name: "non_existing_subgraph") { - success - } + let success_resp = json!({ + "data": { + "deployment": { + "remove": { + "success": true, } - }"# - }), - VALID_TOKEN, - ) - .await; - - let data = &resp["data"]["deployment"]; - let errors = resp["errors"].as_array().unwrap(); + } + } + }); - assert!(data.is_null()); - assert_eq!(errors.len(), 1); - assert_eq!( - errors[0]["message"].as_str().unwrap(), - "store error: subgraph not found: non_existing_subgraph" - ); + assert_ne!(resp, success_resp); }); } diff --git a/store/postgres/src/primary.rs b/store/postgres/src/primary.rs index 21cbef3b9a1..d7f506ff024 100644 --- a/store/postgres/src/primary.rs +++ b/store/postgres/src/primary.rs @@ -1124,7 +1124,7 @@ impl Connection { .await?; self.remove_unused_assignments().await } else { - Err(StoreError::SubgraphNotFound(name.to_string())) + Ok(vec![]) } } diff --git a/store/test-store/src/store.rs b/store/test-store/src/store.rs index 8c2fb08d188..5f2cc52949b 100644 --- a/store/test-store/src/store.rs +++ b/store/test-store/src/store.rs @@ -260,19 +260,9 @@ pub async fn create_test_subgraph_with_features( locator } -pub async fn create_subgraph_name(name: &str) -> Result<(), StoreError> { - let subgraph_name = SubgraphName::new_unchecked(name.to_string()); - SUBGRAPH_STORE.create_subgraph(subgraph_name).await?; - Ok(()) -} - pub async fn remove_subgraph(id: &DeploymentHash) { let name = SubgraphName::new_unchecked(id.to_string()); - // Ignore SubgraphNotFound errors during cleanup - match SUBGRAPH_STORE.remove_subgraph(name).await { - Ok(_) | Err(StoreError::SubgraphNotFound(_)) => {} - Err(e) => panic!("unexpected error removing subgraph: {}", e), - } + SUBGRAPH_STORE.remove_subgraph(name).await.unwrap(); let locs = SUBGRAPH_STORE.locators(id.as_str()).await.unwrap(); let mut conn = primary_connection().await; for loc in locs { diff --git a/store/test-store/tests/postgres/subgraph.rs b/store/test-store/tests/postgres/subgraph.rs index 5fa9b8c89ba..23b60ecc52c 100644 --- a/store/test-store/tests/postgres/subgraph.rs +++ b/store/test-store/tests/postgres/subgraph.rs @@ -1232,18 +1232,3 @@ fn fail_unfail_non_deterministic_error_noop() { test_store::remove_subgraphs().await; }) } - -#[test] -fn remove_nonexistent_subgraph_returns_not_found() { - test_store::run_test_sequentially(|store| async move { - remove_subgraphs().await; - - let name = SubgraphName::new("nonexistent/subgraph").unwrap(); - let result = store.subgraph_store().remove_subgraph(name.clone()).await; - - assert!(matches!( - result, - Err(StoreError::SubgraphNotFound(n)) if n == name.to_string() - )); - }) -} diff --git a/tests/src/fixture/mod.rs b/tests/src/fixture/mod.rs index c36c9043830..62b8eb1f7f4 100644 --- a/tests/src/fixture/mod.rs +++ b/tests/src/fixture/mod.rs @@ -627,12 +627,7 @@ pub async fn cleanup( hash: &DeploymentHash, ) -> Result<(), Error> { let locators = subgraph_store.locators(hash).await?; - // Remove subgraph if it exists, ignore not found errors - match subgraph_store.remove_subgraph(name.clone()).await { - Ok(_) | Err(graph::prelude::StoreError::SubgraphNotFound(_)) => {} - Err(e) => return Err(e.into()), - } - + subgraph_store.remove_subgraph(name.clone()).await?; for locator in locators { subgraph_store.remove_deployment(locator.id.into()).await?; }