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 44847 - WebModule API review
Summary: WebModule API review
Status: CLOSED FIXED
Alias: None
Product: javaee
Classification: Unclassified
Component: Code (show other bugs)
Version: 4.x
Hardware: All All
: P2 blocker (vote)
Assignee: Pavel Buzek
URL:
Keywords: API_REVIEW_FAST
Depends on:
Blocks:
 
Reported: 2004-06-15 02:06 UTC by Pavel Buzek
Modified: 2006-03-24 13:00 UTC (History)
0 users

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
Javadoc documentation and architecture document (80.83 KB, application/octet-stream)
2004-06-15 02:08 UTC, Pavel Buzek
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Buzek 2004-06-15 02:06:37 UTC
Please review the new API for web modules
introduced in promoD. I will attach javadoc with
arch answers. I suggest to use fast track as this
API is fairly small and simple, but I am ok with
full review if you think that is more appropriate.
Thanks.
Comment 1 Pavel Buzek 2004-06-15 02:08:16 UTC
Created attachment 15691 [details]
Javadoc documentation and architecture document
Comment 2 Jaroslav Tulach 2004-06-15 07:39:03 UTC
Please replace or enhance "Use classpath API to obtain classpath for
the document base." with a code snippet.

Do not expose Node.Cookie in API. The only place it is currently still
necessary is DataObject, but Node already supports Lookup. I know that
the usage is connected to data objects, but maybe you could replace
this with the query pattern used in projects.

According to famous Effective Java book, the WebProjectConstants is
poor pattern.

SPI looks perfect to me.

Please make sure the api appears on 
http://www.netbeans.org/download/dev/javadoc/

Comment 3 Pavel Buzek 2004-06-15 16:39:52 UTC
I copied the pattern from JavaProjectConstants that went through this
kind of review and is unchanged. 
If you have a particular suggestion please explain it to me.
Comment 4 Jesse Glick 2004-06-15 17:18:01 UTC
Actually JavaProjectConstants was added after the main buildsys
inception review. I will change it to be a class with a private
constructor.
Comment 5 Vince Kraemer 2004-06-15 18:12:57 UTC
This set of comments is related to the API.

Interface URLCookie: 

Since the getURL() method doesn't return a full URL, it should not be
named getURL.  I would recommend using a name that parallels the names
used in the java.net.URL.  Based on your description, getFile() is
probably the closest match.

Interface WebProjectConstants:

This should be a class NOT an interface. If JavaProjectsConstants has
this bug, you should not follow it.  Please read Item 17 in Effective
Java.  It is on Page 89 of the copy that I have.

Class WebModule:

It seems like this object should implement
javax.enterprise.deploy.model.DeployableObject.  Why doesn't it?

If you implement equals(Object), you should probably also implement
hashCode(). Item 8 in Effective Java.

I think the api is missing a couple getters:

getMetaInf() - in case some module will need to put data in there.
getClasses(), getLib(), getTld() - in case we want to move these from
    underneath the FileObject that is the return value of getWebInf().
 This would allow us to change the web module projects physical layout
without breaking its clients....
Comment 6 Pavel Buzek 2004-06-23 16:48:02 UTC
URLCookie - I will change this to query pattern, implement it in
project and store the settings in private.propeties instead of
nbattrs. I agree getURL() is not a good name. I am leaning towards
something even less general then getFile() - what about
getRequestPath()? Any other suggestions?

> It seems like this object should implement
> javax.enterprise.deploy.model.DeployableObject.  Why doesn't it?
1. Why should it? You can ask for both interfaces separately.
2. sometime it is not needed - freeform project will use Ant tasks for
deployment so it does not need j2eeserver api implemented on it

I would prefer not to mix these 2 things.
This reminds me that when we get to j2eeserver review we may want to
change the J2eeModuleProvider to query pattern, but that's a separate
issue.

getLib, getClasses,getTags - what are they needed for? Of course
clients should not use getFileObject ("WEB-INF/classes") but they are
not used in existing code, AFAIK. Provide use cases if you want them
added. (I assume this is in build area, right?)

the rest I will fix (WebModule.hasCode, WebProjectConstants being
class, doc about using classpath)
Comment 7 Jaroslav Tulach 2004-11-22 07:00:20 UTC
What is this? Forgotten review? Was it integrated? Is the javadoc
published? At least a fake of tests written? Please update the status
here.
Comment 8 Pavel Buzek 2004-11-22 19:21:44 UTC
Oops. Looks like you are right.

I fixed the poor pattern in WebProjectConstants and added code sample
for classpath in WebModule. I have not fixed the URLCookie. It can be
fixed in 4.1 (this is a friend API) or in 4.0 if you insist on it.
Obviously I'd prefer to have a P2 for 4.1 and close this issue.

There are actually some fake tests :-) Are they too much fake?

The javadoc is not published but that can be fixed very easily if you
think it should be published (even when I have not fixed 1 API issue).

cvs diff -u build.properties (in directory E:\nb_all\nbbuild\)
Index: build.properties
===================================================================
RCS file: /cvs/nbbuild/build.properties,v
retrieving revision 1.219
diff -u -r1.219 build.properties
--- build.properties	12 Nov 2004 22:09:47 -0000	1.219
+++ build.properties	22 Nov 2004 19:18:47 -0000
@@ -364,6 +364,7 @@
         debuggerjpda/api, \
         ant/project, \
         j2eeserver, \
+        web/webapi, \
         java/api, \
         java/javacore, \
         java/javamodel, \

Let me know what I should do.
Comment 9 Jaroslav Tulach 2004-11-23 16:52:47 UTC
Publish javadoc for 4.1 (release40 contains stable apis only anyway).
Otherwise I am fine with the rest for 4.1 (I think).
Comment 10 Pavel Buzek 2004-12-06 10:40:27 UTC
the right term for request parameters is defined in
j2ee/platform/javahelp/org/netbeans/modules/j2ee/platform/docs/web/glossary/URL_query_string.html
- URLQueryString
Comment 11 Pavel Buzek 2005-01-27 20:09:26 UTC
URLCookie removed. Closing as fixed.
Comment 12 zikmund 2005-03-31 12:25:41 UTC
v