[GitHub] struts pull request: WW-4594: Configure TilesDefs by annotating Ac...

[cnenning]

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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org
For additional commands, e-mail: dev-h...@struts.apache.org

[GitHub] struts pull request: WW-4594: Configure TilesDefs by annotating Ac...

[lukaszlenart]

Github user lukaszlenart commented on the pull request:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org
For additional commands, e-mail: dev-h...@struts.apache.org

[GitHub] struts pull request: WW-4594: Configure TilesDefs by annotating Ac...

[asfgit]

Github user asfgit closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org
For additional commands, e-mail: dev-h...@struts.apache.org

[GitHub] struts pull request: WW-4594: Configure TilesDefs by annotating Ac...

[lukaszlenart]

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!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org
For additional commands, e-mail: dev-h...@struts.apache.org

[GitHub] struts pull request: WW-4594: Configure TilesDefs by annotating Ac...

[lukaszlenart]

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?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org
For additional commands, e-mail: dev-h...@struts.apache.org

[GitHub] struts pull request: WW-4594: Configure TilesDefs by annotating Ac...

[lukaszlenart]

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 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", 

[GitHub] struts pull request: WW-4594: Configure TilesDefs by annotating Ac...

[lukaszlenart]

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 :D


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org
For additional commands, e-mail: dev-h...@struts.apache.org

[GitHub] struts pull request: WW-4594: Configure TilesDefs by annotating Ac...

[lukaszlenart]

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`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org
For additional commands, e-mail: dev-h...@struts.apache.org

[GitHub] struts pull request: WW-4594: Configure TilesDefs by annotating Ac...

[lukaszlenart]

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`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org
For additional commands, e-mail: dev-h...@struts.apache.org

[GitHub] struts pull request: WW-4594: Configure TilesDefs by annotating Ac...

[cnenning]

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:


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org
For additional commands, e-mail: dev-h...@struts.apache.org

[GitHub] struts pull request: WW-4594: Configure TilesDefs by annotating Ac...

[cnenning]

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 
Date:   2016-01-22T13:59:48Z

fixed tiles showcase by setting dtd to 3.0

commit 9ac326aa2458fe43140c1b13b61c87752d282e3d
Author: cnenning 
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 
Date:   2016-01-22T14:27:50Z

added sample for tiles annotations

commit d76357fd829a3ea8ddca21d625c49b606cca88d5
Author: cnenning 
Date:   2016-01-25T10:23:10Z

added tests for StrutsTilesAnnotationProcessor

commit a53deac7ce8732053edd43dddac329448055aef0
Author: cnenning 
Date:   2016-01-25T12:39:47Z

updated javadoc

commit b1588ddc84d876a676bab66ad80ec34637e98536
Author: cnenning 
Date:   2016-01-25T12:50:45Z

fixed line endings




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org
For additional commands, e-mail: dev-h...@struts.apache.org