I have a use case that is not covered by the
current implementation of lookup and getCookie in
* have a Node's subclass that gets lookup in its
* have a FilterNode's subclass that overrides
* 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
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
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?"
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
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;