Migrate RC to depend directly on Installations SDK. #1579

danasilver · 2020-05-21T00:46:05Z

No description provided.

google-oss-bot · 2020-05-21T00:53:02Z

Coverage Report

Affected SDKs

firebase-config

SDK overall coverage changed from 88.83% (43f5c6d) to 88.71% (aea06f22) by -0.12%.

Filename Base (43f5c6d) Head (aea06f22) Diff

ConfigFetchHandler.java 98.55% 97.20% -1.35%

FirebaseRemoteConfig.java 88.19% 88.28% +0.08%

Test Logs

Notes

HTML coverage reports can be produced locally with ./gradlew <product>:checkCoverage.
Report files are located at <product-build-dir>/reports/jacoco/.

Head commit (aea06f22) is created by Prow via merging commits: 43f5c6d 5dcf294.

google-oss-bot · 2020-05-21T00:54:34Z

Binary Size Report

Affected SDKs

firebase-abt

Type Base (43f5c6d) Head (aea06f22) Diff

apk (aggressive) 85.3 kB 85.3 kB +6 B (+0.0%)
firebase-common

Type Base (43f5c6d) Head (aea06f22) Diff

apk (aggressive) 82.5 kB 82.5 kB +18 B (+0.0%)

apk (debug) 770 kB 770 kB +5 B (+0.0%)
firebase-common-ktx

Type Base (43f5c6d) Head (aea06f22) Diff

apk (aggressive) 272 kB 272 kB +5 B (+0.0%)

apk (debug) 1.56 MB 1.56 MB -82 B (-0.0%)
firebase-components

Type Base (43f5c6d) Head (aea06f22) Diff

apk (aggressive) 11.0 kB 11.0 kB +6 B (+0.1%)

apk (debug) 35.9 kB 35.9 kB +3 B (+0.0%)

`firebase-config`

Type	Base (`43f5c6d`)	Head (aea06f22)	Diff
aar	215 kB	215 kB	+40 B (+0.0%)
apk (aggressive)	403 kB	129 kB	-274 kB (-67.9%)
apk (debug)	1.34 MB	1.05 MB	-294 kB (-21.9%)
apk (release)	1.15 MB	870 kB	-280 kB (-24.4%)

`firebase-config-ktx`

Type	Base (`43f5c6d`)	Head (aea06f22)	Diff
aar	6.16 kB	5.83 kB	-328 B (-5.3%)
apk (aggressive)	593 kB	319 kB	-274 kB (-46.1%)
apk (debug)	2.13 MB	1.84 MB	-295 kB (-13.8%)
apk (release)	1.78 MB	1.50 MB	-279 kB (-15.7%)

firebase-installations

Type Base (43f5c6d) Head (aea06f22) Diff

apk (aggressive) 84.2 kB 84.3 kB +21 B (+0.0%)

apk (debug) 794 kB 794 kB -27 B (-0.0%)
firebase-installations-interop

Type Base (43f5c6d) Head (aea06f22) Diff

apk (aggressive) 61.7 kB 61.7 kB +7 B (+0.0%)

apk (debug) 744 kB 744 kB -1 B (-0.0%)

Test Logs

Notes

Head commit (aea06f22) is created by Prow via merging commits: 43f5c6d 5dcf294.

danasilver · 2020-06-02T00:28:48Z

/test smoke-tests

firebase-config/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfig.java

firebase-config/src/main/java/com/google/firebase/remoteconfig/internal/ConfigFetchHandler.java

egilmorez

A couple of terminology things for you, thanks!

firebase-config/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfig.java

.../bandwagoner/src/main/java/com/googletest/firebase/remoteconfig/bandwagoner/ApiFragment.java

andirayo · 2020-06-09T00:48:13Z

.../bandwagoner/src/main/java/com/googletest/firebase/remoteconfig/bandwagoner/ApiFragment.java

