Skip to content

[NETBEANS-4946] - Improve JAVA/JAKARTA EE with Ant based projects#2491

Merged
matthiasblaesing merged 1 commit intoapache:masterfrom
pepness:improve-jee
Mar 20, 2021
Merged

[NETBEANS-4946] - Improve JAVA/JAKARTA EE with Ant based projects#2491
matthiasblaesing merged 1 commit intoapache:masterfrom
pepness:improve-jee

Conversation

@pepness
Copy link
Member

@pepness pepness commented Oct 26, 2020

Commit 1. Improve JakartaEE8/JAVAEE8 support with Ant

  • Add support for app-client version 8 when using ant
  • Add methods that detect support for CDI 1.0 and CDI 2.0
  • Add missing resource and license file
  • Add missing deployment descriptor for JEE 8
  • Remove use of deprecated method "getServerID" in one class
  • Remove unnecessary validation that checks if running JDK 7
  • Remove use of deprecated method ".getDefault().getJ2eePlatform"
  • Use try-with-resources
  • Fix memory leak by closing the PrintWriter
  • Add missing validations for JEE7 and higher
  • Remove unnecessary use of Integer constructor
  • Fix wrong way of comparing Integers
  • Remove unnecessary assignment inside an if condition
  • Add missing validations for EAR applications
  • Wrong profile for JAKARTA_EE_8_FULL
  • Add missing version for JEE 8
  • Add missing validation for JEE8 Application
  • Add missing validation for EJB 3.2
  • Add validation for WebFragment 3.1
  • Add missing MIME_PREFIX for JEE 8
  • Add missing validations for JakartaEE8
  • Change logic to define isEE6Plus variable
  • New logic to evaluate when J2EE is used
  • New logic to evaluate the profile to use
  • Change mdd files to implement and extend local classes
  • Add 1 TODO (make work web-fragment version 4.0)

Commit 2. Update javac.source and compilerargs properties in some Java EE modules
Commit 3. Cleanup

Commit 4. Add logging for test purposes (I will remove added logging)
Commit 5. Add PrintWriter to the try-with-resource construct
Commit 6. Remove unnecessary parameter
Commit 7. Add curly braces for better readability
Commit 8. Fix wrong specification version
Commit 9. Add missing validations for JDK 11, 8 and 7 when setting the target source level
Commit 10. Remove redundant word in javadoc
Commit 11. Add curly braces and align code for better readability
Commit 12. Add logging for testing purposes (I will remove added logging)

Testing:

  • Full build done
  • Verify successful execution of unit tests for 14 modules [j2ee.clientproject-j2ee.kit] and web.beans
  • Verify successful execution of ant verify-libs-and-licenses
  • Tested with Glassfish 5.1 and TomEE 8
  • Create EAR, EJB, WEB and Application Client with JEE 5, 6, 6 w/CDI , 7 , 8 and JAKARTA EE 8
  • Verify the correct creation of deployment descriptors and CDI files for every profile in the projects mention above
  • Verify that every project can clean and build successfully
    NETBEANS-4946-1

Things to improve:

  • Could not make work web-fragment version 4.0, it was not working from the beggining. Left the code commented.

Doubts:

  • Don't know where to obtain or to create correct mdd files that will work correctly with NetBeans, I modify the mdd files that generate the models for JEE 8 Application and Application-client classes. I tried to do the same with web-fragment 4.0 but it failed to compile.

I left some logging for testing/review purposes, I will remove them and squash the commits at the end.

@pepness pepness changed the title [NETBEANS-4946] - Improve JAVA EE with Ant based projects [NETBEANS-4946] - Improve JAVA/JAKARTA EE with Ant based projects Oct 26, 2020
@lkishalmi lkishalmi added the Java EE/Jakarta EE [ci] enable enterprise job label Dec 20, 2020
@lkishalmi lkishalmi added this to the 12.3 milestone Dec 20, 2020
@lkishalmi
Copy link
Contributor

Is there anything against not to merge this one?

@matthiasblaesing
Copy link
Contributor

The testing reads good. Reviewing is difficult, because there are to many commits to review per commit and to many changes in general to review by file. I suggest to do the cleanup you suggested and squash the changes. If possible split cleanup (@OverRide, loggers) and actually function changes into separate commits. After that it should be easier to review.

@pepness
Copy link
Member Author

pepness commented Jan 9, 2021

@matthiasblaesing @lkishalmi I have separated the commits for better reviewing, and updated the description.

Copy link
Contributor

@matthiasblaesing matthiasblaesing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks mostly sane and like a good cleanup to me. I left a few inline comments. The biggest one affects the handling of source level, which looks suspicious.

if (sourceLevel.equals("1.7")) {
sourceLevel = "1.6";
}
// #181215: Not neccessary anymore because NetBeans should run on minimum JDK 8
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read this as the target source level. If NetBeans runs on 8, it gets even worse. The description implies, that a build project should be compatible with Java 1.6, when generated for a JavaEE 6 client profile, this is ensures by the original code, you move all projects to 8.

In the area where JavaEE is at home (large companies) I can imaginge setups where indeed Java 1.6 ist the latest installed version.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After a more detailed testing to these validations, the methods were these validations reside are called every time independently of the selected profile. The sourceLevel variable is equal to the Java version that NetBeans is running.

JavaPlatform defaultPlatform = JavaPlatformManager.getDefault().getDefaultPlatform();
SpecificationVersion v = defaultPlatform.getSpecification().getVersion();
String sourceLevel = v.toString();

In a new commit I added some missing validation that sets the correct target source level.

String srcLevel = sourceLevel;
if (sourceLevel.equals("1.7")) {
srcLevel = "1.6";
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment above - probably same situation.

@matthiasblaesing
Copy link
Contributor

Looks sane to me - I admit, that I did not check every change, but overall it looks like a good cleanup. A final cleanup is necessary though as indicated.

@neilcsmith-net
Copy link
Member

Thanks for comments @matthiasblaesing - guess no option but to bump forward to 12.4 now.

@neilcsmith-net neilcsmith-net modified the milestones: 12.3, 12.4 Jan 23, 2021
@matthiasblaesing
Copy link
Contributor

@pepness I suggest to rebase this one current master, do the final cleanup and get this in. We are early in the 12.4 and given the big size it would be good if this could land early.

@matthiasblaesing
Copy link
Contributor

@pepness thank you for the update. Would you mind removing the commits:

I assume, that the last three commits revert changes introduced in 7b7b525. Squash all four should result in an empty commit then. If you feel uncomfortable with that, please say so and I'll take a look.

-Add support for app-client version 8 when using ant
-Add methods that detect support for CDI 1.0 and CDI 2.0
-Add missing resource and license file
-Add missing deployment descriptor for JEE 8
-Add missing validations for JDK 11, 8, and 7
-Remove use of deprecated method "getServerID" in one class
-Remove unnecessary validation that checks if running JDK 7
-Remove use of deprecated method ".getDefault().getJ2eePlatform"
-Use try-with-resources
-Fix memory leak by closing the PrintWriter
-Add missing validations for JEE7 and higher
-Remove unnecessary use of Integer constructor
-Fix wrong way of comparing Integers
-Remove unnecessary assignment inside an if condition
-Add missing validations for EAR applications
-Wrong profile for JAKARTA_EE_8_FULL
-Add missing version for JEE 8
-Add missing validation for JEE8 Application
-Add missing validation for EJB 3.2
-Add validation for WebFragment 3.1
-Add missing MIME_PREFIX for JEE 8
-Add missing validations for JakartaEE8
-Change logic to define isEE6Plus variable
-New logic to evaluate when J2EE is used
-New logic to evaluate the profile to use
-Change mdd files to implement and extend local classes
-Add 1 TODO
-Update javac.source and compilerargs properties in some Java EE modules
-Cleanup:
-Add @OverRide tags and other small fixes
-Remove unnecessary parameter in a logging sentence
-Remove redundant word in javadoc
-Add curly braces and align code for better readability
@pepness
Copy link
Member Author

pepness commented Mar 19, 2021

@matthiasblaesing I did the rebase, remove all added logging, squash the commits and re-test everything, I did not see your previous comment.

@matthiasblaesing
Copy link
Contributor

Thank you for your work and patience - lets get this in.

@matthiasblaesing matthiasblaesing merged commit fd26cb2 into apache:master Mar 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Java EE/Jakarta EE [ci] enable enterprise job

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants