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

Ambiguous OAuthConnectionInfo constructors #270

Closed
Jericho opened this issue Feb 6, 2023 · 1 comment
Closed

Ambiguous OAuthConnectionInfo constructors #270

Jericho opened this issue Feb 6, 2023 · 1 comment
Assignees
Labels
Enhancement New feature or request
Milestone

Comments

@Jericho
Copy link
Owner

Jericho commented Feb 6, 2023

I have faced a few scenarios lately where I attempted to add optional parameters to OAuthConnectionInfo constructors to accommodate specific scenarios but it resulted in Visual Studio complaining that constructors are ambiguous. This is due to the fact that this class has four different constructors (one for each of the four major OAuth scenarios) and invoking one of these constructor while omitting certain optional parameters makes it unclear which of the 4 constructors the developer is attempting to invoke.

I propose replacing these 4 constructors and with 4 static methods with distinct names (hence removing the ambiguity even in scenarios where the parameters are similar). The constructors would be marked as obsolete for some time and eventually removed from the library which would allow developers to transition to the new syntax at their own pace.

For example, here's how the syntax for a Server-to-Server scenario would be impacted:

// Old syntax:
var connectionInfo = new OAuthConnectionInfo(clientId, clientSecret, accountId, null);

// New syntax:
var connectionInfo = OAuthConnectionInfo.ForServerToServer(clientId, clientSecret, accountId);

Here's how the syntax for a scenario where developer provides the refresh token would be impacted:

// Old syntax:
var connectionInfo = new OAuthConnectionInfo(clientId, clientSecret, refreshToken, null,
    (newRefreshToken, newAccessToken) =>
    {
        Environment.SetEnvironmentVariable("ZOOM_OAUTH_REFRESHTOKEN", newRefreshToken, EnvironmentVariableTarget.User);
    });

// New syntax:
var connectionInfo = OAuthConnectionInfo.WithRefreshToken(clientId, clientSecret, refreshToken, null,
    (newRefreshToken, newAccessToken) =>
    {
        Environment.SetEnvironmentVariable("ZOOM_OAUTH_REFRESHTOKEN", newRefreshToken, EnvironmentVariableTarget.User);
    });
@Jericho
Copy link
Owner Author

Jericho commented Mar 29, 2023

🎉 This issue has been resolved in version 0.59.0 🎉

The release is available on:

Your GitReleaseManager bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant