[BEAM-698] Use AutoValue in MongoDB GridFS

[jbonofre]

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

• Make sure the PR title is formatted like:
  [BEAM-<Jira issue #>] Description of pull request
• Make sure tests pass via mvn clean verify. (Even better, enable
  Travis-CI on your fork and ensure the whole test matrix passes).
• Replace <Jira issue #> in the title with the actual Jira issue
  number, if there is one.
• If this contribution is large, please file an Apache
  Individual Contributor License Agreement.

---

[jbonofre]

R: @jkff
CC: @dkulp

[dkulp]

I'll do a more thorough review later, but two quick comments:

1. With the builder pattern, it's possible that the required Parser is null. This should be checked in the validate method.

2. I'm not exactly "happy" about making the common Read case more complex. I understand the AutoValue/builder pattern might be limiting some flexibility there. I'll try and play around with a few things later. One thought is to have two Read methods:

    Read<String> read()
    Read<T> read(Parser<T> p)

since the parser is required for the non-string case.

[jbonofre]

Good point.

Using two read methods with different signature can work for sure.

[dkulp]

I believe something like:

   /** Read data from GridFS. */
-  public static <T> Read<T> read() {
-    return new AutoValue_MongoDbGridFSIO_Read.Builder<T>().build();
-  }

+  public static Read<String> read() {
+    return new AutoValue_MongoDbGridFSIO_Read.Builder<String>().build()
+        .withParser(TEXT_PARSER).withCoder(StringUtf8Coder.of());
+  }
   /**
    * A {@link PTransform} to read data from MongoDB GridFS.
    */
@@ -185,9 +187,11 @@ public class MongoDbGridFSIO {
       return toBuilder().setBucket(bucket).build();
     }

-    public Read<T> withParser(Parser<T> parser) {
+    public <X> Read<X> withParser(Parser<X> parser) {
       checkNotNull(parser);
-      return toBuilder().setParser(parser).build();
+      @SuppressWarnings("unchecked")
+      Builder<X> builder = (Builder<X>) toBuilder();
+      return builder.setParser(parser).setCoder(null).build();
     }

would also work. The unchecked cast is slightly annoying, but would work.

[dkulp]

LGTM

I'll submit a patch for the eclipse warnings (as there are some other eclipse warnings in the code that can all be fixed at once)

[dkulp]

minor: this generates a warning in eclipse as the <read> is not needed. The other two tests have a similar issue.

[jbonofre]

OK. I will push then you can provide a patch to fix the eclipse warning (I didn't have any warning with IntelliJ). Thanks.