gotham-rs / gotham

A flexible web framework that promotes stability, safety, security and speed.
https://gotham.rs
Other
2.24k stars 125 forks source link

File exclusion from glob route? #291

Open danielpclark opened 5 years ago

danielpclark commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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

whitfin commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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