Configure service with user-related parameters in .NET Core dependency injection

Question

I have service class which loads data from external service. This service needs API key which loads from db and unique for every user.

public class GoogleCloudService
{
 private readonly ILogger<GoogleCloudService> logger;
 public GoogleCloudService(string key)
 {
 Key = key;
 logger = Startup.ServiceProvider.GetService<ILogger<GoogleCloudService>>();
 }
 public string Key { get; set; }
 public async Task<string> ImageTextRecognizeAsync(byte[] imageBytes, string type = "TEXT_DETECTION", string languageHints = "en-t-i0-handwrit")
 {
 string url = "https://vision.googleapis.com/v1/images:annotate"
 + "?key=" + Key;
 using (var client = new HttpClient())
 {
 try
 {
 var values = new
 {
 requests = new[]
 {
 new {
 image = new { content = Convert.ToBase64String(imageBytes) },
 features = new[] { new { type = type } },
 imageContext = new { languageHints = new[] { languageHints } }
 }
 }
 };
 var response = await client.PostAsJsonAsync(url, values);
 try
 {
 response.EnsureSuccessStatusCode();
 var jsonSettings = new JsonSerializerOptions();
 jsonSettings.Converters.Add(new DynamicJsonConverter());
 dynamic responseJson = await response.Content.ReadFromJsonAsync<dynamic>(jsonSettings);
 if (responseJson is not null && responseJson.responses is not null && responseJson.responses[0] is not null && responseJson.responses[0].fullTextAnnotation is not null)
 {
 return responseJson.responses[0].fullTextAnnotation.text;
 }
 else
 {
 return null;
 }
 throw new HttpRequestException();
 }
 catch (Exception ex)
 {
 ex.Data.Add("Request", await response.RequestMessage.ToRawAsync());
 ex.Data.Add("Response", await response.ToRawAsync());
 throw;
 }
 }
 catch (Exception ex)
 {
 logger.LogError(ex, "Возникла ошибка при отправке запроса в Google Cloud.");
 }
 }
 return null;
 }
}

and usage:

var gService = new GoogleCloudService(entityDB.GoogleApiKey);
string text = await gService.ImageTextRecognizeAsync(imageBytes);

Everything works fine, but I think using global ServiceProvider from static Startup.ServiceProvider is not a good idea. How can I eliminate service locator pattern and configure service key dynamically by user in code? What are best practices?

• Alexander Petrov: public GoogleCloudService(string key, ILogger<GoogleCloudService> logger) - the constructor must accept one more parameter
• Alexander Petrov: HttpClient is intended to be instantiated once and re-used throughout the life of an application.
• sDima: Problem not in a HttpClient - it's just an example. Problem in using dependency injection with other params from DB.
• pacmaninbw: Please do not edit the question, especially the code after an answer has been posted. Everyone needs to be able to see what the reviewer was referring to. What to do after the question has been answered.

Answer 1

Here are my observations:

• Startup.ServiceProvider.GetService<ILogger<GoogleCloudService>>();: As you might know Inversion of Control are usually achieved either via Dependency Injection or via Service Locator. The later one should be avoided if possible. So, please prefer Dependency Injection:

public class GoogleCloudService
{
 private readonly ILogger<GoogleCloudService> logger;
 public GoogleCloudService(string key, ILogger<GoogleCloudService> logger)
 {
 Key = key;
 this.logger = logger;
 }

• public string Key { get; set; }: Do you really need to expose this as public? Do you really need to allow external modification?

