serveStaticFile should match the pattern given to the router

[Geod24]

Use case:

• I have Phobos' doc under 'public/phobos//yyyy_xxxx.html'
• I want to serve them under '/phobos/yyyy_xxxx.html'

So obviously I tried: router.get("/phobos/*", serveStaticFiles("./public/phobos/2.066.1/"));

Which doesn't work, because serveStaticFiles expects to find ''./public/phobos/2.066.1/phobos". I think this behavior isn't what an user would expect by default.

Now, this is hard to change as-is IMO (because we'll have to repeat ourselve with the pattern. Maybe serveStaticFiles and consort should be UFCS function call on the router ?

router.serveStaticFiles("/phobos/*", "./public/phobos/2.066.1"); This could also allow for more advanced use: router.serveStaticFiles("/phobos/(2\.[0-9]{3}\.[0-9])/*", "./public/phobos/{0}/");

Thoughts ? I was planning to develop something similar for the REST module (pattern matching + input validation), but this could be a good starter to define a set of functionalities library-wide.

[s-ludwig]

There is HTTPFileServerSettings.serverPathPrefix for this use case. It's not short and you have to repeat the pattern/prefix, but I think that both, the use case and the use of staticFiles within a program is rare enough, that a convenience change that sacrifices on separation of concerns isn't worth the trade-off.

For the REST, module, there is the vibe.web.validation module, currently only used for vibe.web.web. vibe.web.rest should definitely be adjusted to be compatible with that instead of rolling a custom solution.

[Geod24]

There is HTTPFileServerSettings.serverPathPrefix for this use case.

Nice.

but I think that both, the use case and the use of staticFiles within a program is rare enough, that a convenience change that sacrifices on separation of concerns isn't worth the trade-off.

The real question is how often, when you use serveStaticFiles do you need to specify the prefix ? My rule of thumb for sane defaults: If you find yourself using some configuration a lot for different tasks, then it should be the default. And I keep bumping into this issue, and I'm surely not the only one. I'm actually pretty surprised by your statement, I doubt there's a lot of sites out there that don't need to serve css / js / images, so most of the site would have a use of serveStaticFile (even if not intensive).

Also, SoC is a thing, but so is DRY. I think this change would cover both. For example, serveStaticFile (no 's') has an assert to warn the user about serverPathPrefix, which clearly goes against SoC.

For the REST, module, there is the vibe.web.validation module, currently only used for vibe.web.web. vibe.web.rest should definitely be adjusted to be compatible with that instead of rolling a custom solution.

Right, I'll see how I can re-use that. It's way higher-level than what I was thinking about, which is probably a good thing.