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

Unable to nest prefixes/routes #1116

Open
DenuxPlays opened this issue Dec 30, 2024 · 4 comments
Open

Unable to nest prefixes/routes #1116

DenuxPlays opened this issue Dec 30, 2024 · 4 comments

Comments

@DenuxPlays
Copy link
Contributor

Description

Nesting prefixes inside AppRoutes is impossible.

To Reproduce

        AppRoutes::with_default_routes() // controller routes below
            .prefix("/api")
            .add_route(controllers::status::non_versioned_routes())
            .prefix("/v1")
            .add_route(controllers::user::routes())
            .add_route(controllers::session::routes())
            .add_route(controllers::openapi::routes())
            .add_route(controllers::status::routes())

Expected Behavior

The route in controllers::status::non_versioned_routes() /status/version should be reachable at /api/status/version

It is actually only reachable at /v1/status/version

Environment:

Additional Context

Is it possible to nest routes like this at all?
Currently the only way I see is to create one, two Routes (one with /api/v1 and one with /api) and then list all routes there.
This would require to make all route functions public and remove all routes() method from every controller.

@DenuxPlays
Copy link
Contributor Author

Same applies to normal Routes

@DenuxPlays DenuxPlays changed the title Unable to nest prefixes Unable to nest prefixes/routes Dec 30, 2024
@DenuxPlays
Copy link
Contributor Author

Maybe a nest() function just like the one that axum uses would be great for Routes.
Like:

Routes::at("api").nest(other_routes)

Or

Routes::new()
    .nest("api", other_routes)

@DenuxPlays
Copy link
Contributor Author

Here is my workaround for now:

use loco_rs::controller::AppRoutes;
use loco_rs::prelude::Routes;

pub struct ExtendedAppRoutes {
    prefix: Option<String>,
    internal_app_routes: AppRoutes,
}

impl ExtendedAppRoutes {
    pub fn empty() -> Self {
        Self {
            prefix: None,
            internal_app_routes: AppRoutes::empty(),
        }
    }

    pub fn prefix(mut self, prefix: &str) -> Self {
        match self.prefix {
            None => self.prefix = Some(prefix.to_string()),
            Some(mut old_prefix) => {
                old_prefix.push_str(prefix);
                self.prefix = Some(old_prefix.to_string())
            }
        }

        self
    }

    pub fn reset_prefix(mut self) -> Self {
        self.prefix = None;

        self
    }

    pub fn add_route(mut self, mut routes: Routes) -> Self {
        let routes_prefix = {
            if let Some(mut prefix) = self.prefix.clone() {
                let routes_prefix = routes.prefix.clone().unwrap_or("".to_string());

                prefix.push_str(routes_prefix.as_str());
                Some(prefix)
            } else {
                routes.prefix.clone()
            }
        };

        if let Some(prefix) = routes_prefix {
            routes = routes.prefix(prefix.as_str());
        }

        self.internal_app_routes = self.internal_app_routes.add_route(routes);

        self
    }
}

impl From<ExtendedAppRoutes> for AppRoutes {
    fn from(value: ExtendedAppRoutes) -> Self {
        value.internal_app_routes
    }
}

Which now the following works as expected:

        ExtendedAppRoutes::empty() // controller routes below
            .prefix("/api")
            .add_route(controllers::status::non_versioned_routes())
            .prefix("/v1")
            .add_route(controllers::user::routes())
            .add_route(controllers::session::routes())
            .add_route(controllers::openapi::routes())
            .add_route(controllers::status::routes())

DenuxPlays added a commit to financrr/financrr-app that referenced this issue Dec 30, 2024
DenuxPlays added a commit to financrr/financrr-app that referenced this issue Jan 2, 2025
* added new entities and repositories

* added first (but useless) controller

* started on implementing account controller

* Getting started on a completely new backend using different technologies

* Started work on new schema

* added transaction tables

* move back to a single permissions table

* added contracts and tags

* added budget tables

* added file attachment support

* updated compose file

* Inited loco-project

Add todo because transaction type has some problems

* schema optimizations

- added global bank accounts
- added type to transaction
- added purpose and note to transactions

* removed useless api_key column

* Added missing schemas and data

- expanded schema
- added currency data

* added some more workspace files

* hopefully finalized schema

- removed inheritance
- fixed budget tables

* More basic setup

* A lot of schema and setup work

* updated crates

* removed useless `linked_transactions` table

* Some more dev fixes

* more initial setup work

* updated dependencies

* Yeah some restructure

* hopefully fixed basic workflows

* added entities and removed some other stuff

* Basic OpenApi setup

* updated rust

* remvoed uuids

* More auth, services, snowflakes etc.

* enhance snowflake generator + added scheduler

* used initializers & clippy fixes

* some improvvements idk

* Updated crates

* Errors and auth start

* Errors and auth start

* use scheduler

* Allow 0 as node id

* More errors and other shit

* renamed a response

* enhanced docker image builds

* Added verify endpoint to users

* Replaced basic macro with proc_macro for enhanced code generation

* enhanced macro with utoipa response generation

* added some more docs

* implemented new macro

* improved released profile

* updated config + storage integration

* mapped all loco errors

* added instance manager

* Improved instance handling and removed cleanup task

* completed user controller

* added session controller

* finished session controller

* made details field optional

* fixed docs

* added status endpoints

* temporary route fixes

Fixes: loco-rs/loco#1116

* Fixed docker builds (hopefully)

* started fixing tests

* some improvements

* added comments on how to solve the ValidationError thing

* fixed model::users tests

* fixed build and test workflow

* added test case for openapi routes

* added some new test + fixed a few endpoints and schema issues

* added more tests

Note:
Somehow if we run tests only the first one succeeds and closes the pool

* fixed all remaining tests + db error

* fixed `.env.dist`

* added extra build step to test ci

* fixed insta building
@anhnmt
Copy link
Contributor

anhnmt commented Jan 6, 2025

I need this feature

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants