Skip to content

Commit

Permalink
Extract common extra map constants and make producer names public
Browse files Browse the repository at this point in the history
Summary: As some tooling and logging can rely on producer names in the `RequestListener`, we should make the producer names public. This makes it easier for the client application to be kept in-sync with changes of the producers or fail early during compiling.

Reviewed By: massimocarli

Differential Revision: D3841676

fbshipit-source-id: 88ef24f07a19604ea7934c8dab388ff10a6bc687
  • Loading branch information
Daniel Hugenroth authored and Facebook Github Bot 5 committed Sep 12, 2016
1 parent 8c361ed commit 401e779
Show file tree
Hide file tree
Showing 24 changed files with 114 additions and 61 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
*/
public class BitmapMemoryCacheGetProducer extends BitmapMemoryCacheProducer {

@VisibleForTesting static final String PRODUCER_NAME = "BitmapMemoryCacheGetProducer";
public static final String PRODUCER_NAME = "BitmapMemoryCacheGetProducer";

public BitmapMemoryCacheGetProducer(
MemoryCache<CacheKey, CloseableImage> memoryCache,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
*/
public class BitmapMemoryCacheProducer implements Producer<CloseableReference<CloseableImage>> {

@VisibleForTesting static final String PRODUCER_NAME = "BitmapMemoryCacheProducer";
@VisibleForTesting static final String VALUE_FOUND = "cached_value_found";
public static final String PRODUCER_NAME = "BitmapMemoryCacheProducer";
public static final String EXTRA_CACHED_VALUE_FOUND = ProducerConstants.EXTRA_CACHED_VALUE_FOUND;

private final MemoryCache<CacheKey, CloseableImage> mMemoryCache;
private final CacheKeyFactory mCacheKeyFactory;
Expand Down Expand Up @@ -60,7 +60,9 @@ public void produceResults(
listener.onProducerFinishWithSuccess(
requestId,
getProducerName(),
listener.requiresExtraMap(requestId) ? ImmutableMap.of(VALUE_FOUND, "true") : null);
listener.requiresExtraMap(requestId)
? ImmutableMap.of(EXTRA_CACHED_VALUE_FOUND, "true")
: null);
consumer.onProgressUpdate(1f);
}
consumer.onNewResult(cachedReference, isFinal);
Expand All @@ -75,7 +77,9 @@ public void produceResults(
listener.onProducerFinishWithSuccess(
requestId,
getProducerName(),
listener.requiresExtraMap(requestId) ? ImmutableMap.of(VALUE_FOUND, "false") : null);
listener.requiresExtraMap(requestId)
? ImmutableMap.of(EXTRA_CACHED_VALUE_FOUND, "false")
: null);
consumer.onNewResult(null, true);
return;
}
Expand All @@ -84,7 +88,9 @@ public void produceResults(
listener.onProducerFinishWithSuccess(
requestId,
getProducerName(),
listener.requiresExtraMap(requestId) ? ImmutableMap.of(VALUE_FOUND, "false") : null);
listener.requiresExtraMap(requestId)
? ImmutableMap.of(EXTRA_CACHED_VALUE_FOUND, "false")
: null);
mInputProducer.produceResults(wrappedConsumer, producerContext);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
*/
public class DataFetchProducer extends LocalFetchProducer {

private static final String PRODUCER_NAME = "DataFetchProducer";
public static final String PRODUCER_NAME = "DataFetchProducer";

public DataFetchProducer(
PooledByteBufferFactory pooledByteBufferFactory,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,9 @@
* <p>This implementation delegates disk cache requests to BufferedDiskCache.
*/
public class DiskCacheProducer implements Producer<EncodedImage> {
@VisibleForTesting static final String PRODUCER_NAME = "DiskCacheProducer";
@VisibleForTesting static final String VALUE_FOUND = "cached_value_found";

public static final String PRODUCER_NAME = "DiskCacheProducer";
public static final String EXTRA_CACHED_VALUE_FOUND = ProducerConstants.EXTRA_CACHED_VALUE_FOUND;

private final BufferedDiskCache mDefaultBufferedDiskCache;
private final BufferedDiskCache mSmallImageBufferedDiskCache;
Expand Down Expand Up @@ -181,7 +182,7 @@ static Map<String, String> getExtraMap(
if (!listener.requiresExtraMap(requestId)) {
return null;
}
return ImmutableMap.of(VALUE_FOUND, String.valueOf(valueFound));
return ImmutableMap.of(EXTRA_CACHED_VALUE_FOUND, String.valueOf(valueFound));
}

private void subscribeTaskForRequestCancellation(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@
* Memory cache producer for the encoded memory cache.
*/
public class EncodedMemoryCacheProducer implements Producer<EncodedImage> {
@VisibleForTesting static final String PRODUCER_NAME = "EncodedMemoryCacheProducer";
@VisibleForTesting static final String VALUE_FOUND = "cached_value_found";

public static final String PRODUCER_NAME = "EncodedMemoryCacheProducer";
public static final String EXTRA_CACHED_VALUE_FOUND = ProducerConstants.EXTRA_CACHED_VALUE_FOUND;

private final MemoryCache<CacheKey, PooledByteBuffer> mMemoryCache;
private final CacheKeyFactory mCacheKeyFactory;
Expand Down Expand Up @@ -60,7 +61,9 @@ public void produceResults(
listener.onProducerFinishWithSuccess(
requestId,
PRODUCER_NAME,
listener.requiresExtraMap(requestId) ? ImmutableMap.of(VALUE_FOUND, "true") : null);
listener.requiresExtraMap(requestId)
? ImmutableMap.of(EXTRA_CACHED_VALUE_FOUND, "true")
: null);
consumer.onProgressUpdate(1f);
consumer.onNewResult(cachedEncodedImage, true);
return;
Expand All @@ -74,7 +77,9 @@ public void produceResults(
listener.onProducerFinishWithSuccess(
requestId,
PRODUCER_NAME,
listener.requiresExtraMap(requestId) ? ImmutableMap.of(VALUE_FOUND, "false") : null);
listener.requiresExtraMap(requestId)
? ImmutableMap.of(EXTRA_CACHED_VALUE_FOUND, "false")
: null);
consumer.onNewResult(null, true);
return;
}
Expand Down Expand Up @@ -122,7 +127,9 @@ public void onNewResultImpl(EncodedImage newResult, boolean isLast) {
listener.onProducerFinishWithSuccess(
requestId,
PRODUCER_NAME,
listener.requiresExtraMap(requestId) ? ImmutableMap.of(VALUE_FOUND, "false") : null);
listener.requiresExtraMap(requestId)
? ImmutableMap.of(EXTRA_CACHED_VALUE_FOUND, "false")
: null);
mInputProducer.produceResults(consumerOfInputProducer, producerContext);
} finally {
CloseableReference.closeSafely(cachedReference);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import android.content.res.AssetFileDescriptor;
import android.content.res.AssetManager;

import com.facebook.common.internal.VisibleForTesting;
import com.facebook.imagepipeline.image.EncodedImage;
import com.facebook.imagepipeline.memory.PooledByteBufferFactory;
import com.facebook.imagepipeline.request.ImageRequest;
Expand All @@ -24,7 +23,8 @@
* Executes a local fetch from an asset.
*/
public class LocalAssetFetchProducer extends LocalFetchProducer {
@VisibleForTesting static final String PRODUCER_NAME = "LocalAssetFetchProducer";

public static final String PRODUCER_NAME = "LocalAssetFetchProducer";

private final AssetManager mAssetManager;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import android.provider.ContactsContract;
import android.provider.MediaStore;

import com.facebook.common.internal.VisibleForTesting;
import com.facebook.common.util.UriUtil;
import com.facebook.imagepipeline.image.EncodedImage;
import com.facebook.imagepipeline.memory.PooledByteBufferFactory;
Expand All @@ -34,7 +33,8 @@
*/
public class LocalContentUriFetchProducer extends LocalFetchProducer {

@VisibleForTesting static final String PRODUCER_NAME = "LocalContentUriFetchProducer";
public static final String PRODUCER_NAME = "LocalContentUriFetchProducer";

private static final String[] PROJECTION = new String[] {
MediaStore.Images.Media._ID,
MediaStore.Images.ImageColumns.DATA
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import android.net.Uri;
import android.provider.MediaStore;

import com.facebook.common.internal.VisibleForTesting;
import com.facebook.common.logging.FLog;
import com.facebook.common.util.UriUtil;
import com.facebook.imagepipeline.common.ResizeOptions;
Expand All @@ -40,7 +39,8 @@ public class LocalContentUriThumbnailFetchProducer extends LocalFetchProducer

private static final Class<?> TAG = LocalContentUriThumbnailFetchProducer.class;

@VisibleForTesting static final String PRODUCER_NAME = "LocalContentUriThumbnailFetchProducer";
public static final String PRODUCER_NAME = "LocalContentUriThumbnailFetchProducer";

private static final String[] PROJECTION = new String[] {
MediaStore.Images.Media._ID,
MediaStore.Images.ImageColumns.DATA
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public class LocalExifThumbnailProducer implements ThumbnailProducer<EncodedImag

private static final int COMMON_EXIF_THUMBNAIL_MAX_DIMENSION = 512;

@VisibleForTesting static final String PRODUCER_NAME = "LocalExifThumbnailProducer";
public static final String PRODUCER_NAME = "LocalExifThumbnailProducer";
@VisibleForTesting static final String CREATED_THUMBNAIL = "createdThumbnail";

private final Executor mExecutor;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import java.io.IOException;
import java.util.concurrent.Executor;

import com.facebook.common.internal.VisibleForTesting;
import com.facebook.imagepipeline.image.EncodedImage;
import com.facebook.imagepipeline.memory.PooledByteBufferFactory;
import com.facebook.imagepipeline.request.ImageRequest;
Expand All @@ -22,7 +21,8 @@
* Represents a local file fetch producer.
*/
public class LocalFileFetchProducer extends LocalFetchProducer {
@VisibleForTesting static final String PRODUCER_NAME = "LocalFileFetchProducer";

public static final String PRODUCER_NAME = "LocalFileFetchProducer";

public LocalFileFetchProducer(
Executor executor,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
* Executes a local fetch from a resource.
*/
public class LocalResourceFetchProducer extends LocalFetchProducer {
@VisibleForTesting static final String PRODUCER_NAME = "LocalResourceFetchProducer";

public static final String PRODUCER_NAME = "LocalResourceFetchProducer";

private final Resources mResources;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
public class LocalVideoThumbnailProducer implements
Producer<CloseableReference<CloseableImage>> {

@VisibleForTesting static final String PRODUCER_NAME = "VideoThumbnailProducer";
public static final String PRODUCER_NAME = "VideoThumbnailProducer";
@VisibleForTesting static final String CREATED_THUMBNAIL = "createdThumbnail";

private final Executor mExecutor;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
*/
public class NetworkFetchProducer implements Producer<EncodedImage> {

@VisibleForTesting static final String PRODUCER_NAME = "NetworkFetchProducer";
public static final String PRODUCER_NAME = "NetworkFetchProducer";
public static final String INTERMEDIATE_RESULT_PRODUCER_EVENT = "intermediate_result";
private static final int READ_SIZE = 16 * 1024;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
public class PostprocessedBitmapMemoryCacheProducer
implements Producer<CloseableReference<CloseableImage>> {

@VisibleForTesting static final String PRODUCER_NAME = "PostprocessedBitmapMemoryCacheProducer";
public static final String PRODUCER_NAME = "PostprocessedBitmapMemoryCacheProducer";
@VisibleForTesting static final String VALUE_FOUND = "cached_value_found";

private final MemoryCache<CacheKey, CloseableImage> mMemoryCache;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
*/
public class PostprocessorProducer implements Producer<CloseableReference<CloseableImage>> {

@VisibleForTesting static final String NAME = "PostprocessorProducer";
public static final String NAME = "PostprocessorProducer";
@VisibleForTesting static final String POSTPROCESSOR = "Postprocessor";

private final Producer<CloseableReference<CloseableImage>> mInputProducer;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* Copyright (c) 2015-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/

package com.facebook.imagepipeline.producers;

/**
* Constants to be used various {@link Producer}s for logging purposes in the extra maps for the
* {@link com.facebook.imagepipeline.listener.RequestListener}.
*
* The elements are package visible on purpose such that the individual producers create public
* constants of the ones that they actually use.
*/
class ProducerConstants {

static final String EXTRA_CACHED_VALUE_FOUND = "cached_value_found";
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
* <p>Should not be used if downsampling is in use.
*/
public class ResizeAndRotateProducer implements Producer<EncodedImage> {
private static final String PRODUCER_NAME = "ResizeAndRotateProducer";
public static final String PRODUCER_NAME = "ResizeAndRotateProducer";
private static final String ORIGINAL_SIZE_KEY = "Original size";
private static final String REQUESTED_SIZE_KEY = "Requested size";
private static final String FRACTION_KEY = "Fraction";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,15 @@

package com.facebook.imagepipeline.producers;

import java.util.concurrent.Executor;

import com.facebook.common.internal.Preconditions;
import com.facebook.common.internal.VisibleForTesting;

/**
* Uses ExecutorService to move further computation to different thread
*/
public class ThreadHandoffProducer<T> implements Producer<T> {

@VisibleForTesting
protected static final String PRODUCER_NAME = "BackgroundThreadHandoffProducer";
public static final String PRODUCER_NAME = "BackgroundThreadHandoffProducer";

private final Producer<T> mInputProducer;
private final ThreadHandoffProducerQueue mThreadHandoffProducerQueue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,14 @@
import android.util.Pair;

import com.facebook.common.internal.Preconditions;
import com.facebook.common.internal.VisibleForTesting;

/**
* Only permits a configurable number of requests to be kicked off simultaneously. If that number
* is exceeded, then requests are queued up and kicked off once other requests complete.
*/
public class ThrottlingProducer<T> implements Producer<T> {

@VisibleForTesting static final String PRODUCER_NAME = "ThrottlingProducer";
public static final String PRODUCER_NAME = "ThrottlingProducer";

private final Producer<T> mInputProducer;
private final int mMaxSimultaneousRequests;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
* <p> If the image is not WebP, no transformation is applied.
*/
public class WebpTranscodeProducer implements Producer<EncodedImage> {
private static final String PRODUCER_NAME = "WebpTranscodeProducer";
public static final String PRODUCER_NAME = "WebpTranscodeProducer";
private static final int DEFAULT_JPEG_QUALITY = 80;

private final Executor mExecutor;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ public void testBitmapMemoryCacheGetSuccessful() {
mBitmapMemoryCacheGetProducer.produceResults(mConsumer, mProducerContext);
verify(mConsumer).onNewResult(mFinalImageReference, true);
verify(mProducerListener).onProducerStart(mRequestId, PRODUCER_NAME);
Map<String, String> extraMap = ImmutableMap.of(BitmapMemoryCacheProducer.VALUE_FOUND, "true");
Map<String, String> extraMap =
ImmutableMap.of(BitmapMemoryCacheProducer.EXTRA_CACHED_VALUE_FOUND, "true");
verify(mProducerListener).onProducerFinishWithSuccess(mRequestId, PRODUCER_NAME, extraMap);
Assert.assertTrue(!mFinalImageReference.isValid());
}
Expand All @@ -101,7 +102,8 @@ public void testBitmapMemoryCacheGetNotFoundInputProducerSuccess() {
mBitmapMemoryCacheGetProducer.produceResults(mConsumer, mProducerContext);
verify(mConsumer).onNewResult(mFinalImageReference, true);
verify(mProducerListener).onProducerStart(mRequestId, PRODUCER_NAME);
Map<String, String> extraMap = ImmutableMap.of(BitmapMemoryCacheProducer.VALUE_FOUND, "false");
Map<String, String> extraMap =
ImmutableMap.of(BitmapMemoryCacheProducer.EXTRA_CACHED_VALUE_FOUND, "false");
verify(mProducerListener).onProducerFinishWithSuccess(mRequestId, PRODUCER_NAME, extraMap);
}

Expand All @@ -112,7 +114,8 @@ public void testBitmapMemoryCacheGetNotFoundInputProducerNotFound() {
mBitmapMemoryCacheGetProducer.produceResults(mConsumer, mProducerContext);
verify(mConsumer).onNewResult(null, true);
verify(mProducerListener).onProducerStart(mRequestId, PRODUCER_NAME);
Map<String, String> extraMap = ImmutableMap.of(BitmapMemoryCacheProducer.VALUE_FOUND, "false");
Map<String, String> extraMap =
ImmutableMap.of(BitmapMemoryCacheProducer.EXTRA_CACHED_VALUE_FOUND, "false");
verify(mProducerListener).onProducerFinishWithSuccess(mRequestId, PRODUCER_NAME, extraMap);
}

Expand All @@ -123,7 +126,8 @@ public void testBitmapMemoryCacheGetNotFoundInputProducerFailure() {
mBitmapMemoryCacheGetProducer.produceResults(mConsumer, mProducerContext);
verify(mConsumer).onFailure(mException);
verify(mProducerListener).onProducerStart(mRequestId, PRODUCER_NAME);
Map<String, String> extraMap = ImmutableMap.of(BitmapMemoryCacheProducer.VALUE_FOUND, "false");
Map<String, String> extraMap =
ImmutableMap.of(BitmapMemoryCacheProducer.EXTRA_CACHED_VALUE_FOUND, "false");
verify(mProducerListener).onProducerFinishWithSuccess(mRequestId, PRODUCER_NAME, extraMap);
}

Expand All @@ -135,7 +139,8 @@ public void testBitmapMemoryCacheGetNotFoundLowestLevelReached() {
mBitmapMemoryCacheGetProducer.produceResults(mConsumer, mProducerContext);
verify(mConsumer).onNewResult(null, true);
verify(mProducerListener).onProducerStart(mRequestId, PRODUCER_NAME);
Map<String, String> extraMap = ImmutableMap.of(BitmapMemoryCacheProducer.VALUE_FOUND, "false");
Map<String, String> extraMap =
ImmutableMap.of(BitmapMemoryCacheProducer.EXTRA_CACHED_VALUE_FOUND, "false");
verify(mProducerListener).onProducerFinishWithSuccess(mRequestId, PRODUCER_NAME, extraMap);
verifyNoMoreInteractions(mInputProducer);
}
Expand Down
Loading

0 comments on commit 401e779

Please sign in to comment.