diff --git a/.idea/codeStyles/codeStyleConfig.xml b/.idea/codeStyles/codeStyleConfig.xml index 79ee123..3c617fb 100644 --- a/.idea/codeStyles/codeStyleConfig.xml +++ b/.idea/codeStyles/codeStyleConfig.xml @@ -1,5 +1,6 @@ \ No newline at end of file diff --git a/.idea/saveactions_settings.xml b/.idea/saveactions_settings.xml new file mode 100644 index 0000000..db0de01 --- /dev/null +++ b/.idea/saveactions_settings.xml @@ -0,0 +1,14 @@ + + + + + + \ No newline at end of file diff --git a/build.gradle b/build.gradle index b3218a2..b7b4e34 100644 --- a/build.gradle +++ b/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) { diff --git a/gradle.properties b/gradle.properties index 9a730e8..6dcb9be 100644 --- a/gradle.properties +++ b/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 diff --git a/protosimplestore/build.gradle b/protosimplestore/build.gradle index 87d4642..6a0307d 100644 --- a/protosimplestore/build.gradle +++ b/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,6 +35,27 @@ 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']) @@ -41,6 +63,11 @@ dependencies { 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' diff --git a/protosimplestore/src/androidTest/java/com/uber/simplestore/proto/ExampleInstrumentedTest.java b/protosimplestore/src/androidTest/java/com/uber/simplestore/proto/ExampleInstrumentedTest.java deleted file mode 100644 index c154580..0000000 --- a/protosimplestore/src/androidTest/java/com/uber/simplestore/proto/ExampleInstrumentedTest.java +++ /dev/null @@ -1,25 +0,0 @@ -package com.uber.simplestore.proto; - -import static org.junit.Assert.assertEquals; - -import android.content.Context; -import android.support.test.InstrumentationRegistry; -import android.support.test.runner.AndroidJUnit4; -import org.junit.Test; -import org.junit.runner.RunWith; - -/** - * Instrumented test, which will execute on an Android device. - * - * @see Testing documentation - */ -@RunWith(AndroidJUnit4.class) -public class ExampleInstrumentedTest { - @Test - public void useAppContext() { - // Context of the app under test. - Context appContext = InstrumentationRegistry.getTargetContext(); - - assertEquals("com.uber.simplestore.proto.test", appContext.getPackageName()); - } -} diff --git a/protosimplestore/src/main/java/com/uber/simplestore/proto/SimpleProtoStore.java b/protosimplestore/src/main/java/com/uber/simplestore/proto/SimpleProtoStore.java index eab60f0..28ab47d 100644 --- a/protosimplestore/src/main/java/com/uber/simplestore/proto/SimpleProtoStore.java +++ b/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 ListenableFuture get(String key, Parser parser); + @CheckReturnValue ListenableFuture put(String key, @Nullable T value); } diff --git a/protosimplestore/src/main/java/com/uber/simplestore/proto/impl/SimpleProtoStoreFactory.java b/protosimplestore/src/main/java/com/uber/simplestore/proto/impl/SimpleProtoStoreFactory.java new file mode 100644 index 0000000..2395b9b --- /dev/null +++ b/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); + } +} diff --git a/protosimplestore/src/main/java/com/uber/simplestore/proto/impl/SimpleProtoStoreImpl.java b/protosimplestore/src/main/java/com/uber/simplestore/proto/impl/SimpleProtoStoreImpl.java index a8fdc5e..c2e90eb 100644 --- a/protosimplestore/src/main/java/com/uber/simplestore/proto/impl/SimpleProtoStoreImpl.java +++ b/protosimplestore/src/main/java/com/uber/simplestore/proto/impl/SimpleProtoStoreImpl.java @@ -1,27 +1,25 @@ 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 @@ -29,7 +27,27 @@ public ListenableFuture get(String key, Parser par 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 ListenableFuture 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 ListenableFuture put(String key, @Nullable T v SimpleStoreConfig.getComputationExecutor()); } + @Override + public ListenableFuture contains(String key) { + return Futures.transform( + get(key), + value -> value != null && value.length > 0, + SimpleStoreConfig.getComputationExecutor()); + } + @Override public ListenableFuture getString(String key) { return simpleStore.getString(key); diff --git a/protosimplestore/src/test/java/com/uber/simplestore/proto/ExampleUnitTest.java b/protosimplestore/src/test/java/com/uber/simplestore/proto/ExampleUnitTest.java deleted file mode 100644 index 7edc2aa..0000000 --- a/protosimplestore/src/test/java/com/uber/simplestore/proto/ExampleUnitTest.java +++ /dev/null @@ -1,17 +0,0 @@ -package com.uber.simplestore.proto; - -import static org.junit.Assert.assertEquals; - -import org.junit.Test; - -/** - * Example local unit test, which will execute on the development machine (host). - * - * @see Testing documentation - */ -public class ExampleUnitTest { - @Test - public void addition_isCorrect() { - assertEquals(4, 2 + 2); - } -} diff --git a/protosimplestore/src/test/java/com/uber/simplestore/proto/SimpleProtoStoreImplTest.java b/protosimplestore/src/test/java/com/uber/simplestore/proto/SimpleProtoStoreImplTest.java new file mode 100644 index 0000000..d754e50 --- /dev/null +++ b/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 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 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 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()); + } + } +} diff --git a/protosimplestore/src/test/proto/TestProto.proto b/protosimplestore/src/test/proto/TestProto.proto new file mode 100644 index 0000000..cebc06f --- /dev/null +++ b/protosimplestore/src/test/proto/TestProto.proto @@ -0,0 +1,16 @@ +syntax = "proto2"; + +package uber.simplestore; + +option java_package = "com.uber.simplestore.proto.test"; +option java_outer_classname = "TestProto"; + +message Basic { + optional string name = 1; +} + +message Required { + optional string option = 1; + required string withDefault = 2 [default = "default"]; + required string noDefault = 3; +} \ No newline at end of file diff --git a/sample/build.gradle b/sample/build.gradle index 8081281..236deea 100644 --- a/sample/build.gradle +++ b/sample/build.gradle @@ -43,6 +43,8 @@ dependencies { implementation 'com.google.guava:guava:27.0.1-android' implementation project(":simplestore") implementation project(":protosimplestore") + annotationProcessor "com.uber.nullaway:nullaway:0.6.4" + testAnnotationProcessor "com.uber.nullaway:nullaway:0.6.4" testImplementation 'junit:junit:4.12' androidTestImplementation 'com.android.support.test:runner:1.0.2' diff --git a/simplestore/build.gradle b/simplestore/build.gradle index 7481ea4..2af35bd 100644 --- a/simplestore/build.gradle +++ b/simplestore/build.gradle @@ -39,6 +39,8 @@ dependencies { implementation 'com.google.code.findbugs:jsr305:3.0.2' implementation 'com.google.guava:guava:27.0.1-android' + 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' diff --git a/simplestore/src/main/java/com/uber/simplestore/ScopeConfig.java b/simplestore/src/main/java/com/uber/simplestore/ScopeConfig.java index 4854e61..3905563 100644 --- a/simplestore/src/main/java/com/uber/simplestore/ScopeConfig.java +++ b/simplestore/src/main/java/com/uber/simplestore/ScopeConfig.java @@ -5,7 +5,11 @@ public final class ScopeConfig { /** No-op currently. */ public static final ScopeConfig CRITICAL = new ScopeConfig(); - /** Use the cache directory. */ + /** + * Use the cache directory. + * + *

Hides errors due to data corruption by returning a miss. + */ public static final ScopeConfig CACHE = new ScopeConfig(); /** Default settings. */ diff --git a/simplestore/src/main/java/com/uber/simplestore/SimpleStore.java b/simplestore/src/main/java/com/uber/simplestore/SimpleStore.java index abee875..017529f 100644 --- a/simplestore/src/main/java/com/uber/simplestore/SimpleStore.java +++ b/simplestore/src/main/java/com/uber/simplestore/SimpleStore.java @@ -42,10 +42,20 @@ public interface SimpleStore extends Closeable { @CheckReturnValue ListenableFuture put(String key, @Nullable byte[] value); + /** + * Determine if a key exists in storage. + * + * @param key to check + * @return if key is set + */ + @CheckReturnValue + ListenableFuture contains(String key); + /** Delete all keys in this direct scope. */ @CheckReturnValue ListenableFuture deleteAll(); /** Fails all outstanding operations then releases the memory cache. */ + @Override void close(); } diff --git a/simplestore/src/main/java/com/uber/simplestore/SimpleStoreConfig.java b/simplestore/src/main/java/com/uber/simplestore/SimpleStoreConfig.java index 0a0d9d8..77f6c6b 100644 --- a/simplestore/src/main/java/com/uber/simplestore/SimpleStoreConfig.java +++ b/simplestore/src/main/java/com/uber/simplestore/SimpleStoreConfig.java @@ -13,9 +13,9 @@ public final class SimpleStoreConfig { private static final Object writeLock = new Object(); - @Nullable private static Executor ioExecutor; + @Nullable private static volatile Executor ioExecutor; - @Nullable private static Executor computationExecutor; + @Nullable private static volatile Executor computationExecutor; public static Executor getIOExecutor() { if (ioExecutor == null) { diff --git a/simplestore/src/main/java/com/uber/simplestore/impl/SimpleStoreImpl.java b/simplestore/src/main/java/com/uber/simplestore/impl/SimpleStoreImpl.java index 7dc2a2a..c0c5877 100644 --- a/simplestore/src/main/java/com/uber/simplestore/impl/SimpleStoreImpl.java +++ b/simplestore/src/main/java/com/uber/simplestore/impl/SimpleStoreImpl.java @@ -9,6 +9,7 @@ import java.io.File; import java.io.FileOutputStream; import java.io.IOException; +import java.nio.charset.Charset; import java.util.HashMap; import java.util.Map; import java.util.Objects; @@ -21,6 +22,7 @@ final class SimpleStoreImpl implements SimpleStore { private static final int OPEN = 0; private static final int CLOSED = 1; private static final int TOMBSTONED = 2; + private static final byte[] EMPTY_BYTES = new byte[0]; private final Context context; private final String scope; @@ -56,7 +58,7 @@ public ListenableFuture getString(String key) { get(key), (bytes) -> { if (bytes != null && bytes.length > 0) { - return new String(bytes); + return new String(bytes, Charset.defaultCharset()); } else { return null; } @@ -65,8 +67,8 @@ public ListenableFuture getString(String key) { } @Override - public ListenableFuture putString(String key, String value) { - byte[] bytes = value != null ? value.getBytes() : null; + public ListenableFuture putString(String key, @Nullable String value) { + byte[] bytes = value != null ? value.getBytes(Charset.defaultCharset()) : null; return Futures.transform(put(key, bytes), (b) -> value, MoreExecutors.directExecutor()); } @@ -87,11 +89,10 @@ public ListenableFuture get(String key) { } catch (IOException e) { return Futures.immediateFailedFuture(e); } - if (value == null) { - cache.remove(key); - } else { - cache.put(key, value); + if (value == null || value.length == 0) { + value = EMPTY_BYTES; } + cache.put(key, value); } return Futures.immediateFuture(value); }, @@ -106,9 +107,10 @@ public ListenableFuture put(String key, @Nullable byte[] value) { if (isClosed()) { return Futures.immediateFailedFuture(new StoreClosedException()); } - if (value == null) { - cache.remove(key); + if (value == null || value.length == 0) { + cache.put(key, EMPTY_BYTES); deleteFile(key); + return Futures.immediateFuture(EMPTY_BYTES); } else { cache.put(key, value); try { @@ -116,12 +118,21 @@ public ListenableFuture put(String key, @Nullable byte[] value) { } catch (IOException e) { return Futures.immediateFailedFuture(e); } + return Futures.immediateFuture(value); } - return Futures.immediateFuture(value); }, orderedIoExecutor); } + @Override + public ListenableFuture contains(String key) { + requireOpen(); + return Futures.transform( + get(key), + (value) -> value != null && value.length > 0, + SimpleStoreConfig.getComputationExecutor()); + } + @Override public ListenableFuture deleteAll() { requireOpen(); @@ -184,6 +195,7 @@ private void deleteFile(String key) { file.delete(); } + @Nullable private byte[] readFile(String key) throws IOException { File baseFile = new File(scopedDirectory, key); AtomicFile file = new AtomicFile(baseFile); diff --git a/simplestore/src/test/java/com/uber/simplestore/impl/SimpleStoreImplTest.java b/simplestore/src/test/java/com/uber/simplestore/impl/SimpleStoreImplTest.java index 3ae6ccb..c8a9335 100644 --- a/simplestore/src/test/java/com/uber/simplestore/impl/SimpleStoreImplTest.java +++ b/simplestore/src/test/java/com/uber/simplestore/impl/SimpleStoreImplTest.java @@ -32,10 +32,10 @@ public void reset() { } @Test - public void nullWhenMissing() throws Exception { + public void zeroLengthWhenMissing() throws Exception { try (SimpleStore store = SimpleStoreFactory.create(context, "")) { ListenableFuture future = store.get(TEST_KEY); - assertThat(future.get()).isNull(); + assertThat(future.get()).hasLength(0); } } @@ -44,7 +44,7 @@ public void puttingNullDeletesKey() throws Exception { try (SimpleStore store = SimpleStoreFactory.create(context, "")) { ListenableFuture first = store.put(TEST_KEY, new byte[1]); ListenableFuture second = store.put(TEST_KEY, null); - assertThat(second.get()).isNull(); + assertThat(second.get()).isEmpty(); } } @@ -54,7 +54,7 @@ public void deleteAll() throws Exception { ListenableFuture first = store.put(TEST_KEY, new byte[1]); ListenableFuture second = store.deleteAll(); ListenableFuture empty = store.get(TEST_KEY); - assertThat(empty.get()).isNull(); + assertThat(empty.get()).isEmpty(); } }