fix: ghac doesn't support delete anymore (#5628)

* fix: ghac doesn't support delete anymore

Signed-off-by: Xuanwo <[email protected]>

* Fix tests

Signed-off-by: Xuanwo <[email protected]>

* Fix tests

Signed-off-by: Xuanwo <[email protected]>

* Fix tests

Signed-off-by: Xuanwo <[email protected]>

---------

Signed-off-by: Xuanwo <[email protected]>

Author: Xuanwo

.github/workflows/service_test_ghac.yml

@@ -59,7 +59,7 @@ jobs:
       - name: Test
         shell: bash
         working-directory: core
-        run: cargo nextest run behavior --features tests,services-ghac
+        run: cargo test behavior --features tests,services-ghac
         env:
           OPENDAL_TEST: ghac
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

core/src/services/ghac/backend.rs

@@ -26,15 +26,13 @@ use http::header::AUTHORIZATION;
 use http::header::CONTENT_LENGTH;
 use http::header::CONTENT_RANGE;
 use http::header::CONTENT_TYPE;
-use http::header::USER_AGENT;
 use http::Request;
 use http::Response;
 use http::StatusCode;
 use log::debug;
 use serde::Deserialize;
 use serde::Serialize;
 
-use super::delete::GhacDeleter;
 use super::error::parse_error;
 use super::writer::GhacWriter;
 use crate::raw::*;
@@ -54,14 +52,6 @@ const ACTIONS_CACHE_URL: &str = "ACTIONS_CACHE_URL";
 /// This token will be valid for 6h and github action will running for 6
 /// hours at most. So we don't need to refetch it again.
 const ACTIONS_RUNTIME_TOKEN: &str = "ACTIONS_RUNTIME_TOKEN";
-/// The token provided by workflow;
-const GITHUB_TOKEN: &str = "GITHUB_TOKEN";
-/// The github api url for ghac.
-const GITHUB_API_URL: &str = "GITHUB_API_URL";
-/// The repository that runs this action.
-const GITHUB_REPOSITORY: &str = "GITHUB_REPOSITORY";
-/// The github API version that used by OpenDAL.
-const GITHUB_API_VERSION: &str = "2022-11-28";
 
 fn value_or_env(
     explicit_value: Option<String>,
@@ -198,11 +188,6 @@ impl Builder for GhacBuilder {
                 .clone()
                 .unwrap_or_else(|| "opendal".to_string()),
 
-            api_url: env::var(GITHUB_API_URL)
-                .unwrap_or_else(|_| "https://api.github.com".to_string()),
-            api_token: env::var(GITHUB_TOKEN).unwrap_or_default(),
-            repo: env::var(GITHUB_REPOSITORY).unwrap_or_default(),
-
             client,
         };
 
@@ -220,18 +205,14 @@ pub struct GhacBackend {
     catch_token: String,
     version: String,
 
-    api_url: String,
-    pub api_token: String,
-    repo: String,
-
     pub client: HttpClient,
 }
 
 impl Access for GhacBackend {
     type Reader = HttpBody;
     type Writer = GhacWriter;
     type Lister = ();
-    type Deleter = oio::OneShotDeleter<GhacDeleter>;
+    type Deleter = ();
     type BlockingReader = ();
     type BlockingWriter = ();
     type BlockingLister = ();
@@ -258,7 +239,6 @@ impl Access for GhacBackend {
 
                 write: true,
                 write_can_multi: true,
-                delete: true,
 
                 shared: true,
 
@@ -356,13 +336,6 @@ impl Access for GhacBackend {
 
         Ok((RpWrite::default(), GhacWriter::new(self.clone(), cache_id)))
     }
-
-    async fn delete(&self) -> Result<(RpDelete, Self::Deleter)> {
-        Ok((
-            RpDelete::default(),
-            oio::OneShotDeleter::new(GhacDeleter::new(self.clone())),
-        ))
-    }
 }
 
 impl GhacBackend {
@@ -463,26 +436,6 @@ impl GhacBackend {
 
         Ok(req)
     }
-
-    pub async fn ghac_delete(&self, path: &str) -> Result<Response<Buffer>> {
-        let p = build_abs_path(&self.root, path);
-
-        let url = format!(
-            "{}/repos/{}/actions/caches?key={}",
-            self.api_url,
-            self.repo,
-            percent_encode_path(&p)
-        );
-
-        let mut req = Request::delete(&url);
-        req = req.header(AUTHORIZATION, format!("Bearer {}", self.api_token));
-        req = req.header(USER_AGENT, format!("opendal/{VERSION} (service ghac)"));
-        req = req.header("X-GitHub-Api-Version", GITHUB_API_VERSION);
-
-        let req = req.body(Buffer::new()).map_err(new_request_build_error)?;
-
-        self.client.send(req).await
-    }
 }
 
 #[derive(Deserialize)]

core/src/services/ghac/delete.rs

@@ -15,8 +15,6 @@
 // specific language governing permissions and limitations
 // under the License.
 
-#[cfg(feature = "services-ghac")]
-mod delete;
 #[cfg(feature = "services-ghac")]
 mod error;
 #[cfg(feature = "services-ghac")]

core/src/services/ghac/mod.rs

@@ -40,12 +40,11 @@ pub fn tests(op: &Operator, tests: &mut Vec<Trial>) {
 
 /// Copy a file and test with stat.
 pub fn test_blocking_copy_file(op: BlockingOperator) -> Result<()> {
-    let source_path = uuid::Uuid::new_v4().to_string();
-    let (source_content, _) = gen_bytes(op.info().full_capability());
+    let (source_path, source_content, _) = TEST_FIXTURE.new_file(op.clone());
 
     op.write(&source_path, source_content.clone())?;
 
-    let target_path = uuid::Uuid::new_v4().to_string();
+    let target_path = TEST_FIXTURE.new_file_path();
 
     op.copy(&source_path, &target_path)?;
 
@@ -55,8 +54,6 @@ pub fn test_blocking_copy_file(op: BlockingOperator) -> Result<()> {
         format!("{:x}", Sha256::digest(&source_content)),
     );
 
-    op.delete(&source_path).expect("delete must succeed");
-    op.delete(&target_path).expect("delete must succeed");
     Ok(())
 }
 
@@ -96,46 +93,37 @@ pub fn test_blocking_copy_target_dir(op: BlockingOperator) -> Result<()> {
         return Ok(());
     }
 
-    let source_path = uuid::Uuid::new_v4().to_string();
-    let (source_content, _) = gen_bytes(op.info().full_capability());
+    let (source_path, source_content, _) = TEST_FIXTURE.new_file(op.clone());
 
     op.write(&source_path, source_content)?;
 
-    let target_path = format!("{}/", uuid::Uuid::new_v4());
+    let target_path = TEST_FIXTURE.new_dir_path();
 
     op.create_dir(&target_path)?;
 
     let err = op
         .copy(&source_path, &target_path)
         .expect_err("copy must fail");
     assert_eq!(err.kind(), ErrorKind::IsADirectory);
-
-    op.delete(&source_path).expect("delete must succeed");
-    op.delete(&target_path).expect("delete must succeed");
     Ok(())
 }
 
 /// Copy a file to self should return an error.
 pub fn test_blocking_copy_self(op: BlockingOperator) -> Result<()> {
-    let source_path = uuid::Uuid::new_v4().to_string();
-    let (source_content, _size) = gen_bytes(op.info().full_capability());
+    let (source_path, source_content, _) = TEST_FIXTURE.new_file(op.clone());
 
     op.write(&source_path, source_content)?;
 
     let err = op
         .copy(&source_path, &source_path)
         .expect_err("copy must fail");
     assert_eq!(err.kind(), ErrorKind::IsSameFile);
-
-    op.delete(&source_path).expect("delete must succeed");
     Ok(())
 }
 
 /// Copy to a nested path, parent path should be created successfully.
 pub fn test_blocking_copy_nested(op: BlockingOperator) -> Result<()> {
-    let source_path = uuid::Uuid::new_v4().to_string();
-    let (source_content, _) = gen_bytes(op.info().full_capability());
-
+    let (source_path, source_content, _) = TEST_FIXTURE.new_file(op.clone());
     op.write(&source_path, source_content.clone())?;
 
     let target_path = format!(
@@ -144,6 +132,7 @@ pub fn test_blocking_copy_nested(op: BlockingOperator) -> Result<()> {
         uuid::Uuid::new_v4(),
         uuid::Uuid::new_v4()
     );
+    TEST_FIXTURE.add_path(target_path.clone());
 
     op.copy(&source_path, &target_path)?;
 
@@ -152,23 +141,15 @@ pub fn test_blocking_copy_nested(op: BlockingOperator) -> Result<()> {
         format!("{:x}", Sha256::digest(target_content)),
         format!("{:x}", Sha256::digest(&source_content)),
     );
-
-    op.delete(&source_path).expect("delete must succeed");
-    op.delete(&target_path).expect("delete must succeed");
     Ok(())
 }
 
 /// Copy to a exist path should overwrite successfully.
 pub fn test_blocking_copy_overwrite(op: BlockingOperator) -> Result<()> {
-    let source_path = uuid::Uuid::new_v4().to_string();
-    let (source_content, _) = gen_bytes(op.info().full_capability());
-
+    let (source_path, source_content, _) = TEST_FIXTURE.new_file(op.clone());
     op.write(&source_path, source_content.clone())?;
 
-    let target_path = uuid::Uuid::new_v4().to_string();
-    let (target_content, _) = gen_bytes(op.info().full_capability());
-    assert_ne!(source_content, target_content);
-
+    let (target_path, target_content, _) = TEST_FIXTURE.new_file(op.clone());
     op.write(&target_path, target_content)?;
 
     op.copy(&source_path, &target_path)?;
@@ -178,8 +159,5 @@ pub fn test_blocking_copy_overwrite(op: BlockingOperator) -> Result<()> {
         format!("{:x}", Sha256::digest(target_content)),
         format!("{:x}", Sha256::digest(&source_content)),
     );
-
-    op.delete(&source_path).expect("delete must succeed");
-    op.delete(&target_path).expect("delete must succeed");
     Ok(())
 }

core/tests/behavior/blocking_copy.rs

@@ -37,14 +37,12 @@ pub fn test_blocking_create_dir(op: BlockingOperator) -> Result<()> {
         return Ok(());
     }
 
-    let path = format!("{}/", uuid::Uuid::new_v4());
+    let path = TEST_FIXTURE.new_dir_path();
 
     op.create_dir(&path)?;
 
     let meta = op.stat(&path)?;
     assert_eq!(meta.mode(), EntryMode::DIR);
-
-    op.delete(&path).expect("delete must succeed");
     Ok(())
 }
 
@@ -54,15 +52,12 @@ pub fn test_blocking_create_dir_existing(op: BlockingOperator) -> Result<()> {
         return Ok(());
     }
 
-    let path = format!("{}/", uuid::Uuid::new_v4());
-
+    let path = TEST_FIXTURE.new_dir_path();
     op.create_dir(&path)?;
 
     op.create_dir(&path)?;
 
     let meta = op.stat(&path)?;
     assert_eq!(meta.mode(), EntryMode::DIR);
-
-    op.delete(&path).expect("delete must succeed");
     Ok(())
 }

core/tests/behavior/blocking_create_dir.rs

@@ -43,7 +43,7 @@ pub fn test_blocking_list_dir(op: BlockingOperator) -> Result<()> {
     let parent = uuid::Uuid::new_v4().to_string();
     let path = format!("{parent}/{}", uuid::Uuid::new_v4());
     debug!("Generate a random file: {}", &path);
-    let (content, size) = gen_bytes(op.info().full_capability());
+    let (_, content, size) = TEST_FIXTURE.new_file_with_path(op.clone(), &path);
 
     op.write(&path, content).expect("write must succeed");
 
@@ -61,14 +61,12 @@ pub fn test_blocking_list_dir(op: BlockingOperator) -> Result<()> {
         }
     }
     assert!(found, "file should be found in list");
-
-    op.delete(&path).expect("delete must succeed");
     Ok(())
 }
 
 /// List non exist dir should return nothing.
 pub fn test_blocking_list_non_exist_dir(op: BlockingOperator) -> Result<()> {
-    let dir = format!("{}/", uuid::Uuid::new_v4());
+    let dir = TEST_FIXTURE.new_dir_path();
 
     let obs = op.lister(&dir)?;
     let mut objects = HashMap::new();
@@ -84,6 +82,10 @@ pub fn test_blocking_list_non_exist_dir(op: BlockingOperator) -> Result<()> {
 
 // Remove all should remove all in this path.
 pub fn test_blocking_remove_all(op: BlockingOperator) -> Result<()> {
+    if !op.info().full_capability().delete {
+        return Ok(());
+    }
+
     let parent = uuid::Uuid::new_v4().to_string();
 
     let expected = [