+
+    Tasks.whenAllComplete(idTask, tokenTask)
+        .addOnCompleteListener(
+            unusedCompletedTasks -> {


How about ignoredTasks?
I am not sure what Android naming convention is here, but do you also use the underscore "_" prefix?
_ignoredTasks

Not sure there's a real convention in Android or Java, but we use "unused" consistently as a prefix in Remote Config at least, so I'd lean towards keeping it.

.../bandwagoner/src/main/java/com/googletest/firebase/remoteconfig/bandwagoner/ApiFragment.java

firebase-config/src/main/java/com/google/firebase/remoteconfig/internal/ConfigFetchHandler.java

andirayo · 2020-06-09T03:16:11Z

firebase-config/src/main/java/com/google/firebase/remoteconfig/internal/ConfigFetchHandler.java

+                    return fetchFromBackendAndCacheResponse(
+                        installationId, installationToken, currentTime);


I know we already talked about this, but I strongly recommend to only upload the FIS auth token, because the auth token contains all the information needed on server side, particularly the FID.
Thus, uploading both values is redundancy and might lead to inconsistencies on server side.

As far as I understood our conversations, you are planning to get rid of uploading the FID in the future, but want to upload it for now to keep the current processes consistent.
That is absolutely fine. I just wanted to mention it again.

Thanks for calling it out!

firebase-config/src/main/java/com/google/firebase/remoteconfig/internal/ConfigFetchHandler.java

firebase-config/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigTest.java

...e-config/src/test/java/com/google/firebase/remoteconfig/internal/ConfigFetchHandlerTest.java

danasilver · 2020-06-17T00:22:45Z

/test smoke-tests

danasilver added 4 commits May 19, 2020 16:50

Begin migrating to direct FIS dependency.

95cb9bd

Migrate RC to directly depend on Installations SDK.

fa509ae

Migrate RC tests and ktx to FIS dependency.

a30b919

Migrate RC Bandwagoner app to direct FIS dependency.

847cbac

googlebot added the cla: yes Override cla label May 21, 2020

google-oss-bot added the size/L label May 21, 2020

firebase deleted a comment from taiyokun May 21, 2020

danasilver added 4 commits May 21, 2020 11:26

Remove google-services plugin from checked in bandwagoner app.

62fbcb3

Update IID -> FIS in integration test.

4e0fb24

Update docs to reference Installations SDK.

baadaa8

Run formatter.

34e356d

danasilver requested a review from ankitaj224 June 2, 2020 19:36

ankitaj224 reviewed Jun 4, 2020

View reviewed changes

firebase-config/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfig.java Outdated Show resolved Hide resolved

ankitaj224 reviewed Jun 4, 2020

View reviewed changes

firebase-config/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfig.java Outdated Show resolved Hide resolved

ankitaj224 reviewed Jun 5, 2020

View reviewed changes

firebase-config/src/main/java/com/google/firebase/remoteconfig/internal/ConfigFetchHandler.java Show resolved Hide resolved

ankitaj224 requested a review from andirayo June 5, 2020 00:08

egilmorez reviewed Jun 5, 2020

View reviewed changes

Fix installation docs.

512df9d

ankitaj224 approved these changes Jun 5, 2020

View reviewed changes

andirayo approved these changes Jun 9, 2020

View reviewed changes

danasilver added 4 commits June 9, 2020 17:44

Update some variables and comments for FIS.

01b4f73

Update error messages to directly mention Firebase Installations.

0481a94

Update tests with installations

df620df

Update missing auth token test.

5dcf294

danasilver merged commit d4776b3 into master Jun 17, 2020

danasilver deleted the rc-fis-dep branch June 17, 2020 23:07

firebase locked and limited conversation to collaborators Jul 18, 2020

		return fetchFromBackendAndCacheResponse(
		installationId, installationToken, currentTime);

Migrate RC to depend directly on Installations SDK. #1579

Migrate RC to depend directly on Installations SDK. #1579

Uh oh!

Conversation

danasilver commented May 21, 2020

Uh oh!

google-oss-bot commented May 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report

Affected SDKs

firebase-config

Test Logs

Notes

Uh oh!

google-oss-bot commented May 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Binary Size Report

Affected SDKs

firebase-abt

firebase-common

firebase-common-ktx

firebase-components

firebase-config

firebase-config-ktx

firebase-installations

firebase-installations-interop

Test Logs

Notes

Uh oh!

danasilver commented Jun 2, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

egilmorez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andirayo Jun 9, 2020

Choose a reason for hiding this comment

Uh oh!

danasilver Jun 9, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

andirayo Jun 9, 2020

Choose a reason for hiding this comment

Uh oh!

danasilver Jun 9, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

danasilver commented Jun 17, 2020

Uh oh!

Uh oh!

google-oss-bot commented May 21, 2020 •

edited

Loading

`firebase-config`

google-oss-bot commented May 21, 2020 •

edited

Loading

`firebase-abt`

`firebase-common`

`firebase-common-ktx`

`firebase-components`

`firebase-config`

`firebase-config-ktx`

`firebase-installations`

`firebase-installations-interop`