search

apache / netbeans

Apache NetBeans
https://netbeans.apache.org/
Apache License 2.0
2.64k stars 849 forks source link

Support for transformation from jakartaee8 to jakartaee10 applications #5847

Closed breakponchito closed 1 year ago

breakponchito commented 1 year ago

This PR adds support to transform applications from Jakarta EE 8 to Jakarta 10. When you have an older application and you want to transform to the Jakarta EE 10, then you can use the new option from the new project wizard to make the transformation. This is useful for developers that want to migrate older applications to the new Jakarta 10.

Case 1: file to a different folder

https://github.com/apache/netbeans/assets/279375/13856fa1-ba24-4c88-819d-b7b41278b481

Case 2: folder to a different folder

https://github.com/apache/netbeans/assets/279375/76233730-f9ad-4d1f-a4f6-f526b29d0f31

Case 3: file on the same location (override)

https://user-images.githubusercontent.com/279375/232970341-caf1855a-8ac6-4cb6-a9ec-7fdd8bdb5c18.mp4

Case 4: folder on the same location (override)

https://user-images.githubusercontent.com/279375/232970553-cc42b495-a888-4676-bcf5-6e3b66491a9a.mp4

^Add meaningful description above

By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -

Please make sure (eg. git log) that all commits have a valid name and email address for you in the Author field.

If you're a first time contributor, see the Contributing guidelines for more information.

If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.

breakponchito commented 1 year ago

@mbien Can you help to start again the validations?

breakponchito commented 1 year ago

@mbien please send another test for the validations

mbien commented 1 year ago

@breakponchito you can run commit validation locally, build it first then simply run ant commit-validation

going to start it later since we have 7 test runs in parallel running atm and I don't want to DoS the shared apache infrastructure.

breakponchito commented 1 year ago

@mbien Do you know what is causing the following error on the pipeline? [junit] Testcase: testInvisibleModules(org.netbeans.core.validation.ValidateModulesTest): FAILED [junit] Some regular modules (that no one depends on) neither AutoUpdate-Show-In-Client=true nor AutoUpdate-Essential-Module=true (thus unreachable through Plugin Manager) [junit] org.netbeans.modules.jakarta.transformer Log: [junit] Starting test shuttingDown[org.netbeans.core.validation.ValidateModulesTest] [junit] [org.netbeans.core.NbLifecycleManager] THREAD: Test Watch Dog: shuttingDown[org.netbeans.core.validation.ValidateModulesTest] MSG: Initiating exit with status 0 [junit] [org.netbeans.core.NbLifecycleManager] THREAD: Test Watch Dog: shuttingDown[org.netbeans.core.validation.ValidateModulesTest] MSG: blockForExit, new org.netbeans.core.NbLifecycleManager$1@0[Count = 1] [junit] [org.netbeans.core.NbLifecycleManager] THREAD: Test Watch Dog: shuttingDown[org.netbeans.core.validation.ValidateModulesTest] MSG: NbLifeExit(0, 0, null, org.netbeans.core.NbLifecycleManager$1@0[Count = 1]) = org.netbeans.core.NbLifeExit@1

I tried to add both properties but this said that both are not reachable for the Plugin Manager, and this is happening for the following JDK's: 11, 17 and 21. Please let me know if you have any suggestion

neilcsmith-net commented 1 year ago

@breakponchito it's complaining that neither of those properties are true. Probably need to make this autoload, and then make it a dependency or recommendation of j2ee.kit. If this requires jakartaee10 modules to be enabled, then probably should follow similar to https://github.com/neilcsmith-net/netbeans/commit/5d3aa156d7807aeec74821ec0911c6bebdafc27d

Now we don't need to special case these for JDK 8, should consider a different way to enable these too, but I guess should be done together.

breakponchito commented 1 year ago

@mbien @neilcsmith-net Do you know how to debug unit tests from local environment? or Do you have some references to see how to debug the commit-validations tests?

breakponchito commented 1 year ago

@mbien @neilcsmith-net to fix my local issue for the commit-validation it was needed to edit the following file:

breakponchito commented 1 year ago

@mbien I merged last changes from master branch, could you help me to send the pipeline validations?

breakponchito commented 1 year ago

I added some commits, could you help me to send the pipeline validations?

breakponchito commented 1 year ago

Hi, @breakponchito , thank you for this contribution. I'm looking forward to start using it when it's released!

@OndroMih @jGauravGupta please let me know if any other comment about this PR. I'm waiting for approvals

jGauravGupta commented 1 year ago

Hi @breakponchito,

I have reviewed the code and it looks good to me. There are two improvements that I suggest:

breakponchito commented 1 year ago

Hi @breakponchito,

I have reviewed the code and it looks good to me. There are two improvements that I suggest:

  • Bind an event to the source and target text fields: Currently, there is a warning message displayed when a path is added directly in the text field. To improve this, you can bind an event to the text fields so that the warning message is automatically removed when a path is added directly into the text fields instead of using the browse button.
  • Remove the package goal from the execution command: It seems that the execution command is invoking the maven-war-plugin unnecessarily. To optimize the command, you can remove the package goal, as it may not be required for your specific use case. This will help streamline the execution process and remove any unnecessary steps.

already done, please review

breakponchito commented 1 year ago

@mbien I added a new commit, Could you help me to send the pipeline validations?

mbien commented 1 year ago

@breakponchito I am going to add a commit - I hope this is ok. I wanted to add review comments but it is faster to code it right away since some of it is UI.

breakponchito commented 1 year ago

