refactor: java 8 syntax update #179
Conversation
Signed-off-by: Hantsy Bai <[email protected]>
|
I looked at the merged changes thus far. I think I need to do some minor refactoring and also do end-to-end regression testing. Looks like I need to update the documentation as well. After that, I hope to get some time to review this PR. |
|
@hantsy, when I tried to test the already merged code, I found that the batch process is broken: 2021-02-07T20:54:00.037-0500|WARNING: Caught throwable in main execution loop with Throwable message: null, and stack trace: java.lang.NullPointerException Are you encountering this? If not, what version of Payara are you running? |
|
@hantsy, it looks like in the already merged code, there are several profiles intended for Arquillian. Thus far, I have not gotten any of these profiles to work. They all fail for various reasons. Is this expected? If it is not expected, are there missing instructions to get them running? Wasn't this previously working code at least for Payara 4 remote? |
|
I have configured it to use Payara 5.2020.7 in the pom.xml file.
In the merged codes, the testing codes are not changed.
`mvn clean verify -Parq-payara-managed`
The tests are improved in a series of commits in my forked version.
But only the payara-managed(I think payara-remote is also ok) in CI got
passed. But there are some wired issues when running on Payara micro and
embedded.
*Hantsy Bai*
Self-employed consultant, fullstack developer, agile coach
GitHub: https://github.com/hantsy
Twitter: https://twitter.com/@hantsy
Medium: https://medium.com/@hantsy
…On Mon, Feb 8, 2021 at 10:30 AM Reza Rahman ***@***.***> wrote:
@hantsy <https://github.com/hantsy>, it looks like in the already merged
code, there are several profiles intended for Arquillian. Thus far, I have
not gotten any of these profiles to work. They all fail for various
reasons. Is this expected? If it is not expected, are there missing
instructions to get them running? Wasn't this previously working code at
least for Payara 4 remote?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#179 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGQT6FLIHMKE2U5ARWCLW3S55EDBANCNFSM4W27YINQ>
.
|
@m-reza-rahman Every commit I have tried in my local system, it is running successfully. or deploy to a running Payara server. |
|
I had a free moment in the morning so got the tests working again and also simplified the profiles back to the original intent - which was to have a profile for each supported application server, primarily for Arquillian (I don't think there is a need to have Cargo startup for each application server, it is just a way to simplify the default getting started experience). I have to say I don't quite understand the compelling use case for the other profiles, other than to simply demonstrate they are possible. Can you help me understand the desired use cases so we can properly re-add and document them? I also still need help understanding why the batch process is failing. When I get time, I will test with other operating systems, JVM versions and Payara versions. Right now I am trying with Windows 10, Java SE 8 and Payara 5.2020.7. The file batch import was working fine with Java SE 7/Payara 4. |
|
@m-reza-rahman Ran it(the upstream/master branch) again today, Windows 10, Java 8. D:\hantsylabs\cargotracker>set JAVA_HOME=D:\jdk8
D:\hantsylabs\cargotracker>mvn clean package cargo:run
[INFO] Scanning for projects...
[INFO]
[INFO] -------------------< org.eclipse.ee4j:cargo-tracker >-------------------
[INFO] Building Eclipse Cargo Tracker 1.0-SNAPSHOT
[INFO] --------------------------------[ war ]---------------------------------
[INFO]
[INFO] --- maven-clean-plugin:2.5:clean (default-clean) @ cargo-tracker ---
[INFO] Deleting D:\hantsylabs\cargotracker\target
[INFO]
[INFO] --- maven-resources-plugin:2.6:resources (default-resources) @ cargo-tracker ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] Copying 3 resources
[INFO]
[INFO] --- maven-compiler-plugin:3.8.1:compile (default-compile) @ cargo-tracker ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 103 source files to D:\hantsylabs\cargotracker\target\classes
[INFO]
[INFO] --- maven-resources-plugin:2.6:testResources (default-testResources) @ cargo-tracker ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] Copying 3 resources
[INFO]
[INFO] --- maven-compiler-plugin:3.8.1:testCompile (default-testCompile) @ cargo-tracker ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 11 source files to D:\hantsylabs\cargotracker\target\test-classes
[INFO]
[INFO] --- maven-surefire-plugin:3.0.0-M5:test (default-test) @ cargo-tracker ---
[INFO] Tests are skipped.
[INFO]
[INFO] --- maven-war-plugin:3.3.1:war (default-war) @ cargo-tracker ---
[INFO] Packaging webapp
[INFO] Assembling webapp [cargo-tracker] in [D:\hantsylabs\cargotracker\target\cargo-tracker]
[INFO] Processing war project
[INFO] Copying webapp resources [D:\hantsylabs\cargotracker\src\main\webapp]
[INFO] Building war: D:\hantsylabs\cargotracker\target\cargo-tracker.war
[INFO]
[INFO] --- cargo-maven2-plugin:1.8.4:run (default-cli) @ cargo-tracker ---
[INFO] [en2.ContainerRunMojo] Resolved container artifact org.codehaus.cargo:cargo-core-container-payara:jar:1.8.4 for container payara
[INFO] [talledLocalContainer] Parsed GlassFish version = [5.2020.7]
[INFO] [talledLocalContainer] Payara 5.2020.7 starting...
[INFO] [talledLocalContainer] Using port 4848 for Admin.
[INFO] [talledLocalContainer] Using port 8080 for HTTP Instance.
[INFO] [talledLocalContainer] Using port 7676 for JMS.
[INFO] [talledLocalContainer] Using port 3700 for IIOP.
[INFO] [talledLocalContainer] Using port 8181 for HTTP_SSL.
[INFO] [talledLocalContainer] Using port 3820 for IIOP_SSL.
[INFO] [talledLocalContainer] Using port 3920 for IIOP_MUTUALAUTH.
[INFO] [talledLocalContainer] Using port 8686 for JMX_ADMIN.
[INFO] [talledLocalContainer] Using port 6666 for OSGI_SHELL.
[INFO] [talledLocalContainer] Using port 9009 for JAVA_DEBUGGER.
[INFO] [talledLocalContainer] Using default port 4900 for Hazelcast DAS.
[INFO] [talledLocalContainer] Using default port 5900 for Hazelcast Start.
[INFO] [talledLocalContainer] Distinguished Name of the self-signed X.509 Server Certificate is:
[INFO] [talledLocalContainer] [CN=hantsy-t540p.mshome.net,OU=Payara,O=Payara Foundation,L=Great Malvern,ST=Worcestershire,C=UK]
[INFO] [talledLocalContainer] Distinguished Name of the self-signed X.509 Server Certificate is:
[INFO] [talledLocalContainer] [CN=hantsy-t540p.mshome.net-instance,OU=Payara,O=Payara Foundation,L=Great Malvern,ST=Worcestershire,C=UK]
[INFO] [talledLocalContainer] Domain cargo-domain created.
[INFO] [talledLocalContainer] Domain cargo-domain admin port is 4848.
[INFO] [talledLocalContainer] Domain cargo-domain allows admin login as user "admin" with no password.
[INFO] [talledLocalContainer] Command create-domain executed successfully.
[INFO] [talledLocalContainer] Attempting to start cargo-domain.... Please look at the server log for more details.....
[INFO] [talledLocalContainer] JDBC resource jdbc/__default deleted successfully
[INFO] [talledLocalContainer] Command delete-jdbc-resource executed successfully.
[INFO] [talledLocalContainer] Command delete-jdbc-connection-pool failed.
[INFO] [talledLocalContainer] remote failure: A JDBC connection pool named DerbyPool does not exist.
[INFO] [talledLocalContainer] Command delete-jdbc-connection-pool failed.
[INFO] [talledLocalContainer] remote failure: A JDBC connection pool named DerbyPool does not exist.
[INFO] [talledLocalContainer] Application deployed with name cargo-tracker.
[INFO] [talledLocalContainer] Command deploy executed successfully.
[INFO] [talledLocalContainer] Application deployed with name cargocpc.
[INFO] [talledLocalContainer] Command deploy executed successfully.
[INFO] [talledLocalContainer] Payara 5.2020.7 started on port [8080]
[INFO] Press Ctrl-C to stop the container...And the server log. |
|
I was able to reliably reproduce the issue and have taken it up with the Payara folks: https://groups.google.com/g/payara-forum/c/GT3e_hsPJM8/m/ilZk0k5hDwAJ. We can move forward with the PR once we either resolve this issue and/or register it as a known issue with Payara 5 for now. I will keep you posted. I don't think this is an issue on the Cargo Tracker side per se. P.S.: The issue is actually visible in the logs you shared @hantsy. The "warning" at the end is actually stopping the batch process from working. Instructions to test the batch file processing is described at the end of this section: https://github.com/eclipse-ee4j/cargotracker#exploring-the-application. As a key contributor, I encourage you to explore it. |
|
By default I used Java 11 in my system, I used I have not noticed this error before, it seems it is never occurred in the front-end console. |
|
While I will test with Java SE 11 as well, we do need to support Java SE 8 as this is where a majority of users still are. I will also check if prior Java EE 8/Jakarta EE 8 releases of Payara fixes the issue in parallel to seeing what the Payara folks have to say. There is a small possibility there is something wrong with the application code, so that is also worth investigating. Lastly, these kinds of issues may be the impetus to switch to WildFly or Open Liberty as the default runtime for Cargo Tracker. It is too early yet to consider a switch. I am sure those runtimes will have their own set of challenges. |
Signed-off-by: Hantsy Bai <[email protected]>
| @NotNull(message = "Missing completion time.") | ||
| @Size(min = 16, max = 16, message = "Completion time value must be sixteen characters long.") | ||
| // TODO [DDD] Apply regular expression validation. | ||
| @NotNull |
There was a problem hiding this comment.
This is a sync mismatch. Can you kindly restore the more updated code?
| // TODO [Jakarta EE 8] Use not blank instead. | ||
| @NotNull(message = "Missing tracking ID.") | ||
| @Size(min = 4, message = "Tracking ID must be at least four characters.") | ||
| @NotNull |
There was a problem hiding this comment.
This is a sync mismatch. Can you kindly restore the more updated code?
| max = 7, | ||
| message = "Event type value must be one of: RECEIVE, LOAD, UNLOAD, CUSTOMS, CLAIM") | ||
| // TODO [DDD] Apply regular expression validation. | ||
| @NotNull |
There was a problem hiding this comment.
This is a sync mismatch. Can you kindly restore the more updated code?
| private String eventType; | ||
|
|
||
| // TODO [Jakarta EE 8] Use not blank instead. | ||
| @NotNull(message = "UN location code missing.") | ||
| @Size(min = 5, max = 5, message = "UN location code must be five characters long.") |
There was a problem hiding this comment.
This is a sync mismatch. Can you kindly restore the more updated code?
| min = 4, | ||
| max = 5, | ||
| message = "Voyage number value must be between four and five characters long.") | ||
| @Size(min = 4, max = 5) |
There was a problem hiding this comment.
This is a sync mismatch. Can you kindly restore the more updated code?
| Date completionTime = | ||
| new SimpleDateFormat(ISO_8601_FORMAT).parse(handlingReport.getCompletionTime()); | ||
| VoyageNumber voyageNumber = null; | ||
| @Consumes(MediaType.APPLICATION_JSON) |
There was a problem hiding this comment.
This is a sync mismatch. Can you kindly restore the more updated code?
| @QueryParam("deadline") | ||
| String deadline) { | ||
| Date date = nextDate(new Date()); | ||
| @NotNull @Size(min = 5, max = 5) @QueryParam("origin") String originUnLocode, |
There was a problem hiding this comment.
This is a sync mismatch. Can you kindly restore the more updated code?
| File[] extraJars = | ||
| Maven.resolver() | ||
| .loadPomFromFile("pom.xml") | ||
| .resolve("org.apache.commons:commons-lang3", "org.postgresql:postgresql") |
There was a problem hiding this comment.
As discussed, kindly remove PostGRESQL for now.
|
I have tested versions up to Payara 5.194 to be working properly. The Payara folks are looking at resolving the issue in the next releases: https://groups.google.com/g/payara-forum/c/GT3e_hsPJM8/m/e0Hg8ASyEAAJ. In the meanwhile, I have adjusted the code and updated the documentation. I think we are ready to move forward with this PR now. @hantsy, can you kindly address the PR comments, re-sync your code and fix the auto-merge conflicts? I will then merge it as soon as I have adequate time to properly regression test (there are quite a few changes that can possibly stop things from working across the application in subtle ways). In the meanwhile, I am concerned about the stability of Payara. I have prioritized supporting WildFly or Open Liberty (#180). We can work on this after the initial Jakarta EE 8/Java SE 8 migration is done. Reza Rahman Please note views expressed here are my own as an individual community member and do not reflect the views of my employer. |
|
@m-reza-rahman I've updated the codes according to you feedback, please review it. |
|
Thanks for making the changes. I should have some time this evening or tomorrow. The merge is easy, but I also need time to regression test the code. |
|
@hantsy, have these changes actually been properly tested with Java SE 8? It seems the date time changes do not work with Java SE 8 at all. Below are some of the exceptions I am getting. Can you kindly help test and properly address these? At least for now, we must continue to support Java SE 8. org.jboss.weld.exceptions.WeldException: WELD-000049: Unable to invoke public void org.eclipse.cargotracker.interfaces.booking.web.ListCargo.init() on org.eclipse.cargotracker.interfaces.booking.web.ListCargo@a79e3fe |
Signed-off-by: Hantsy Bai [email protected]