Add equivalent to Flask's url_for() method for templates to construct Robyn routes

dave42w commented 5 days ago

In a template we want to be able to create a URL to a Robyn endpoint and we don't want to hard code it.

So if we consider the example: https://robyn.tech/documentation/example_app/modeling_routes and imagine that

@app.get("/crimes")
async def get_crimes(request):

renders a template with all the crimes listed. Each crime in the table will need URLs for the details, to edit, to delete. The whole page will need a url to add an additional crime. The end points are all defined in the code:

@app.post("/crimes")
async def add_crime(request):
...
@app.get("/crimes/:crime_id", auth_required=True)
async def get_crime(request):
...
@app.put("/crimes/:crime_id")
async def update_crime(request):
...
@app.delete("/crimes/{crime_id}")
async def delete_crime(request):

Flask uses url_for() to do this eg:

    <table>
        {% for cime in crimes %}
            <tr>
                <td><a href={{url_for('get_crime',id=crime.id)}}>{{ crime.description }}</a></td></td>
            </tr>
        {% endfor %}
    </table>

As the example is currently written that URL would resolve to <a href="/crime/42">Crime number 42</a>

The flask url_for has extra capabilities. For example, the need to change the scheme and host is very rare.

The Flask url_for is based on the function name rather than the argument passed to the decorator. If the function name is within a blueprint then the name of the blueprint needs to be in the argument to url_for. In Robyn that means we want the first argument to be the name argument to the SubRouter (in the examples whatever name resolves to eg "frontend") plus the name of the function within that, plus the arguments.

This way if we change the SubRoute prefix or the decorator path the template does not need to be changed.

So these changes will not change the template (as they neither change the name of the module nor the function name)

crimes = SubRouter(__name__, prefix="/sub_router")
@crimes.get("/crimes/:crime_id", auth_required=True)

to

g_crimes = SubRouter(__name__, prefix="/gotham/crimes")
@g_crimes.get("/acrime/:crime_id", auth_required=True)

Note we will still need to change the template if we move the function from global into a SubRouter, change the module name, change the function name, or the function arguments.

Adding this function helps make templates less brittle and more reusable.

dave42w commented 3 days ago

I have a working call to url_for from a jenja template, it does not affect anything else. Only 2 changes needed to templating.py

First, we define url_for (it just returns a fixed str at the moment):

from .robyn import Headers, Response

def url_for():
    return "url_for not fully implemented yet"
...

Second, we add this to the jenja environment (just one line needed):

class JinjaTemplate(TemplateInterface):
    def __init__(self, directory, encoding="utf-8", followlinks=False):
        self.env = Environment(loader=FileSystemLoader(searchpath=directory, encoding=encoding, followlinks=followlinks))
        self.env.globals['url_for'] = url_for

we can see this working by adding url_for to the test.html body:

<body>
  <h1>{{framework}} 🤝 {{templating_engine}} {{url_for()}} </h1>
</body>

Then, a one-line addition to test_get_requests.py it fails if url_for does not return the correct fixed str, it passes with the code above:

@pytest.mark.benchmark
@pytest.mark.parametrize("function_type", ["sync", "async"])
def test_template(function_type: str, session):
    def check_response(r: Response):
        assert r.text.startswith("\n\n<!DOCTYPE html>")
        assert "Jinja2" in r.text
        assert "Robyn" in r.text
        assert "url_for not fully implemented yet" in r.text

    check_response(get(f"/{function_type}/template"))

Will you accept a PR with these changes?

sansyrox commented 3 days ago

Hey @dave42w 👋

Thank you for volunteering. One of the contributors tried implementing url_for previously. You can have a look here - https://github.com/sparckles/Robyn/pull/758/files

Unfortunately, it was abandoned and we couldn't merge it. However, I liked the approach in the same PR and would be happy to accept a new one 😄

Maybe you can use the PR(https://github.com/sparckles/Robyn/pull/758/files ) as an inspiration?

dave42w commented 3 days ago

I'm very happy to take inspiration from that. I'm working at this very part-time so my intention is to work in small steps that you don't have to wait for and won't get out of sync with the codebase plus don't break anything (continuous delivery style). So far I don't think I'm contradicting the previous approach at all.

I'll create a PR with what I have and see what you think.

dave42w commented 3 days ago

See https://github.com/sparckles/Robyn/pull/1004

dave42w commented 3 days ago

I suggest the next step should be to take something from the earlier PR https://github.com/sparckles/Robyn/pull/758/files#diff-1276ef09a4b09c2f539dfc4e497e5054f03f2388440a91da0354e74f7fa828cd

    def add_template_global(self, func: Callable, name: Optional[str] = None):
        """
        Add a global function to the Jinja environment.

Instead of simply adding url_for to the environment as in my first PR we will have 2 benefits

url_for will be type checked as a callable as it is added to the Jinja environment
developers can use the same function to add their own functions to Jinja

The current tests should still pass and I'll add an additional one for adding a custom function to the Jinja environment.

sparckles / Robyn

Add equivalent to Flask's url_for() method for templates to construct Robyn routes #996