Skip to content

Parse NO_CHANGE fetch response for template version #4177

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 9 commits into from
Oct 25, 2022
Merged

Parse NO_CHANGE fetch response for template version #4177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d879dea
Parse and store fetch response for NO_CHANGE bug as well
qdpham13 Oct 6, 2022
bcc3fe3
Fix test
qdpham13 Oct 6, 2022
fe89cbd
Revert change
qdpham13 Oct 11, 2022
9993bfa
Format file
qdpham13 Oct 11, 2022
1a08ab5
Merge in Realtime rc master branch
qdpham13 Oct 13, 2022
9c64716
Format file
qdpham13 Oct 13, 2022
0af0de3
Merge branch 'realtime-rc-merge' into realtime-rc-forward-comp
qdpham13 Oct 13, 2022
6c2aaa4
Merge in master branch
qdpham13 Oct 19, 2022
7c58599
Add comments and update tests
qdpham13 Oct 25, 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
7 changes: 5 additions & 2 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 @@ -575,11 +575,14 @@ public static FetchResponse forBackendUpdatesFetched(
lastFetchETag);
}

public static FetchResponse forBackendHasNoUpdates(Date fetchTime) {
// Passing in param fetchedConfigs because it contains templateVersion but no other active
// fields.
public static FetchResponse forBackendHasNoUpdates(
Date fetchTime, ConfigContainer fetchedConfigs) {
return new FetchResponse(
fetchTime,
Status.BACKEND_HAS_NO_UPDATES,
/*fetchedConfigs=*/ null,
/*fetchedConfigs=*/ fetchedConfigs,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might add a comment that we're including the fetchConfigs because they include the template version (even though they don't include the config entries or metadata)

/*lastFetchETag=*/ null);
}

Expand Down
4 changes: 2 additions & 2 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 @@ -216,12 +216,12 @@ FetchResponse fetch(
} catch (IOException e) {
}
}
ConfigContainer fetchedConfigs = extractConfigs(fetchResponse, currentTime);

if (!backendHasUpdates(fetchResponse)) {
return FetchResponse.forBackendHasNoUpdates(currentTime);
return FetchResponse.forBackendHasNoUpdates(currentTime, fetchedConfigs);
}

ConfigContainer fetchedConfigs = extractConfigs(fetchResponse, currentTime);
return FetchResponse.forBackendUpdatesFetched(fetchedConfigs, fetchResponseETag);
}

Expand Down
2 changes: 1 addition & 1 deletion ...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 @@ -794,7 +794,7 @@ private void setBackendResponseToNoChange(Date date) throws Exception {
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any()))
.thenReturn(FetchResponse.forBackendHasNoUpdates(date));
.thenReturn(FetchResponse.forBackendHasNoUpdates(date, firstFetchedContainer));
}

private void fetchCallToBackendThrowsException(int httpErrorCode) throws Exception {
Expand Down
6 changes: 4 additions & 2 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 @@ -161,13 +161,15 @@ public void fetch_newValues_responseSet() throws Exception {
}

@Test
public void fetch_noChange_responseNotSet() throws Exception {
public void fetch_noChange_responseOnlyContainsTemplateVersion() throws Exception {
setServerResponseTo(noChangeResponseBody, SECOND_ETAG);

FetchResponse response = fetch(SECOND_ETAG);

assertThat(response.getLastFetchETag()).isNull();
assertThat(response.getFetchedConfigs()).isNull();
assertThat(response.getFetchedConfigs()).isNotNull();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe assert it only contains template version?

assertThat(response.getFetchedConfigs().getTemplateVersionNumber()).isNotNull();
assertThat(response.getFetchedConfigs().getConfigs().length()).isEqualTo(0);
}

@Test
Expand Down