Skip to content

Commit 166cb9c

Browse files
qdpham13qdpham13
authored
remove NO_CHANGE bug fixes (#4181)
* remove NO_CHANGE bug fixes * Fix test
1 parent dffa923 commit 166cb9c

File tree

6 files changed

+28
-160
lines changed

6 files changed

+28
-160
lines changed

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -189,14 +189,7 @@ public void run() {
189189

190190
@VisibleForTesting
191191
public synchronized void fetchLatestConfig(int remainingAttempts, long targetVersion) {
192-
boolean excludeEtagHeaderForRealtime = configFetchHandler.getTemplateVersionNumber() == 0;
193-
194-
Task<ConfigFetchHandler.FetchResponse> fetchTask;
195-
if (excludeEtagHeaderForRealtime) {
196-
fetchTask = configFetchHandler.fetchWithoutEtag(0L);
197-
} else {
198-
fetchTask = configFetchHandler.fetch(0L);
199-
}
192+
Task<ConfigFetchHandler.FetchResponse> fetchTask = configFetchHandler.fetch(0L);
200193
fetchTask.onSuccessTask(
201194
(fetchResponse) -> {
202195
long newTemplateVersion = 0;

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

Lines changed: 7 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -162,53 +162,7 @@ public Task<FetchResponse> fetch(long minimumFetchIntervalInSeconds) {
162162
executor,
163163
(cachedFetchConfigsTask) ->
164164
fetchIfCacheExpiredAndNotThrottled(
165-
cachedFetchConfigsTask,
166-
minimumFetchIntervalInSeconds, /*excludeEtagHeader */
167-
false));
168-
}
169-
170-
/**
171-
* Starts fetching configs from the Firebase Remote Config server without the Etag header in the
172-
* request.
173-
*
174-
* <p>Guarantees consistency between memory and disk; fetched configs are saved to memory only
175-
* after they have been written to disk.
176-
*
177-
* <p>Fetches even if the read of the fetch cache fails (assumes there are no cached fetched
178-
* configs in that case).
179-
*
180-
* <p>Fetch request does not include Etag in the header. This is to force a fetch response with an
181-
* UPDATE status.
182-
*
183-
* <p>If the fetch request could not be created or there was error connecting to the server, the
184-
* returned Task throws a {@link FirebaseRemoteConfigClientException}.
185-
*
186-
* <p>If the server responds with an error, the returned Task throws a {@link
187-
* FirebaseRemoteConfigServerException}.
188-
*
189-
* <p>If any of the following is true, then the returned Task throws a {@link
190-
* FirebaseRemoteConfigFetchThrottledException}:
191-
*
192-
* <ul>
193-
* <li>The backoff duration from a previous throttled exception has not expired,
194-
* <li>The backend responded with a throttled error, or
195-
* <li>The backend responded with unavailable errors for the last two fetch requests.
196-
* </ul>
197-
*
198-
* @return A {@link Task} representing the fetch call that returns a {@link FetchResponse} with
199-
* the configs fetched from the backend. If the backend was not called or the backend had no
200-
* updates, the {@link FetchResponse}'s configs will be {@code null}.
201-
*/
202-
public Task<FetchResponse> fetchWithoutEtag(long minimumFetchIntervalInSeconds) {
203-
return fetchedConfigsCache
204-
.get()
205-
.continueWithTask(
206-
executor,
207-
(cachedFetchConfigsTask) ->
208-
fetchIfCacheExpiredAndNotThrottled(
209-
cachedFetchConfigsTask,
210-
minimumFetchIntervalInSeconds, /*excludeEtagHeader */
211-
true));
165+
cachedFetchConfigsTask, minimumFetchIntervalInSeconds));
212166
}
213167

214168
/**
@@ -219,9 +173,7 @@ public Task<FetchResponse> fetchWithoutEtag(long minimumFetchIntervalInSeconds)
219173
* fetch time and {@link BackoffMetadata} in {@link ConfigMetadataClient}.
220174
*/
221175
private Task<FetchResponse> fetchIfCacheExpiredAndNotThrottled(
222-
Task<ConfigContainer> cachedFetchConfigsTask,
223-
long minimumFetchIntervalInSeconds,
224-
boolean excludeEtagHeader) {
176+
Task<ConfigContainer> cachedFetchConfigsTask, long minimumFetchIntervalInSeconds) {
225177
Date currentTime = new Date(clock.currentTimeMillis());
226178
if (cachedFetchConfigsTask.isSuccessful()
227179
&& areCachedFetchConfigsValid(minimumFetchIntervalInSeconds, currentTime)) {
@@ -266,7 +218,7 @@ && areCachedFetchConfigsValid(minimumFetchIntervalInSeconds, currentTime)) {
266218
String installationId = installationIdTask.getResult();
267219
String installationToken = installationAuthTokenTask.getResult().getToken();
268220
return fetchFromBackendAndCacheResponse(
269-
installationId, installationToken, currentTime, excludeEtagHeader);
221+
installationId, installationToken, currentTime);
270222
});
271223
}
272224

@@ -326,10 +278,9 @@ private String createThrottledMessage(long throttledDurationInMillis) {
326278
* {@code fetchedConfigsCache}.
327279
*/
328280
private Task<FetchResponse> fetchFromBackendAndCacheResponse(
329-
String installationId, String installationToken, Date fetchTime, boolean excludeEtagHeader) {
281+
String installationId, String installationToken, Date fetchTime) {
330282
try {
331-
FetchResponse fetchResponse =
332-
fetchFromBackend(installationId, installationToken, fetchTime, excludeEtagHeader);
283+
FetchResponse fetchResponse = fetchFromBackend(installationId, installationToken, fetchTime);
333284
if (fetchResponse.getStatus() != Status.BACKEND_UPDATES_FETCHED) {
334285
return Tasks.forResult(fetchResponse);
335286
}
@@ -352,7 +303,7 @@ private Task<FetchResponse> fetchFromBackendAndCacheResponse(
352303
*/
353304
@WorkerThread
354305
private FetchResponse fetchFromBackend(
355-
String installationId, String installationToken, Date currentTime, boolean excludeEtagHeader)
306+
String installationId, String installationToken, Date currentTime)
356307
throws FirebaseRemoteConfigException {
357308
try {
358309
HttpURLConnection urlConnection = frcBackendApiClient.createHttpURLConnection();
@@ -366,8 +317,7 @@ private FetchResponse fetchFromBackend(
366317
frcMetadata.getLastFetchETag(),
367318
customHttpHeaders,
368319
getFirstOpenTime(),
369-
currentTime,
370-
excludeEtagHeader);
320+
currentTime);
371321

372322
if (response.getFetchedConfigs() != null) {
373323
// Set template version in metadata to be saved on disk.

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

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -182,11 +182,9 @@ FetchResponse fetch(
182182
String lastFetchETag,
183183
Map<String, String> customHeaders,
184184
Long firstOpenTime,
185-
Date currentTime,
186-
Boolean excludeEtagHeader)
185+
Date currentTime)
187186
throws FirebaseRemoteConfigException {
188-
setUpUrlConnection(
189-
urlConnection, lastFetchETag, installationAuthToken, customHeaders, excludeEtagHeader);
187+
setUpUrlConnection(urlConnection, lastFetchETag, installationAuthToken, customHeaders);
190188

191189
String fetchResponseETag;
192190
JSONObject fetchResponse;
@@ -231,19 +229,14 @@ private void setUpUrlConnection(
231229
HttpURLConnection urlConnection,
232230
String lastFetchEtag,
233231
String installationAuthToken,
234-
Map<String, String> customHeaders,
235-
boolean excludeEtagHeader) {
232+
Map<String, String> customHeaders) {
236233
urlConnection.setDoOutput(true);
237234
urlConnection.setConnectTimeout((int) SECONDS.toMillis(connectTimeoutInSeconds));
238235
urlConnection.setReadTimeout((int) SECONDS.toMillis(readTimeoutInSeconds));
239236

240-
// Flag to indicate whether or not to add Etag header.
241-
// Should only be false for Realtime Fetchs' when the stored templateVersion is 0.
242-
if (!excludeEtagHeader) {
243-
// Send the last successful Fetch ETag to the FRC Server to calculate if there has been any
244-
// change in the Fetch Response since the last fetch call.
245-
urlConnection.setRequestProperty(IF_NONE_MATCH_HEADER, lastFetchEtag);
246-
}
237+
// Send the last successful Fetch ETag to the FRC Server to calculate if there has been any
238+
// change in the Fetch Response since the last fetch call.
239+
urlConnection.setRequestProperty(IF_NONE_MATCH_HEADER, lastFetchEtag);
247240

248241
setCommonRequestHeaders(urlConnection, installationAuthToken);
249242
setCustomRequestHeaders(urlConnection, customHeaders);

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

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1221,23 +1221,6 @@ public void realtime_stream_listen_and_failsafe_disabled() throws Exception {
12211221
verify(mockFetchHandler).getTemplateVersionNumber();
12221222
}
12231223

1224-
@Test
1225-
public void realtimeFetch_defaultTemplateVersion_excludeFetchEtag() throws Exception {
1226-
when(mockHttpURLConnection.getResponseCode()).thenReturn(200);
1227-
when(mockHttpURLConnection.getInputStream())
1228-
.thenReturn(
1229-
new ByteArrayInputStream(
1230-
"{ \"featureDisabled\": false, \"latestTemplateVersionNumber\": 2 }"
1231-
.getBytes(StandardCharsets.UTF_8)));
1232-
when(mockFetchHandler.getTemplateVersionNumber()).thenReturn(0L);
1233-
when(mockFetchHandler.fetchWithoutEtag(0L))
1234-
.thenReturn(Tasks.forResult(realtimeFetchedContainerResponse));
1235-
configAutoFetch.fetchLatestConfig(0, 1);
1236-
1237-
verify(mockListener).onEvent();
1238-
verify(mockFetchHandler).fetchWithoutEtag(0L);
1239-
}
1240-
12411224
@Test
12421225
public void realtime_stream_listen_get_inputstream_fail() throws Exception {
12431226
when(mockHttpURLConnection.getResponseCode()).thenReturn(200);

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

Lines changed: 9 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -198,8 +198,7 @@ public void fetch_firstFetch_includesInstallationAuthToken() throws Exception {
198198
/* lastFetchETag= */ any(),
199199
/* customHeaders= */ any(),
200200
/* firstOpenTime= */ any(),
201-
/* currentTime= */ any(),
202-
/* excludeEtagHeader */ any());
201+
/* currentTime= */ any());
203202
}
204203

205204
@Test
@@ -398,8 +397,7 @@ public void fetch_gettingFetchCacheFails_doesNotThrowException() throws Exceptio
398397

399398
@Test
400399
public void fetch_fetchBackendCallFails_taskThrowsException() throws Exception {
401-
when(mockBackendFetchApiClient.fetch(
402-
any(), any(), any(), any(), any(), any(), any(), any(), any()))
400+
when(mockBackendFetchApiClient.fetch(any(), any(), any(), any(), any(), any(), any(), any()))
403401
.thenThrow(
404402
new FirebaseRemoteConfigClientException("Fetch failed due to an unexpected error."));
405403

@@ -738,17 +736,6 @@ public void getInfo_hitsThrottleLimit_throttledStatus() throws Exception {
738736
.isEqualTo(firstFetchedContainer.getFetchTime());
739737
}
740738

741-
@Test
742-
public void fetchWithoutEtag_andSuccess() throws Exception {
743-
fetchCallToHttpClientUpdatesClockAndReturnsConfig(secondFetchedContainer);
744-
745-
fetchHandler.fetchWithoutEtag(0L);
746-
747-
assertThat(metadataClient.getLastFetchStatus()).isEqualTo(LAST_FETCH_STATUS_SUCCESS);
748-
assertThat(metadataClient.getLastSuccessfulFetchTime())
749-
.isEqualTo(secondFetchedContainer.getFetchTime());
750-
}
751-
752739
private ConfigFetchHandler getNewFetchHandler(AnalyticsConnector analyticsConnector) {
753740
ConfigFetchHandler fetchHandler =
754741
spy(
@@ -792,8 +779,7 @@ private void setBackendResponseConfigsTo(ConfigContainer container) throws Excep
792779
/* lastFetchETag= */ any(),
793780
/* customHeaders= */ any(),
794781
/* firstOpenTime= */ any(),
795-
/* currentTime= */ any(),
796-
/* excludeEtagHeader */ any());
782+
/* currentTime= */ any());
797783
}
798784

799785
private void setBackendResponseToNoChange(Date date) throws Exception {
@@ -805,8 +791,7 @@ private void setBackendResponseToNoChange(Date date) throws Exception {
805791
/* lastFetchETag= */ any(),
806792
/* customHeaders= */ any(),
807793
/* firstOpenTime= */ any(),
808-
/* currentTime= */ any(),
809-
/* excludeEtagHeader */ any()))
794+
/* currentTime= */ any()))
810795
.thenReturn(FetchResponse.forBackendHasNoUpdates(date));
811796
}
812797

@@ -821,8 +806,7 @@ private void fetchCallToBackendThrowsException(int httpErrorCode) throws Excepti
821806
/* lastFetchETag= */ any(),
822807
/* customHeaders= */ any(),
823808
/* firstOpenTime= */ any(),
824-
/* currentTime= */ any(),
825-
/* excludeEtagHeader */ any());
809+
/* currentTime= */ any());
826810
}
827811

828812
/**
@@ -902,8 +886,7 @@ private void verifyBackendIsCalled() throws Exception {
902886
/* lastFetchETag= */ any(),
903887
/* customHeaders= */ any(),
904888
/* firstOpenTime= */ any(),
905-
/* currentTime= */ any(),
906-
/* excludeEtagHeader */ any());
889+
/* currentTime= */ any());
907890
}
908891

909892
private void verifyBackendIsCalled(Map<String, String> userProperties, Long firstOpenTime)
@@ -917,8 +900,7 @@ private void verifyBackendIsCalled(Map<String, String> userProperties, Long firs
917900
/* lastFetchETag= */ any(),
918901
/* customHeaders= */ any(),
919902
/* firstOpenTime= */ eq(firstOpenTime),
920-
/* currentTime= */ any(),
921-
/* excludeEtagHeader */ any());
903+
/* currentTime= */ any());
922904
}
923905

924906
private void verifyBackendIsNeverCalled() throws Exception {
@@ -931,8 +913,7 @@ private void verifyBackendIsNeverCalled() throws Exception {
931913
/* lastFetchETag= */ any(),
932914
/* customHeaders= */ any(),
933915
/* firstOpenTime= */ any(),
934-
/* currentTime= */ any(),
935-
/* excludeEtagHeader */ any());
916+
/* currentTime= */ any());
936917
}
937918

938919
private void verifyETags(@Nullable String requestETag, String responseETag) throws Exception {
@@ -945,8 +926,7 @@ private void verifyETags(@Nullable String requestETag, String responseETag) thro
945926
/* lastFetchETag= */ eq(requestETag),
946927
/* customHeaders= */ any(),
947928
/* firstOpenTime= */ any(),
948-
/* currentTime= */ any(),
949-
/* excludeEtagHeader */ any());
929+
/* currentTime= */ any());
950930
assertThat(metadataClient.getLastFetchETag()).isEqualTo(responseETag);
951931
}
952932

‎firebase-config/src/test/java/com/google/firebase/remoteconfig/internal/ConfigFetchHttpClientTest.java

Lines changed: 5 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -336,32 +336,6 @@ public void fetch_serverReturnsException_throwsFirebaseRemoteConfigException() {
336336
assertThat(exception).hasMessageThat().isEqualTo("Bad Request");
337337
}
338338

339-
@Test
340-
public void realtimeFetch_NoTemplateVersion_noEtag() throws Exception {
341-
configFetchHttpClient =
342-
new ConfigFetchHttpClient(
343-
context,
344-
APP_ID,
345-
API_KEY,
346-
DEFAULT_NAMESPACE,
347-
/* connectTimeoutInSeconds= */ 15L,
348-
/* readTimeoutInSeconds= */ 20L);
349-
setServerResponseTo(hasChangeResponseBody, FIRST_ETAG);
350-
351-
FetchResponse response =
352-
configFetchHttpClient.fetch(
353-
fakeHttpURLConnection,
354-
INSTALLATION_ID_STRING,
355-
INSTALLATION_AUTH_TOKEN_STRING,
356-
/* analyticsUserProperties= */ ImmutableMap.of(),
357-
FIRST_ETAG,
358-
/* customHeaders= */ ImmutableMap.of(),
359-
/* firstOpenTime= */ null,
360-
/* currentTime= */ new Date(mockClock.currentTimeMillis()),
361-
/* excludeEtagHeader= */ true);
362-
assertThat(fakeHttpURLConnection.getRequestProperty("If-None-Match")).isNull();
363-
}
364-
365339
private FetchResponse fetch(String eTag) throws Exception {
366340
return configFetchHttpClient.fetch(
367341
fakeHttpURLConnection,
@@ -371,8 +345,7 @@ private FetchResponse fetch(String eTag) throws Exception {
371345
eTag,
372346
/* customHeaders= */ ImmutableMap.of(),
373347
/* firstOpenTime= */ null,
374-
/* currentTime= */ new Date(mockClock.currentTimeMillis()),
375-
/* excludeEtagHeader= */ false);
348+
/* currentTime= */ new Date(mockClock.currentTimeMillis()));
376349
}
377350

378351
private FetchResponse fetch(String eTag, Map<String, String> userProperties, Long firstOpenTime)
@@ -385,8 +358,7 @@ private FetchResponse fetch(String eTag, Map<String, String> userProperties, Lon
385358
eTag,
386359
/* customHeaders= */ ImmutableMap.of(),
387360
firstOpenTime,
388-
new Date(mockClock.currentTimeMillis()),
389-
false);
361+
new Date(mockClock.currentTimeMillis()));
390362
}
391363

392364
private FetchResponse fetch(String eTag, Map<String, String> customHeaders) throws Exception {
@@ -398,8 +370,7 @@ private FetchResponse fetch(String eTag, Map<String, String> customHeaders) thro
398370
eTag,
399371
customHeaders,
400372
/* firstOpenTime= */ null,
401-
new Date(mockClock.currentTimeMillis()),
402-
false);
373+
new Date(mockClock.currentTimeMillis()));
403374
}
404375

405376
private FetchResponse fetchWithoutInstallationId() throws Exception {
@@ -411,8 +382,7 @@ private FetchResponse fetchWithoutInstallationId() throws Exception {
411382
/* lastFetchETag= */ "bogus-etag",
412383
/* customHeaders= */ ImmutableMap.of(),
413384
/* firstOpenTime= */ null,
414-
new Date(mockClock.currentTimeMillis()),
415-
false);
385+
new Date(mockClock.currentTimeMillis()));
416386
}
417387

418388
private FetchResponse fetchWithoutInstallationAuthToken() throws Exception {
@@ -424,8 +394,7 @@ private FetchResponse fetchWithoutInstallationAuthToken() throws Exception {
424394
/* lastFetchETag= */ "bogus-etag",
425395
/* customHeaders= */ ImmutableMap.of(),
426396
/* firstOpenTime= */ null,
427-
new Date(mockClock.currentTimeMillis()),
428-
false);
397+
new Date(mockClock.currentTimeMillis()));
429398
}
430399

431400
private void setServerResponseTo(JSONObject requestBody, String eTag) {

0 commit comments

Comments
 (0)