Skip to content

remove NO_CHANGE bug fixes #4181

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 13, 2022
Merged

remove NO_CHANGE bug fixes #4181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b0860e2
remove NO_CHANGE bug fixes
qdpham13 Oct 6, 2022
977ea6f
Fix test
qdpham13 Oct 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions firebase-config/src/main/java/com/google/firebase/remoteconfig/internal/ConfigAutoFetch.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,14 +189,7 @@ public void run() {

@VisibleForTesting
public synchronized void fetchLatestConfig(int remainingAttempts, long targetVersion) {
boolean excludeEtagHeaderForRealtime = configFetchHandler.getTemplateVersionNumber() == 0;

Task<ConfigFetchHandler.FetchResponse> fetchTask;
if (excludeEtagHeaderForRealtime) {
fetchTask = configFetchHandler.fetchWithoutEtag(0L);
} else {
fetchTask = configFetchHandler.fetch(0L);
}
Task<ConfigFetchHandler.FetchResponse> fetchTask = configFetchHandler.fetch(0L);
fetchTask.onSuccessTask(
(fetchResponse) -> {
long newTemplateVersion = 0;
Expand Down
64 changes: 7 additions & 57 deletions ...se-config/src/main/java/com/google/firebase/remoteconfig/internal/ConfigFetchHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,53 +162,7 @@ public Task<FetchResponse> fetch(long minimumFetchIntervalInSeconds) {
executor,
(cachedFetchConfigsTask) ->
fetchIfCacheExpiredAndNotThrottled(
cachedFetchConfigsTask,
minimumFetchIntervalInSeconds, /*excludeEtagHeader */
false));
}

/**
* Starts fetching configs from the Firebase Remote Config server without the Etag header in the
* request.
*
* <p>Guarantees consistency between memory and disk; fetched configs are saved to memory only
* after they have been written to disk.
*
* <p>Fetches even if the read of the fetch cache fails (assumes there are no cached fetched
* configs in that case).
*
* <p>Fetch request does not include Etag in the header. This is to force a fetch response with an
* UPDATE status.
*
* <p>If the fetch request could not be created or there was error connecting to the server, the
* returned Task throws a {@link FirebaseRemoteConfigClientException}.
*
* <p>If the server responds with an error, the returned Task throws a {@link
* FirebaseRemoteConfigServerException}.
*
* <p>If any of the following is true, then the returned Task throws a {@link
* FirebaseRemoteConfigFetchThrottledException}:
*
* <ul>
* <li>The backoff duration from a previous throttled exception has not expired,
* <li>The backend responded with a throttled error, or
* <li>The backend responded with unavailable errors for the last two fetch requests.
* </ul>
*
* @return A {@link Task} representing the fetch call that returns a {@link FetchResponse} with
* the configs fetched from the backend. If the backend was not called or the backend had no
* updates, the {@link FetchResponse}'s configs will be {@code null}.
*/
public Task<FetchResponse> fetchWithoutEtag(long minimumFetchIntervalInSeconds) {
return fetchedConfigsCache
.get()
.continueWithTask(
executor,
(cachedFetchConfigsTask) ->
fetchIfCacheExpiredAndNotThrottled(
cachedFetchConfigsTask,
minimumFetchIntervalInSeconds, /*excludeEtagHeader */
true));
cachedFetchConfigsTask, minimumFetchIntervalInSeconds));
}

/**
Expand All @@ -219,9 +173,7 @@ public Task<FetchResponse> fetchWithoutEtag(long minimumFetchIntervalInSeconds)
* fetch time and {@link BackoffMetadata} in {@link ConfigMetadataClient}.
*/
private Task<FetchResponse> fetchIfCacheExpiredAndNotThrottled(
Task<ConfigContainer> cachedFetchConfigsTask,
long minimumFetchIntervalInSeconds,
boolean excludeEtagHeader) {
Task<ConfigContainer> cachedFetchConfigsTask, long minimumFetchIntervalInSeconds) {
Date currentTime = new Date(clock.currentTimeMillis());
if (cachedFetchConfigsTask.isSuccessful()
&& areCachedFetchConfigsValid(minimumFetchIntervalInSeconds, currentTime)) {
Expand Down Expand Up @@ -266,7 +218,7 @@ && areCachedFetchConfigsValid(minimumFetchIntervalInSeconds, currentTime)) {
String installationId = installationIdTask.getResult();
String installationToken = installationAuthTokenTask.getResult().getToken();
return fetchFromBackendAndCacheResponse(
installationId, installationToken, currentTime, excludeEtagHeader);
installationId, installationToken, currentTime);
});
}

Expand Down Expand Up @@ -326,10 +278,9 @@ private String createThrottledMessage(long throttledDurationInMillis) {
* {@code fetchedConfigsCache}.
*/
private Task<FetchResponse> fetchFromBackendAndCacheResponse(
String installationId, String installationToken, Date fetchTime, boolean excludeEtagHeader) {
String installationId, String installationToken, Date fetchTime) {
try {
FetchResponse fetchResponse =
fetchFromBackend(installationId, installationToken, fetchTime, excludeEtagHeader);
FetchResponse fetchResponse = fetchFromBackend(installationId, installationToken, fetchTime);
if (fetchResponse.getStatus() != Status.BACKEND_UPDATES_FETCHED) {
return Tasks.forResult(fetchResponse);
}
Expand All @@ -352,7 +303,7 @@ private Task<FetchResponse> fetchFromBackendAndCacheResponse(
*/
@WorkerThread
private FetchResponse fetchFromBackend(
String installationId, String installationToken, Date currentTime, boolean excludeEtagHeader)
String installationId, String installationToken, Date currentTime)
throws FirebaseRemoteConfigException {
try {
HttpURLConnection urlConnection = frcBackendApiClient.createHttpURLConnection();
Expand All @@ -366,8 +317,7 @@ private FetchResponse fetchFromBackend(
frcMetadata.getLastFetchETag(),
customHttpHeaders,
getFirstOpenTime(),
currentTime,
excludeEtagHeader);
currentTime);

if (response.getFetchedConfigs() != null) {
// Set template version in metadata to be saved on disk.
Expand Down
19 changes: 6 additions & 13 deletions ...config/src/main/java/com/google/firebase/remoteconfig/internal/ConfigFetchHttpClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,11 +182,9 @@ FetchResponse fetch(
String lastFetchETag,
Map<String, String> customHeaders,
Long firstOpenTime,
Date currentTime,
Boolean excludeEtagHeader)
Date currentTime)
throws FirebaseRemoteConfigException {
setUpUrlConnection(
urlConnection, lastFetchETag, installationAuthToken, customHeaders, excludeEtagHeader);
setUpUrlConnection(urlConnection, lastFetchETag, installationAuthToken, customHeaders);

String fetchResponseETag;
JSONObject fetchResponse;
Expand Down Expand Up @@ -231,19 +229,14 @@ private void setUpUrlConnection(
HttpURLConnection urlConnection,
String lastFetchEtag,
String installationAuthToken,
Map<String, String> customHeaders,
boolean excludeEtagHeader) {
Map<String, String> customHeaders) {
urlConnection.setDoOutput(true);
urlConnection.setConnectTimeout((int) SECONDS.toMillis(connectTimeoutInSeconds));
urlConnection.setReadTimeout((int) SECONDS.toMillis(readTimeoutInSeconds));

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

setCommonRequestHeaders(urlConnection, installationAuthToken);
setCustomRequestHeaders(urlConnection, customHeaders);
Expand Down
17 changes: 0 additions & 17 deletions firebase-config/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1221,23 +1221,6 @@ public void realtime_stream_listen_and_failsafe_disabled() throws Exception {
verify(mockFetchHandler).getTemplateVersionNumber();
}

@Test
public void realtimeFetch_defaultTemplateVersion_excludeFetchEtag() throws Exception {
when(mockHttpURLConnection.getResponseCode()).thenReturn(200);
when(mockHttpURLConnection.getInputStream())
.thenReturn(
new ByteArrayInputStream(
"{ \"featureDisabled\": false, \"latestTemplateVersionNumber\": 2 }"
.getBytes(StandardCharsets.UTF_8)));
when(mockFetchHandler.getTemplateVersionNumber()).thenReturn(0L);
when(mockFetchHandler.fetchWithoutEtag(0L))
.thenReturn(Tasks.forResult(realtimeFetchedContainerResponse));
configAutoFetch.fetchLatestConfig(0, 1);

verify(mockListener).onEvent();
verify(mockFetchHandler).fetchWithoutEtag(0L);
}

@Test
public void realtime_stream_listen_get_inputstream_fail() throws Exception {
when(mockHttpURLConnection.getResponseCode()).thenReturn(200);
Expand Down
38 changes: 9 additions & 29 deletions ...onfig/src/test/java/com/google/firebase/remoteconfig/internal/ConfigFetchHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,7 @@ public void fetch_firstFetch_includesInstallationAuthToken() throws Exception {
/* lastFetchETag= */ any(),
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any(),
/* excludeEtagHeader */ any());
/* currentTime= */ any());
}

@Test
Expand Down Expand Up @@ -398,8 +397,7 @@ public void fetch_gettingFetchCacheFails_doesNotThrowException() throws Exceptio

@Test
public void fetch_fetchBackendCallFails_taskThrowsException() throws Exception {
when(mockBackendFetchApiClient.fetch(
any(), any(), any(), any(), any(), any(), any(), any(), any()))
when(mockBackendFetchApiClient.fetch(any(), any(), any(), any(), any(), any(), any(), any()))
.thenThrow(
new FirebaseRemoteConfigClientException("Fetch failed due to an unexpected error."));

Expand Down Expand Up @@ -738,17 +736,6 @@ public void getInfo_hitsThrottleLimit_throttledStatus() throws Exception {
.isEqualTo(firstFetchedContainer.getFetchTime());
}

@Test
public void fetchWithoutEtag_andSuccess() throws Exception {
fetchCallToHttpClientUpdatesClockAndReturnsConfig(secondFetchedContainer);

fetchHandler.fetchWithoutEtag(0L);

assertThat(metadataClient.getLastFetchStatus()).isEqualTo(LAST_FETCH_STATUS_SUCCESS);
assertThat(metadataClient.getLastSuccessfulFetchTime())
.isEqualTo(secondFetchedContainer.getFetchTime());
}

private ConfigFetchHandler getNewFetchHandler(AnalyticsConnector analyticsConnector) {
ConfigFetchHandler fetchHandler =
spy(
Expand Down Expand Up @@ -792,8 +779,7 @@ private void setBackendResponseConfigsTo(ConfigContainer container) throws Excep
/* lastFetchETag= */ any(),
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any(),
/* excludeEtagHeader */ any());
/* currentTime= */ any());
}

private void setBackendResponseToNoChange(Date date) throws Exception {
Expand All @@ -805,8 +791,7 @@ private void setBackendResponseToNoChange(Date date) throws Exception {
/* lastFetchETag= */ any(),
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any(),
/* excludeEtagHeader */ any()))
/* currentTime= */ any()))
.thenReturn(FetchResponse.forBackendHasNoUpdates(date));
}

Expand All @@ -821,8 +806,7 @@ private void fetchCallToBackendThrowsException(int httpErrorCode) throws Excepti
/* lastFetchETag= */ any(),
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any(),
/* excludeEtagHeader */ any());
/* currentTime= */ any());
}

/**
Expand Down Expand Up @@ -902,8 +886,7 @@ private void verifyBackendIsCalled() throws Exception {
/* lastFetchETag= */ any(),
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any(),
/* excludeEtagHeader */ any());
/* currentTime= */ any());
}

private void verifyBackendIsCalled(Map<String, String> userProperties, Long firstOpenTime)
Expand All @@ -917,8 +900,7 @@ private void verifyBackendIsCalled(Map<String, String> userProperties, Long firs
/* lastFetchETag= */ any(),
/* customHeaders= */ any(),
/* firstOpenTime= */ eq(firstOpenTime),
/* currentTime= */ any(),
/* excludeEtagHeader */ any());
/* currentTime= */ any());
}

private void verifyBackendIsNeverCalled() throws Exception {
Expand All @@ -931,8 +913,7 @@ private void verifyBackendIsNeverCalled() throws Exception {
/* lastFetchETag= */ any(),
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any(),
/* excludeEtagHeader */ any());
/* currentTime= */ any());
}

private void verifyETags(@Nullable String requestETag, String responseETag) throws Exception {
Expand All @@ -945,8 +926,7 @@ private void verifyETags(@Nullable String requestETag, String responseETag) thro
/* lastFetchETag= */ eq(requestETag),
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any(),
/* excludeEtagHeader */ any());
/* currentTime= */ any());
assertThat(metadataClient.getLastFetchETag()).isEqualTo(responseETag);
}

Expand Down
41 changes: 5 additions & 36 deletions ...ig/src/test/java/com/google/firebase/remoteconfig/internal/ConfigFetchHttpClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,32 +336,6 @@ public void fetch_serverReturnsException_throwsFirebaseRemoteConfigException() {
assertThat(exception).hasMessageThat().isEqualTo("Bad Request");
}

@Test
public void realtimeFetch_NoTemplateVersion_noEtag() throws Exception {
configFetchHttpClient =
new ConfigFetchHttpClient(
context,
APP_ID,
API_KEY,
DEFAULT_NAMESPACE,
/* connectTimeoutInSeconds= */ 15L,
/* readTimeoutInSeconds= */ 20L);
setServerResponseTo(hasChangeResponseBody, FIRST_ETAG);

FetchResponse response =
configFetchHttpClient.fetch(
fakeHttpURLConnection,
INSTALLATION_ID_STRING,
INSTALLATION_AUTH_TOKEN_STRING,
/* analyticsUserProperties= */ ImmutableMap.of(),
FIRST_ETAG,
/* customHeaders= */ ImmutableMap.of(),
/* firstOpenTime= */ null,
/* currentTime= */ new Date(mockClock.currentTimeMillis()),
/* excludeEtagHeader= */ true);
assertThat(fakeHttpURLConnection.getRequestProperty("If-None-Match")).isNull();
}

private FetchResponse fetch(String eTag) throws Exception {
return configFetchHttpClient.fetch(
fakeHttpURLConnection,
Expand All @@ -371,8 +345,7 @@ private FetchResponse fetch(String eTag) throws Exception {
eTag,
/* customHeaders= */ ImmutableMap.of(),
/* firstOpenTime= */ null,
/* currentTime= */ new Date(mockClock.currentTimeMillis()),
/* excludeEtagHeader= */ false);
/* currentTime= */ new Date(mockClock.currentTimeMillis()));
}

private FetchResponse fetch(String eTag, Map<String, String> userProperties, Long firstOpenTime)
Expand All @@ -385,8 +358,7 @@ private FetchResponse fetch(String eTag, Map<String, String> userProperties, Lon
eTag,
/* customHeaders= */ ImmutableMap.of(),
firstOpenTime,
new Date(mockClock.currentTimeMillis()),
false);
new Date(mockClock.currentTimeMillis()));
}

private FetchResponse fetch(String eTag, Map<String, String> customHeaders) throws Exception {
Expand All @@ -398,8 +370,7 @@ private FetchResponse fetch(String eTag, Map<String, String> customHeaders) thro
eTag,
customHeaders,
/* firstOpenTime= */ null,
new Date(mockClock.currentTimeMillis()),
false);
new Date(mockClock.currentTimeMillis()));
}

private FetchResponse fetchWithoutInstallationId() throws Exception {
Expand All @@ -411,8 +382,7 @@ private FetchResponse fetchWithoutInstallationId() throws Exception {
/* lastFetchETag= */ "bogus-etag",
/* customHeaders= */ ImmutableMap.of(),
/* firstOpenTime= */ null,
new Date(mockClock.currentTimeMillis()),
false);
new Date(mockClock.currentTimeMillis()));
}

private FetchResponse fetchWithoutInstallationAuthToken() throws Exception {
Expand All @@ -424,8 +394,7 @@ private FetchResponse fetchWithoutInstallationAuthToken() throws Exception {
/* lastFetchETag= */ "bogus-etag",
/* customHeaders= */ ImmutableMap.of(),
/* firstOpenTime= */ null,
new Date(mockClock.currentTimeMillis()),
false);
new Date(mockClock.currentTimeMillis()));
}

private void setServerResponseTo(JSONObject requestBody, String eTag) {
Expand Down