Hello Josu,
I have reviewed your patch and you are right.
It appears that the code in ProcessUtils.getMaxVisitedRowKey() never
does the comparison when the navigation outcome changes for the current
request. That's definitely why you are seeing the max visited icons fix
themselves during the subsequent request.
I made some minor changes to your patch fix and have uploaded a new one.
Mostly I did the following:
1. assert that argument 'maxPathKey' is not null. This method assumes
that we are using the Max Visited logic. It maxPathKey was never set
ProcessMenuModel will revert to the 'Plus One' logic.
2. Retrieve the focusRowKey once in the beginning and then resuse it
everywhere
3. Rename 'maxPath' to 'maxRowKey' to be more accurate.
Please review, test in your usecase and let me know. I'll work on
getting the changes committed.
Thanks for uncovering it!
Pavitra
On 7/2/2010 12:09 AM, Josu Vergara (JIRA) wrote:
Issue in org.apache.myfaces.trinidad.model.ProccessUtils.getMaxVisitedRowKey
----------------------------------------------------------------------------
Key: TRINIDAD-1844
URL: https://issues.apache.org/jira/browse/TRINIDAD-1844
Project: MyFaces Trinidad
Issue Type: Bug
Affects Versions: 1.2.12-plugins
Environment: Mac OSX 10.5 with JVM 1.6.0_20.
Running using ADF with JDeveloper 11.1.1.3.0
Reporter: Josu Vergara
Priority: Minor
I have been running the following example:
http://adf-samples.googlecode.com/files/SampleTrainModelWithCustomAction.zip
I just changed the view.train.TrainIdMenuModel class constructor to:
public TrainIdMenuModel() {
super();
super.setMaxPathKey("MyMaxPathKey");
}
This is done to get a max visited node behavior. However when I click on the
button 'Next' to navigate on the tree the nodes are not enabled as expected and
the user has to click on 'Back' and then 'Next' twice to get the nodes enabled
correctly.
The example can be found in this blog entry:
http://jobinesh.blogspot.com/2010/04/custom-model.html which is based on the
ADF rich client demo
(http://www.oracle.com/technology/products/adf/adffaces/11/doc/demo/adf_faces_rc_demo.html).
The rich client demo can also be used to reproduce the issue.
I accept that using a case test that relies in a whole framework like ADF is not ideal,
but looking at the code of org.apache.myfaces.trinidad.model.ProcessUtils I think that
the problem lies there. In the javadoc it is stated: "If set but the focus rowKey
is after maxVisitedRowKey, set maxVisitedRowKey to the focus rowKey."
However if we look at the code, we can see that this is not respected since the
maxPath variable is 'cached' at the request map.
If we assume that a MenuModel is going to be updated during the processing of a
request BUT that the ProccessUtils.getMaxVisitedRowKey is invoked BEFORE the
model is actually updated, the maxPath value will be calculated (and cached)
using the non updated model. In particular the maxPath will be calculated
incorrectly if the focusRowKey of the model is NOT updated before the first
call to ProccessUtils.getMaxVisitedRowKey occurs during the request. After
this, if the focusRowKey is updated in the model and
ProccessUtils.getMaxVisitedRowKey is called (always being in the context of the
same request), the contract will not be respected and the cached maxPath value
will be returned (which can correspond to a node BEFORE the current focus node,
which is wrong).
