Fix conflicting HttpClient configs

[stormaref]

Changed DefaultHttpClientBuilder name and added new test

Fix #110996

[stormaref]

@stormaref please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@dotnet-policy-service agree [company="{your company}"]

Options:

• (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.

@dotnet-policy-service agree

• (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.

@dotnet-policy-service agree company="Microsoft"

Contributor License Agreement

@dotnet-policy-service agree

[pinkfloydx33]

This makes the documentation on the method (and any that call it) now incorrect. (line 440)

[stormaref]

You're right I'm gonna change the documentation

[pinkfloydx33]

Fix #110996

Isn't this documented and expected behavior?
From https://learn.microsoft.com/en-us/dotnet/core/extensions/httpclient-factory-troubleshooting#different-typed-clients-are-registered-on-a-common-interface

DO NOT register multiple typed clients on a single interface without explicitly specifying the name.

Also doesn't it just create the inverse problem? Ie. you'd not be able to reuse a single concrete type for multiple interfaces (though I don't know if that's a valid use case to begin with)

[stormaref]

Fix #110996

Isn't this documented and expected behavior? From https://learn.microsoft.com/en-us/dotnet/core/extensions/httpclient-factory-troubleshooting#different-typed-clients-are-registered-on-a-common-interface

DO NOT register multiple typed clients on a single interface without explicitly specifying the name.

Also doesn't it just create the inverse problem? Ie. you'd not be able to reuse a single concrete type for multiple interfaces (though I don't know if that's a valid use case to begin with)

It's so common to have one interface with multiple implementation and it is also common to add one interface with multiple implementations; actually it is the whole definition of IoC. Meanwhile it's not common to add one implementation with two interface.
Also if you think that's a serious problem, I can simply concat the name of the interface with the name of the implementation so it would fix both problems

[ArminShoeibi]

I believe this introduces a breaking change. However, if you're looking to have multiple implementations for an interface, you can take advantage of the options available for named and typed HttpClients. It’s a great way to handle different scenarios!

[stormaref]

There are indeed several ways to achieve this, but this is something that ought to work as expected out of the box. With dependency injection, the expected behavior is to allow different implementations of an interface to coexist without conflicts. However, the current setup leads to issues, as evidenced by many people encountering this problem.

Here’s a related discussion on StackOverflow that highlights this issue: https://stackoverflow.com/questions/74005464/add-httpclients-with-same-interface-ended-up-having-the-same-base-url-asp-net-co

Also, what is the problem with changing this behavior? What do you think could go wrong with allowing multiple HttpClient registrations under the same interface to retain their independent configurations?

[ArminShoeibi]

I noticed that this code might be missing a few other implementations. Just wanted to point it out.

runtime/src/libraries/Microsoft.Extensions.Http/src/DependencyInjection/HttpClientFactoryServiceCollectionExtensions.cs

Lines 707 to 716 in 6eba467

	public static IHttpClientBuilder AddHttpClient<TClient, TImplementation>(this IServiceCollection services, Func<HttpClient, TImplementation> factory)
	where TClient : class
	where TImplementation : class, TClient
	{
	ThrowHelper.ThrowIfNull(services);
	ThrowHelper.ThrowIfNull(factory);
	string name = TypeNameHelper.GetTypeDisplayName(typeof(TClient), fullName: false);
	return AddHttpClient<TClient, TImplementation>(services, name, factory);
	}

About your question:

what is the problem with changing this behavior?

I don't know, but @CarnaViire can help.

I encourage you to explore these links and review the related issues.

runtime/src/libraries/Microsoft.Extensions.Http/src/DependencyInjection/HttpClientBuilderExtensions.cs

Lines 765 to 785 in 95814d0

	if (registry.NamedClientRegistrations.TryGetValue(name, out Type? otherType) &&
	// Allow using the same name with multiple types in some cases (see callers).
	validateSingleType &&
	// Allow registering the same name twice to the same type.
	type != otherType)
	{
	string message =
	$"The HttpClient factory already has a registered client with the name '{name}', bound to the type '{otherType.FullName}'. " +
	$"Client names are computed based on the type name without considering the namespace ('{otherType.Name}'). " +
	$"Use an overload of AddHttpClient that accepts a string and provide a unique name to resolve the conflict.";
	throw new InvalidOperationException(message);
	}
	if (validateSingleType)
	{
	registry.NamedClientRegistrations[name] = type;
	}
	}

1. Two httpclients configured by httpclientfactory for classes with same name conflict extensions#960
2. Fix #519 extensions#1563