diff --git a/framework/src/play/classloading/enhancers/ControllersEnhancer.java b/framework/src/play/classloading/enhancers/ControllersEnhancer.java index 10db39cc83..44d66dae71 100644 --- a/framework/src/play/classloading/enhancers/ControllersEnhancer.java +++ b/framework/src/play/classloading/enhancers/ControllersEnhancer.java @@ -4,7 +4,7 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; -import java.util.Stack; +import java.util.Deque; import javassist.CannotCompileException; import javassist.CtClass; import javassist.CtField; @@ -24,7 +24,7 @@ */ public class ControllersEnhancer extends Enhancer { - public static final ThreadLocal> currentAction = new ThreadLocal<>(); + public static final ThreadLocal> currentAction = new ThreadLocal<>(); @Override public void enhanceThisClass(final ApplicationClass applicationClass) throws Exception { diff --git a/framework/src/play/classloading/enhancers/LocalvariablesNamesEnhancer.java b/framework/src/play/classloading/enhancers/LocalvariablesNamesEnhancer.java index 17a26e408e..a4e67f54e5 100644 --- a/framework/src/play/classloading/enhancers/LocalvariablesNamesEnhancer.java +++ b/framework/src/play/classloading/enhancers/LocalvariablesNamesEnhancer.java @@ -1,11 +1,12 @@ package play.classloading.enhancers; import java.lang.reflect.Field; +import java.util.ArrayDeque; import java.util.ArrayList; +import java.util.Deque; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Stack; import javassist.CtClass; import javassist.CtMethod; @@ -172,7 +173,7 @@ public interface LocalVariablesSupport { */ public static class LocalVariablesNamesTracer { - static final ThreadLocal>> localVariables = new ThreadLocal<>(); + static final ThreadLocal>> localVariables = new ThreadLocal<>(); public static void checkEmpty() { if (localVariables.get() != null && !localVariables.get().isEmpty()) { @@ -186,9 +187,9 @@ public static void clear() { public static void enter() { if (localVariables.get() == null) { - localVariables.set(new Stack>()); + localVariables.set(new ArrayDeque<>()); } - localVariables.get().push(new HashMap()); + localVariables.get().push(new HashMap<>()); } public static void exit() { @@ -199,7 +200,7 @@ public static void exit() { } public static Map locals() { - if (localVariables.get() != null && !localVariables.get().empty()) { + if (localVariables.get() != null && !localVariables.get().isEmpty()) { return localVariables.get().peek(); } return new HashMap<>(); @@ -251,7 +252,7 @@ public static List getAllLocalVariableNames(Object o) { if (getLocalVariables().get(variable) == o) { allNames.add(variable); } - if (o != null && o instanceof Number && o.equals(getLocalVariables().get(variable))) { + if (o instanceof Number && o.equals(getLocalVariables().get(variable))) { allNames.add(variable); } } @@ -262,17 +263,17 @@ public static Object getLocalVariable(String variable) { return getLocalVariables().get(variable); } - public static Stack> getLocalVariablesStateBeforeAwait() { - Stack> state = localVariables.get(); + public static Deque> getLocalVariablesStateBeforeAwait() { + Deque> state = localVariables.get(); // must clear the ThreadLocal to prevent destroying the state when exit() is called due to // continuations-suspend - localVariables.set(new Stack>()); + localVariables.set(new ArrayDeque<>()); return state; } - public static void setLocalVariablesStateAfterAwait(Stack> state) { + public static void setLocalVariablesStateAfterAwait(Deque> state) { if (state == null) { - state = new Stack<>(); + state = new ArrayDeque<>(); } localVariables.set(state); } diff --git a/framework/src/play/mvc/ActionInvoker.java b/framework/src/play/mvc/ActionInvoker.java index 32f5ae100d..cc9b045105 100644 --- a/framework/src/play/mvc/ActionInvoker.java +++ b/framework/src/play/mvc/ActionInvoker.java @@ -36,10 +36,10 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import java.util.ArrayDeque; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Stack; import java.util.concurrent.Future; /** @@ -103,7 +103,7 @@ private static void initActionContext(Http.Request request, Http.Response respon Scope.Flash.current.set(Scope.Flash.restore()); CachedBoundActionMethodArgs.init(); - ControllersEnhancer.currentAction.set(new Stack<>()); + ControllersEnhancer.currentAction.set(new ArrayDeque<>()); } public static void invoke(Http.Request request, Http.Response response) { diff --git a/framework/src/play/mvc/Controller.java b/framework/src/play/mvc/Controller.java index 8e9dfe8afd..1e972b9399 100644 --- a/framework/src/play/mvc/Controller.java +++ b/framework/src/play/mvc/Controller.java @@ -7,10 +7,10 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.lang.reflect.Type; +import java.util.Deque; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Stack; import java.util.concurrent.Future; import org.apache.commons.javaflow.Continuation; @@ -1122,7 +1122,7 @@ private static void storeOrRestoreDataStateForContinuations(Boolean isRestoring) // we are restoring after suspend // localVariablesState - Stack> localVariablesState = (Stack>) Http.Request.current().args + Deque> localVariablesState = (Deque>) Http.Request.current().args .remove(ActionInvoker.CONTINUATIONS_STORE_LOCAL_VARIABLE_NAMES); LocalvariablesNamesEnhancer.LocalVariablesNamesTracer.setLocalVariablesStateAfterAwait(localVariablesState); diff --git a/framework/src/play/templates/GroovyTemplate.java b/framework/src/play/templates/GroovyTemplate.java index 6848fb205b..82ed40a35a 100644 --- a/framework/src/play/templates/GroovyTemplate.java +++ b/framework/src/play/templates/GroovyTemplate.java @@ -261,7 +261,7 @@ protected String internalRender(Map args) { binding.setVariable("_response_encoding", currentResponse.encoding); } StringWriter writer = null; - Boolean applyLayouts = false; + boolean applyLayouts = false; // must check if this is the first template being rendered.. // If this template is called from inside another template, diff --git a/framework/src/play/templates/TagContext.java b/framework/src/play/templates/TagContext.java index 7793b6ab00..d6f8aaa833 100644 --- a/framework/src/play/templates/TagContext.java +++ b/framework/src/play/templates/TagContext.java @@ -1,30 +1,33 @@ package play.templates; +import java.util.ArrayDeque; +import java.util.Deque; import java.util.HashMap; +import java.util.Iterator; import java.util.Map; -import java.util.Stack; /** * Tag Context (retrieve who call you) */ public class TagContext { - private static final ThreadLocal> currentStack = new ThreadLocal<>(); + private static final ThreadLocal> currentStack = new ThreadLocal<>(); - public final String tagName; public final Map data = new HashMap<>(); + public final String tagName; + public TagContext(String tagName) { this.tagName = tagName; } public static void init() { - currentStack.set(new Stack()); + currentStack.set(new ArrayDeque<>()); enterTag("ROOT"); } public static void enterTag(String name) { - currentStack.get().add(new TagContext(name)); + currentStack.get().push(new TagContext(name)); } public static void exitTag() { @@ -36,15 +39,22 @@ public static TagContext current() { } public static TagContext parent() { - if(currentStack.get().size() < 2) { - return null; + Iterator it = currentStack.get().iterator(); + if (it.hasNext()) { + // skip + it.next(); + + if (it.hasNext()) { + return it.next(); + } } - return currentStack.get().get(currentStack.get().size()-2); + + return null; } public static boolean hasParentTag(String name) { - for(int i=currentStack.get().size()-1; i>=0; i--) { - if(name.equals(currentStack.get().get(i).tagName)) { + for (TagContext current : currentStack.get()) { + if (name.equals(current.tagName)) { return true; } } @@ -52,19 +62,33 @@ public static boolean hasParentTag(String name) { } public static TagContext parent(String name) { - for(int i=currentStack.get().size()-2; i>=0; i--) { - if(name.equals(currentStack.get().get(i).tagName)) { - return currentStack.get().get(i); + Iterator it = currentStack.get().iterator(); + if (it.hasNext()) { + // skip head + it.next(); + + while (it.hasNext()) { + TagContext parent = it.next(); + if (name.equals(parent.tagName)) { + return parent; + } } } return null; } public static TagContext parent(String... names) { - for (int i = currentStack.get().size() - 2; i >= 0; i--) { - for (String name : names) { - if (name.equals(currentStack.get().get(i).tagName)) { - return currentStack.get().get(i); + Iterator it = currentStack.get().iterator(); + if (it.hasNext()) { + // skip head + it.next(); + + while (it.hasNext()) { + TagContext parent = it.next(); + for (String name : names) { + if (name.equals(parent.tagName)) { + return parent; + } } } } @@ -73,9 +97,7 @@ public static TagContext parent(String... names) { @Override public String toString() { - return tagName+""+data; + return this.tagName + this.data; } - - } diff --git a/framework/src/play/templates/TemplateCompiler.java b/framework/src/play/templates/TemplateCompiler.java index be77c23ed7..d7d91812ce 100644 --- a/framework/src/play/templates/TemplateCompiler.java +++ b/framework/src/play/templates/TemplateCompiler.java @@ -1,6 +1,7 @@ package play.templates; -import java.util.Stack; +import java.util.ArrayDeque; +import java.util.Deque; import play.Logger; import play.vfs.VirtualFile; import play.exceptions.TemplateCompilationException; @@ -29,7 +30,7 @@ public BaseTemplate compile(VirtualFile file) { } protected final StringBuilder compiledSource = new StringBuilder(); - protected final Stack tagsStack = new Stack<>(); + protected final Deque tagsStack = new ArrayDeque<>(); protected BaseTemplate template; protected TemplateParser parser; protected boolean doNextScan = true; @@ -99,7 +100,7 @@ protected void generate(BaseTemplate template) { end(); // Check tags imbrication - if (!tagsStack.empty()) { + if (!tagsStack.isEmpty()) { Tag tag = tagsStack.peek(); throw new TemplateCompilationException(template, tag.startLine, "#{" + tag.name + "} is not closed."); } @@ -139,7 +140,7 @@ protected void markLine(int line) { } protected void println() { - compiledSource.append("\n"); + compiledSource.append('\n'); currentLine++; } diff --git a/framework/test-src/play/templates/TagContextTest.java b/framework/test-src/play/templates/TagContextTest.java new file mode 100644 index 0000000000..214c18abcd --- /dev/null +++ b/framework/test-src/play/templates/TagContextTest.java @@ -0,0 +1,154 @@ +package play.templates; + +import org.junit.After; +import org.junit.BeforeClass; +import org.junit.Test; + +import java.lang.reflect.Field; +import java.util.Deque; + +import static java.util.Arrays.asList; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatNoException; + +public class TagContextTest { + + private static ThreadLocal> CURRENT_STACK; + + @BeforeClass + public static void getCurrentStackThreadLocal() { + assertThatNoException().isThrownBy(() -> { + Field currentStackField = TagContext.class.getDeclaredField("currentStack"); + assertThat(currentStackField) + .extracting(Field::getType) + .isEqualTo(ThreadLocal.class); + + currentStackField.setAccessible(true); + CURRENT_STACK = (ThreadLocal>) currentStackField.get(null); + }); + } + + @After + public void clearCurrentStack() { + CURRENT_STACK.remove(); + } + + @Test + public void lifecycle() { + assertThat(CURRENT_STACK.get()) + .as("before init") + .isNull(); + + TagContext.init(); + assertThat(CURRENT_STACK.get()) + .as("after init()") + .singleElement() + .extracting("tagName") + .isEqualTo("ROOT"); + + TagContext.enterTag("new-parent-tag"); + assertThat(CURRENT_STACK.get()) + .as("after enterTag(\"new-parent-tag\")") + .hasSize(2) + .first() + .extracting("tagName") + .isEqualTo("new-parent-tag"); + + TagContext.enterTag("new-child-tag"); + assertThat(CURRENT_STACK.get()) + .as("after enterTag(\"new-child-tag\")") + .hasSize(3) + .first() + .extracting("tagName") + .isEqualTo("new-child-tag"); + + TagContext.exitTag(); + assertThat(CURRENT_STACK.get()) + .as("after exitTag(): new-child-tag") + .hasSize(2) + .first() + .extracting("tagName") + .isEqualTo("new-parent-tag"); + + TagContext.exitTag(); + assertThat(CURRENT_STACK.get()) + .as("after exitTag(): new-parent-tag") + .singleElement() + .extracting("tagName") + .isEqualTo("ROOT"); + } + + @Test + public void currentShouldReturnLastElement() { + TagContext.init(); + + assertThat(TagContext.current()) + .as("empty stack") + .extracting("tagName") + .isEqualTo("ROOT"); + + asList("grandfather", "father", "child").forEach(tag -> { + TagContext.enterTag(tag); + assertThat(TagContext.current()) + .as("after enterTag(\"%s\")", tag) + .extracting("tagName") + .isEqualTo(tag); + }); + } + + @Test + public void parentShouldSearchTags() { + TagContext.init(); + assertThat(TagContext.parent()).isNull(); + + asList("grandfather", "father", "child").forEach(TagContext::enterTag); + + assertThat(TagContext.parent()) + .as("parent()") + .extracting("tagName") + .isEqualTo("father"); + + assertThat(TagContext.parent("grandfather")) + .as("parent(\"grandfather\")") + .extracting("tagName") + .isEqualTo("grandfather"); + + assertThat(TagContext.parent("grandfather")) + .as("parent(\"father\")") + .extracting("tagName") + .isEqualTo("grandfather"); + + assertThat(TagContext.parent("child")) + .as("parent(\"child\")") + .isNull(); + + assertThat(TagContext.parent("grandpa")) + .as("parent(\"grandpa\")") + .isNull(); + + assertThat(TagContext.parent("child", "grandfather", "father")) + .as("parent(\"child\", \"grandfather\", \"father\")") + .isNotNull() + .extracting("tagName") + .isEqualTo("father"); + + assertThat(TagContext.parent("qwer", "asdf", "zxcv")) + .as("parent(\"qwer\", \"asdf\", \"zxcv\")") + .isNull(); + } + + @Test + public void hasParentShouldSearchTags() { + TagContext.init(); + assertThat(TagContext.hasParentTag("any-tag")).isFalse(); + + asList("grandfather", "father", "child").forEach(TagContext::enterTag); + + // assertThat(TagContext.hasParentTag("child")).isFalse(); + assertThat(TagContext.hasParentTag("child")).isTrue(); + assertThat(TagContext.hasParentTag("father")).isTrue(); + assertThat(TagContext.hasParentTag("grandfather")).isTrue(); + assertThat(TagContext.hasParentTag("any-other-tag")).isFalse(); + } + +} \ No newline at end of file diff --git a/samples-and-tests/just-test-cases/test/tagContexts.test.html b/samples-and-tests/just-test-cases/test/tagContexts.test.html index 37e62bd97e..e9b53b1abb 100644 --- a/samples-and-tests/just-test-cases/test/tagContexts.test.html +++ b/samples-and-tests/just-test-cases/test/tagContexts.test.html @@ -2,7 +2,7 @@ open('@{Application.tagContexts()}') assertTextNotPresent('Yop') - assertTextPresent('[ROOT{_executeNextElse=false}, list{_executeNextElse=true}, if{}, ifnot{}, verbatim{_executeNextElse=true}, else{}]') + assertTextPresent('[else{}, verbatim{_executeNextElse=true}, ifnot{}, if{}, list{_executeNextElse=true}, ROOT{_executeNextElse=false}]') assertTextPresent('Hello World') assertTextPresent('Hello Bab')