Viincenttt / MollieApi

This project allows you to easily add the Mollie payment provider to your application.
MIT License
144 stars 83 forks source link

Bugfix: Use the retrypolicy from the options when available #354

Closed synercoder closed 3 months ago

synercoder commented 3 months ago

Currently, you can set the retry policy in the Action<MollieOptions> mollieOptionsDelegate when calling the AddMollieApi, but the RetryPolicy property of the options is never used. Instead the retryPolicy (optional) parameter is used (if provided).

This PR adds an if statement that checks if the retryPolicy is not yet set, if so, the use the policy from the options.

As shown in the screenshot: The property is only set in the entire solution, but never used. Screenshot showing no usage of property

Current workaround:

Don't use the RetryPolicy property, but use the available optional parameter.

builder.Services.AddMollieApi(options =>
{
    options.ApiKey = builder.Configuration["Mollie:ApiKey"];
}, MollieHttpRetryPolicies.TransientHttpErrorRetryPolicy());
Viincenttt commented 3 months ago

Hmmm... good catch!

Looking at the current code it is actually kind of strange that you can set a retrypolicy in the AddMollieApi method, but also in the MollieOptions object. What should the expected behaviour be if both of these options were set at the same time? One of them would have to be ignored.

I think it makes more sense to remove one of the two options, so it is more clear which retry policy is in effect. In my opinion, it would be best if the setting in the MollieOptions stays and we remove the optional retryPolicy parameter. Could you make this change and set the target branch to 4.0.0.0? Since this change is not backwards compatible.

synercoder commented 3 months ago

Sure, but still keep this branch around I guess? Because currently the property doesn't work, and this PR could be seen as a fix.

Viincenttt commented 3 months ago

For sure! Thanks for your PR!