You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by "volosied (via GitHub)" <gi...@apache.org> on 2023/01/27 04:03:28 UTC
[GitHub] [myfaces] volosied opened a new pull request, #504: MYFACES-4553: Fix WELD-001303 By Forcing Flow Transition Earlier
volosied opened a new pull request, #504:
URL: https://github.com/apache/myfaces/pull/504
Draft fix for https://issues.apache.org/jira/browse/MYFACES-4553
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] volosied commented on pull request #504: MYFACES-4553: Fix WELD-001303 By Forcing Flow Transition Earlier
Posted by "volosied (via GitHub)" <gi...@apache.org>.
volosied commented on PR #504:
URL: https://github.com/apache/myfaces/pull/504#issuecomment-1407295681
```
Error: Failures:
Error: FlowMyFacesRequestTestCase.testFlow1_10:940 expected:<flow[3]> but was:<flow[1]>
Error: FlowMyFacesRequestTestCase.testFlow1_10_1:1028 expected:<flow[3]> but was:<flow[1]>
Error: FlowMyFacesRequestTestCase.testFlow1_10_2:1116 expected:<flow[3]> but was:<flow[1]>
Error: FlowMyFacesRequestTestCase.testFlow1_8:684 expected:<flow[3]> but was:<flow[1]>
```
Made a update and it's down to 4 failures now. However, I don't really like this approach, but I don't see much we can do. We follow the spec (mostly), but I think Mojarra just made more updates made to address edge cases and the spec was never updated.
Basically, when should a flow be entered so that flowScope is active?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] tandraschko commented on a diff in pull request #504: MYFACES-4553: Fix WELD-001303 By Forcing Flow Transition Earlier
Posted by "tandraschko (via GitHub)" <gi...@apache.org>.
tandraschko commented on code in PR #504:
URL: https://github.com/apache/myfaces/pull/504#discussion_r1093503344
##########
impl/src/main/java/org/apache/myfaces/application/NavigationHandlerImpl.java:
##########
@@ -735,17 +709,59 @@ public NavigationCase getNavigationCommand(
}
continue;
}
- if (!complete && flowNode instanceof ViewNode)
+ if(!complete && flowHandler.getCurrentFlow() == null) // See MYFACES-4553 for details
{
- ViewNode viewNode = (ViewNode) flowNode;
- navigationCase = createNavigationCase(viewId, flowNode.getId(),
- viewNode.getVdlDocumentId());
- complete = true;
+ flowHandler.transition(facesContext, null, targetFlow, null, outcomeToGo);
+ facesContext.getAttributes().put(STARTED_FLOW_TRANSITION, true);
+ continue;
+ }
+ if (!complete && flowNode instanceof FlowCallNode)
+ {
+ // "... If the node is a FlowCallNode, save it aside as facesFlowCallNode. ..."
+ FlowCallNode flowCallNode = (FlowCallNode) flowNode;
+ targetFlow = calculateFlowCallTargetFlow(facesContext,
+ flowHandler, flowCallNode, currentFlow);
+ if (targetFlow != null)
+ {
+ targetFlowCallNode = flowCallNode;
+ startFlow = true;
+ continue;
+ }
+ else
+ {
+ // Ask the FlowHandler for a Flow for this flowId, flowDocumentId pair. Obtain a
+ // reference to the start node and execute this algorithm again, on that start node.
+ complete = true;
+ }
+ }
+ if (!complete && flowNode instanceof SwitchNode)
+ {
+ outcomeToGo = calculateSwitchOutcome(facesContext, (SwitchNode) flowNode);
+ // Start over again checking if the node exists.
+ //fromAction = currentFlow.getId();
Review Comment:
commented code? remove?!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] melloware commented on pull request #504: MYFACES-4553: Fix WELD-001303 By Forcing Flow Transition Earlier
Posted by "melloware (via GitHub)" <gi...@apache.org>.
melloware commented on PR #504:
URL: https://github.com/apache/myfaces/pull/504#issuecomment-1407390932
I wonder if this is a question for BalusC?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] melloware commented on pull request #504: MYFACES-4553: Fix WELD-001303 By Forcing Flow Transition Earlier
Posted by "melloware (via GitHub)" <gi...@apache.org>.
melloware commented on PR #504:
URL: https://github.com/apache/myfaces/pull/504#issuecomment-1409810671
Congratulations!! I know you have been fighting this one!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] tandraschko commented on a diff in pull request #504: MYFACES-4553: Fix WELD-001303 By Forcing Flow Transition Earlier
Posted by "tandraschko (via GitHub)" <gi...@apache.org>.
tandraschko commented on code in PR #504:
URL: https://github.com/apache/myfaces/pull/504#discussion_r1093502826
##########
impl/src/main/java/org/apache/myfaces/application/NavigationHandlerImpl.java:
##########
@@ -364,6 +366,15 @@ else if (fromViewId != null && !fromViewId.equals(toViewId))
}
}
}
+
+ private Boolean didFlowTransitionAlready(FacesContext facesContext)
+ {
+ if(facesContext.getAttributes().containsKey(STARTED_FLOW_TRANSITION))
Review Comment:
return facesContext.getAttributes().containsKey(STARTED_FLOW_TRANSITION);
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] volosied commented on pull request #504: MYFACES-4553: Fix WELD-001303 By Forcing Flow Transition Earlier
Posted by "volosied (via GitHub)" <gi...@apache.org>.
volosied commented on PR #504:
URL: https://github.com/apache/myfaces/pull/504#issuecomment-1405997332
This code addresses the issue found in the TCK, but it fails in our tests. You can find the failures in the build log, for example:
```
Error: Tests run: 5, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 2.055 s <<< FAILURE! - in org.apache.myfaces.application.flow.FlowMyFacesCDIRequestTestCase
Error: org.apache.myfaces.application.flow.FlowMyFacesCDIRequestTestCase.testFlow1_1 Time elapsed: 0.369 s <<< FAILURE!
java.lang.AssertionError:
expected null, but was:<<navigation-case>
<from-view-id>/flow1_1.xhtml</from-view-id>
<from-outcome>content</from-outcome>
<to-view-id>/flow1/content.xhtml</to-view-id>
</navigation-case>>
at org.junit.Assert.fail(Assert.java:89)
at org.junit.Assert.failNotNull(Assert.java:756)
at org.junit.Assert.assertNull(Assert.java:738)
at org.junit.Assert.assertNull(Assert.java:748)
at org.apache.myfaces.application.flow.FlowMyFacesCDIRequestTestCase.testFlow1_1(FlowMyFacesCDIRequestTestCase.java:72)
```
https://github.com/apache/myfaces/blob/93a47f3c78716f0ff3c01227ee1cd5972aeb965b/impl/src/test/java/org/apache/myfaces/application/flow/FlowMyFacesCDIRequestTestCase.java#L59-L72
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] volosied merged pull request #504: MYFACES-4553: Fix WELD-001303 By Forcing Flow Transition Earlier
Posted by "volosied (via GitHub)" <gi...@apache.org>.
volosied merged PR #504:
URL: https://github.com/apache/myfaces/pull/504
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] volosied commented on pull request #504: MYFACES-4553: Fix WELD-001303 By Forcing Flow Transition Earlier
Posted by "volosied (via GitHub)" <gi...@apache.org>.
volosied commented on PR #504:
URL: https://github.com/apache/myfaces/pull/504#issuecomment-1409727137
GREEN GREEN GREEN! 💚 Never been so happy to see a "BUILD SUCCESSFUL".
Still need do some more testing and refactor the changes a bit, but I think I've fixed this without causing any regressions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] volosied commented on a diff in pull request #504: MYFACES-4553: Fix WELD-001303 By Forcing Flow Transition Earlier
Posted by "volosied (via GitHub)" <gi...@apache.org>.
volosied commented on code in PR #504:
URL: https://github.com/apache/myfaces/pull/504#discussion_r1093513692
##########
impl/src/main/java/org/apache/myfaces/application/NavigationHandlerImpl.java:
##########
@@ -735,17 +709,59 @@ public NavigationCase getNavigationCommand(
}
continue;
}
- if (!complete && flowNode instanceof ViewNode)
+ if(!complete && flowHandler.getCurrentFlow() == null) // See MYFACES-4553 for details
{
- ViewNode viewNode = (ViewNode) flowNode;
- navigationCase = createNavigationCase(viewId, flowNode.getId(),
- viewNode.getVdlDocumentId());
- complete = true;
+ flowHandler.transition(facesContext, null, targetFlow, null, outcomeToGo);
+ facesContext.getAttributes().put(STARTED_FLOW_TRANSITION, true);
+ continue;
+ }
+ if (!complete && flowNode instanceof FlowCallNode)
+ {
+ // "... If the node is a FlowCallNode, save it aside as facesFlowCallNode. ..."
+ FlowCallNode flowCallNode = (FlowCallNode) flowNode;
+ targetFlow = calculateFlowCallTargetFlow(facesContext,
+ flowHandler, flowCallNode, currentFlow);
+ if (targetFlow != null)
+ {
+ targetFlowCallNode = flowCallNode;
+ startFlow = true;
+ continue;
+ }
+ else
+ {
+ // Ask the FlowHandler for a Flow for this flowId, flowDocumentId pair. Obtain a
+ // reference to the start node and execute this algorithm again, on that start node.
+ complete = true;
+ }
+ }
+ if (!complete && flowNode instanceof SwitchNode)
+ {
+ outcomeToGo = calculateSwitchOutcome(facesContext, (SwitchNode) flowNode);
+ // Start over again checking if the node exists.
+ //fromAction = currentFlow.getId();
Review Comment:
ah, it was originally there, but let me remove it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org