Merge pull request #18 from uber/tests

Improvements around empty data handling

build.gradle

@@ -4,23 +4,40 @@ buildscript {
     ext.kotlin_version = '1.3.20'
     repositories {
         google()
+        maven {
+            url "https://plugins.gradle.org/m2/"
+        }
         jcenter()
     }
     dependencies {
         classpath 'com.android.tools.build:gradle:3.3.1'
         classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version"
         classpath "com.diffplug.spotless:spotless-plugin-gradle:3.18.0"
-
-        // NOTE: Do not place your application dependencies here; they belong
-        // in the individual module build.gradle files
+        classpath 'com.google.protobuf:protobuf-gradle-plugin:0.8.8'
+        classpath "net.ltgt.gradle:gradle-errorprone-plugin:0.6"
     }
 }
 
+import net.ltgt.gradle.errorprone.CheckSeverity
 allprojects {
+    apply plugin: 'net.ltgt.errorprone'
     repositories {
         google()
         jcenter()
     }
+
+    dependencies {
+        errorprone "com.google.errorprone:error_prone_core:2.3.2"
+        errorproneJavac "com.google.errorprone:javac:9-dev-r4023-3"
+    }
+
+    tasks.withType(JavaCompile) {
+        options.errorprone {
+            excludedPaths=".*/build/generated/.*"
+            check("NullAway", CheckSeverity.ERROR)
+            option("NullAway:AnnotatedPackages", "com.uber")
+        }
+    }
 }
 
 task clean(type: Delete) {

gradle.properties

@@ -14,7 +14,7 @@ org.gradle.jvmargs=-Xmx1536m
 # Kotlin code style for this project: "official" or "obsolete":
 kotlin.code.style=official
 
-VERSION_NAME=0.0.2
+VERSION_NAME=0.0.3
 VERSION_CODE=2
 GROUP=com.uber.simplestore
 

protosimplestore/build.gradle

@@ -1,4 +1,5 @@
 apply plugin: 'com.android.library'
+apply plugin: 'com.google.protobuf'
 apply plugin: "com.diffplug.gradle.spotless"
 
 android {
@@ -34,13 +35,39 @@ spotless {
     }
 }
 
+protobuf {
+    protoc {
+        artifact = 'com.google.protobuf:protoc:3.0.0'
+    }
+    plugins {
+        javalite {
+            artifact = 'com.google.protobuf:protoc-gen-javalite:3.0.0'
+        }
+    }
+    generateProtoTasks {
+        all().each { task ->
+            task.builtins {
+                remove java
+            }
+            task.plugins {
+                javalite { }
+            }
+        }
+    }
+}
+
 dependencies {
     implementation fileTree(dir: 'libs', include: ['*.jar'])
 
     implementation project(":simplestore")
     implementation 'com.google.guava:guava:27.0.1-android'
     implementation 'com.google.protobuf:protobuf-lite:3.0.1'
     implementation 'com.android.support:appcompat-v7:28.0.0'
+    annotationProcessor "com.uber.nullaway:nullaway:0.6.4"
+    testAnnotationProcessor "com.uber.nullaway:nullaway:0.6.4"
+
+    testImplementation "com.google.truth:truth:0.42"
+    testImplementation "org.robolectric:robolectric:4.1"
     testImplementation 'junit:junit:4.12'
     androidTestImplementation 'com.android.support.test:runner:1.0.2'
     androidTestImplementation 'com.android.support.test.espresso:espresso-core:3.0.2'

protosimplestore/src/main/java/com/uber/simplestore/proto/SimpleProtoStore.java

@@ -4,10 +4,13 @@
 import com.google.protobuf.MessageLite;
 import com.google.protobuf.Parser;
 import com.uber.simplestore.SimpleStore;
+import javax.annotation.CheckReturnValue;
 import javax.annotation.Nullable;
 
 public interface SimpleProtoStore extends SimpleStore {
+  @CheckReturnValue
   <T extends MessageLite> ListenableFuture<T> get(String key, Parser<T> parser);
 
+  @CheckReturnValue
   <T extends MessageLite> ListenableFuture<T> put(String key, @Nullable T value);
 }

protosimplestore/src/main/java/com/uber/simplestore/proto/impl/SimpleProtoStoreFactory.java

@@ -0,0 +1,21 @@
+package com.uber.simplestore.proto.impl;
+
+import android.content.Context;
+import com.uber.simplestore.ScopeConfig;
+import com.uber.simplestore.impl.SimpleStoreFactory;
+import com.uber.simplestore.proto.SimpleProtoStore;
+
+/**
+ * Obtain an instance of a storage scope with proto support. Only one instance per scope may exist
+ * at any time.
+ */
+public final class SimpleProtoStoreFactory {
+
+  public static SimpleProtoStore create(Context context, String scope) {
+    return create(context, scope, ScopeConfig.DEFAULT);
+  }
+
+  public static SimpleProtoStore create(Context context, String scope, ScopeConfig config) {
+    return new SimpleProtoStoreImpl(SimpleStoreFactory.create(context, scope, config), config);
+  }
+}

protosimplestore/src/main/java/com/uber/simplestore/proto/impl/SimpleProtoStoreImpl.java

@@ -1,35 +1,53 @@
 package com.uber.simplestore.proto.impl;
 
-import android.content.Context;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
+import com.google.protobuf.ByteString;
+import com.google.protobuf.InvalidProtocolBufferException;
 import com.google.protobuf.MessageLite;
 import com.google.protobuf.Parser;
 import com.uber.simplestore.ScopeConfig;
 import com.uber.simplestore.SimpleStore;
 import com.uber.simplestore.SimpleStoreConfig;
-import com.uber.simplestore.impl.SimpleStoreFactory;
 import com.uber.simplestore.proto.SimpleProtoStore;
 import javax.annotation.Nullable;
 
 @SuppressWarnings("UnstableApiUsage")
 public final class SimpleProtoStoreImpl implements SimpleProtoStore {
   private final SimpleStore simpleStore;
+  private final ScopeConfig config;
 
-  private SimpleProtoStoreImpl(SimpleStore simpleStore) {
+  SimpleProtoStoreImpl(SimpleStore simpleStore, ScopeConfig config) {
     this.simpleStore = simpleStore;
-  }
-
-  public static SimpleProtoStoreImpl create(Context context, String scope, ScopeConfig config) {
-    return new SimpleProtoStoreImpl(SimpleStoreFactory.create(context, scope, config));
+    this.config = config;
   }
 
   @Override
   public <T extends MessageLite> ListenableFuture<T> get(String key, Parser<T> parser) {
     return Futures.transformAsync(
         simpleStore.get(key),
         (bytes) -> {
-          T parsed = parser.parseFrom(bytes);
+          T parsed;
+          if (bytes == null || bytes.length == 0) {
+            try {
+              parsed = parser.parseFrom(ByteString.EMPTY);
+            } catch (InvalidProtocolBufferException e) {
+              // Has required fields, so we will pass this error forward.
+              return Futures.immediateFailedFuture(e);
+            }
+          } else {
+            try {
+              parsed = parser.parseFrom(bytes);
+            } catch (InvalidProtocolBufferException e) {
+              if (config.equals(ScopeConfig.CACHE)) {
+                // A cache is allowed to be cleared whenever and we will try and give you a default
+                // instance instead.
+                return Futures.immediateFuture(parser.parseFrom(ByteString.EMPTY));
+              } else {
+                return Futures.immediateFailedFuture(e);
+              }
+            }
+          }
           return Futures.immediateFuture(parsed);
         },
         SimpleStoreConfig.getComputationExecutor());
@@ -41,7 +59,7 @@ public <T extends MessageLite> ListenableFuture<T> put(String key, @Nullable T v
         Futures.submitAsync(
             () -> {
               byte[] bytes = null;
-              if (value != null) {
+              if (value != null && !value.equals(value.getDefaultInstanceForType())) {
                 bytes = value.toByteArray();
               }
               return Futures.immediateFuture(bytes);
@@ -55,6 +73,14 @@ public <T extends MessageLite> ListenableFuture<T> put(String key, @Nullable T v
         SimpleStoreConfig.getComputationExecutor());
   }
 
+  @Override
+  public ListenableFuture<Boolean> contains(String key) {
+    return Futures.transform(
+        get(key),
+        value -> value != null && value.length > 0,
+        SimpleStoreConfig.getComputationExecutor());
+  }
+
   @Override
   public ListenableFuture<String> getString(String key) {
     return simpleStore.getString(key);

protosimplestore/src/test/java/com/uber/simplestore/proto/SimpleProtoStoreImplTest.java

@@ -0,0 +1,106 @@
+package com.uber.simplestore.proto;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.fail;
+
+import android.content.Context;
+import com.google.common.util.concurrent.Futures;
+import com.google.common.util.concurrent.ListenableFuture;
+import com.google.protobuf.InvalidProtocolBufferException;
+import com.uber.simplestore.ScopeConfig;
+import com.uber.simplestore.SimpleStore;
+import com.uber.simplestore.proto.impl.SimpleProtoStoreFactory;
+import com.uber.simplestore.proto.test.TestProto;
+import java.nio.charset.Charset;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.RuntimeEnvironment;
+import org.robolectric.annotation.Config;
+
+@SuppressWarnings("UnstableApiUsage")
+@RunWith(RobolectricTestRunner.class)
+@Config(manifest = Config.NONE)
+public class SimpleProtoStoreImplTest {
+  private static final String TEST_KEY = "test";
+  private static final String FOO = "foo";
+  private Context context = RuntimeEnvironment.systemContext;
+
+  @Test
+  public void defaultInstanceWhenEmpty() throws Exception {
+    try (SimpleProtoStore store = SimpleProtoStoreFactory.create(context, "")) {
+      ListenableFuture<TestProto.Basic> future = store.get(TEST_KEY, TestProto.Basic.parser());
+      assertThat(future.get()).isNotNull();
+      assertThat(future.get()).isEqualTo(TestProto.Basic.getDefaultInstance());
+    }
+  }
+
+  @Test
+  public void defaultInstanceWhenEmpty_withRequiredField() throws Exception {
+    try (SimpleProtoStore store = SimpleProtoStoreFactory.create(context, "")) {
+      ListenableFuture<TestProto.Required> future =
+          store.get(TEST_KEY, TestProto.Required.parser());
+      try {
+        Futures.getChecked(future, InvalidProtocolBufferException.class);
+        fail();
+      } catch (InvalidProtocolBufferException e) {
+        // expected
+        assertThat(e).hasMessageThat().contains("Message was missing required fields.");
+      }
+    }
+  }
+
+  @Test
+  public void savesDefaultInstance() throws Exception {
+    TestProto.Basic basic = TestProto.Basic.getDefaultInstance();
+    try (SimpleProtoStore store = SimpleProtoStoreFactory.create(context, "")) {
+      store.put(TEST_KEY, basic).get();
+    }
+    try (SimpleProtoStore store = SimpleProtoStoreFactory.create(context, "")) {
+      TestProto.Basic out = store.get(TEST_KEY, TestProto.Basic.parser()).get();
+      assertThat(out).isEqualTo(basic);
+    }
+  }
+
+  @Test
+  public void savesValue() throws Exception {
+    TestProto.Basic basic = TestProto.Basic.newBuilder().setName(FOO).build();
+    try (SimpleProtoStore store = SimpleProtoStoreFactory.create(context, "")) {
+      store.put(TEST_KEY, basic).get();
+    }
+    try (SimpleProtoStore store = SimpleProtoStoreFactory.create(context, "")) {
+      TestProto.Basic out = store.get(TEST_KEY, TestProto.Basic.parser()).get();
+      assertThat(out).isEqualTo(basic);
+    }
+  }
+
+  @Test
+  public void failsGracefullyOnParsingFail() throws Exception {
+    try (SimpleStore simpleStore = SimpleProtoStoreFactory.create(context, "")) {
+      simpleStore.put(TEST_KEY, "garbage".getBytes(Charset.defaultCharset())).get();
+    }
+
+    try (SimpleProtoStore store = SimpleProtoStoreFactory.create(context, "")) {
+      ListenableFuture<TestProto.Basic> out = store.get(TEST_KEY, TestProto.Basic.parser());
+      try {
+        Futures.getChecked(out, InvalidProtocolBufferException.class);
+        fail();
+      } catch (InvalidProtocolBufferException e) {
+        // expected
+        assertThat(e).hasMessageThat().contains("invalid wire type");
+      }
+    }
+  }
+
+  @Test
+  public void whenCache_returnsDefaultOnParseFailure() throws Exception {
+    try (SimpleStore simpleStore = SimpleProtoStoreFactory.create(context, "")) {
+      simpleStore.put(TEST_KEY, "garbage".getBytes(Charset.defaultCharset())).get();
+    }
+
+    try (SimpleProtoStore store = SimpleProtoStoreFactory.create(context, "", ScopeConfig.CACHE)) {
+      TestProto.Basic failed = store.get(TEST_KEY, TestProto.Basic.parser()).get();
+      assertThat(failed).isEqualTo(TestProto.Basic.getDefaultInstance());
+    }
+  }
+}