  • If no then private readonly string key would be sufficient or if you stick to property then private string Key { get; init; }

• using (var client = new HttpClient()): You don't need to create and dispose an HttpClient for each and every request. Please prefer a static HttpClient per domain or use HttpClientFactory if you can.

• Convert.ToBase64String(imageBytes): I would suggest to perform some sort of preliminary check to make sure that imageBytes are not null and contains some data

• var jsonSettings = new JsonSerializerOptions(); This could be declared as static because this is not changing from request to request

• await response.Content.ReadFromJsonAsync<dynamic>: Try to avoid deserializing data into dynamic. Please create a concrete class, which could be used as the contract between your client and the service provider. dynamic can hide a lot of problems

• responseJson.responses[0] is not null: This might cause OutOfRangeException if the responses is defined but empty. Please check the collection's length as well prior accessing a member directly

• throw new HttpRequestException();: According to my understanding you will never reach this code. The else block is also unnecessary:

if (responseJson is not null 
 && responseJson.responses is not null 
 && responseJson.responses.Length > 0
 && responseJson.responses[0] is not null 
 && responseJson.responses[0].fullTextAnnotation is not null)
{
 return responseJson.responses[0].fullTextAnnotation.text;
}
return null;

• ex.Data.Add("Request", await response.RequestMessage.ToRawAsync()); Capturing the whole request-response might be expensive if their body are too lengthy. It might make sense to truncate them. Please bear in mind that you already have values variable which contains the request body. Please also bear in mind that you are not capture headers, which might contain valuable information
• catch(Exception ex): You can get rid of the outer try-catch if you perform preliminary checks

if(imageBytes == null || (imagesBytes != null && imagesBytes.Length < 1))
 return ...
var values = new ...
var response = await client.PostAsJsonAsync(url, values);
try
{
 response.EnsureSuccessStatusCode();
 ...
 {
 return responseJson.responses[0].fullTextAnnotation.text;
 }
 return null;
}
catch (Exception ex)
{
 ex.Data.Add("Request", await response.RequestMessage.ToRawAsync());
 ex.Data.Add("Response", await response.ToRawAsync());
 logger.LogError(ex, "Возникла ошибка при отправке запроса в Google Cloud.");
}

---

UPDATE: Reflect to comment

You can create a Factory method which is responsible for creating GoogleCloudService instances. You can take advantage ILoggerFactory (Reference) to avoid Service Locator.

public class GoogleCloudServiceFactory: ICloudServiceFactory
{
 private readonly ILoggerFactory loggerFactory;
 public GoogleCloudServiceFactory(ILoggerFactory loggerFactory)
 {
 this.loggerFactory = loggerFactory;
 }
 public ICloudService Create(string key)
 {
 return new GoogleCloudService(key, loggerFactory);
 }
}

Usage

foreach (var entityDB in entities) 
{ 
 ICloudService gService = serviceFactory.Create(entityDB.GoogleApiKey); 
 string text = await gService.ImageTextRecognizeAsync(entityDB.ImageBytes); 
}

• sDima: Thanks for your reply. My question is how to use the service without service locator in a scenario like this: foreach (var entityDB in entities) { var gService = new GoogleCloudService(entityDB.GoogleApiKey); string text = await gService.ImageTextRecognizeAsync(entityDB.ImageBytes); }
• Peter Csala: @sDima I've extended my post to reflect to your question. Please check it.

Answer 2

why not use Request headers?

You can register the IHttpContextAccessor service at Startup :

public void ConfigureServices(IServiceCollection services)
{
 services.AddControllers();
 //register IHttpContextAccessor
 services.AddHttpContextAccessor();
}

now, configure your service to construct with IHttpContextAccessor :

public class GoogleCloudService
{
 private readonly ILogger<GoogleCloudService> _logger;
 private readonly IHttpContextAccessor _httpContextAccessor;
 public GoogleCloudService(IHttpContextAccessor httpContextAccessor, ILogger<GoogleCloudService> logger)
 {
 _httpContextAccessor = httpContextAccessor;
 _logger = logger;
 
 // Get apiKey
 Key = httpContextAccessor.HttpContext.Request.Headers["apiKey"];
 }
 
 // ..etc.
}

now, everything can be configured from the controllers side. Suppose you need to validate the header and do some extra action based on it for every request, to do that, we can add a FilterAction and register it, something like this :

 // this filter will be executed on each request. 
public class ApiKeyFilterAttribute : ActionFilterAttribute
{
 public async override Task OnActionExecutionAsync(ActionExecutingContext context , ActionExecutionDelegate next)
 { 
 // get ApiKey 
 var ApiKey = context.HttpContext.Request.Headers["ApiKey"];
 
 if(string.IsNullOrWhiteSpace(ApiKey))
 {
 // the apiKey is lost or user is not authenticated
 // redirect user to the proper page 
 }
 
 // what you need to do with the ApiKey ?
 // do some async actions ..etc. 
 await base.OnActionExecutionAsync(context, next);
 }
}

now, register this filter at Startup :

public void ConfigureServices(IServiceCollection services)
{
 services.AddControllersWithViews(options =>
 {
 options.Filters.Add(new ApiKeyFilterAttribute());
 });
}

• sDima: The ApiKey is not obtained from the request and has nothing to do with authorization. It is in the database. I am making requests to a remote service using user keys on their behalf. The question is not the key, but how to organize dependency injection with passing different parameters.
• iSR5: @sDima I see now, so you need a way to use DI for passing different parameters that would be initialized from the database to be reused across the project ? is that what you ask for ?
• sDima: Yes, exactly, I have updated the example for clarity. I know I can just add a service to a dependency container with a default constructor and manually assign each property before each use, but is that the right approach?
• iSR5: @sDima if i'm following your correctly, I think you're looking for IOptions service, which you can bind your class of parameters, and reuse it where you needed through DI. Have you thought about it ?
• sDima: IOptions is static, but I need the ability to call the service in a loop with different parameters.