feat: add jfrog test setup for webdav (#4265)

* feat: add jfrog test for webdav * feat: add jfrog test for webdav * feat: add jfrog test for webdav * add webdav copy success statuscode * add webdav copy success statuscode * Take nginx as default test to run Signed-off-by: Xuanwo <[email protected]> * Use volumn instead Signed-off-by: Xuanwo <[email protected]> * fix webdav lister path panic * fix webdav lister entrymode panic * Try fix XML de Signed-off-by: Xuanwo <[email protected]> * jfrog doesn't support copy Signed-off-by: Xuanwo <[email protected]> * Fix list Signed-off-by: Xuanwo <[email protected]> * Fix other tests Signed-off-by: Xuanwo <[email protected]> * check metakey first Signed-off-by: Xuanwo <[email protected]> --------- Signed-off-by: Xuanwo <[email protected]> Co-authored-by: Xuanwo <[email protected]>
apache · Feb 27, 2024 · b788b14 · b788b14
1 parent 92b7a63
commit b788b14
Show file tree

Hide file tree

Showing 5 changed files with 171 additions and 40 deletions.
diff --git a/.github/services/webdav/nginx/action.yml → .github/services/webdav/0_nginx/action.yml b/.github/services/webdav/nginx/action.yml → .github/services/webdav/0_nginx/action.yml
diff --git a/.github/services/webdav/jfrog/action.yml b/.github/services/webdav/jfrog/action.yml
@@ -0,0 +1,41 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+name: jfrog
+description: "Behavior test for webdav in jfrog"
+
+runs:
+  using: "composite"
+  steps:
+    - name: Setup webdav in jfrog
+      shell: bash
+      working-directory: fixtures/webdav
+      run: |
+        # Can we remove this?
+        # touch artifactory/etc/system.yaml
+        docker compose -f docker-compose-webdav-jfrog.yml up -d --wait
+
+    - name: Setup
+      shell: bash
+      run: |
+        cat << EOF >> $GITHUB_ENV
+        OPENDAL_WEBDAV_ENDPOINT=http://127.0.0.1:8081/artifactory/example-repo-local
+        OPENDAL_WEBDAV_USERNAME=admin
+        OPENDAL_WEBDAV_PASSWORD=password
+        OPENDAL_WEBDAV_DISABLE_COPY=true
+        RUST_TEST_THREADS=1
+        EOF
diff --git a/core/src/services/webdav/backend.rs b/core/src/services/webdav/backend.rs
@@ -32,7 +32,6 @@ use serde::Deserialize;
 
 use super::error::parse_error;
 use super::lister::Multistatus;
-use super::lister::MultistatusOptional;
 use super::lister::WebdavLister;
 use super::writer::WebdavWriter;
 use crate::raw::*;
@@ -53,6 +52,8 @@ pub struct WebdavConfig {
     pub token: Option<String>,
     /// root of this backend
     pub root: Option<String>,
+    /// WebDAV Service doesn't support copy.
+    pub disable_copy: bool,
 }
 
 impl Debug for WebdavConfig {
@@ -157,15 +158,13 @@ impl Builder for WebdavBuilder {
     type Accessor = WebdavBackend;
 
     fn from_map(map: HashMap<String, String>) -> Self {
-        let mut builder = WebdavBuilder::default();
+        let config = WebdavConfig::deserialize(ConfigDeserializer::new(map))
+            .expect("config deserialize must succeed");
 
-        map.get("root").map(|v| builder.root(v));
-        map.get("endpoint").map(|v| builder.endpoint(v));
-        map.get("username").map(|v| builder.username(v));
-        map.get("password").map(|v| builder.password(v));
-        map.get("token").map(|v| builder.token(v));
-
-        builder
+        WebdavBuilder {
+            config,
+            http_client: None,
+        }
     }
 
     fn build(&mut self) -> Result<Self::Accessor> {
@@ -206,6 +205,7 @@ impl Builder for WebdavBuilder {
         Ok(WebdavBackend {
             endpoint: endpoint.to_string(),
             authorization: auth,
+            disable_copy: self.config.disable_copy,
             root,
             client,
         })
@@ -218,6 +218,7 @@ pub struct WebdavBackend {
     endpoint: String,
     root: String,
     client: HttpClient,
+    disable_copy: bool,
 
     authorization: Option<String>,
 }
@@ -258,7 +259,7 @@ impl Accessor for WebdavBackend {
                 create_dir: true,
                 delete: true,
 
-                copy: true,
+                copy: !self.disable_copy,
 
                 rename: true,
 
@@ -292,8 +293,14 @@ impl Accessor for WebdavBackend {
         }
 
         let bs = resp.into_body().bytes().await?;
-        let result: MultistatusOptional =
-            quick_xml::de::from_reader(bs.reader()).map_err(new_xml_deserialize_error)?;
+        let s = String::from_utf8_lossy(&bs);
+
+        // Make sure the string is escaped.
+        // Related to <https://github.com/tafia/quick-xml/issues/719>
+        //
+        // This is a temporary solution, we should find a better way to handle this.
+        let s = s.replace("&()_+-=;", "%26%28%29_%2B-%3D%3B");
+        let result: Multistatus = quick_xml::de::from_str(&s).map_err(new_xml_deserialize_error)?;
 
         let response = match result.response {
             Some(v) => v,
@@ -379,6 +386,13 @@ impl Accessor for WebdavBackend {
                 let result: Multistatus =
                     quick_xml::de::from_reader(bs.reader()).map_err(new_xml_deserialize_error)?;
 
+                let result = match result.response {
+                    Some(v) => v,
+                    None => {
+                        return Ok((RpList::default(), None));
+                    }
+                };
+
                 let l = WebdavLister::new(&self.endpoint, &self.root, path, result);
 
                 Ok((RpList::default(), Some(oio::PageLister::new(l))))
@@ -421,7 +435,9 @@ impl Accessor for WebdavBackend {
         let status = resp.status();
 
         match status {
-            StatusCode::CREATED | StatusCode::NO_CONTENT => Ok(RpRename::default()),
+            StatusCode::CREATED | StatusCode::NO_CONTENT | StatusCode::OK => {
+                Ok(RpRename::default())
+            }
             _ => Err(parse_error(resp).await?),
         }
     }
@@ -569,6 +585,8 @@ impl WebdavBackend {
     async fn webdav_copy(&self, from: &str, to: &str) -> Result<Response<IncomingAsyncBody>> {
         let source = build_abs_path(&self.root, from);
         let target = build_abs_path(&self.root, to);
+        // Make sure target's dir is exist.
+        self.ensure_parent_path(&target).await?;
 
         let source = format!("{}/{}", self.endpoint, percent_encode_path(&source));
         let target = format!("{}/{}", self.endpoint, percent_encode_path(&target));
@@ -592,8 +610,13 @@ impl WebdavBackend {
     }
 
     async fn webdav_move(&self, from: &str, to: &str) -> Result<Response<IncomingAsyncBody>> {
+        // Check if the source exists first.
+        self.stat(from, OpStat::new()).await?;
+
         let source = build_abs_path(&self.root, from);
         let target = build_abs_path(&self.root, to);
+        // Make sure target's dir is exist.
+        self.ensure_parent_path(&target).await?;
 
         let source = format!("{}/{}", self.endpoint, percent_encode_path(&source));
         let target = format!("{}/{}", self.endpoint, percent_encode_path(&target));
@@ -648,7 +671,7 @@ impl WebdavBackend {
 
         let mut dirs = VecDeque::default();
 
-        while path != "/" {
+        loop {
             // check path first.
             let parent = get_parent(path);
 
@@ -661,13 +684,32 @@ impl WebdavBackend {
                 .webdav_propfind_absolute_path(parent, Some(header_map))
                 .await?;
             match resp.status() {
-                StatusCode::OK | StatusCode::MULTI_STATUS => break,
+                StatusCode::OK => {
+                    break;
+                }
+                StatusCode::MULTI_STATUS => {
+                    let bs = resp.into_body().bytes().await?;
+                    let s = String::from_utf8_lossy(&bs);
+                    let result: Multistatus =
+                        quick_xml::de::from_str(&s).map_err(new_xml_deserialize_error)?;
+
+                    if result.response.is_some() {
+                        break;
+                    }
+
+                    dirs.push_front(parent);
+                    path = parent
+                }
                 StatusCode::NOT_FOUND => {
                     dirs.push_front(parent);
                     path = parent
                 }
                 _ => return Err(parse_error(resp).await?),
             }
+
+            if path == "/" {
+                break;
+            }
         }
 
         for dir in dirs {

diff --git a/core/src/services/webdav/lister.rs b/core/src/services/webdav/lister.rs
@@ -26,12 +26,12 @@ pub struct WebdavLister {
     server_path: String,
     root: String,
     path: String,
-    multistates: Multistatus,
+    response: Vec<ListOpResponse>,
 }
 
 impl WebdavLister {
     /// TODO: sending request in `next_page` instead of in `new`.
-    pub fn new(endpoint: &str, root: &str, path: &str, multistates: Multistatus) -> Self {
+    pub fn new(endpoint: &str, root: &str, path: &str, response: Vec<ListOpResponse>) -> Self {
         // Some services might return the path with suffix `/remote.php/webdav/`, we need to trim them.
         let server_path = http::Uri::from_str(endpoint)
             .expect("must be valid http uri")
@@ -42,37 +42,51 @@ impl WebdavLister {
             server_path,
             root: root.into(),
             path: path.into(),
-            multistates,
+            response,
         }
     }
 }
 
 #[async_trait]
 impl oio::PageList for WebdavLister {
     async fn next_page(&self, ctx: &mut oio::PageContext) -> Result<()> {
-        // Build request instead of clone here.
-        let oes = self.multistates.response.clone();
-
-        for res in oes {
-            let path = res
+        for res in &self.response {
+            let mut path = res
                 .href
                 .strip_prefix(&self.server_path)
-                .unwrap_or(&res.href);
+                .unwrap_or(&res.href)
+                .to_string();
+
+            let meta = res.parse_into_metadata()?;
+
+            // Append `/` to path if it's a dir
+            if !path.ends_with('/') && meta.is_dir() {
+                path += "/"
+            }
 
             // Ignore the root path itself.
             if self.root == path {
                 continue;
             }
 
-            let normalized_path = build_rel_path(&self.root, path);
+            let normalized_path = build_rel_path(&self.root, &path);
             let decoded_path = percent_decode_path(normalized_path.as_str());
 
             if normalized_path == self.path || decoded_path == self.path {
                 // WebDav server may return the current path as an entry.
                 continue;
             }
 
-            let meta = res.parse_into_metadata()?;
+            // Mark files complete if it's an `application/x-checksum` file.
+            //
+            // AFAIK, this content type is only used by jfrog artifactory. And this file is
+            // a shadow file that can't be stat, so we mark it as complete.
+            if meta.contains_metakey(Metakey::ContentType)
+                && meta.content_type() == Some("application/x-checksum")
+            {
+                continue;
+            }
+
             ctx.entries.push_back(oio::Entry::new(&decoded_path, meta))
         }
         ctx.done = true;
@@ -83,11 +97,6 @@ impl oio::PageList for WebdavLister {
 
 #[derive(Deserialize, Debug, PartialEq, Eq, Clone)]
 pub struct Multistatus {
-    pub response: Vec<ListOpResponse>,
-}
-
-#[derive(Deserialize, Debug, PartialEq, Eq, Clone)]
-pub struct MultistatusOptional {
     pub response: Option<Vec<ListOpResponse>>,
 }
 
@@ -333,10 +342,12 @@ mod tests {
         </D:multistatus>"#;
 
         let multistatus = from_str::<Multistatus>(xml).unwrap();
-        assert_eq!(multistatus.response.len(), 2);
-        assert_eq!(multistatus.response[0].href, "/");
+
+        let response = multistatus.response.unwrap();
+        assert_eq!(response.len(), 2);
+        assert_eq!(response[0].href, "/");
         assert_eq!(
-            multistatus.response[0].propstat.prop.getlastmodified,
+            response[0].propstat.prop.getlastmodified,
             "Tue, 01 May 2022 06:39:47 GMT"
         );
     }
@@ -420,22 +431,23 @@ mod tests {
 
         let multistatus = from_str::<Multistatus>(xml).unwrap();
 
-        assert_eq!(multistatus.response.len(), 3);
-        let first_response = &multistatus.response[0];
+        let response = multistatus.response.unwrap();
+        assert_eq!(response.len(), 3);
+        let first_response = &response[0];
         assert_eq!(first_response.href, "/");
         assert_eq!(
             first_response.propstat.prop.getlastmodified,
             "Tue, 07 May 2022 06:39:47 GMT"
         );
 
-        let second_response = &multistatus.response[1];
+        let second_response = &response[1];
         assert_eq!(second_response.href, "/testdir/");
         assert_eq!(
             second_response.propstat.prop.getlastmodified,
             "Tue, 07 May 2022 06:40:10 GMT"
         );
 
-        let third_response = &multistatus.response[2];
+        let third_response = &response[2];
         assert_eq!(third_response.href, "/test_file");
         assert_eq!(
             third_response.propstat.prop.getlastmodified,
@@ -552,9 +564,10 @@ mod tests {
 
         let multistatus: Multistatus = from_str(xml).unwrap();
 
-        assert_eq!(multistatus.response.len(), 9);
+        let response = multistatus.response.unwrap();
+        assert_eq!(response.len(), 9);
 
-        let first_response = &multistatus.response[0];
+        let first_response = &response[0];
         assert_eq!(first_response.href, "/");
         assert_eq!(
             first_response.propstat.prop.getlastmodified,

diff --git a/fixtures/webdav/docker-compose-webdav-jfrog.yml b/fixtures/webdav/docker-compose-webdav-jfrog.yml
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+version: '3.8'
+
+services:
+  jfrog:
+    image: releases-docker.jfrog.io/jfrog/artifactory-oss:latest
+    ports:
+      - "8081:8081"
+      - "8082:8082"
+    volumes:
+      - jfrog-data:/var/opt/jfrog/artifactory
+    healthcheck:
+      test: [ "CMD", "curl", "-f", "-X", "PROPFIND", "-H", "Depth: 1", "-u", "admin:password", "http://localhost:8081/artifactory/example-repo-local" ]
+      interval: 10s
+      timeout: 5s
+      retries: 5
+
+volumes:
+  jfrog-data: