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

Transitions do not honour service Scope #67

Closed
smudge202 opened this issue May 22, 2015 · 7 comments · Fixed by #79
Closed

Transitions do not honour service Scope #67

smudge202 opened this issue May 22, 2015 · 7 comments · Fixed by #79
Assignees
Milestone

Comments

@smudge202
Copy link
Collaborator

This issue has multiple parts, which I'll add as separate comments once I get my head around them.

The first issue is that ITransition<T>s which are the wrapper interfaces used to register dynamic proxies, are always registered as Singletons, regardless of the original service registration.

For example:

  services
    .AddTransient<IFoo, Bar>()
    .AsTransitional();

This will register <IFoo, Bar> as Transient, then register an <ITransition<IFoo>> as Singleton.
When attempting to resolve an IFoo, the singleton ITransition<IFoo> will actually be resolved instead, and the Bar will be resolved independently and injected into the proxy.

This means, although we honour the scope when resolving the Bar, the Singleton proxy will maintain a reference to Bar forever, extending the life span (that's bad).

@smudge202
Copy link
Collaborator Author

To resolve the first issue, we need to register the respective ITransition<> interfaces in accordance with the original service registration (if IFoo is Transient, then so must ITransition<IFoo>).

The reason that we use Singletons currently is so that we can get the instance when a Transition is requested, and perform the swap.

Going forward, we are going to need to maintain a Transition Register to track active instances of the proxies.

@smudge202
Copy link
Collaborator Author

The second part of this issue is Snapshotting.

Given the following example:

  services.AddTransient<IFoo, Foo>().AsTransitional();
  services.Snapshot<IFoo>();
  services.AddSingleton<Bar, Bar>(); // self-bind
  services.Transition<IFoo, Bar>();

With the current architecture, we have a problem in that the transition proxy is not able to honour both the scope of Foo and Bar because they are different, yet exist as fields in the proxy.

To resolve this, we will need to maintain a Snapshot Register, which is able to store the snapshotted services for a proxy. (This is complicated and has many scenarios to consider such as WeakReferencing the transients so they can naturally fall out of scope!)

@smudge202
Copy link
Collaborator Author

This issue can also manifest itself when resolving an interface implementing IDisposable more than once. Disposing one instance, would dispose them all, regardless of the scope they were originally registered under.

@smudge202
Copy link
Collaborator Author

This has been made ellegible for 4com dev because it is blocking progress on #61

@smudge202 smudge202 self-assigned this May 28, 2015
@smudge202
Copy link
Collaborator Author

We've discussed this internally and come up with a plan.

The current flow for transitions (for comparison to proposed flow below):

  1. Determine which services in the IServiceCollection should be transitional as per the Transition Markers
  2. Generate Dynamic Proxies for each Service that can be transitioned, accepting the interface as a constructor argument, and acting as both pass-through facade for the interface, and implementation of ITransition<>, allowing Snapshot, Restore, and Change operations.
  3. Register Dynamic Proxies in the Transitional Provider alongside the Service Type to allow requests for the interface to be redirected.
  4. Register the Dynamic Proxies within the Underlying Provider as Self-Bound Transient - redirected to by step 3 when service is requested directly.
  5. Register the Dynamic Proxies within the Underlying Provider as Singleton implementations of the respective ITransition<> interface.

When the interface is requested directly
Transitional provider redirects, requesting self bound dynamic from underlying provider
When the interface is requested indirectly
BUG #70: Resolves original Foo, no transitional behaviour provided
When the interface is registered as Transient
BUG #67: Resolves as singleton
Transition Service
Resolves the Singleton ITransition<> for the service.

The proposed flow and subsequent results:

  1. Determine which services in the IServiceCollection should be transitional as per the Transition Markers
  2. Generate Dynamic Proxies for each Service that can be transitioned, accepting the service implementation as a constructor argument, and acting as both pass-through facade for the interface, and implementation of ITransition<>, allowing Snapshot, Restore, and Change operations. Each generated proxy type will included a static List<WeakReference> which will be appended to by the instance constructor.
  3. Register Dynamic Proxies in the Transitional Provider alongside the Service Type to allow requests for the interface to be redirected. Register the original service implementations as self bound in the Underlying provider, honouring the original scope.
  4. Replace the original interface registrations with Registrations pointing the original interface to the dynamic proxy, honouring the original scope, within the Underlying Provider.
  5. Register the Dynamic Proxies within the Underlying Provider in accordance with their original scope for the respective ITransition<> interfaces.

When the interface is requested directly
Underlying provider handles as expected
When the interface is requested indirectly
Underlying provider handles as expected
When the interface is registered as Transient
Honours original interface scope
Transition Service
Resolves a new instance of the proxy which as per step 2, contains weak references to all instantiated instances. These can be enumerated and therefore transitioned as expected.

@smudge202
Copy link
Collaborator Author

new compose transition setup flow

@smudge202 smudge202 modified the milestones: 0.1.4-beta, 0.1.5-beta Jun 2, 2015
@smudge202
Copy link
Collaborator Author

The above process (diagram in previous comment) will offer some guide lines as to how compose handles different implementation techniques, however the code turned out to be far more involved (beyond the scope of a quick diagram).

To get an idea of how this works, examine the IServiceCollection after building up applications for each of the following:

  1. services.AddTransient<IDependency, Dependency>().AsTransitional()
  2. services.AddSingleton<IDependency, Dependency>().AsTransitional()
  3. services.AddTransient<IDependency>(provider => new Dependency()).AsTransitional()
  4. services.Singleton<IDependency>(provider => new Dependency()).AsTransitional()

There are also a series of behavioural tests available in ScopeTests once 4com/compose#1 is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant