Bug 161286 - Define Services tab nodes in lazy way
Define Services tab nodes in lazy way
Status: RESOLVED FIXED
Product: platform
Classification: Unclassified
Component: Nodes
6.x
Other All
: P2 (vote)
: 6.x
Assigned To: Jaroslav Tulach
issues@platform
: API_REVIEW_FAST
Depends on: 163977 165223
Blocks: 159810
  Show dependency treegraph
 
Reported: 2009-03-26 16:45 UTC by Jaroslav Tulach
Modified: 2009-06-16 17:06 UTC (History)
2 users (show)

See Also:
Issue Type: DEFECT
:


Attachments
API and its use throughout the IDE (14.55 KB, patch)
2009-03-26 16:56 UTC, Jaroslav Tulach
Details | Diff
Patch with new classes in API as well as change of implementation (24.63 KB, patch)
2009-03-26 20:32 UTC, Jaroslav Tulach
Details | Diff
New patch with @annotations (93.62 KB, patch)
2009-03-31 15:51 UTC, Jaroslav Tulach
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jaroslav Tulach 2009-03-26 16:45:30 UTC
Currently, when user iterates through various windows visible after start, it can "activate" a lot of different 
functionality just be watching. Most visible example of this phenomena is "Services" tab. It few nodes, each coming 
from different module, each doing something useless for majority of users, yet just clicking and making "Services" tab 
visible loads all these nodes (and many of associated classes) into memory.

It is desirable to eliminate such lazy initialization and rather wait for the user to really choose which of available 
nodes he wants to use. This can be achieved by declarative registration of icon, name, and displayname and delaying 
loading of the rest of the node till the user expands it or invokes popup menu.
Comment 1 Jaroslav Tulach 2009-03-26 16:48:57 UTC
I should also mention that the primary motivation for fixing the problem is issue 158765 - e.g. ergonomic IDE needs a 
way to analyse registered nodes and providing icon&co. declaratively with be of great help.
Comment 2 Jaroslav Tulach 2009-03-26 16:56:23 UTC
Created attachment 78924 [details]
API and its use throughout the IDE
Comment 3 Jesse Glick 2009-03-26 17:34:01 UTC
There is no impl of LazyNode visible in the patch. I presume it delegates to original upon addNotify of children, or
getActions; and thereafter delegates all methods to original (e.g. getPasteTypes, ...).


[JG01] Do not make people write this stuff in their layer; define an annotation. For example, I would like to see

public class HudsonRootNode ... {
  ...
  @Node.Registration(location="UI/Services",
                     name="hudson",
                     displayName="org.netbeans.modules.hudson.ui.nodes.Bundle#LBL_HudsonNode",
                     icon="org/netbeans/modules/hudson/ui/resources/hudson.png",
                     position=488)
  public static Node getDefault() {...}
  ...
}

The processor would be trivial to implement.

But consider instead defining a more specialized annotation in an appropriate module specifically for the Services tab.
This would avoid the need to hardcode the UI/Runtime path and make code clearer. E.g.

  @ServicesNodeRegistration(name="hudson",
                            displayName="org.netbeans.modules.hudson.ui.nodes.Bundle#LBL_HudsonNode",
                            icon="org/netbeans/modules/hudson/ui/resources/hudson.png",
                            position=488)
  public static Node getDefault() {...}

Of course such an annotation would be less general, but we might want specialized annotations for other purposes anyway,
and I don't know of any other place in the IDE where this kind of registration of singleton nodes (as opposed to node
factories) would make sense. The NodeOp.factory method could stay where it is for possible future use - just the
annotation would be in a different module.

The only question is which module to put the annotation in. One possibility would be openide.windows. UI/Runtime/ is
defined by core.ide, the TopComponent (NbMainExplorer.MainTab.createEnvironmentTab) is implemented by o.n.core, and it
is registered in core.execution - i.e. a mess. Ideally the folder, TC, and annotation would all be defined in the same
module. A new module could be created for the purpose, or as the most appropriate-seeming existing module, core.ide
could be given a public API (with just the annotation) and the TC and its registration moved there. (AFAIK there is no
other use of NbMainExplorer and it could probably be killed.)


[JG02] Is there any purpose for having the public factory method? There is no situation where you would want to call
this method overload (you have to instantiate the original node just to pass it in!), so better to remove it.
Comment 4 Jaroslav Tulach 2009-03-26 20:32:45 UTC
Created attachment 78937 [details]
Patch with new classes in API as well as change of implementation
Comment 5 Jaroslav Tulach 2009-03-26 20:36:39 UTC
Re. JG01 - I happily provide annotation if there is sensible place to put it to. Creating new module however seems 
like overkill to me. Rather than that I prefer to keep just layer based registration.

Re. JG02 - The factory method is the only place to put the documentation to (in absence of the annotation). It used to 
be good habit to have both methods one visible in JavaDoc with all docs including layer one, one callable from layer. 
If JG01 is resolved, I am OK with deleting the public method.
Comment 6 Jesse Glick 2009-03-26 20:56:19 UTC
JG01 - I suggest core.ide. Would that work? I could if necessary help with prep work - moving the current Services tab
infra to that module - which would be desirable independently of this issue.


JG02 - barring JG01, info could be in class Javadoc.


[JG03] LazyNode should behave sensibly if optional attributes are missing. At least it seems that shortDescription is
optional (e.g. HudsonRootNode does not set it currently). Not sure if setShortDescription(null) is legitimate.


[JG04] switchToOriginal should handle the case that map.get("original") returns null, e.g. after a CNFE is thrown by XMLFS.


[JG05] Careful with JUnit 4, it probably will not work correctly with NbTestCase, and we need to use NbTestCase so we
can add @RandomlyFails etc.
Comment 7 Jaroslav Tulach 2009-03-27 06:01:32 UTC
Re. core.ide - usually we want API to be in autoload module (although in case of compile time processor this may not 
be that important). core.ide can't be autoload. Moreover core.ide is in ideXY cluster, while the functionality is in 
platform. 
Comment 8 Jesse Glick 2009-03-27 13:11:32 UTC
I don't think the module needs to be autoload. That is certainly appropriate for an API which is purely a bunch of Java
classes you can use. But this deals with a major added GUI element, which you need to decide to enable.

As to the cluster - NodeOp is indeed in the platform cluster, and that is fine. The Services tab is however defined,
intentionally, in the ide cluster. And all potential users of the registration would also be in the ide cluster or
higher; this GUI element is intended for use in an IDE. Putting the registration in a module in the platform cluster
would be incorrect.
Comment 9 Jesse Glick 2009-03-27 19:46:56 UTC
Preparation: Services tab moved to core.ide. core-main #99eb1a5de1b6
Comment 10 Quality Engineering 2009-03-28 20:39:43 UTC
Integrated into 'main-golden', will be available in build *200903281400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/99eb1a5de1b6
User: Jesse Glick <jglick@netbeans.org>
Log: #161286 prep: move Services tab into core.ide module.
Comment 11 Jaroslav Tulach 2009-03-31 15:51:12 UTC
Created attachment 79153 [details]
New patch with @annotations
Comment 12 Jaroslav Tulach 2009-03-31 17:46:22 UTC
I'd like to integrate the change soon.
Comment 13 Jesse Glick 2009-03-31 19:02:42 UTC
JG02 still unaddressed in attached patch, JG03-05 as well I think though less important.


[JG06] Modules using the new annotation should have a <run-dependency>; they really expect the Services tab to be there.


[JG07] Replace

if (reg.position() != Integer.MAX_VALUE) {
    f.intvalue("position", reg.position());
}

with

f.position(reg.position());


[JG08] Do not use both instanceFile here, as that indicates that you want to bind the instance to the annotated element,
which is not the case; use only instanceAttribute. (An RFE for a method to create a nicely named file in a given folder
based on the element name would be accepted.)


[JG09] displayName in db.explorer.node.RootNode is malformed; use . not / as sep.


[JG10] The tab is called "Services", not "Service", so suggest "ServicesTabNodeRegistration".
                                                                       ^

[JG11] core.ide should dep on new openide.nodes.
Comment 14 Quality Engineering 2009-03-31 19:53:34 UTC
Integrated into 'main-golden', will be available in build *200903311400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/491c989f09c8
User: Jesse Glick <jglick@netbeans.org>
Log: #161286 prep addendum: set icon for Services tab.
Comment 15 Jaroslav Tulach 2009-04-01 15:52:34 UTC
core-main#fcbe7ad3185b

Re. JG03 - imho the node behaves sensibly. setSD(null) seems to be allowed. Report a bug if you find otherwise.
Re. JG04 - the code handles such situation by reporting an exception and continuing. I am not sure what else you want 
it to do.
Re. JG05 - I do not plan to have unstable tests in this area.
Re. JG08 - You want me to not use an API that works and report you an issue to create another API that will work!? 
Sorry, I don't get the point. Feel free to do it yourself.

Otherwise I think I addressed your comments.
Comment 16 Quality Engineering 2009-04-02 07:33:05 UTC
Integrated into 'main-golden', will be available in build *200904020200* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/757b11c277c1
User: Jaroslav Tulach <jtulach@netbeans.org>
Log: #161286: Addressing JG06 and JG11. E.g. fixing runtime dependencies of those who use the new annotation
Comment 17 Jaroslav Tulach 2009-04-02 08:26:02 UTC
core-main#0109f9b6fc77 - fixing broken javadoc links

btw. Jesse: now when all modules using @ServicesTabNodeRegistration have runtime dependency on core.ide, the core.ide 
itself shall be autoload, shall it not?
Comment 18 Jesse Glick 2009-04-02 15:31:33 UTC
core.ide defines a couple of other assorted UI elements. No strong opinion.
Comment 19 Quality Engineering 2009-04-03 20:05:21 UTC
Integrated into 'main-golden', will be available in build *200904031400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/d5707b4a8011
User: Jesse Glick <jglick@netbeans.org>
Log: Dependencies added in 757b11c277c1 (for #161286) were invalid acc. to schema.
Correcting, and also making sure this mistake does not happen again.


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