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.

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

Conversation

qdpham13
Copy link
Contributor

@qdpham13 qdpham13 commented Oct 6, 2022

Parse NO_CHANGE fetch response for template version. As part of go/realtime-rc-forward-compatibility

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Oct 6, 2022

Coverage Report 1

Affected Products

  • firebase-config

    Overall coverage changed from ? (6243355) to 80.00% (b0415e7) by ?.

    30 individual files with coverage change

    FilenameBase (6243355)Merge (b0415e7)Diff
    Code.java?0.00%?
    ConfigAutoFetch.java?88.24%?
    ConfigCacheClient.java?93.33%?
    ConfigContainer.java?93.06%?
    ConfigFetchHandler.java?96.69%?
    ConfigFetchHttpClient.java?86.21%?
    ConfigGetParameterHandler.java?96.45%?
    ConfigMetadataClient.java?90.63%?
    ConfigRealtimeHandler.java?21.57%?
    ConfigRealtimeHttpClient.java?31.58%?
    ConfigStorageClient.java?100.00%?
    ConfigUpdateListener.java?0.00%?
    ConfigUpdateListenerRegistration.java?0.00%?
    DefaultsXmlParser.java?0.00%?
    FirebaseRemoteConfig.java?87.83%?
    FirebaseRemoteConfigClientException.java?100.00%?
    FirebaseRemoteConfigException.java?100.00%?
    FirebaseRemoteConfigFetchThrottledException.java?100.00%?
    FirebaseRemoteConfigInfo.java?0.00%?
    FirebaseRemoteConfigInfoImpl.java?100.00%?
    FirebaseRemoteConfigRealtimeUpdateFetchException.java?100.00%?
    FirebaseRemoteConfigRealtimeUpdateStreamException.java?50.00%?
    FirebaseRemoteConfigServerException.java?100.00%?
    FirebaseRemoteConfigSettings.java?61.54%?
    FirebaseRemoteConfigValue.java?0.00%?
    FirebaseRemoteConfigValueImpl.java?84.62%?
    Personalization.java?91.43%?
    RemoteConfigComponent.java?84.71%?
    RemoteConfigConstants.java?0.00%?
    RemoteConfigRegistrar.java?100.00%?

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/W5HXjKiLEp.html

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Oct 6, 2022

Size Report 1

Affected Products

  • firebase-config

    TypeBase (6243355)Merge (e1dc03c)Diff
    aar83.1 kB83.1 kB+25 B (+0.0%)
    apk (aggressive)119 kB119 kB-16 B (-0.0%)
    apk (release)743 kB743 kB+40 B (+0.0%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/hbA8r1VW4z.html

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2022

Unit Test Results

1 225 tests  ±0   1 225 ✔️ ±0   1m 48s ⏱️ -10s
     62 suites ±0          0 💤 ±0 
     62 files   ±0          0 ±0 

Results for commit 7c58599. ± Comparison against base commit 6243355.

♻️ This comment has been updated with latest results.

@google-oss-bot
Copy link
Contributor

1 Warning
⚠️ Did you forget to add a changelog entry? (Add the 'no-changelog' label to the PR to silence this warning.)

Generated by 🚫 Danger

@qdpham13 qdpham13 requested a review from danasilver October 14, 2022 19:20
@qdpham13 qdpham13 changed the title Parse and store fetch response for NO_CHANGE bug as well Parse and store fetch template version for NO_CHANGE response Oct 19, 2022
@qdpham13 qdpham13 changed the title Parse and store fetch template version for NO_CHANGE response Parse NO_CHANGE fetch response for template version Oct 24, 2022
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)

@@ -167,7 +167,7 @@ public void fetch_noChange_responseNotSet() throws Exception {
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?

@qdpham13 qdpham13 merged commit 290a487 into realtime-rc-merge Oct 25, 2022
@qdpham13 qdpham13 deleted the realtime-rc-forward-comp branch October 25, 2022 20:19
@firebase firebase locked and limited conversation to collaborators Nov 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants