Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DependencyOverride incorrect matches #200

Open
ENikS opened this issue Jun 2, 2020 · 0 comments
Open

DependencyOverride incorrect matches #200

ENikS opened this issue Jun 2, 2020 · 0 comments
Assignees
Labels
Breaking change ❕ This change breaks public API Enhancement 🔨 Improvement of existing features

Comments

@ENikS
Copy link
Contributor

ENikS commented Jun 2, 2020

DependencyOverride

The DependencyOverride provides an override for any resolved dependency that matches the contract.

Usage

Override is passed to the Resolve method alone with Type and Name of the requested contract:

Container.Resolve(someType, someName, new DependencyOverride(target, type, name, someValue)

Variants

public DependencyOverride(Type contractType,  ...)
public DependencyOverride(string contractName, ...)
public DependencyOverride(Type contractType, string? contractName, ...)

Override by Type

The constructor DependencyOverride(Type contractType, ...) creates an override that matches any dependency with specified `Type' regardless of dependency name. So many of these imports will match the override:

TestType(TDependency value)
TestTypeNamed([OptionalDependency("Name")] TDependency value)
TestType([Dependency] TDependency value)

Override by Name

The constructor string contractName creates an override that matches any dependency with specified contractName, regardless of the dependency's Type. So many of these imports will match the override:

TestTypeNamed([OptionalDependency("Name")] Type1 value)
TestType([Dependency("Name")] Type2 value)

Override by Contract

Finally, the constructor DependencyOverride(Type contractType, string? contractName, ...) creates an override that matches only dependencies with exact Type and contracts Name.

Problems

The current implementation has several issues:

  • DependencyOverride creates false positives on targeted overrides
  • Overrides are not matched to dependencies correctly
  • Type override only works for "null" name

False positives

During the equality check, the target is ignored. This example illustrates the issue:

[TestMethod]
public void SomeDescriptiveName()
{
  var a = new DependencyOverride(typeof(object), typeof(object), string.Empty, OverrideValue);
  var b = new DependencyOverride(typeof(string), typeof(object), string.Empty, OverrideValue);

  Assert.IsFalse(a.Equals(b));
}

So, during resolution, this override will be applied to either target.

Not matching to dependency

This example illustrates the issue:

[TestMethod]
public void CtorParameterNative()
{
    // Arrange
    Container.RegisterType<Service>(new InjectionConstructor("data"));

    // Act
    var value = Container.Resolve<Service>(new DependencyOverride(typeof(string), "override"));

    // Verify
    Assert.AreEqual("override", value.Data);
}

A type Service is registered with a container and requested to inject a constructor with the string "data".
During resolution, a DependencyOverride for type 'string' is passed into the Resolve. This override should be applied to every dependency with the type string, but it fails.

@ENikS ENikS self-assigned this Jun 2, 2020
@ENikS ENikS transferred this issue from unitycontainer/abstractions Sep 26, 2020
@ENikS ENikS changed the title DependencyOverride.Equals() creates false positive on targeted overrides DependencyOverride incorrectly matches dependencies Nov 5, 2020
@ENikS ENikS changed the title DependencyOverride incorrectly matches dependencies DependencyOverride incorrect matches Nov 5, 2020
@ENikS ENikS added the Bug 🐛 label Nov 5, 2020
@ENikS ENikS added this to the 6.0.0 milestone Nov 5, 2020
@ENikS ENikS added Breaking change ❕ This change breaks public API and removed Breaking change ❕ This change breaks public API labels Nov 5, 2020
ENikS referenced this issue in unitycontainer/container Nov 5, 2020
@github-actions github-actions bot added the release Release branch label Nov 5, 2020
@ENikS ENikS added Breaking change ❕ This change breaks public API Enhancement 🔨 Improvement of existing features labels Dec 2, 2020
@ENikS ENikS removed this from the 6.0.0 milestone Jun 27, 2021
@ENikS ENikS removed the release Release branch label Mar 10, 2023
@ENikS ENikS transferred this issue from unitycontainer/container Mar 11, 2023
@ENikS ENikS removed the Bug 🐛 label Mar 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking change ❕ This change breaks public API Enhancement 🔨 Improvement of existing features
Projects
None yet
Development

No branches or pull requests

1 participant