meadsteve / lagom

📦 Autowiring dependency injection container for python 3
https://lagom-di.readthedocs.io/en/latest/
MIT License
278 stars 15 forks source link

Injecting instances with a __call__ method on the class #206

Closed bejoygm closed 2 years ago

bejoygm commented 2 years ago

One of the use case is to Inject an instance of HttpBearer in FastAPI routes

since the instance will have a different type than the class definition itself, this doesn't inject the instance as a dependency. Is there a workaround? Thanks

meadsteve commented 2 years ago

@bejoygm Have you got a short code snippet that doesn't work correctly so I can get my head around this?

meadsteve commented 2 years ago

My instinct is that with HttpBearer it makes more sense to make your code depend on thing it needs HTTPAuthorizationCredentials and then have something like:


container[HTTPAuthorizationCredentials] = lambda c: c[HttpBearer](c[Request])
bejoygm commented 2 years ago

@meadsteve here's one of the ways

from fastapi import Depends
from fastapi.security import HTTPBearer

@router.get("")
async def get_something(
    auth_token=Depends(HTTPBearer()),  # noqa
):
    pass

you can find similar examples here

meadsteve commented 2 years ago

So if I understood correctly the following might be what you want. I've not tested it but the way I would write something like that with lagom would be:

container = Container()
deps = FastApiIntegration(container)

container[HTTPAuthorizationCredentials] = lambda: HTTPBearer()() # I think - I'm not really sure here what's going on

@router.get("")
async def get_something(
    auth_token= deps.depends(HTTPAuthorizationCredentials)
):
    pass

One of the goals of lagom was to make code work well with type checking so that deps.depends(X) will always return an instance of X.

So it really depends what type you want auth_token to have.

bejoygm commented 2 years ago
container[HTTPAuthorizationCredentials] = lambda: HTTPBearer()()

☝️ This would have worked I guess but the HTTPBearer object's __call__ takes a request parameter. Pasting the code here for ref.

async def __call__(self, request: Request) -> Optional[HTTPAuthorizationCredentials]:
        authorization: str = request.headers.get("Authorization")
        scheme, credentials = get_authorization_scheme_param(authorization)
        if not (authorization and scheme and credentials):
            if self.auto_error:
                raise HTTPException(status_code=HTTP_403_FORBIDDEN, detail="Not authenticated")
            else:
                return None
...

I tried passing the request parameter explicitly, but the injected object is not correct (in this case, a coroutine object)

container[HTTPBearer] = lambda: HTTPBearer()
container[HTTPAuthorizationCredentials] = lambda c: c[HttpBearer](c[Request])
meadsteve commented 2 years ago

ah I missed that it took a request objection as an argument. I believe what you reference here:

container[HTTPBearer] = lambda: HTTPBearer()
container[HTTPAuthorizationCredentials] = lambda c: c[HttpBearer](c[Request])

should have worked. Let me take a deeper look at what's happening.

meadsteve commented 2 years ago

🤦 I totally missed that HTTPBearer is an async __call__ I'll need to rework my suggestion

meadsteve commented 2 years ago

So the "simple" solution is to tweak the above to:

container[HTTPBearer] = Singleton(lambda: HTTPBearer())
container[Awaitable[HTTPAuthorizationCredentials]] = lambda c: c[HTTPBearer](c[Request])

@router.get("")
async def get_something(
    auth_token= deps.depends(Awaitable[HTTPAuthorizationCredentials])
):
    await auth_token

but I'm a bit hesitant to suggest this as if auth_token is not awaited then the auth logic is skipped.

bejoygm commented 2 years ago

with the explicit await, the generated docs will not have the token documented properly. Anyways, I think it is not a big issue. I can always use fastAPI Depends for this stuff. We have started using lagom at other places for dep injection and it is a joy to work with. Thanks for the library! 🙌

meadsteve commented 2 years ago

@bejoygm thank you that's really nice to hear. And using the fastapi one sounds like the best approach here 👍 especially for anything related to security/auth.