spanner: Options: Fix null dereference, expand test coverage (googlea…

…pis#3706)

The equals code does not handle the comparison of two Options objects
where only one of them has either limit, pageSize or prefetchChunks
defined. The argument to Object.equals() takes for instance limit() and
this tries to deref the null to convert it to a long.

Add hasXXX() checks before calling Object.equals().

This patch also removes functions that will not be called since they are
internal.

This expands coverage of Options to 100%.

google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java

@@ -110,10 +110,6 @@ static final class FlowControlOption extends InternalOption implements ReadAndQu
       this.prefetchChunks = prefetchChunks;
     }
 
-    int prefetchChunks() {
-      return prefetchChunks;
-    }
-
     @Override
     void appendToOptions(Options options) {
       options.prefetchChunks = prefetchChunks;
@@ -202,10 +198,13 @@ public boolean equals(Object o) {
     }
 
     Options that = (Options) o;
-    return (!hasLimit() && !that.hasLimit() || Objects.equals(limit(), that.limit()))
+    return (!hasLimit() && !that.hasLimit()
+            || hasLimit() && that.hasLimit() && Objects.equals(limit(), that.limit()))
         && (!hasPrefetchChunks() && !that.hasPrefetchChunks()
-            || Objects.equals(prefetchChunks(), that.prefetchChunks()))
-        && (!hasPageSize() && !that.hasPageSize() || Objects.equals(pageSize(), that.pageSize()))
+            || hasPrefetchChunks() && that.hasPrefetchChunks()
+                && Objects.equals(prefetchChunks(), that.prefetchChunks()))
+        && (!hasPageSize() && !that.hasPageSize()
+            || hasPageSize() && that.hasPageSize() && Objects.equals(pageSize(), that.pageSize()))
         && Objects.equals(pageToken(), that.pageToken())
         && Objects.equals(filter(), that.filter());
   }
@@ -266,10 +265,6 @@ static class LimitOption extends InternalOption implements ReadOption {
       this.limit = limit;
     }
 
-    long limit() {
-      return limit;
-    }
-
     @Override
     void appendToOptions(Options options) {
       options.limit = limit;
@@ -283,10 +278,6 @@ static class PageSizeOption extends InternalOption implements ListOption {
       this.pageSize = pageSize;
     }
 
-    int pageSize() {
-      return pageSize;
-    }
-
     @Override
     void appendToOptions(Options options) {
       options.pageSize = pageSize;
@@ -300,10 +291,6 @@ static class PageTokenOption extends InternalOption implements ListOption {
       this.pageToken = pageToken;
     }
 
-    String pageToken() {
-      return pageToken;
-    }
-
     @Override
     void appendToOptions(Options options) {
       options.pageToken = pageToken;

google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java

@@ -67,5 +67,135 @@ public void allOptionsAbsent() {
     Options options = Options.fromReadOptions();
     assertThat(options.hasLimit()).isFalse();
     assertThat(options.hasPrefetchChunks()).isFalse();
+    assertThat(options.hasFilter()).isFalse();
+    assertThat(options.hasPageToken()).isFalse();
+    assertThat(options.toString()).isEqualTo("");
+    assertThat(options.equals(options)).isTrue();
+    assertThat(options.equals(null)).isFalse();
+    assertThat(options.equals(this)).isFalse();
+
+    assertThat(options.hashCode()).isEqualTo(31);
+  }
+
+  @Test
+  public void listOptTest() {
+    int pageSize = 3;
+    String pageToken = "ptok";
+    String filter = "env";
+    Options opts = Options
+        .fromListOptions(
+            Options.pageSize(pageSize), Options.pageToken(pageToken), Options.filter(filter));
+
+    assertThat(opts.toString()).isEqualTo("pageSize: " + Integer.toString(pageSize)
+        + " pageToken: " + pageToken + " filter: " + filter + " ");
+
+    assertThat(opts.hasPageSize()).isTrue();
+    assertThat(opts.hasPageToken()).isTrue();
+    assertThat(opts.hasFilter()).isTrue();
+
+    assertThat(opts.pageSize()).isEqualTo(pageSize);
+    assertThat(opts.pageToken()).isEqualTo(pageToken);
+    assertThat(opts.filter()).isEqualTo(filter);
+    assertThat(opts.hashCode()).isEqualTo(108027089);
+  }
+
+  @Test
+  public void listEquality() {
+    Options o1;
+    Options o2;
+    Options o3;
+
+    o1 = Options.fromListOptions();
+    o2 = Options.fromListOptions();
+    assertThat(o1.equals(o2)).isTrue();
+
+    o2 = Options.fromListOptions(Options.pageSize(1));
+    assertThat(o1.equals(o2)).isFalse();
+    assertThat(o2.equals(o1)).isFalse();
+
+    o3 = Options.fromListOptions(Options.pageSize(1));
+    assertThat(o2.equals(o3)).isTrue();
+
+    o3 = Options.fromListOptions(Options.pageSize(2));
+    assertThat(o2.equals(o3)).isFalse();
+
+    o2 = Options.fromListOptions(Options.pageToken("t1"));
+    assertThat(o1.equals(o2)).isFalse();
+
+    o3 = Options.fromListOptions(Options.pageToken("t1"));
+    assertThat(o2.equals(o3)).isTrue();
+
+    o3 = Options.fromListOptions(Options.pageToken("t2"));
+    assertThat(o2.equals(o3)).isFalse();
+
+    o2 = Options.fromListOptions(Options.filter("f1"));
+    assertThat(o1.equals(o2)).isFalse();
+
+    o3 = Options.fromListOptions(Options.filter("f1"));
+    assertThat(o2.equals(o3)).isTrue();
+
+    o3 = Options.fromListOptions(Options.filter("f2"));
+    assertThat(o2.equals(o3)).isFalse();
+  }
+
+  @Test
+  public void readOptTest() {
+    int limit = 3;
+    Options opts = Options.fromReadOptions(Options.limit(limit));
+
+    assertThat(opts.toString()).isEqualTo("limit: " + Integer.toString(limit) + " ");
+    assertThat(opts.hashCode()).isEqualTo(964);
+  }
+
+  @Test
+  public void readEquality() {
+    Options o1;
+    Options o2;
+    Options o3;
+
+    o1 = Options.fromReadOptions();
+    o2 = Options.fromReadOptions();
+    assertThat(o1.equals(o2)).isTrue();
+
+    o2 = Options.fromReadOptions(Options.limit(1));
+    assertThat(o1.equals(o2)).isFalse();
+    assertThat(o2.equals(o1)).isFalse();
+
+    o3 = Options.fromReadOptions(Options.limit(1));
+    assertThat(o2.equals(o3)).isTrue();
+
+    o3 = Options.fromReadOptions(Options.limit(2));
+    assertThat(o2.equals(o3)).isFalse();
+  }
+
+  @Test
+  public void queryOptTest() {
+    int chunks = 3;
+    Options opts = Options.fromQueryOptions(Options.prefetchChunks(chunks));
+    assertThat(opts.toString())
+        .isEqualTo("prefetchChunks: " + Integer.toString(chunks) + " ");
+    assertThat(opts.prefetchChunks()).isEqualTo(chunks);
+    assertThat(opts.hashCode()).isEqualTo(964);
+  }
+
+  @Test
+  public void queryEquality() {
+    Options o1;
+    Options o2;
+    Options o3;
+
+    o1 = Options.fromQueryOptions();
+    o2 = Options.fromQueryOptions();
+    assertThat(o1.equals(o2)).isTrue();
+
+    o2 = Options.fromReadOptions(Options.prefetchChunks(1));
+    assertThat(o1.equals(o2)).isFalse();
+    assertThat(o2.equals(o1)).isFalse();
+
+    o3 = Options.fromReadOptions(Options.prefetchChunks(1));
+    assertThat(o2.equals(o3)).isTrue();
+
+    o3 = Options.fromReadOptions(Options.prefetchChunks(2));
+    assertThat(o2.equals(o3)).isFalse();
   }
 }