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

Migrate to Jakarta EE #81

Open
9 tasks
albfernandez opened this issue Jun 22, 2024 · 52 comments
Open
9 tasks

Migrate to Jakarta EE #81

albfernandez opened this issue Jun 22, 2024 · 52 comments
Assignees

Comments

@albfernandez
Copy link
Owner

albfernandez commented Jun 22, 2024

This issue aggregate al tasks necessaries to complete the migration to Jakarta EE
It's a open list, new tasks may will be added when doing the migration work

Projects
This projects have to be migrated in order

  • jsf-test
  • richfaces-cdk
  • richfaces (this project)

Tasks

jsf-test

richfaces-cdk

richfaces

@albfernandez albfernandez added this to the 5.0 Jakarta EE milestone Jun 22, 2024
@albfernandez albfernandez self-assigned this Jun 22, 2024
@melloware
Copy link

@albfernandez you may also want to rip out the CKEditor component while I think the license is perfectly fine the PrimeFaces Extensions project got hounded by their lawyers until we just ripped it out. See: primefaces-extensions/primefaces-extensions#1106

@miguelzapaton
Copy link

Anybody has started with the Jakarta EE migration yet?

@miguelzapaton
Copy link

I just did a work-in-progress commit to share an intermediate result (with MZ comments, println logging, etc.) of my Jakarta EE migration intent:

  • versioned as 10.0.0-SNAPSHOT to align with ee10
  • currently myfaces only
  • builds a jar (when tests are bypassed)

Most important issues:

  • tests with errors or failing, especially: core, a4j, rich
  • managed beans are removed, CDI environment required

Artifacts:

https://github.com/miguelzapaton/jsf-test

https://github.com/miguelzapaton/richfaces-cdk

https://github.com/miguelzapaton/richfaces

Hints to try it out:

  1. mvn clean install jsf-test (with tests, resolve workspace artifacts, update snapshots)

  2. mvn clean install richfaces-cdk (with tests, resolve workspace artifacts, update snapshots)

  3. mvn clean install only the core project of richfaces (with tests, resolve workspace artifacts, update snapshots)

  4. mvn install richfaces (currently skip tests, resolve workspace artifacts, update snapshots)

Feedback (and help) very welcome!

@melloware
Copy link

Nice work @miguelzapaton ! Making great progress!

@albfernandez
Copy link
Owner Author

Great work @miguelzapaton

I like the 10.0 version idea.

Maybe setting java version to 11 (and ensuring it works with 11,17,21)

I think next step should be migrate examples/showcase project to manual testing

Note to me (and others)

jsf-test:           mvn clean install
richfaces-cdk:      mvn clean install
richfaces (core):   mvn clean -DskipTests install
richfaces(root):    mvn clean -Dmaven.test.skip install

@albfernandez
Copy link
Owner Author

I've created the jakarta branch for jsf-test, richfaces-cdk and richfaces in my repository and incorporated your changes

@MilovdZee
Copy link

MilovdZee commented Jan 6, 2025

I have been working on it and fixed some issues. In my project that uses richfaces most is now working: https://github.com/MilovdZee/richfaces-jakarta in the main branch.
I also re-added the mediaOutput as I need that. If needed we can fix the security issues or enable it optionally.

Problem now is that http://localhost:8080/org.richfaces.resources/jakarta.faces.resource/org.richfaces/richfaces.js is not found. I did enable the resource mapper and resource servlet but somehow it does not work. When I copy all the richfaces resources to my own project in the resources folder it does work. Could it be that JSF changed in that it does not allow downloading resources from dependencies?

@melloware
Copy link

Jakarta JSF still allows resources the same as JavaX version no change there.

@MilovdZee
Copy link

MilovdZee commented Jan 6, 2025

Oke, then there must be something else why it can't find the resources. I traced it a lot and at some point it just forwards the resource request to the default one after chopping of the org.richfaces.resources part. And then it is not found. I'll research a bit further the coming days.

