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.

Sign up for GitHub

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

Jump to bottom

Add InvalidateCache to DependencyInjectionNamingSubsystem #556

Merged
merged 7 commits into from
Oct 22, 2020
Merged

Add InvalidateCache to DependencyInjectionNamingSubsystem #556

merged 7 commits into from
Oct 22, 2020

Conversation

generik0
Copy link
Contributor

@generik0 generik0 commented Oct 7, 2020

Add InvalidateCache to DependencyInjectionNamingSubsystem when services are registering

#552

@generik0 generik0 mentioned this pull request Oct 7, 2020
Closed
jonorossi
jonorossi requested changes Oct 8, 2020
View reviewed changes
...stle.Windsor.Extensions.DependencyInjection/SubSystems/DependencyInjectionNamingSubsystem.cs Outdated
Comment on lines 126 to 131
protected override void InvalidateCache()
{
handlerListsRegistrationOrderByTypeCache.Clear();
handlerListsPriorityOrderByTypeCache.Clear();
base.InvalidateCache();
}
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a unit test to show what this is fixing and so we don't get that defect back.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but not until next week.
Good point, Thabks

@jonorossi jonorossi added this to the v5.1.0 milestone Oct 8, 2020
@generik0
Copy link
Contributor Author

Hi @jonorossi. Tests added. Also cleaned up unused collection. I have changed the handlerListsByTypeCache to be protected, so the overriding GetHandler uses the same collection as the base, And hence the InvalidateCache in the base does not need to be overridden.
Is this as to be expected? @ltines @AntonioDrusin @robertcoltheart @twenzel and thoughts/comments?

@generik0
Copy link
Contributor Author

Thanks @twenzel ypu are welcome to push to my fork if you want.
Otherwise i can fix in a couple of days.

@twenzel
Copy link
Contributor

twenzel commented Oct 20, 2020

@generik0 Sure I can. Can you provide me some permissions.
Let's talk per twitter DM (@twenzel) or Slack (twenzel)

@generik0
Copy link
Contributor Author

Hi @twenzel I don't think the fork is locked is it? Other have been able to push to it. Is you try?
I don't have Twitter, and slack I do not believe you can communicate directly?
You can email and we will sort it all out. [email protected]

@generik0
Copy link
Contributor Author

@jonorossi if you have time, looks like we are ready to complete :-)

@jonorossi jonorossi linked an issue Oct 22, 2020 that may be closed by this pull request
Decorators does not work anymore (5.1.0-beta001) #552
Closed
@jonorossi jonorossi merged commit 506b762 into castleproject:master Oct 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonorossi jonorossi jonorossi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v5.1.0
Development

Successfully merging this pull request may close these issues.

Decorators does not work anymore (5.1.0-beta001)
3 participants
@generik0 @twenzel @jonorossi