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

File exclusion from glob route? #291

Open
danielpclark opened this issue Nov 30, 2018 · 8 comments
Open

File exclusion from glob route? #291

danielpclark opened this issue Nov 30, 2018 · 8 comments

Comments

@danielpclark
Copy link

danielpclark commented Nov 30, 2018

Given

pub fn router() -> Router {
    build_simple_router(|route| {
        route.
            get("public/*").
            to_dir(
                FileOptions::new("public").
                    with_cache_control("no-cache").
                    with_gzip(true).
                    build(),
            );
    })  
}

How can I exclude a packs/manifest.json file in that directory?

@yanganto
Copy link

yanganto commented Dec 2, 2018

I have a similar problem, but to sending file only with .json file extension.
Besides, is there any possible way when requesting /pubic/somedata and the handler send the /public/somdata.json.

Thanks.

@whitfin
Copy link
Contributor

whitfin commented Dec 2, 2018

@danielpclark I don't believe the API supposed exclusion yet, but we'd be totally willing to accept a PR adding it! I wonder if @colinbankier has any thoughts on API methods for that?

@yanganto for that you would have to expose that manually via to_file.

@danielpclark
Copy link
Author

danielpclark commented Dec 2, 2018

Just thinking out loud…

The get method has a signature requiring a &str at the moment. I think the responsibility of the collection should be a custom object passed to get and not that of get itself — so changing this:

fn get<'b>(&'b mut self, path: &str) -> DefaultSingleRouteBuilder<'b, C, P> {
    self.request(vec![Method::GET], path)
}

To something like this:

fn get<'b>(&'b mut self, path: Into<FileCollection>) -> DefaultSingleRouteBuilder<'b, C, P> {
    self.request(vec![Method::GET], path.into())
}

And we'll of course have our own impl Into<FileCollection> for &str which makes it backwards compatible.

Since every type has impl Into<T> for T on it anyone can pass in their own custom FileCollection instance as a parameter.

The internals would be super simple to… pretty much a container for a Vec I'm thinking. Although I haven't looked too closely at the glob semantics but I think it's doable in a similar fashion.

Further thinking, without looking into glob semantics, the FileCollection object can be given any string representing a path in something like a given method and either return a Some() or None based on the matching rules FileCollection determines. That would mean the FileCollection struct would require people to implement this one trait/method:

trait Given {
    fn given(&self, &str) -> Option<>;
}

@danielpclark
Copy link
Author

Maybe FileCollection is a bad name... perhaps PathGrouping?

@whitfin
Copy link
Contributor

whitfin commented Dec 2, 2018

@danielpclark I think I'd prefer something like AssetGroup or AssetCollection, rather than Path or File.

Into<AssetGroup> seems fine; we could implement it for &str, and it would be possible to do it for something like Regex fairly easily. For the globbing aspect, perhaps we just evaluate it at creation time and create a mapping index based on the globs at that time.

@colinbankier
Copy link
Collaborator

Thanks @danielpclark - I hadn't put any thought into file exclusion yet. Sounds like a useful feature for sure. (Although I did wonder whether in some cases it's preferable to update your static assets pipeline to output only what you want instead.)
I like the direction of your suggestion - however, the get function is on the router builder, and applies to all kinds of routes, not just static file handlers, so changing that wouldn't seem a good fit. (Unless I misunderstood your suggestion?)
Adding it as an option to FileOptions would seem the most natural fit to me. Thinking out load, something simple like exclude(regex) option on FileOptions was my first thought. This would be a simpler api, and be pretty simple to implement. Thoughts on that?

@danielpclark
Copy link
Author

danielpclark commented Dec 4, 2018

@colinbankier Yes you've understood me. I wasn't sure where the acceptance and denial of routing occured and had assumed it was in the get side of things. Clearly I was mistaken.

Adding exclude(regex) to FileOptions sounds perfect.

Although I did wonder whether in some cases it's preferable to update your static assets pipeline to output only what you want instead.

Yeah I've been thinking about this myself and I'll give you a scenario.

ScenarioWe now have access to the entire JavaScript asset management system Webpack provides with the webpacker crate. This pre-processes and bundles assets of all types and places them in a public folder. Since JavaScript apps are often entirely client side loaded they've included a manifest file of all the generated asset output for the JavaScript client to potentially download. Since we're handling routing on the server with Gotham we don't need to deliver this file as we'll take care of the routing manifest.json provides ourselves. Now the webpacker crate currently processes this file during the build phase of compilation and during deployment on a server like Heroku the files are made into a read only image. So if I wanted to change the manifest.json file directory I would need to move it before the build process and add the new path to the manifest creation command or move/remove it after the manifest is created... which is probably okay. But
— do I really want to tell the users of the webpacker crate that they have to add in the extra step of moving/removing files in their build process every time?

So your suggestion of the exclude method sounds much more preferable.

@colinbankier
Copy link
Collaborator

Ok, great, thanks for the background on your scenario.
Let us know if you're interested in working on the exclude feature :)

@msrd0 msrd0 added this to the 0.6 milestone Sep 11, 2020
@msrd0 msrd0 added help wanted and removed refactor labels Mar 2, 2021
@msrd0 msrd0 removed this from the 0.6 milestone Mar 2, 2021
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

5 participants