WW-4864 Upgrade to the latest Jetty plugin in all examples #16

sdutry · 2017-10-11T21:55:31Z

This is still a Work In Progress

TODO:

Update to the 9.4.8 version when it's released
Check what needs to be done to upgrade portlet application

lukaszlenart

Left few comments but this is good 💪

lukaszlenart · 2017-10-12T04:38:47Z

file-upload/pom.xml

-                    <webAppSourceDirectory>${basedir}/src/main/webapp/</webAppSourceDirectory>
-                    <webAppConfig>
-                        <descriptor>${basedir}/src/main/webapp/WEB-INF/web.xml</descriptor>
-                    </webAppConfig>


I think it would be good to add instead

<webApp> <contextPath>/${project.artifactId}</contextPath> </webApp>

lukaszlenart · 2017-10-12T04:40:02Z

json/pom.xml

-          <webAppSourceDirectory>${basedir}/src/main/webapp/</webAppSourceDirectory>
-          <webAppConfig>
-            <descriptor>${basedir}/src/main/webapp/WEB-INF/web.xml</descriptor>
-          </webAppConfig>


Same here, what do you think about adding:

<webApp> <contextPath>/${project.artifactId}</contextPath> </webApp>

lukaszlenart · 2017-10-12T04:40:25Z

json/pom.xml

            <systemProperty>
              <name>xwork.loggerFactory</name>
              <value>com.opensymphony.xwork2.util.logging.log4j2.Log4j2LoggerFactory</value>
            </systemProperty>


This property can be removed as well

lukaszlenart · 2017-10-12T04:40:52Z

message-store/pom.xml

-          <webAppSourceDirectory>${basedir}/src/main/webapp/</webAppSourceDirectory>
-          <webAppConfig>
-            <descriptor>${basedir}/src/main/webapp/WEB-INF/web.xml</descriptor>
-          </webAppConfig>


Same here as above

lukaszlenart · 2017-10-12T04:41:01Z

pom.xml

        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
        <struts2.version>2.5.12</struts2.version>
        <log4j2.version>2.8.2</log4j2.version>
+        <jetty-plugin.version>9.4.7.v20170914</jetty-plugin.version>


lukaszlenart · 2017-10-12T04:41:41Z

unknown-handler/src/main/webapp/WEB-INF/web.xml

-    <filter-class>
-      org.apache.struts2.dispatcher.ng.filter.StrutsPrepareAndExecuteFilter
-    </filter-class>
+    <filter-class>org.apache.struts2.dispatcher.filter.StrutsPrepareAndExecuteFilter</filter-class>


lukaszlenart · 2017-10-12T04:41:51Z

wildcard-regex/src/main/webapp/WEB-INF/web.xml

-    <filter-class>
-      org.apache.struts2.dispatcher.ng.filter.StrutsPrepareAndExecuteFilter
-    </filter-class>
+    <filter-class>org.apache.struts2.dispatcher.filter.StrutsPrepareAndExecuteFilter</filter-class>


lukaszlenart · 2017-10-12T04:42:02Z

crud/src/main/webapp/WEB-INF/jsp/editPerson.jsp

 <%@ taglib prefix="s" uri="/struts-tags" %>
 <s:if test="person==null || person.personId == null">
-    <s:set name="title" value="%{'Add new person'}"/>
+    <s:set var="title" value="%{'Add new person'}"/>


lukaszlenart · 2017-10-12T04:42:10Z

crud/src/main/webapp/WEB-INF/jsp/persons.jsp

        <div class="titleDiv"><s:text name="application.title"/></div>
        <h1><s:text name="label.persons"/></h1>
-        <s:url id="url" action="inputPerson" />
+        <s:url var="url" action="inputPerson" />


sdutry · 2017-10-12T19:46:07Z

adjusted according to the remarks

lukaszlenart · 2017-10-13T06:08:19Z

Looks good, do you want to wait to fulfill the rest of TODOs?

sdutry · 2017-10-13T18:52:00Z

do you want to wait to fulfill the rest of TODOs?

There's 2 or so example applications that didn't get their jetty version upgraded yet because i couldn't run them. I'd like to fix those before merging

sdutry · 2017-10-13T19:11:25Z

Also, i've noted that some of the class files have old styled copyright/license info. (correct me if i'm wrong)

/*
 * $Id$
 *
 * Licensed to the Apache Software Foundation (ASF) under one
 * or more contributor license agreements.  See the NOTICE file
 * distributed with this work for additional information
 * regarding copyright ownership.  The ASF licenses this file
 * to you 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.
 */

Should we make a seperate issue for changing them?
If so, what's the wanted copyright/license info currently?

lukaszlenart · 2017-10-14T07:10:38Z

The above is a proper header
https://www.apache.org/legal/src-headers.html#headers

but please open another issue where we can fix that. Do you have an example of a bad license header?

lukaszlenart · 2017-10-14T07:11:56Z

There's 2 or so example applications that didn't get their jetty version upgraded yet because i couldn't run them. I'd like to fix those before merging

Probably one of them is a portlet based application ... I have failed so many time to fix it :(

sdutry · 2017-10-15T12:45:54Z

Do you have an example of a bad license header?

i just meant that the following line is in there:

 * $Id$

Probably one of them is a portlet based application

Yes it is. Unfortunately i don't have any experience with portlets and i'm not finding enough information to understand it.

The other one is the shiro-basic one which is just missing a jetty-plugin declaration.

lukaszlenart · 2017-10-15T14:24:05Z

The $Id$ is from Subversion days, it's not a problem but if possible I clear those :)

I to do not have experience with portlets ... maybe we should ask our community?

sdutry · 2017-10-15T16:53:47Z

from my side, i'm fine with the state of the pull request to merge it.

Maybe we should create a seperate issue to get the portlet application updated, that way we don't loose track of it.

Afterwards it should be easy to update the jetty-plugin once the 9.4.8 version gets released.

sdutry · 2017-10-16T19:03:15Z

@lukaszlenart
old $id$ comments are also removed

lukaszlenart · 2017-10-17T10:14:37Z

Great work! The final 👀

lukaszlenart · 2017-10-17T10:19:07Z

💯

WW-4864 upgrade jetty plugin to version 9.4.7.20170914

9e58624

sdutry requested a review from lukaszlenart October 11, 2017 21:55

lukaszlenart self-assigned this Oct 12, 2017

lukaszlenart reviewed Oct 12, 2017

View reviewed changes

added contextPath

17d168b

sdutry added 3 commits October 15, 2017 14:51

added jetty plugin to shiro-basic example

bb345cf

added contextPath

ceb93d3

updated struts DTD in struts.xml files

7b53379

removed old subversion $id$ comments

bdb9211

lukaszlenart merged commit 45af147 into apache:master Oct 17, 2017

sdutry deleted the WW-4864 branch October 17, 2017 17:55

WW-4864 Upgrade to the latest Jetty plugin in all examples #16

WW-4864 Upgrade to the latest Jetty plugin in all examples #16

Uh oh!

Conversation

sdutry commented Oct 11, 2017

Uh oh!

lukaszlenart left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sdutry commented Oct 12, 2017

Uh oh!

lukaszlenart commented Oct 13, 2017

Uh oh!

sdutry commented Oct 13, 2017

Uh oh!

sdutry commented Oct 13, 2017

Uh oh!

lukaszlenart commented Oct 14, 2017

Uh oh!

lukaszlenart commented Oct 14, 2017

Uh oh!

sdutry commented Oct 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lukaszlenart commented Oct 15, 2017

Uh oh!

sdutry commented Oct 15, 2017

Uh oh!

sdutry commented Oct 16, 2017

Uh oh!

lukaszlenart commented Oct 17, 2017

Uh oh!

lukaszlenart commented Oct 17, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sdutry commented Oct 15, 2017 •

edited

Loading