XRENDERING-636: Previous event information in BlockStateChainingListener is inconsistent

* Add unit tests to test previous event information
* Handle group and metadata events
* Only set the previous event information after handling an event

xwiki-rendering-api/pom.xml

@@ -32,7 +32,7 @@
   <packaging>jar</packaging>
   <description>XWiki Rendering - Api</description>
   <properties>
-    <xwiki.jacoco.instructionRatio>0.33</xwiki.jacoco.instructionRatio>
+    <xwiki.jacoco.instructionRatio>0.4</xwiki.jacoco.instructionRatio>
     <!-- Skipping revapi since xwiki-rendering-legacy-api wraps this module and runs checks on it -->
     <xwiki.revapi.skip>true</xwiki.revapi.skip>
   </properties>

xwiki-rendering-api/src/main/java/org/xwiki/rendering/listener/chaining/BlockStateChainingListener.java

@@ -72,7 +72,9 @@ public class BlockStateChainingListener extends AbstractChainingListener impleme
         VERBATIM_STANDALONE,
         WORD,
         FIGURE,
-        FIGURE_CAPTION
+        FIGURE_CAPTION,
+        META_DATA,
+        GROUP
     }
 
     private Event previousEvent = Event.NONE;
@@ -439,9 +441,9 @@ public void endDefinitionTerm()
     @Override
     public void endDocument(MetaData metadata)
     {
-        this.previousEvent = Event.DOCUMENT;
-
         super.endDocument(metadata);
+
+        this.previousEvent = Event.DOCUMENT;
     }
 
     @Override
@@ -491,7 +493,7 @@ public void endListItem(Map<String, String> parameters)
         super.endListItem(parameters);
 
         --this.inlineDepth;
-        this.previousEvent = Event.LIST_ITEM;        
+        this.previousEvent = Event.LIST_ITEM;
     }
 
     @Override
@@ -503,6 +505,32 @@ public void endMacroMarker(String name, Map<String, String> parameters, String c
         --this.macroDepth;
     }
 
+    /**
+     * {@inheritDoc}
+     *
+     * @since 14.0RC1
+     */
+    @Override
+    public void endMetaData(MetaData metadata)
+    {
+        super.endMetaData(metadata);
+
+        this.previousEvent = Event.META_DATA;
+    }
+
+    /**
+     * {@inheritDoc}
+     *
+     * @since 14.0RC1
+     */
+    @Override
+    public void endGroup(Map<String, String> parameters)
+    {
+        super.endGroup(parameters);
+
+        this.previousEvent = Event.GROUP;
+    }
+
     @Override
     public void endParagraph(Map<String, String> parameters)
     {
@@ -628,25 +656,25 @@ public void onRawText(String text, Syntax syntax)
     @Override
     public void onEmptyLines(int count)
     {
-        this.previousEvent = Event.EMPTY_LINES;
-
         super.onEmptyLines(count);
+
+        this.previousEvent = Event.EMPTY_LINES;
     }
 
     @Override
     public void onHorizontalLine(Map<String, String> parameters)
     {
-        this.previousEvent = Event.HORIZONTAL_LINE;
-
         super.onHorizontalLine(parameters);
+
+        this.previousEvent = Event.HORIZONTAL_LINE;
     }
 
     @Override
     public void onId(String name)
     {
-        this.previousEvent = Event.ID;
-
         super.onId(name);
+
+        this.previousEvent = Event.ID;
     }
 
     /**
@@ -657,57 +685,61 @@ public void onId(String name)
     @Override
     public void onImage(ResourceReference reference, boolean freestanding, Map<String, String> parameters)
     {
-        this.previousEvent = Event.IMAGE;
-
         super.onImage(reference, freestanding, parameters);
+
+        this.previousEvent = Event.IMAGE;
     }
 
     @Override
     public void onNewLine()
     {
-        this.previousEvent = Event.NEW_LINE;
-
         super.onNewLine();
+
+        this.previousEvent = Event.NEW_LINE;
     }
 
     @Override
     public void onSpace()
     {
-        this.previousEvent = Event.SPACE;
-
         super.onSpace();
+
+        this.previousEvent = Event.SPACE;
     }
 
     @Override
     public void onSpecialSymbol(char symbol)
     {
-        this.previousEvent = Event.SPECIAL_SYMBOL;
-
         super.onSpecialSymbol(symbol);
+
+        this.previousEvent = Event.SPECIAL_SYMBOL;
     }
 
     @Override
     public void onVerbatim(String content, boolean inline, Map<String, String> parameters)
     {
-        this.previousEvent = Event.VERBATIM_STANDALONE;
-
         super.onVerbatim(content, inline, parameters);
+
+        if (inline) {
+            this.previousEvent = Event.VERBATIM_INLINE;
+        } else {
+            this.previousEvent = Event.VERBATIM_STANDALONE;
+        }
     }
 
     @Override
     public void onWord(String word)
     {
-        this.previousEvent = Event.WORD;
-
         super.onWord(word);
+
+        this.previousEvent = Event.WORD;
     }
 
     @Override
     public void onMacro(String id, Map<String, String> parameters, String content, boolean inline)
     {
-        this.previousEvent = Event.MACRO;
-
         super.onMacro(id, parameters, content, inline);
+
+        this.previousEvent = Event.MACRO;
     }
 
     private static class ListState

xwiki-rendering-api/src/test/java/org/xwiki/rendering/listener/chaining/BlockStateChainingListenerTest.java

+/*
+ * See the NOTICE file distributed with this work for additional
+ * information regarding copyright ownership.
+ *
+ * This is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as
+ * published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This software is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this software; if not, write to the Free
+ * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
+ */
+package org.xwiki.rendering.listener.chaining;
+
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.Arrays;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.apache.commons.text.CaseUtils;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.TestFactory;
+import org.mockito.stubbing.Stubber;
+import org.xwiki.rendering.listener.Format;
+import org.xwiki.rendering.listener.HeaderLevel;
+import org.xwiki.rendering.listener.ListType;
+import org.xwiki.rendering.listener.Listener;
+import org.xwiki.rendering.listener.MetaData;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+/**
+ * Unit tests for {@link BlockStateChainingListener}.
+ *
+ * @version $Id$
+ * @since 14.0RC1
+ */
+public class BlockStateChainingListenerTest
+{
+    private BlockStateChainingListener listener;
+
+    private ChainingListener mockListener;
+
+    @BeforeEach
+    void setUpChain()
+    {
+        ListenerChain chain = new ListenerChain();
+        this.listener = new BlockStateChainingListener(chain);
+        chain.addListener(this.listener);
+        this.mockListener = mock(ChainingListener.class);
+        chain.addListener(this.mockListener);
+    }
+
+    /**
+     * Tests for all "begin/end"-methods if the previous event is correctly set, but only after the end event has been
+     * forwarded in the chain.
+     */
+    @TestFactory
+    Stream<DynamicTest> beginEndMethods()
+    {
+        return Arrays.stream(Listener.class.getMethods())
+            .filter(m -> m.getName().startsWith("begin"))
+            .map(beginMethod ->
+                DynamicTest.dynamicTest(getTestName(beginMethod),
+                    () -> testBeginEndMethod(beginMethod)));
+    }
+
+    /**
+     * Tests for all "on..." methods if the previous event is correctly set, but only after the event has been forwarded
+     * in the chain.
+     */
+    @TestFactory
+    Stream<DynamicTest> onMethods()
+    {
+        return Arrays.stream(Listener.class.getMethods())
+            .filter(m -> m.getName().startsWith("on"))
+            .map(beginMethod ->
+                DynamicTest.dynamicTest(getTestName(beginMethod),
+                    () -> testOnMethod(beginMethod)));
+    }
+
+    private String getTestName(Method method)
+    {
+        return method.getName() + "(" + Arrays.stream(method.getParameterTypes()).map(Class::getName)
+            .collect(Collectors.joining(", ")) + ")";
+    }
+
+    private void testBeginEndMethod(Method beginMethod)
+    {
+        String endMethodName = beginMethod.getName().replace("begin", "end");
+        Class<?>[] parameterClasses = beginMethod.getParameterTypes();
+
+        try {
+            Method endMethod = Listener.class.getMethod(endMethodName, parameterClasses);
+
+            boolean isListItem = beginMethod.getName().equals("beginListItem");
+            boolean isDefinitionItem =
+                beginMethod.getName().equals("beginDefinitionTerm") || beginMethod.getName().equals(
+                    "beginDefinitionDescription");
+
+            if (isListItem) {
+                this.listener.beginList(ListType.NUMBERED, Listener.EMPTY_PARAMETERS);
+            } else if (isDefinitionItem) {
+                this.listener.beginDefinitionList(Listener.EMPTY_PARAMETERS);
+            }
+
+            Object[] parameters = Arrays.stream(parameterClasses).map(this::mockParameter).toArray();
+
+            this.listener.onId("MockID");
+
+            Stubber verifyPreviousAndParentEventStubber = doAnswer(invocation -> {
+                assertEquals(BlockStateChainingListener.Event.ID, this.listener.getPreviousEvent());
+                return null;
+            }).doNothing();
+
+            // Assert that in the begin method, the previous event are unchanged
+            beginMethod.invoke(verifyPreviousAndParentEventStubber.when(this.mockListener), parameters);
+
+            // Actually call the begin method.
+            beginMethod.invoke(this.listener, parameters);
+
+            // Verify the mock listener in the chain has been called.
+            beginMethod.invoke(verify(this.mockListener), parameters);
+
+            // Assert that in the end method, the previous event is unchanged
+            endMethod.invoke(verifyPreviousAndParentEventStubber.when(this.mockListener), parameters);
+
+            // Actually call the end method.
+            endMethod.invoke(this.listener, parameters);
+
+            // Verify the mock listener in the chain has been called.
+            endMethod.invoke(verify(this.mockListener), parameters);
+
+            // Verify that the previous event has been set to the event corresponding to the current methods.
+            String previousEventName = this.listener.getPreviousEvent().name();
+            String previousEventCamelCase = CaseUtils.toCamelCase(previousEventName, true, '_');
+            assertEquals(beginMethod.getName(), "begin" + previousEventCamelCase,
+                "Wrong event " + previousEventName + " generated for " + beginMethod.getName());
+
+            if (isDefinitionItem) {
+                this.listener.endDefinitionList(Listener.EMPTY_PARAMETERS);
+            } else if (isListItem) {
+                this.listener.endList(ListType.NUMBERED, Listener.EMPTY_PARAMETERS);
+            }
+        } catch (NoSuchMethodException e) {
+            fail("Expected end method " + endMethodName + " for " + beginMethod.getName() + " not found: "
+                + e.getMessage());
+        } catch (InvocationTargetException e) {
+            fail("Listener method has thrown exception: " + e.getMessage());
+        } catch (IllegalAccessException e) {
+            fail("Listener method not callable: " + e.getMessage());
+        }
+    }
+
+    private void testOnMethod(Method method)
+    {
+        this.listener.beginDocument(MetaData.EMPTY);
+
+        // Make sure the previous event is one that we never trigger.
+        this.listener.beginParagraph(Listener.EMPTY_PARAMETERS);
+        this.listener.endParagraph(Listener.EMPTY_PARAMETERS);
+
+        Object[] parameters = Arrays.stream(method.getParameterTypes()).map(this::mockParameter).toArray();
+
+        try {
+            // Verify that the next in the chain still gets the old previous event.
+            method.invoke(
+                doAnswer(invocationOnMock -> {
+                    assertEquals(BlockStateChainingListener.Event.PARAGRAPH, this.listener.getPreviousEvent());
+                    return null;
+                })
+                    .doThrow(new AssertionError("Listener must only be called once"))
+                    .when(this.mockListener),
+                parameters);
+
+            // Actually call the listener method.
+            method.invoke(this.listener, parameters);
+
+            // Verify that the call has been correctly forwarded.
+            method.invoke(verify(this.mockListener), parameters);
+        } catch (InvocationTargetException e) {
+            fail("Listener method has thrown exception: " + e.getMessage());
+        } catch (IllegalAccessException e) {
+            fail("Listener method not callable: " + e.getMessage());
+        }
+
+        // Check if the previous event is the expected event.
+        String previousEventName = this.listener.getPreviousEvent().name();
+
+        // Verbatim has two events, as our mock boolean is true we always get the inline event.
+        if (this.listener.getPreviousEvent().equals(BlockStateChainingListener.Event.VERBATIM_INLINE)) {
+            previousEventName = "VERBATIM";
+        }
+
+        String eventCamelCaseName = CaseUtils.toCamelCase(previousEventName, true, '_');
+        assertEquals(method.getName(), "on" + eventCamelCaseName, "Previous event " + previousEventName + " "
+            + "does not match method name " + method.getName());
+
+        this.listener.endDocument(MetaData.EMPTY);
+    }
+
+    /**
+     * @param classToMock The class to return a mock object for.
+     * @return Either a mock object or in the case of an enum or primitive type a concrete value.
+     */
+    private Object mockParameter(Class<?> classToMock)
+    {
+        if (classToMock.equals(Format.class)) {
+            return Format.BOLD;
+        }
+
+        if (classToMock.equals(ListType.class)) {
+            return ListType.BULLETED;
+        }
+
+        if (classToMock.equals(HeaderLevel.class)) {
+            return HeaderLevel.LEVEL1;
+        }
+
+        if (classToMock.equals(String.class)) {
+            return "Mock";
+        }
+
+        if (classToMock.equals(boolean.class)) {
+            return true;
+        }
+
+        if (classToMock.equals(char.class)) {
+            return '{';
+        }
+
+        if (classToMock.equals(int.class)) {
+            return 42;
+        }
+
+        return mock(classToMock);
+    }
+}