Skip to content

Fix #519#1563

Merged
rynowak merged 1 commit intomasterfrom
rynowak/fix-519
May 1, 2019
Merged

Fix #519#1563
rynowak merged 1 commit intomasterfrom
rynowak/fix-519

Conversation

@rynowak
Copy link
Copy Markdown
Member

@rynowak rynowak commented Apr 25, 2019

Adds tracking for the relationship between named clients and the types
that they are bound to.

This allows us to throw a better in common cases that won't work:

  • using clients with the same type name but different FQNs
  • using the came client type twice with different names

// See comments on HttpClientMappingRegistry.
private static void ReserveClient(IHttpClientBuilder builder, Type type, string name)
{
var registry = (HttpClientMappingRegistry)builder.Services.Single(sd => sd.ServiceType == typeof(HttpClientMappingRegistry)).ImplementationInstance;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@davidfowl if you don't like this I have another cool idea b44b459

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is horrible but it's less horrible than the other way.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I really wish you had dont the cool idea way - just hit a bug with this in Orchard Core. The Cool Way would have worked :(

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I was trying to get @davidfowl to allow ConditionalWeakTable into the codebase for years 😆

// Misc infrastructure
//
services.TryAddEnumerable(ServiceDescriptor.Singleton<IHttpMessageHandlerBuilderFilter, LoggingHttpMessageHandlerBuilderFilter>());
services.TryAddSingleton(new HttpClientMappingRegistry());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add a comment here saying that this has to be an instance or it'll break the upstream code.

Adds tracking for the relationship between named clients and the types
that they are bound to.

This allows us to throw a better in common cases that won't work:

- using clients with the same type name but different FQNs
- using the came client type twice with different names
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants