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

Dependency Injection Fails with mixed classes and structs when depending on an interface #512

Open
syeopite opened this issue Feb 4, 2025 · 5 comments
Labels
component:dependency-injection kind:bug An existing feature isn't working as expected

Comments

@syeopite
Copy link

syeopite commented Feb 4, 2025

A service that depends on an interface cannot receive additional injected implementations of the interface when the first implementation is a Struct and the additional implementation is a Class

module Interface
  abstract def foo
end

@[ADI::Register]
@[ADI::AsAlias(Interface)]
struct Dependency
  include Interface

  def foo
    return "Dependency"
  end
end

@[ADI::Register]
@[ADI::AsAlias(Interface)]
class InjectedDependency
  include Interface

  def foo
    return "Injected Dependency"
  end
end

@[ADI::Register(public: true)]
class Service
  def initialize(@service : Interface)
  end

  def run
    return @service.foo
  end
end

puts ADI.container.service.run # => "Dependency"
@Blacksmoke16 Blacksmoke16 added kind:bug An existing feature isn't working as expected component:dependency-injection labels Feb 4, 2025
@Blacksmoke16
Copy link
Member

@syeopite What version are you on to produce this? I just tried to reproduce and got Injected Dependency.

@syeopite
Copy link
Author

syeopite commented Feb 4, 2025

Oops I messed up in my report. Looks like the bug only applies when Dependency is a Struct and InjectedDependency is a Class. This is on dependeny-injection 0.4.2 and Crystal 1.15.0

@Blacksmoke16
Copy link
Member

Blacksmoke16 commented Feb 4, 2025

Okay, yea I think this is a bug in that the last usage should always "win" being the default implementation. But is there a specific use case you're wanting to solve? I'd say it's somewhat unexpected to have AsAlias on multiple services for the same interface given there should only really be a single default implementation.

Dev Note

This seems to be a result of Object.all_subclasses returning structs after all the classes versus the order in which the types were defined.

@syeopite
Copy link
Author

syeopite commented Feb 4, 2025

Am I not supposed to use ADI::AsAlias for subsequent implementations?

Anyways, my use case is for mocking a struct service and needing to pass a
pass a variable to the mock during the test case. Something like this for example:

@[ADI::Register(public: true)]
@[ADI::AsAlias(ServiceInterface)]
class Mock
  include ServiceInterface

  property retrieve_mock_data_from : String = ""

  def get(uri)
    return JSON.parse(File.read(@retrieve_mock_data_from))["uri"]
  end
end


it "Test" do
    ADI.container.mock.retrieve_mock_data_from = "spec/mocks/response.json"
    response = ADI.container.fetch_data.fetch()

    response.should eq "data"
end

Real world example here https://github.com/syeopite/instances-api/blob/6301642b7e7353214d77e791d8016616bcb04386/spec/populate-spec/populate_spec_helper.cr#L74

@Blacksmoke16
Copy link
Member

Blacksmoke16 commented Feb 4, 2025

Am I not supposed to use ADI::AsAlias for subsequent implementations?

No, the purpose of ADI::AsAlias is to define the default implementation for a given interface. Like normally it is able to look at the type restriction of a constructor parameter in order to figure out what service it needs to inject. But when you have something like:

module Interface; end

@[ADI::Register]
class One
  include Interface
end

@[ADI::Register]
class Two
  include Interface
end

@[ADI::Register(public: true)]
class Service
  getter service : Interface

  def initialize(@service : Interface); end
end

puts ADI.container.service.service.class # => "???"

There is no real way to know what service should be used for the @service service, since there are multiple implementations of the interface. This is where AsAlias comes into play, allowing you to apply it to One or Two to tell it that service should be used when a Interface type restriction is encountered. This way you avoid the boilerplate of needing to explicitly specify what service you want every time.

If there is a case where you may want an implementation that isn't the default, you can still do something like this:

# Assume `One` is the configured alias of `Interface`.
# Having the constructor parameter name match the service ID of `two` makes it provide that one instead.
# Using a local var such that the name of ivar in the type 
# can be kept separate if we want to change it to `Three` for example.
def initialize(two : Interface)
  @service = two
end

Real world example here

Hmm okay, I'll have to read thru this code more closely. I think there are some changes you'll want to make to take the most advantage of this design pattern compared to your current approach.

EDIT: @syeopite I DM'd you on gitter.

syeopite added a commit to syeopite/instances-api that referenced this issue Feb 4, 2025
The original `FetchInstancesFromDocs` service was a struct preventing
the mock service, `MockFetchInstances` which is a class from being used
due to athena-framework/athena#512
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:dependency-injection kind:bug An existing feature isn't working as expected
Development

No branches or pull requests

2 participants