Adding Util class for FIrebaseInstallations APIs. #676

ankitaj224 · 2019-07-31T18:47:27Z

No description provided.

This reverts commit 2ec4aef.

This reverts commit 7b4ebcf.

This reverts commit 2ec4aef.

… into ankita_fis

* Adding an interface library for Firebase Installations SDK * Adding Firebase Installations module * Adding Firebase Installations module. * Readding .idea files that were deleted in previous commit * Revert "Adding Firebase Installations module" This reverts commit 2ec4aef. * Revert "Readding .idea files that were deleted in previous commit" This reverts commit 7b4ebcf. * Add firebase installations project path * Adding Firebase Installations module. * Readding .idea files that were deleted in previous commit * Revert "Adding Firebase Installations module" This reverts commit 2ec4aef. * Revert "Readding .idea files that were deleted in previous commit" This reverts commit 7b4ebcf. * Add firebase installations project path * Fixing formattinf issues. * Revert "Adding Firebase Installations module" with hidden files This reverts commit 2ec4aef. * Addressing review comments. * Making InstallationTokenResult an AutoValue class.

This reverts commit 2ec4aef.

This reverts commit 7b4ebcf.

This reverts commit 2ec4aef.

… into ankita_fis

…to ankita_fis

… into ankita_fis

vkryachko

Looks like this PR contains a lot of code, some of which I'd previously reviewed for @diwu-arete . Can you make a PR that only contains new changes? otherwise the PR is too big and I am not sure which parts to review and which to skip

…to ankita_fis

… into ankita_fis

diwu-arete · 2019-08-07T22:59:19Z

...allations/src/main/java/com/google/firebase/installations/FirebaseInstallationException.java

