Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

cdi-decorator and cdi-interceptors quickstarts added #472

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
Contributor

ishulga commented Mar 13, 2013

I created a simple cdi-decorator quickstart. If I missed something please let me know.
Thanks,
Ievgen Shulga

Owner

LightGuard commented Mar 18, 2013

The coding looks fine to me, could probably use some more explanation comments (why we're doing something a particular way), but other than that, this seems to be good.

Contributor

ishulga commented Mar 20, 2013

Hi, I added more explanation to cdi-decorator quickstart and added new cdi-interceptors quickstart. Could you please approve fixes to cdi-decorator and review cdi-interceptors quickstart?

Thanks.

Contributor

ishulga commented Mar 25, 2013

Thanks for review, I added fixes to both quickstarts.

Owner

LightGuard commented Mar 25, 2013

I'm happy with this now. @sgilda I think we're good for a merge

Contributor

sgilda commented Mar 25, 2013

I still need to review this for usability and run qstools against it. I'm tied up with some doc deadlines at the moment, but will try to review this soon.

Contributor

sgilda commented Apr 1, 2013

I tested this and am not sure what I should be seeing. I tested as checked in and when I access the application, I see:

Staff position: Team Lead
Staff Bonus: 200

I edited the beans.xml file to uncommented the decorator and see the same thing when I run the application.

I think it might be nice to explain that the decorator, in this case, logs something to the server so people know what to look for. I'll make a few comments in the files to explain better. :-)

I also ran the qs tool checker against the files and it reported 145 tab characters. Can you replace the tab characters with spaces? You can find the coding rules and how to run the automated tool here: https://github.com/jboss-jdf/jboss-as-quickstart/blob/master/CONTRIBUTING.md

@sgilda sgilda commented on an outdated diff Apr 1, 2013

cdi-decorator/README.md
@@ -0,0 +1,90 @@
+cdi-decorator: Demostrates CDI Decorator
+======================================================
+Author: Ievgen Shulga
+Level: Intermediate
+Technologies: CDI
+Summary: Demonstrates the use of CDI Decorator where the bean is can be decorated.
+Target Product: EAP
+Source: <https://github.com/jboss-jdf/jboss-as-quickstart/>
+
+What is it?
+-----------
+This example demonstrates the use of CDI Decorator.
+It represents a common decorator design pattern. We take a class and we wrap decorator class around it.
+So when we call the class, we always pass through the surrounding decorator class before we reach the inner class.
@sgilda

sgilda Apr 1, 2013

Contributor

s/So when/When

I would change this to read:

When we call the class, we always pass through the surrounding decorator class before we reach the inner class. In this example, the decorator class simply logs a message to the server.

@sgilda sgilda commented on an outdated diff Apr 1, 2013

cdi-decorator/README.md
@@ -0,0 +1,90 @@
+cdi-decorator: Demostrates CDI Decorator
+======================================================
+Author: Ievgen Shulga
+Level: Intermediate
+Technologies: CDI
+Summary: Demonstrates the use of CDI Decorator where the bean is can be decorated.
+Target Product: EAP
+Source: <https://github.com/jboss-jdf/jboss-as-quickstart/>
+
+What is it?
+-----------
+This example demonstrates the use of CDI Decorator.
+It represents a common decorator design pattern. We take a class and we wrap decorator class around it.
+So when we call the class, we always pass through the surrounding decorator class before we reach the inner class.
+By default, all decorators are disabled. We need to enable our decorator in the beans.xml descriptor.
@sgilda

sgilda Apr 1, 2013

Contributor

By default, all decorators are disabled, so we need to enable our decorator in the beans.xml descriptor.

@sgilda sgilda commented on an outdated diff Apr 1, 2013

cdi-decorator/README.md
+2. Open a command line and navigate to the root directory of this quickstart.
+3. Type this command to build and deploy the archive:
+
+ mvn clean package jboss-as:deploy
+
+4. This will deploy `target/cdi-decorator.ear` to the running instance of the server.
+
+
+Access the application
+---------------------
+
+The application will be running at the following URL <http://localhost:8080/cdi-decorator>.
+
+You can specify decorator of the bean in the WEB-INF/beans.xml file by doing one of the following:
+
+1. you can add decorator class to the 'decorators' tag
@sgilda

sgilda Apr 1, 2013

Contributor
  1. You can add a decorators tag and specify a decorator class.
  2. You can specify a different decorator class name in the decorators tag.

@sgilda sgilda commented on an outdated diff Apr 1, 2013

cdi-decorator/README.md
+
+ mvn clean package jboss-as:deploy
+
+4. This will deploy `target/cdi-decorator.ear` to the running instance of the server.
+
+
+Access the application
+---------------------
+
+The application will be running at the following URL <http://localhost:8080/cdi-decorator>.
+
+You can specify decorator of the bean in the WEB-INF/beans.xml file by doing one of the following:
+
+1. you can add decorator class to the 'decorators' tag
+2. you can change the class name.
+
@sgilda

sgilda Apr 1, 2013

Contributor

For this example, uncomment the <decorators> tag in the WEB-INF/beans.xml file and redeploy the application. When you access the application, you will see the following in the server log:

    `CDI decorator method was called!`

@sgilda sgilda and 1 other commented on an outdated diff Apr 1, 2013

cdi-decorator/src/main/webapp/WEB-INF/beans.xml
+ and/or its affiliates, and individual contributors by the @authors tag. See
+ the copyright.txt in the distribution for a full listing of individual contributors.
+ Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ use this file except in compliance with the License. You may obtain a copy
+ of the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required
+ by applicable law or agreed to in writing, software distributed under the
+ License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS
+ OF ANY KIND, either express or implied. See the License for the specific
+ language governing permissions and limitations under the License. -->
+<!-- Marker file indicating CDI should be enabled -->
+<beans xmlns="http://java.sun.com/xml/ns/javaee" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="
+ http://java.sun.com/xml/ns/javaee
+ http://java.sun.com/xml/ns/javaee/beans_1_0.xsd">
+ <!-- To activate CDI decorator, it must be specified below-->
+ <!-- decorators>
@sgilda

sgilda Apr 1, 2013

Contributor

Rather than comment like this, could you comment something like the following:

See some of the quickstart pom.xml files for how we comment XML files.

@ishulga

ishulga Apr 4, 2013

Contributor

Meaning that we need to explain users how to comment XML files? I believe they should know that.

@sgilda

sgilda Apr 4, 2013

Contributor

My comments above didn't render.

`

`

No, I don't think we have to explain how to comment XML files. I just think it's easier and less error prone to remove beginning <!-- and ending --> comment markers that to remove them from within the tags.

I also think it's good to be consistent in the way we do things across all the files in all the quickstarts to make them easier for developers to follow and also make them look more polished.

Anyway, it was just a suggestion. I didn't many any offense.

@ishulga

ishulga Apr 4, 2013

Contributor

I see, of course, I will do tags as mentioned above.

Contributor

sgilda commented Apr 1, 2013

This is a very nice example! Very clean and easy to follow.

Once last comment: When you've done the updates, would you mind squashing the commits down into 1 commit.

Contributor

ishulga commented Apr 4, 2013

By default, with commented decorator tag, browser must show Java Developer, but only with uncommented decorator tag it should show Team Lead. I added it in README file. If it dont work for you, could you tell me the version of JBoss server you use so I can test it. Because a lot of CDI bugs were fixed in minor versions of AS.
Have you reviewed the cdi-interceptors quickstart? Should I include it in 1 commit?

@sgilda sgilda commented on an outdated diff Apr 4, 2013

cdi-decorator/pom.xml
+ Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ use this file except in compliance with the License. You may obtain a copy
+ of the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required
+ by applicable law or agreed to in writing, software distributed under the
+ License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS
+ OF ANY KIND, either express or implied. See the License for the specific
+ language governing permissions and limitations under the License. -->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+
+ <!-- The pom builds the web WAR artifact. -->
+
+ <modelVersion>4.0.0</modelVersion>
+
+ <groupId>org.jboss.as.quickstarts</groupId>
+ <artifactId>cdi-decorator</artifactId>
@sgilda

sgilda Apr 4, 2013

Contributor

If you look at the other quickstarts, you'll notice they prefix the artifactId with jboss-as-. The reason we do this is to make sure the deployment is unique and prevent a quickstart from accidentally wiping out a deployment with the same name. So in this case it should be:
<artifactId>jboss-as-cdi-decorator</artifactId>

It looks like we need to update our Contributing Guide because it's missing this information. Sorry about that.

@sgilda sgilda commented on an outdated diff Apr 4, 2013

cdi-decorator/README.md
+ For Linux: JBOSS_HOME/bin/standalone.sh
+ For Windows: JBOSS_HOME\bin\standalone.bat
+
+
+Build and Deploy the Quickstart
+-------------------------
+
+_NOTE: The following build command assumes you have configured your Maven user settings. If you have not, you must include Maven setting arguments on the command line. See [Build and Deploy the Quickstarts](../README.md#buildanddeploy) for complete instructions and additional options._
+
+1. Make sure you have started the JBoss Server as described above.
+2. Open a command line and navigate to the root directory of this quickstart.
+3. Type this command to build and deploy the archive:
+
+ mvn clean package jboss-as:deploy
+
+4. This will deploy `target/cdi-decorator.ear` to the running instance of the server.
@sgilda

sgilda Apr 4, 2013

Contributor

This actually deploys a WAR, not an EAR. With the change to the POM file:

This will deploy target/jboss-as-cdi-decorator.war to the running instance of the server.

@sgilda sgilda commented on an outdated diff Apr 4, 2013

cdi-decorator/README.md
+
+_NOTE: The following build command assumes you have configured your Maven user settings. If you have not, you must include Maven setting arguments on the command line. See [Build and Deploy the Quickstarts](../README.md#buildanddeploy) for complete instructions and additional options._
+
+1. Make sure you have started the JBoss Server as described above.
+2. Open a command line and navigate to the root directory of this quickstart.
+3. Type this command to build and deploy the archive:
+
+ mvn clean package jboss-as:deploy
+
+4. This will deploy `target/cdi-decorator.ear` to the running instance of the server.
+
+
+Access the application
+---------------------
+
+The application will be running at the following URL <http://localhost:8080/cdi-decorator>.
@sgilda

sgilda Apr 4, 2013

Contributor

With the change to the artifactId in the POM file:

The application will be running at the following URL http://localhost:8080/jboss-as-cdi-decorator.

Contributor

sgilda commented Apr 4, 2013

I tested the cdi-decorator and it works great!

I added a comment to the cdi-decorator/pom.xml file and a few to the README file.

I also ran the QS Tools plugin and it reported quite a few violations:

  • The license in a few files do not match the license template: pom.xml, src/main/webapp/WEB-INF/beans.xml, src/main/webapp/WEB-INF/faces-config.xml
  • The tag in the pom.xml file should use the variable: <finalName>${project.artifactId}</finalName>-
    Quite a few files contain tab characters. See the General Guidelines in the CONTRIBUTING.md file for formatting requirements.

Other than that, it looks great!

Contributor

sgilda commented Apr 4, 2013

Just tested the cdi-interceptors quickstart and it works great too!
It needs the same updates to the files that the other quickstart needs (formatting, etc.).

I will add a few suggestions to the README also. It's a very cool quickstart and it would be good to expand on how you can change the behavior for auditing and logging just by commenting out the particular class in the beans.xml file and where to look to see the differences.

@sgilda sgilda commented on an outdated diff Apr 4, 2013

cdi-interceptors/README.md
@@ -0,0 +1,90 @@
+cdi-interceptors: Example Using CDI-interceptors.
+=================================================================================
+Author: Ievgen Shulga
+Level: Intermediate
+Technologies: JPA,JSF,EJB
+Summary: Demonstrates using cdi-interceptors for logging and auditing
+Target Product: EAP
+
+What is it?
+-----------
+The quickstart demonstrates using CDI interceptors for cross-cutting concerns such as logging and simple auditing.
+Interceptors can be applied to any business methods or beans, simply by adding appropriate interceptor binding type annotation. The project contains EJB service that can create and retrieve object from database.
+It has 2 interceptors: AuditInterceptor and LoggingInterceptor. EJB methods marked with annotations, which specify that an annotation type is an interceptor binding type.
@sgilda

sgilda Apr 4, 2013

Contributor

I would start a new paragraph here to make it stand out more. We use tick marks around names so they're more obvious. I also had a little trouble understanding the rest of the paragraph, so I tried rewording and enhancing it. If what I changed it to is wrong, feel free to fix it or ignore it. :-)

This example demonstrates 2 interceptors: AuditInterceptor and LoggingInterceptor

The quickstart defines the @Audit and @Logging interceptor binding types. The AuditInterceptor and LoggingInterceptor classes are annotated with the binding type and contain a method annotated @AroundInvoke. If the interceptor is enabled, this method will be called when the intercepted methods are invoked. In the ItemServiceBean bean, notice the create()and getList() methods are annotated with the @Audit and @Logging binding types. This means the aroundInvoke() method in the AuditInterceptor and LoggingInterceptor classes will be called when the ItemServiceBean bean's create()and getList() methods are called, but only if that interceptor is enabled.

To enable an interceptor, you must add the interceptor class to the WEB-INF/beans.xml descriptor file.

@sgilda sgilda commented on an outdated diff Apr 4, 2013

cdi-interceptors/README.md
+_NOTE: The following build command assumes you have configured your Maven user settings. If you have not, you must include Maven setting arguments on the command line. See [Build and Deploy the Quickstarts](../README.md#buildanddeploy) for complete instructions and additional options._
+
+1. Make sure you have started the JBoss Server as described above.
+2. Open a command line and navigate to the root directory of this quickstart.
+3. Type this command to build and deploy the archive:
+
+ mvn clean package jboss-as:deploy
+
+4. This will deploy `target/cdi-interceptors.war` to the running instance of the server.
+
+
+Access the application
+---------------------
+
+The application will be running at the following URL <http://localhost:8080/cdi-interceptors>.
+
@sgilda

sgilda Apr 4, 2013

Contributor

In the server log file, you should see the following logs created by the LoggingInterceptor class:

    Executing ItemServiceBean.create method
    Executing ItemServiceBean.getList method

In the Web browser page, you should see the output provided by the AuditInterceptor class:

    Method invocation history
    Executing ItemServiceBean.create method at 3:11:22 PM 
    Executing ItemServiceBean.getList method at 3:11:22 PM

@sgilda sgilda commented on an outdated diff Apr 4, 2013

cdi-interceptors/README.md
+
+4. This will deploy `target/cdi-interceptors.war` to the running instance of the server.
+
+
+Access the application
+---------------------
+
+The application will be running at the following URL <http://localhost:8080/cdi-interceptors>.
+
+You can specify interceptor of the bean in the WEB-INF/beans.xml file by doing one of the following:
+
+1. you can add interceptor class to the 'interceptors' tag
+2. you can change the class name.
+
+In this quickstart, in order to switch back to the default implementation,
+comment the 'interceptors' block in the WEB-INF/beans.xml file and redeploy the quickstart.
@sgilda

sgilda Apr 4, 2013

Contributor

I would change this to:

You can now comment out classes in the WEB-INF/beans.xlm file to disable one or both of the interceptors and view the results.

  • Comment the <class>org.jboss.as.quickstarts.cdi.interceptor.AuditInterceptor</class> and you will no longer see the audit history on the browser page.
  • Comment the <class>org.jboss.as.quickstarts.cdi.interceptor.LoggerInterceptor</class> and you will no longer see the log messages in the server log.

@sgilda sgilda commented on an outdated diff Apr 4, 2013

cdi-interceptors/src/main/webapp/item.xhtml
+http://www.apache.org/licenses/LICENSE-2.0
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+<!DOCTYPE html>
+<html xmlns="http://www.w3.org/1999/xhtml" xmlns:f="http://java.sun.com/jsf/core" xmlns:h="http://java.sun.com/jsf/html">
+<h:head>
+ <title>CDI-interceptors</title>
+</h:head>
+<h:body>
+ <h:messages />
+ <h:form id="item_form">
+ <h:outputLabel value="Name:" for="nameInput" />
@sgilda

sgilda Apr 4, 2013

Contributor

I know it's not necessary, but I would like to see a some sort of instructions here. For example:

Enter a name and click Add to see CDI Interceptors in action.

@sgilda sgilda commented on an outdated diff Apr 4, 2013

cdi-interceptors/src/main/webapp/item.xhtml
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+<!DOCTYPE html>
+<html xmlns="http://www.w3.org/1999/xhtml" xmlns:f="http://java.sun.com/jsf/core" xmlns:h="http://java.sun.com/jsf/html">
+<h:head>
+ <title>CDI-interceptors</title>
+</h:head>
+<h:body>
+ <h:messages />
+ <h:form id="item_form">
+ <h:outputLabel value="Name:" for="nameInput" />
+ <h:inputText id="nameInput" value="#{itemBean.name}" />
+ <h:commandButton action="#{itemBean.add}" id="addName" value="add" />
@sgilda

sgilda Apr 4, 2013

Contributor

value="Add"

Contributor

ishulga commented Apr 5, 2013

Thanks for useful comments, I fixed I believe everything you mentioned above.

Contributor

sgilda commented Apr 5, 2013

Looks great! Thanks so much for your efforts and for being so patient with me.

One last request: Would you mind squashing the commits down to one? :-)

@pmuir: Would you like to review this one before I merge?

cdi-decorator and cdi-interceptors squashed commit
added cdi-decorator quickstart

added jboss license to sources

decorator class changed to abstract and added more explanation comments.

added cdi-interceptors quickstart skeleton

added module cdi-interceptors to parant pom.xml

added license headers and javadocs

cdi-decorator(removed unused import), cdi-interceptors(fixed README.md, refactored and added comment to interceptors)

Fixed code formatting and changed README.md for better understanding

Fixed header license, tabs, README.md in both quickstarts.

added comments to cdi-interceptors quickstart
Contributor

ishulga commented Apr 8, 2013

Of course :) I squashed the commits down to one.

Contributor

sgilda commented Apr 8, 2013

@pmuir: Please let me know if you would like to review before I merge.

Contributor

pmuir commented Apr 26, 2013

@ishulga These don't seem to be on your master anymore?

Contributor

sgilda commented Apr 26, 2013

@ishulga: I still have your source code for cdi-decorator and cdi-interceptorst if you need it. :-)

Contributor

ishulga commented Apr 27, 2013

@pmuir Sorry for that, I recommited these quickstarts to master. @sgilda Thanks :)

Contributor

sgilda commented May 2, 2013

@pmuir : The files are back and ready for your review. :-)

Contributor

sgilda commented May 8, 2013

These quickstarts were added to pull 494, so I am closing this one.

@sgilda sgilda closed this May 8, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment