Validation on build_provider about more-rs-di HOT 3 CLOSED

Some additional validation is definitely something I'm behind. It certainly makes sense to put that behind build_provider. That is something for a v2. Once the approach for how validation can/will be done, then it can be evaluated whether it should be behind build_provider as an Error (but I think it's reasonable to assume that will be the case).

1.

If a Singleton depends on a Scoped, it effectively causes the Scoped to become a Singleton. This is arguably wrong, but I started with the caveat emptor approach. build_provider definitely doesn't need to panic. Returning some type of validation (e.g. Error) would make sense.

If a Transient depends on a Scoped, I don't think that is a problem because in the current design, it's not possible for a Transient to depend on a Scoped at any other level than itself. In other words, a Transient in the root can only depend on a Scoped that is also at the root (which is effectively a Singleton). A Transient in a nested scope can only depend on a Scoped at the same level. A Scoped should be able to live longer than a Transient. The inverse is also still valid. If a Scoped depends on a Transient, then it is effectively promoted to Scoped because it will live as long as Scoped. Even if it's not what we would do, that should be allowed.

The approach of separating ServiceCollection and ServiceProvider where ServiceProvider is immutable solves a number of these problems. It's not possible to re-register things in some bizarre combination. You could create a new ServiceCollection that closes of some other existing ServiceProvider, but that is a very bad idea and I'd argue "you're doing it completely wrong.". 😆

2.

A circular dependency loop is definitely a possibility - unfortunately. Validating or being able to detect this is worth trying to support.

3.

The exact behavior can vary, but in most cases this will be handled via ServiceProvider.get_required::<T>() which will panic with an error message that will indicate, to the best of Rust's ability, the type that could not be resolved (see this test). Every object is constructed via a ServiceFactory callback function. A constructor function isn't required for things to work, you only need to define a closure which knows how to create the object.

The behavior is slightly different when you use the inject feature with the code generation macro. In this scenario, the macro has to know the call site to use for construction. Since Rust doesn't support method overloading, this makes establishing a convention straight forward. The macro will look for a function called new. It doesn't even need to be pub since the macro will implement Injectable for the struct itself (which is allowed to call a private function). If you don't like or can't use that convention, then you can decorate the desired activator function with #[inject] which will tell the macro which call site to use. #[inject] supersedes new if both are defined. If neither of these are satisfied or there is more than one function decorated with #[inject] the macro panics, which results in a compile-time error.

4.

I'm not sure I 100% follow this one. A trait can only be implemented by a struct exactly once. You can naturally have multiple, different structs implement the same trait. I think you may be asking, "What happens if multiple registrations are performed, but I only expect one?". Currently, this has the same behavior as replace, which will use the last registration, when a single instance is requested.

I think it may have to work this way because it is technically valid for both call sites to exist together:

trait Foo;

struct Bar {
  foo: Rc<dyn Foo>
}

impl Bar {
  fn new(foo: Rc<dyn Foo>) -> Self {
    Self { foo }
  }
}

struct Bar2 {
  foos: Vec<Rc<dyn Foo>>
}

impl Bar2 {
  fn new(foos: Vec<Rc<dyn Foo>>) -> Self {
    Self { foos }
  }
}

In this scenario, Bar gets the last registered service for dyn Foo, but Bar2 gets all registered dyn Foo services.

5.

So far I haven't been able to discover any serious ownership issues. This was one of the first challenges in making anything work. Ultimately, everything is some form of Rc. A Transient is always created new from ServiceProvider and is never owned by it. This means it is dropped as soon as the caller is done with with. A Singleton is created once by ServiceProvider and an incremented count is returned via Rc.clone(). This is almost the same behavior for Scoped. The difference between Scoped and Singleton is that Scoped are recreated in the ServiceProvider in each new scope. When that ServiceProvider is dropped, everything else is dropped/decremented. Naturally, this can cause a memory leak, but not in some special way that ServiceProvider introduces. Typically, at the point where ServiceProvider is being dropped, everything else in the same scope (root or otherwise) is also being dropped.

from more-rs-di.

2.0.1 has now be released, which includes support for validation. The build_provider() now returns Result<ServiceProvider, ValidationError>.

Mapping to the rules above:

1. A singleton cannot have a scoped service dependency (transient is an odd case, but allowed)
2. Circular dependencies are not allowed
3. A required, unregistered type is not allowed
   a. An optional, unregistered type is allowed
   b. There are no constructors and you always have to map a factory function to create types
4. Ambiguous traits are not an issue
5. There are no ownership issues:
   a. A references are read-only and Rc, Arc, or ServiceRef (which is an convenient alias)
   b. Collections of the same service type are projected into a Vec per injection call site

I believe that covers all the scenarios. Feel free to share other scenarios if you have them.

from more-rs-di.

I believe all the validation scenarios are now covered in some form or another. Thanks for the suggestions. If you have more, keep them coming.

from more-rs-di.