+
+  public enum Status {
+    SDK_INTERNAL_ERROR,
+


I suggest to sync with mertcan@ or Rayo about what status we want to expose to dev

@andirayo or @mmermerkaya Whats the high level exception code to be shown to developers in case FIS requests fail? Currently, I have SDK_INTERNAL_ERROR.

I would check with Mertcan's Web-SDK or Maksym's iOS-SDK, but as far as I remember (without checking) I think we are pretty explicit with our error messages.
There are some error messages we receive from the server that our SDK reacts to by resending the same request once. If we can not fix the error, we print a pretty descriptive error message for the developer to be able to understand and fix.

firebase-installations/src/main/java/com/google/firebase/installations/Utils.java

...lations-interop/src/main/java/com/google/firebase/installations/InstallationTokenResult.java

.../androidTest/java/com/google/firebase/installation/FirebaseInstallationInstrumentedTest.java

...allations/src/main/java/com/google/firebase/installations/FirebaseInstallationException.java

...rc/main/java/com/google/firebase/installations/remote/FirebaseInstallationServiceClient.java

andirayo · 2019-08-09T21:19:10Z

...rc/main/java/com/google/firebase/installations/remote/FirebaseInstallationServiceClient.java

@@ -91,16 +91,16 @@ public InstallationResponse createFirebaseInstallation(
          return readCreateResponse(httpsURLConnection);
        case 401:
          throw new FirebaseInstallationServiceException(
-              UNAUTHORIZED_ERROR_MESSAGE, FirebaseInstallationServiceException.Code.UNAUTHORIZED);
+              UNAUTHORIZED_ERROR_MESSAGE, FirebaseInstallationServiceException.Status.UNAUTHORIZED);
        default:


Can this ever happen?
How is this a Server-Error?
Seems that this is a client error for not catching all possible error codes.
I think, we should give more information to the developer what is happening here.

Added a TODO in FirebaseInstallationsException class. I will send out another PR for exception handling changes.

...rc/main/java/com/google/firebase/installations/remote/FirebaseInstallationServiceClient.java

andirayo · 2019-08-09T21:22:38Z

...main/java/com/google/firebase/installations/remote/FirebaseInstallationServiceException.java

@@ -19,39 +19,38 @@

 /** The class for all Exceptions thrown by {@link FirebaseInstallationServiceClient}. */
 public class FirebaseInstallationServiceException extends FirebaseException {
-
-  public enum Code {
+  public enum Status {
    SERVER_ERROR,

    NETWORK_ERROR,

    UNAUTHORIZED


what about INVALID_ARGUMENT, FORBIDDEN, NOT_FOUND, TOO_MANY_REQUEST?

I will handle this in another PR.

andirayo · 2019-08-09T21:24:32Z

...ions/src/test/java/com/google/firebase/installations/FirebaseInstallationsRegistrarTest.java

+        FirebaseApp.initializeApp(
+            ApplicationProvider.getApplicationContext(),
+            new FirebaseOptions.Builder().setApplicationId("1:987654321:android:abcdef").build(),
+            "firebase_app_1");


this seems to cover 2 tests in once:
i.e. getting a different instance for a different App-ID, but also getting a different instance for different (nick)name.
Maybe those can be distinguished into 2 separate tests, i.e getting a different instance for different App-ID, and one getting different instance for different (nick)name?

We are not able to create a diff firebase app with different App-ID without a nick-name. As a result, they cant be split in two. On a high level test the instance creation.

andirayo

The title for this commit/PR is missing an "s"! haha

firebase-installations/src/main/java/com/google/firebase/installations/Utils.java

ankitaj224 · 2019-08-14T22:47:12Z

The title for this commit/PR is missing an "s"! haha

Updated.

…to ankita_fis

* getId() implementation with instrumented tests. * Addressing rayo's comments. - Detailed JavaDocs - Renaming FiidCache as PersistedFid * Addressing rayo's comments. - Detailed JavaDocs - Renaming FiidCache as PersistedFid * Addresing comments to resoleve the following: - Make registerAnsSaveFid non blocking call in getId() - PersistedFidEntry builder with default values * Addressing Ciaran and Rayo's comments. * Addressing Ciaran's comments * Addressing Ciaran's comments * Adding param comments and checking if registration status is valid. * Correcting lint warning: uses-permission should be declared before application in AndroidManifest.xml * Adding custom assertThat with more readable assertions * Correcting instrumented tests to be reliable.

google-oss-bot · 2019-08-30T20:29:36Z

@ankitaj224: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
api-information	`429581e`	link	`/test api-information`

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

* Adding Firebase Installations module * Readding .idea files that were deleted in previous commit * Revert "Adding Firebase Installations module" This reverts commit 2ec4aef. * Revert "Readding .idea files that were deleted in previous commit" This reverts commit 7b4ebcf. * Adding Firebase Installations module. * Readding .idea files that were deleted in previous commit * Revert "Readding .idea files that were deleted in previous commit" This reverts commit 7b4ebcf. * Revert "Adding Firebase Installations module" with hidden files This reverts commit 2ec4aef. * Addressing review comments. * Http client to call FIS backend service. * Http client to call FIS backend service. * Http client to call FIS backend service. * Adding Firebase Installations module * Adding Firebase Installations module. * Readding .idea files that were deleted in previous commit * Readding .idea files that were deleted in previous commit * Revert "Adding Firebase Installations module" This reverts commit 2ec4aef. * Revert "Readding .idea files that were deleted in previous commit" This reverts commit 7b4ebcf. * Revert "Readding .idea files that were deleted in previous commit" This reverts commit 7b4ebcf. * Revert "Adding Firebase Installations module" with hidden files This reverts commit 2ec4aef. * Addressing review comments. * Http client to call FIS backend service. * Http client to call FIS backend service. * Initial Code structure for FIS Android SDK (#648) * Adding an interface library for Firebase Installations SDK * Adding Firebase Installations module * Adding Firebase Installations module. * Readding .idea files that were deleted in previous commit * Revert "Adding Firebase Installations module" This reverts commit 2ec4aef. * Revert "Readding .idea files that were deleted in previous commit" This reverts commit 7b4ebcf. * Add firebase installations project path * Adding Firebase Installations module. * Readding .idea files that were deleted in previous commit * Revert "Adding Firebase Installations module" This reverts commit 2ec4aef. * Revert "Readding .idea files that were deleted in previous commit" This reverts commit 7b4ebcf. * Add firebase installations project path * Fixing formattinf issues. * Revert "Adding Firebase Installations module" with hidden files This reverts commit 2ec4aef. * Addressing review comments. * Making InstallationTokenResult an AutoValue class. * Http client to call FIS backend service. * Addresing comments and introducing new FirebaseInstallationService Exception. * Implementing getId method in FirebaseInstallation to call backend. * 1. Adding instrumentation tests. 2. Introducing serviceClient level and main class exceptions. * Addressing Di's comments. * Addressing Rayo's comments * Updating parameters order in ServiceClient. * Updating createRandomFid as per Rayo's comments. * getId() implementation with instrumented tests. (#703) * getId() implementation with instrumented tests. * Addressing rayo's comments. - Detailed JavaDocs - Renaming FiidCache as PersistedFid * Addressing rayo's comments. - Detailed JavaDocs - Renaming FiidCache as PersistedFid * Addresing comments to resoleve the following: - Make registerAnsSaveFid non blocking call in getId() - PersistedFidEntry builder with default values * Addressing Ciaran and Rayo's comments. * Addressing Ciaran's comments * Addressing Ciaran's comments * Adding param comments and checking if registration status is valid. * Correcting lint warning: uses-permission should be declared before application in AndroidManifest.xml * Adding custom assertThat with more readable assertions * Correcting instrumented tests to be reliable.

ankitaj224 added 30 commits July 24, 2019 14:03

Adding Firebase Installations module

d39159b

Readding .idea files that were deleted in previous commit

e321d7d

Revert "Adding Firebase Installations module"

c25ca83

This reverts commit 2ec4aef.

Revert "Readding .idea files that were deleted in previous commit"

6c54e73

This reverts commit 7b4ebcf.

Adding Firebase Installations module.

1923730

Readding .idea files that were deleted in previous commit

e79e3ad

Revert "Readding .idea files that were deleted in previous commit"

bb41402

This reverts commit 7b4ebcf.

Revert "Adding Firebase Installations module" with hidden files

2851062

This reverts commit 2ec4aef.

Addressing review comments.

a2f7f64

Http client to call FIS backend service.

cde2320

Http client to call FIS backend service.

a27b347

Http client to call FIS backend service.

53dc90b

Merge branch 'ankita_fis' of github.com:firebase/firebase-android-sdk…

a7982e1

… into ankita_fis

Adding Firebase Installations module

19c5573

Readding .idea files that were deleted in previous commit

892fddf

Revert "Adding Firebase Installations module"

8d3dc73

This reverts commit 2ec4aef.

Revert "Readding .idea files that were deleted in previous commit"

4ad27d7

This reverts commit 7b4ebcf.

Adding Firebase Installations module.

7f45186

Readding .idea files that were deleted in previous commit

1b1766f

Revert "Readding .idea files that were deleted in previous commit"

4ac3956

This reverts commit 7b4ebcf.

Revert "Adding Firebase Installations module" with hidden files

da9aac6

This reverts commit 2ec4aef.

Addressing review comments.

e9c10c2

Http client to call FIS backend service.

ef6cf7d

Http client to call FIS backend service.

284ab52

Http client to call FIS backend service.

4332d7f

Merge branch 'ankita_fis' of github.com:firebase/firebase-android-sdk…

113216e

… into ankita_fis

Merge branch 'fis_sdk' of github.com:firebase/firebase-android-sdk in…

936e2f8

…to ankita_fis

Merge branch 'fis_sdk' of github.com:firebase/firebase-android-sdk in…

1f427eb

…to ankita_fis

Merge branch 'ankita_fis' of github.com:firebase/firebase-android-sdk…

5462241

… into ankita_fis

vkryachko reviewed Aug 7, 2019

View reviewed changes

ankitaj224 added 3 commits August 7, 2019 14:10

Merge branch 'fis_sdk' of github.com:firebase/firebase-android-sdk in…

0a79342

…to ankita_fis

Merge branch 'fis_sdk' of github.com:firebase/firebase-android-sdk in…

71c90f0

…to ankita_fis

Merge branch 'ankita_fis' of github.com:firebase/firebase-android-sdk…

d11417b

… into ankita_fis

google-oss-bot added size/L and removed size/XL labels Aug 7, 2019

diwu-arete reviewed Aug 7, 2019

View reviewed changes

firebase-installations/src/main/java/com/google/firebase/installations/Utils.java Outdated Show resolved Hide resolved

diwu-arete reviewed Aug 7, 2019

View reviewed changes

firebase-installations/src/main/java/com/google/firebase/installations/Utils.java Outdated Show resolved Hide resolved

Addressing Di's comments.

a2e0dba

ankitaj224 changed the title ~~Implement getId method in FIrebaseInstallation to interact with cache and backend.~~ Adding util class for FIrebaseInstallation APIs. Aug 7, 2019

ankitaj224 changed the title ~~Adding util class for FIrebaseInstallation APIs.~~ Adding Util class for FIrebaseInstallation APIs. Aug 7, 2019

andirayo reviewed Aug 9, 2019

View reviewed changes

ankitaj224 added 2 commits August 12, 2019 13:57

Addressing Rayo's comments

8706286

Updating parameters order in ServiceClient.

34848c6

andirayo approved these changes Aug 14, 2019

View reviewed changes

Updating createRandomFid as per Rayo's comments.

f4c44b0

ankitaj224 changed the title ~~Adding Util class for FIrebaseInstallation APIs.~~ Adding Util class for FIrebaseInstallations APIs. Aug 14, 2019

andirayo approved these changes Aug 17, 2019

View reviewed changes

ankitaj224 added 3 commits August 29, 2019 10:37

Merge branch 'fis_sdk' of github.com:firebase/firebase-android-sdk in…

44bef2f

…to ankita_fis

Merge branch 'fis_sdk' of github.com:firebase/firebase-android-sdk in…

e5e0d67

…to ankita_fis

google-oss-bot added size/XXL and removed size/L labels Aug 30, 2019

ankitaj224 merged commit 77b6d97 into fis_sdk Aug 30, 2019

ankitaj224 deleted the ankita_fis branch August 30, 2019 21:48

firebase locked and limited conversation to collaborators Oct 8, 2019

Adding Util class for FIrebaseInstallations APIs. #676

Adding Util class for FIrebaseInstallations APIs. #676

Uh oh!

Conversation

ankitaj224 commented Jul 31, 2019

Uh oh!

vkryachko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ankitaj224 Aug 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andirayo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ankitaj224 commented Aug 14, 2019

Uh oh!

google-oss-bot commented Aug 30, 2019

Uh oh!

Uh oh!

ankitaj224 Aug 7, 2019 •

edited

Loading