CyberSource / cybersource-rest-client-dotnetstandard

.NET Standard client library for the CyberSource REST API
7 stars 19 forks source link

Make this library dependency injection friendly #17

Open xantari opened 2 years ago

xantari commented 2 years ago

The way this library interacts with the MerchantConfiguration doesn't allow for making it easily testable / injectable.

Example:

We have a report service that allows us to use any of our 8 merchant accounts to retrieve reports.

However, in order to get your nuget to work we have to set these nested configuration objects, which is a bit bizarre, as they should just be injected into the classes that need them in your library.

Our Report Service constructor with the IReportsApi and IReportDownloadsApi services injected:

        public ReportService(IOptions<ProjectOptions> options, ILogger<ReportService> logger, IReportsApi reportClient, IReportDownloadsApi reportDownloadClient)
        {
            _options = options.Value;
            _logger = logger;
            _reportClient = reportClient;
            _reportDownloadClient = reportDownloadClient;
        }

An example method in order to get your nuget methods to function. Notice we have to set the configuration data in multiple places, which is strange behavior from a client library:

private async Task<ReportDownloadResult> DownloadReportAsync(ReportNames reportName, DateTime reportDate, MerchantIds merchant)
        {
            _reportDownloadClient.Configuration = GetCybersourceConfiguration(merchant);
            _reportDownloadClient.Configuration.ApiClient.Configuration = GetCybersourceConfiguration(merchant);
     ....
   }

Why are you storing the configuration object in multiple locations?

In order to do proper unit testing we can't "new" up your ReportsApi's continually as then we wouldn't be able to mock the objects and results.

Is there some better way to do dependency injection with your library?

wrightsonm commented 2 years ago

@xantari i would be very careful with your approach in a multi threaded environment. It would be much safer to create a new ReportDownloadClient for every use.

We ensure thread safety by never reusing Configuration, ApiClient or Api object i.e. PaymentsApi between threads.

We use a factory to do this. i.e. CybersourceFactory : ICybersourceFactory If you want to mock out the responses, just replace the CybersourceFactory with a MockedCybersourceFactory.

I have recently realised that most of the structure of the api has come directly from swagger-codegen. i.e.

https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/csharp/api.mustache

wrightsonm commented 2 years ago

There is a newer swagger template repository here. i think this new one is for use with openapi v3 https://github.com/swagger-api/swagger-codegen-generators/blob/master/src/main/resources/handlebars/csharp/api.mustache