Liberty starter validation - #677
Conversation
| // Validate Group | ||
| String groupRaw = groupText.getText(); | ||
| String group = groupRaw.trim(); | ||
| String groupErrorMsg = "Valid characters for package names include a-z, A-Z, '_' and 0-9. Packages must be separated by '.'"; |
There was a problem hiding this comment.
This and other messages in this class also need to be translated.
| } | ||
|
|
||
| // Should not start or end with hyphen | ||
| if (artifact.startsWith("-") || artifact.endsWith("-")) { |
There was a problem hiding this comment.
I think this and the following check can be done more efficient with a regular expression. For example, what if the user enters --- or ----
| return false; | ||
| } | ||
|
|
||
| // Check for leading/trailing dots or consecutive dots |
There was a problem hiding this comment.
Same as previous comment. A regular expression would be better here to validate ending starting with ... or ...., etc.
Similarly with _ or -: here and in the previous case. do we allow users entering multiple _ or -?
| * @param mpVersion The MicroProfile version | ||
| * @return The highest compatible Jakarta EE version, or null if none found | ||
| */ | ||
| private String getFirstCompatibleEEVersion(String mpVersion) { |
There was a problem hiding this comment.
You might want to rename this method to getHighestCompatibleEEVersion. I was a bit confused about it until i read the content.
|
|
||
| // Add listeners to combo boxes. | ||
| SelectionListener comboListener = new SelectionAdapter() { | ||
| // javaSECombo validates page and also checks compatibility with current EE/MP selections. |
There was a problem hiding this comment.
Is this is still needed here? I am asking because a listener was already added and are doing the validation.
There was a problem hiding this comment.
Yes, this is needed. Unlike javaEECombo and microProfileCombo which have their own SelectionAdapters added in createVersionsSection(), javaSECombo(Called when user manually changes Java SE) only has this one listener. It calls checkAndUpdateJavaSE() to validate Java SE compatibility with current EE/MP selections and validatePage() for page completion tracking.
| * @param eeVersion The Jakarta EE version | ||
| * @param mpVersion The MicroProfile version | ||
| */ | ||
| private void checkAndUpdateJavaSE(String eeVersion, String mpVersion) { |
There was a problem hiding this comment.
Are we not getting the java SE validation data from the starter ... that is why you are manually verifying this?
If not, we need to take a different approach; otherwise, we will need to update this code every time a new compatibilities/versions are created.
There was a problem hiding this comment.
The Liberty Starter API (/api/start/info) does not provide Java SE compatibility constraints for Jakarta EE or MicroProfile versions ie it only provides EE↔MP compatibility via the constraints field under "e". Therefore, the Java SE validation rules are hardcoded based on the same logic used by the Liberty Starter website's frontend (builds.js). We are aware of this and will need to be updated when new versions are released, but there is no API-driven alternative currently available.
There was a problem hiding this comment.
Hi Malavika, i got around to trying the code. Here are a few of things i saw:
- When opening the package/project explorer, i do not see the link to create a Liberty starter.
- When i change the Java SE Version to an invalid version and it auto corrects, the error message does not go away even after subsequent correct selections.
- If i have a version of SE=8, EE=9.1, MP=5.0, and i change EE=10, The SE version changes to 11, and the MP version changes 7.1. These two changes are reflected in the site message, but not ours. We say just this: "Java SE Version has been automatically updated from 8 to 11. Jakarta EE 10.0 requires a minimum of Java SE 11." The site says this: "MicroProfile Version has been automatically updated from 5.0 to 7.1 for compatibility with Java EE / Jakarta EE Version.
MicroProfile Version 7.1 and Java EE/Jakarta EE Version 10.0 require a minimum of Java SE Version 11."
Please double check the combinations. We also need to make sure that the warning/error messages on the wizard are cleared as soon as the user starts selecting a new option. - The site messages say "with Java EE / Jakarta EE Version.", you are just using Jakarta EE. Let's match the messages.
- You also deleted a few .project files. Was that by accident?
Aside from this.
- Can you please add a trace for each of the Logger warning/error messages we issue.
After this PR is merged:
- We need to add some integration tests.
2 participants
No description provided.