Merge pull request #198 from dandi/gh-164

Add a `DavRequest` extractor
dandi · Nov 12, 2024 · 1e4148d · 1e4148d
2 parents 0d53f0e + ccb94af
commit 1e4148d
Show file tree

Hide file tree

Showing 3 changed files with 98 additions and 57 deletions.
diff --git a/doc/architecture.md b/doc/architecture.md
@@ -1,7 +1,7 @@
 Overview of `dandidav` Architecture
 ===================================
 
-*This document is up-to-date as of 2024 July 26.*
+*This document is up-to-date as of 2024 November 12.*
 
 > [!NOTE]
 > A new architecture is currently being planned for the code.  See [issue
@@ -75,13 +75,10 @@ General
       resource and request method
 
 - The entry point to `DandiDav`'s functionality is its
-  [`handle_request()`][handle-request] method, which calls
-  `DandiDav::inner_handle_request()` and then performs error handling and
-  setting of universal response headers on the result.
-
-- [`DandiDav::inner_handle_request()`][] parses the request and passes the
-  results to either `DandiDav::get()` or `DandiDav::propfind()`, depending on
-  the request verb.
+  [`handle_request()`][handle-request] method, which parses the request, passes
+  the results to either `DandiDav::get()` or `DandiDav::propfind()` depending
+  on the request verb, and then performs error handling and setting of
+  universal response headers on the result.
 
 - [`DandiDav::get()`][] and [`DandiDav::propfind()`][] both call
   `DandiDav::get_resource_with_children()` to fetch information about the
@@ -214,10 +211,9 @@ implementation-defined expiry criteria are met.
 [extract-depth]: https://github.com/dandi/dandidav/blob/9b9b04872065b8132657b878bad324b2dff68a97/src/dav/util.rs#L99-L111
 
 [`DandiDav`]: https://github.com/dandi/dandidav/blob/8d058fe0e561e56ecd3d4c5cd49ca9403b0d196a/src/dav/mod.rs#L37
-[handle-request]: https://github.com/dandi/dandidav/blob/8d058fe0e561e56ecd3d4c5cd49ca9403b0d196a/src/dav/mod.rs#L71
+[handle-request]: https://github.com/dandi/dandidav/blob/d0401d96a45bd381b86bdf2e31d6d80898ccf737/src/dav/mod.rs#L70
 [`DandiDav::get()`]: https://github.com/dandi/dandidav/blob/8d058fe0e561e56ecd3d4c5cd49ca9403b0d196a/src/dav/mod.rs#L129
 [`DandiDav::propfind()`]: https://github.com/dandi/dandidav/blob/8d058fe0e561e56ecd3d4c5cd49ca9403b0d196a/src/dav/mod.rs#L165
-[`DandiDav::inner_handle_request()`]: https://github.com/dandi/dandidav/blob/8d058fe0e561e56ecd3d4c5cd49ca9403b0d196a/src/dav/mod.rs#L95
 [`DandiDav::get_resource()`]: https://github.com/dandi/dandidav/blob/8d058fe0e561e56ecd3d4c5cd49ca9403b0d196a/src/dav/mod.rs#L216
 [`DandiDav::get_resource_with_children()`]: https://github.com/dandi/dandidav/blob/8d058fe0e561e56ecd3d4c5cd49ca9403b0d196a/src/dav/mod.rs#L272
 

diff --git a/src/dav/mod.rs b/src/dav/mod.rs
@@ -18,7 +18,7 @@ use crate::zarrman::*;
 use axum::{
     body::Body,
     extract::Request,
-    http::{header::CONTENT_TYPE, response::Response, Method, StatusCode},
+    http::{header::CONTENT_TYPE, response::Response, StatusCode},
     response::{IntoResponse, Redirect},
     RequestExt,
 };
@@ -60,16 +60,26 @@ impl DandiDav {
     /// Handle an incoming HTTP request and return a response.  This method
     /// must return `Result<T, Infallible>` for compatibility with `axum`.
     ///
-    /// This method delegates almost all work to
-    /// [`DandiDav::inner_handle_request()`], after which it handles any
-    /// errors returned by logging them and converting them to 4xx or 5xx
-    /// responses, as appropriate.  The final response also has
+    /// The request parameters from the URL path and (for `PROPFIND`) "Depth"
+    /// header & request body are parsed & extracted and then passed to the
+    /// appropriate method for the request's verb for dedicated handling.
+    ///
+    /// Any errors returned are logged and converted to 4xx or 5xx responses,
+    /// as appropriate.  The final response also has
     /// [`WEBDAV_RESPONSE_HEADERS`] added.
     pub(crate) async fn handle_request(
         &self,
         req: Request<Body>,
     ) -> Result<Response<Body>, Infallible> {
-        let resp = self.inner_handle_request(req).await.unwrap_or_else(|e| {
+        let resp = match req.extract::<DavRequest, _>().await {
+            Ok(DavRequest::Get { path, pathparts }) => self.get(&path, pathparts).await,
+            Ok(DavRequest::Propfind { path, depth, query }) => {
+                self.propfind(&path, depth, query).await
+            }
+            Ok(DavRequest::Options) => Ok(StatusCode::NO_CONTENT.into_response()),
+            Err(r) => Ok(r),
+        };
+        let resp = resp.unwrap_or_else(|e| {
                 let class = e.class();
                 let e = anyhow::Error::from(e);
                 tracing::info!(error = ?e, status = class.to_status().as_u16(), "Error processing request");
@@ -82,39 +92,6 @@ impl DandiDav {
         Ok((WEBDAV_RESPONSE_HEADERS, resp).into_response())
     }
 
-    /// Extract & parse request parameters from the URL path and (for
-    /// `PROPFIND`) "Depth" header and request body.  The parsed parameters are
-    /// then passed to the appropriate method for the request's verb for
-    /// dedicated handling.
-    async fn inner_handle_request(&self, req: Request<Body>) -> Result<Response<Body>, DavError> {
-        let uri_path = req.uri().path();
-        match req.method() {
-            &Method::GET => {
-                let Some(parts) = split_uri_path(uri_path) else {
-                    // TODO: Log something
-                    return Ok(not_found());
-                };
-                let Some(path) = DavPath::from_components(parts.clone()) else {
-                    // TODO: Log something
-                    return Ok(not_found());
-                };
-                self.get(&path, parts).await
-            }
-            &Method::OPTIONS => Ok(StatusCode::NO_CONTENT.into_response()),
-            m if m.as_str().eq_ignore_ascii_case("PROPFIND") => {
-                let Some(path) = split_uri_path(uri_path).and_then(DavPath::from_components) else {
-                    // TODO: Log something
-                    return Ok(not_found());
-                };
-                match req.extract::<(FiniteDepth, PropFind), _>().await {
-                    Ok((depth, pf)) => self.propfind(&path, depth, pf).await,
-                    Err(r) => Ok(r),
-                }
-            }
-            _ => Ok(StatusCode::METHOD_NOT_ALLOWED.into_response()),
-        }
-    }
-
     /// Handle a `GET` request for the given `path`.
     ///
     /// `pathparts` contains the individual components of the request URL path
@@ -473,8 +450,3 @@ impl ErrorClass {
         }
     }
 }
-
-/// Generate a 404 response
-fn not_found() -> Response<Body> {
-    (StatusCode::NOT_FOUND, "404\n").into_response()
-}
diff --git a/src/dav/util.rs b/src/dav/util.rs
@@ -1,14 +1,17 @@
+use super::path::{split_uri_path, DavPath};
+use super::xml::PropFind;
 use super::VersionSpec;
 use crate::consts::DAV_XML_CONTENT_TYPE;
 use crate::dandi::DandisetId;
 use crate::httputil::HttpUrl;
-use crate::paths::PureDirPath;
+use crate::paths::{Component, PureDirPath};
 use axum::{
     async_trait,
     body::Body,
-    extract::FromRequestParts,
-    http::{header::CONTENT_TYPE, request::Parts, response::Response, StatusCode},
+    extract::{FromRequest, FromRequestParts, Request},
+    http::{header::CONTENT_TYPE, request::Parts, response::Response, Method, StatusCode},
     response::IntoResponse,
+    RequestExt,
 };
 use indoc::indoc;
 use percent_encoding::{percent_encode, AsciiSet, NON_ALPHANUMERIC};
@@ -86,6 +89,71 @@ pub(super) fn format_modifieddate(dt: OffsetDateTime) -> String {
         .expect("formatting an OffsetDateTime in RFC 1123 format should not fail")
 }
 
+/// A request to the WebDAV server, parsed into its constituent parts
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub(super) enum DavRequest {
+    /// A `GET` request
+    Get {
+        /// The request path
+        path: DavPath,
+
+        /// The individual components of the request path prior to parsing into
+        /// `path`.  This is needed for things like breadcrumbs in HTML views
+        /// of collection resources.
+        pathparts: Vec<Component>,
+    },
+
+    /// A `PROPFIND` request
+    Propfind {
+        /// The request path
+        path: DavPath,
+
+        /// The value of the `Depth` header
+        depth: FiniteDepth,
+
+        /// The parsed request body.  (Empty bodies are defaulted to "allprop"
+        /// during parsing as per the RFC.)
+        query: PropFind,
+    },
+
+    /// An `OPTIONS` request
+    Options,
+}
+
+#[async_trait]
+impl<S: Send + Sync> FromRequest<S> for DavRequest {
+    type Rejection = Response<Body>;
+
+    async fn from_request(req: Request<Body>, state: &S) -> Result<Self, Self::Rejection> {
+        let uri_path = req.uri().path();
+        match req.method() {
+            &Method::GET => {
+                let Some(pathparts) = split_uri_path(uri_path) else {
+                    // TODO: Log something
+                    return Err(not_found());
+                };
+                let Some(path) = DavPath::from_components(pathparts.clone()) else {
+                    // TODO: Log something
+                    return Err(not_found());
+                };
+                Ok(DavRequest::Get { path, pathparts })
+            }
+            &Method::OPTIONS => Ok(DavRequest::Options),
+            m if m.as_str().eq_ignore_ascii_case("PROPFIND") => {
+                let Some(path) = split_uri_path(uri_path).and_then(DavPath::from_components) else {
+                    // TODO: Log something
+                    return Err(not_found());
+                };
+                let (depth, query) = req
+                    .extract_with_state::<(FiniteDepth, PropFind), _, _>(state)
+                    .await?;
+                Ok(DavRequest::Propfind { path, depth, query })
+            }
+            _ => Err(StatusCode::METHOD_NOT_ALLOWED.into_response()),
+        }
+    }
+}
+
 /// A non-infinite `Depth` WebDAV header value
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
 pub(super) enum FiniteDepth {
@@ -151,6 +219,11 @@ impl Serialize for Href {
     }
 }
 
+/// Generate a 404 response
+pub(super) fn not_found() -> Response<Body> {
+    (StatusCode::NOT_FOUND, "404\n").into_response()
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;