breaking: change url format: {usecase}/{scope1}/.../data/{key} (#203)

### main points
- changes objectstore's url format to:
  ```
  {usecase}/{scope1}/.../objects/{key}
  ```
- updates both clients to use the new format

any deployed uses of objectstore will break when we roll this out. we
are pre-production so that feels acceptable as long as we make sure we
turn off any feature flags we've got out there first until we deploy +
update clients

we are discussing renaming `scope` but i'll do that in a different PR

### extra detail
axum's route syntax doesn't allow `{usecase}/{*scope}/objects/{*key}`
for variable-length scopes because that sort of `*` wildcard is only
allowed at the end of a path. to work around that, i wrote a custom
`serde::Deserialize` implementation for `ObjectPath`. now a
`/{*object_path}` route produces a `Path<ObjectPath>`. as a bonus, it
lets us get rid of the mostly-duplicated `put_object_nokey` handler

`ObjectPath` itself was changed in two ways:
- `scope` is now a `Vec<String>` containing each scope component. this
will make auth logic easier
- i opted not to split each scope component on `.` and store each k/v
pair in an `IndexMap` as i don't think we really need them separated. it
just makes re-serializing the path more expensive. we can change this
later though
- the `Deserialize` implementation will handle generating a UUIDv4 if
one wasn't found in the path

Close FS-212

---------

Co-authored-by: Arpad Borsos <[email protected]>

Author: matt-codecov

clients/python/src/objectstore_client/client.py

@@ -5,7 +5,6 @@
 from dataclasses import asdict, dataclass
 from io import BytesIO
 from typing import IO, Any, Literal, NamedTuple, cast
-from urllib.parse import urlencode
 
 import sentry_sdk
 import urllib3
@@ -207,12 +206,11 @@ def _make_headers(self) -> dict[str, str]:
         return {}
 
     def _make_url(self, key: str | None, full: bool = False) -> str:
-        base_path = f"/v1/{key}" if key else "/v1/"
-        qs = urlencode({"usecase": self._usecase.name, "scope": self._scope})
+        base_path = f"/v1/{self._usecase.name}/{self._scope}/objects/{key or ''}"
         if full:
-            return f"http://{self._pool.host}:{self._pool.port}{base_path}?{qs}"
+            return f"http://{self._pool.host}:{self._pool.port}{base_path}"
         else:
-            return f"{base_path}?{qs}"
+            return f"{base_path}"
 
     def put(
         self,

clients/python/tests/test_smoke.py

@@ -1,3 +1,18 @@
+from objectstore_client import Client, Usecase
+
+
 def test_imports() -> None:
     import objectstore_client  # noqa: F401
     from objectstore_client import client, metadata, metrics  # noqa: F401
+
+
+def test_object_url() -> None:
+    client = Client("http://127.0.0.1:8888/")
+    session = client.session(
+        Usecase("testing"), org=12345, project=1337, app_slug="email_app"
+    )
+
+    assert (
+        session.object_url("foo/bar")
+        == "http://127.0.0.1:8888/v1/testing/org.12345/project.1337/app_slug.email_app/objects/foo/bar"
+    )

clients/rust/src/client.rs

@@ -7,7 +7,7 @@ use futures_util::stream::BoxStream;
 use objectstore_types::ExpirationPolicy;
 use url::Url;
 
-pub use objectstore_types::{Compression, PARAM_SCOPE, PARAM_USECASE};
+pub use objectstore_types::Compression;
 
 const USER_AGENT: &str = concat!("objectstore-client/", env!("CARGO_PKG_VERSION"));
 
@@ -258,7 +258,7 @@ impl Scope {
     }
 
     /// Extends this Scope by creating a new sub-scope nested within it.
-    pub fn push<V>(self, key: &str, value: &V) -> Self
+    pub fn push<V>(self, key: &str, value: V) -> Self
     where
         V: std::fmt::Display,
     {
@@ -410,22 +410,29 @@ pub struct Session {
 pub type ClientStream = BoxStream<'static, io::Result<Bytes>>;
 
 impl Session {
+    /// Generates a GET url to the object with the given `key`.
+    ///
+    /// This can then be used by downstream services to fetch the given object.
+    /// NOTE however that the service does not strictly follow HTTP semantics,
+    /// in particular in relation to `Accept-Encoding`.
+    pub fn object_url(&self, object_key: &str) -> Url {
+        let mut url = self.client.service_url.clone();
+        let path = format!(
+            "v1/{}/{}/objects/{object_key}",
+            self.scope.usecase.name, self.scope.scope
+        );
+        url.set_path(&path);
+        url
+    }
+
     pub(crate) fn request(
         &self,
         method: reqwest::Method,
-        resource_id: &str,
-    ) -> crate::Result<reqwest::RequestBuilder> {
-        let mut url = self.client.service_url.clone();
-        url.path_segments_mut()
-            .map_err(|_| crate::Error::InvalidUrl {
-                message: format!("The URL {} cannot be a base", self.client.service_url),
-            })?
-            .extend(&["v1", resource_id]);
+        object_key: &str,
+    ) -> reqwest::RequestBuilder {
+        let url = self.object_url(object_key);
 
-        let mut builder = self.client.reqwest.request(method, url).query(&[
-            (PARAM_SCOPE, self.scope.scope.as_str()),
-            (PARAM_USECASE, self.scope.usecase.name.as_ref()),
-        ]);
+        let mut builder = self.client.reqwest.request(method, url);
 
         if self.client.propagate_traces {
             let trace_headers =
@@ -435,6 +442,26 @@ impl Session {
             }
         }
 
-        Ok(builder)
+        builder
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_object_url() {
+        let client = Client::new("http://127.0.0.1:8888/").unwrap();
+        let usecase = Usecase::new("testing");
+        let scope = usecase
+            .for_project(12345, 1337)
+            .push("app_slug", "email_app");
+        let session = client.session(scope).unwrap();
+
+        assert_eq!(
+            session.object_url("foo/bar").to_string(),
+            "http://127.0.0.1:8888/v1/testing/org.12345/project.1337/app_slug.email_app/objects/foo/bar"
+        )
     }
 }

clients/rust/src/delete.rs

@@ -24,7 +24,7 @@ impl DeleteBuilder {
     /// Sends the `DELETE` request.
     pub async fn send(self) -> crate::Result<DeleteResponse> {
         self.session
-            .request(reqwest::Method::DELETE, &self.key)?
+            .request(reqwest::Method::DELETE, &self.key)
             .send()
             .await?;
         Ok(())

clients/rust/src/get.rs

@@ -77,7 +77,7 @@ impl GetBuilder {
     pub async fn send(self) -> crate::Result<Option<GetResponse>> {
         let response = self
             .session
-            .request(reqwest::Method::GET, &self.key)?
+            .request(reqwest::Method::GET, &self.key)
             .send()
             .await?;
         if response.status() == StatusCode::NOT_FOUND {

clients/rust/src/put.rs

@@ -141,7 +141,7 @@ impl PutBuilder {
         let mut builder = self.session.request(
             reqwest::Method::PUT,
             self.key.as_deref().unwrap_or_default(),
-        )?;
+        );
 
         let body = match (self.metadata.compression, self.body) {
             (Some(Compression::Zstd), PutBody::Buffer(bytes)) => {

objectstore-server/src/endpoints.rs

@@ -4,22 +4,22 @@ use std::io;
 
 use anyhow::Context;
 use axum::body::Body;
-use axum::extract::{Path, Query, State};
+use axum::extract::{Path, State};
 use axum::http::{HeaderMap, StatusCode};
 use axum::response::{IntoResponse, Response};
 use axum::routing::{get, put};
 use axum::{Json, Router};
 use futures_util::{StreamExt, TryStreamExt};
 use objectstore_service::ObjectPath;
 use objectstore_types::Metadata;
-use serde::{Deserialize, Serialize};
+use serde::Serialize;
 
 use crate::error::ApiResult;
 use crate::state::ServiceState;
 
 pub fn routes() -> Router<ServiceState> {
-    let service_routes = Router::new().route("/", put(put_object_nokey)).route(
-        "/{*key}",
+    let service_routes = Router::new().route(
+        "/{*path}",
         put(put_object).get(get_object).delete(delete_object),
     );
 
@@ -32,52 +32,17 @@ async fn health() -> impl IntoResponse {
     "OK"
 }
 
-#[derive(Deserialize, Debug)]
-struct ContextParams {
-    scope: String,
-    usecase: String,
-}
-
 #[derive(Debug, Serialize)]
 struct PutBlobResponse {
     key: String,
 }
 
-async fn put_object_nokey(
-    State(state): State<ServiceState>,
-    Query(params): Query<ContextParams>,
-    headers: HeaderMap,
-    body: Body,
-) -> ApiResult<impl IntoResponse> {
-    let path = ObjectPath {
-        usecase: params.usecase,
-        scope: params.scope,
-        key: uuid::Uuid::new_v4().to_string(),
-    };
-    populate_sentry_scope(&path);
-    let metadata =
-        Metadata::from_headers(&headers, "").context("extracting metadata from headers")?;
-
-    let stream = body.into_data_stream().map_err(io::Error::other).boxed();
-    let key = state.service.put_object(path, &metadata, stream).await?;
-
-    Ok(Json(PutBlobResponse {
-        key: key.key.to_string(),
-    }))
-}
-
 async fn put_object(
     State(state): State<ServiceState>,
-    Query(params): Query<ContextParams>,
-    Path(key): Path<String>,
+    Path(path): Path<ObjectPath>,
     headers: HeaderMap,
     body: Body,
 ) -> ApiResult<impl IntoResponse> {
-    let path = ObjectPath {
-        usecase: params.usecase,
-        scope: params.scope,
-        key,
-    };
     populate_sentry_scope(&path);
     let metadata =
         Metadata::from_headers(&headers, "").context("extracting metadata from headers")?;
@@ -92,16 +57,9 @@ async fn put_object(
 
 async fn get_object(
     State(state): State<ServiceState>,
-    Query(params): Query<ContextParams>,
-    Path(key): Path<String>,
+    Path(path): Path<ObjectPath>,
 ) -> ApiResult<Response> {
-    let path = ObjectPath {
-        usecase: params.usecase,
-        scope: params.scope,
-        key,
-    };
     populate_sentry_scope(&path);
-
     let Some((metadata, stream)) = state.service.get_object(&path).await? else {
         return Ok(StatusCode::NOT_FOUND.into_response());
     };
@@ -114,14 +72,8 @@ async fn get_object(
 
 async fn delete_object(
     State(state): State<ServiceState>,
-    Query(params): Query<ContextParams>,
-    Path(key): Path<String>,
+    Path(path): Path<ObjectPath>,
 ) -> ApiResult<impl IntoResponse> {
-    let path = ObjectPath {
-        usecase: params.usecase,
-        scope: params.scope,
-        key,
-    };
     populate_sentry_scope(&path);
 
     state.service.delete_object(&path).await?;

objectstore-service/src/backend/bigtable.rs

@@ -359,7 +359,7 @@ mod tests {
     fn make_key() -> ObjectPath {
         ObjectPath {
             usecase: "testing".into(),
-            scope: "testing".into(),
+            scope: vec!["testing".into()],
             key: Uuid::new_v4().to_string(),
         }
     }

objectstore-service/src/backend/gcs.rs

@@ -450,7 +450,7 @@ mod tests {
     fn make_key() -> ObjectPath {
         ObjectPath {
             usecase: "testing".into(),
-            scope: "testing".into(),
+            scope: vec!["testing".into()],
             key: Uuid::new_v4().to_string(),
         }
     }

objectstore-service/src/backend/local_fs.rs

@@ -122,7 +122,7 @@ mod tests {
 
         let key = ObjectPath {
             usecase: "testing".into(),
-            scope: "testing".into(),
+            scope: vec!["testing".into()],
             key: "testing".into(),
         };
         let metadata = Metadata {