Feature: optionally suppress file extensions in generated links

[guspower]

Small feature enhancement for your consideration.

Added suppress_file_extensions configuration property that, when set to true, will cause sqlpage.link to suppress the .sql extension in the generated urls. Useful when behind a rewriting reverse proxy.

[lovasoa]

Hello and thank you for the PR !

I think I need some context here. I'd like to understand the underlying problem that this is trying to solve.

1. You rewrite your URLs using nginx to add .sql to incoming request, so that for instance the end user sees just /search but your sqlpage application receives a request to /search.sql
2. But in your codebase, you are using sqlpage.link('search.sql'), with the .sql extension, instead of sqlpage.link('search') (presumably because you want to be able to test it locally, without having to setup an nginx proxy locally ?)
3. So currently, links generated by sqlpage.link work both locally and in production, but they look ugly and you would like to make them look better in production

What I'm currently thinking

• If we want to tackle this problem inside sqlpage, then best solution is probably to let sqlpage do the url rewriting internally (load the search.sql file when the user requests /search), without requiring nginx.
• If you want a solution immediately specific to your setup, then the best option is probably to define a redirect inside nginx, to keep the logic in one place. This way your application does not have to be aware that its URLs are being rewritten, and your rewriting logic is entirely contained in your nginx config.

  •  location ~ ^(.+)\.sql$ {
        return 301 $1$is_args$args;
     }

[guspower]

Yes, you are correct. It's a) presenting public facing links without .sql extensions in production, which still having the local development links work. I don't really fancy having to run a local nginx for development, but it's not out of the question.

How difficult would it be to get sqlpage to do the url rewriting internally? ('/search' -> '/search.sql'). I assume the work would be around the file_cache? This sounds like a good option! :)

I considered using redirects, and considered using content rewriting on the way out to trim out the .sql extensions; they seem like workarounds.

[guspower]

This also pairs up with the related idea of having an explicit redirect configuration for certain paths, e.g. something like

{ 
    redirects: [
        "/notes/${org}/${ref}":"/notes/note.sql"
    ]
}

[lovasoa]

Handling requests without an .sql extension directly in SQLPage

How difficult would it be to get sqlpage to do the url rewriting internally? ('/search' -> '/search.sql'). I assume the work would be around the file_cache? This sounds like a good option! :)

Probably not very difficult, and I know it would be useful to many; the topic has been raised before. Let's do it.
This can be done in file_cache, but I think it would be more appropriate to have this logic in http.rs (or even better, in a new file/module that is called from http.rs). The logic should probably be factored with the existing logic to load index.sql when hitting a folder root. I think the new logic should be:

• If the query path ends with .sql, just load the .sql file
• Otherwise, try loading, in sequence: path/index.sql then path.sql. The first one that succeeds gets served.
• Otherwise, let the static file server serve the request as a static asset

Creating a configurable redirects mapping

   { 
     "redirects": {
         "/notes/${org}/${ref}":"/notes/note.sql"
     }
   }

That would be very useful too ! But let's maybe do this afterwards, in a second pull request.

[guspower]

OK cool. I will read through the code this afternoon to get up to speed with it.

Also also, I added parameterized tests using #[rstest] to the above branch - I dunno if you're ok with using that or would prefer to stay with the stock test runner?

[lovasoa]

Honestly I'd prefer if we kept tests consistent. You can just define a function and call it multiple times with different arguments from a single #[test].

[guspower]

OK, error handling aside, is this the logic we're looking for?:

PATH	TARGET
/	/index.sql
/index	/index.sql if exists otherwise redirect to /index/
/index.sql	/index.sql
/index/	/index/index.sql
/index.png	serve raw file

[lovasoa]

@guspower : perfect !

[guspower]

@lovasoa : OK first thoughts here - let me know if this seems like an acceptable approach.

[lovasoa]

I love how clean and testable this looks. This is still missing the /x -> /x.sql part, right?

Also, the store needs to be asynchronous.

[guspower]

OK cool, I'll progress it a bit further.

This is still missing the /x -> /x.sql part, right?

Yep, that is correct. Adding this is next up.

Also, the store needs to be asynchronous.

+1 was avoiding async-trait

[guspower]

@lovasoa OK next pass pushed to the branch for review.
The store is now async, and it handles /x -> /x.sql.
I added the site prefix check right at the start.
Needs some more thought and tlc.

[lovasoa]

Great ! I'm closing this pr in favor of a new one that points to your new branch: #794