Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade DWR from 1.x to 2.x #283

Merged
merged 1 commit into from
Dec 2, 2018

Conversation

JCgH4164838Gh792C124B5
Copy link
Contributor

Upgrade DWR from 1.x to 2.x

  • intended to resolve Jenkins build failure with old DWR version.

pom.xml changes:

  • entry change in struts2-parent pom.xml
    • upgrade 1.1.1 (uk.ltd.getahead) -> 2.0.11-RELEASE (org.directwebremoting)
  • entry change in struts2-dwr-plugin pom.xml
  • entry change in struts2-showcase pom.xml

- intended to resolve Jenkins build failure with old DWR version.

pom.xml changes:
- entry change in struts2-parent pom.xml
  - upgrade 1.1.1 (uk.ltd.getahead) -> 2.0.11-RELEASE (org.directwebremoting)
- entry change in struts2-dwr-plugin pom.xml
- entry change in struts2-showcase pom.xml
@coveralls
Copy link

Coverage Status

Coverage remained the same at 46.893% when pulling 0e41063 on JCgH4164838Gh792C124B5:localS2Branch into 7530042 on apache:master.

@yasserzamani
Copy link
Member

Unfortunately this plugin has zero test coverage and so we cannot verify if this upgrade breaks anything or not :( there are 8 years between those versions!

@lukaszlenart
Copy link
Member

We can use the Showcase app to manually test if it works or not.

@JCgH4164838Gh792C124B5
Copy link
Contributor Author

Hello @yasserzamani and @lukaszlenart .

There is a DWR upgrade notes page (http://directwebremoting.org/dwr/downloads/upgrading/dwr20.html ) which talks about upgrading from 1.x to 2.x. The 2.x appears to maintain general backward-compatibility with most of 1.x, but has changes in error handling and dropped some deprecated methods.

As you stated earlier, there is no automated way to confirm there are not any breaking changes (currently I can only confirm it compiles the plugin and that nothing obvious appeared to fail in the showcase in a quick navigation run-through).

It seems like upgrading to 2.x may be the only way to progress past the DWR plugin build issue ("Build failed in Jenkins: Struts-master-JDK7-dependency-check #121") reported by Jenkins to the Struts-Dev mailing list.

@yasserzamani
Copy link
Member

How do you test dwr functionality via showcase? I'm not able to find related ui in main page. I only found http://localhost:8080/tags/ui/showDynamicTreeAction.action which truly fails with HTTP Status 404 - /WEB-INF/tags/ui/treeExampleDynamic.jsp.

Anyway IMHO it seems undeniable to add a minimal main scenario covering unit test to this PR when we found some time.

@JCgH4164838Gh792C124B5
Copy link
Contributor Author

Hello @yasserzamani .

I know next-to-nothing about DWR, but from the overview page it mentions browser/web-container interaction with AJAX. Not sure how an automated test might work for it ...

As for the ShowDynamicTreeAction in the showcase there's apparently no way to "mouse-navigate" to it.
Looks like the treeExampleDynamic.jsp is missing too (both from the .war and the app source) - which explains the 404 return.

In terms of DWR, the /dwr/* servlet mapping should be in effect (based on configuration).
After re-checking, it seems the DWR example is interfered-with by one of the many /* filter-mappings defined in the showcase web.xml.

By commenting out all of the /* filter-mappings in the showcase web.xml I was able to
navigate to: http://localhost:8080/struts2-showcase/dwr/index.html. That displays a page with
a validator link which when clicked launches a test page of sorts.

Don't know how the DWR validator page is supposed to work, but it seems to look/behave similarly
for both the 2.6-snapshot and the 2.5.19-snapshot
...

What do you think ?

@lukaszlenart
Copy link
Member

It looks good to me, just a reference to a JIRA ticket is missing :) Thank you!

@JCgH4164838Gh792C124B5
Copy link
Contributor Author

A JIRA ticket (https://issues.apache.org/jira/browse/WW-4988) has been created which references back to this PR, as requested. :)

@yasserzamani
Copy link
Member

Thank you! yesterday I tried to add an unit-test but was so complicated that I thought. It was needing mocking servletConfig, servletContext, request, response and etc for DWRServlet and in same time, to be able to execute a validation aware action outside of the core tests. So let's keep showcase manual test enough and merge :)

@lukaszlenart
Copy link
Member

LGTM 👍

@lukaszlenart lukaszlenart merged commit c489253 into apache:master Dec 2, 2018
@JCgH4164838Gh792C124B5 JCgH4164838Gh792C124B5 deleted the localS2Branch branch December 7, 2018 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants