diff --git a/app/src/main/java/ru/noties/markwon/MarkdownRenderer.java b/app/src/main/java/ru/noties/markwon/MarkdownRenderer.java index c082312a..265b9cd5 100644 --- a/app/src/main/java/ru/noties/markwon/MarkdownRenderer.java +++ b/app/src/main/java/ru/noties/markwon/MarkdownRenderer.java @@ -8,6 +8,7 @@ import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.text.Spanned; +import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; @@ -71,9 +72,17 @@ public class MarkdownRenderer { cancel(); task = service.submit(new Runnable() { + @Override public void run() { + try { + execute(); + } catch (Throwable t) { + Debug.e(t); + } + } + private void execute() { final UrlProcessor urlProcessor; if (uri == null) { urlProcessor = new UrlProcessorInitialReadme(); diff --git a/build.gradle b/build.gradle index 05e0927e..88a298fa 100644 --- a/build.gradle +++ b/build.gradle @@ -77,14 +77,12 @@ ext { ] deps['test'] = [ - 'junit' : 'junit:junit:4.12', - 'robolectric' : 'org.robolectric:robolectric:3.8', - 'ix-java' : 'com.github.akarnokd:ixjava:1.0.0', - 'jackson-yaml' : 'com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.9.0', - 'jackson-databind': 'com.fasterxml.jackson.core:jackson-databind:2.9.6', - 'gson' : 'com.google.code.gson:gson:2.8.5', - 'commons-io' : 'commons-io:commons-io:2.6', - 'mockito' : 'org.mockito:mockito-core:2.21.0' + 'junit' : 'junit:junit:4.12', + 'robolectric': 'org.robolectric:robolectric:3.8', + 'ix-java' : 'com.github.akarnokd:ixjava:1.0.0', + 'gson' : 'com.google.code.gson:gson:2.8.5', + 'commons-io' : 'commons-io:commons-io:2.6', + 'mockito' : 'org.mockito:mockito-core:2.21.0' ] registerArtifact = this.®isterArtifact diff --git a/markwon/src/main/java/ru/noties/markwon/MarkwonBuilderImpl.java b/markwon/src/main/java/ru/noties/markwon/MarkwonBuilderImpl.java index e0eabdf6..e4cf56d9 100644 --- a/markwon/src/main/java/ru/noties/markwon/MarkwonBuilderImpl.java +++ b/markwon/src/main/java/ru/noties/markwon/MarkwonBuilderImpl.java @@ -2,7 +2,7 @@ package ru.noties.markwon; import android.content.Context; import android.support.annotation.NonNull; -import android.util.Log; +import android.support.annotation.VisibleForTesting; import android.widget.TextView; import org.commonmark.parser.Parser; @@ -12,6 +12,7 @@ import java.util.Collections; import java.util.Iterator; import java.util.List; +import ru.noties.markwon.core.CorePlugin; import ru.noties.markwon.core.MarkwonTheme; import ru.noties.markwon.image.AsyncDrawableLoader; import ru.noties.markwon.priority.PriorityProcessor; @@ -19,6 +20,7 @@ import ru.noties.markwon.priority.PriorityProcessor; /** * @since 3.0.0 */ +@SuppressWarnings("WeakerAccess") class MarkwonBuilderImpl implements Markwon.Builder { private final Context context; @@ -66,6 +68,7 @@ class MarkwonBuilderImpl implements Markwon.Builder { return this; } + @SuppressWarnings("UnusedReturnValue") @NonNull public MarkwonBuilderImpl priorityProcessor(@NonNull PriorityProcessor priorityProcessor) { this.priorityProcessor = priorityProcessor; @@ -76,6 +79,23 @@ class MarkwonBuilderImpl implements Markwon.Builder { @Override public Markwon build() { + if (plugins.isEmpty()) { + throw new IllegalStateException("No plugins were added to this builder. Use #usePlugin " + + "method to add them"); + } + + // this class will sort plugins to match a priority/dependency graph that we have + PriorityProcessor priorityProcessor = this.priorityProcessor; + if (priorityProcessor == null) { + // strictly speaking we do not need updating this field + // as we are not building this class to be reused between multiple `build` calls + priorityProcessor = this.priorityProcessor = PriorityProcessor.create(); + } + + // please note that this method must not modify supplied collection + // if nothing should be done -> the same collection can be returned + final List plugins = preparePlugins(priorityProcessor, this.plugins); + final Parser.Builder parserBuilder = new Parser.Builder(); final MarkwonTheme.Builder themeBuilder = MarkwonTheme.builderWithDefaults(context); final AsyncDrawableLoader.Builder asyncDrawableLoaderBuilder = new AsyncDrawableLoader.Builder(); @@ -84,18 +104,7 @@ class MarkwonBuilderImpl implements Markwon.Builder { final MarkwonSpansFactory.Builder spanFactoryBuilder = new MarkwonSpansFactoryImpl.BuilderImpl(); final RenderProps renderProps = new RenderPropsImpl(); - PriorityProcessor priorityProcessor = this.priorityProcessor; - if (priorityProcessor == null) { - // strictly speaking we do not need updating this field - // as we are not building this class to be reused between multiple `build` calls - priorityProcessor = this.priorityProcessor = PriorityProcessor.create(); - } - final List plugins = priorityProcessor.process(this.plugins); - for (MarkwonPlugin plugin : plugins) { - if (true) { - Log.e("PLUGIN", plugin.getClass().getName()); - } plugin.configureParser(parserBuilder); plugin.configureTheme(themeBuilder); plugin.configureImages(asyncDrawableLoaderBuilder); @@ -116,4 +125,59 @@ class MarkwonBuilderImpl implements Markwon.Builder { Collections.unmodifiableList(plugins) ); } + + @VisibleForTesting + @NonNull + static List preparePlugins( + @NonNull PriorityProcessor priorityProcessor, + @NonNull List plugins) { + + // with this method we will ensure that CorePlugin is added IF and ONLY IF + // there are plugins that depend on it. If CorePlugin is added, or there are + // no plugins that require it, CorePlugin won't be added + final List out = ensureImplicitCoreIfHasDependents(plugins); + + return priorityProcessor.process(out); + } + + // this method will _implicitly_ add CorePlugin if there is at least one plugin + // that depends on CorePlugin + @VisibleForTesting + @NonNull + static List ensureImplicitCoreIfHasDependents(@NonNull List plugins) { + // loop over plugins -> if CorePlugin is found -> break; + // iterate over all plugins and check if CorePlugin is requested + + boolean hasCore = false; + boolean hasCoreDependents = false; + + for (MarkwonPlugin plugin : plugins) { + + // here we do not check for exact match (a user could've subclasses CorePlugin + // and supplied it. In this case we DO NOT implicitly add CorePlugin + if (CorePlugin.class.isAssignableFrom(plugin.getClass())) { + hasCore = true; + break; + } + + // if plugin has CorePlugin in dependencies -> mark for addition + if (!hasCoreDependents) { + // here we check for direct CorePlugin, if it's not CorePlugin (exact, not a subclass + // or something -> ignore) + if (plugin.priority().after().contains(CorePlugin.class)) { + hasCoreDependents = true; + } + } + } + + if (hasCoreDependents && !hasCore) { + final List out = new ArrayList<>(plugins.size() + 1); + // add default instance of CorePlugin + out.add(CorePlugin.create()); + out.addAll(plugins); + return out; + } + + return plugins; + } } diff --git a/markwon/src/main/java/ru/noties/markwon/image/ImagesPlugin.java b/markwon/src/main/java/ru/noties/markwon/image/ImagesPlugin.java index dc51504f..d32ee10c 100644 --- a/markwon/src/main/java/ru/noties/markwon/image/ImagesPlugin.java +++ b/markwon/src/main/java/ru/noties/markwon/image/ImagesPlugin.java @@ -21,6 +21,9 @@ import ru.noties.markwon.image.data.DataUriSchemeHandler; import ru.noties.markwon.image.file.FileSchemeHandler; import ru.noties.markwon.image.network.NetworkSchemeHandler; +/** + * @since 3.0.0 + */ public class ImagesPlugin extends AbstractMarkwonPlugin { @NonNull diff --git a/markwon/src/main/java/ru/noties/markwon/priority/PriorityProcessorImpl.java b/markwon/src/main/java/ru/noties/markwon/priority/PriorityProcessorImpl.java index 08e820b4..e676a9c9 100644 --- a/markwon/src/main/java/ru/noties/markwon/priority/PriorityProcessorImpl.java +++ b/markwon/src/main/java/ru/noties/markwon/priority/PriorityProcessorImpl.java @@ -2,6 +2,7 @@ package ru.noties.markwon.priority; import android.support.annotation.NonNull; +import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; @@ -18,7 +19,10 @@ class PriorityProcessorImpl extends PriorityProcessor { @NonNull @Override - public List process(@NonNull List plugins) { + public List process(@NonNull List in) { + + // create new collection based on supplied argument + final List plugins = new ArrayList<>(in); final int size = plugins.size(); @@ -32,7 +36,6 @@ class PriorityProcessorImpl extends PriorityProcessor { } } - // change to Map final Map cache = new HashMap<>(size); for (MarkwonPlugin plugin : plugins) { cache.put(plugin, eval(plugin, map)); @@ -126,8 +129,4 @@ class PriorityProcessorImpl extends PriorityProcessor { return map.get(o1).compareTo(map.get(o2)); } } - - private static class NoCorePluginAddedException extends Exception { - - } } diff --git a/markwon/src/test/java/ru/noties/markwon/MarkwonBuilderImplTest.java b/markwon/src/test/java/ru/noties/markwon/MarkwonBuilderImplTest.java new file mode 100644 index 00000000..2ce7c25e --- /dev/null +++ b/markwon/src/test/java/ru/noties/markwon/MarkwonBuilderImplTest.java @@ -0,0 +1,231 @@ +package ru.noties.markwon; + +import android.support.annotation.NonNull; +import android.text.Spanned; +import android.widget.TextView; + +import org.commonmark.node.Node; +import org.commonmark.parser.Parser; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentMatchers; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.RuntimeEnvironment; +import org.robolectric.annotation.Config; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import ru.noties.markwon.core.CorePlugin; +import ru.noties.markwon.core.MarkwonTheme; +import ru.noties.markwon.image.AsyncDrawableLoader; +import ru.noties.markwon.priority.Priority; +import ru.noties.markwon.priority.PriorityProcessor; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.isA; +import static org.hamcrest.Matchers.not; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.atLeast; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static ru.noties.markwon.MarkwonBuilderImpl.ensureImplicitCoreIfHasDependents; +import static ru.noties.markwon.MarkwonBuilderImpl.preparePlugins; + +@RunWith(RobolectricTestRunner.class) +@Config(manifest = Config.NONE) +public class MarkwonBuilderImplTest { + + @Test + public void implicit_core_created() { + // a plugin explicitly requests CorePlugin, but CorePlugin is not added manually by user + // we validate that default CorePlugin instance is added + + final MarkwonPlugin plugin = new AbstractMarkwonPlugin() { + @NonNull + @Override + public Priority priority() { + // strictly speaking we do not need to override this method + // as all children of AbstractMarkwonPlugin specify CorePlugin as a dependency. + // but we still add it to make things explicit and future proof, if this + // behavior changes + return Priority.after(CorePlugin.class); + } + }; + + final List plugins = ensureImplicitCoreIfHasDependents(Collections.singletonList(plugin)); + + assertThat(plugins, hasSize(2)); + assertThat(plugins, hasItem(isA(CorePlugin.class))); + } + + @Test + public void implicit_core_no_dependents_not_added() { + final MarkwonPlugin a = new AbstractMarkwonPlugin() { + @NonNull + @Override + public Priority priority() { + return Priority.none(); + } + }; + + final MarkwonPlugin b = new AbstractMarkwonPlugin() { + @NonNull + @Override + public Priority priority() { + return Priority.after(a.getClass()); + } + }; + + final List plugins = ensureImplicitCoreIfHasDependents(Arrays.asList(a, b)); + assertThat(plugins, hasSize(2)); + assertThat(plugins, not(hasItem(isA(CorePlugin.class)))); + } + + @Test + public void implicit_core_present() { + // if core is present it won't be added (whether or not there are dependents) + + final MarkwonPlugin plugin = new AbstractMarkwonPlugin() { + @NonNull + @Override + public Priority priority() { + return Priority.after(CorePlugin.class); + } + }; + + final CorePlugin corePlugin = CorePlugin.create(); + + final List plugins = ensureImplicitCoreIfHasDependents(Arrays.asList(plugin, corePlugin)); + assertThat(plugins, hasSize(2)); + assertThat(plugins, hasItem(plugin)); + assertThat(plugins, hasItem(corePlugin)); + } + + @Test + public void implicit_core_subclass_present() { + // core was subclassed by a user and provided (implicit core won't be added) + + final MarkwonPlugin plugin = new AbstractMarkwonPlugin() { + @NonNull + @Override + public Priority priority() { + return Priority.after(CorePlugin.class); + } + }; + + // our subclass + final CorePlugin corePlugin = new CorePlugin(false) { + + }; + + final List plugins = ensureImplicitCoreIfHasDependents(Arrays.asList(plugin, corePlugin)); + assertThat(plugins, hasSize(2)); + assertThat(plugins, hasItem(plugin)); + assertThat(plugins, hasItem(corePlugin)); + } + + @Test + public void prepare_plugins() { + // validate that prepare plugins is calling `ensureImplicitCoreIfHasDependents` and + // priority processor + + final PriorityProcessor priorityProcessor = mock(PriorityProcessor.class); + when(priorityProcessor.process(ArgumentMatchers.anyList())) + .thenAnswer(new Answer() { + @Override + public Object answer(InvocationOnMock invocation) { + return invocation.getArgument(0); + } + }); + + final MarkwonPlugin plugin = new AbstractMarkwonPlugin() { + @NonNull + @Override + public Priority priority() { + return Priority.after(CorePlugin.class); + } + }; + + final List plugins = preparePlugins(priorityProcessor, Collections.singletonList(plugin)); + assertThat(plugins, hasSize(2)); + assertThat(plugins, hasItem(plugin)); + assertThat(plugins, hasItem(isA(CorePlugin.class))); + + verify(priorityProcessor, times(1)) + .process(ArgumentMatchers.anyList()); + } + + @Test + public void user_supplied_priority_processor() { + // verify that if user supplied priority processor it will be used + + final PriorityProcessor priorityProcessor = mock(PriorityProcessor.class); + final MarkwonBuilderImpl impl = new MarkwonBuilderImpl(RuntimeEnvironment.application); + + // add some plugin (we do not care which one, but it must be present as we do not + // allow empty plugins list) + impl.usePlugin(new AbstractMarkwonPlugin() { + @NonNull + @Override + public Priority priority() { + return Priority.none(); + } + }); + impl.priorityProcessor(priorityProcessor); + impl.build(); + + verify(priorityProcessor, times(1)).process(ArgumentMatchers.anyList()); + } + + @Test + public void no_plugins_added_throws() { + // there is no sense in having an instance with no plugins registered + + try { + new MarkwonBuilderImpl(RuntimeEnvironment.application).build(); + fail(); + } catch (IllegalStateException e) { + assertThat(e.getMessage(), e.getMessage(), containsString("No plugins were added")); + } + } + + @Test + public void plugin_configured() { + // verify that all configuration methods (applicable) are called + + final MarkwonPlugin plugin = mock(MarkwonPlugin.class); + when(plugin.priority()).thenReturn(Priority.none()); + + final MarkwonBuilderImpl impl = new MarkwonBuilderImpl(RuntimeEnvironment.application); + impl.usePlugin(plugin).build(); + + verify(plugin, times(1)).configureParser(any(Parser.Builder.class)); + verify(plugin, times(1)).configureTheme(any(MarkwonTheme.Builder.class)); + verify(plugin, times(1)).configureImages(any(AsyncDrawableLoader.Builder.class)); + verify(plugin, times(1)).configureConfiguration(any(MarkwonConfiguration.Builder.class)); + verify(plugin, times(1)).configureVisitor(any(MarkwonVisitor.Builder.class)); + verify(plugin, times(1)).configureSpansFactory(any(MarkwonSpansFactory.Builder.class)); + + // we do not know how many times exactly, but at least once it must be called + verify(plugin, atLeast(1)).priority(); + + // note, no render props -> they must be configured on render stage + verify(plugin, times(0)).configureRenderProps(any(RenderProps.class)); + verify(plugin, times(0)).processMarkdown(anyString()); + verify(plugin, times(0)).beforeRender(any(Node.class)); + verify(plugin, times(0)).afterRender(any(Node.class), any(MarkwonVisitor.class)); + verify(plugin, times(0)).beforeSetText(any(TextView.class), any(Spanned.class)); + verify(plugin, times(0)).afterSetText(any(TextView.class)); + } +} \ No newline at end of file diff --git a/markwon/src/test/java/ru/noties/markwon/priority/PriorityProcessorTest.java b/markwon/src/test/java/ru/noties/markwon/priority/PriorityProcessorTest.java index 28612ca4..2274a7f6 100644 --- a/markwon/src/test/java/ru/noties/markwon/priority/PriorityProcessorTest.java +++ b/markwon/src/test/java/ru/noties/markwon/priority/PriorityProcessorTest.java @@ -159,6 +159,33 @@ public class PriorityProcessorTest { assertEquals(third, plugins.get(2)); } + @Test + public void five_plugins_sequential() { + + final MarkwonPlugin a = new NamedPlugin("a") { + }; + + final MarkwonPlugin b = new NamedPlugin("b", a) { + }; + + final MarkwonPlugin c = new NamedPlugin("c", b) { + }; + + final MarkwonPlugin d = new NamedPlugin("d", c) { + }; + + final MarkwonPlugin e = new NamedPlugin("e", d) { + }; + + final List plugins = processor.process(Arrays.asList(d, e, a, c, b)); + assertEquals(5, plugins.size()); + assertEquals(a, plugins.get(0)); + assertEquals(b, plugins.get(1)); + assertEquals(c, plugins.get(2)); + assertEquals(d, plugins.get(3)); + assertEquals(e, plugins.get(4)); + } + @Test public void plugin_duplicate() {