yixiaoshen commented on code in PR #22175:
URL: https://github.com/apache/beam/pull/22175#discussion_r915191331
##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/firestore/FirestoreV1FnRunQueryTest.java:
##########
@@ -283,62 +292,86 @@ protected RunQueryFn getFn(
private static final class TestData {
+ static final FieldReference FILTER_FIELD_PATH =
FieldReference.newBuilder().setFieldPath("foo")
+ .build();
+
private final RunQueryRequest request;
private final RunQueryResponse response1;
private final RunQueryResponse response2;
private final RunQueryResponse response3;
- public TestData(String projectId, Function<FieldReference, List<Order>>
orderFunction) {
- String fieldPath = "foo";
- FieldReference foo =
FieldReference.newBuilder().setFieldPath(fieldPath).build();
+ public TestData(String projectId, Function<FieldReference, List<Order>>
orderFunction,
+ Filter filter) {
StructuredQuery.Builder builder =
StructuredQuery.newBuilder()
.addFrom(
CollectionSelector.newBuilder()
.setAllDescendants(false)
.setCollectionId("collection"))
- .setWhere(
- Filter.newBuilder()
- .setFieldFilter(
- FieldFilter.newBuilder()
- .setField(foo)
- .setOp(Operator.EQUAL)
-
.setValue(Value.newBuilder().setStringValue("bar"))
- .build()));
-
- orderFunction.apply(foo).forEach(builder::addOrderBy);
+ .setWhere(filter);
+
+ orderFunction.apply(FILTER_FIELD_PATH).forEach(builder::addOrderBy);
request =
RunQueryRequest.newBuilder()
.setParent(String.format("projects/%s/databases/(default)/document", projectId))
.setStructuredQuery(builder)
.build();
- response1 = newResponse(fieldPath, 1);
- response2 = newResponse(fieldPath, 2);
- response3 = newResponse(fieldPath, 3);
+ response1 = newResponse(1);
+ response2 = newResponse(2);
+ response3 = newResponse(3);
}
- private static RunQueryResponse newResponse(String field, int docNumber) {
+ /**
+ * Returns single-document response like this:
+ * {
+ * "__name__": "doc-{docNumber}"
+ * "foo": "bar"
+ * "foobar": {"bar": "foo"}
+ * }
+ */
+ private static RunQueryResponse newResponse(int docNumber) {
String docId = String.format("doc-%d", docNumber);
return RunQueryResponse.newBuilder()
.setDocument(
Document.newBuilder()
.setName(docId)
.putAllFields(
- ImmutableMap.of(field,
Value.newBuilder().setStringValue("bar").build()))
- .build())
+ ImmutableMap.of(
+ "foo",
Value.newBuilder().setStringValue("bar").build(),
+ "foobar",
Value.newBuilder().setMapValue(MapValue.newBuilder()
+ .putFields("bar",
Value.newBuilder().setStringValue("foo").build())
+ .build()).build()
+ )))
.build();
}
private static Builder fieldEqualsBar() {
- return new Builder();
+ return new Builder().setFilter(Filter.newBuilder()
+ .setFieldFilter(
+ FieldFilter.newBuilder()
+ .setField(FILTER_FIELD_PATH)
+ .setOp(Operator.EQUAL)
+ .setValue(Value.newBuilder().setStringValue("bar"))
Review Comment:
maybe just declare two Filter constants FOO_EQUALS_BAR and
FOO_NOT_EQUALS_FOO around line 297? there's no argument to these two methods so
everytime these methods get called they just return the same filter, constants
will do just as good as methods.
##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/firestore/FirestoreV1FnRunQueryTest.java:
##########
@@ -117,65 +119,62 @@ public void resumeFromLastReadValue() throws Exception {
.setStartAt(
Cursor.newBuilder()
.setBefore(false)
-
.addValues(Value.newBuilder().setStringValue("bar"))))
+
.addValues(Value.newBuilder().setStringValue("bar"))
+ .addValues(
+ Value.newBuilder()
+
.setReferenceValue(testData.response2.getDocument().getName()))
+ )
+ .addOrderBy(
+ Order.newBuilder()
+
.setField(FieldReference.newBuilder().setFieldPath("__name__"))
+ .setDirection(Direction.ASCENDING))
+ )
.build();
- List<RunQueryResponse> responses =
- ImmutableList.of(testData.response1, testData.response2,
testData.response3);
- when(responseStream1.iterator())
- .thenReturn(
- new AbstractIterator<RunQueryResponse>() {
- private int invocationCount = 1;
-
- @Override
- protected RunQueryResponse computeNext() {
- int count = invocationCount++;
- if (count == 1) {
- return responses.get(0);
- } else if (count == 2) {
- return responses.get(1);
- } else {
- throw RETRYABLE_ERROR;
- }
- }
- });
-
- when(callable.call(testData.request)).thenReturn(responseStream1);
- doNothing().when(attempt).checkCanRetry(any(), eq(RETRYABLE_ERROR));
-
when(responseStream2.iterator()).thenReturn(ImmutableList.of(responses.get(2)).iterator());
- when(callable.call(request2)).thenReturn(responseStream2);
-
- when(stub.runQueryCallable()).thenReturn(callable);
-
- when(ff.getFirestoreStub(any())).thenReturn(stub);
- when(ff.getRpcQos(any())).thenReturn(rpcQos);
- when(rpcQos.newReadAttempt(any())).thenReturn(attempt);
- when(attempt.awaitSafeToProceed(any())).thenReturn(true);
-
- ArgumentCaptor<RunQueryResponse> responsesCaptor =
- ArgumentCaptor.forClass(RunQueryResponse.class);
-
- doNothing().when(processContext).output(responsesCaptor.capture());
-
- when(processContext.element()).thenReturn(testData.request);
-
- RunQueryFn fn = new RunQueryFn(clock, ff, rpcQosOptions);
-
- runFunction(fn);
+ runQueryRetryTest(testData, expectedRetryRequest);
+ }
- List<RunQueryResponse> allValues = responsesCaptor.getAllValues();
- assertEquals(responses, allValues);
+ @Test
+ public void resumeFromLastReadValue_nestedOrderBy() throws Exception {
Review Comment:
maybe also add a test for backticked fields, e.g. the query has an
inequality on field "\`x.y\`" or has an order by on field "\`x.y\`", and also
add a field "x.y" to the returned document in newResponse() to support this
test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
