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.

Bug 59129 - Prevent the interfaces in org.netbeans.api.debugger.jpda package from being implementable.
Summary: Prevent the interfaces in org.netbeans.api.debugger.jpda package from being i...
Status: CLOSED FIXED
Alias: None
Product: debugger
Classification: Unclassified
Component: Java (show other bugs)
Version: 5.x
Hardware: PC All
: P2 blocker (vote)
Assignee: Martin Entlicher
URL:
Keywords: API, API_REVIEW
Depends on:
Blocks: 59072
  Show dependency tree
 
Reported: 2005-05-20 16:21 UTC by Martin Entlicher
Modified: 2010-04-29 09:22 UTC (History)
1 user (show)

See Also:
Issue Type: TASK
Exception Reporter:


Attachments
The ugly javadoc that is created when a package private interface is added as a basis of JPDAThread. (32.13 KB, text/html)
2005-05-20 17:42 UTC, Martin Entlicher
Details
The requested API change. (5.63 KB, patch)
2005-08-05 10:58 UTC, Martin Entlicher
Details | Diff
The new JPDA Javadoc (281.56 KB, application/x-compressed)
2005-08-05 11:00 UTC, Martin Entlicher
Details
The complete diff of this API change including all affected modules. (45.51 KB, patch)
2005-08-12 10:11 UTC, Martin Entlicher
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Entlicher 2005-05-20 16:21:44 UTC
We should modify the interfaces in org.netbeans.api.debugger.jpda package so
that they can not be implemented from outside.

The reason is, that from time to time we need to add methods to these interfaces
(see issue #59072, or add evaluate() method to CallStackFrame, which would allow
us to remove the ugly altCSF field from JPDADebuggerImpl).
Also JDI is changing from time to time, they can add new methods to their
interfaces at any time.
Comment 1 Martin Entlicher 2005-05-20 16:47:46 UTC
I suggest to document that the interfaces must not be implemented in client code.
If it's necessary to enforce this, we can create a new private interface, that
would be a basis of all existing interfaces.
This is my prefered solution, since it will not affect users of the API.

Other solution can be to change the interfaces to abstract classes, which will
have an abstract package private method, or final classes, or create static
delegate methods (like JPDA.isSuspended(JPDAThread)).

Comment 2 Martin Entlicher 2005-05-20 17:41:36 UTC
Well, to create a private basis of the interfaces is not nice at all. It adds an
ugly and confusing clutter to the javadoc.

IMHO it would be best if the description in the javadoc would be sufficient.

It's also problematic to change the interfaces into abstract classes, since the
interfaces form a hierarchy (e.g. Variable - ObjectVariable - LocalVariable),
which is not possible using abstract classes. We would have to rewrite the API,
but this would be incompatible also from the API point of view.
Comment 3 Martin Entlicher 2005-05-20 17:42:52 UTC
Created attachment 22231 [details]
The ugly javadoc that is created when a package private interface is added as a basis of JPDAThread.
Comment 4 Jesse Glick 2005-05-20 18:38:49 UTC
It is forbidden for a publically visible class or interface to reference a
private class or interface in any part of its signature.

I think a note in the Javadoc that the interface may only be implemented by
certain parts of the code is sufficient. We already use this style in other APIs.
Comment 5 Martin Entlicher 2005-05-23 13:27:09 UTC
Cool, thanks Jesse. Therefore I propose to add this into the javadoc of every
interface in org.netbeans.api.debugger.jpda package:

"This interface may evolve in the future. Do not try to implement it. Objects
implementing this interface are produced by debugger core module."

Please adjust the text as appropriate. Any comments from others?
Comment 6 Jaroslav Tulach 2005-05-23 13:42:31 UTC
JG: "We already use this style in other APIs" 
 
Well Jesse, I cannot realize where we use final interfaces. I know that 
org.nb.jmi does that, but that is ok as we selected org.nb.jmi namespace to 
indicate that the evolution of the api will be different. Maybe we did this 
once in window system API, but otherwise I do not know about any other stable 
API that would use this poor style. 
 
