[Feature Request] introduce a base class for IDownstreamApi to simplify mocking in tests
Is your feature request related to a problem? Please describe. A clear and concise description of what the problem is. Ex. I'm always frustrated when [...].
mocking the IDownstreamApi class in tests isn't trivial, the interface contains 36 methods (by creating a test class the implements this interface).
Describe the solution you'd like
To have a base abstract class DownstreamApiBase that implements the interface and exposes just a single method
protected async Task<HttpResponseMessage> CallApiInternalAsync(
HttpRequestMessage httpRequestMessage,
DownstreamApiOptions effectiveOptions,
bool appToken,
HttpContent? content = null,
ClaimsPrincipal? user = null,
CancellationToken cancellationToken = default);
this would mean that the current https://github.com/AzureAD/microsoft-identity-web/blob/master/src/Microsoft.Identity.Web.DownstreamApi/DownstreamApi.cs#L455 API would be slightly different
internal /* for tests */ async Task<HttpResponseMessage> CallApiInternalAsync(
string? serviceName,
DownstreamApiOptions effectiveOptions,
bool appToken,
HttpContent? content = null,
ClaimsPrincipal? user = null,
CancellationToken cancellationToken = default)
{
// Downstream API URI
string apiUrl = effectiveOptions.GetApiUrl();
// Create an HTTP request message
using HttpRequestMessage httpRequestMessage = new(
new HttpMethod(effectiveOptions.HttpMethod),
apiUrl);
if (content != null)
{
httpRequestMessage.Content = content;
}
// call the new protected API
return await CallApiInternalAsync(message, effectiveOptions, appToken, user, cancellationToken);
then the current DownstreamApi class would extend this abstract-class and override the CallApiInternalAsync method.
This would allow any HttpClient mocking library to easily be leveraged here since the new API would be close to the HttpClient.SendAsync API.
Describe alternatives you've considered
using the Moq library and mocking the interface IDownstreamApi - mocking the correct method is a bit of a nuisance since there are many similar API's.
Additional context Add any other context or screenshots about the feature request here.
@bgavrilMS: any thoughts?
@jmprieur @bgavrilMS any update on this?
I'm willing to create the PR here, just want to be sure it's relevant and that the API change as suggested is approved.
Thanks for offering, @Meir017 What changes do you have in mind for the public API?
@jmprieur I drafted an initial PR to highlight the API changes this would include
@jmprieur Maybe we could take another path and make many interface API's extension methods in the same assembly? This will result in the interface containing just the critical method, allowing for easy mocking.
This might be more complex as it involves multiple repositories.
Maybe we could start by making the class public and making the core method with the logic overrideable.
-internal partial class DownstreamApi : IDownstreamApi
+public partial class DownstreamApi : IDownstreamApi
{
- internal /* for tests */ async Task<HttpResponseMessage> CallApiInternalAsync(
+ protected virtual async Task<HttpResponseMessage> CallApiInternalAsync(
}
This should be a good start, If this makes sense, I'll be happy to create a proper PR for this
all this is great but can someone show me how to currently do a unit test for a call to .GetForAppAsync<T>() i need to finish a pull and i am trying to get this solved.... thanks!