[GitHub] [beam] yixiaoshen commented on a diff in pull request #22175: [BEAM-22089] Fix query retry in Java FirestoreIO

[GitBox <gi...@apache.org>]

yixiaoshen commented on code in PR #22175:
URL: https://github.com/apache/beam/pull/22175#discussion_r915186619


##########
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"

Review Comment:
   nit: since the field names are using "foo", "foobar", "bar", I'd suggest changing field values to something else instead of "bar" and "foo" to avoid confusion between field names and field values.



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org