Imho creating new interface JPDAThread2 is the most standard solution. That 
one has imho chance to be accepted in API_REVIEW_FAST. All others (including 
comment in javadoc) are in fact incompatible and need to go thru standard 
review. The goal is to make sure that all users of the api agree with it being 
changed incompatibly and record that decision. 
 
 
Side note: changing interfaces to abstract classes is incompatible change for 
callers. Methods are called as #invokeinterface vs. #invokevirtual, so such 
change is going to break binary compatibility. 
 
 
 
Comment 7 Jesse Glick 2005-05-23 16:12:27 UTC
Re. interfaces with restricted implementors: e.g. ProjectState.

Anyway it is true that if you did not *initially* specify on these interfaces
who was permitted to implement them, you cannot add that restriction now
(without breaking compatibility, requiring a new major release version as a
signal). I agree that making a new interface and deprecating the old one is the
only compatible approach in this case.
Comment 8 Jan Jancura 2005-05-24 09:35:01 UTC
1) JPDA Debugger APIs are separated to API / SPI, so it is clear what is
designed to be implemented by client, even without specific notification in each
 class.

2) There is no reason to extend interfaces from this API. You can not use them
in any reasonable way. 

Thats why I am suggesting to:
- add some notification in JavaDoc.
- add this new method to existing interface
- if you think that its "incompatible" change we can update version number,
notify on nbdev etc, do full review.

But I do not see any reason for polluting our apis with some xxx2 things.
Comment 9 Jan Jancura 2005-05-24 09:44:19 UTC
One more question. Do we have some business reason why our API related processes
are more strict than processes used by JDK team?

JPDA debugger is based on JDI interface. It contains many interfaces. These
interfaces are incompatibly changed in "each" new version of JDK. JDI has
several implementations, and many clients.

We should have some strong reason to be more strict with API changes than JDK
team...
Comment 10 Jaroslav Tulach 2005-05-24 14:45:06 UTC
The only bussiness reason is that NetBeans debugger interfaces are clasified 
as stable while JDI is not. Comparing these two cases is like comparing apples 
and oranges. If you want to compare NetBeans debugger interface evolution with 
some other one, select something that is stable. For example java.util.*. 
Comment 11 Jan Jancura 2005-05-24 15:33:37 UTC
As far as I know our DevRev procedures, java.util.* has a small chance to be
approved. For example:
1) this api is not divided to API <> SPI, this is BIG problem!

2) Classes like Arrays, Collections, Date, shoud be final - TCR!

3) They are adding new methods to (API - SPI) classes It can be incompatible
change according our rules:
http://openide.netbeans.org/versioning-policy.html#2

Anyway, I would suggest to do this incompatible change (+update version number),
and add some notice to JavaDocs. It allows to do more changes of this API in the
future. Solution with Threads2 interface does not fix problem with future
imporvements. This solution is fully compatible with our processes too.

Comment 12 Jesse Glick 2005-05-24 17:12:47 UTC
Off-topic: I would indeed complain about the design of many Java platform API
classes if they were submitted as new proposals; there are many mistakes in
java.** and javax.** classes that it's too late to fix now. But this has nothing
to do with what we are reviewing.

Personally I am +0 on permitting the new method to be added, provided that

1. There is no plausible reason for anyone else to impl this interface (e.g. for
purposes of proxying or composition), nor are there any known instances of third
parties implementing these interfaces.

2. The design of the API and its existing documentation do not imply that the
interface could or should be implemented.

3. The Javadoc (for each such interface individually) be edited to expressly
forbid impl of the interface by anyone except the the debugger module.

4. Perhaps also mark an incompatible rev on the major version.

But let's see what other reviewers besides myself and Yarda think about it.
Comment 13 Miloslav Metelka 2005-05-24 17:44:12 UTC
I agree with Jesse in that if there are no known clients of that interface then
it's not necessary to create extra xxx2 interface. I have no strong opinion
whether major module version should be increased or not.

Awyway this particular case reminds me that all the NB reviewers should first
sit and agree on (and document) the proper solutions for the particular types of
API changes (like e.g. this one) and then finally say a consistent answer to the
review submitter.
Comment 14 Tomas Pavek 2005-05-24 18:22:04 UTC
I agree with Jesse's summary as for what should be done from the technical point
of view (with issue 59072). Adding new method to the interfaces is IMO
reasonable under the mentioned conditions.

From the process point of view, Yarda is right that according to our rules this
can be considered incompatible change in official API, thus controversial and
need to go through standard review. We should do the review probably also
because we should discuss this case in general, whether to make it a precedent -
a possible way of extending API that will not be considered incompatible in
future and will be able to go through fast review (without heated discussions).
If we agree on that.
Comment 15 Jaroslav Tulach 2005-06-03 05:15:52 UTC
The #1 thing we need is consistency across all NetBeans stable 
(http://openide.netbeans.org/tutorial/api-design.html#category-stable) and official 
APIs. The evolution of platform, project, java, debugger, and all other APIs declared 
as stable or official has to be the same. That is what our users (module writers) 
expect.  
 
Btw. We do not want them to differentiate individual apis (for example platform and 
debugger ones). We want them to use "NetBeans 4.x APIs" (like 
http://www.netbeans.org/download/4_1/javadoc/). They all shall follow the same 
rules of evolution. 
Comment 16 Martin Entlicher 2005-08-04 17:22:59 UTC
The intent is to have all interfaces in org.netbeans.api.debugger.jpda package
categorized as "not-to-implement" by client code.
There's one exception - JPDABreakpointListener. This is expected to be
implemented by clients I guess. So we'll create a package
org.netbeans.api.debugger.jpda.event and move it there (likely together with
JPDABreakpointEvent).

I'll prepare the full change by tomorrow and attach it here...
Comment 17 Martin Entlicher 2005-08-05 10:58:49 UTC
Created attachment 23523 [details]
The requested API change.
Comment 18 Martin Entlicher 2005-08-05 11:00:35 UTC
Created attachment 23524 [details]
The new JPDA Javadoc
Comment 19 Jaroslav Tulach 2005-08-08 13:25:28 UTC
Can the "do not subclass" warning be added to javadoc of each class and 
interface in the org.netbeans.api.debugger.jpda package? 
 
Comment 20 Jaroslav Tulach 2005-08-08 13:29:41 UTC
y02 Now, when the binary compatiblity is broken, would it not be better to fix 
some of the design and turn the interfaces into classes? For example 
SmartSteppingFilter has no reason to be interface, imho. 
 
y03 do not forget to increase major version of the module version. 
 
y04 any problem with making jpdathreadgroup, jpdathread, callstackframe 
classes? Any paying need to reuse multiple inheritance? 
Comment 21 Martin Entlicher 2005-08-11 14:43:16 UTC
y02)
Well, I've tried to convert the SmartSteppingFilter into a final class, but I've
run into a bunch of problems. The implementation of the SmartSteppingFilter is
available via lookup. We can not get it into the lookup if it would not have a
public constructor. But we do not want to provide the ability to construct the
instances of this class... Another problem is, that SmartSteppingFilterImpl have
one extra public method, which is not declared in SmartSteppingFilter and is
used in JPDA module. We would have to have SmartSteppingFilterImpl, which does
not implement SmartSteppingFilter (can not if it's final), but delegates to it
somehow... The usages would have to be verified... there is more problems with
that then advantages IMHO.
So I suggest to leave SmartSteppingFilter as it is.

y03)
it's done in the manifest.mf: org.netbeans.api.debugger.jpda/2

y04)
I would rather leave them as they are... I do not want to do many changes in
that area...
Comment 22 Martin Entlicher 2005-08-12 10:11:49 UTC
Created attachment 23751 [details]
The complete diff of this API change including all affected modules.
Comment 23 Martin Entlicher 2005-08-12 10:13:05 UTC
Thanks for the review, I'm going to commit that change later today.
Comment 24 Martin Entlicher 2005-08-12 13:50:05 UTC
Committed into trunk:

/cvs/tomcatint/tomcat5/nbproject/project.xml,v  <--  project.xml
new revision: 1.12; previous revision: 1.11

/cvs/j2ee/debug/nbproject/project.xml,v  <--  project.xml
new revision: 1.5; previous revision: 1.4

/cvs/j2ee/earproject/nbproject/project.xml,v  <--  project.xml
new revision: 1.11; previous revision: 1.10

/cvs/j2ee/ejbjarproject/nbproject/project.xml,v  <--  project.xml
new revision: 1.19; previous revision: 1.18

/cvs/j2eeserver/nbproject/project.xml,v  <--  project.xml
new revision: 1.10; previous revision: 1.9

/cvs/debuggerjpda/ant/nbproject/project.xml,v  <--  project.xml
new revision: 1.19; previous revision: 1.18

/cvs/debuggerjpda/api/apichanges.xml,v  <--  apichanges.xml
new revision: 1.10; previous revision: 1.9

/cvs/debuggerjpda/api/manifest.mf,v  <--  manifest.mf
new revision: 1.11; previous revision: 1.10

/cvs/debuggerjpda/api/nbproject/project.xml,v  <--  project.xml
new revision: 1.8; previous revision: 1.7

/cvs/debuggerjpda/api/src/org/netbeans/api/debugger/jpda/CallStackFrame.java,v 
<--  CallStackFrame.java
new revision: 1.6; previous revision: 1.5

/cvs/debuggerjpda/api/src/org/netbeans/api/debugger/jpda/Field.java,v  <-- 
Field.java
new revision: 1.5; previous revision: 1.4

/cvs/debuggerjpda/api/src/org/netbeans/api/debugger/jpda/JPDABreakpoint.java,v 
<--  JPDABreakpoint.java
new revision: 1.9; previous revision: 1.8

/cvs/debuggerjpda/api/src/org/netbeans/api/debugger/jpda/JPDABreakpointEvent.java,v
 <--  JPDABreakpointEvent.java
new revision: delete; previous revision: 1.4

/cvs/debuggerjpda/api/src/org/netbeans/api/debugger/jpda/JPDABreakpointListener.java,v
 <--  JPDABreakpointListener.java
new revision: delete; previous revision: 1.2

/cvs/debuggerjpda/api/src/org/netbeans/api/debugger/jpda/JPDADebugger.java,v 
<--  JPDADebugger.java
new revision: 1.18; previous revision: 1.17

/cvs/debuggerjpda/api/src/org/netbeans/api/debugger/jpda/JPDAThread.java,v  <--
 JPDAThread.java
new revision: 1.6; previous revision: 1.5

/cvs/debuggerjpda/api/src/org/netbeans/api/debugger/jpda/JPDAThreadGroup.java,v
 <--  JPDAThreadGroup.java
new revision: 1.3; previous revision: 1.2

/cvs/debuggerjpda/api/src/org/netbeans/api/debugger/jpda/JPDAWatch.java,v  <-- 
JPDAWatch.java
new revision: 1.7; previous revision: 1.6

/cvs/debuggerjpda/api/src/org/netbeans/api/debugger/jpda/LocalVariable.java,v 
<--  LocalVariable.java
new revision: 1.5; previous revision: 1.4

/cvs/debuggerjpda/api/src/org/netbeans/api/debugger/jpda/ObjectVariable.java,v 
<--  ObjectVariable.java
new revision: 1.7; previous revision: 1.6

/cvs/debuggerjpda/api/src/org/netbeans/api/debugger/jpda/SmartSteppingFilter.java,v
 <--  SmartSteppingFilter.java
new revision: 1.4; previous revision: 1.3

/cvs/debuggerjpda/api/src/org/netbeans/api/debugger/jpda/Super.java,v  <-- 
Super.java
new revision: 1.3; previous revision: 1.2

/cvs/debuggerjpda/api/src/org/netbeans/api/debugger/jpda/This.java,v  <--  This.java
new revision: 1.3; previous revision: 1.2

/cvs/debuggerjpda/api/src/org/netbeans/api/debugger/jpda/Variable.java,v  <-- 
Variable.java
new revision: 1.3; previous revision: 1.2

/cvs/debuggerjpda/api/src/org/netbeans/api/debugger/jpda/package.html,v  <-- 
package.html
new revision: 1.3; previous revision: 1.2

RCS file:
/cvs/debuggerjpda/api/src/org/netbeans/api/debugger/jpda/event/JPDABreakpointEvent.java,v
done

/cvs/debuggerjpda/api/src/org/netbeans/api/debugger/jpda/event/JPDABreakpointEvent.java,v
 <--  JPDABreakpointEvent.java
initial revision: 1.1

RCS file:
/cvs/debuggerjpda/api/src/org/netbeans/api/debugger/jpda/event/JPDABreakpointListener.java,v

/cvs/debuggerjpda/api/src/org/netbeans/api/debugger/jpda/event/JPDABreakpointListener.java,v
 <--  JPDABreakpointListener.java
initial revision: 1.1

/cvs/debuggerjpda/nbproject/project.xml,v  <--  project.xml
new revision: 1.13; previous revision: 1.12

/cvs/debuggerjpda/src/org/netbeans/modules/debugger/jpda/JPDADebuggerImpl.java,v
 <--  JPDADebuggerImpl.java
new revision: 1.79; previous revision: 1.78

/cvs/debuggerjpda/src/org/netbeans/modules/debugger/jpda/breakpoints/BreakpointImpl.java,v
 <--  BreakpointImpl.java
new revision: 1.21; previous revision: 1.20

/cvs/debuggerjpda/test/unit/src/org/netbeans/api/debugger/jpda/BreakpointResumeTest.java,v
 <--  BreakpointResumeTest.java
new revision: 1.5; previous revision: 1.4

/cvs/debuggerjpda/test/unit/src/org/netbeans/api/debugger/jpda/ClassBreakpointTest.java,v
 <--  ClassBreakpointTest.java
new revision: 1.5; previous revision: 1.4

/cvs/debuggerjpda/test/unit/src/org/netbeans/api/debugger/jpda/ExceptionBreakpointTest.java,v
 <--  ExceptionBreakpointTest.java
new revision: 1.5; previous revision: 1.4

/cvs/debuggerjpda/test/unit/src/org/netbeans/api/debugger/jpda/FieldBreakpointTest.java,v
 <--  FieldBreakpointTest.java
new revision: 1.9; previous revision: 1.8

/cvs/debuggerjpda/test/unit/src/org/netbeans/api/debugger/jpda/JspLineBreakpointTest.java,v
 <--  JspLineBreakpointTest.java
new revision: 1.3; previous revision: 1.2

/cvs/debuggerjpda/test/unit/src/org/netbeans/api/debugger/jpda/LineBreakpointTest.java,v
 <--  LineBreakpointTest.java
new revision: 1.9; previous revision: 1.8

/cvs/debuggerjpda/test/unit/src/org/netbeans/api/debugger/jpda/MethodBreakpointTest.java,v
 <--  MethodBreakpointTest.java
new revision: 1.8; previous revision: 1.7

/cvs/debuggerjpda/test/unit/src/org/netbeans/api/debugger/jpda/ThreadBreakpointTest.java,v
 <--  ThreadBreakpointTest.java
new revision: 1.6; previous revision: 1.5

/cvs/debuggerjpda/ui/nbproject/project.xml,v  <--  project.xml
new revision: 1.10; previous revision: 1.9

/cvs/debuggerjpda/ui/src/org/netbeans/modules/debugger/jpda/ui/BreakpointOutput.java,v
 <--  BreakpointOutput.java
new revision: 1.15; previous revision: 1.14

/cvs/debuggerjpda/ui/src/org/netbeans/modules/debugger/jpda/ui/actions/RunIntoMethodActionProvider.java,v
 <--  RunIntoMethodActionProvider.java
new revision: 1.11; previous revision: 1.10

/cvs/web/jspdebug/nbproject/project.xml,v  <--  project.xml
new revision: 1.12; previous revision: 1.11

/cvs/serverplugins/sun/appsrv81/manifest.mf,v  <--  manifest.mf
new revision: 1.6; previous revision: 1.5

/cvs/serverplugins/sun/appsrv81/nbproject/project.xml,v  <--  project.xml
new revision: 1.5; previous revision: 1.4

/cvs/web/project/nbproject/project.xml,v  <--  project.xml
new revision: 1.32; previous revision: 1.31

/cvs/ide/golden/cluster-deps.txt,v  <--  cluster-deps.txt
new revision: 1.27; previous revision: 1.26

/cvs/ide/golden/deps.txt,v  <--  deps.txt
new revision: 1.137; previous revision: 1.136

/cvs/ide/golden/modules.txt,v  <--  modules.txt
new revision: 1.35; previous revision: 1.34

/cvs/ide/golden/public-packages.txt,v  <--  public-packages.txt
new revision: 1.29; previous revision: 1.28
Comment 25 Quality Engineering 2010-04-29 09:22:44 UTC
Verified ... and Closing all issues resolved into NetBeans 6.7 and earlier.