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

Big sweep: migrate codebase to Jakarta EE 10 and use Payara 6 now (third iteration) #9685

Merged
merged 57 commits into from
Aug 11, 2023

Conversation

poikilotherm
Copy link
Contributor

@poikilotherm poikilotherm commented Jun 28, 2023

THIS IS READY BUT SHOULD NOT BE MARKED NON-DRAFT UNTIL 5.14 HAS BEEN RELEASED

What this PR does / why we need it:

Payara 5 is going EOL for community usage (= without a payed support contract). Payara 6 is the successor, but allows to use Jakarta EE 10 only. Issue #8305 is the deliverable, where this PR is a part of.

At tech hour we agreed that we want to do a clean cut after the 5.14 release and migrate develop to Jakarta EE 10 and hack on it further. This PR prepares this step.

This PR is a successor to PR #9116

Which issue(s) this PR closes:

Special notes for your reviewer:

Note that as 5.14 is not yet released, this PR will need an update from develop once done and will need to add some more migrations of the outstanding code parts that are due for 5.14. (These are minor, so we started the branch)

Open Tasks this PR did not address (and maybe should be fed back to #8305):

Suggestions on how to test this:

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

(No)

Is there a release notes update needed for this change?:

Yes, a stub is included, but needs work. Maybe can be done as part of another issue for Dataverse 6 release notes.

Additional documentation:

None but references in the guides to Payara 5 have been switched to Payara 6.

poikilotherm and others added 27 commits June 28, 2023 16:56
1. The Jersey dependencies in use did not use the versions imported via the
Payara BOM (which itself imports a Jersey BOM).
2. The coordinates for the server package changed, re-aligned.
…nly #8305

This avoids potential trouble were it's unclear from where the higher exceptions come from, javax or jakarta namespace.
Also add provided dependency of our persistence provider to POM to allow class resolution in IDEs.
Many projects have moved to Eclipse foundation, have been replaced
or renamed with the move to Jakarta EE 10.

With the import of the new EE 10 Payara BOM, we need to follow
their changes (or provide the deps on our own).
As the SPI module used Java EE, we need to release a v2,
as the move to Jakarta EE is a breaking change.
Removing some rogue usages of OcpSoft PrettyFaces internal utils
for String care and replacing with either Apache Commons Lang3
or native JVM code.
…8305

Any users registered before 2021 release to oss.sonatype.org.
As gdcc was installed after that, we only had s01.oss.sonatype.org.

Also enabling both snapshot repos for Rewrite and SWORD2

See also https://central.sonatype.org/news/20210223_new-users-on-s01
…8305

This is due to the fact that Payara 6 Servlets are more strict in their
parsing of HTTP headers.
In EditDDI I was seeing a NullPointerException on line 136
(createNewDraftVersion). Switch from `@Context HttpServletRequest`
to our ordinary way of API auth fixed it.

In DataversesIT I was seeing an OptimisticLockException so I
added a sleep.
Involves Jakarta Faces 4.0 and Servlet 6.0
The backing bean actions were attached to normal "<a>" elements,
which means they were executed on page load and not, as expected,
via JavaScript onClick. Former versions of JSF were less strict,
now we need to use the standard compliant "<h:commandLink>"
@poikilotherm poikilotherm added Component: Containers Anything related to cloudy Dataverse, shipped in containers. D: Payara 6 Upgrade Issues and PRs about the move to Jakarta EE 10 + Payara 6 Size: 30 A percentage of a sprint. 21 hours. (formerly size:33) labels Jun 28, 2023
@github-actions

This comment has been minimized.

@kcondon kcondon self-assigned this Aug 7, 2023
The Perl installer was removed in b829ca7
@github-actions

This comment has been minimized.

@kcondon
Copy link
Contributor

kcondon commented Aug 8, 2023

Issues found:

  1. Repeated errors thrown in logs when doing nothing and many tasks, publish, nav to dataset page from homepage link, create dataverse, dataset, etc. [KC] Bob saw this in his p5 logs, checking java ver. Leonid will check on his p6 env. [Update] seems isolated to loading a missing/incorrect dataverse so opened a separate issue Error Handling: 404 page thrown as a result of trying to load missing/incorrect Dataverse results in large stack trace but otherwise functions. #9770 , not blocking this one.
    idle_log_err.txt
    faces_context_large_stack_trace.txt
    faces_context_stack_trace.txt

[X] 2. Click dataset page, file kebab, metadata, stack trace but ui ok [KC] Phil is looking into this [Update] fixed, no log error.
file_kebab_metadata_err.txt

Two proposed fixes:

  • b7ec905: switch to DatasetPage.editFileMetadata()
  • fedb103: switch to bean.editFileMetadata() and add no-op for Page.editFileMetadata()

[X] 3. Clicking on filename from tree view throws 500 error in ui, saw same context validation errs in log, same error just clicking on file in dataset file table. [KC] Fixed by Oliver.
Screen Shot 2023-08-08 at 12 04 06 PM
[X] 4. Possibly related to 2, tabular tags silently can't be added, throws server log err when added from edit metadata popup from files page: [KC] This is fixed now Jim's prior fix was merged/refresh from develop
file_page_tab_tag_select_err.txt

  1. Possibly related to 2, embargo fails with file already published even when dataset hasn't yet been published. Two failure modes from dataset page kebab: a. if file not selected but use file level kebab to embargo, get only error in popup, b. if select file checkbox then choose embargo from file level kebab, get normal embargo popup but with error and no save and server log error.
Screen Shot 2023-08-09 at 1 13 29 PM Screen Shot 2023-08-09 at 1 15 12 PM [embargo_fail.txt](https://github.com/IQSS/dataverse/files/12305211/embargo_fail.txt)

Note: Embargo works from file page
[KC] I've opened a separate issue for this to provide clarity and more room to work: #9773

pdurbin and others added 2 commits August 8, 2023 13:54
The backing bean actions were attached to normal "<a>" elements,
which means they were executed on page load and not, as expected,
via JavaScript onClick. Former versions of JSF were less strict,
now we need to use the standard compliant "<h:commandLink>"
@poikilotherm
Copy link
Contributor Author

Issues found:
[...]
3. Clicking on filename from tree view throws 500 error in ui, saw same context validation errs in log, same error just clicking on file in dataset file table.

Should be fixed with 3716c5c.

I scanned all XHTML code for other examples of some <a> with an jsf:action or jsf:actionHandler and could not find other appearances. Looks like dataset and file page citation widget were the only places.

@github-actions

This comment has been minimized.

The widget for the dataset citation on the file page was forgotten
to be converted to <h:commandLink> as well to avoid action execution on page load.
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@landreev
Copy link
Contributor

landreev commented Aug 9, 2023

Regarding item 1. above - those long "illegal State" and "validation failed" exception stack traces are very easy to reproduce: just try accessing a dataverse that does not exist in the database - http://localhost:8080/dataverse/blahblahblah
I'm seeing it in my own payara6 build environment too.

@poikilotherm
Copy link
Contributor Author

poikilotherm commented Aug 10, 2023

Regarding item 1. above - those long "illegal State" and "validation failed" exception stack traces are very easy to reproduce: just try accessing a dataverse that does not exist in the database.

I can verify this procedure and see error logs as well.

As I wrote on internal Slack, this issue is triggered by how the rewrite filter works. From following the execution step by step via debugger, I can see that Jakarta Faces does the abort and the rewrite filter tries to forward us. The first log message is written by the filter detecting we want to abort.

I see an old discussion about this in ocpsoft/rewrite#151. We should create a simpler reproducer and create an issue over there.

For now, the bug does not seem to be fatal, just annoying - the 404 page still works. If I would have a say in this, I'd suggest creating a follow up issue to track this but not let it block merging this PR.

I'm attaching the full output here as well: illegalstate-rewrite-404.txt (from accessing http://localhost:8080/dataverse/test)

@scolapasta
Copy link
Contributor

My vote for the kebab metadata issue above would be the 2nd fix: fedb103
as I don't love referencing the DatasetPage on a multi-use bean. I'm also not too particularly concerned, as eventually all this will be going away.

As of Payara 6/JSF 4/Jakarta EE 10, this syntax doesn't work:

action="#{bean[editFileAction]()}"

(Or it logs errors in server.log at the very least.)

So we switch to this syntax:

action=#{bean.editFileMetadata()}"

This means we have to make sure the method exists on all the beans,
though, or we get errors. (It's unclear why we weren't seeing errors
before!) So we add the method to FilePage, which gets passed as bean.

We also remove the now unused "editFileAction".
@github-actions
Copy link

📦 Pushed preview images as

ghcr.io/gdcc/dataverse:8305-payara6-ee10-v3
ghcr.io/gdcc/configbaker:8305-payara6-ee10-v3

🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name.

@kcondon
Copy link
Contributor

kcondon commented Aug 11, 2023

Merging this after discussing on tech. @scolapasta Re: P6 I'm ok with merge now and resolve remaining issues as top priorities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Containers Anything related to cloudy Dataverse, shipped in containers. D: Payara 6 Upgrade Issues and PRs about the move to Jakarta EE 10 + Payara 6 Size: 30 A percentage of a sprint. 21 hours. (formerly size:33)
Projects
None yet
7 participants