Cannot use qualifiers on factory with Optional Types

[ranjanprasad96]

Qualifier is ignored on a factory returning Optional[T]

Summary

When a factory is registered with a qualifier and returns an Optional[T] type, the qualifier appears to be dropped during registration. Instead of registering the qualified factory, the container tries to resolve T as a plain (unqualified) dependency, which fails validation with a self-dependency error.

Could you please help me with this?
This is blocking integration of wireup into our codebase.

Environment

• wireup: 2.10.0
• Python: 3.9.x

Minimal reproduction

import asyncio
from typing import Optional

import wireup
from wireup import injectable


class AuthContext:
    pass


@injectable
async def require_authentication() -> AuthContext:
    return AuthContext()


# Factory with a qualifier that returns Optional[AuthContext] — this combination breaks.
@injectable(qualifier="optional")
async def maybe_get_authentication() -> Optional[AuthContext]:
    return None


async def main() -> None:
    container = wireup.create_async_container(
        injectables=[require_authentication, maybe_get_authentication],
    )


if __name__ == "__main__":
    asyncio.run(main())

Actual behavior

Container creation fails at registry validation:

WireupError: Parameter 'raw_type_instance' of <class '__main__.AuthContext'>
has an unknown dependency on <class '__main__.AuthContext'>.

Traceback (most recent call last):
  check_wireup.py:46  in <module>          asyncio.run(main())
  check_wireup.py:40  in main              container = wireup.create_async_container(...)
  wireup/ioc/container/__init__.py:241     in create_async_container → _create_container(...)
  wireup/ioc/container/__init__.py:114     in _create_container → ContainerRegistry(...)
  wireup/ioc/registry.py:97                in __init__ → self.extend(...)
  wireup/ioc/registry.py:137               in extend → validate_registry(self)
  wireup/ioc/registry_validation.py:30     in validate_registry → assert_dependency_exists(...)
  wireup/ioc/registry_validation.py:118    in assert_dependency_exists → raise WireupError(msg)

Expected behavior

The factory should be registered under the "optional" qualifier and resolve as Optional[AuthContext], without the qualifier being discarded or a spurious self-dependency being created.

Investigation

While debugging, it looks like the compatibility/Optional-unwrapping path does not carry the qualifier through when the return type is Optional[T]. Because the qualifier is lost, the factory ends up registered against the bare type T, which then appears to depend on itself — producing the raw_type_instance self-dependency error above. A non-Optional return type with the same qualifier registers fine.

Suggested fixes

File wireup 2.10.0: https://github.com/maldoinc/wireup/blob/v2.10.0/wireup/ioc/registry.py#L258
File wireup 2.11.0: https://github.com/maldoinc/wireup/blob/v2.11.0/wireup/ioc/registry.py#L396

               compat_fn.__signature__ = inspect.Signature(  # type: ignore[attr-defined]
                   parameters=[
                       inspect.Parameter(
                           "raw_type_instance",
                           kind=inspect.Parameter.POSITIONAL_OR_KEYWORD,
  -                        annotation=klass,
  +                        annotation=klass if qualifier is None else Annotated[klass, 
  InjectableQualifier(qualifier)], 
                       )
                   ],
               )

[ranjanprasad96]

@maldoinc Could you please help me with this?
This is blocking integration with our codebase since we need two different factories (one for AuthContext and one for Optional[AuthContext]).

Really appreciate your help.
Thank you so much

[maldoinc]

Given you already found the root cause and have a fix, feel free to send a pr for this.

If you want to keep this model and unblock now you can newtype the optional variant to MaybeAuthContext = NewType("MaybeAuthContext", AuthContext | None).

However, personally I'd say the way I'd have modeled this with the information I have is to always return an Auth context and have its state/callers handle the absence of a user instead of a new registration with our without qualifiers.

@dataclass
class AuthContext:
    maybe_user: User | None

    @property
    def authenticated_user(self) # assert user is not none and return maybe_user

However, in this case the backwards compat/synthesized registrations should not prevent user registrations. Wireup already let's user ones have precedence for collection injection.

[ranjanprasad96]

@maldoinc Thanks for the quick reply!

Since we're still on Python 3.9, I'd like to contribute the fix to both 2.10.0 and master. Once that's in, would you be open to cutting a 2.10.1 release? That would help me unblock my current work without needing to jump to 2.11.0.

[ranjanprasad96]

@maldoinc I have raised a PR for master - PR for master branch.

Also, I have created a branch - Branch with fixes for 2.10.0 and would like to cut a release from 2.10.0 since I need this patch for python3.9.x codebase.

Could you please help me with this?
Thanks a lot

[ranjanprasad96]

@maldoinc Could you please help me expedite this fix? Really appreciate your help!
Thanks

[maldoinc]

Change looks good. I'll backport this for py38/39 as well. Release may take another day or two since I'm away currently, but as mentioned earlier you if you need to unblock now, either options above will help you out immediately.

[ranjanprasad96]

@maldoinc Thats totally fine :)
Thanks for the quick review.
Really appreciate it!

[ranjanprasad96]

@maldoinc Had a query on this.

From the framework side, it looks like registering a factory that returns Optional[T] automatically also registers the raw type T (via the backwards-compatibility alias, so that container.get(T) keeps working with a deprecation warning).

The side effect is that this auto-registration conflicts with an explicit T factory — registering both an Optional[T] factory and a T factory together raises DuplicateServiceRegistrationError, even though the user only intended to register one "real" provider.

Would it make sense to expose a way to opt out of the compatibility alias (e.g. a flag on @Injectable / the factory registration), with the default left as the current behaviour so existing code is unaffected?

That would let users who don't rely on the deprecated container.get(T) path register an Optional[T] factory and a separate T factory without the conflict.

[maldoinc]

I've been thinking about this as well tbh but the that the backwards compat is narrower than that it is currently doing.

Right now the factory supports that behavior for container.get and injected calls but the compatibility should be only for container.get so we can potentially remove the compatibility factory entirely and handle it only in container.get calls as a fallback on an unknown types.

I'm be a bit reluctant to add flags to the main injectable api just for this.

[ranjanprasad96]

@maldoinc Thanks for the suggestions.
I have raised a PR to remove the compatibility factory.
Please review at your convenience.
Thanks

PR - #140

[ranjanprasad96]

@maldoinc Thanks for merging the PR and making a release. Really appreciate it :)

I have resolved the conflicts for the PR - #140