Uploaded image for project: 'Struts 2'
  1. Struts 2
  2. WW-4594

Configure TilesDefs by annotating Actions

    Details

    • Type: New Feature
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.5
    • Component/s: Plugin - Tiles
    • Labels:
      None

      Description

      Large applications often consist of hundrets of actions. IMO it is a major benefit of struts-convention-plugin that it is not necessary to configure all those actions in xml files. As xml files would become huge and hard to maintain. The same applies to tiles xml files. Currently it is necessary to configure tiles-definitions for all those actions in xml files which makes them huge and hard to maintain.

      It would be great if configuration of tiles-definitions could be done by annotating actions, in a similar way actions can be configured with annotations from convention-plugin.

      This somewhat relates to WW-3937 and WW-4161

        Issue Links

          Activity

          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Struts-JDK7-master #422 (See https://builds.apache.org/job/Struts-JDK7-master/422/)
          Added tiles annotations, see WW-4594. (cnenning: rev 9ac326aa2458fe43140c1b13b61c87752d282e3d)

          • plugins/tiles/src/main/java/org/apache/struts2/tiles/annotation/TilesDefinition.java
          • plugins/tiles/src/main/java/org/apache/struts2/tiles/annotation/TilesAddAttribute.java
          • plugins/tiles/src/main/java/org/apache/struts2/tiles/annotation/TilesPutListAttribute.java
          • plugins/tiles/src/main/java/org/apache/struts2/tiles/annotation/TilesDefinitions.java
          • plugins/tiles/src/main/java/org/apache/struts2/tiles/annotation/TilesAddListAttribute.java
          • plugins/tiles/src/main/java/org/apache/struts2/views/tiles/TilesResult.java
          • plugins/tiles/src/main/java/org/apache/struts2/tiles/annotation/TilesPutAttribute.java
          • plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsTilesAnnotationProcessor.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Struts-JDK7-master #422 (See https://builds.apache.org/job/Struts-JDK7-master/422/ ) Added tiles annotations, see WW-4594 . (cnenning: rev 9ac326aa2458fe43140c1b13b61c87752d282e3d) plugins/tiles/src/main/java/org/apache/struts2/tiles/annotation/TilesDefinition.java plugins/tiles/src/main/java/org/apache/struts2/tiles/annotation/TilesAddAttribute.java plugins/tiles/src/main/java/org/apache/struts2/tiles/annotation/TilesPutListAttribute.java plugins/tiles/src/main/java/org/apache/struts2/tiles/annotation/TilesDefinitions.java plugins/tiles/src/main/java/org/apache/struts2/tiles/annotation/TilesAddListAttribute.java plugins/tiles/src/main/java/org/apache/struts2/views/tiles/TilesResult.java plugins/tiles/src/main/java/org/apache/struts2/tiles/annotation/TilesPutAttribute.java plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsTilesAnnotationProcessor.java
          Hide
          cn42 Christoph Nenning added a comment -

          yes, i must tell everybody how great and awesome this stuff is !!!!

          Show
          cn42 Christoph Nenning added a comment - yes, i must tell everybody how great and awesome this stuff is !!!!
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          Could you add a note to the docs [1] plus some example configuration (based on what you already have done)
          [1] https://cwiki.apache.org/confluence/display/WW/Tiles+Plugin

          Show
          lukaszlenart Lukasz Lenart added a comment - Could you add a note to the docs [1] plus some example configuration (based on what you already have done) [1] https://cwiki.apache.org/confluence/display/WW/Tiles+Plugin
          Hide
          cn42 Christoph Nenning added a comment -

          Annotations have been added

          Show
          cn42 Christoph Nenning added a comment - Annotations have been added
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/struts/pull/85

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/struts/pull/85
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 15688132eff8a5f718ac24794822133fb3fe2d35 in struts's branch refs/heads/master from cnenning
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=1568813 ]

          WW-4594 Merges #85 which adds annotations to configure tiles

          Show
          jira-bot ASF subversion and git services added a comment - Commit 15688132eff8a5f718ac24794822133fb3fe2d35 in struts's branch refs/heads/master from cnenning [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=1568813 ] WW-4594 Merges #85 which adds annotations to configure tiles
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on the pull request:

          https://github.com/apache/struts/pull/85#issuecomment-179082503

          Great!

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on the pull request: https://github.com/apache/struts/pull/85#issuecomment-179082503 Great!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user cnenning commented on the pull request:

          https://github.com/apache/struts/pull/85#issuecomment-179082240

          Alright, thanks for reviewing. I agree with your findings and pushed updates.

          I'm going to merge it later.

          Show
          githubbot ASF GitHub Bot added a comment - Github user cnenning commented on the pull request: https://github.com/apache/struts/pull/85#issuecomment-179082240 Alright, thanks for reviewing. I agree with your findings and pushed updates. I'm going to merge it later.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on the pull request:

          https://github.com/apache/struts/pull/85#issuecomment-176859924

          Left some minor comment, this looks solid, great job!

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on the pull request: https://github.com/apache/struts/pull/85#issuecomment-176859924 Left some minor comment, this looks solid, great job!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on a diff in the pull request:

          https://github.com/apache/struts/pull/85#discussion_r51285182

          — Diff: plugins/tiles/src/test/java/org/apache/struts2/tiles/TilesTestActionSingleAnnotation.java —
          @@ -0,0 +1,49 @@
          +package org.apache.struts2.tiles;
          +
          +import org.apache.struts2.tiles.annotation.TilesAddAttribute;
          +import org.apache.struts2.tiles.annotation.TilesAddListAttribute;
          +import org.apache.struts2.tiles.annotation.TilesDefinition;
          +import org.apache.struts2.tiles.annotation.TilesPutAttribute;
          +import org.apache.struts2.tiles.annotation.TilesPutListAttribute;
          +
          +@TilesDefinition(
          + name = "definition-name",
          + extend = "base-definition",
          + preparer = "preparer",
          + role = "role",
          + template = "template",
          + templateExpression = "templ*",
          + templateType = "type",
          + putAttributes =

          { + @TilesPutAttribute( + cascade = true, + expression = "lang:expr", + name = "put-attr", + role = "attr-role", + type = "attr-type", + value = "attr-val") + }

          ,
          + putListAttributes = {
          + @TilesPutListAttribute(
          + cascade = true,
          + inherit = true,
          + name = "list-name",
          + role = "list-role",
          + addAttributes =

          { + @TilesAddAttribute( + expression = "list-attr-expr", + role = "list-attr-role", + type = "list-attr-type", + value = "list-attr-val") + }

          ,
          + addListAttributes = {
          + @TilesAddListAttribute(
          + role = "list-list-attr-role",
          + addAttributes =

          {@TilesAddAttribute("list-list-add-attr")}

          )
          + }
          + )
          + }
          +)
          — End diff –

          Extreme example

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on a diff in the pull request: https://github.com/apache/struts/pull/85#discussion_r51285182 — Diff: plugins/tiles/src/test/java/org/apache/struts2/tiles/TilesTestActionSingleAnnotation.java — @@ -0,0 +1,49 @@ +package org.apache.struts2.tiles; + +import org.apache.struts2.tiles.annotation.TilesAddAttribute; +import org.apache.struts2.tiles.annotation.TilesAddListAttribute; +import org.apache.struts2.tiles.annotation.TilesDefinition; +import org.apache.struts2.tiles.annotation.TilesPutAttribute; +import org.apache.struts2.tiles.annotation.TilesPutListAttribute; + +@TilesDefinition( + name = "definition-name", + extend = "base-definition", + preparer = "preparer", + role = "role", + template = "template", + templateExpression = "templ*", + templateType = "type", + putAttributes = { + @TilesPutAttribute( + cascade = true, + expression = "lang:expr", + name = "put-attr", + role = "attr-role", + type = "attr-type", + value = "attr-val") + } , + putListAttributes = { + @TilesPutListAttribute( + cascade = true, + inherit = true, + name = "list-name", + role = "list-role", + addAttributes = { + @TilesAddAttribute( + expression = "list-attr-expr", + role = "list-attr-role", + type = "list-attr-type", + value = "list-attr-val") + } , + addListAttributes = { + @TilesAddListAttribute( + role = "list-list-attr-role", + addAttributes = {@TilesAddAttribute("list-list-add-attr")} ) + } + ) + } +) — End diff – Extreme example
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on a diff in the pull request:

          https://github.com/apache/struts/pull/85#discussion_r51285068

          — Diff: plugins/tiles/src/test/java/org/apache/struts2/tiles/StrutsTilesAnnotationProcessorTest.java —
          @@ -0,0 +1,148 @@
          +package org.apache.struts2.tiles;
          +
          +import java.util.List;
          +import java.util.Set;
          +
          +import org.apache.struts2.tiles.annotation.TilesDefinition;
          +import org.apache.tiles.Attribute;
          +import org.apache.tiles.Definition;
          +import org.apache.tiles.Expression;
          +import org.junit.Test;
          +
          +import org.junit.Assert;
          +
          +public class StrutsTilesAnnotationProcessorTest {
          +
          + @Test
          + public void findAnnotationSingleAction()

          { + StrutsTilesAnnotationProcessor annotationProcessor = new StrutsTilesAnnotationProcessor(); + TilesDefinition tilesDefinition = annotationProcessor.findAnnotation(new TilesTestActionSingleAnnotation(), null); + Assert.assertNotNull(tilesDefinition); + Assert.assertEquals("definition-name", tilesDefinition.name()); + }

          +
          + @Test
          + public void findAnnotationMultipleActionNameNull()

          { + StrutsTilesAnnotationProcessor annotationProcessor = new StrutsTilesAnnotationProcessor(); + TilesDefinition tilesDefinition = annotationProcessor.findAnnotation(new TilesTestActionMultipleAnnotations(), null); + Assert.assertNotNull(tilesDefinition); + Assert.assertEquals("def1", tilesDefinition.name()); + }

          +
          + @Test
          + public void findAnnotationMultipleActionNameGiven()

          { + StrutsTilesAnnotationProcessor annotationProcessor = new StrutsTilesAnnotationProcessor(); + TilesDefinition tilesDefinition = annotationProcessor.findAnnotation(new TilesTestActionMultipleAnnotations(), "def2"); + Assert.assertNotNull(tilesDefinition); + Assert.assertEquals("def2", tilesDefinition.name()); + }

          +
          + @Test
          + public void findAnnotationMultipleActionNotFound()

          { + StrutsTilesAnnotationProcessor annotationProcessor = new StrutsTilesAnnotationProcessor(); + TilesDefinition tilesDefinition = annotationProcessor.findAnnotation(new TilesTestActionMultipleAnnotations(), "def3"); + Assert.assertNull(tilesDefinition); + }

          +
          + @Test
          + public void buildDefiniton()

          { + StrutsTilesAnnotationProcessor annotationProcessor = new StrutsTilesAnnotationProcessor(); + TilesDefinition tilesDefinition = annotationProcessor.findAnnotation(new TilesTestActionSingleAnnotation(), null); + + Definition definition = annotationProcessor.buildTilesDefinition("tileName", tilesDefinition); + + Assert.assertNotNull(definition); + Assert.assertEquals("tileName", definition.getName()); + Assert.assertEquals("preparer", definition.getPreparer()); + Assert.assertEquals("base-definition", definition.getExtends()); + Attribute templateAttribute = definition.getTemplateAttribute(); + Assert.assertEquals("template", templateAttribute.getValue()); + Assert.assertEquals("type", templateAttribute.getRenderer()); + Assert.assertEquals("role", templateAttribute.getRole()); + Expression definitionExpressionObject = templateAttribute.getExpressionObject(); + Assert.assertEquals("templ*", definitionExpressionObject.getExpression()); + Assert.assertNull(definitionExpressionObject.getLanguage()); + + Attribute putAttribute = definition.getAttribute("put-attr"); + Assert.assertNotNull(putAttribute); + Assert.assertEquals("attr-val", putAttribute.getValue()); + Assert.assertEquals("attr-type", putAttribute.getRenderer()); + Assert.assertEquals("attr-role", putAttribute.getRole()); + Expression putAttrExpressionObject = putAttribute.getExpressionObject(); + Assert.assertEquals("expr", putAttrExpressionObject.getExpression()); + Assert.assertEquals("lang", putAttrExpressionObject.getLanguage()); + + Attribute listAttribute = definition.getAttribute("list-name"); + Assert.assertEquals("list-role", listAttribute.getRole()); + List<Attribute> listValue = getListValue(listAttribute); + Assert.assertEquals(2, listValue.size()); + + Attribute addAttribute = listValue.get(0); + Assert.assertEquals("list-attr-role", addAttribute.getRole()); + Assert.assertEquals("list-attr-val", addAttribute.getValue()); + Assert.assertEquals("list-attr-type", addAttribute.getRenderer()); + Expression addAttrExpressionObject = addAttribute.getExpressionObject(); + Assert.assertEquals("list-attr-expr", addAttrExpressionObject.getExpression()); + + Attribute addListAttribute = listValue.get(1); + Assert.assertEquals("list-list-attr-role", addListAttribute.getRole()); + List<Attribute> addListValue = getListValue(addListAttribute); + Assert.assertEquals(1, addListValue.size()); + Assert.assertEquals("list-list-add-attr", addListValue.get(0).getValue()); + + Set<String> cascadedAttributeNames = definition.getCascadedAttributeNames(); + Assert.assertEquals(2, cascadedAttributeNames.size()); + Assert.assertTrue(cascadedAttributeNames.contains("put-attr")); + Assert.assertTrue(cascadedAttributeNames.contains("list-name")); + }

          +
          + @Test
          + public void buildDefinitonAllEmpty() {
          + StrutsTilesAnnotationProcessor annotationProcessor = new StrutsTilesAnnotationProcessor();
          + TilesDefinition tilesDefinition = annotationProcessor.findAnnotation(new TilesTestActionSingleAnnotationAllEmpty(), null);
          +
          + Definition definition = annotationProcessor.buildTilesDefinition(null, tilesDefinition);
          +
          + Assert.assertNotNull(definition);
          + Assert.assertNull(definition.getName());
          + Assert.assertNull(definition.getPreparer());
          + Assert.assertNull(definition.getExtends());
          + Attribute templateAttribute = definition.getTemplateAttribute();
          + Assert.assertNull(templateAttribute.getValue());
          + //Assert.assertNull(templateAttribute.getRenderer());
          — End diff –

          If not used maybe it can be dropped?

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on a diff in the pull request: https://github.com/apache/struts/pull/85#discussion_r51285068 — Diff: plugins/tiles/src/test/java/org/apache/struts2/tiles/StrutsTilesAnnotationProcessorTest.java — @@ -0,0 +1,148 @@ +package org.apache.struts2.tiles; + +import java.util.List; +import java.util.Set; + +import org.apache.struts2.tiles.annotation.TilesDefinition; +import org.apache.tiles.Attribute; +import org.apache.tiles.Definition; +import org.apache.tiles.Expression; +import org.junit.Test; + +import org.junit.Assert; + +public class StrutsTilesAnnotationProcessorTest { + + @Test + public void findAnnotationSingleAction() { + StrutsTilesAnnotationProcessor annotationProcessor = new StrutsTilesAnnotationProcessor(); + TilesDefinition tilesDefinition = annotationProcessor.findAnnotation(new TilesTestActionSingleAnnotation(), null); + Assert.assertNotNull(tilesDefinition); + Assert.assertEquals("definition-name", tilesDefinition.name()); + } + + @Test + public void findAnnotationMultipleActionNameNull() { + StrutsTilesAnnotationProcessor annotationProcessor = new StrutsTilesAnnotationProcessor(); + TilesDefinition tilesDefinition = annotationProcessor.findAnnotation(new TilesTestActionMultipleAnnotations(), null); + Assert.assertNotNull(tilesDefinition); + Assert.assertEquals("def1", tilesDefinition.name()); + } + + @Test + public void findAnnotationMultipleActionNameGiven() { + StrutsTilesAnnotationProcessor annotationProcessor = new StrutsTilesAnnotationProcessor(); + TilesDefinition tilesDefinition = annotationProcessor.findAnnotation(new TilesTestActionMultipleAnnotations(), "def2"); + Assert.assertNotNull(tilesDefinition); + Assert.assertEquals("def2", tilesDefinition.name()); + } + + @Test + public void findAnnotationMultipleActionNotFound() { + StrutsTilesAnnotationProcessor annotationProcessor = new StrutsTilesAnnotationProcessor(); + TilesDefinition tilesDefinition = annotationProcessor.findAnnotation(new TilesTestActionMultipleAnnotations(), "def3"); + Assert.assertNull(tilesDefinition); + } + + @Test + public void buildDefiniton() { + StrutsTilesAnnotationProcessor annotationProcessor = new StrutsTilesAnnotationProcessor(); + TilesDefinition tilesDefinition = annotationProcessor.findAnnotation(new TilesTestActionSingleAnnotation(), null); + + Definition definition = annotationProcessor.buildTilesDefinition("tileName", tilesDefinition); + + Assert.assertNotNull(definition); + Assert.assertEquals("tileName", definition.getName()); + Assert.assertEquals("preparer", definition.getPreparer()); + Assert.assertEquals("base-definition", definition.getExtends()); + Attribute templateAttribute = definition.getTemplateAttribute(); + Assert.assertEquals("template", templateAttribute.getValue()); + Assert.assertEquals("type", templateAttribute.getRenderer()); + Assert.assertEquals("role", templateAttribute.getRole()); + Expression definitionExpressionObject = templateAttribute.getExpressionObject(); + Assert.assertEquals("templ*", definitionExpressionObject.getExpression()); + Assert.assertNull(definitionExpressionObject.getLanguage()); + + Attribute putAttribute = definition.getAttribute("put-attr"); + Assert.assertNotNull(putAttribute); + Assert.assertEquals("attr-val", putAttribute.getValue()); + Assert.assertEquals("attr-type", putAttribute.getRenderer()); + Assert.assertEquals("attr-role", putAttribute.getRole()); + Expression putAttrExpressionObject = putAttribute.getExpressionObject(); + Assert.assertEquals("expr", putAttrExpressionObject.getExpression()); + Assert.assertEquals("lang", putAttrExpressionObject.getLanguage()); + + Attribute listAttribute = definition.getAttribute("list-name"); + Assert.assertEquals("list-role", listAttribute.getRole()); + List<Attribute> listValue = getListValue(listAttribute); + Assert.assertEquals(2, listValue.size()); + + Attribute addAttribute = listValue.get(0); + Assert.assertEquals("list-attr-role", addAttribute.getRole()); + Assert.assertEquals("list-attr-val", addAttribute.getValue()); + Assert.assertEquals("list-attr-type", addAttribute.getRenderer()); + Expression addAttrExpressionObject = addAttribute.getExpressionObject(); + Assert.assertEquals("list-attr-expr", addAttrExpressionObject.getExpression()); + + Attribute addListAttribute = listValue.get(1); + Assert.assertEquals("list-list-attr-role", addListAttribute.getRole()); + List<Attribute> addListValue = getListValue(addListAttribute); + Assert.assertEquals(1, addListValue.size()); + Assert.assertEquals("list-list-add-attr", addListValue.get(0).getValue()); + + Set<String> cascadedAttributeNames = definition.getCascadedAttributeNames(); + Assert.assertEquals(2, cascadedAttributeNames.size()); + Assert.assertTrue(cascadedAttributeNames.contains("put-attr")); + Assert.assertTrue(cascadedAttributeNames.contains("list-name")); + } + + @Test + public void buildDefinitonAllEmpty() { + StrutsTilesAnnotationProcessor annotationProcessor = new StrutsTilesAnnotationProcessor(); + TilesDefinition tilesDefinition = annotationProcessor.findAnnotation(new TilesTestActionSingleAnnotationAllEmpty(), null); + + Definition definition = annotationProcessor.buildTilesDefinition(null, tilesDefinition); + + Assert.assertNotNull(definition); + Assert.assertNull(definition.getName()); + Assert.assertNull(definition.getPreparer()); + Assert.assertNull(definition.getExtends()); + Attribute templateAttribute = definition.getTemplateAttribute(); + Assert.assertNull(templateAttribute.getValue()); + //Assert.assertNull(templateAttribute.getRenderer()); — End diff – If not used maybe it can be dropped?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on a diff in the pull request:

          https://github.com/apache/struts/pull/85#discussion_r51285002

          — Diff: plugins/tiles/src/main/java/org/apache/struts2/views/tiles/TilesResult.java —
          @@ -111,6 +133,31 @@ public void doExecute(String location, ActionInvocation invocation) throws Excep

          Request request = new ServletRequest(applicationContext, httpRequest, httpResponse);

          + boolean definitionValid = false;
          + try {
          + LOG.debug("checking if tiles definition exists '{}'", location);
          + definitionValid = container.isValidDefinition(location, request);
          + } catch (TilesException e)

          { + LOG.warn("got TilesException while checking if definiton exists, ignoring it", e); + }

          + if (!definitionValid) {
          + if (tilesDefinition == null) {
          + // tilesDefinition not found yet, search in action
          — End diff –

          The same here, `LOG.trace`?

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on a diff in the pull request: https://github.com/apache/struts/pull/85#discussion_r51285002 — Diff: plugins/tiles/src/main/java/org/apache/struts2/views/tiles/TilesResult.java — @@ -111,6 +133,31 @@ public void doExecute(String location, ActionInvocation invocation) throws Excep Request request = new ServletRequest(applicationContext, httpRequest, httpResponse); + boolean definitionValid = false; + try { + LOG.debug("checking if tiles definition exists '{}'", location); + definitionValid = container.isValidDefinition(location, request); + } catch (TilesException e) { + LOG.warn("got TilesException while checking if definiton exists, ignoring it", e); + } + if (!definitionValid) { + if (tilesDefinition == null) { + // tilesDefinition not found yet, search in action — End diff – The same here, `LOG.trace`?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on a diff in the pull request:

          https://github.com/apache/struts/pull/85#discussion_r51284943

          — Diff: plugins/tiles/src/main/java/org/apache/struts2/views/tiles/TilesResult.java —
          @@ -99,6 +109,18 @@ public TilesResult(String location) {

          • HTTP request.
            */
            public void doExecute(String location, ActionInvocation invocation) throws Exception {
            + StrutsTilesAnnotationProcessor annotationProcessor = new StrutsTilesAnnotationProcessor();
            + TilesDefinition tilesDefinition = null;
            + Object action = invocation.getAction();
            + String actionName = invocation.getInvocationContext().getName();
            +
            + if (StringUtils.isEmpty(location)) {
            + // location not set -> action must have one @TilesDefinition
              • End diff –

          Could you turn this comment into a `LOG.trace`?

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on a diff in the pull request: https://github.com/apache/struts/pull/85#discussion_r51284943 — Diff: plugins/tiles/src/main/java/org/apache/struts2/views/tiles/TilesResult.java — @@ -99,6 +109,18 @@ public TilesResult(String location) { HTTP request. */ public void doExecute(String location, ActionInvocation invocation) throws Exception { + StrutsTilesAnnotationProcessor annotationProcessor = new StrutsTilesAnnotationProcessor(); + TilesDefinition tilesDefinition = null; + Object action = invocation.getAction(); + String actionName = invocation.getInvocationContext().getName(); + + if (StringUtils.isEmpty(location)) { + // location not set -> action must have one @TilesDefinition End diff – Could you turn this comment into a `LOG.trace`?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on a diff in the pull request:

          https://github.com/apache/struts/pull/85#discussion_r51284785

          — Diff: apps/showcase/src/main/java/org/apache/struts2/showcase/tiles/TilesAnnotationsAction.java —
          @@ -0,0 +1,21 @@
          +package org.apache.struts2.showcase.tiles;
          — End diff –

          Could you add license header to top of the file?

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on a diff in the pull request: https://github.com/apache/struts/pull/85#discussion_r51284785 — Diff: apps/showcase/src/main/java/org/apache/struts2/showcase/tiles/TilesAnnotationsAction.java — @@ -0,0 +1,21 @@ +package org.apache.struts2.showcase.tiles; — End diff – Could you add license header to top of the file?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on a diff in the pull request:

          https://github.com/apache/struts/pull/85#discussion_r51284673

          — Diff: apps/showcase/src/main/java/org/apache/struts2/showcase/tiles/TilesAnnotationsAction.java —
          @@ -0,0 +1,21 @@
          +package org.apache.struts2.showcase.tiles;
          +
          +import org.apache.struts2.convention.annotation.Namespace;
          +import org.apache.struts2.convention.annotation.ParentPackage;
          +import org.apache.struts2.convention.annotation.Result;
          +import org.apache.struts2.tiles.annotation.TilesDefinition;
          +import org.apache.struts2.tiles.annotation.TilesPutAttribute;
          +
          +import com.opensymphony.xwork2.ActionSupport;
          +
          +@Namespace("/tiles")
          +@ParentPackage("tiles")
          +@Result(name = "success", type="tiles")
          +@TilesDefinition(extend = "showcase.annotations", putAttributes =

          { + @TilesPutAttribute(name = "header", value = "/WEB-INF/tiles/header.jsp"), + @TilesPutAttribute(name = "body", value = "/WEB-INF/tiles/body.ftl"), }

          )
          — End diff –

          I think the last comma isn't needed and also what do you think about formatting like this:
          ```java
          @Namespace("/tiles")
          @ParentPackage("tiles")
          @Result(name = "success", type="tiles")
          @TilesDefinition(extend = "showcase.annotations", putAttributes =

          { @TilesPutAttribute(name = "header", value = "/WEB-INF/tiles/header.jsp"), @TilesPutAttribute(name = "body", value = "/WEB-INF/tiles/body.ftl") }

          )
          ```
          ?

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on a diff in the pull request: https://github.com/apache/struts/pull/85#discussion_r51284673 — Diff: apps/showcase/src/main/java/org/apache/struts2/showcase/tiles/TilesAnnotationsAction.java — @@ -0,0 +1,21 @@ +package org.apache.struts2.showcase.tiles; + +import org.apache.struts2.convention.annotation.Namespace; +import org.apache.struts2.convention.annotation.ParentPackage; +import org.apache.struts2.convention.annotation.Result; +import org.apache.struts2.tiles.annotation.TilesDefinition; +import org.apache.struts2.tiles.annotation.TilesPutAttribute; + +import com.opensymphony.xwork2.ActionSupport; + +@Namespace("/tiles") +@ParentPackage("tiles") +@Result(name = "success", type="tiles") +@TilesDefinition(extend = "showcase.annotations", putAttributes = { + @TilesPutAttribute(name = "header", value = "/WEB-INF/tiles/header.jsp"), + @TilesPutAttribute(name = "body", value = "/WEB-INF/tiles/body.ftl"), } ) — End diff – I think the last comma isn't needed and also what do you think about formatting like this: ```java @Namespace("/tiles") @ParentPackage("tiles") @Result(name = "success", type="tiles") @TilesDefinition(extend = "showcase.annotations", putAttributes = { @TilesPutAttribute(name = "header", value = "/WEB-INF/tiles/header.jsp"), @TilesPutAttribute(name = "body", value = "/WEB-INF/tiles/body.ftl") } ) ``` ?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user cnenning commented on a diff in the pull request:

          https://github.com/apache/struts/pull/85#discussion_r50693282

          — Diff: plugins/tiles/src/test/java/org/apache/struts2/tiles/TestStrutsTilesAnnotationProcessor.java —
          @@ -0,0 +1,148 @@
          +package org.apache.struts2.tiles;
          +
          +import java.util.List;
          +import java.util.Set;
          +
          +import org.apache.struts2.tiles.annotation.TilesDefinition;
          +import org.apache.tiles.Attribute;
          +import org.apache.tiles.Definition;
          +import org.apache.tiles.Expression;
          +import org.junit.Test;
          +
          +import org.junit.Assert;
          +
          +public class TestStrutsTilesAnnotationProcessor {
          — End diff –

          oh, of course :sweat_smile:

          Show
          githubbot ASF GitHub Bot added a comment - Github user cnenning commented on a diff in the pull request: https://github.com/apache/struts/pull/85#discussion_r50693282 — Diff: plugins/tiles/src/test/java/org/apache/struts2/tiles/TestStrutsTilesAnnotationProcessor.java — @@ -0,0 +1,148 @@ +package org.apache.struts2.tiles; + +import java.util.List; +import java.util.Set; + +import org.apache.struts2.tiles.annotation.TilesDefinition; +import org.apache.tiles.Attribute; +import org.apache.tiles.Definition; +import org.apache.tiles.Expression; +import org.junit.Test; + +import org.junit.Assert; + +public class TestStrutsTilesAnnotationProcessor { — End diff – oh, of course :sweat_smile:
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on a diff in the pull request:

          https://github.com/apache/struts/pull/85#discussion_r50692670

          — Diff: plugins/tiles/src/test/java/org/apache/struts2/tiles/TestStrutsTilesAnnotationProcessor.java —
          @@ -0,0 +1,148 @@
          +package org.apache.struts2.tiles;
          +
          +import java.util.List;
          +import java.util.Set;
          +
          +import org.apache.struts2.tiles.annotation.TilesDefinition;
          +import org.apache.tiles.Attribute;
          +import org.apache.tiles.Definition;
          +import org.apache.tiles.Expression;
          +import org.junit.Test;
          +
          +import org.junit.Assert;
          +
          +public class TestStrutsTilesAnnotationProcessor {
          — End diff –

          Can we keep convention and instead of `TestStrutsTilesAnnotationProcessor` use `StrutsTilesAnnotationProcessorTest`?

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on a diff in the pull request: https://github.com/apache/struts/pull/85#discussion_r50692670 — Diff: plugins/tiles/src/test/java/org/apache/struts2/tiles/TestStrutsTilesAnnotationProcessor.java — @@ -0,0 +1,148 @@ +package org.apache.struts2.tiles; + +import java.util.List; +import java.util.Set; + +import org.apache.struts2.tiles.annotation.TilesDefinition; +import org.apache.tiles.Attribute; +import org.apache.tiles.Definition; +import org.apache.tiles.Expression; +import org.junit.Test; + +import org.junit.Assert; + +public class TestStrutsTilesAnnotationProcessor { — End diff – Can we keep convention and instead of `TestStrutsTilesAnnotationProcessor` use `StrutsTilesAnnotationProcessorTest`?
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user cnenning opened a pull request:

          https://github.com/apache/struts/pull/85

          WW-4594: Configure TilesDefs by annotating Actions

          Adds annotations for each element from `tiles.xml` to annotate actions. Those annotations are processed by a new class in tiles-plugin which is used by TilesResult.

          With those annotations it is possible to keep `tiles.xml` very short (e.g. just put layout in there) and configure concrete tiles-definitions just by annotating actions.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/cnenning/struts master

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/struts/pull/85.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #85


          commit d9f4054b1367cd7ab6e3f22b9cc677f62def4e83
          Author: cnenning <cnenning@apache.org>
          Date: 2016-01-22T13:59:48Z

          fixed tiles showcase by setting dtd to 3.0

          commit 9ac326aa2458fe43140c1b13b61c87752d282e3d
          Author: cnenning <cnenning@apache.org>
          Date: 2016-01-22T14:27:09Z

          Added tiles annotations, see WW-4594.

          Added tiles annotations, created StrutsTilesAnnotationProcessor to
          create Definitons from them and using it in TilesResult.

          commit e50c37c5ba5781900edf53f9ec71b8649471d448
          Author: cnenning <cnenning@apache.org>
          Date: 2016-01-22T14:27:50Z

          added sample for tiles annotations

          commit d76357fd829a3ea8ddca21d625c49b606cca88d5
          Author: cnenning <cnenning@apache.org>
          Date: 2016-01-25T10:23:10Z

          added tests for StrutsTilesAnnotationProcessor

          commit a53deac7ce8732053edd43dddac329448055aef0
          Author: cnenning <cnenning@apache.org>
          Date: 2016-01-25T12:39:47Z

          updated javadoc

          commit b1588ddc84d876a676bab66ad80ec34637e98536
          Author: cnenning <cnenning@apache.org>
          Date: 2016-01-25T12:50:45Z

          fixed line endings


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user cnenning opened a pull request: https://github.com/apache/struts/pull/85 WW-4594 : Configure TilesDefs by annotating Actions Adds annotations for each element from `tiles.xml` to annotate actions. Those annotations are processed by a new class in tiles-plugin which is used by TilesResult. With those annotations it is possible to keep `tiles.xml` very short (e.g. just put layout in there) and configure concrete tiles-definitions just by annotating actions. You can merge this pull request into a Git repository by running: $ git pull https://github.com/cnenning/struts master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/struts/pull/85.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #85 commit d9f4054b1367cd7ab6e3f22b9cc677f62def4e83 Author: cnenning <cnenning@apache.org> Date: 2016-01-22T13:59:48Z fixed tiles showcase by setting dtd to 3.0 commit 9ac326aa2458fe43140c1b13b61c87752d282e3d Author: cnenning <cnenning@apache.org> Date: 2016-01-22T14:27:09Z Added tiles annotations, see WW-4594 . Added tiles annotations, created StrutsTilesAnnotationProcessor to create Definitons from them and using it in TilesResult. commit e50c37c5ba5781900edf53f9ec71b8649471d448 Author: cnenning <cnenning@apache.org> Date: 2016-01-22T14:27:50Z added sample for tiles annotations commit d76357fd829a3ea8ddca21d625c49b606cca88d5 Author: cnenning <cnenning@apache.org> Date: 2016-01-25T10:23:10Z added tests for StrutsTilesAnnotationProcessor commit a53deac7ce8732053edd43dddac329448055aef0 Author: cnenning <cnenning@apache.org> Date: 2016-01-25T12:39:47Z updated javadoc commit b1588ddc84d876a676bab66ad80ec34637e98536 Author: cnenning <cnenning@apache.org> Date: 2016-01-25T12:50:45Z fixed line endings

            People

            • Assignee:
              cn42 Christoph Nenning
              Reporter:
              cn42 Christoph Nenning
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development