@breakponchito I am going to add a commit - I hope this is ok. I wanted to add review comments but it is faster to code it right away since some of it is UI.

  • made the wizard panel a bit more compact and layout gaps more consistent
  • removed a file chooser component which was stored in the form but not used
  • the text fields should validate on input change rather than action performed (which is pressing return). Otherwise the user can't simply type /tmp and press finish. For that we can use a DocumentListener
  • code cleanup: overrides, generics, arm blocks, compiler warnings etc
  • fixed a NPE which happened while closing the wizard.
java.lang.NullPointerException: Cannot invoke "org.openide.WizardDescriptor.putProperty(String, Object)" because "this.wizardDescriptor" is null
  at org.netbeans.modules.fish.payara.jakarta.transformer.TransformerWizardIterator.uninitialize(TransformerWizardIterator.java:446)
  at org.openide.loaders.TemplateWizard$InstantiatingIteratorBridge.uninitialize(TemplateWizard.java:1076)
  at org.openide.loaders.TemplateWizardIterImpl.uninitialize(TemplateWizardIterImpl.java:215)
  at org.openide.loaders.TemplateWizardIteratorWrapper.uninitialize(TemplateWizardIteratorWrapper.java:131)
  at org.openide.WizardDescriptor.callUninitialize(WizardDescriptor.java:1541)
  at org.openide.WizardDescriptor.resetWizardOpen(WizardDescriptor.java:1383)
  at org.openide.WizardDescriptor.resetWizard(WizardDescriptor.java:1362)
[catch] at org.openide.WizardDescriptor.setValueOpen(WizardDescriptor.java:1346)

thank you for your fixes

breakponchito commented 1 year ago

I am not 100% convinced that a new project wizard is the best vehicle to integrate this kind of feature which is essentially code refactoring. It is working but it feels a bit like it is sidelining all other IDE features, e.g diffs before changes are applied etc. I am glad that it is here though since it certainly could be useful for some.

Regarding the place of this, at the beginning we though to add the functionality to the pop up menus when selecting folder or file, but after reviewing we though to add it on the project wizard. If you have an idea where to add it to be a better place, please let me know and I can change it

OndroMih commented 1 year ago

I am not 100% convinced that a new project wizard is the best vehicle to integrate this kind of feature which is essentially code refactoring. It is working but it feels a bit like it is sidelining all other IDE features, e.g diffs before changes are applied etc. I am glad that it is here though since it certainly could be useful for some.

I agree that this solution is not ideal and feels half-baked. But I understand it was already a lot of effort to build this solution and it's better than nothing.

In the future, I'd like to have this as an Inspect & Transform rule in the Refactor menu, here: image

This would allow to open a Java EE project and apply refactoring in place, directly on the project, including preview of changes if possible.

If we decide to keep the approach of a new project, I would at least expect that the generated project is opened in Netbeans after it's generated. That's what generally New project wizards do - they create a project in Netbeans, not just on the filesystem.

mbien commented 1 year ago

If you have an idea where to add it to be a better place, please let me know and I can change it

There are several places where code transformation could be integrated. It could be triggered by an in-editor code hint/inspection. Clicking at the light bulb (or running the code inspection manually) could: suggest to transform the current file or the whole project and should ideally result in a refactoring preview/diff.

But this could be still extended later if the feature remains popular. Not everything has to be solved by the same PR.

matthiasblaesing commented 1 year ago

The suggestion from @OndroMih sound sane, it would also imply, that not all suggested modes would work (at least I'm under the impression, that the hints engine always works in-place).

Locating this in the "New project" dialog is odd and the last place I would look for modifying an existing project.

If we want to get this in fast and now, then I suggest to make this an entry in the "Tools" menu. From a UX perspective though, I would prefer integration through the hint interface, as this nicely aligns with general cleanup and platform updates.

mbien commented 1 year ago

Locating this in the "New project" dialog is odd and the last place I would look for modifying an existing project.

If we want to get this in fast and now, then I suggest to make this an entry in the "Tools" menu. From a UX perspective though, I would prefer integration through the hint interface, as this nicely aligns with general cleanup and platform updates.

@matthiasblaesing Honestly, tools menu isn't much better IMO - its also not a place where I look for refactoring tasks. The implementation right now is separate from everything and not intrusive, I would be ok with integrating it as experimental feature, it seems like there is interest to properly integrate it at some point in future. (But my initial reaction was similar to yours)

The module has no public API (I removed that export), so we could remove it in future again if there is no interest to improve it. It could be extracted to a plugin (plugin portal) for example in that case.

This could be even delivered as maven hint, since it could directly annotate the dependency in the pom, replace it, then run the transformation.

We could of course also move this to NB 20, but the feature does work - maybe someone will find it useful in its early stage.

breakponchito commented 1 year ago

I'm agree with you guys that this tool can be improved, as @mbien said probably we can accept, merge and go with this as an experimental feature, then I will work to improve it for the next release. What do you think?

mbien commented 1 year ago

I have a squashed and rebased version of this PR here: https://github.com/mbien/netbeans/tree/FISH-6771-support-transformation-jakartaee8-to-jakartaee10

(I removed the FISH-6771 from the msg since I believe that this is an internal issue tracker)

If everyone is ok with this I can force push into this PR and merge on Sunday evening. (we don't use gh ui for squashing since it caused issues before with noreply mails in author headers etc)

juneau001 commented 1 year ago

I'd be happy to see this in NetBeans 19 and improved in a future release, as needed. Nice work! This will be very nice indeed for helping to further Jakarta EE 10 adoption.

mbien commented 1 year ago

@breakponchito congrats to your first contribution and thanks for your patience -> merging

breakponchito commented 1 year ago

@mbien @jGauravGupta @OndroMih @matthiasblaesing thank you guys for your help an patience