aminalaee / sqladmin

SQLAlchemy Admin for FastAPI and Starlette
https://aminalaee.dev/sqladmin/
BSD 3-Clause "New" or "Revised" License
1.73k stars 177 forks source link

What about Router per View? #771

Open hasansezertasan opened 2 months ago

hasansezertasan commented 2 months ago

Checklist

Is your feature related to a problem? Please describe.

Introduction

At #767, I talked about building a RedisCLI view like in the Flask Admin.

I started building this view, but I realized that routing in the view is a bit different from the Flask Admin.

Flask Admin uses one Blueprint (Router equivalent in Flask) per view but SQLAdmin registers exposed methods directly to the main Starlette application instance (BaseAdmin.admin), not a dedicated Router instance of the view.

So in my case, if I do this:

from sqladmin import BaseView, expose

class RedisCLIView(BaseView):
    name = "Redis CLI"
    redis_instance = None
    identity = "rediscli"

    @expose("/rediscli", methods=["GET", "POST"], identity="index")
    async def index_page(self, request):
        # Do something with redis_instance
        return await self.templates.TemplateResponse(request, "sqladmin/redis/index.html")

admin.add_view(RedisCLIView)

The url_for method won't work as expected.

url_for("admin:rediscli:index") will raise an error because the view is not registered as a Router with a name ("rediscli") and the exposed method is not registered as a Route with a name ("index").

url_for("admin:index") will work because exposed methods are registered as a Route to the main Starlette application instance.

What is the problem?

In general, every view is expected to have an entry point (like a home page) and other pages. This is generally the index and other pages.

What happens if we have multiple views with the same entry point name?

from starlette.applications import Starlette
from sqladmin import Admin, BaseView, expose

class RedisCLIView(BaseView):
    name = "Redis CLI"
    redis_instance = None
    identity = "rediscli"

    @expose("/rediscli", methods=["GET", "POST"], identity="index")
    async def index_page(self, request):
        # Do something with redis_instance
        return await self.templates.TemplateResponse(request, "sqladmin/redis/index.html")

class RedisCLI1(RedisCLIView):
    name = "Redis CLI 1"
    redis_instance = None

class RedisCLI2(RedisCLIView):
    name = "Redis CLI 2"
    redis_instance = None

app = Starlette()
admin = Admin(app)
admin.add_view(RedisCLI1)
admin.add_view(RedisCLI2)

Even though RedisCLI1 and RedisCLI2 are different views, they both have the same entry point (index) with the same URL (/rediscli).

Thinking RedisCLIView comes from SQLAdmin, will cause unexpected behavior because the index method is already registered.

Describe the solution you would like.

Solution

Every view should have its own Router and its exposed methods should be registered to that Router.

Describe alternatives you considered

I'm actively using Starlette Admin, it doesn't have a built-in expose method and it has a similar structure to SQLAdmin. The problem that exists here also exists there.

I tried to adopt Flask Admin to work with Starlette but that's not an easy job 😆.

Additional context

Other Problems

Since the index, list, details, delete, create, edit, export, and ajax_lookup endpoints are registered directly to the main Starlette application instance, we can't override them for each view.

https://github.com/aminalaee/sqladmin/blob/d062cfa845fdbd2f09cbf0a1335ea8cce6e285c9/sqladmin/application.py#L387-L419

The user might need to override the list method in the ModelView but might require different logic per model. So the user must override Admin directly.

hasansezertasan commented 2 months ago

I'm not entirely sure and I didn't think thoroughly but this approach might solve #681.

hasansezertasan commented 2 months ago

What I learned so far

Flask Admin Routing

As I said, Flask Admin creates a dedicated blueprint for each view and they are all registered directly to the application.

Consider two views: UserView and PostView. UserView has a blueprint named user_view and PostView has a blueprint named post_view.

Flask Admin also registers an AdminIndexView by default and its blueprint is named admin.

In Flask Admin, every view has an exposed method named index_view.

So the URL-building process is like this:

Flask Admin registers AdminIndexView before all other views. That's why I failed when I tried to adopt its core to use Starlette. AdminIndexView occupies all URLs starting with /admin/* when I use Mount.

Starlette Routing

Mount builds an application (if not provided), and the URL that is used while mounting an application, router, or group of routes is occupied by this application.

Route naming: we can name our routes with a colon separator.

admin_routes = [
  Route(..., name="admin:index_view"),
  Route(..., name="admin:user_view:index_view"),
  Route(..., name="admin:rediscli1:index_view"),
  Route(..., name="admin:rediscli2:index_view"),
]

Conclusion

Each view gets registered (mounted) to the main application. Not to an application to be mounted to the main application.

Each view has a dedicated blueprint (Router or Mount instance) and each exposed function gets registered to this blueprint.

Questions

In the end problem remains: View can not have a name prefix... So if the user has two custom views with the same exposed URLs, there will be conflicts...