This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
I have a use case that is not covered by the current implementation of lookup and getCookie in FilterNode. * have a Node's subclass that gets lookup in its constructor * have a FilterNode's subclass that overrides getCookie * the filter node lookup does not merge FN.getCookie and N.getLookup but contains just FN.getCookie contents which is wrong. The usecase relates to node factories in java/srcmodel module. I need to migrate them to represent JMI objects (not Node.Cookies) and still support old factories. As a workaround I could introduce new cookie but it means to introduce new useless api.
Created attachment 16129 [details] After 8hours of continuous mangling of the implementation I think I now satisfy all the tests
Dear api reviewers, Honza wants me to do an incompatible change in the behaviour of lookup (see the part of the diff with the test). In 3.6 if one used FilterNode with overriden getCookie, none of the non-cookie content of lookup of the original node was visible in the filter node's lookup. This was done in expectation that when one overrides getCookie one wants to filter things out from the original node and would not be glad if some cookies (or other content) just magically appeared there. But Honza claims that he needs to propagate his JMI interfaces (non-cookies) thru the chain of any FilterNodes even those that override cookie (because they want to add something). This sounds logical, so I'd like to apply the change, but as I mentioned it is in fact incompatible with 3.6 so review of that would be good. Also Honza, please verify that the attached patch solves your problem. If no objections I am going to integrate this at the end of Jul.
-1 from me; the 3.6 behavior seems more correct. In Honza's case, the FilterNode which overrides getCookie should be fixed to operate the lookup level.
Yarda, it seems this request didn't pass devrev. Will you close it?
Why to implement the same thing in subclasses when Jarda has a general solution? See the current cookies/lookup implementation in nodes, it is not the code that I would like to replicate whenever I need it. Moreover whoever who will filter my nodes will have to do the same again. IMO we have not encountered this issue yet since no one uses the node's lookup in current codebase. Sure, I can live without this :-) but then at least tweak FilterNode.getCookie javadoc and describe what user have to do if they want to override the method. Now it makes an impression that calling super.getCookie is all what you need.
From user point of view I see two different reasons why to write FilterNode (and override getCookie): 1. I want to provide everything that the original node provides and add some new cookie 2. I want to filter the cookies of original node and return just those that I like of course there can be a mixture, but it makes no sence to argue what is more natural. That fact is that the 3.6 version expects case #2. Jan tried to convince me that the more suitable is case #1, that is why I provided this implementation, but I am not sure, whether #1 is more offen than #2. From point of view of backward compatibility it might make sence to allow the creator of FilterNode to decide whether the filter node should conform to case #1 or case #2. Both usecases seem to be needed and I have a feeling they should be supported. Jan: Can you please enumerate the FilterNode subclasses that would like to use style #1 so we can take a look at their source code and try to understand the reasons behind them?
If I get it right, this is - not a defect - a request for API (incompatible?) change - a controversial issue - and we are after feature freeze If the above is correct, we shouldn't do it for Promo D. DevRev: you should remove API_REVIEW_FAST keyword, this issue failed the fasttrack I believe. Need full review
I guess Jan is failing to provide enough info and justification in reasonable timeframe. We cannot keep this as a bug. Reassigning to myself and closing.
Not enough justification from submiter.
Your closing this causes mmatula to remove the @deprecation from JMIElementCookie.java. Intentional? Do we need to open a new bug for javacore, since AFAIK we do not want new APIs to use Node.Cookie?
1. Adding new interfaces that implement cookie is bad. 2. jglick objected against the impl of this issue 3. jpokorsky has not been able to provide missing information for 10 weeks 4. ttran pointed out that this is not non-contraversional issue and need real review I can deal with #2, #4, but if the submiter accepts wontfix (#3) I have no need except #1. I agree #1 is bad, but I do not feel it so strongly to make me solve #2 and #4.
Jardo, I cannot believe that you write this. What are you missing??? Node lookup does not work so I filed this defect describing what is not working and the usecase. Today it is an enhancement closed as wontfix. You also know about code that works around of this issue since there is filed another issue #48997 assigned to you. It is closed as wontfix too. Simply I do not accept this as wontfix but I cannot wait for the next release. So I have to work around.
What I am missing? Answer to my question from 22/07/2004: "Jan: Can you please enumerate the FilterNode subclasses that would like to use style #1 so we can take a look at their source code and try to understand the reasons behind them?"
It is org.netbeans.modules.java.ui.nodes.JavaSourceNodeFactory$CompatibleNode.
I see. If there is no other node causing you the problem, then do not override getCookie, but use ProxyLookup (original.getLookup(), oldElementNode.getLookup()) where oldElementNode is a node that delegates its getCookie to oldElement.getCookie.
I opened issue #49731 to track the JMIElementCookie question separately.
OK, I verified that ProxyLookup works for me. So what next? If you still consider this not so important to fix you should at least improve javadoc to clarify FilterNode.getCookie contract properly.
Agreed, Javadoc should clarify in which cases lookup and getCookie delegate to one another, depending on what you override.
cvs ci -m "#45361: Per jpokorsky and jglick requests I am improving javadoc a bit." src/org/openide/nodes/FilterNode.java Checking in src/org/openide/nodes/FilterNode.java; /cvs/openide/src/org/openide/nodes/FilterNode.java,v <-- FilterNode.java new revision: 1.97;