Skip to content

Castle.Windsor.Extensions.DependencyInjection: support parallel containers (see #563)#577

Merged
jonorossi merged 28 commits intocastleproject:masterfrom
rvdginste:feature/support-parallel-containers
Dec 16, 2020
Merged

Castle.Windsor.Extensions.DependencyInjection: support parallel containers (see #563)#577
jonorossi merged 28 commits intocastleproject:masterfrom
rvdginste:feature/support-parallel-containers

Conversation

@rvdginste
Copy link
Contributor

@rvdginste rvdginste commented Nov 30, 2020

An attempt to fix the issue in #563.

@rvdginste rvdginste mentioned this pull request Nov 30, 2020
Closed
@generik0
Copy link
Contributor

generik0 commented Dec 1, 2020 •
edited
Loading

@rvdginste @jonorossi
Reviewed. Looks great, thanks.

@rvdginste can you give me access to the fork? I have a test to add for this change that tests the muli assembly issues easily.

@castleproject castleproject deleted a comment from jonorossi Dec 1, 2020
@rvdginste rvdginste force-pushed the feature/support-parallel-containers branch from 5194020 to 4c4a616 Compare December 1, 2020 12:04
@rvdginste
Copy link
Contributor Author

rvdginste commented Dec 1, 2020

@rvdginste can you give me access to the fork? I have a test to add for this change that tests the muli assembly issues easily.

@generik0 I sent you an invite.. not clear whether I still have to give you write access though, just let me know.

generik0
generik0 approved these changes Dec 2, 2020
View reviewed changes
@generik0
Copy link
Contributor

generik0 commented Dec 2, 2020 •
edited
Loading

@rvdginste good work.
I still don't know why I didn't catch the scope factory, as of cause we should control from the factory...! But sometimes one just goes down the wrong track. Thanks for getting the change back on track 👍
@jonorossi I have added uTest that used to fail before due to the root scope being statically bound to the test assembly, but now it is bound to the provider factory instance. Much better.
The test also documents how you can add your own container to the mix, allow for extra control and registrations.

If the tests pass in a bit, then I think this change is good to merge.

@rvdginste rvdginste force-pushed the feature/support-parallel-containers branch from c838b20 to d46be5b Compare December 2, 2020 16:10
jonorossi
jonorossi requested changes Dec 3, 2020
View reviewed changes
jonorossi
jonorossi requested changes Dec 3, 2020
View reviewed changes
Copy link
Member

@jonorossi jonorossi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @rvdginste. I've included some review comments. Could you please also update the changelog.

@jonorossi jonorossi linked an issue Dec 4, 2020 that may be closed by this pull request
Castle.Windsor.Extensions.DependencyInjection parallelism issues #563
Closed
@jonorossi jonorossi mentioned this pull request Dec 4, 2020
Closed
jonorossi
jonorossi requested changes Dec 8, 2020
View reviewed changes
jonorossi
jonorossi requested changes Dec 9, 2020
View reviewed changes
Copy link
Member

@jonorossi jonorossi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @generik0 for the back and forth on this pull request, I've got a few more review comments on the recent commits.

jonorossi
jonorossi requested changes Dec 10, 2020
View reviewed changes
@generik0
Rename CurrentOrThrow to Current
ecfa403 
castleproject#563
@generik0
Add check that the disposing scope is current, before changing the pa…
36ac910 
…rent.

castleproject#563
@generik0
Move the set of the curr3ent.Value to the ctor from the ExtensionCont…
3a3a63e 
…ainerRoot

castleproject#563
@generik0
Merge remote-tracking branch 'castle.windsor/master' into feature/sup…
bdf3507 
…port-parallel-containers
@generik0
Improve ordering for readability
32ce9c5 
castleproject#563
@generik0
Rename ExtensionContainerScope to Base
f6740c4 
castleproject#563
@generik0
Make ExtensionContainerRootScope and ExtensionContainerRoot use a base
414887d 
castleproject#563
@generik0
Change BeginScope and BeginRootScope to set current
e039c17 
castleproject#563
@generik0
Move Current into its own static class
5c7f535 
castleproject#563
@generik0
Cleanup
6d4d75e 
castleproject#563
@generik0
Cleanup
b9753fb 
castleproject#563
@generik0
Copy link
Contributor

generik0 commented Dec 10, 2020

@jonorossi i cleaned up, and improved substitution and single responsibility a bit:

  • ExtensionContainerScopeBase handles common code for ExtensionContainerScope and ExtensionContainerRootScope now
  • ExtensionContainerScopeCache is now responsible for caching the scoping

I can revert if you do not feel i have SOLIDfied the Castle.Windsor.Extensions.DependencyInjection more

jonorossi
jonorossi requested changes Dec 14, 2020
View reviewed changes
@jonorossi jonorossi added this to the vNext milestone Dec 16, 2020
@jonorossi jonorossi merged commit 14089ae into castleproject:master Dec 16, 2020
@jonorossi jonorossi mentioned this pull request Jul 23, 2023
Open
rvdginste added a commit to rvdginste/Windsor that referenced this pull request Aug 6, 2023
@rvdginste
Fix castleproject#646: fix scope and scope-accessor implementation
464595b 
To fix issue castleproject#563 (support concurrently existing containers) a fix was
created in castleproject#577.  After the initial fix some refactorings were done on
the scope implementation that were not correct.

This commit changes the scope implementation back to the original
implementation of @ltines, but with the support for concurrent
containers.
rvdginste added a commit to rvdginste/Windsor that referenced this pull request Aug 6, 2023
@rvdginste
Fix castleproject#646: fix scope and scope-accessor implementation
1203337 
To fix issue castleproject#563 (support concurrently existing containers) a fix was
created in castleproject#577.  After the initial fix some refactorings were done on
the scope implementation that were not correct.

This commit changes the scope implementation back to the original
implementation of @ltines, but with the support for concurrent
containers.
gavinschultz pushed a commit to spectraqest/Windsor that referenced this pull request Sep 12, 2023
@rvdginste @gavinschultz
Fix castleproject#646: fix scope and scope-accessor implementation
b65b43f 
To fix issue castleproject#563 (support concurrently existing containers) a fix was
created in castleproject#577.  After the initial fix some refactorings were done on
the scope implementation that were not correct.

This commit changes the scope implementation back to the original
implementation of @ltines, but with the support for concurrent
containers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jonorossi jonorossi jonorossi requested changes

+1 more reviewer

@generik0 generik0 generik0 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v6.0.0

Development

Successfully merging this pull request may close these issues.

Castle.Windsor.Extensions.DependencyInjection parallelism issues

3 participants

@rvdginste @generik0 @jonorossi