Bug 45361 - FilterNode's lookup&cookies not always work
FilterNode's lookup&cookies not always work
Status: RESOLVED WONTFIX
Product: platform
Classification: Unclassified
Component: Nodes
4.x
All All
: P2 (vote)
: 6.x
Assigned To: Jaroslav Tulach
issues@platform
: API, API_REVIEW_FAST
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-06-22 14:33 UTC by Jan Pokorsky
Modified: 2008-12-22 20:50 UTC (History)
2 users (show)

See Also:
Issue Type: ENHANCEMENT
:


Attachments
After 8hours of continuous mangling of the implementation I think I now satisfy all the tests (12.14 KB, patch)
2004-07-03 00:46 UTC, Jaroslav Tulach
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Pokorsky 2004-06-22 14:33:29 UTC
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.
Comment 1 Jaroslav Tulach 2004-07-03 00:46:06 UTC
Created attachment 16129 [details]
After 8hours of continuous mangling of the implementation I think I now satisfy all the tests
Comment 2 Jaroslav Tulach 2004-07-03 00:54:29 UTC
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.
Comment 3 Jesse Glick 2004-07-04 05:49:13 UTC
-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.
Comment 4 _ ttran 2004-07-22 10:52:02 UTC
Yarda, it seems this request didn't pass devrev.  Will you close it?
Comment 5 Jan Pokorsky 2004-07-22 12:33:47 UTC
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.
Comment 6 Jaroslav Tulach 2004-07-22 15:16:04 UTC
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?
Comment 7 _ ttran 2004-07-25 21:31:21 UTC
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
Comment 8 Jaroslav Tulach 2004-07-26 09:38:40 UTC
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.
Comment 9 Jaroslav Tulach 2004-07-26 09:39:06 UTC
Not enough justification from submiter.
Comment 10 Jesse Glick 2004-09-27 16:52:25 UTC
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?
Comment 11 Jaroslav Tulach 2004-09-30 09:20:00 UTC
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.

Comment 12 Jan Pokorsky 2004-09-30 10:12:53 UTC
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.
Comment 13 Jaroslav Tulach 2004-09-30 10:22:51 UTC
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?" 
Comment 14 Jan Pokorsky 2004-09-30 10:39:26 UTC
It is
org.netbeans.modules.java.ui.nodes.JavaSourceNodeFactory$CompatibleNode. 
Comment 15 Jaroslav Tulach 2004-09-30 11:00:16 UTC
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.
Comment 16 Jesse Glick 2004-09-30 13:01:16 UTC
I opened issue #49731 to track the JMIElementCookie question separately.
Comment 17 Jan Pokorsky 2004-09-30 13:48:03 UTC
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.
Comment 18 Jesse Glick 2004-09-30 14:35:38 UTC
Agreed, Javadoc should clarify in which cases lookup and getCookie
delegate to one another, depending on what you override.
Comment 19 Jaroslav Tulach 2004-10-04 17:28:08 UTC
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;


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo