Skip to content

Commit dfecfc6

Browse files
author
Varun Rathore
committed
Resolve comment related to revert of ServerVersion Class
1 parent 8932e7e commit dfecfc6

File tree

6 files changed

+91
-54
lines changed

6 files changed

+91
-54
lines changed

src/main/java/com/google/firebase/remoteconfig/ServerTemplateData.java

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ final class ServerTemplateData {
4040
private Map<String, Parameter> parameters;
4141
private List<ServerCondition> serverConditions;
4242
private Map<String, ParameterGroup> parameterGroups;
43-
private ServerVersion serverVersion;
43+
private Version version;
4444

4545

4646
ServerTemplateData(String etag) {
@@ -78,7 +78,7 @@ final class ServerTemplateData {
7878
}
7979
}
8080
if (serverTemplateResponse.getVersion() != null) {
81-
this.serverVersion = new ServerVersion(serverTemplateResponse.getVersion());
81+
this.version = new Version(serverTemplateResponse.getVersion());
8282
}
8383
this.etag = serverTemplateResponse.getEtag();
8484
}
@@ -120,8 +120,8 @@ Map<String, ParameterGroup> getParameterGroups() {
120120
return parameterGroups;
121121
}
122122

123-
ServerVersion getVersion() {
124-
return serverVersion;
123+
Version getVersion() {
124+
return version;
125125
}
126126

127127
ServerTemplateData setParameters(@NonNull Map<String, Parameter> parameters) {
@@ -144,8 +144,8 @@ ServerTemplateData setParameterGroups(
144144
return this;
145145
}
146146

147-
ServerTemplateData setVersion(ServerVersion serverVersion) {
148-
this.serverVersion = serverVersion;
147+
ServerTemplateData setVersion(Version serverVersion) {
148+
this.version = serverVersion;
149149
return this;
150150
}
151151

@@ -178,7 +178,7 @@ ServerTemplateResponse toServerTemplateResponse(boolean includeAll) {
178178
parameterGroupResponse.put(entry.getKey(), entry.getValue().toParameterGroupResponse());
179179
}
180180
TemplateResponse.VersionResponse versionResponse =
181-
(this.serverVersion == null) ? null : this.serverVersion.toVersionResponse(includeAll);
181+
(this.version == null) ? null : this.version.toVersionResponse(includeAll);
182182
ServerTemplateResponse serverTemplateResponse =
183183
new ServerTemplateResponse()
184184
.setParameters(parameterResponses)
@@ -204,12 +204,12 @@ public boolean equals(Object o) {
204204
&& Objects.equals(parameters, template.parameters)
205205
&& Objects.equals(serverConditions, template.serverConditions)
206206
&& Objects.equals(parameterGroups, template.parameterGroups)
207-
&& Objects.equals(serverVersion, template.serverVersion);
207+
&& Objects.equals(version, template.version);
208208
}
209209

210210
@Override
211211
public int hashCode() {
212-
return Objects.hash(etag, parameters, serverConditions, parameterGroups, serverVersion);
212+
return Objects.hash(etag, parameters, serverConditions, parameterGroups, version);
213213
}
214214
}
215215

src/main/java/com/google/firebase/remoteconfig/ServerTemplateImpl.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@
2222
import com.google.firebase.ErrorCode;
2323
import com.google.firebase.internal.Nullable;
2424
import com.google.firebase.remoteconfig.internal.TemplateResponse.ParameterValueResponse;
25-
import com.google.gson.Gson;
26-
import com.google.gson.GsonBuilder;
2725

2826
import java.util.HashMap;
2927
import java.util.Map;
@@ -130,8 +128,7 @@ public String getCachedTemplate() {
130128

131129
@Override
132130
public String toJson() {
133-
Gson gson = new GsonBuilder().setPrettyPrinting().create();
134-
return gson.toJson(this.cache);
131+
return this.cache.toJSON();
135132
}
136133

137134
private void mergeDerivedConfigValues(ImmutableMap<String, Boolean> evaluatedCondition,

src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImplTest.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1446,7 +1446,7 @@ public void testGetServerTemplate() throws Exception {
14461446
assertEquals(
14471447
convertObjectToString(EXPECTED_SERVER_CONDITIONS),
14481448
convertObjectToString(serverTemplateData.getServerConditions()));
1449-
assertEquals("2020-11-15T06:57:26.342763941Z", serverTemplateData.getVersion().getUpdateTime());
1449+
assertEquals(1605423446000L, serverTemplateData.getVersion().getUpdateTime());
14501450
checkGetRequestHeaderForServer(interceptor.getLastRequest());
14511451
}
14521452

@@ -1470,10 +1470,9 @@ public void testGetServerTemplateWithTimestampUpToNanosecondPrecision() throws E
14701470

14711471
String receivedTemplate = client.getServerTemplate();
14721472
ServerTemplateData serverTemplateData = ServerTemplateData.fromJSON(receivedTemplate);
1473-
14741473
assertEquals(TEST_ETAG, serverTemplateData.getETag());
14751474
assertEquals("17", serverTemplateData.getVersion().getVersionNumber());
1476-
assertEquals(timestamp, serverTemplateData.getVersion().getUpdateTime());
1475+
assertEquals(1605423446000L, serverTemplateData.getVersion().getUpdateTime());
14771476
checkGetRequestHeaderForServer(interceptor.getLastRequest());
14781477
}
14791478
}

src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigTest.java

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
import com.google.firebase.TestOnlyImplFirebaseTrampolines;
2929
import com.google.firebase.auth.MockGoogleCredentials;
3030
import com.google.firebase.remoteconfig.internal.TemplateResponse;
31+
import com.google.gson.JsonElement;
32+
import com.google.gson.JsonParser;
3133
import java.util.concurrent.ExecutionException;
3234
import org.junit.After;
3335
import org.junit.Test;
@@ -42,13 +44,6 @@ public class FirebaseRemoteConfigTest {
4244
.setCredentials(new MockGoogleCredentials("test-token"))
4345
.setProjectId("test-project")
4446
.build();
45-
private static final String TEST_SERVER_TEMPLATE =
46-
"{\n"
47-
+ " \"etag\": \"etag-123456789012-1\",\n"
48-
+ " \"parameters\": {},\n"
49-
+ " \"serverConditions\": [],\n"
50-
+ " \"parameterGroups\": {}\n"
51-
+ "}";
5247
private static final FirebaseRemoteConfigException TEST_EXCEPTION =
5348
new FirebaseRemoteConfigException(ErrorCode.INTERNAL, "Test error message");
5449

@@ -629,14 +624,16 @@ private FirebaseRemoteConfig getRemoteConfig(FirebaseRemoteConfigClient client)
629624

630625
@Test
631626
public void testGetServerTemplate() throws FirebaseRemoteConfigException {
632-
MockRemoteConfigClient client =
633-
MockRemoteConfigClient.fromServerTemplate(
634-
new ServerTemplateData().setETag(TEST_ETAG).toJSON());
627+
MockRemoteConfigClient client = MockRemoteConfigClient.fromServerTemplate(
628+
new ServerTemplateData().setETag(TEST_ETAG).toJSON());
635629
FirebaseRemoteConfig remoteConfig = getRemoteConfig(client);
636630

637631
ServerTemplate template = remoteConfig.getServerTemplate();
638632
String templateData = template.toJson();
639-
assertEquals(TEST_SERVER_TEMPLATE, templateData);
633+
JsonElement expectedJson = JsonParser.parseString(new ServerTemplateData().setETag(TEST_ETAG).toJSON());
634+
JsonElement actualJson = JsonParser.parseString(templateData);
635+
636+
assertEquals(expectedJson, actualJson);
640637
}
641638

642639
@Test
@@ -653,14 +650,16 @@ public void testGetServerTemplateFailure() {
653650

654651
@Test
655652
public void testGetServerTemplateAsync() throws Exception {
656-
MockRemoteConfigClient client =
657-
MockRemoteConfigClient.fromServerTemplate(
658-
new ServerTemplateData().setETag(TEST_ETAG).toJSON());
653+
MockRemoteConfigClient client = MockRemoteConfigClient.fromServerTemplate(
654+
new ServerTemplateData().setETag(TEST_ETAG).toJSON());
659655
FirebaseRemoteConfig remoteConfig = getRemoteConfig(client);
660656

661657
ServerTemplate template = remoteConfig.getServerTemplateAsync().get();
662658
String templateData = template.toJson();
663-
assertEquals(TEST_SERVER_TEMPLATE, templateData);
659+
JsonElement expectedJson = JsonParser.parseString(new ServerTemplateData().setETag(TEST_ETAG).toJSON());
660+
JsonElement actualJson = JsonParser.parseString(templateData);
661+
662+
assertEquals(expectedJson, actualJson);
664663
}
665664

666665
@Test

src/test/java/com/google/firebase/remoteconfig/ServerTemplateImplTest.java

Lines changed: 55 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
import com.google.firebase.testing.TestUtils;
2727
import org.junit.BeforeClass;
2828
import org.junit.Test;
29+
import com.google.gson.JsonElement;
30+
import com.google.gson.JsonParser;
2931

3032
/**
3133
* Tests for {@link ServerTemplateImpl}.
@@ -37,13 +39,6 @@ public class ServerTemplateImplTest {
3739
.setCredentials(new MockGoogleCredentials("test-token"))
3840
.setProjectId("test-project")
3941
.build();
40-
private static final String TEST_SERVER_TEMPLATE =
41-
"{\n"
42-
+ " \"etag\": \"etag-123456789012-1\",\n"
43-
+ " \"parameters\": {},\n"
44-
+ " \"serverConditions\": [],\n"
45-
+ " \"parameterGroups\": {}\n"
46-
+ "}";
4742

4843
private static String cacheTemplate;
4944

@@ -354,27 +349,66 @@ private FirebaseRemoteConfig getRemoteConfig(FirebaseRemoteConfigClient client)
354349
return new FirebaseRemoteConfig(app, client);
355350
}
356351

357-
@Test
358-
public void testLoad() throws Exception {
359-
KeysAndValues defaultConfig =
360-
new KeysAndValues.Builder().put("Unset default config", "abc").build();
352+
@Test
353+
public void testLoad() throws Exception {
354+
// 1. Define the template data that the mock client will return.
355+
// This is the EXPECTED state after `load()` is called.
356+
final String expectedTemplateJsonAfterLoad = new ServerTemplateData().setETag(TEST_ETAG).toJSON();
361357

362-
// Mock the HTTP client to return a predefined response
358+
// 2. Mock the HTTP client to return the predefined response.
363359
MockRemoteConfigClient client =
364-
MockRemoteConfigClient.fromServerTemplate(
365-
new ServerTemplateData().setETag(TEST_ETAG).toJSON());
360+
MockRemoteConfigClient.fromServerTemplate(expectedTemplateJsonAfterLoad);
366361
FirebaseRemoteConfig remoteConfig = getRemoteConfig(client);
367-
ServerTemplate template1 =
362+
363+
// 3. Build the template instance.
364+
// It's initialized with a complex `cacheTemplate` to ensure `load()` properly overwrites it.
365+
KeysAndValues defaultConfig =
366+
new KeysAndValues.Builder().put("Unset default config", "abc").build();
367+
ServerTemplate template =
368368
remoteConfig
369369
.serverTemplateBuilder()
370370
.defaultConfig(defaultConfig)
371+
.cachedTemplate(cacheTemplate) // This is the initial state before `load()`
372+
.build();
373+
374+
// 4. Call the load method, which fetches the new template from the mock client.
375+
ApiFuture<Void> loadFuture = template.load();
376+
loadFuture.get(); // Wait for the async operation to complete.
377+
378+
// 5. Get the ACTUAL state of the template after `load()` has executed.
379+
String actualJsonAfterLoad = template.toJson();
380+
381+
// 6. Assert that the template's state has been updated to match what the mock client returned.
382+
// Parsing to JsonElement performs a deep, order-insensitive comparison.
383+
JsonElement expectedJson = JsonParser.parseString(expectedTemplateJsonAfterLoad);
384+
JsonElement actualJson = JsonParser.parseString(actualJsonAfterLoad);
385+
386+
assertEquals(expectedJson, actualJson);
387+
}
388+
389+
@Test
390+
public void testBuilderParsesCachedTemplateCorrectly() throws FirebaseRemoteConfigException {
391+
// Arrange:
392+
// 1. Create a canonical JSON string by parsing the input file and then
393+
// re-serializing it. This gives us the precise expected output format,
394+
// accounting for any formatting or default value differences.
395+
ServerTemplateData canonicalData = ServerTemplateData.fromJSON(cacheTemplate);
396+
String expectedJsonString = canonicalData.toJSON();
397+
398+
// Act:
399+
// 2. Build a ServerTemplate instance from the original cached JSON string,
400+
// which triggers the parsing logic we want to test.
401+
ServerTemplate template = new ServerTemplateImpl.Builder(null)
371402
.cachedTemplate(cacheTemplate)
372403
.build();
373404

374-
// Call the load method
375-
ApiFuture<Void> loadFuture = template1.load();
376-
loadFuture.get();
377-
String cachedTemplate = template1.toJson();
378-
assertEquals(TEST_SERVER_TEMPLATE, cachedTemplate);
379-
}
405+
// Assert:
406+
// 3. Compare the JSON from the newly built template against the canonical version.
407+
// This verifies that the internal state was parsed and stored correctly.
408+
// Using JsonElement ensures the comparison is not affected by key order.
409+
JsonElement expectedJsonTree = JsonParser.parseString(expectedJsonString);
410+
JsonElement actualJsonTree = JsonParser.parseString(template.toJson());
411+
412+
assertEquals(expectedJsonTree, actualJsonTree);
413+
}
380414
}

src/test/resources/getServerTemplateData.json

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,15 @@
167167
}
168168
},
169169
"version": {
170-
"versionNumber": "27",
171-
"isLegacy": true
170+
"versionNumber": "17",
171+
"updateOrigin": "ADMIN_SDK_NODE",
172+
"updateType": "INCREMENTAL_UPDATE",
173+
"updateUser": {
174+
"email": "firebase-user@account.com",
175+
"name": "dev-admin",
176+
"imageUrl": "http://image.jpg"
177+
},
178+
"updateTime": "2020-11-15T06:57:26.342763941Z",
179+
"description": "promo config"
172180
}
173181
}

0 commit comments

Comments
 (0)