Skip to content

Commit 7b0925f

Browse files
schmidt-sebastianschmidt-sebastian
authored
Make FieldValue a test-only class (#1208)
1 parent 3c6d8d1 commit 7b0925f

File tree

13 files changed

+144
-59
lines changed

13 files changed

+144
-59
lines changed

‎firebase-firestore/src/main/java/com/google/firebase/firestore/model/value/FieldValue.java renamed to ‎firebase-firestore/ktx/src/test/java/com/google/firebase/firestore/model/value/FieldValue.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
* Represents a FieldValue that is backed by a single Firestore V1 Value proto and implements
2222
* Firestore's Value semantics for ordering and equality.
2323
*/
24+
// TODO(mrschmidt): Drop this class
2425
public class FieldValue implements Comparable<FieldValue> {
2526
final Value internalValue;
2627

‎firebase-firestore/src/main/java/com/google/firebase/firestore/UserDataWriter.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ Object convertValue(Value value) {
5353
switch (value.getValueTypeCase()) {
5454
case MAP_VALUE:
5555
if (ServerTimestampValue.isServerTimestamp(value)) {
56-
return convertServerTimestamp(ServerTimestampValue.valueOf(value));
56+
return convertServerTimestamp(new ServerTimestampValue(value));
5757
}
5858
return convertObject(value.getMapValue().getFieldsMap());
5959
case ARRAY_VALUE:

‎firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteCollectionIndex.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
import com.google.firebase.firestore.model.DocumentKey;
2020
import com.google.firebase.firestore.model.FieldPath;
2121
import com.google.firebase.firestore.model.ResourcePath;
22-
import com.google.firebase.firestore.model.value.FieldValue;
22+
import com.google.firestore.v1.Value;
2323

2424
/**
2525
* A persisted "collection index" of all documents in the local cache (with mutations overlaid on
@@ -38,12 +38,12 @@ public class SQLiteCollectionIndex {
3838
}
3939

4040
/** Adds the specified entry to the index. */
41-
public void addEntry(FieldPath fieldPath, FieldValue fieldValue, DocumentKey documentKey) {
41+
public void addEntry(FieldPath fieldPath, Value fieldValue, DocumentKey documentKey) {
4242
throw new RuntimeException("Not yet implemented.");
4343
}
4444

4545
/** Adds the specified entry to the index. */
46-
public void removeEntry(FieldPath fieldPath, FieldValue fieldValue, DocumentKey documentKey) {
46+
public void removeEntry(FieldPath fieldPath, Value fieldValue, DocumentKey documentKey) {
4747
throw new RuntimeException("Not yet implemented.");
4848
}
4949

‎firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/ServerTimestampOperation.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public static ServerTimestampOperation getInstance() {
3131

3232
@Override
3333
public Value applyToLocalView(@Nullable Value previousValue, Timestamp localWriteTime) {
34-
return ServerTimestampValue.valueOf(localWriteTime, previousValue).getProto();
34+
return ServerTimestampValue.valueOf(localWriteTime, previousValue);
3535
}
3636

3737
@Override

‎firebase-firestore/src/main/java/com/google/firebase/firestore/model/value/ObjectValue.java

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@
2828

2929
/** A structured object value stored in Firestore. */
3030
// TODO(mrschmidt): Rename to DocumentValue
31-
public class ObjectValue extends FieldValue {
31+
public class ObjectValue {
32+
private Value internalValue;
33+
3234
private static final ObjectValue EMPTY_INSTANCE =
3335
new ObjectValue(Value.newBuilder().setMapValue(MapValue.getDefaultInstance()).build());
3436

@@ -38,13 +40,13 @@ public static ObjectValue fromMap(Map<String, Value> value) {
3840
}
3941

4042
public ObjectValue(Value value) {
41-
super(value);
4243
hardAssert(
4344
value.getValueTypeCase() == Value.ValueTypeCase.MAP_VALUE,
4445
"ObjectValues should be backed by a MapValue");
4546
hardAssert(
4647
!ServerTimestampValue.isServerTimestamp(value),
47-
"ServerTimestamps should be converted to ServerTimestampValue");
48+
"ServerTimestamps should not be used as an ObjectValue");
49+
this.internalValue = value;
4850
}
4951

5052
public static ObjectValue emptyObject() {
@@ -109,6 +111,26 @@ private FieldMask extractFieldMask(MapValue value) {
109111
}
110112
}
111113

114+
/** Returns the Protobuf that backs this ObjectValue. */
115+
public Value getProto() {
116+
return internalValue;
117+
}
118+
119+
@Override
120+
public boolean equals(Object o) {
121+
if (this == o) {
122+
return true;
123+
} else if (o instanceof ObjectValue) {
124+
return ProtoValues.equals(internalValue, ((ObjectValue) o).internalValue);
125+
}
126+
return false;
127+
}
128+
129+
@Override
130+
public int hashCode() {
131+
return internalValue.hashCode();
132+
}
133+
112134
/** Creates a ObjectValue.Builder instance that is based on the current value. */
113135
public ObjectValue.Builder toBuilder() {
114136
return new ObjectValue.Builder(this);

‎firebase-firestore/src/main/java/com/google/firebase/firestore/model/value/ServerTimestampValue.java

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
* <li>They sort after all TimestampValues. With respect to other ServerTimestampValues, they sort
3838
* by their localWriteTime.
3939
*/
40-
public final class ServerTimestampValue extends FieldValue {
40+
public final class ServerTimestampValue {
4141
private static final String SERVER_TIMESTAMP_SENTINEL = "server_timestamp";
4242
private static final String TYPE_KEY = "__type__";
4343
private static final String PREVIOUS_VALUE_KEY = "__previous_value__";
@@ -48,18 +48,16 @@ public static boolean isServerTimestamp(@Nullable Value value) {
4848
return type != null && SERVER_TIMESTAMP_SENTINEL.equals(type.getStringValue());
4949
}
5050

51-
ServerTimestampValue(Value value) {
52-
super(value);
51+
private final Value internalValue;
52+
53+
public ServerTimestampValue(Value value) {
5354
hardAssert(
5455
ServerTimestampValue.isServerTimestamp(value),
5556
"Backing value is not a ServerTimestampValue");
57+
this.internalValue = value;
5658
}
5759

58-
public static ServerTimestampValue valueOf(Value value) {
59-
return new ServerTimestampValue(value);
60-
}
61-
62-
public static FieldValue valueOf(Timestamp localWriteTime, @Nullable Value previousValue) {
60+
public static Value valueOf(Timestamp localWriteTime, @Nullable Value previousValue) {
6361
Value encodedType = Value.newBuilder().setStringValue(SERVER_TIMESTAMP_SENTINEL).build();
6462
Value encodeWriteTime =
6563
Value.newBuilder()
@@ -78,7 +76,7 @@ public static FieldValue valueOf(Timestamp localWriteTime, @Nullable Value previ
7876
mapRepresentation.putFields(PREVIOUS_VALUE_KEY, previousValue);
7977
}
8078

81-
return new ServerTimestampValue(Value.newBuilder().setMapValue(mapRepresentation).build());
79+
return Value.newBuilder().setMapValue(mapRepresentation).build();
8280
}
8381

8482
/**

‎firebase-firestore/src/test/java/com/google/firebase/firestore/QuerySnapshotTest.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,7 @@ public void testToObjects() {
8686
.thenReturn(new FirebaseFirestoreSettings.Builder().build());
8787

8888
ObjectValue objectData =
89-
ObjectValue.fromMap(
90-
map("timestamp", ServerTimestampValue.valueOf(Timestamp.now(), null).getProto()));
89+
ObjectValue.fromMap(map("timestamp", ServerTimestampValue.valueOf(Timestamp.now(), null)));
9190
QuerySnapshot foo = TestUtil.querySnapshot("foo", map(), map("a", objectData), true, false);
9291

9392
List<POJO> docs = foo.toObjects(POJO.class);

‎firebase-firestore/src/test/java/com/google/firebase/firestore/UserDataWriterTest.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import java.util.HashMap;
4040
import java.util.List;
4141
import java.util.Map;
42-
import java.util.TreeMap;
4342
import org.junit.Test;
4443
import org.junit.runner.RunWith;
4544
import org.robolectric.RobolectricTestRunner;
@@ -226,7 +225,7 @@ public void testConvertsGeoPointValue() {
226225

227226
@Test
228227
public void testConvertsEmptyObjects() {
229-
assertEquals(wrap(new TreeMap<String, FieldValue>()), ObjectValue.emptyObject());
228+
assertEquals(wrapObject(), ObjectValue.emptyObject());
230229
}
231230

232231
@Test
@@ -242,7 +241,7 @@ public void testConvertsSimpleObjects() {
242241
"c", wrap(true),
243242
"d", wrap(null));
244243

245-
FieldValue wrappedActual = wrapObject(actual);
244+
ObjectValue wrappedActual = wrapObject(actual);
246245
assertEquals(wrappedActual, wrappedExpected);
247246
}
248247

@@ -256,7 +255,7 @@ private static ObjectValue fromMap(Object... entries) {
256255

257256
@Test
258257
public void testConvertsNestedObjects() {
259-
FieldValue actual = wrapObject("a", map("b", map("c", "foo"), "d", true));
258+
ObjectValue actual = wrapObject("a", map("b", map("c", "foo"), "d", true));
260259
ObjectValue.Builder expected = ObjectValue.newBuilder();
261260
expected.set(field("a.b.c"), valueOf("foo"));
262261
expected.set(field("a.d"), valueOf(true));

‎firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -80,16 +80,15 @@ public void testExtractsFields() {
8080

8181
@Test
8282
public void testExtractsFieldMask() {
83-
FieldValue val =
83+
ObjectValue val =
8484
wrapObject(
8585
"a",
8686
"b",
8787
"map",
8888
map("a", 1, "b", true, "c", "string", "nested", map("d", "e")),
8989
"emptymap",
9090
map());
91-
assertTrue(val instanceof ObjectValue);
92-
FieldMask mask = ((ObjectValue) val).getFieldMask();
91+
FieldMask mask = val.getFieldMask();
9392
assertEquals(fieldMask("a", "map.a", "map.b", "map.c", "map.nested.d", "emptymap"), mask);
9493
}
9594

@@ -254,21 +253,22 @@ public void testValueEquality() {
254253
.addEqualityGroup(wrap(timestamp2))
255254
// NOTE: ServerTimestampValues can't be parsed via wrap().
256255
.addEqualityGroup(
257-
ServerTimestampValue.valueOf(new Timestamp(date1), null),
258-
ServerTimestampValue.valueOf(new Timestamp(date1), null))
259-
.addEqualityGroup(ServerTimestampValue.valueOf(new Timestamp(date2), null))
256+
new FieldValue(ServerTimestampValue.valueOf(new Timestamp(date1), null)),
257+
new FieldValue(ServerTimestampValue.valueOf(new Timestamp(date1), null)))
258+
.addEqualityGroup(new FieldValue(ServerTimestampValue.valueOf(new Timestamp(date2), null)))
260259
.addEqualityGroup(wrap(geoPoint1), wrap(new GeoPoint(1, 0)))
261260
.addEqualityGroup(wrap(geoPoint2))
262-
.addEqualityGroup(wrap(ref("coll/doc1")), wrapRef(dbId("project"), key("coll/doc1")))
263-
.addEqualityGroup(wrapRef(dbId("project", "bar"), key("coll/doc2")))
264-
.addEqualityGroup(wrapRef(dbId("project", "baz"), key("coll/doc2")))
261+
.addEqualityGroup(
262+
wrap(ref("coll/doc1")), new FieldValue(wrapRef(dbId("project"), key("coll/doc1"))))
263+
.addEqualityGroup(new FieldValue(wrapRef(dbId("project", "bar"), key("coll/doc2"))))
264+
.addEqualityGroup(new FieldValue(wrapRef(dbId("project", "baz"), key("coll/doc2"))))
265265
.addEqualityGroup(wrap(Arrays.asList("foo", "bar")), wrap(Arrays.asList("foo", "bar")))
266266
.addEqualityGroup(wrap(Arrays.asList("foo", "bar", "baz")))
267267
.addEqualityGroup(wrap(Arrays.asList("foo")))
268-
.addEqualityGroup(wrapObject(map("bar", 1, "foo", 2)), wrapObject(map("foo", 2, "bar", 1)))
269-
.addEqualityGroup(wrapObject(map("bar", 2, "foo", 1)))
270-
.addEqualityGroup(wrapObject(map("bar", 1)))
271-
.addEqualityGroup(wrapObject(map("foo", 1)))
268+
.addEqualityGroup(wrap(map("bar", 1, "foo", 2)), wrap(map("foo", 2, "bar", 1)))
269+
.addEqualityGroup(wrap(map("bar", 2, "foo", 1)))
270+
.addEqualityGroup(wrap(map("bar", 1)))
271+
.addEqualityGroup(wrap(map("foo", 1)))
272272
.testEquals();
273273
}
274274

@@ -312,8 +312,8 @@ public void testValueOrdering() {
312312

313313
// server timestamps come after all concrete timestamps.
314314
// NOTE: server timestamps can't be parsed with wrap().
315-
.addEqualityGroup(ServerTimestampValue.valueOf(new Timestamp(date1), null))
316-
.addEqualityGroup(ServerTimestampValue.valueOf(new Timestamp(date2), null))
315+
.addEqualityGroup(new FieldValue(ServerTimestampValue.valueOf(new Timestamp(date1), null)))
316+
.addEqualityGroup(new FieldValue(ServerTimestampValue.valueOf(new Timestamp(date2), null)))
317317

318318
// strings
319319
.addEqualityGroup(wrap(""))
@@ -335,12 +335,12 @@ public void testValueOrdering() {
335335
.addEqualityGroup(wrap(blob(255)))
336336

337337
// resource names
338-
.addEqualityGroup(wrapRef(dbId("p1", "d1"), key("c1/doc1")))
339-
.addEqualityGroup(wrapRef(dbId("p1", "d1"), key("c1/doc2")))
340-
.addEqualityGroup(wrapRef(dbId("p1", "d1"), key("c10/doc1")))
341-
.addEqualityGroup(wrapRef(dbId("p1", "d1"), key("c2/doc1")))
342-
.addEqualityGroup(wrapRef(dbId("p1", "d2"), key("c1/doc1")))
343-
.addEqualityGroup(wrapRef(dbId("p2", "d1"), key("c1/doc1")))
338+
.addEqualityGroup(new FieldValue(wrapRef(dbId("p1", "d1"), key("c1/doc1"))))
339+
.addEqualityGroup(new FieldValue(wrapRef(dbId("p1", "d1"), key("c1/doc2"))))
340+
.addEqualityGroup(new FieldValue(wrapRef(dbId("p1", "d1"), key("c10/doc1"))))
341+
.addEqualityGroup(new FieldValue(wrapRef(dbId("p1", "d1"), key("c2/doc1"))))
342+
.addEqualityGroup(new FieldValue(wrapRef(dbId("p1", "d2"), key("c1/doc1"))))
343+
.addEqualityGroup(new FieldValue(wrapRef(dbId("p2", "d1"), key("c1/doc1"))))
344344

345345
// geo points
346346
.addEqualityGroup(wrap(new GeoPoint(-90, -180)))
@@ -363,11 +363,11 @@ public void testValueOrdering() {
363363
.addEqualityGroup(wrap(Arrays.asList("foo", "0")))
364364

365365
// objects
366-
.addEqualityGroup(wrapObject(map("bar", 0)))
367-
.addEqualityGroup(wrapObject(map("bar", 0, "foo", 1)))
368-
.addEqualityGroup(wrapObject(map("foo", 1)))
369-
.addEqualityGroup(wrapObject(map("foo", 2)))
370-
.addEqualityGroup(wrapObject(map("foo", "0")))
366+
.addEqualityGroup(wrap(map("bar", 0)))
367+
.addEqualityGroup(wrap(map("bar", 0, "foo", 1)))
368+
.addEqualityGroup(wrap(map("foo", 1)))
369+
.addEqualityGroup(wrap(map("foo", 2)))
370+
.addEqualityGroup(wrap(map("foo", "0")))
371371
.testCompare();
372372
}
373373

@@ -381,7 +381,7 @@ public void testCanonicalIds() {
381381
assertCanonicalId(wrap(new Timestamp(30, 1000)), "time(30,1000)");
382382
assertCanonicalId(wrap("a"), "a");
383383
assertCanonicalId(wrap(blob(1, 2, 3)), "010203");
384-
assertCanonicalId(wrapRef(dbId("p1", "d1"), key("c1/doc1")), "c1/doc1");
384+
assertCanonicalId(new FieldValue(wrapRef(dbId("p1", "d1"), key("c1/doc1"))), "c1/doc1");
385385
assertCanonicalId(wrap(new GeoPoint(30, 60)), "geo(30.0,60.0)");
386386
assertCanonicalId(wrap(Arrays.asList(1, 2, 3)), "[1,2,3]");
387387
assertCanonicalId(wrap(map("a", 1, "b", 2, "c", "3")), "{a:1,b:2,c:3}");

‎firebase-firestore/src/test/java/com/google/firebase/firestore/model/MutationTest.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import static com.google.firebase.firestore.testutil.TestUtil.wrapObject;
3333
import static org.junit.Assert.assertEquals;
3434
import static org.junit.Assert.assertNull;
35+
import static org.junit.Assert.assertTrue;
3536

3637
import com.google.firebase.Timestamp;
3738
import com.google.firebase.firestore.FieldValue;
@@ -44,6 +45,7 @@
4445
import com.google.firebase.firestore.model.mutation.Precondition;
4546
import com.google.firebase.firestore.model.mutation.TransformMutation;
4647
import com.google.firebase.firestore.model.value.ObjectValue;
48+
import com.google.firebase.firestore.model.value.ProtoValues;
4749
import com.google.firebase.firestore.model.value.ServerTimestampValue;
4850
import java.util.Arrays;
4951
import java.util.Collections;
@@ -156,7 +158,8 @@ public void testAppliesLocalServerTimestampTransformsToDocuments() {
156158
ObjectValue expectedData =
157159
wrapObject(map("foo", map("bar", "<server-timestamp>"), "baz", "baz-value"));
158160
com.google.firebase.firestore.model.value.FieldValue fieldValue =
159-
ServerTimestampValue.valueOf(timestamp, valueOf("bar-value"));
161+
new com.google.firebase.firestore.model.value.FieldValue(
162+
ServerTimestampValue.valueOf(timestamp, valueOf("bar-value")));
160163
expectedData = expectedData.toBuilder().set(field("foo.bar"), fieldValue.getProto()).build();
161164

162165
Document expectedDoc =
@@ -308,9 +311,10 @@ public void testCreateArrayUnionTransform() {
308311
transformMutation(
309312
"collection/key",
310313
map(
311-
"a", FieldValue.arrayUnion("tag"),
314+
"a",
315+
FieldValue.arrayUnion("tag"),
312316
"bar.baz",
313-
FieldValue.arrayUnion(true, map("nested", map("a", Arrays.asList(1, 2))))));
317+
FieldValue.arrayUnion(true, map("nested", map("a", Arrays.asList(1, 2))))));
314318
assertEquals(2, transform.getFieldTransforms().size());
315319

316320
FieldTransform first = transform.getFieldTransforms().get(0);
@@ -688,7 +692,7 @@ public void testNumericIncrementBaseValue() {
688692
0,
689693
"nested",
690694
map("double", 42.0, "long", 42, "string", 0, "map", 0, "missing", 0)));
691-
assertEquals(expected, baseValue);
695+
assertTrue(ProtoValues.equals(expected.getProto(), baseValue.getProto()));
692696
}
693697

694698
@Test

‎firebase-firestore/src/test/java/com/google/firebase/firestore/remote/RemoteSerializerTest.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import com.google.firebase.firestore.model.SnapshotVersion;
5252
import com.google.firebase.firestore.model.mutation.Mutation;
5353
import com.google.firebase.firestore.model.value.FieldValue;
54+
import com.google.firebase.firestore.model.value.ObjectValue;
5455
import com.google.firebase.firestore.model.value.ProtoValues;
5556
import com.google.firebase.firestore.remote.WatchChange.WatchTargetChange;
5657
import com.google.firebase.firestore.remote.WatchChange.WatchTargetChangeType;
@@ -121,7 +122,7 @@ private void assertRoundTrip(
121122
com.google.firestore.v1.Value actual = value.getProto();
122123
assertEquals(typeCase, actual.getValueTypeCase());
123124
assertEquals(proto, actual);
124-
assertEquals(value, new FieldValue(proto));
125+
assertTrue(ProtoValues.equals(value.getProto(), proto));
125126
}
126127

127128
@Test
@@ -258,7 +259,7 @@ public void testEncodeArrays() {
258259

259260
@Test
260261
public void testEncodesNestedObjects() {
261-
FieldValue model =
262+
ObjectValue model =
262263
TestUtil.wrapObject(
263264
map(
264265
"b",
@@ -304,7 +305,7 @@ public void testEncodesNestedObjects() {
304305
.putFields("o", valueBuilder().setMapValue(middle).build());
305306

306307
com.google.firestore.v1.Value proto = valueBuilder().setMapValue(obj).build();
307-
assertRoundTrip(model, proto, ValueTypeCase.MAP_VALUE);
308+
assertRoundTrip(new FieldValue(model.getProto()), proto, ValueTypeCase.MAP_VALUE);
308309
}
309310

310311
@Test

0 commit comments

Comments
 (0)