CombinedResourceHandler make Primeface 6.0 can not write new resource for partial request #273

Closed
thojava opened this Issue Jun 22, 2016 · 14 comments

Projects

None yet

3 participants

@thojava
thojava commented Jun 22, 2016

Hi BalusC,

After i upgraded our system to PF 6.0 today, your CombinedResourceHandler seem not work well with the new version. Symptom is resources are required by ajax request is not encoded.

As i investigated, PrimePartialResponseWriter changed its algorithm on encoded dynamic resource, they only reload resource ( update tag ) if number of current resources is greater than number of resources in last request.

// dynamic resource loading
ArrayList<ResourceUtils.ResourceInfo> initialResources = DynamicResourcesPhaseListener.getInitialResources(context);
ArrayList<ResourceUtils.ResourceInfo> currentResources = ResourceUtils.getComponentResources(context);
if (initialResources != null && currentResources != null && currentResources.size() > initialResources.size()) {}

The initialResources is evaluated at restored view before your handler executed.
While the current resourced have been combined by your handler so above condition likely failed.

Could you help to look on this issue, let me know if you have any enquiries

@thojava thojava closed this Jun 22, 2016
@thojava thojava reopened this Jun 22, 2016
@BalusC BalusC referenced this issue in TheCoder4eu/BootsFaces-OSP Jun 22, 2016
Closed

PrimeFaces 6.0 Incompatibility #411

@BalusC
Member
BalusC commented Jun 22, 2016

Same problem is reported in BootsFaces: TheCoder4eu/BootsFaces-OSP#411

@BalusC
Member
BalusC commented Jun 24, 2016

I tried creating a reproducer so I could debug it, but I can't reproduce the problem. I imagined conditionally rendering or even building new PrimeFaces components would trigger it, but it doesn't. What's the minimum required code to reproduce this problem?

@thojava
thojava commented Jun 27, 2016

In my case, I have problem with p:fileUpload component.
The scenario is, at first request system just display list of employee in table layout. p:fileUpload is not required to rendered.
User click to choose one employee, system navigate he to the employee detail page. In this page, we need to render p:fileUpload but the required resource used by p:fileUpload are fileUpload.css and fileUpload.js are not loaded due to the problem explained originally.

Let me know if you need more info

@BalusC
Member
BalusC commented Jun 27, 2016 edited

So, navigating by ajax should trigger it? Sorry, I still can't reproduce it with below snippets on Tomcat 8.5.3 + Mojarra 2.2.13 + PrimeFaces 6.0 + OmniFaces 2.4-RC2.

index.xhtml

<h:form>
    <p:commandButton value="navigate to upload" action="upload" />
</h:form>   

upload.xhtml

<h:form>
    <p:fileUpload />
</h:form>   

It just returns a partial response with <update id="javax.faces.ViewRoot"> with the correct combined script resource already enclosed.

More detail or a minimal reproducer would be helpful.

@thojava
thojava commented Jun 27, 2016

Actually the summary and detail page i mentioned above are the same f:view. I conditionally render the summary or detail page.

So in your case index.xhtml and upload.xhtml share the same f:view or not ?

@BalusC
Member
BalusC commented Jun 27, 2016 edited

No, as you said it's a navigation.

As said in my comment 3 days ago I can't reproduce it when conditionally rendering the upload in a single view. Here's the full test snippet:

<h:form>
    <p:commandButton value="show upload" update=":upload" />
</h:form>   
<h:form id="upload">
    <p:fileUpload rendered="#{facesContext.postback}" />
</h:form>

Please provide a reproducer.

@thojava
thojava commented Jun 27, 2016

Sorry, I has been used the term navigation wrongly.
So the problem is with ajax request within the same f:view and conditionally render p:fileUpload

@BalusC
Member
BalusC commented Jun 27, 2016 edited

I can't do much without a reproducer. Because, the so far described cases work just fine for me. So I guess you just wrongly described the problem, or omitted some environment setting/detail which is different from default. Having concrete code or even a reproducer WAR says more than thousand words and avoids ambiguity and misnomers in description.

@thojava
thojava commented Jun 27, 2016

Okay. I will give you the reproducer project tomorrow.

@BalusC
Member
BalusC commented Jun 27, 2016

Great, looking forward it.

@thojava
thojava commented Jun 28, 2016

CombinedResourceTest.zip

I have uploaded source project here, it is a maven one, running on Wildfly but hope that you can run it with tomcat as well.

When building this sample project I have found out more clue.
The problem appear when I have added component and component resource dynamically at the PreRenderEventListener. You can easily check my source and comment there.

@mydeadlyvenoms

Hi BalusC, since you referenced this issue (https://github.com/TheCoder4eu/BootsFaces-OSP/issues/411) you may want to use https://github.com/mydeadlyvenoms/BootsFacesIssues to reproduce the issue, it is maven based..

Just upgrade to PrimeFaces 6.0 and add the CombinedResourceHandler feature.

@BalusC
Member
BalusC commented Jun 29, 2016 edited

I reproduced the problem and I have locally fixed it (just let CombinedResourceHandler skip its job when the current request is an ajax request with partial rendering, it's unnecessary to re-combine resources when they're not going to be rendered anyway). I will do some more intensive testing before committing the fix into 2.4 (because theoretically, state management might get confused by this and possibly do weird things).

@BalusC BalusC closed this in f4e74ac Jun 29, 2016
@BalusC
Member
BalusC commented Jun 29, 2016 edited

No bad things as to state management happened in both Mojarra 2.2.13 and MyFaces 2.2.10. Fix is available in today's latest 2.4-SNAPSHOT. Please let me know if that works out for you.

By the way, with new PF dynamic resource management, there's also no need anymore to explicitly exclude jquery.js and primefaces.js from combining (I saw you did that in the reproducer project).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment