Skip to content

Commit d300042

Browse files
qdpham13danasilver
qdpham13
authored and
danasilver
committed
remove NO_CHANGE bug fixes (#4181)
* remove NO_CHANGE bug fixes * Fix test
1 parent fd6dec4 commit d300042

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
@@ -1223,23 +1223,6 @@ public void realtime_stream_listen_and_failsafe_disabled() throws Exception {
12231223
verify(mockFetchHandler).getTemplateVersionNumber();
12241224
}
12251225

1226-
@Test
1227-
public void realtimeFetch_defaultTemplateVersion_excludeFetchEtag() throws Exception {
1228-
when(mockHttpURLConnection.getResponseCode()).thenReturn(200);
1229-
when(mockHttpURLConnection.getInputStream())
1230-
.thenReturn(
1231-
new ByteArrayInputStream(
1232-
"{ \"featureDisabled\": false, \"latestTemplateVersionNumber\": 2 }"
1233-
.getBytes(StandardCharsets.UTF_8)));
1234-
when(mockFetchHandler.getTemplateVersionNumber()).thenReturn(0L);
1235-
when(mockFetchHandler.fetchWithoutEtag(0L))
1236-
.thenReturn(Tasks.forResult(realtimeFetchedContainerResponse));
1237-
configAutoFetch.fetchLatestConfig(0, 1);
1238-
1239-
verify(mockListener).onEvent();
1240-
verify(mockFetchHandler).fetchWithoutEtag(0L);
1241-
}
1242-
12431226
@Test
12441227
public void realtime_stream_listen_get_inputstream_fail() throws Exception {
12451228
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
@@ -200,8 +200,7 @@ public void fetch_firstFetch_includesInstallationAuthToken() throws Exception {
200200
/* lastFetchETag= */ any(),
201201
/* customHeaders= */ any(),
202202
/* firstOpenTime= */ any(),
203-
/* currentTime= */ any(),
204-
/* excludeEtagHeader */ any());
203+
/* currentTime= */ any());
205204
}
206205

207206
@Test
@@ -400,8 +399,7 @@ public void fetch_gettingFetchCacheFails_doesNotThrowException() throws Exceptio
400399

401400
@Test
402401
public void fetch_fetchBackendCallFails_taskThrowsException() throws Exception {
403-
when(mockBackendFetchApiClient.fetch(
404-
any(), any(), any(), any(), any(), any(), any(), any(), any()))
402+
when(mockBackendFetchApiClient.fetch(any(), any(), any(), any(), any(), any(), any(), any()))
405403
.thenThrow(
406404
new FirebaseRemoteConfigClientException("Fetch failed due to an unexpected error."));
407405

@@ -740,17 +738,6 @@ public void getInfo_hitsThrottleLimit_throttledStatus() throws Exception {
740738
.isEqualTo(firstFetchedContainer.getFetchTime());
741739
}
742740

743-
@Test
744-
public void fetchWithoutEtag_andSuccess() throws Exception {
745-
fetchCallToHttpClientUpdatesClockAndReturnsConfig(secondFetchedContainer);
746-
747-
fetchHandler.fetchWithoutEtag(0L);
748-
749-
assertThat(metadataClient.getLastFetchStatus()).isEqualTo(LAST_FETCH_STATUS_SUCCESS);
750-
assertThat(metadataClient.getLastSuccessfulFetchTime())
751-
.isEqualTo(secondFetchedContainer.getFetchTime());
752-
}
753-
754741
private ConfigFetchHandler getNewFetchHandler(AnalyticsConnector analyticsConnector) {
755742
ConfigFetchHandler fetchHandler =
756743
spy(
@@ -794,8 +781,7 @@ private void setBackendResponseConfigsTo(ConfigContainer container) throws Excep
794781
/* lastFetchETag= */ any(),
795782
/* customHeaders= */ any(),
796783
/* firstOpenTime= */ any(),
797-
/* currentTime= */ any(),
798-
/* excludeEtagHeader */ any());
784+
/* currentTime= */ any());
799785
}
800786

801787
private void setBackendResponseToNoChange(Date date) throws Exception {
@@ -807,8 +793,7 @@ private void setBackendResponseToNoChange(Date date) throws Exception {
807793
/* lastFetchETag= */ any(),
808794
/* customHeaders= */ any(),
809795
/* firstOpenTime= */ any(),
810-
/* currentTime= */ any(),
811-
/* excludeEtagHeader */ any()))
796+
/* currentTime= */ any()))
812797
.thenReturn(FetchResponse.forBackendHasNoUpdates(date));
813798
}
814799

@@ -823,8 +808,7 @@ private void fetchCallToBackendThrowsException(int httpErrorCode) throws Excepti
823808
/* lastFetchETag= */ any(),
824809
/* customHeaders= */ any(),
825810
/* firstOpenTime= */ any(),
826-
/* currentTime= */ any(),
827-
/* excludeEtagHeader */ any());
811+
/* currentTime= */ any());
828812
}
829813

830814
/**
@@ -904,8 +888,7 @@ private void verifyBackendIsCalled() throws Exception {
904888
/* lastFetchETag= */ any(),
905889
/* customHeaders= */ any(),
906890
/* firstOpenTime= */ any(),
907-
/* currentTime= */ any(),
908-
/* excludeEtagHeader */ any());
891+
/* currentTime= */ any());
909892
}
910893

911894
private void verifyBackendIsCalled(Map<String, String> userProperties, Long firstOpenTime)
@@ -919,8 +902,7 @@ private void verifyBackendIsCalled(Map<String, String> userProperties, Long firs
919902
/* lastFetchETag= */ any(),
920903
/* customHeaders= */ any(),
921904
/* firstOpenTime= */ eq(firstOpenTime),
922-
/* currentTime= */ any(),
923-
/* excludeEtagHeader */ any());
905+
/* currentTime= */ any());
924906
}
925907

926908
private void verifyBackendIsNeverCalled() throws Exception {
@@ -933,8 +915,7 @@ private void verifyBackendIsNeverCalled() throws Exception {
933915
/* lastFetchETag= */ any(),
934916
/* customHeaders= */ any(),
935917
/* firstOpenTime= */ any(),
936-
/* currentTime= */ any(),
937-
/* excludeEtagHeader */ any());
918+
/* currentTime= */ any());
938919
}
939920

940921
private void verifyETags(@Nullable String requestETag, String responseETag) throws Exception {
@@ -947,8 +928,7 @@ private void verifyETags(@Nullable String requestETag, String responseETag) thro
947928
/* lastFetchETag= */ eq(requestETag),
948929
/* customHeaders= */ any(),
949930
/* firstOpenTime= */ any(),
950-
/* currentTime= */ any(),
951-
/* excludeEtagHeader */ any());
931+
/* currentTime= */ any());
952932
assertThat(metadataClient.getLastFetchETag()).isEqualTo(responseETag);
953933
}
954934

‎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)