Report InternalFeedError when storage fails.
PiperOrigin-RevId: 258380186
Change-Id: I5abb4701f08686b8528f7ecde32165e194914906
diff --git a/src/main/java/com/google/android/libraries/feed/api/host/logging/InternalFeedError.java b/src/main/java/com/google/android/libraries/feed/api/host/logging/InternalFeedError.java
index 975bc81..22888ab 100644
--- a/src/main/java/com/google/android/libraries/feed/api/host/logging/InternalFeedError.java
+++ b/src/main/java/com/google/android/libraries/feed/api/host/logging/InternalFeedError.java
@@ -83,6 +83,12 @@
*/
int TASK_QUEUE_STARVATION = 12;
+ /** Represents a call to ContentStorage that returned with a missing item. */
+ int CONTENT_STORAGE_MISSING_ITEM = 13;
+
+ /** Represents bytes from ContentStorage that cannot be parsed into the proto. */
+ int ITEM_NOT_PARSED = 14;
+
/** The next value that should be used when adding additional values to the IntDef. */
- int NEXT_VALUE = 13;
+ int NEXT_VALUE = 15;
}
diff --git a/src/main/java/com/google/android/libraries/feed/feedstore/FeedStore.java b/src/main/java/com/google/android/libraries/feed/feedstore/FeedStore.java
index 9574229..9cec44f 100644
--- a/src/main/java/com/google/android/libraries/feed/feedstore/FeedStore.java
+++ b/src/main/java/com/google/android/libraries/feed/feedstore/FeedStore.java
@@ -112,7 +112,9 @@
journalStorage,
threadUtils,
this.clock,
- this.storeHelper);
+ this.storeHelper,
+ basicLoggingApi,
+ mainThreadRunner);
delegate = persistentStore;
}
diff --git a/src/main/java/com/google/android/libraries/feed/feedstore/internal/BUILD b/src/main/java/com/google/android/libraries/feed/feedstore/internal/BUILD
index c3d4749..e918abc 100644
--- a/src/main/java/com/google/android/libraries/feed/feedstore/internal/BUILD
+++ b/src/main/java/com/google/android/libraries/feed/feedstore/internal/BUILD
@@ -6,10 +6,12 @@
name = "internal",
srcs = glob(["*.java"]),
deps = [
+ "//src/main/java/com/google/android/libraries/feed/api/host/logging",
"//src/main/java/com/google/android/libraries/feed/api/host/storage",
"//src/main/java/com/google/android/libraries/feed/api/internal/common",
"//src/main/java/com/google/android/libraries/feed/api/internal/store",
"//src/main/java/com/google/android/libraries/feed/common",
+ "//src/main/java/com/google/android/libraries/feed/common/concurrent",
"//src/main/java/com/google/android/libraries/feed/common/functional",
"//src/main/java/com/google/android/libraries/feed/common/intern",
"//src/main/java/com/google/android/libraries/feed/common/logging",
diff --git a/src/main/java/com/google/android/libraries/feed/feedstore/internal/PersistentFeedStore.java b/src/main/java/com/google/android/libraries/feed/feedstore/internal/PersistentFeedStore.java
index fbab4dd..84dfca1 100644
--- a/src/main/java/com/google/android/libraries/feed/feedstore/internal/PersistentFeedStore.java
+++ b/src/main/java/com/google/android/libraries/feed/feedstore/internal/PersistentFeedStore.java
@@ -20,6 +20,8 @@
import static com.google.android.libraries.feed.feedstore.internal.FeedStoreConstants.UPLOADABLE_ACTION_PREFIX;
import android.util.Base64;
+import com.google.android.libraries.feed.api.host.logging.BasicLoggingApi;
+import com.google.android.libraries.feed.api.host.logging.InternalFeedError;
import com.google.android.libraries.feed.api.host.storage.CommitResult;
import com.google.android.libraries.feed.api.host.storage.ContentMutation.Builder;
import com.google.android.libraries.feed.api.host.storage.ContentStorage;
@@ -38,6 +40,7 @@
import com.google.android.libraries.feed.api.internal.store.StoreListener;
import com.google.android.libraries.feed.api.internal.store.UploadableActionMutation;
import com.google.android.libraries.feed.common.Result;
+import com.google.android.libraries.feed.common.concurrent.MainThreadRunner;
import com.google.android.libraries.feed.common.functional.Supplier;
import com.google.android.libraries.feed.common.intern.Interner;
import com.google.android.libraries.feed.common.intern.InternerWithStats;
@@ -80,6 +83,8 @@
private final ThreadUtils threadUtils;
private final Clock clock;
private final FeedStoreHelper storeHelper;
+ private final BasicLoggingApi basicLoggingApi;
+ private final MainThreadRunner mainThreadRunner;
// We use a common string interner pool because the same content IDs are reused across different
// protos. The actual proto interners below are backed by thisl common pool.
@@ -97,7 +102,9 @@
JournalStorageDirect journalStorageDirect,
ThreadUtils threadUtils,
Clock clock,
- FeedStoreHelper storeHelper) {
+ FeedStoreHelper storeHelper,
+ BasicLoggingApi basicLoggingApi,
+ MainThreadRunner mainThreadRunner) {
this.timingUtils = timingUtils;
this.extensionRegistry = extensionRegistry;
this.contentStorageDirect = contentStorageDirect;
@@ -105,6 +112,8 @@
this.threadUtils = threadUtils;
this.clock = clock;
this.storeHelper = storeHelper;
+ this.basicLoggingApi = basicLoggingApi;
+ this.mainThreadRunner = mainThreadRunner;
}
@Override
@@ -119,8 +128,15 @@
return Result.failure();
}
- // TODO: This code should be asserting that all requested contentIds have been
- // provided, otherwise an unbound child could be exposed to the UI.
+ if (contentResult.getValue().entrySet().size() != contentIds.size()) {
+ Logger.e(TAG, "ContentStorage is missing content");
+ mainThreadRunner.execute(
+ "CONTENT_STORAGE_MISSING_ITEM",
+ () -> {
+ basicLoggingApi.onInternalError(InternalFeedError.CONTENT_STORAGE_MISSING_ITEM);
+ });
+ }
+
for (Map.Entry<String, byte[]> entry : contentResult.getValue().entrySet()) {
try {
StreamPayload streamPayload =
@@ -130,9 +146,14 @@
payloads.add(new PayloadWithId(entry.getKey(), streamPayload));
} catch (InvalidProtocolBufferException e) {
Logger.e(TAG, "Couldn't parse content proto for id %s", entry.getKey());
+ mainThreadRunner.execute(
+ "ITEM_NOT_PARSED",
+ () -> {
+ basicLoggingApi.onInternalError(InternalFeedError.ITEM_NOT_PARSED);
+ });
}
}
- tracker.stop("", "getPayloads", "items", contentIds.size());
+ tracker.stop("", "getPayloads", "items", payloads.size());
return Result.success(payloads);
}
diff --git a/src/test/java/com/google/android/libraries/feed/feedstore/internal/BUILD b/src/test/java/com/google/android/libraries/feed/feedstore/internal/BUILD
index 7bf57ba..b428e85 100644
--- a/src/test/java/com/google/android/libraries/feed/feedstore/internal/BUILD
+++ b/src/test/java/com/google/android/libraries/feed/feedstore/internal/BUILD
@@ -73,6 +73,7 @@
aapt_version = "aapt2",
manifest_values = DEFAULT_ANDROID_LOCAL_TEST_MANIFEST,
deps = [
+ "//src/main/java/com/google/android/libraries/feed/api/host/logging",
"//src/main/java/com/google/android/libraries/feed/api/host/storage",
"//src/main/java/com/google/android/libraries/feed/api/internal/common",
"//src/main/java/com/google/android/libraries/feed/api/internal/store",
@@ -83,9 +84,11 @@
"//src/main/java/com/google/android/libraries/feed/feedstore/internal",
"//src/main/java/com/google/android/libraries/feed/feedstore/testing",
"//src/main/java/com/google/android/libraries/feed/hostimpl/storage/testing",
+ "//src/main/java/com/google/android/libraries/feed/testing/host/logging",
"//src/main/proto/com/google/android/libraries/feed/api/internal/proto:client_feed_java_proto_lite",
"//third_party:robolectric",
"@com_google_protobuf_javalite//:protobuf_java_lite",
+ "@maven//:com_google_guava_guava",
"@maven//:com_google_truth_truth",
"@maven//:org_mockito_mockito_core",
"@robolectric//bazel:android-all",
diff --git a/src/test/java/com/google/android/libraries/feed/feedstore/internal/PersistentFeedStoreTest.java b/src/test/java/com/google/android/libraries/feed/feedstore/internal/PersistentFeedStoreTest.java
index 9895bc8..d0b9072 100644
--- a/src/test/java/com/google/android/libraries/feed/feedstore/internal/PersistentFeedStoreTest.java
+++ b/src/test/java/com/google/android/libraries/feed/feedstore/internal/PersistentFeedStoreTest.java
@@ -22,6 +22,7 @@
import static org.mockito.Mockito.spy;
import static org.mockito.MockitoAnnotations.initMocks;
+import com.google.android.libraries.feed.api.host.logging.InternalFeedError;
import com.google.android.libraries.feed.api.host.storage.CommitResult;
import com.google.android.libraries.feed.api.host.storage.ContentMutation;
import com.google.android.libraries.feed.api.host.storage.ContentStorageDirect;
@@ -41,8 +42,12 @@
import com.google.android.libraries.feed.feedstore.testing.DelegatingJournalStorage;
import com.google.android.libraries.feed.hostimpl.storage.testing.InMemoryContentStorage;
import com.google.android.libraries.feed.hostimpl.storage.testing.InMemoryJournalStorage;
+import com.google.android.libraries.feed.testing.host.logging.FakeBasicLoggingApi;
+import com.google.common.collect.ImmutableList;
import com.google.protobuf.ByteString;
+import com.google.search.now.feed.client.StreamDataProto.StreamFeature;
import com.google.search.now.feed.client.StreamDataProto.StreamLocalAction;
+import com.google.search.now.feed.client.StreamDataProto.StreamPayload;
import com.google.search.now.feed.client.StreamDataProto.StreamSharedState;
import com.google.search.now.feed.client.StreamDataProto.StreamStructure;
import com.google.search.now.feed.client.StreamDataProto.StreamUploadableAction;
@@ -67,12 +72,12 @@
private final FeedExtensionRegistry extensionRegistry = new FeedExtensionRegistry(ArrayList::new);
private final ContentStorageDirect contentStorage = new InMemoryContentStorage();
private final JournalStorageDirect journalStorage = new InMemoryJournalStorage();
- private FakeMainThreadRunner mainThreadRunner;
+ private final FakeBasicLoggingApi basicLoggingApi = new FakeBasicLoggingApi();
+ private final FakeMainThreadRunner mainThreadRunner = FakeMainThreadRunner.runTasksImmediately();
@Before
public void setUp() throws Exception {
initMocks(this);
- mainThreadRunner = FakeMainThreadRunner.runTasksImmediately();
}
@Override
@@ -84,7 +89,9 @@
journalStorage,
threadUtils,
fakeClock,
- new FeedStoreHelper());
+ new FeedStoreHelper(),
+ basicLoggingApi,
+ mainThreadRunner);
}
@Test
@@ -98,7 +105,9 @@
journalStorage,
threadUtils,
fakeClock,
- new FeedStoreHelper());
+ new FeedStoreHelper(),
+ basicLoggingApi,
+ mainThreadRunner);
doAnswer(ans -> Result.failure()).when(contentStorageSpy).getAllKeys();
boolean clearSuccess = store.clearNonActionContent();
@@ -116,7 +125,9 @@
journalStorage,
threadUtils,
fakeClock,
- new FeedStoreHelper());
+ new FeedStoreHelper(),
+ basicLoggingApi,
+ mainThreadRunner);
CommitResult commitResult = store.editContent().add(CONTENT_ID, STREAM_PAYLOAD).commit();
assertThat(commitResult).isEqualTo(CommitResult.SUCCESS);
@@ -146,7 +157,9 @@
journalStorageSpy,
threadUtils,
fakeClock,
- new FeedStoreHelper());
+ new FeedStoreHelper(),
+ basicLoggingApi,
+ mainThreadRunner);
boolean commitResult =
store
@@ -179,7 +192,9 @@
journalStorageSpy,
threadUtils,
fakeClock,
- new FeedStoreHelper());
+ new FeedStoreHelper(),
+ basicLoggingApi,
+ mainThreadRunner);
doAnswer(ans -> Result.failure()).when(journalStorageSpy).getAllJournals();
boolean clearSuccess = store.clearNonActionContent();
@@ -414,6 +429,41 @@
}
@Test
+ public void getPayloads_noContent() {
+ Result<List<PayloadWithId>> result =
+ getStore(mainThreadRunner).getPayloads(ImmutableList.of("foo"));
+ assertThat(result.isSuccessful()).isTrue();
+ assertThat(result.getValue()).isEmpty();
+ assertThat(basicLoggingApi.lastInternalError)
+ .isEqualTo(InternalFeedError.CONTENT_STORAGE_MISSING_ITEM);
+ }
+
+ @Test
+ public void getPayloads_withContent() {
+ StreamPayload streamPayload =
+ StreamPayload.newBuilder().setStreamFeature(StreamFeature.getDefaultInstance()).build();
+ contentStorage.commit(
+ new ContentMutation.Builder().upsert("foo", streamPayload.toByteArray()).build());
+
+ Result<List<PayloadWithId>> result =
+ getStore(mainThreadRunner).getPayloads(ImmutableList.of("foo"));
+ assertThat(result.isSuccessful()).isTrue();
+ assertThat(result.getValue()).hasSize(1);
+ assertThat(result.getValue().get(0).payload).isEqualTo(streamPayload);
+ }
+
+ @Test
+ public void getPayloads_cannotParse() {
+ contentStorage.commit(new ContentMutation.Builder().upsert("foo", new byte[] {5}).build());
+
+ Result<List<PayloadWithId>> result =
+ getStore(mainThreadRunner).getPayloads(ImmutableList.of("foo"));
+ assertThat(result.isSuccessful()).isTrue();
+ assertThat(result.getValue()).isEmpty();
+ assertThat(basicLoggingApi.lastInternalError).isEqualTo(InternalFeedError.ITEM_NOT_PARSED);
+ }
+
+ @Test
public void getSharedStates_noContent() {
Result<List<StreamSharedState>> result = getStore(mainThreadRunner).getSharedStates();
assertThat(result.isSuccessful()).isTrue();
