Support optional parameters

[AvlWx2014]

Overview

Support bindings for optional parameters.

Background

In application code some pieces of configuration can be considered optional. For example, consider an API server that supports authentication/authorization in staging and production, but can have this feature "turned off" for development by simply omitting the auth section of the server configuration.

injector already supports providers for optional types e.g.:

AuthConfig: TypeAlias = OAuth2Config | OidcConfig


@dataclass
class AppConfig:
    auth: AuthConfig | None = None


class ConfigurationModule(Module):
    @singleton
    @provider
    def provide_app_config(self) -> AppConfig:
        return AppConfig.load()

    @singleton
    @provider
    def provide_auth_config_or_none(self, app_config: AppConfig) -> AuthConfig | None:
        return app_config.auth

In this example, I can easily set up an injector and get AuthConfig | None from the graph:

World: Final[Injector] = Injector(ConfigurationModule())

if __name__ == "__main__":
    appconfig = World.get(AppConfig)
    print(f"AppConfig resolved: {appconfig!s}"
    
    authconfig = World.get(AuthConfig | None)
    print(f"AuthConfig | None resolved: {authconfig!s}")

When this is run it prints:

AppConfig resolved: ...   # real values hidden for brevity and security
AuthConfig | None resolved: None

However, if I then write a provider function which requests AuthConfig | None as a dependency like:

class MiddlewareModule(Module):
    @provider
    @singleton
    def provide_authn_middleware(
        self, config: AuthConfig | None, injector: Injector
    ) -> AuthenticationMiddleware[Secret[str] | None]:
        middleware: AuthenticationMiddleware[Secret[str] | None]
        match config:
            case OAuth2Config() | OidcConfig():
                middleware = injector.get(BearerTokenMiddleware)
            case None:
                middleware = NoopAuthenticationMiddleware()
            case _:
                assert_never(config)
        return middleware

The injector raises indicating there is no provider for Union[OAuth2Config, OidcConfig] (type names shortened for brevity). Given the AuthConfig: TypeAlias = OAuth2Config | OidcConfig alias from earlier this message implies something somewhere is requesting plain AuthConfig as a dependency rather than AuthConfig | None, which has a provider in the graph.

However, after combing my whole dependency graph I could not find a place where this was actually happening in my application code so I started digging further. After a lot of debugging I found that the function _infer_injected_bindings removes NoneType from unions before resolving the rest of the injectable dependencies from a function signature:

# We don't treat Optional parameters in any special way at the moment.
union_members = v.__args__
new_members = tuple(set(union_members) - {type(None)})

After removing the set difference operation from the assignment to new_members the code examples I share above start working instead of raising an exception about missing providers.

Solution

The solution proposed here removes - {type(None)} from new_members = ... to allow optional dependencies to be fulfilled if there is a provider configured for them.

I also included a test to demonstrate the behavior in test_get_bindings. In the first commit the new test fails, but in the second commit with the proposed change applied it passes.

This patch induces a change in the test for test_get_bindings_for_pep_604 since now a: int | None resolves to int | None instead of just int.

The rest of the test suite passes with this patch in place, so from what I can tell there aren't any unintended side effects on the rest of the codebase, but let me know if I'm missing something. Looking forward to getting your thoughts on this!

[AvlWx2014]

@davidparsson @jstasiak wanted to get your thoughts on this. Feel free to tag anyone else for discussion that I may have missed!

[davidparsson]

@AvlWx2014, thanks for your contribution. Your use case is reasonable, but I will have to consider the implications of this. I am pretty sure this will break things for me, and probably for other people as well, since str | None previously was resolved to str.

I think Injector's handling of optional arguments is not ideal either, and while it's not the same issue it might be related.

The obvious workaround without breaking typing with the current version would in your example be to only provide AppConfig, but I understand that this is not what you want.

[AvlWx2014]

Hi @davidparsson sorry for the delayed reply, I was on vacation 😄

I am pretty sure this will break things for me, and probably for other people as well, since str | None previously was resolved to str.

I see what you mean here. My thinking originally was that this change would remain backwards compatible since str | None is a wider type than just str. Theoretically any existing dependency graphs relying on str | None resolving to just str would not contain any requests for optional types since this wouldn't work in the past. It's based on this I think that such graphs would still work without breakage.

I imagine you've seen plenty more use cases than I have which might prove my assumptions wrong, so let me know what you think!

The obvious workaround without breaking typing with the current version would in your example be to only provide AppConfig, but I understand that this is not what you want.

I definitely could, yes, and there's also the option to use the null object pattern here. Something like:

@dataclass
class NullConfig:
    pass

    def __bool__(self) -> bool:
        return False


AuthConfig: TypeAlias = OAuth2Config | OidcConfig | NullConfig


@dataclass
class AppConfig:
    auth: AuthConfig = NullConfig()

but I think supporting requests for optional types is still useful.