@MilovdZee
Copy link

I'm very stuck. In my application richfaces tries to request http://localhost:8080/org.richfaces.resources/jakarta.faces.resource/org.richfaces/richfaces-base-component.js but it does not find the resource. I see it entering org.richfaces.webapp.ResourceServlet.httpService() and it forwards the request to facesServlet.service(request, response)but then nothing is found.
Any suggestion is very welcome. This looks like one of the last hurdles to get this really useful.

@melloware
Copy link

your URL looks weird? Here is what PrimeFaces looks like asking for core.js

https://www.primefaces.org/showcase/jakarta.faces.resource/core.js.xhtml?ln=primefaces

so yours should not have a JS extension? Why is org.richfaces after resource but before the JS? the ln=primefaces i would expect to be ln=richfaces for standard JSF?

@MilovdZee
Copy link

Agreed. But this is how it used to be. In a working application that uses the javax original richfaces the request looks like this: https://<host>/org.richfaces.resources/javax.faces.resource/org.richfaces/richfaces.js

@melloware
Copy link

ok good. Weird...but good.

@MilovdZee
Copy link

From what I understand the org.richfaces.resources is to map it to the resourcehandler instead of by using .xhtml or .jsf. And the org.richfacesis handled as an alternative to the ln= part. Should be fine and the request is actually picked up by the resourceHandler.

@melloware
Copy link

so then in the JAR file i would expect META-INF/resources/org.richfaces/richfaces.js to be there can you confirm that file is there exactly in your Richfaces JAR?

@MilovdZee
Copy link

Yes, I checked the jar and the files are there

@MilovdZee
Copy link

image

@melloware
Copy link

OK then i am stumped...

@MilovdZee
Copy link

https://issues.apache.org/jira/browse/MYFACES-4524
This creates the problem. In FacesServletMappingUtils.createMappingFromServletRegistration() is the servletPath matched exactly then it used to return a prefixMapping but now it returns an exactMapping and this is later fully ignored. Probably I can update the servlet org.richfaces.webapp.ResourceServlet to trigger a prefixMapping

@MilovdZee
Copy link

MilovdZee commented Jan 11, 2025

Updating org.richfaces.webapp.ResourceServlet fixed the problem of not finding resources:
It was public class ResourceServlet implements Servlet {} and by adding DelegatedFacesServletir now creates the correct mapping.
Fixed: public class ResourceServlet implements Servlet, DelegatedFacesServlet {}

@MilovdZee
Copy link

MilovdZee commented Jan 11, 2025

Next issue... jsf.js does not seem to exist anymore and RichFaces uses it in it's js files
Found that myfaces now uses jsf.ts files. So probably I need to change the config for MyFaces a bit
=> File is now called faces.js and is loaded fine

@melloware
Copy link

yep faces.js. And yeah MyFaces issue probably closed a gap in the spec.

@MilovdZee
Copy link

I updated the javascript to use faces instead of jsf and all seems to work fine now :)

Maybe time to merge my changes into the main jakarta branch. I'll try to create a fork and then a merge request

@MilovdZee
Copy link

skin does not work

@miguelzapaton
Copy link

Really cool what my eyes are seeing here @MilovdZee

Thank you @albfernandez for creating the jakarta branch and using my intermediate result as a starting point.
Currently I'm too busy with other things but I hope to have time for further improvements soon. I still have some entries on my list regarding the tests etc.

Following the discussion above I'm a bit skeptical about the editor and mediaOutput components. I think there are strong reasons to no longer include them and maybe that's the case for other components too.

@melloware Do you think that a well running Jakarta RF 10 could be run in parallel with Primefaces without further adaptions?
I (and maybe others) would like to do an inplace migration in one of my applications.

Looking forward to getting this fully done together.

@melloware
Copy link

As for PF it should play just fine with this Jakarta RF branch!

I really think the CKeditor component should be removed as I mentioned they harassed PF extensions project about licensing until we finally removed it.

@MilovdZee
Copy link

I don't care about the CKeditor (I'm using TinyMCE) but I do need the mediaOutput. What is the reason not to keep the mediaOutput? If needed we can introduce a flag that enables the component.

@albfernandez
Copy link
Owner Author

mediaoutput was removed to fix CVE-2018-12532, If reintroduced, this vulnerability have to be fixed first.

ckeditor has two issues:

  • The license issue. I think Richfaces license is compatible, but if they are harassing other opensource projects it's safe to remove.
  • Ckeditor 4 (open source) is not maintained anymore.

@MilovdZee
Copy link

MilovdZee commented Jan 12, 2025

@ManagedBean("richfacesVersion")
@ApplicationScoped
public class RichfacesVersion {
    public String getVersion() {
        return "richfaces version";
    }
}

This does not work. If I replace ManagedBean with Named then it works. This is probably the reason that the RF beans are not picked up. They are defined as managedbean in the core.faces-config.xml. I checked that the core.faces-config.xml file is scanned and it is. When I create a classname fault I do indeed get an error. So the 'managed-bean' entries are not read somehow.

I also tried annotating the beans in RF with Named but even then they are not picked up. Any suggestions?

@melloware
Copy link

Yes ManagedBeans are gone you have to use CDI and @nAmed

@MilovdZee
Copy link

MilovdZee commented Jan 12, 2025

That is indeed what I remember but why is @ Named not being picked up when I use it in the RF bean definitions?
=> Duhhh, simple: the file beans.xml was missing. Pfffff

@MilovdZee
Copy link

That also solved the skinning problem

@MilovdZee
Copy link

And more is working. Annoying thing is that 'render' seems to behave differently. I'm re-rendering components inside composite facelets but that does not work anymore. Weird

@MilovdZee
Copy link

MilovdZee commented Jan 14, 2025

The response looks fine:

<?xml version="1.0" encoding="UTF-8"?>
  <partial-response id="j_id__v_0">
    <changes>
      <update id="messageTabBody:newMessage:newMessagePanel">
        <![CDATA[<div id="messageTabBody:newMessage:newMessagePanel"....

html:
image

And some re-renders work fine and other don't. I'll create a minimal test.

@melloware
Copy link

what version of MyFaces and Mojarra are you testing with? MyFaces 4.0.2 and Mojarra 4.0.9?

@MilovdZee
Copy link

MilovdZee commented Jan 14, 2025

Yes 4.0.2 and no Mojarra

I also see that the response is exactly the same in a working environment:

<?xml version="1.0" encoding="UTF-8"?>
  <partial-response id="j_id__v_0">
    <changes>
      <update id="messageTabBody:newMessage:newMessagePanel">
        <![CDATA[<div id="messageTabBody:newMessage:newMessagePanel" 

html:
image

@MilovdZee
Copy link

After comparing I see that after the render the failing one is actually updated but with visibility set to hidden and the height and width are not updated. So this is probably an issue with the RF popupPanel.js. It probably needs to update some values after rendering.

@MilovdZee
Copy link

MilovdZee commented Jan 14, 2025

Looks like the popupPanel.js show() is not called. If I do a refresh of the page with f5 then the popup is shown.

I also notice that an 'ajaxcomplete' message is received by the popupPanel code and it tries to resize. But in working version the panel is by then visible but in the failing version nothing is shown.

@melloware
Copy link

@MilovdZee if its not calling show() is sounds like the AJAX oncomplete or something is not working. Any error in F12 console?

@MilovdZee
Copy link

MilovdZee commented Jan 14, 2025

No errors at all. Also the ajaxcomplete was triggered and handled by the popupPanel code.

I'm now calling the show() after a window.setTimeout() of 10ms and it works fine :)
Probably the dom was not ready yet and in the old version was a bit quicker or something.
This might not have to do anything with RF. This might be me abusing RF beyond what it can handle.

The panel uses rendered="#{cc.attrs.show}" and I set that flag right after a button click. Then I render the parent element and call the show(). That might be a bit timing tricky.

@MilovdZee
Copy link

MilovdZee commented Jan 15, 2025

image

With a lot of the tests. I disable all tests for the 'rich components' module and some scattered around but many are already working fine.

All pushed to https://github.com/MilovdZee/richfaces-jakarta

@MilovdZee
Copy link

Might it be an idea to just integrate CDK and jsf-test into this project instead of having them external?

@albfernandez
Copy link
Owner Author

Might it be an idea to just integrate CDK and jsf-test into this project instead of having them external?

I think there should be separate projects, I've jboss-seam fork that relays on richfaces-cdk

@MilovdZee
Copy link

Really? Oke, yes, if it is actually used elsewhere then it need to be separate.

For the tests to work we do need some serious fixing though.

@MilovdZee
Copy link

My application now does a lot more full page reloads. So that is annoying. Something with the rendering changed...

@MilovdZee
Copy link

MilovdZee commented Jan 25, 2025

I tested a lot and I don't see any issues any more.
Except for that I now see a bit more full-page renders that result in a flickering screen. But that is only at moments I actually switch pages and so might be expected. But it used to be nicer. And I think this issue comes from MyFaces and not RichFaces. I investigated the response xml and they are 100% the same between the old Richfaces with the old Myfaces and this newly migrated version with a much newer Myfaces. So it must be in the handling of the received xml page fragment. And that is handled by Myfaces.

I'll try to create a merge request to the 'jakarta' branch => #86

@albfernandez
Copy link
Owner Author

Good work! Thanks.
I'll try to review and test the changes this weekend

I'm worried about mediaoutput... CVE-2018-12532 has to be fixed before release.

@MilovdZee
Copy link

Agreed. Maybe just do it the easy way and make it opt-in functionality with a big warning? That gives us (you?) some time to fix it.

And I disabled quite some tests...

@MilovdZee
Copy link

The flickering was due to that I used 'redirect' in the faces-config. I now removed that and that now is fine.

@MilovdZee
Copy link

MilovdZee commented Feb 2, 2025

But there is a bug that looks related to MyFaces but I don't know:
in status.js:

        const statusAjaxEventHandler = function (data, methodName) {
        if (methodName) {
            //global status name
            const statusName = getGlobalStatusNameVariable();

            let statusApplied = false;
            const statusDataAttribute = getStatusDataAttributeName(statusName);

            let statusContainers;
            if (statusName) {
                statusContainers = [$(document)];
            } else {
                const source = data.source;
                if (source === null) {
                    console.log(`Error: source is null. methodName='${methodName}', statusName='${statusName}'`)
                    return;
                }

                // the element reference will be stale if it was rerendered
                // we need to find the current element in the DOM
                const currentSource = document.getElementById(source.id);

                statusContainers = [$(currentSource).parents('form'), $(document)];

This very often does not find the source and so shows the error message. This results in that in:
<a4j:status id="status" startText="" stopText="" onstart="showAjaxActive();" onstop="hideAjaxActive();"/>
The onstop is not called.

https://stackoverflow.com/questions/79406526/myfaces-4-0-2-does-not-always-seem-to-know-what-the-source-of-the-ajax-event-was

@MilovdZee
Copy link

This piece of code often shows the warning. Not sure why. Does not give any problems though

    public void addQueue(String clientName, UIComponent component) {
        if (!containsQueue(clientName)) {
            queuesData.put(clientName, component);
        } else {
            LOGGER.warn("Queue with name '" + clientName + "' has already been registered");
        }
    }

@MilovdZee
Copy link

Good work! Thanks. I'll try to review and test the changes this weekend

I'm worried about mediaoutput... CVE-2018-12532 has to be fixed before release.

Any luck yet? I deployed the application to production and a team now has been using it for about a week. I did not get any feedback about things that are not working. At least not related to the RichFaces upgrade.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants