Skip to content

fix(auth): Make user import hashing classes final #425

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

Merged
merged 1 commit into from
Jun 13, 2020

Conversation

MathBunny
Copy link
Contributor

@MathBunny MathBunny commented May 27, 2020

While working on the .NET port of user imports I brought up the fact that these classes weren't final in the Java version; this PR revisits that idea.

API CHANGE: All public implementations of UserImportHash class are now marked final.

@MathBunny MathBunny self-assigned this May 27, 2020
@hiranya911
Copy link
Contributor

Thanks @MathBunny. The PR looks good. Unfortunately this is also a breaking API change. So we have to wait until the next major release before we can merge this.

@hiranya911
Copy link
Contributor

@MathBunny can you change the base branch of this PR to v7 (instead of master)? Then we can approve and merge it there.

@MathBunny MathBunny changed the base branch from master to v7 May 29, 2020 03:39
@MathBunny
Copy link
Contributor Author

Looks like I need to rebase my branch, I'll look into this tomorrow

@MathBunny MathBunny force-pushed the make-hash-classes-final branch from 313d769 to 9935293 Compare May 29, 2020 13:39
@hiranya911 hiranya911 changed the title chore: Make user import hashing classes final fix(auth): Make user import hashing classes final Jun 12, 2020
Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@MathBunny MathBunny merged commit be73285 into firebase:v7 Jun 13, 2020
hiranya911 added a commit that referenced this pull request Aug 18, 2020
* Added core types for the error handling revamp (#350)

* Added core types for the error handling revamp

* Fixed copyright year

* Added ErrorHandlingHttpClient API (#353)

* Added core error handling abstractions

* Added unit tests for the new functionality

* Exposed getErrorCode as getErrorCodeNew

* Enabled error code assertions

* Error handling revamp for the FirebaseMessaging API (#357)

* Added core error handling abstractions

* Added unit tests for the new functionality

* Exposed getErrorCode as getErrorCodeNew

* Enabled error code assertions

* Error handling revamp for FCM APIs

* Cleaned up the FirebaseMessagingException

* Cleaned up the AbstractHttpErrorHandler class

* Updated tests

* Error handling revamp for FirebaseInstanceId API (#359)

* Delete instance ID API error handling revamp

* Added tests for IO and parse errors

* fix(auth): Migrated user management APIs to the new error handling scheme (#360)

* Error handling revamp in FirebaseUserManager

* Updated integration tests; Added documentation

* Assigning the correct ErrorCode for auth errors

* Moved AuthErrorHandler to a separate top-level class

* Error handling revamp for token verification APIs (#362)

* Error handling revamp for token verification APIs

* Updated javadocs

* Error handling revamp for the custom token creation API (#366)

* Error handlign revamp for the custom token creation API

* Using the correct authorized HTTP client for IAM requests

* Error handling revamp for the project management API (#367)

* Error handling revamp for the project management API

* Minor code and test cleanup

* Fixed some lint errors; Removed requestFactory reference from project mgt service impl

* Renamed getErrorCodeNew() to getErrorCode() (#379)

* Minor code and test cleanup

* Renamed getErrorCodeNew() to getErrorCode() in FirebaseException

* Fixing checkstyle error

* Fixing some deprecation warnings (#380)

* Handling IID error codes correctly (#381)

* Removed old deprecated APIs (#383)

* fix: Removed unused FirebaseAppStore abstraction (#427)

* fix: Removed unused FirebaseAppStore abstraction

* Using the keySet of App instances to populate the app names list

* chore: Removing redundant test dependency (#441)

* chore: Make user import hashing classes final (#425)

* chore: Merged with v7 branch with master (#456)

* fix(fcm): Replacing deprecated Batch API constructor (#460)

* fix: Handling http method override in ErrorHandlingHttpClient (#459)

* fix: Handling JSON serialization/response interception at ErrorHandlingHttpClient (#462)

* fix: Handling JSON serialization and response interception at ErrorHandlingHttpClient

* fix: Removing redundant method override header

* feat: Added new error codes for IdP management and multitenancy (#458)

* feat: Added new error codes for IdP management and multitenancy

* fix: Updated integration tests

* fix: Renamed helper method

* fix: Removing some calls to deprecated APIs (#464)

* chore: Support for specifying query parameters in HttpRequestInfo (#463)

* chore: Support for specifying query parameters in HttpRequestInfo

* fix: Removing redundant JsonObjectParser from HttpClient

* fix: Fixing a series of javadoc warnings (#466)

* fix: Made some APIs on AbstractFirebaseAuth.Builder package-protected for consistency

* Apply suggestions from code review

Co-authored-by: egilmorez <[email protected]>
Co-authored-by: Kevin Cheung <[email protected]>

* fix: Minor updates to API ref docs based on code review comments

* fix: Fixing API doc wording

Co-authored-by: Horatiu Lazu <[email protected]>
Co-authored-by: egilmorez <[email protected]>
Co-authored-by: Kevin Cheung <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants