From 4406a5faafe6797ae2b809cbfed66f989397d9c5 Mon Sep 17 00:00:00 2001 From: Dimitry Ivanov Date: Wed, 28 Aug 2019 13:24:55 +0300 Subject: [PATCH] Introduce MarkwonVisitorFactory --- .../io/noties/markwon/MarkwonBuilderImpl.java | 8 +++- .../java/io/noties/markwon/MarkwonImpl.java | 14 +++++-- .../noties/markwon/MarkwonVisitorFactory.java | 26 +++++++++++++ .../io/noties/markwon/MarkwonVisitorImpl.java | 5 +++ .../io/noties/markwon/MarkwonImplTest.java | 37 +++++++++++++------ 5 files changed, 73 insertions(+), 17 deletions(-) create mode 100644 markwon-core/src/main/java/io/noties/markwon/MarkwonVisitorFactory.java diff --git a/markwon-core/src/main/java/io/noties/markwon/MarkwonBuilderImpl.java b/markwon-core/src/main/java/io/noties/markwon/MarkwonBuilderImpl.java index 77de961f..1fe21edb 100644 --- a/markwon-core/src/main/java/io/noties/markwon/MarkwonBuilderImpl.java +++ b/markwon-core/src/main/java/io/noties/markwon/MarkwonBuilderImpl.java @@ -104,11 +104,17 @@ class MarkwonBuilderImpl implements Markwon.Builder { final RenderProps renderProps = new RenderPropsImpl(); + // @since 4.1.1-SNAPSHOT + final MarkwonVisitorFactory visitorFactory = MarkwonVisitorFactory.create( + visitorBuilder, + configuration, + renderProps); + return new MarkwonImpl( bufferType, textSetter, parserBuilder.build(), - visitorBuilder.build(configuration, renderProps), + visitorFactory, Collections.unmodifiableList(plugins) ); } diff --git a/markwon-core/src/main/java/io/noties/markwon/MarkwonImpl.java b/markwon-core/src/main/java/io/noties/markwon/MarkwonImpl.java index 9080527f..9d7ffee0 100644 --- a/markwon-core/src/main/java/io/noties/markwon/MarkwonImpl.java +++ b/markwon-core/src/main/java/io/noties/markwon/MarkwonImpl.java @@ -20,7 +20,7 @@ class MarkwonImpl extends Markwon { private final TextView.BufferType bufferType; private final Parser parser; - private final MarkwonVisitor visitor; + private final MarkwonVisitorFactory visitorFactory; // @since 4.1.1-SNAPSHOT private final List plugins; // @since 4.1.0 @@ -31,12 +31,12 @@ class MarkwonImpl extends Markwon { @NonNull TextView.BufferType bufferType, @Nullable TextSetter textSetter, @NonNull Parser parser, - @NonNull MarkwonVisitor visitor, + @NonNull MarkwonVisitorFactory visitorFactory, @NonNull List plugins) { this.bufferType = bufferType; this.textSetter = textSetter; this.parser = parser; - this.visitor = visitor; + this.visitorFactory = visitorFactory; this.plugins = plugins; } @@ -60,16 +60,22 @@ class MarkwonImpl extends Markwon { plugin.beforeRender(node); } + // @since 4.1.1-SNAPSHOT obtain visitor via factory + final MarkwonVisitor visitor = visitorFactory.create(); + node.accept(visitor); for (MarkwonPlugin plugin : plugins) { plugin.afterRender(node, visitor); } + //noinspection UnnecessaryLocalVariable final Spanned spanned = visitor.builder().spannableStringBuilder(); // clear render props and builder after rendering - visitor.clear(); + // @since 4.1.1-SNAPSHOT as we no longer reuse visitor - there is no need to clean it + // we might still do it if we introduce a thread-local storage though +// visitor.clear(); return spanned; } diff --git a/markwon-core/src/main/java/io/noties/markwon/MarkwonVisitorFactory.java b/markwon-core/src/main/java/io/noties/markwon/MarkwonVisitorFactory.java new file mode 100644 index 00000000..429e5080 --- /dev/null +++ b/markwon-core/src/main/java/io/noties/markwon/MarkwonVisitorFactory.java @@ -0,0 +1,26 @@ +package io.noties.markwon; + +import androidx.annotation.NonNull; + +/** + * @since 4.1.1-SNAPSHOT + */ +abstract class MarkwonVisitorFactory { + + @NonNull + abstract MarkwonVisitor create(); + + @NonNull + static MarkwonVisitorFactory create( + @NonNull final MarkwonVisitorImpl.Builder builder, + @NonNull final MarkwonConfiguration configuration, + @NonNull final RenderProps renderProps) { + return new MarkwonVisitorFactory() { + @NonNull + @Override + MarkwonVisitor create() { + return builder.build(configuration, renderProps); + } + }; + } +} diff --git a/markwon-core/src/main/java/io/noties/markwon/MarkwonVisitorImpl.java b/markwon-core/src/main/java/io/noties/markwon/MarkwonVisitorImpl.java index 42db74ac..ea4d6118 100644 --- a/markwon-core/src/main/java/io/noties/markwon/MarkwonVisitorImpl.java +++ b/markwon-core/src/main/java/io/noties/markwon/MarkwonVisitorImpl.java @@ -269,6 +269,11 @@ class MarkwonVisitorImpl implements MarkwonVisitor { @NonNull @Override public Builder on(@NonNull Class node, @Nullable NodeVisitor nodeVisitor) { + + // @since 4.1.1-SNAPSHOT we might actually introduce a local flag to check if it's been built + // and throw an exception here if some modification is requested + // NB, as we might be built from different threads this flag must be synchronized + // we should allow `null` to exclude node from being visited (for example to disable // some functionality) if (nodeVisitor == null) { diff --git a/markwon-core/src/test/java/io/noties/markwon/MarkwonImplTest.java b/markwon-core/src/test/java/io/noties/markwon/MarkwonImplTest.java index 8e24e578..c557cdb7 100644 --- a/markwon-core/src/test/java/io/noties/markwon/MarkwonImplTest.java +++ b/markwon-core/src/test/java/io/noties/markwon/MarkwonImplTest.java @@ -31,6 +31,7 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.RETURNS_MOCKS; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -47,7 +48,7 @@ public class MarkwonImplTest { TextView.BufferType.SPANNABLE, null, mock(Parser.class), - mock(MarkwonVisitor.class), + mock(MarkwonVisitorFactory.class), Collections.singletonList(plugin)); impl.parse("whatever"); @@ -70,7 +71,7 @@ public class MarkwonImplTest { TextView.BufferType.SPANNABLE, null, parser, - mock(MarkwonVisitor.class), + mock(MarkwonVisitorFactory.class), Arrays.asList(first, second)); impl.parse("zero"); @@ -89,6 +90,7 @@ public class MarkwonImplTest { final MarkwonPlugin plugin = mock(MarkwonPlugin.class); + final MarkwonVisitorFactory visitorFactory = mock(MarkwonVisitorFactory.class); final MarkwonVisitor visitor = mock(MarkwonVisitor.class); final SpannableBuilder builder = mock(SpannableBuilder.class); @@ -96,9 +98,10 @@ public class MarkwonImplTest { TextView.BufferType.SPANNABLE, null, mock(Parser.class), - visitor, + visitorFactory, Collections.singletonList(plugin)); + when(visitorFactory.create()).thenReturn(visitor); when(visitor.builder()).thenReturn(builder); final Node node = mock(Node.class); @@ -132,24 +135,30 @@ public class MarkwonImplTest { public void render_clears_visitor() { // each render call should have empty-state visitor (no previous rendering info) + final MarkwonVisitorFactory visitorFactory = mock(MarkwonVisitorFactory.class); final MarkwonVisitor visitor = mock(MarkwonVisitor.class, RETURNS_MOCKS); + when(visitorFactory.create()).thenReturn(visitor); + final MarkwonImpl impl = new MarkwonImpl( TextView.BufferType.SPANNABLE, null, mock(Parser.class), - visitor, + visitorFactory, Collections.emptyList()); impl.render(mock(Node.class)); - verify(visitor, times(1)).clear(); + // obsolete starting with 4.1.1-SNAPSHOT +// verify(visitor, times(1)).clear(); + verify(visitor, never()).clear(); } @Test public void render_props() { // render props are configured properly and cleared after render function + final MarkwonVisitorFactory visitorFactory = mock(MarkwonVisitorFactory.class); final MarkwonVisitor visitor = mock(MarkwonVisitor.class, RETURNS_MOCKS); final RenderProps renderProps = mock(RenderProps.class); @@ -161,6 +170,7 @@ public class MarkwonImplTest { } }).when(visitor).clear(); + when(visitorFactory.create()).thenReturn(visitor); when(visitor.renderProps()).thenReturn(renderProps); final MarkwonPlugin plugin = mock(MarkwonPlugin.class); @@ -169,7 +179,7 @@ public class MarkwonImplTest { TextView.BufferType.SPANNABLE, null, mock(Parser.class), - visitor, + visitorFactory, Collections.singletonList(plugin)); final AtomicBoolean flag = new AtomicBoolean(false); @@ -191,7 +201,9 @@ public class MarkwonImplTest { assertTrue(flag.get()); - verify(renderProps, times(1)).clearAll(); + // obsolete starting with 4.1.1-SNAPSHOT +// verify(renderProps, times(1)).clearAll(); + verify(renderProps, never()).clearAll(); } @Test @@ -205,7 +217,7 @@ public class MarkwonImplTest { TextView.BufferType.EDITABLE, null, mock(Parser.class), - mock(MarkwonVisitor.class, RETURNS_MOCKS), + mock(MarkwonVisitorFactory.class, RETURNS_MOCKS), Collections.singletonList(plugin)); final TextView textView = mock(TextView.class); @@ -252,7 +264,7 @@ public class MarkwonImplTest { TextView.BufferType.SPANNABLE, null, mock(Parser.class), - mock(MarkwonVisitor.class), + mock(MarkwonVisitorFactory.class), plugins); assertTrue("First", impl.hasPlugin(First.class)); @@ -274,7 +286,7 @@ public class MarkwonImplTest { TextView.BufferType.EDITABLE, textSetter, mock(Parser.class), - mock(MarkwonVisitor.class), + mock(MarkwonVisitorFactory.class), Collections.singletonList(plugin)); final TextView textView = mock(TextView.class); @@ -317,7 +329,8 @@ public class MarkwonImplTest { TextView.BufferType.SPANNABLE, null, mock(Parser.class), - mock(MarkwonVisitor.class), plugins); + mock(MarkwonVisitorFactory.class), + plugins); // should be returned assertNotNull(impl.requirePlugin(MarkwonPlugin.class)); @@ -346,7 +359,7 @@ public class MarkwonImplTest { TextView.BufferType.SPANNABLE, null, mock(Parser.class), - mock(MarkwonVisitor.class), + mock(MarkwonVisitorFactory.class), plugins); final List list = impl.getPlugins();