Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add path based routing #751

Merged
merged 2 commits into from
Dec 18, 2023
Merged

Add path based routing #751

merged 2 commits into from
Dec 18, 2023

Conversation

avinassh
Copy link
Member

@avinassh avinassh commented Dec 4, 2023

This PR adds a way to extract namespace from the path. This is done so that users can have smooth experience in turso dev and they can connect to multiple namespaces without editing /etc/hosts file. This patch does not require any changes in shell or libraries.

  1. The expected path format is /dev/<namespace> I am not sure if we want to enable to this feature on the platform. So for now, I kept it behind the /dev sub path
  2. Similarly, I have enabled only /dev/:namespace/v2/pipeline and /dev/:namespace/v3/pipeline paths.

I have tested this with turso shell and go sdk.

fn split_namespace(host: &str) -> crate::Result<NamespaceName> {
let (ns, _) = host.split_once('.').ok_or_else(|| {
Error::InvalidHost("host header should be in the format <namespace>.<...>".into())
})?;
let ns = NamespaceName::from_string(ns.to_owned())?;
Ok(ns)
}

fn extract_namespace(parts: &Parts) -> Option<String> {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is probably an axum way to extract this namespace from the parts, but I could not figure out how to do it

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's simpler than it seems: you need implement Deserialize for NamespaceName:

impl<'de> Deserialize for NamespaceName<'de> {
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
    where
        D: serde::Deserializer<'de> {
            struct V;

            impl<'de> Visitor<'de> for V {
                type Value = NamespaceName;

                fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result {
                    write!(f, "a valid namespace name");
                }

                fn visit_string<E>(self, v: String) -> Result<Self::Value, E>
                where
                    E: serde::de::Error, {
                        NamespaceName::from_string(v).map_err(|e| E::custom(e))
                }
            }
        }
}

when you have that, you can use the Path extractor: https://docs.rs/axum/latest/axum/extract/struct.Path.html

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can now create an extractor, that gets the namespace from path, without parsing anything yourself:

pub struct MakeConnectionExtractorPath<D>(pub Arc<dyn MakeConnection<Connection = D>>);
#[async_trait::async_trait]
impl<F> FromRequestParts<AppState<F>>
    for MakeConnectionExtractorPath<<F::Database as Database>::Connection>
where
    F: MakeNamespace,
{
    type Rejection = Error;

    async fn from_request_parts(
        parts: &mut Parts,
        state: &AppState<F>,
    ) -> Result<Self, Self::Rejection> {
        let auth = Authenticated::from_request_parts(parts, state).await?;
        let ns = axum::extract::Path::<NamespaceName>::from_request_parts(parts, state).await?;
        Ok(Self(
            state
                .namespaces
                .with_authenticated(ns, auth, |ns| ns.db.connection_maker())
                .await?,
        ))
    }
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

finaly, you create a handler:

                    async fn handle_hrana_pipeline<F: MakeNamespace>(
                        AxumState(state): AxumState<AppState<F>>,
                        MakeConnectionExtractorPath(connection_maker): MakeConnectionExtractorPath<
                            <F::Database as Database>::Connection,
                        >,
                        auth: Authenticated,
                        req: Request<Body>,
                    ) -> Result<Response<Body>, Error> {
                        Ok(state
                            .hrana_http_srv
                            .handle_request(
                                connection_maker,
                                auth,
                                req,
                                hrana::http::Endpoint::Pipeline,
                                hrana::Version::Hrana2,
                                hrana::Encoding::Json,
                            )
                            .await?)
                    }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much, I shall try this 🙏

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MarinPostma I have rewritten it and implemented from_request_parts. It is mostly same as you suggested, one minor change I did was to introduce dynamic hrana version in the path so that same method can be used for both versions.

@MarinPostma MarinPostma added this pull request to the merge queue Dec 18, 2023
Merged via the queue into main with commit a321b86 Dec 18, 2023
12 checks passed
@MarinPostma MarinPostma deleted the ns-path branch December 18, 2023 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants