Skip to content

Commit c7f1295

Browse files
hiranya911hiranya911
authored
Porting the fix to double serialization along with tests (firebase#117)
* Porting the fix to double serialization along with tests * Updated test case
1 parent f1e55dc commit c7f1295

13 files changed

Lines changed: 339 additions & 66 deletions

File tree

‎src/main/java/com/google/firebase/database/DataSnapshot.java‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package com.google.firebase.database;
1818

19+
import com.google.firebase.database.annotations.Nullable;
1920
import com.google.firebase.database.core.Path;
2021
import com.google.firebase.database.snapshot.IndexedNode;
2122
import com.google.firebase.database.snapshot.NamedNode;
@@ -116,8 +117,10 @@ public boolean exists() {
116117
* <p>This list is recursive; the possible types for {@link java.lang.Object} in the above list
117118
* is given by the same list. These types correspond to the types available in JSON.
118119
*
119-
* @return The data contained in this snapshot as native types
120+
* @return The data contained in this snapshot as native types or null if there is no data at this
121+
* location.
120122
*/
123+
@Nullable
121124
public Object getValue() {
122125
return node.getNode().getValue();
123126
}
@@ -143,8 +146,10 @@ public Object getValue() {
143146
* a map, the map will also include a .value key with the data.
144147
*
145148
* @param useExportFormat Whether or not to include priority information
146-
* @return The data, along with its priority, in native types
149+
* @return The data, along with its priority, in native types or null if there is no data at this
150+
* location.
147151
*/
152+
@Nullable
148153
public Object getValue(boolean useExportFormat) {
149154
return node.getNode().getValue(useExportFormat);
150155
}
@@ -190,8 +195,10 @@ public Object getValue(boolean useExportFormat) {
190195
*
191196
* @param valueType The class into which this snapshot should be marshalled
192197
* @param <T> The type to return. Implicitly defined from the class passed in
193-
* @return An instance of the class passed in, populated with the data from this snapshot
198+
* @return An instance of the class passed in, populated with the data from this snapshot, or null
199+
* if there is no data at this location.
194200
*/
201+
@Nullable
195202
public <T> T getValue(Class<T> valueType) {
196203
Object value = node.getNode().getValue();
197204
return CustomClassMapper.convertToCustomClass(value, valueType);
@@ -216,8 +223,10 @@ public <T> T getValue(Class<T> valueType) {
216223
* to be returned.
217224
* @param <T> The type to return. Implicitly defined from the {@link GenericTypeIndicator} passed
218225
* in
219-
* @return A properly typed collection, populated with the data from this snapshot
226+
* @return A properly typed collection, populated with the data from this snapshot, or null if
227+
* there is no data at this location.
220228
*/
229+
@Nullable
221230
public <T> T getValue(GenericTypeIndicator<T> t) {
222231
Object value = node.getNode().getValue();
223232
return CustomClassMapper.convertToCustomClass(value, t);

‎src/main/java/com/google/firebase/database/DatabaseReference.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ public ApiFuture<Void> setValueAsync(Object value, Object priority) {
226226
* @deprecated Use {@link #setValueAsync(Object)}
227227
*/
228228
public Task<Void> setValue(Object value) {
229-
return setValueInternal(value, PriorityUtilities.parsePriority(null), null);
229+
return setValueInternal(value, PriorityUtilities.parsePriority(this.path, null), null);
230230
}
231231

232232
/**
@@ -238,7 +238,7 @@ public Task<Void> setValue(Object value) {
238238
* @deprecated Use {@link #setValueAsync(Object, Object)}
239239
*/
240240
public Task<Void> setValue(Object value, Object priority) {
241-
return setValueInternal(value, PriorityUtilities.parsePriority(priority), null);
241+
return setValueInternal(value, PriorityUtilities.parsePriority(this.path, priority), null);
242242
}
243243

244244
/**
@@ -275,7 +275,7 @@ public Task<Void> setValue(Object value, Object priority) {
275275
* @param listener A listener that will be triggered with the results of the operation
276276
*/
277277
public void setValue(Object value, CompletionListener listener) {
278-
setValueInternal(value, PriorityUtilities.parsePriority(null), listener);
278+
setValueInternal(value, PriorityUtilities.parsePriority(this.path, null), listener);
279279
}
280280

281281
/**
@@ -312,7 +312,7 @@ public void setValue(Object value, CompletionListener listener) {
312312
* @param listener A listener that will be triggered with the results of the operation
313313
*/
314314
public void setValue(Object value, Object priority, CompletionListener listener) {
315-
setValueInternal(value, PriorityUtilities.parsePriority(priority), listener);
315+
setValueInternal(value, PriorityUtilities.parsePriority(this.path, priority), listener);
316316
}
317317

318318
private Task<Void> setValueInternal(Object value, Node priority, CompletionListener optListener) {
@@ -375,7 +375,7 @@ public ApiFuture<Void> setPriorityAsync(Object priority) {
375375
* @deprecated Use {@link #setPriorityAsync(Object)}
376376
*/
377377
public Task<Void> setPriority(Object priority) {
378-
return setPriorityInternal(PriorityUtilities.parsePriority(priority), null);
378+
return setPriorityInternal(PriorityUtilities.parsePriority(this.path, priority), null);
379379
}
380380

381381
/**
@@ -408,7 +408,7 @@ public Task<Void> setPriority(Object priority) {
408408
* @param listener A listener that will be triggered with results of the operation
409409
*/
410410
public void setPriority(Object priority, CompletionListener listener) {
411-
setPriorityInternal(PriorityUtilities.parsePriority(priority), listener);
411+
setPriorityInternal(PriorityUtilities.parsePriority(this.path, priority), listener);
412412
}
413413

414414
// Remove

‎src/main/java/com/google/firebase/database/MutableData.java‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package com.google.firebase.database;
1818

19+
import com.google.firebase.database.annotations.Nullable;
1920
import com.google.firebase.database.core.Path;
2021
import com.google.firebase.database.core.SnapshotHolder;
2122
import com.google.firebase.database.core.ValidationPath;
@@ -179,8 +180,10 @@ public String getKey() {
179180
* <p>This list is recursive; the possible types for {@link java.lang.Object} in the above list is
180181
* given by the same list. These types correspond to the types available in JSON.
181182
*
182-
* @return The data contained in this instance as native types
183+
* @return The data contained in this instance as native types, or null if there is no data at
184+
* this location.
183185
*/
186+
@Nullable
184187
public Object getValue() {
185188
return getNode().getValue();
186189
}
@@ -203,8 +206,10 @@ public Object getValue() {
203206
* to be returned.
204207
* @param <T> The type to return. Implicitly defined from the {@link GenericTypeIndicator} passed
205208
* in
206-
* @return A properly typed collection, populated with the data from this instance
209+
* @return A properly typed collection, populated with the data from this instance, or null if
210+
* there is no data at this location.
207211
*/
212+
@Nullable
208213
public <T> T getValue(GenericTypeIndicator<T> t) {
209214
Object value = getNode().getValue();
210215
return CustomClassMapper.convertToCustomClass(value, t);
@@ -251,8 +256,10 @@ public <T> T getValue(GenericTypeIndicator<T> t) {
251256
*
252257
* @param valueType The class into which this data in this instance should be marshalled
253258
* @param <T> The type to return. Implicitly defined from the class passed in
254-
* @return An instance of the class passed in, populated with the data from this instance
259+
* @return An instance of the class passed in, populated with the data from this instance, or null
260+
* if there is no data at this location.
255261
*/
262+
@Nullable
256263
public <T> T getValue(Class<T> valueType) {
257264
Object value = getNode().getValue();
258265
return CustomClassMapper.convertToCustomClass(value, valueType);
@@ -320,7 +327,8 @@ public Object getPriority() {
320327
* @param priority The desired priority
321328
*/
322329
public void setPriority(Object priority) {
323-
holder.update(prefixPath, getNode().updatePriority(PriorityUtilities.parsePriority(priority)));
330+
holder.update(prefixPath, getNode().updatePriority(
331+
PriorityUtilities.parsePriority(prefixPath, priority)));
324332
}
325333

326334
@Override

‎src/main/java/com/google/firebase/database/OnDisconnect.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public Task<Void> setValue(Object value) {
7272
* @deprecated Use {@link #setValueAsync(Object, String)}
7373
*/
7474
public Task<Void> setValue(Object value, String priority) {
75-
return onDisconnectSetInternal(value, PriorityUtilities.parsePriority(priority), null);
75+
return onDisconnectSetInternal(value, PriorityUtilities.parsePriority(path, priority), null);
7676
}
7777

7878
/**
@@ -84,7 +84,7 @@ public Task<Void> setValue(Object value, String priority) {
8484
* @deprecated Use {@link #setValueAsync(Object, double)}
8585
*/
8686
public Task<Void> setValue(Object value, double priority) {
87-
return onDisconnectSetInternal(value, PriorityUtilities.parsePriority(priority), null);
87+
return onDisconnectSetInternal(value, PriorityUtilities.parsePriority(path, priority), null);
8888
}
8989

9090
/**
@@ -113,7 +113,7 @@ public void setValue(Object value, CompletionListener listener) {
113113
* @param listener A listener that will be triggered once the server has queued up the operation
114114
*/
115115
public void setValue(Object value, String priority, CompletionListener listener) {
116-
onDisconnectSetInternal(value, PriorityUtilities.parsePriority(priority), listener);
116+
onDisconnectSetInternal(value, PriorityUtilities.parsePriority(path, priority), listener);
117117
}
118118

119119
/**
@@ -128,7 +128,7 @@ public void setValue(Object value, String priority, CompletionListener listener)
128128
* @param listener A listener that will be triggered once the server has queued up the operation
129129
*/
130130
public void setValue(Object value, double priority, CompletionListener listener) {
131-
onDisconnectSetInternal(value, PriorityUtilities.parsePriority(priority), listener);
131+
onDisconnectSetInternal(value, PriorityUtilities.parsePriority(path, priority), listener);
132132
}
133133

134134
/**
@@ -143,7 +143,7 @@ public void setValue(Object value, double priority, CompletionListener listener)
143143
* @param listener A listener that will be triggered once the server has queued up the operation
144144
*/
145145
public void setValue(Object value, Map priority, CompletionListener listener) {
146-
onDisconnectSetInternal(value, PriorityUtilities.parsePriority(priority), listener);
146+
onDisconnectSetInternal(value, PriorityUtilities.parsePriority(path, priority), listener);
147147
}
148148

149149
/**

‎src/main/java/com/google/firebase/database/snapshot/PriorityUtilities.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.google.firebase.database.snapshot;
1818

1919
import com.google.firebase.database.DatabaseException;
20+
import com.google.firebase.database.core.Path;
2021

2122
public class PriorityUtilities {
2223

@@ -35,6 +36,10 @@ public static boolean isValidPriority(Node priority) {
3536
}
3637

3738
public static Node parsePriority(Object value) {
39+
return parsePriority(null, value);
40+
}
41+
42+
public static Node parsePriority(Path nodePath, Object value) {
3843
Node priority = NodeUtilities.NodeFromJSON(value);
3944
if (priority instanceof LongNode) {
4045
priority =
@@ -43,7 +48,8 @@ public static Node parsePriority(Object value) {
4348
}
4449
if (!isValidPriority(priority)) {
4550
throw new DatabaseException(
46-
"Invalid Firebase Database priority (must be a string, double, ServerValue, or null)");
51+
(nodePath != null ? "Path '" + nodePath + "'" : "Node")
52+
+ " contains invalid priority: Must be a string, double, ServerValue, or null");
4753
}
4854
return priority;
4955
}

‎src/main/java/com/google/firebase/database/utilities/Validation.java‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,12 @@ public static void validateNullableKey(String key) throws DatabaseException {
8383
}
8484
}
8585

86+
private static void validateDoubleValue(double d) {
87+
if (Double.isInfinite(d) || Double.isNaN(d)) {
88+
throw new DatabaseException("Invalid value: Value cannot be NaN, Inf or -Inf.");
89+
}
90+
}
91+
8692
private static boolean isWritablePath(Path path) {
8793
// Getting a path with invalid keys will throw earlier in the process, so we should just
8894
// check the first token
@@ -107,6 +113,8 @@ public static void validateWritableObject(Object object) {
107113
for (Object child : list) {
108114
validateWritableObject(child);
109115
}
116+
} else if (object instanceof Double || object instanceof Float) {
117+
validateDoubleValue((double) object);
110118
} else {
111119
// It's a primitive, should be fine
112120
}
@@ -138,17 +146,14 @@ public static Map<Path, Node> parseAndValidateUpdate(Path path, Map<String, Obje
138146
throw new DatabaseException(
139147
"Path '" + updatePath + "' contains disallowed child name: " + childName);
140148
}
149+
Node parsedValue;
141150
if (childName.equals(".priority")) {
142-
if (!PriorityUtilities.isValidPriority(NodeUtilities.NodeFromJSON(newValue))) {
143-
throw new DatabaseException(
144-
"Path '"
145-
+ updatePath
146-
+ "' contains invalid priority "
147-
+ "(must be a string, double, ServerValue, or null).");
148-
}
151+
parsedValue = PriorityUtilities.parsePriority(updatePath, newValue);
152+
} else {
153+
parsedValue = NodeUtilities.NodeFromJSON(newValue);
149154
}
150155
Validation.validateWritableObject(newValue);
151-
parsedUpdate.put(updatePath, NodeUtilities.NodeFromJSON(newValue));
156+
parsedUpdate.put(updatePath, parsedValue);
152157
}
153158
// Check that update keys are not ancestors of each other.
154159
Path prevPath = null;

‎src/main/java/com/google/firebase/database/utilities/encoding/CustomClassMapper.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,14 +115,20 @@ private static <T> Object serialize(T obj) {
115115
if (obj == null) {
116116
return null;
117117
} else if (obj instanceof Number) {
118-
if (obj instanceof Float) {
119-
return ((Float) obj).doubleValue();
118+
if (obj instanceof Float || obj instanceof Double) {
119+
double doubleValue = ((Number) obj).doubleValue();
120+
if (doubleValue <= Long.MAX_VALUE
121+
&& doubleValue >= Long.MIN_VALUE
122+
&& Math.floor(doubleValue) == doubleValue) {
123+
return ((Number) obj).longValue();
124+
}
125+
return doubleValue;
120126
} else if (obj instanceof Short) {
121127
throw new DatabaseException("Shorts are not supported, please use int or long");
122128
} else if (obj instanceof Byte) {
123129
throw new DatabaseException("Bytes are not supported, please use int or long");
124130
} else {
125-
// Long, Integer, Double
131+
// Long, Integer
126132
return obj;
127133
}
128134
} else if (obj instanceof String) {

‎src/test/java/com/google/firebase/database/MapperTest.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,16 @@ public void serializeDoubleBean() {
429429
assertJson("{'value': 1.1}", serialize(bean));
430430
}
431431

432+
@Test
433+
public void serializeDoubleBeanAsLong() {
434+
DoubleBean bean = new DoubleBean();
435+
bean.value = 1234567890123L;
436+
assertJson("{'value': 1234567890123}", serialize(bean));
437+
438+
bean.value = 1234567890123.0;
439+
assertJson("{'value': 1234567890123}", serialize(bean));
440+
}
441+
432442
@Test
433443
public void serializeIntBean() {
434444
IntBean bean = new IntBean();

‎src/test/java/com/google/firebase/database/integration/DataTestIT.java‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -904,6 +904,36 @@ public void testGetPriority()
904904
assertNull(events.get(6).getSnapshot().getPriority());
905905
}
906906

907+
@Test
908+
public void testNormalizeDifferentIntegerAndDoubleValues()
909+
throws DatabaseException, InterruptedException, TimeoutException, TestFailure {
910+
final long intMaxPlusOne = 2147483648L;
911+
912+
DatabaseReference node = IntegrationTestUtils.getRandomNode(masterApp);
913+
Object[] writtenValues = {
914+
intMaxPlusOne,
915+
(double) intMaxPlusOne,
916+
-intMaxPlusOne,
917+
(double) -intMaxPlusOne,
918+
Integer.MAX_VALUE,
919+
0L,
920+
0.0,
921+
-0.0f,
922+
0
923+
};
924+
925+
Object[] readValues = {intMaxPlusOne, -intMaxPlusOne, (long) Integer.MAX_VALUE, 0L};
926+
ReadFuture readFuture = ReadFuture.untilCountAfterNull(node, readValues.length);
927+
for (Object value : writtenValues) {
928+
node.setValueAsync(value);
929+
}
930+
931+
List<EventRecord> events = readFuture.timedGet();
932+
for (int i = 0; i < readValues.length; ++i) {
933+
assertEquals(readValues[i], events.get(i).getSnapshot().getValue());
934+
}
935+
}
936+
907937
@Test
908938
public void testExportFormatIncludesPriorities()
909939
throws TimeoutException, InterruptedException, TestFailure {
@@ -1175,6 +1205,29 @@ public void testAsciiControlCharacters() throws DatabaseException {
11751205
}
11761206
}
11771207

1208+
@Test
1209+
public void invalidDoubleValues()
1210+
throws DatabaseException, TestFailure, TimeoutException, InterruptedException {
1211+
DatabaseReference node = IntegrationTestUtils.getRandomNode(masterApp);
1212+
Object[] invalidValues =
1213+
new Object[] {
1214+
Double.NEGATIVE_INFINITY,
1215+
Double.POSITIVE_INFINITY,
1216+
Double.NaN,
1217+
Float.NEGATIVE_INFINITY,
1218+
Float.POSITIVE_INFINITY,
1219+
Float.NaN
1220+
};
1221+
for (Object invalidValue : invalidValues) {
1222+
try {
1223+
node.setValueAsync(invalidValue);
1224+
fail("NaN or Inf are not allowed as values.");
1225+
} catch (DatabaseException expected) {
1226+
assertEquals("Invalid value: Value cannot be NaN, Inf or -Inf.", expected.getMessage());
1227+
}
1228+
}
1229+
}
1230+
11781231
@Test
11791232
@Ignore
11801233
// TODO: Stop ignoring this test once JSON parsing has been fixed.

0 commit comments

Comments
 (0)