Merge pull request #3001 from ProvableHQ/fix/query-with-v1-base-url

[Feature] Detect API version in a `Query`'s `base_url` and support V1 errors

Author: vicsn

ledger/query/src/query/rest.rs

@@ -33,12 +33,19 @@ use std::str::FromStr;
 #[derive(Clone)]
 pub struct RestQuery<N: Network> {
     base_url: http::Uri,
+    /// `true` if the api version is already contained in the base URL.
+    has_api_version: bool,
     _marker: std::marker::PhantomData<N>,
 }
 
 impl<N: Network> From<http::Uri> for RestQuery<N> {
     fn from(base_url: http::Uri) -> Self {
-        Self { base_url, _marker: Default::default() }
+        // Avoid trailing slash when checking the version
+        let path = base_url.path().strip_suffix('/').unwrap_or(base_url.path());
+
+        let has_api_version = path.ends_with(Self::API_V1) || path.ends_with(Self::API_V2);
+
+        Self { base_url, has_api_version, _marker: Default::default() }
     }
 }
 
@@ -78,6 +85,7 @@ impl<N: Network> FromStr for RestQuery<N> {
     fn from_str(str_representation: &str) -> Result<Self> {
         let base_url = str_representation.parse::<http::Uri>().with_context(|| "Failed to parse URL")?;
 
+        // Perform checks.
         if let Some(scheme) = base_url.scheme()
             && *scheme != uri::Scheme::HTTP
             && *scheme != uri::Scheme::HTTPS
@@ -97,7 +105,7 @@ impl<N: Network> FromStr for RestQuery<N> {
             bail!("Base URL for REST endpoints cannot contain a query");
         }
 
-        Ok(Self { base_url, _marker: Default::default() })
+        Ok(Self::from(base_url))
     }
 }
 
@@ -153,8 +161,8 @@ impl<N: Network> QueryTrait<N> for RestQuery<N> {
 }
 
 impl<N: Network> RestQuery<N> {
-    /// The API version to use when querying REST endpoints.
-    const API_VERSION: &str = "v2";
+    const API_V1: &str = "v1";
+    const API_V2: &str = "v2";
 
     /// Returns the transaction for the given transaction ID.
     pub fn get_transaction(&self, transaction_id: &N::TransactionID) -> Result<Transaction<N>> {
@@ -187,22 +195,15 @@ impl<N: Network> RestQuery<N> {
         // This function is only called internally but check for additional sanity.
         ensure!(!route.starts_with('/'), "path cannot start with a slash");
 
+        // Add the API version if it is not already contained in the base URL.
+        let api_version = if self.has_api_version { "" } else { "v2/" };
+
         // Work around a bug in the `http` crate where empty paths will be set to '/' but other paths are not appended with a slash.
         // See [this issue](https://github.com/hyperium/http/issues/507).
         let path = if self.base_url.path().ends_with('/') {
-            format!(
-                "{base_url}{api_version}/{network}/{route}",
-                base_url = self.base_url,
-                api_version = Self::API_VERSION,
-                network = N::SHORT_NAME
-            )
+            format!("{base_url}{api_version}{network}/{route}", base_url = self.base_url, network = N::SHORT_NAME)
         } else {
-            format!(
-                "{base_url}/{api_version}/{network}/{route}",
-                base_url = self.base_url,
-                api_version = Self::API_VERSION,
-                network = N::SHORT_NAME
-            )
+            format!("{base_url}/{api_version}{network}/{route}", base_url = self.base_url, network = N::SHORT_NAME)
         };
 
         Ok(path)
@@ -225,10 +226,23 @@ impl<N: Network> RestQuery<N> {
         if response.status().is_success() {
             response.body_mut().read_json().with_context(|| "Failed to parse JSON response")
         } else {
+            let content_type = response
+                .headers()
+                .get("Content-Type")
+                .ok_or_else(|| anyhow!("Endpoint return error without ContentType"))?
+                .to_str()
+                .with_context(|| "Endpoint returned invalid ContentType")?;
+
             // Convert returned error into an `anyhow::Error`.
-            let error: RestError =
-                response.body_mut().read_json().with_context(|| "Failed to parse JSON error response")?;
-            Err(error.parse().context(format!("Failed to fetch from {endpoint}")))
+            // Depending on the API version, the error is either encoded as a string or as a JSON.
+            if content_type.contains("json") {
+                let error: RestError =
+                    response.body_mut().read_json().with_context(|| "Failed to parse JSON error response")?;
+                Err(error.parse().context(format!("Failed to fetch from {endpoint}")))
+            } else {
+                let error = response.body_mut().read_to_string().with_context(|| "Failed to read error message")?;
+                Err(anyhow!(error).context(format!("Failed to fetch from {endpoint}")))
+            }
         }
     }
 
@@ -304,6 +318,8 @@ mod tests {
         let base = "http://localhost:3030/a/prefix";
         let route = "a/route";
         let query = base.parse::<CurrentQuery>().unwrap();
+        let Query::REST(rest_query) = &query else { panic!() };
+        assert!(!rest_query.has_api_version);
 
         // Test without trailing slash.
         let Query::REST(rest) = query else { panic!() };
@@ -316,4 +332,17 @@ mod tests {
 
         Ok(())
     }
+
+    #[test]
+    fn test_rest_url_parse_with_api_version() {
+        let base = "http://localhost:3030/a/prefix/v1";
+        let query = base.parse::<CurrentQuery>().unwrap();
+        let Query::REST(rest_query) = &query else { panic!() };
+        assert!(rest_query.has_api_version);
+
+        let base = "http://localhost:3030/a/prefix/v2/";
+        let query = base.parse::<CurrentQuery>().unwrap();
+        let Query::REST(rest_query) = &query else { panic!() };
+        assert!(rest_query.has_api_version);
+    }
 }