Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggested refactoring #30

Open
BluMichele opened this issue Jan 18, 2024 · 2 comments
Open

Suggested refactoring #30

BluMichele opened this issue Jan 18, 2024 · 2 comments
Labels
3.0.0 To be included in the version 3.0.0

Comments

@BluMichele
Copy link

Hi there,

I was working on subscriptions for which there is no API class like SendEmailApi and I noticed that the following logic si repeated multiple times in each API class

// authentication (APIKeyHeader) required
if (!string.IsNullOrEmpty(Configuration.GetApiKeyWithPrefix("Authorization")))
    if (!localVarRequestOptions.HeaderParameters.ContainsKey("Authorization"))
        localVarRequestOptions.HeaderParameters.Add("Authorization",
            Configuration.GetApiKeyWithPrefix("Authorization"));
// authentication (Basic) required
// http basic authentication required
if (!string.IsNullOrEmpty(Configuration.Username) || !string.IsNullOrEmpty(Configuration.Password))
    localVarRequestOptions.HeaderParameters.Add("Authorization",
        "Basic " + ClientUtils.Base64Encode(Configuration.Username + ":" + Configuration.Password));
// authentication (IBSSOTokenHeader) required
if (!string.IsNullOrEmpty(Configuration.GetApiKeyWithPrefix("Authorization")))
    if (!localVarRequestOptions.HeaderParameters.ContainsKey("Authorization"))
        localVarRequestOptions.HeaderParameters.Add("Authorization",
            Configuration.GetApiKeyWithPrefix("Authorization"));
// authentication (OAuth2) required
// oauth required
if (!string.IsNullOrEmpty(Configuration.AccessToken))
    localVarRequestOptions.HeaderParameters.Add("Authorization", "Bearer " + Configuration.AccessToken);

// make the HTTP request

If I might, I suggest creating a new method like AddAuthenticationHeaders in ApiClient and calling it at the beginning of ExecAsync, you are already setting things up from configuration in that location, so why do half the job in ApiClient and half in each method of each API class, move all there 🤷🏻‍♂️.

@BluMichele
Copy link
Author

Also, the code will never enter the third block of if-if-add which is identical to the first.

if (!string.IsNullOrEmpty(Configuration.GetApiKeyWithPrefix("Authorization")))
    if (!localVarRequestOptions.HeaderParameters.ContainsKey("Authorization"))
        localVarRequestOptions.HeaderParameters.Add("Authorization",
            Configuration.GetApiKeyWithPrefix("Authorization"));

@ib-plivaja
Copy link

Hi @BluMichele,

thank you for suggested updates and for using our library.
We are planning to fully refactor this library in the next major version release your suggestions will be taken into account. We'll keep you informed.

Kind regards,
Petra

@ib-plivaja ib-plivaja added the 3.0.0 To be included in the version 3.0.0 label Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.0.0 To be included in the version 3.0.0
Projects
None yet
Development

No branches or pull requests

2 participants