Skip to content

Commit 940a268

Browse files
schmidt-sebastianschmidt-sebastian
committed
Migrate ArrayTransforms to use Values
1 parent 98953a3 commit 940a268

File tree

11 files changed

+57
-63
lines changed

11 files changed

+57
-63
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import com.google.firebase.firestore.model.value.ServerTimestampValue;
4646
import com.google.firebase.firestore.util.Executors;
4747
import com.google.firebase.firestore.util.Util;
48+
import com.google.firestore.v1.Value;
4849
import java.util.ArrayList;
4950
import java.util.Arrays;
5051
import java.util.List;
@@ -336,9 +337,9 @@ private Query whereHelper(@NonNull FieldPath fieldPath, Operator op, Object valu
336337
+ "' queries on FieldPath.documentId().");
337338
} else if (op == Operator.IN) {
338339
validateDisjunctiveFilterElements(value, op);
339-
List<FieldValue> referenceList = new ArrayList<>();
340+
List<Value> referenceList = new ArrayList<>();
340341
for (Object arrayValue : (List) value) {
341-
referenceList.add(parseDocumentIdValue(arrayValue));
342+
referenceList.add(parseDocumentIdValue(arrayValue).getProto());
342343
}
343344
fieldValue = ArrayValue.fromList(referenceList);
344345
} else {

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -357,13 +357,13 @@ private void parseSentinelFieldValue(
357357
context.addToFieldTransforms(context.getPath(), ServerTimestampOperation.getInstance());
358358

359359
} else if (value instanceof ArrayUnionFieldValue) {
360-
List<FieldValue> parsedElements =
360+
List<Value> parsedElements =
361361
parseArrayTransformElements(((ArrayUnionFieldValue) value).getElements());
362362
ArrayTransformOperation arrayUnion = new ArrayTransformOperation.Union(parsedElements);
363363
context.addToFieldTransforms(context.getPath(), arrayUnion);
364364

365365
} else if (value instanceof ArrayRemoveFieldValue) {
366-
List<FieldValue> parsedElements =
366+
List<Value> parsedElements =
367367
parseArrayTransformElements(((ArrayRemoveFieldValue) value).getElements());
368368
ArrayTransformOperation arrayRemove = new ArrayTransformOperation.Remove(parsedElements);
369369
context.addToFieldTransforms(context.getPath(), arrayRemove);
@@ -464,17 +464,17 @@ private Value parseTimestamp(Timestamp timestamp) {
464464
.build();
465465
}
466466

467-
private List<FieldValue> parseArrayTransformElements(List<Object> elements) {
467+
private List<Value> parseArrayTransformElements(List<Object> elements) {
468468
ParseAccumulator accumulator = new ParseAccumulator(UserData.Source.Argument);
469469

470-
ArrayList<FieldValue> result = new ArrayList<>(elements.size());
470+
List<Value> result = new ArrayList<>(elements.size());
471471
for (int i = 0; i < elements.size(); i++) {
472472
Object element = elements.get(i);
473473
// Although array transforms are used with writes, the actual elements
474474
// being unioned or removed are not considered writes since they cannot
475475
// contain any FieldValue sentinels, etc.
476476
ParseContext context = accumulator.rootContext();
477-
result.add(FieldValue.valueOf(convertAndParseFieldData(element, context.childContext(i))));
477+
result.add(convertAndParseFieldData(element, context.childContext(i)));
478478
}
479479
return result;
480480
}

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

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,11 @@
1818
import com.google.firebase.Timestamp;
1919
import com.google.firebase.firestore.model.value.ArrayValue;
2020
import com.google.firebase.firestore.model.value.FieldValue;
21+
import com.google.firebase.firestore.model.value.ProtoValues;
22+
import com.google.firestore.v1.Value;
2123
import java.util.ArrayList;
2224
import java.util.Collections;
25+
import java.util.Iterator;
2326
import java.util.List;
2427

2528
/**
@@ -28,13 +31,13 @@
2831
* <p>Implementations are: ArrayTransformOperation.Union and ArrayTransformOperation.Remove
2932
*/
3033
public abstract class ArrayTransformOperation implements TransformOperation {
31-
private final List<FieldValue> elements;
34+
private final List<Value> elements;
3235

33-
ArrayTransformOperation(List<FieldValue> elements) {
36+
ArrayTransformOperation(List<Value> elements) {
3437
this.elements = Collections.unmodifiableList(elements);
3538
}
3639

37-
public List<FieldValue> getElements() {
40+
public List<Value> getElements() {
3841
return elements;
3942
}
4043

@@ -86,9 +89,9 @@ public int hashCode() {
8689
* Inspects the provided value, returning an ArrayList copy of the internal array if it's an
8790
* ArrayValue and an empty ArrayList if it's null or any other type of FSTFieldValue.
8891
*/
89-
static List<FieldValue> coercedFieldValuesArray(@Nullable FieldValue value) {
92+
static ArrayList<Value> coercedFieldValuesArray(@Nullable FieldValue value) {
9093
if (value instanceof ArrayValue) {
91-
return ((ArrayValue) value).getValues();
94+
return new ArrayList<>(value.getProto().getArrayValue().getValuesList());
9295
} else {
9396
// coerce to empty array.
9497
return new ArrayList<>();
@@ -97,17 +100,16 @@ static List<FieldValue> coercedFieldValuesArray(@Nullable FieldValue value) {
97100

98101
/** An array union transform operation. */
99102
public static class Union extends ArrayTransformOperation {
100-
// TODO(mrschmidt): Migrate to list of Value protos
101-
public Union(List<FieldValue> elements) {
103+
public Union(List<Value> elements) {
102104
super(elements);
103105
}
104106

105107
@Override
106108
protected ArrayValue apply(@Nullable FieldValue previousValue) {
107-
List<FieldValue> result = coercedFieldValuesArray(previousValue);
108-
for (FieldValue element : getElements()) {
109-
if (!result.contains(element)) {
110-
result.add(element);
109+
List<Value> result = coercedFieldValuesArray(previousValue);
110+
for (Value unionElement : getElements()) {
111+
if (!ProtoValues.contains(result, unionElement)) {
112+
result.add(unionElement);
111113
}
112114
}
113115
return ArrayValue.fromList(result);
@@ -116,15 +118,21 @@ protected ArrayValue apply(@Nullable FieldValue previousValue) {
116118

117119
/** An array remove transform operation. */
118120
public static class Remove extends ArrayTransformOperation {
119-
public Remove(List<FieldValue> elements) {
121+
public Remove(List<Value> elements) {
120122
super(elements);
121123
}
122124

123125
@Override
124126
protected ArrayValue apply(@Nullable FieldValue previousValue) {
125-
List<FieldValue> result = coercedFieldValuesArray(previousValue);
126-
for (FieldValue element : getElements()) {
127-
result.removeAll(Collections.singleton(element));
127+
List<Value> result = coercedFieldValuesArray(previousValue);
128+
for (Value removeElement : getElements()) {
129+
Iterator<Value> existingValuesIterator = result.iterator();
130+
while (existingValuesIterator.hasNext()) {
131+
Value existingElement = existingValuesIterator.next();
132+
if (ProtoValues.equals(existingElement, removeElement)) {
133+
existingValuesIterator.remove();
134+
}
135+
}
128136
}
129137
return ArrayValue.fromList(result);
130138
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@ public class ArrayValue extends FieldValue {
2424
super(value);
2525
}
2626

27-
public static ArrayValue fromList(List<FieldValue> list) {
27+
public static ArrayValue fromList(List<Value> list) {
2828
com.google.firestore.v1.ArrayValue.Builder builder =
2929
com.google.firestore.v1.ArrayValue.newBuilder();
30-
for (FieldValue value : list) {
31-
builder.addValues(value.getProto());
30+
for (Value value : list) {
31+
builder.addValues(value);
3232
}
3333
return new ArrayValue(Value.newBuilder().setArrayValue(builder).build());
3434
}

‎firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteSerializer.java

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -390,13 +390,13 @@ private DocumentTransform.FieldTransform encodeFieldTransform(FieldTransform fie
390390
ArrayTransformOperation.Union union = (ArrayTransformOperation.Union) transform;
391391
return DocumentTransform.FieldTransform.newBuilder()
392392
.setFieldPath(fieldTransform.getFieldPath().canonicalString())
393-
.setAppendMissingElements(encodeArrayTransformElements(union.getElements()))
393+
.setAppendMissingElements(ArrayValue.newBuilder().addAllValues(union.getElements()))
394394
.build();
395395
} else if (transform instanceof ArrayTransformOperation.Remove) {
396396
ArrayTransformOperation.Remove remove = (ArrayTransformOperation.Remove) transform;
397397
return DocumentTransform.FieldTransform.newBuilder()
398398
.setFieldPath(fieldTransform.getFieldPath().canonicalString())
399-
.setRemoveAllFromArray(encodeArrayTransformElements(remove.getElements()))
399+
.setRemoveAllFromArray(ArrayValue.newBuilder().addAllValues(remove.getElements()))
400400
.build();
401401
} else if (transform instanceof NumericIncrementTransformOperation) {
402402
NumericIncrementTransformOperation incrementOperation =
@@ -410,14 +410,6 @@ private DocumentTransform.FieldTransform encodeFieldTransform(FieldTransform fie
410410
}
411411
}
412412

413-
private ArrayValue encodeArrayTransformElements(List<FieldValue> elements) {
414-
ArrayValue.Builder result = ArrayValue.newBuilder();
415-
for (FieldValue element : elements) {
416-
result.addValues(element.getProto());
417-
}
418-
return result.build();
419-
}
420-
421413
private FieldTransform decodeFieldTransform(DocumentTransform.FieldTransform fieldTransform) {
422414
switch (fieldTransform.getTransformTypeCase()) {
423415
case SET_TO_SERVER_VALUE:
@@ -433,14 +425,12 @@ private FieldTransform decodeFieldTransform(DocumentTransform.FieldTransform fie
433425
return new FieldTransform(
434426
FieldPath.fromServerFormat(fieldTransform.getFieldPath()),
435427
new ArrayTransformOperation.Union(
436-
decodeArrayTransformElements(
437-
fieldTransform.getAppendMissingElements().getValuesList())));
428+
fieldTransform.getAppendMissingElements().getValuesList()));
438429
case REMOVE_ALL_FROM_ARRAY:
439430
return new FieldTransform(
440431
FieldPath.fromServerFormat(fieldTransform.getFieldPath()),
441432
new ArrayTransformOperation.Remove(
442-
decodeArrayTransformElements(
443-
fieldTransform.getRemoveAllFromArray().getValuesList())));
433+
fieldTransform.getRemoveAllFromArray().getValuesList()));
444434
case INCREMENT:
445435
{
446436
FieldValue operand = FieldValue.valueOf(fieldTransform.getIncrement());
@@ -458,14 +448,6 @@ private FieldTransform decodeFieldTransform(DocumentTransform.FieldTransform fie
458448
}
459449
}
460450

461-
private List<FieldValue> decodeArrayTransformElements(List<Value> elementsProto) {
462-
List<FieldValue> result = new ArrayList<>(elementsProto.size());
463-
for (Value element : elementsProto) {
464-
result.add(FieldValue.valueOf(element));
465-
}
466-
return result;
467-
}
468-
469451
public MutationResult decodeMutationResult(
470452
com.google.firestore.v1.WriteResult proto, SnapshotVersion commitVersion) {
471453
// NOTE: Deletes don't have an updateTime but the commit timestamp from the containing

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import static com.google.firebase.firestore.testutil.TestUtil.blob;
1818
import static com.google.firebase.firestore.testutil.TestUtil.map;
1919
import static com.google.firebase.firestore.testutil.TestUtil.ref;
20+
import static com.google.firebase.firestore.testutil.TestUtil.valueOf;
2021
import static com.google.firebase.firestore.testutil.TestUtil.wrap;
2122
import static com.google.firebase.firestore.testutil.TestUtil.wrapObject;
2223
import static java.util.Arrays.asList;
@@ -270,8 +271,7 @@ public void testConvertsNestedObjects() {
270271

271272
@Test
272273
public void testConvertsLists() {
273-
ArrayValue expected =
274-
ArrayValue.fromList(asList(StringValue.valueOf("value"), BooleanValue.valueOf(true)));
274+
ArrayValue expected = ArrayValue.fromList(asList(valueOf("value"), valueOf(true)));
275275
FieldValue actual = wrap(asList("value", true));
276276
assertEquals(expected, actual);
277277
}

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import static com.google.firebase.firestore.testutil.TestUtil.key;
2222
import static com.google.firebase.firestore.testutil.TestUtil.map;
2323
import static com.google.firebase.firestore.testutil.TestUtil.ref;
24+
import static com.google.firebase.firestore.testutil.TestUtil.valueOf;
2425
import static com.google.firebase.firestore.testutil.TestUtil.wrap;
2526
import static com.google.firebase.firestore.testutil.TestUtil.wrapObject;
2627
import static junit.framework.TestCase.assertTrue;
@@ -128,12 +129,7 @@ public void testAddsNewFields() {
128129
public void testAddsMultipleNewFields() {
129130
ObjectValue object = ObjectValue.emptyObject();
130131
object = setField(object, "a", wrap("a"));
131-
object =
132-
object
133-
.toBuilder()
134-
.set(field("b"), wrap("b").getProto())
135-
.set(field("c"), wrap("c").getProto())
136-
.build();
132+
object = object.toBuilder().set(field("b"), valueOf("b")).set(field("c"), valueOf("c")).build();
137133

138134
assertEquals(wrapObject("a", "a", "b", "b", "c", "c"), object);
139135
}

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import static com.google.firebase.firestore.testutil.TestUtil.setMutation;
2727
import static com.google.firebase.firestore.testutil.TestUtil.transformMutation;
2828
import static com.google.firebase.firestore.testutil.TestUtil.unknownDoc;
29+
import static com.google.firebase.firestore.testutil.TestUtil.valueOf;
2930
import static com.google.firebase.firestore.testutil.TestUtil.version;
3031
import static com.google.firebase.firestore.testutil.TestUtil.wrap;
3132
import static com.google.firebase.firestore.testutil.TestUtil.wrapObject;
@@ -317,14 +318,14 @@ public void testCreateArrayUnionTransform() {
317318
FieldTransform first = transform.getFieldTransforms().get(0);
318319
assertEquals(field("a"), first.getFieldPath());
319320
assertEquals(
320-
new ArrayTransformOperation.Union(Collections.singletonList(wrap("tag"))),
321+
new ArrayTransformOperation.Union(Collections.singletonList(valueOf("tag"))),
321322
first.getOperation());
322323

323324
FieldTransform second = transform.getFieldTransforms().get(1);
324325
assertEquals(field("bar.baz"), second.getFieldPath());
325326
assertEquals(
326327
new ArrayTransformOperation.Union(
327-
Arrays.asList(wrap(true), wrapObject(map("nested", map("a", Arrays.asList(1, 2)))))),
328+
Arrays.asList(valueOf(true), valueOf(map("nested", map("a", Arrays.asList(1, 2)))))),
328329
second.getOperation());
329330
}
330331

@@ -340,7 +341,7 @@ public void testCreateArrayRemoveTransform() {
340341
FieldTransform first = transform.getFieldTransforms().get(0);
341342
assertEquals(field("foo"), first.getFieldPath());
342343
assertEquals(
343-
new ArrayTransformOperation.Remove(Collections.singletonList(wrap("tag"))),
344+
new ArrayTransformOperation.Remove(Collections.singletonList(valueOf("tag"))),
344345
first.getOperation());
345346
}
346347

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

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

1717
import static com.google.firebase.firestore.testutil.TestUtil.field;
1818
import static com.google.firebase.firestore.testutil.TestUtil.map;
19-
import static com.google.firebase.firestore.testutil.TestUtil.wrap;
19+
import static com.google.firebase.firestore.testutil.TestUtil.valueOf;
2020
import static com.google.firebase.firestore.testutil.TestUtil.wrapObject;
2121
import static junit.framework.TestCase.assertEquals;
2222

@@ -31,9 +31,9 @@
3131
@Config(manifest = Config.NONE)
3232
public class ObjectValueBuilderTest {
3333
private String fooString = "foo";
34-
private Value fooValue = wrap(fooString).getProto();
34+
private Value fooValue = valueOf(fooString);
3535
private String barString = "bar";
36-
private Value barValue = wrap(barString).getProto();
36+
private Value barValue = valueOf(barString);
3737
private Value emptyObject = ObjectValue.emptyObject().getProto();
3838

3939
@Test

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import static com.google.firebase.firestore.testutil.TestUtil.ref;
2828
import static com.google.firebase.firestore.testutil.TestUtil.setMutation;
2929
import static com.google.firebase.firestore.testutil.TestUtil.transformMutation;
30+
import static com.google.firebase.firestore.testutil.TestUtil.valueOf;
3031
import static com.google.firebase.firestore.testutil.TestUtil.verifyMutation;
3132
import static com.google.firebase.firestore.testutil.TestUtil.wrap;
3233
import static java.util.Arrays.asList;
@@ -435,8 +436,8 @@ public void testEncodesArrayTransformMutations() {
435436
.setFieldPath("a")
436437
.setAppendMissingElements(
437438
ArrayValue.newBuilder()
438-
.addValues(wrap("a").getProto())
439-
.addValues(wrap(2).getProto())))
439+
.addValues(valueOf("a"))
440+
.addValues(valueOf(2))))
440441
.addFieldTransforms(
441442
DocumentTransform.FieldTransform.newBuilder()
442443
.setFieldPath("bar.baz")

‎firebase-firestore/src/testUtil/java/com/google/firebase/firestore/testutil/TestUtil.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
import com.google.firebase.firestore.remote.WatchChange;
7171
import com.google.firebase.firestore.remote.WatchChange.DocumentChange;
7272
import com.google.firebase.firestore.remote.WatchChangeAggregator;
73+
import com.google.firestore.v1.Value;
7374
import com.google.protobuf.ByteString;
7475
import java.io.IOException;
7576
import java.util.ArrayList;
@@ -143,6 +144,10 @@ public static ObjectValue wrapObject(Object... entries) {
143144
return wrapObject(map(entries));
144145
}
145146

147+
public static Value valueOf(Object value) {
148+
return wrap(value).getProto();
149+
}
150+
146151
public static DocumentKey key(String key) {
147152
return DocumentKey.fromPathString(key);
148153
}

0 commit comments

Comments
 (0)