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

fix: fix method action regex to extract Route MethodRouter #1154

Merged
merged 1 commit into from
Jan 8, 2025

Conversation

lonelyhentxi
Copy link
Contributor

screenshot-before

When added a MethodRouter created by axum's post_service to the routes in a controller of loco-rs, it failed to correctly extract that it was a POST method. The scenario can be seen here: an axum post_service use case.

use async_graphql::http::{playground_source, GraphQLPlaygroundConfig};
use async_graphql_axum::GraphQL;
use axum::{
    response::{Html, IntoResponse},
    routing::{get, post_service},
};
use loco_rs::prelude::Routes;

use crate::graphql::service::AppGraphQLService;

pub async fn graphql_playground() -> impl IntoResponse {
    Html(playground_source(GraphQLPlaygroundConfig::new(
        "/api/graphql",
    )))
}

pub fn routes(graphql_service: &AppGraphQLService) -> Routes {
    Routes::new()
        .prefix("/graphql")
        .add("/playground", get(graphql_playground))
        .add(
            "/",
            post_service(GraphQL::new(graphql_service.schema.clone())),
        )
}

The issue originates from the regex here: source code:

fn get_describe_method_action() -> &'static Regex {
    DESCRIBE_METHOD_ACTION.get_or_init(|| Regex::new(r"\b(\w+):\s*BoxedHandler\b").unwrap())
}

Which does not account for the match cases of MethodEndpoint::Route created by the {method}_service series of functions: MethodEndpoint::Route.

    pub fn on_service<T>(self, filter: MethodFilter, svc: T) -> Self
    where
        T: Service<Request, Error = E> + Clone + Send + Sync + 'static,
        T::Response: IntoResponse + 'static,
        T::Future: Send + 'static,
    {
        self.on_endpoint(filter, MethodEndpoint::Route(Route::new(svc)))
    }

To address this issue, I updated the regular expression to include this case. Below is the behavior before and after the modification.

Regex::new(r"\b(\w+):\s*(BoxedHandler|Route)\b")

After fixed:

screenshot-after

@jondot jondot merged commit e54d1ac into loco-rs:master Jan 8, 2025
9 checks passed
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.

2 participants