Use Call.Factory instead of OkHttpClient in images module

markwon-image/src/main/java/ru/noties/markwon/image/AsyncDrawableLoaderBuilder.java

@@ -29,6 +29,8 @@ class AsyncDrawableLoaderBuilder {
         // we should not use file-scheme as it's a bit complicated to assume file usage (lack of permissions)
         addSchemeHandler(DataUriSchemeHandler.create());
         addSchemeHandler(NetworkSchemeHandler.create());
+
+        defaultMediaDecoder = DefaultImageMediaDecoder.create();
     }
 
     void executorService(@NonNull ExecutorService executorService) {
@@ -78,11 +80,6 @@ class AsyncDrawableLoaderBuilder {
 
         isBuilt = true;
 
-        // @since 4.0.0-SNAPSHOT
-        if (defaultMediaDecoder == null) {
-            defaultMediaDecoder = DefaultImageMediaDecoder.create();
-        }
-
         if (executorService == null) {
             executorService = Executors.newCachedThreadPool();
         }

markwon-image/src/main/java/ru/noties/markwon/image/AsyncDrawableLoaderImpl.java

@@ -4,8 +4,10 @@ import android.graphics.drawable.Drawable;
 import android.net.Uri;
 import android.os.Handler;
 import android.os.Looper;
+import android.os.SystemClock;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
+import android.support.annotation.VisibleForTesting;
 import android.util.Log;
 
 import java.lang.ref.WeakReference;
@@ -24,126 +26,45 @@ class AsyncDrawableLoaderImpl extends AsyncDrawableLoader {
     private final ImagesPlugin.PlaceholderProvider placeholderProvider;
     private final ImagesPlugin.ErrorHandler errorHandler;
 
-    private final Handler mainThread;
+    private final Handler handler;
 
-    // @since 3.1.0-SNAPSHOT use a hash-map with a weak AsyncDrawable as key for multiple requests
+    // @since 4.0.0-SNAPSHOT use a hash-map with a AsyncDrawable as key for multiple requests
     //  for the same destination
-    private final Map<WeakReference<AsyncDrawable>, Future<?>> requests = new HashMap<>(2);
+    private final Map<AsyncDrawable, Future<?>> requests = new HashMap<>(2);
 
     AsyncDrawableLoaderImpl(@NonNull AsyncDrawableLoaderBuilder builder) {
+        this(builder, new Handler(Looper.getMainLooper()));
+    }
+
+    // @since 4.0.0-SNAPSHOT
+    @VisibleForTesting
+    AsyncDrawableLoaderImpl(@NonNull AsyncDrawableLoaderBuilder builder, @NonNull Handler handler) {
         this.executorService = builder.executorService;
         this.schemeHandlers = builder.schemeHandlers;
         this.mediaDecoders = builder.mediaDecoders;
         this.defaultMediaDecoder = builder.defaultMediaDecoder;
         this.placeholderProvider = builder.placeholderProvider;
         this.errorHandler = builder.errorHandler;
-        this.mainThread = new Handler(Looper.getMainLooper());
+        this.handler = handler;
     }
 
     @Override
     public void load(@NonNull final AsyncDrawable drawable) {
-
+        final Future<?> future = requests.get(drawable);
-        // primitive synchronization via main-thread
+        if (future == null) {
-        if (!isMainThread()) {
+            requests.put(drawable, execute(drawable));
-            mainThread.post(new Runnable() {
-                @Override
-                public void run() {
-                    load(drawable);
         }
-            });
-            return;
-        }
-
-        // okay, if by some chance requested drawable already has a future associated -> no-op
-        // as AsyncDrawable cannot change `destination` (immutable field)
-        // @since 3.1.0-SNAPSHOT
-        if (hasTaskAssociated(drawable)) {
-            return;
-        }
-
-        final WeakReference<AsyncDrawable> reference = new WeakReference<>(drawable);
-        requests.put(reference, execute(drawable.getDestination(), reference));
     }
 
     @Override
     public void cancel(@NonNull final AsyncDrawable drawable) {
 
-        if (!isMainThread()) {
+        final Future<?> future = requests.remove(drawable);
-            mainThread.post(new Runnable() {
+        if (future != null) {
-                @Override
+            future.cancel(true);
-                public void run() {
-                    cancel(drawable);
-                }
-            });
-            return;
         }
 
-        final Iterator<Map.Entry<WeakReference<AsyncDrawable>, Future<?>>> iterator =
+        handler.removeCallbacksAndMessages(drawable);
-                requests.entrySet().iterator();
-
-        AsyncDrawable key;
-        Map.Entry<WeakReference<AsyncDrawable>, Future<?>> entry;
-
-        while (iterator.hasNext()) {
-
-            entry = iterator.next();
-            key = entry.getKey().get();
-
-            // if key is null or it contains requested AsyncDrawable -> cancel
-            if (shouldCleanUp(key) || key == drawable) {
-                entry.getValue().cancel(true);
-                iterator.remove();
-            }
-        }
-    }
-
-    private boolean hasTaskAssociated(@NonNull AsyncDrawable drawable) {
-
-        final Iterator<Map.Entry<WeakReference<AsyncDrawable>, Future<?>>> iterator =
-                requests.entrySet().iterator();
-
-        boolean result = false;
-
-        AsyncDrawable key;
-        Map.Entry<WeakReference<AsyncDrawable>, Future<?>> entry;
-
-        while (iterator.hasNext()) {
-
-            entry = iterator.next();
-            key = entry.getKey().get();
-
-            // clean-up
-            if (shouldCleanUp(key)) {
-                entry.getValue().cancel(true);
-                iterator.remove();
-            } else if (key == drawable) {
-                result = true;
-                // do not break, let iteration continue to possibly clean-up the rest references
-            }
-        }
-
-        return result;
-    }
-
-    private void cleanUp() {
-
-        final Iterator<Map.Entry<WeakReference<AsyncDrawable>, Future<?>>> iterator =
-                requests.entrySet().iterator();
-
-        AsyncDrawable key;
-        Map.Entry<WeakReference<AsyncDrawable>, Future<?>> entry;
-
-        while (iterator.hasNext()) {
-
-            entry = iterator.next();
-            key = entry.getKey().get();
-
-            // clean-up of already referenced or detached drawables
-            if (shouldCleanUp(key)) {
-                entry.getValue().cancel(true);
-                iterator.remove();
-            }
-        }
     }
 
     @Nullable
@@ -155,7 +76,7 @@ class AsyncDrawableLoaderImpl extends AsyncDrawableLoader {
     }
 
     @NonNull
-    private Future<?> execute(@NonNull final String destination, @NonNull final WeakReference<AsyncDrawable> reference) {
+    private Future<?> execute(@NonNull final AsyncDrawable asyncDrawable) {
 
         // todo: more efficient DefaultImageMediaDecoder... BitmapFactory.decodeStream is a bit not optimal
         //      for big images for sure. We _could_ introduce internal Drawable that will check for
@@ -166,6 +87,8 @@ class AsyncDrawableLoaderImpl extends AsyncDrawableLoader {
             @Override
             public void run() {
 
+                final String destination = asyncDrawable.getDestination();
+
                 final Uri uri = Uri.parse(destination);
 
                 Drawable drawable = null;
@@ -214,35 +137,22 @@ class AsyncDrawableLoaderImpl extends AsyncDrawableLoader {
 
                 final Drawable out = drawable;
 
-                mainThread.post(new Runnable() {
+                handler.postAtTime(new Runnable() {
                     @Override
                     public void run() {
-
+                        // validate that
-                        if (out != null) {
+                        // * request was not cancelled
-                            // AsyncDrawable cannot change destination, so if it's
+                        // * out-result is present
-                            //  attached and not garbage-collected, we can deliver the result.
+                        // * async-drawable is attached
-                            //  Note that there is no cache, so attach/detach of drawables
+                        final Future<?> future = requests.remove(asyncDrawable);
-                            //  will always request a new entry.. (comment @since 3.1.0-SNAPSHOT)
+                        if (future != null
-                            final AsyncDrawable asyncDrawable = reference.get();
+                                && out != null
-                            if (asyncDrawable != null && asyncDrawable.isAttached()) {
+                                && asyncDrawable.isAttached()) {
                             asyncDrawable.setResult(out);
                         }
                     }
-
+                }, asyncDrawable, SystemClock.uptimeMillis());
-                        requests.remove(reference);
-                        cleanUp();
             }
         });
     }
-        });
-    }
-
-    private static boolean shouldCleanUp(@Nullable AsyncDrawable drawable) {
-        return drawable == null || !drawable.isAttached();
-    }
-
-    @SuppressWarnings("BooleanMethodIsAlwaysInverted")
-    private static boolean isMainThread() {
-        return Looper.myLooper() == Looper.getMainLooper();
-    }
 }

markwon-image/src/main/java/ru/noties/markwon/image/ImagesPlugin.java

@@ -107,13 +107,13 @@ public class ImagesPlugin extends AbstractMarkwonPlugin {
 
     /**
      * Please note that if not specified a {@link DefaultImageMediaDecoder} will be used. So
-     * if you need to disable default-image-media-decoder specify here own no-op implementation.
+     * if you need to disable default-image-media-decoder specify here own no-op implementation or null.
      *
      * @see DefaultImageMediaDecoder
      * @since 4.0.0-SNAPSHOT
      */
     @NonNull
-    public ImagesPlugin defaultMediaDecoder(@NonNull MediaDecoder mediaDecoder) {
+    public ImagesPlugin defaultMediaDecoder(@Nullable MediaDecoder mediaDecoder) {
         checkBuilderState();
         builder.defaultMediaDecoder(mediaDecoder);
         return this;

markwon-image/src/main/java/ru/noties/markwon/image/network/OkHttpNetworkSchemeHandler.java

@@ -7,6 +7,7 @@ import java.io.InputStream;
 import java.util.Arrays;
 import java.util.Collection;
 
+import okhttp3.Call;
 import okhttp3.OkHttpClient;
 import okhttp3.Request;
 import okhttp3.Response;
@@ -24,21 +25,31 @@ public class OkHttpNetworkSchemeHandler extends SchemeHandler {
      */
     @NonNull
     public static OkHttpNetworkSchemeHandler create() {
-        return new OkHttpNetworkSchemeHandler(new OkHttpClient());
+        return create(new OkHttpClient());
     }
 
     @NonNull
     public static OkHttpNetworkSchemeHandler create(@NonNull OkHttpClient client) {
-        return new OkHttpNetworkSchemeHandler(client);
+        // explicit cast, otherwise a recursive call
+        return create((Call.Factory) client);
+    }
+
+    /**
+     * @since 4.0.0-SNAPSHOT
+     */
+    @NonNull
+    public static OkHttpNetworkSchemeHandler create(@NonNull Call.Factory factory) {
+        return new OkHttpNetworkSchemeHandler(factory);
     }
 
     private static final String HEADER_CONTENT_TYPE = "Content-Type";
 
-    private final OkHttpClient client;
+    // @since 4.0.0-SNAPSHOT, previously just OkHttpClient
+    private final Call.Factory factory;
 
     @SuppressWarnings("WeakerAccess")
-    OkHttpNetworkSchemeHandler(@NonNull OkHttpClient client) {
+    OkHttpNetworkSchemeHandler(@NonNull Call.Factory factory) {
-        this.client = client;
+        this.factory = factory;
     }
 
     @NonNull
@@ -52,7 +63,7 @@ public class OkHttpNetworkSchemeHandler extends SchemeHandler {
 
         final Response response;
         try {
-            response = client.newCall(request).execute();
+            response = factory.newCall(request).execute();
         } catch (Throwable t) {
             throw new IllegalStateException("Exception obtaining network resource: " + raw, t);
         }

markwon-image/src/test/java/ru/noties/markwon/image/AsyncDrawableLoaderImplTest.java

@@ -0,0 +1,99 @@
+package ru.noties.markwon.image;
+
+import android.os.Handler;
+import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.annotation.Config;
+
+import java.util.concurrent.ExecutorService;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+@RunWith(RobolectricTestRunner.class)
+@Config(manifest = Config.NONE)
+public class AsyncDrawableLoaderImplTest {
+
+    private BuilderImpl builder;
+    private AsyncDrawableLoaderImpl impl;
+
+    @Before
+    public void before() {
+        builder = new BuilderImpl();
+    }
+
+    @Test
+    public void placeholder() {
+        final ImagesPlugin.PlaceholderProvider placeholderProvider =
+                mock(ImagesPlugin.PlaceholderProvider.class);
+        impl = builder.placeholderProvider(placeholderProvider)
+                .build();
+        impl.placeholder(mock(AsyncDrawable.class));
+        verify(placeholderProvider, times(1))
+                .providePlaceholder(any(AsyncDrawable.class));
+    }
+
+    private static class BuilderImpl {
+
+        AsyncDrawableLoaderBuilder builder;
+        Handler handler = mock(Handler.class);
+
+        public BuilderImpl executorService(@NonNull ExecutorService executorService) {
+            builder.executorService(executorService);
+            return this;
+        }
+
+        public BuilderImpl addSchemeHandler(@NonNull SchemeHandler schemeHandler) {
+            builder.addSchemeHandler(schemeHandler);
+            return this;
+        }
+
+        public BuilderImpl addMediaDecoder(@NonNull MediaDecoder mediaDecoder) {
+            builder.addMediaDecoder(mediaDecoder);
+            return this;
+        }
+
+        public BuilderImpl defaultMediaDecoder(@Nullable MediaDecoder mediaDecoder) {
+            builder.defaultMediaDecoder(mediaDecoder);
+            return this;
+        }
+
+        public BuilderImpl removeSchemeHandler(@NonNull String scheme) {
+            builder.removeSchemeHandler(scheme);
+            return this;
+        }
+
+        public BuilderImpl removeMediaDecoder(@NonNull String contentType) {
+            builder.removeMediaDecoder(contentType);
+            return this;
+        }
+
+        public BuilderImpl placeholderProvider(@NonNull ImagesPlugin.PlaceholderProvider placeholderDrawableProvider) {
+            builder.placeholderProvider(placeholderDrawableProvider);
+            return this;
+        }
+
+        public BuilderImpl errorHandler(@NonNull ImagesPlugin.ErrorHandler errorHandler) {
+            builder.errorHandler(errorHandler);
+            return this;
+        }
+
+        @NonNull
+        public BuilderImpl handler(Handler handler) {
+            this.handler = handler;
+            return this;
+        }
+
+        @NonNull
+        AsyncDrawableLoaderImpl build() {
+            return new AsyncDrawableLoaderImpl(builder, handler);
+        }
+    }
+}