fix: Removed unused FirebaseAppStore abstraction #427
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lahirumaramba
approved these changes
May 27, 2020
| @@ -249,19 +241,15 @@ static void clearInstancesForTest() { | |||
| } | |||
|
|
|||
| private static List<String> getAllAppNames() { | |||
| Set<String> allAppNames = new HashSet<>(); | |||
| List<String> allAppNames = new ArrayList<>(); | |||
There was a problem hiding this comment.
Was there a reason a Set was used here before? Could it be to avoid duplicates?
There was a problem hiding this comment.
I think that's the idea, but it's redundant. App names are unique by definition. When adding a new app we check this:
checkState(
!instances.containsKey(normalizedName),
"FirebaseApp name " + normalizedName + " already exists!");
This actually gives me an idea. We can use the instances.keySet() to populate the List. I'll make the change.
There was a problem hiding this comment.
Great! Makes sense to me.
lahirumaramba
approved these changes
May 27, 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a bit of dead legacy code left over from the Android SDK, and has never been used in any useful or meaningful way. Therefore I'm removing it from the v7 release branch.