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 196466 - Java EE server technologies APIs
Summary: Java EE server technologies APIs
Status: NEW
Alias: None
Product: serverplugins
Classification: Unclassified
Component: Infrastructure (show other bugs)
Version: 7.0
Hardware: PC All
: P3 normal (vote)
Assignee: Sergey Petrov
URL:
Keywords: API, PLAN
Depends on: 196127 196468
Blocks: 197971
  Show dependency tree
 
Reported: 2011-03-09 08:18 UTC by Denis Anisimov
Modified: 2011-06-17 14:39 UTC (History)
7 users (show)

See Also:
Issue Type: TASK
Exception Reporter:


Attachments
sketch of module (3.41 KB, application/x-gzip)
2011-05-17 00:13 UTC, David Konecny
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Denis Anisimov 2011-03-09 08:18:21 UTC
This is an umbrella bug for issue originally filed as #196127 .

Currently Jersey libraries are added into the project via several different ways:
- ServerLibrary(ies) API is used for WL ( it is implemented on the plugin side ).
See issue #195542.
- restlib_gfv3 library is used for GF server. There is an issue with GF3.1 with 
the current approach because of difference in jar files in GF3.0 and GF3.1
- NB bundled Jersey is used in case of absence Jersey bundled libraries .

This task is about feature related libraries usage unification.
Some way to get feature related libraries are needed:
f.e. return libraries via special tag from the server plugin ( it could be based
on the ServerLibrary concept ).
Comment 1 Denis Anisimov 2011-03-09 08:42:39 UTC
See the wiki page http://wiki.netbeans.org/TaggedLibraries.
Comment 2 Petr Hejl 2011-04-19 14:29:42 UTC
Please provide more detail on usecases this should solve. Why the WSStackImplementation can't be used for such abstraction? IIRC it was designed to solve similar things. I have a bad feeling we might get something similar to infamous getToolClasspath(String id) this time with getServerLibrary(String tag).

In order to avoid that write down the broken parts of your code base. At this stage we should focus on what we are actually trying to solve rather than choosing a solution.

Thanks,
P.
Comment 3 Denis Anisimov 2011-04-20 07:15:08 UTC
There are at least two usecases which are the problems for REST:
1) There is detailed description of the problem in two issues #196127 and #196468.
A short summary : there are two different ways to find Jersey server bundled libraries.
The first one is using ServerLibrary API which works only for WL for now.
The second is custom logic to find jar files in the server installation base.
The last one works for GF and it has several problems. One of which is hacky implementation ( this is the problem of REST module itself ) and ugly using of restlib_gfv3 NB library.
It would be preferable to have a common way to access to server bundled libraries which belongs to some feature ( identified by tag ).
2) The common problem with libraries for some feature .
Let's consider the REST configuration dialog ( see the issue #190853 and #195542).
Currently there are a list of ServerLibraries which are added into the project via deployment. 
The logic that identify the list libraries is hardcoded into the REST configuration dialog. It is based on the library specification/implementation title/version. But this information could be changed in the various AS releases.
( war/jar file names could be changed/added along with spec/impl titles ).
Such change ( list of jar files ) happens f.e. with migration from GF3 to GF3.1.
So the client is bad place for hardcoding the search logic of feature related list of libraries.
Please note that GF3.1 and GF3 has the same identifier ( see issue #192767 ).
So I would suppose to put this logic into the plugin module.

Please note that this is not Jersey specific situation : the same thing could happen with any "feature" ( JSF, .... ).
So this is the generic problem.

I don't insist that it should be done in the some specific way. I just suggest one of the possible solution. If you can suggest another good solution we can use it.
Comment 4 Petr Hejl 2011-04-20 08:27:58 UTC
Thanks, I'll look into that.

In meantime is there a reason why this abstraction can't be done via WSStackImplementation? I thought Milan design it this way. The j2eeserver module is not really interested (and should not be if possible) in REST, JSF, JPA... So J2eeModule.getLookup() is used for tunneling the strong contract such as WSStackImplementation and hopefully something similar for JPA in future. So there may be WSStackImplementation for J2SE, GF, Tomcat, WL ... establishing strong contract while keeping the j2eeserver module clean and focused on server functionality.
Comment 5 Denis Anisimov 2011-04-20 09:19:55 UTC
(In reply to comment #4)
> Thanks, I'll look into that.
> 
> In meantime is there a reason why this abstraction can't be done via
> WSStackImplementation? I thought Milan design it this way. The j2eeserver
> module is not really interested (and should not be if possible) in REST, JSF,
> JPA... 
Agreed.
>So J2eeModule.getLookup() is used for tunneling the strong contract such
> as WSStackImplementation and hopefully something similar for JPA in future. So
> there may be WSStackImplementation for J2SE, GF, Tomcat, WL ... establishing
> strong contract while keeping the j2eeserver module clean and focused on server
> functionality.
I don't have objections from this suggestion.
But I don't see a good way to do this without J2EE server details.

Look at the current implementation of the WSStackImplementation for GF.
Once again : it is based on the same identifier ( see issue #192767 ) for GF3.1 and GF3.0.
So I don't see how GF3.0 and GF3.1 can provide different Jersey libraries.

Again : I don't insist on the requested functionality in the special module j2eeserver or another. This task is just about some unified functionality to get
"feature" libraries.
Comment 6 David Konecny 2011-05-10 00:55:40 UTC
(In reply to comment #5)
> But I don't see a good way to do this without J2EE server details.
> Look at the current implementation of the WSStackImplementation for GF.

Is not the problem that WSStackImplementation for GlassFish is implemented in websvc.wsstack.jaxws module? It is SPI which should be implemented by server plugin.

I do not insist on "tagged libraries" approach either. And I also agree that j2eeserver module does not have to be involved. WSStack API/SPI (as I remember it last time I looked at it) looks OK but I would not want to force JPA/JSF/others to defined their JPAStack and JSFStack APIs/SPIs. Perhaps we should just refactor WSStack and turn it into something more generic. And WS, JSF, JPA, etc would only have to define their WSStack.Tool and WSStack.Feature subclasses. (I'm just brainstorming.)
Comment 7 Denis Anisimov 2011-05-10 06:31:33 UTC
(In reply to comment #6)
> WSStack API/SPI (as I remember
> it last time I looked at it) looks OK but I would not want to force
> JPA/JSF/others to defined their JPAStack and JSFStack APIs/SPIs. Perhaps we
> should just refactor WSStack and turn it into something more generic. And WS,
> JSF, JPA, etc would only have to define their WSStack.Tool and WSStack.Feature
> subclasses. (I'm just brainstorming.)
I agree with this.
Comment 8 Petr Hejl 2011-05-10 12:52:36 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > But I don't see a good way to do this without J2EE server details.
> > Look at the current implementation of the WSStackImplementation for GF.
> 
> Is not the problem that WSStackImplementation for GlassFish is implemented in
> websvc.wsstack.jaxws module? It is SPI which should be implemented by server
> plugin.
I was a bit surprised that it does not depend on the plugin. I don't see any good reason for that except a fear of contribution to other module. I guess many obstacles and problems could be avoided by moving it to GF module or making it client of that module.
 
> I do not insist on "tagged libraries" approach either. And I also agree that
> j2eeserver module does not have to be involved. WSStack API/SPI (as I remember
> it last time I looked at it) looks OK but I would not want to force
> JPA/JSF/others to defined their JPAStack and JSFStack APIs/SPIs. Perhaps we
> should just refactor WSStack and turn it into something more generic.
Well I would not be strictly against that, but more generic API we design for such unrelated things as WS, JPA, JSF the more complicated to use it will be - simple things won't be simple.

For example what JPA needs even now is something like this (just random things I remember):
isJPA1Supported()
isJPA2Supported()
getDefaultProvider()
isEE5Provider()
getClassPath()

I think it is better to have such clear API/SPI with strong and clear contract for JPA and different clear API/SPI for JSF and WS rather than generic API with lot of meta-things. I personally do not like WSStack because of all that abstraction, but if it works ok for clients there is no reason to change it now.

> And WS,
> JSF, JPA, etc would only have to define their WSStack.Tool and WSStack.Feature
> subclasses. (I'm just brainstorming.)
Comment 9 David Konecny 2011-05-12 00:33:51 UTC
What Petr says have lots of weight IMO. It is exactly a reason why I never liked WSStack API much - generic to the point you do not know what is it good for till you discover Feature and Tool subclasses.

What about some middle ground compromise? Have a single module, something like j2eeserver.capabilities, which would be a place for different IDE areas to define their APIs. These APIs(and SPIs) would be strongly typed and non-generic, eg. JPA signature examples given by PetrH and similarly for WS and JSF. The server plugin implementors would not have to look at (and depend on) dozen different places for SPI to implement.

I'm just brainstorming. I must say off-hand I do not know how libraries unification would work with this. What are the next steps? Would you Denis want to expand TaggedLibraries document with some design proposal based on this discussion or do we need to discuss this together in a meeting or ... ?
Comment 10 Denis Anisimov 2011-05-12 07:04:10 UTC
(In reply to comment #9)
> What Petr says have lots of weight IMO. It is exactly a reason why I never
> liked WSStack API much - generic to the point you do not know what is it good
> for till you discover Feature and Tool subclasses.
I don't like it either. It is very unclean .
But such approach ( reworked and clarified ) has an advantage : it 
could be used for various areas ( JSF,... ) because of generic approach.

I don't know the details of server libraries usage in the areas other 
than REST/JAX-WS actually. So if you say we don't need to have 
such abstract generic approach  we can really keep existing 
strongly typed approach.
> 
> What about some middle ground compromise? Have a single module, something like
> j2eeserver.capabilities, which would be a place for different IDE areas to
> define their APIs. These APIs(and SPIs) would be strongly typed and
> non-generic, eg. JPA signature examples given by PetrH and similarly for WS and
> JSF. The server plugin implementors would not have to look at (and depend on)
> dozen different places for SPI to implement.
This is the option.

The one thing I really want to have : I need REST, JAX-WS feature stack to be 
implemented on the server plugin side ( GF, WL , .... ). Because the current 
approach is very bad.
> 
> I'm just brainstorming. I must say off-hand I do not know how libraries
> unification would work with this. What are the next steps? Would you Denis want
> to expand TaggedLibraries document with some design proposal based on this
> discussion or do we need to discuss this together in a meeting or ... ?

Again, I cannot say about areas other than REST/JAX-WS. 
But here is what I need for REST area : 
1) I don't need actually the libraries itself. I just need the possibility to add required libraries to the project classpath. 
Libraries could be the plain jar files or deployable ServerLibraries.
So I need just method "addLibraries()" for REST stack without knowing of 
the implementation of this method.
2) Implementation should be J2EE server specific and done on the server plugin side if possible. ( GF shows necessity for this ).

There is also JAX-WS stack implementation ( it is based on the same 
WSStack API ). I don't know the details of its usage in the JAX-WS area because
I'm not an author.
But I can also look at it and formulate requirements. 

I can put this detailed requirement on the wiki after detailed 
analysis of current WSStack usage in the mentioned areas.

Other areas should be reviewed and handled by their owners.
If we agree on the strongly typed approach then we don't need a common API .
Each owner can work on its own requirements and API/SPI classes.

I don't see necessity in the special meeting for this.
Let's just agree here on the way what we choose firstly.
Comment 11 Petr Jiricka 2011-05-12 08:03:33 UTC
> I need REST, JAX-WS feature stack to be implemented on the server plugin side ( GF, WL , .... )

Would it make sense to move the wsstack implementation to the individual server plugins as the first step? After this, we can continue the refactoring and improve the API.
Comment 12 Denis Anisimov 2011-05-12 08:11:53 UTC
(In reply to comment #11)
> > I need REST, JAX-WS feature stack to be implemented on the server plugin side ( GF, WL , .... )
> 
> Would it make sense to move the wsstack implementation to the individual server
> plugins as the first step? After this, we can continue the refactoring and
> improve the API.

It could make sense.
But actually for now I've fixed the main issues with REST libraries ( this fixes
are hacky and not expendable but they works for now ).
So it would make sense to rework the "API" itself and move implementation of the 
API into the server plugins.
Otherwise we will do the work twice:
- Move badly designed code into the server plugins with some rewriting.
- Dramatically change the API and its implementation.

Instead of just one second item step.
And we will not have any advantages on the first step actually with the two-steps approach.
Comment 13 David Konecny 2011-05-16 04:36:31 UTC
Sorry for not responding earlier - I was busy with other stuff. I will comment tomorrow. I need to think about this a bit.
Comment 14 David Konecny 2011-05-17 00:04:59 UTC
Here are my preferences:

* 'strongly typed' approach

* single module where different API/SPI are maintained by different people. My main motivations for this is #1) to give server plugin providers a single module where all SPIs they should implement are located; and #2) to give clients an easy system into which they can easily add their API/SPI for server plugins to implement.

I was thinking about libraries unification and I'm not sure it is doable. I mean ServerLibraries API and IDE Libraries are quite different concepts and require different handling and trying to unify them might result in too much abstraction which is hard to understand. What is your opinion?
Comment 15 David Konecny 2011-05-17 00:13:25 UTC
Created attachment 108341 [details]
sketch of module

Just for illustration of my thoughts here is the API/SPI sketch. SPI would be implemented by server plugin and placed in server lookup. API would be single final class entry point which would accommodate expanding number of SPIs. Just a sketch.
Comment 16 David Konecny 2011-05-17 00:15:29 UTC
Sorry for accidental change of versions.
Comment 17 Denis Anisimov 2011-05-17 08:07:08 UTC
So I agree with suggested approach and generally with API/SPI attached.
I just have doubts about Runtime class necessity :
I understand its probable future extending purpose but I don't see actually 
how it can be expendable. It's purpose is delegate "capability" calls to server API and ServerInstance do this job. In case we will need some new functionality 
we will need extends the server plugin API/SPI along with Runtime.
So I don't see the real need of the Runtime class.
Probably I missed something.
Comment 18 Petr Hejl 2011-05-17 09:00:16 UTC
(In reply to comment #14)
> Here are my preferences:
> 
> * 'strongly typed' approach
+1

> 
> * single module where different API/SPI are maintained by different people. My
> main motivations for this is #1) to give server plugin providers a single
> module where all SPIs they should implement are located; and #2) to give
> clients an easy system into which they can easily add their API/SPI for server
> plugins to implement.
I'm not sure it is ok to have _single_ module for _different_ people. Although it might look more complicated to have couple of modules it would be better in long term (maintenance, ownership etc.) imo. For #2 I don't think it really simplifies the changes in the API/SPI - API review is needed anyway.
 
> I was thinking about libraries unification and I'm not sure it is doable. I
> mean ServerLibraries API and IDE Libraries are quite different concepts and
> require different handling and trying to unify them might result in too much
> abstraction which is hard to understand. What is your opinion?
I agree - that was my concern.
Comment 19 Petr Hejl 2011-05-17 09:28:07 UTC
(In reply to comment #15)
> Created an attachment (id=108341) [details]
> sketch of module
> 
> Just for illustration of my thoughts here is the API/SPI sketch. SPI would be
> implemented by server plugin and placed in server lookup. API would be single
> final class entry point which would accommodate expanding number of SPIs. Just
> a sketch.
Thanks David! I planned to do a sketch, but I haven't had time for that. As it is a sketch I'm not sure on what level of detail I should comment on it.

I would change *Capabilities to *Support it sounds more logical and API/SPI will provide more than capabilities (cp, libraries) I guess. I would avoid enums for versions as addition of field to enum is not really compatible. Perhaps version should be something like org.netbeans.modules.j2ee.deployment.common.api.Version. I would also put the API (not SPI) to the lookup and let clients to use it without support static methods.
Comment 20 David Konecny 2011-05-17 21:01:27 UTC
(In reply to comment #17)
> I just have doubts about Runtime class necessity :
> I understand its probable future extending purpose but I don't see actually 
> how it can be expendable. It's purpose is delegate "capability" calls to server
> API and ServerInstance do this job. In case we will need some new functionality 
> we will need extends the server plugin API/SPI along with Runtime.

Sorry I should have illustrated better what I had in mind. Runtime is basically just

public final class Runtime {
    public Lookup getLookup() [...]
}

where lookup provides implementations of SPI defined in j2ee.runtime.enviroment. This allows any other "runtime" (apart from ServerInstance) to participate.

Just for sake of example imagine (and this is just fantasizing) that we have a support for deploying to Cloud (whatever it is) or some alternative application container (some spring stuff). These runtimes could allow registration of j2ee.runtime.enviroment's SPI and extend Runtime final class with a few more constructors and some conditional logic in getLookup method:

    public static Runtime getJ2EEServerRuntime(ServerInstance si) {
        return new Runtime(si);
    }

    public static Runtime getCloudXRuntime(CloudDescriptor cd) {
        return new Runtime(cd);
    }

    public static Runtime getSpringSomething(SS ss) {
        return new Runtime(ss);
    }

    public Lookup getLookup() {
      if (si != null)
            return si.getJ2eePlatform().getLookup();
      else if (cd != null)
            return cd.getCapabilitiesLookup();
      else if (ss != null)
            return cd.getLookup();
    }

But I must say that it is a bit too artificial. What I wanted to avoid though is to define the API to be too ServerInstance specific. On the other hand that can be achieved differently, for example as PetrH suggested to place directly the API into runtime's lookup. That way Runtime class would not be needed at all and API method signatures would be free of runtime parameter and clients would do directly:

myGlassFish.getLookup().lookup(JSFCapabilities).getJSFServerLibraries()
myCloudX.getCapabilitiesLookup().lookup(JSFCapabilities).getJSFServerLibraries()
Comment 21 David Konecny 2011-05-17 21:09:04 UTC
(In reply to comment #18)
> I'm not sure it is ok to have _single_ module for _different_ people. Although
> it might look more complicated to have couple of modules it would be better in
> long term (maintenance, ownership etc.) imo. For #2 I don't think it really
> simplifies the changes in the API/SPI - API review is needed anyway.

Sure API review is needed always regardless of where the API is defined. What I meant was that a module which may want to define a sever capabilities API/SPI would not have to introduce the API in its own module (perhaps it does not have any API yet).
Comment 22 David Konecny 2011-05-17 21:11:30 UTC
(In reply to comment #19)
> Thanks David! I planned to do a sketch, but I haven't had time for that. As it
> is a sketch I'm not sure on what level of detail I should comment on it.

Just a sketch for discussion. I agree with all your comments and suggestions.
Comment 23 David Konecny 2011-05-17 21:16:30 UTC
One more comment:

(In reply to comment #18)
> > I was thinking about libraries unification and I'm not sure it is doable. I
> > mean ServerLibraries API and IDE Libraries are quite different concepts and
> > require different handling and trying to unify them might result in too much
> > abstraction which is hard to understand. What is your opinion?
>
> I agree - that was my concern.

What do you think Denis? Initially this issue was to "unify libraries" but that might not be doable. The rest what we discussed here is improvement worth to do but clients would have to deal with IDE libraries versus Server Libraries.
Comment 24 Denis Anisimov 2011-05-18 05:43:58 UTC
(In reply to comment #20)
> (In reply to comment #17)
> > I just have doubts about Runtime class necessity :
> > I understand its probable future extending purpose but I don't see actually 
> > how it can be expendable. It's purpose is delegate "capability" calls to server
> > API and ServerInstance do this job. In case we will need some new functionality 
> > we will need extends the server plugin API/SPI along with Runtime.
> 
> Sorry I should have illustrated better what I had in mind. Runtime is basically
> just
> 
> public final class Runtime {
>     public Lookup getLookup() [...]
> }
> 
> where lookup provides implementations of SPI defined in
> j2ee.runtime.enviroment. This allows any other "runtime" (apart from
> ServerInstance) to participate.
> 
> Just for sake of example imagine (and this is just fantasizing) that we have a
> support for deploying to Cloud (whatever it is) or some alternative application
> container (some spring stuff). These runtimes could allow registration of
> j2ee.runtime.enviroment's SPI and extend Runtime final class with a few more
> constructors and some conditional logic in getLookup method:
> 
>     public static Runtime getJ2EEServerRuntime(ServerInstance si) {
>         return new Runtime(si);
>     }
> 
>     public static Runtime getCloudXRuntime(CloudDescriptor cd) {
>         return new Runtime(cd);
>     }
> 
>     public static Runtime getSpringSomething(SS ss) {
>         return new Runtime(ss);
>     }
> 
>     public Lookup getLookup() {
>       if (si != null)
>             return si.getJ2eePlatform().getLookup();
>       else if (cd != null)
>             return cd.getCapabilitiesLookup();
>       else if (ss != null)
>             return cd.getLookup();
>     }
> 
> But I must say that it is a bit too artificial. What I wanted to avoid though
> is to define the API to be too ServerInstance specific. On the other hand that
> can be achieved differently, for example as PetrH suggested to place directly
> the API into runtime's lookup. That way Runtime class would not be needed at
> all and API method signatures would be free of runtime parameter and clients
> would do directly:
> 
> myGlassFish.getLookup().lookup(JSFCapabilities).getJSFServerLibraries()
> myCloudX.getCapabilitiesLookup().lookup(JSFCapabilities).getJSFServerLibraries()

I see , thanks.
Comment 25 Denis Anisimov 2011-05-18 05:56:01 UTC
(In reply to comment #23)
> One more comment:
> 
> (In reply to comment #18)
> > > I was thinking about libraries unification and I'm not sure it is doable. I
> > > mean ServerLibraries API and IDE Libraries are quite different concepts and
> > > require different handling and trying to unify them might result in too much
> > > abstraction which is hard to understand. What is your opinion?
> >
> > I agree - that was my concern.
> 
> What do you think Denis? Initially this issue was to "unify libraries" but that
> might not be doable. The rest what we discussed here is improvement worth to do
> but clients would have to deal with IDE libraries versus Server Libraries.

So the proposition is some sort of unification. :)
I don't have a whole picture for all J2EE features that requires libraries from 
the server. I just wanted to remove logic which is currently in the REST 
configuration dialog where two approaches are used : plain jars ( hacky done ) 
and deployable ServerLibraries.
Clients ( in my case REST config dialog ) are not the place where such logic 
should be done. So my request is: refactor such usages to separate module 
which can be used by clients without knowing details how it is implemented.
In my case I don't need to work with libraries itself ( jars or ServerLibraries ).
I just need methods to add them into classpath.

The strongly typed approach is good enough for me.
The first proposition about library unification was just one possible way 
to do it. If it is not doable then we can use other appropriate way. 
No problem.

Strongly typed approach is not so flexible. This is only one issue. 
But there are advantages.
Comment 26 Denis Anisimov 2011-05-18 06:00:17 UTC
One thing I would like to notice: API review process has a long time period.
Can we do the following : create not real "Open API" but just API/SPI module(s) 
which are not Open API but has friend deps with other required modules.
We can implement/correct API/SPI and usages and then follow API review procedure.
Comment 27 Petr Hejl 2011-05-18 14:21:25 UTC
(In reply to comment #21)
> (In reply to comment #18)
> > I'm not sure it is ok to have _single_ module for _different_ people. Although
> > it might look more complicated to have couple of modules it would be better in
> > long term (maintenance, ownership etc.) imo. For #2 I don't think it really
> > simplifies the changes in the API/SPI - API review is needed anyway.
> 
> Sure API review is needed always regardless of where the API is defined. What I
> meant was that a module which may want to define a sever capabilities API/SPI
> would not have to introduce the API in its own module (perhaps it does not have
> any API yet).
I understand that, but introduction of new module is just minor drawback which improves the architecture imo. I'm afraid the suggested all-in-one module could become a dangerous place to enter in long term ;)
Comment 28 Petr Hejl 2011-05-18 14:24:44 UTC
(In reply to comment #26)
> One thing I would like to notice: API review process has a long time period.
> Can we do the following : create not real "Open API" but just API/SPI module(s) 
> which are not Open API but has friend deps with other required modules.
> We can implement/correct API/SPI and usages and then follow API review
> procedure.

I agree. This is imo the best way how to design an api. Friend or public under development api for one release, api review and public api later.

BTW I can design some prototype, but obviously the usable api has to be designed by owners of respective areas (they know what they need).
Comment 29 David Konecny 2011-05-18 21:48:29 UTC
We are in agreement. Decision whether we will introduce a single API module or each technology will have its own API/module is for PetrH to made as he is the J2EE Server person.

A few more comments to defend my proposal :-) (I do not care really which proposal we choose):

(In reply to comment #27)
> I understand that, but introduction of new module is just minor drawback which
> improves the architecture imo.

Introducing a new module does not automatically improves the architecture. Having multiple modules with API/SPI to describe server features can lead actually to opposite. Set of these modules modules will have to be enumerated somewhere and maintained so that server plugins know what to implement and clients know what to use (single module approach is self descriptive). There is a risk that individual modules use slightly different API programming patterns which adds burned on clients when accessing server features (single module can enforce API consistency as part of API review process).

> I'm afraid the suggested all-in-one module could
> become a dangerous place to enter in long term ;)

It is not all-in-one. In short term there will be likely three interfaces: WS, JPA, and maybe JSF. They all are server feature descriptors. If something else is added it will be again descriptor of server features. Interfaces are coming from different technologies but they are primarily about the same. What danger do you imagine? What would it cause?

My two cents. :-)
Comment 30 Denis Anisimov 2011-05-19 05:25:45 UTC
I don't have some special strong arguments for single module.
But from my point of view the single module is better than several.
Server plugin developer will need somehow to know about new "J2EE feature" 
in new module appearing. 
It is easy to track changes in just one module.
Comment 31 Petr Hejl 2011-05-19 19:15:45 UTC
Ok, I think our point of view slightly differ. In my understanding multiple modules are better in situation where these multiple APIs are used by multiple clients (both code and person) which are unrelated and providers are unrelated as well. I think in our case there are multiple unrelated clients (JPA, JSF, ...). The providers would share same codebase and partially same person would maintain it, but with support of expert in given area. Plus the implementation on serverplugin side may support only selected technologies or the server itself may support only some of them.

Anyway we can postpone the decision because we do not plan the public API now. I'll start with single module.
Comment 32 Petr Hejl 2011-05-20 14:18:58 UTC
I made some prototyping on JPA and WL. I don't expect this would be prefect - it can't be as I'm not an expert in JPA area. Once this is finished, JPA clients should not need isToolSupported and getToolClasspath.

I created branch for prototype in web-main called serverplugins-support. The build job is http://bertram.netbeans.org/hudson/job/serverplugins-next/. Sorry about the name of the job - I don't have permissions to rename it.

changeset 4ac505595e18
Comment 33 Denis Anisimov 2011-05-22 14:31:16 UTC
(In reply to comment #31)
> Ok, I think our point of view slightly differ. In my understanding multiple
> modules are better in situation where these multiple APIs are used by multiple
> clients (both code and person) which are unrelated and providers are unrelated
> as well. I think in our case there are multiple unrelated clients (JPA, JSF,
> ...). The providers would share same codebase and partially same person would
> maintain it, but with support of expert in given area. Plus the implementation
> on serverplugin side may support only selected technologies or the server
> itself may support only some of them.
Yes, it make sense. From the client point of view this approach is better.
> 
> Anyway we can postpone the decision because we do not plan the public API now.
> I'll start with single module.
Agreed.
Comment 34 Petr Hejl 2011-05-23 14:08:32 UTC
I tried to fix usages of isToolSupported in web project used from JPA. As I'm unsure about semantic of certain usages, Sergey should review.

WebJPAModuleInfo.isJPAVersionSupported is actually checking the version supported by default provider. Is that right?

There is a Provider for each provider/version combination in WebPersistenceProviderSupplier. I guess this was used because of limited amount of information we were getting from single isToolSupported call. Now the code looks bit ugly and could be simplified in future, I guess.

web-main branch serverplugins-support 0b30017f4750
Comment 35 David Konecny 2011-05-24 22:47:13 UTC
(In reply to comment #34)
> I tried to fix usages of isToolSupported in web project used from JPA. As I'm
> unsure about semantic of certain usages, Sergey should review.

Sergey it should be appreciated if you could have a look at it and other usages of isToolSupported and getToolClasspath in JPA support and whether they could be replaced as well with new JpaProvider* API. Or suggest how to modify JpaProvider* API. I assume you would be the owner of this API and PetrH is just prototyping.
Comment 36 David Konecny 2011-05-24 23:00:22 UTC
PetrH, it all looks good. The only thing I wonder is: clients will have to update their code to use a new APIs and at that time ideally they would completely got rid of all old (deprecated) API calls. Which is not the case on the branch right now. For example instead of 

  platform.isToolSupported("defaultPersistenceProviderJavaEE5")

they have to call now

  platform.getLookup().lookup(JpaSupport.class).getDefaultProvider() != null ||
  platform.isToolSupported("defaultPersistenceProviderJavaEE5")

which is not good. This shows IMO why SPI implementors should never provide directly API - it is not flexible design pattern. If API is defined in j2ee.specs.support then it can delegate to either a new SPI and if not available use deprecated old API to get as best answer as possible. Such approach centralizes all deprecated API calls into one place which is acceptable exception (with the only purpose being backward compatibility) and which can be in future easily removed without any impact on API clients.
Comment 37 Sergey Petrov 2011-05-25 07:14:00 UTC
Can it be done on main branch? or it will brake all persistence support?

Don't like all this fallbacks also but may be it's good for one release also may be good if move to base branch now.

Also I don't really like 
provider.isJpa1Supported()
provider.isJpa2Supported()
later...
provider.isJpa21Supported()
provider.isJpaXXSupported()
why it's not the way isJpaVersionSupported(String version)? or if it's a provider just provider.getVersion();

Is there any classes implemented JpaProviderImplementation somewhere?
Comment 38 Petr Hejl 2011-05-25 13:50:10 UTC
(In reply to comment #36)
> PetrH, it all looks good. The only thing I wonder is: clients will have to
> update their code to use a new APIs and at that time ideally they would
> completely got rid of all old (deprecated) API calls.

You are absolutely right. I wanted to quickly do the prototyping and missed that. Fixed in web-main, branch serverplugins-support 29c2a4097575.
Comment 39 Petr Hejl 2011-05-25 13:58:55 UTC
(In reply to comment #37)
> Can it be done on main branch? or it will brake all persistence support?
The branch is more convenient and safe. Right now some modules use the new API and some modules are unchanged. Multiple people may contribute to the stabilization in safe way. So I think working in branch is better.

> 
> Don't like all this fallbacks also but may be it's good for one release also
> may be good if move to base branch now.
> 
> Also I don't really like 
> provider.isJpa1Supported()
> provider.isJpa2Supported()
> later...
> provider.isJpa21Supported()
> provider.isJpaXXSupported()
> why it's not the way isJpaVersionSupported(String version)? or if it's a
> provider just provider.getVersion();
I don't see any big problem with method per version. I would agree with isJpaVersionSupported(Version version) - with version being really version not a String. I would not agree with provider.getVersion() unless there is a real need for that - client's would use if(provider.getVersion().equals(XXX)) construction instead of some simple isSomethingAllowed() or getSomething() polluting all the code with versions.

> 
> Is there any classes implemented JpaProviderImplementation somewhere?
It is in JpaProviderFactory allowing some advanced implementations on serverplugin side.
Comment 40 Petr Hejl 2011-05-25 14:16:07 UTC
(In reply to comment #35)
> (In reply to comment #34)
> I assume you would be the owner of this API and PetrH is just
> prototyping.

Yes this is going to be Sergey's API. I'm fixing the "broken" j2eeserver API by this effort.
Comment 41 Sergey Petrov 2011-05-25 14:26:47 UTC
>I would agree with
isJpaVersionSupported(Version version) - with version being really version not
a String.

do you mean creation of some PersistenceVersion enumeration?
right now usage looks a bit complex, first compare string and second call to appropriate isJPAxxSupported();
Comment 42 Petr Hejl 2011-05-25 14:42:01 UTC
(In reply to comment #41)
> >I would agree with
> isJpaVersionSupported(Version version) - with version being really version not
> a String.
> 
> do you mean creation of some PersistenceVersion enumeration?
Not really. Enumeration is not a good solution when you can't really enumerate possible values. Plus addition of field to enum is an incompatible API change. I meant something like org.netbeans.modules.j2ee.deployment.common.api.Version. In fact this class could be reviewed and refactored to j2ee.core as it is quite generic.

I don't like strings for versions as version is really a datatype and usually we used it that way - comparing, getting major number etc. With string such manipulation is substituted with substring and string comparison copy-pasted everywhere.
Comment 43 David Konecny 2011-05-25 23:34:29 UTC
(In reply to comment #38)
> Fixed in web-main, branch serverplugins-support 29c2a4097575.

That's better but now there are two ways how to get JpaSupport:

- JpaSupport.getInstance
- platform.getLookup().lookup(JpaSupport.class)

where second approach is not backward compatible and therefore erroneous. If all clients must use JpaSupport.getInstance then we can as well as ask SPI providers to not put JpaSupport API into lookup but use more widespread pattern (in NetBeans) for SPI providers to place SPI impls into lookup. What's the advantage of changing pattern and asking SPI providers to provide API class in the lookup?
Comment 44 Petr Hejl 2011-05-26 10:50:52 UTC
(In reply to comment #43)
> (In reply to comment #38)
> > Fixed in web-main, branch serverplugins-support 29c2a4097575.
> 
> That's better but now there are two ways how to get JpaSupport:
> 
> - JpaSupport.getInstance
> - platform.getLookup().lookup(JpaSupport.class)
> 
> where second approach is not backward compatible and therefore erroneous. If
> all clients must use JpaSupport.getInstance then we can as well as ask SPI
> providers to not put JpaSupport API into lookup but use more widespread pattern
> (in NetBeans) for SPI providers to place SPI impls into lookup. What's the
> advantage of changing pattern and asking SPI providers to provide API class in
> the lookup?

Putting SPI into lookup would make situation even worse - clients would get access to it.

I think there is no single pattern. It depends on usage of lookup. If the lookup is used as SPI registration facility (and not published to clients) it is ok to put SPIs into it. In this case lookup is used for tunneling (aka teleporting) the API from serverplugin to client with help of lookup (in j2eeserver). So in our situation the lookup is exposed to clients and should contain API.

I changed the solution to one I considered originally. Now the only way to get JpaSupport is via lookup and the fallback is injected to it if there is no other JpaSupport.

web-main serverplugins-support 804070ab31ca
Comment 45 David Konecny 2011-05-26 20:16:19 UTC
(In reply to comment #44)
> Putting SPI into lookup would make situation even worse - clients would get
> access to it.

"even worse"? That's not true! :-) You know that. 

> I think there is no single pattern. It depends on usage of lookup. If the
> lookup is used as SPI registration facility (and not published to clients) it
> is ok to put SPIs into it. In this case lookup is used for tunneling (aka
> teleporting) the API from serverplugin to client with help of lookup (in
> j2eeserver). So in our situation the lookup is exposed to clients and should
> contain API.

It make sense what you are saying but most of current lookups are published to clients and yet they act as SPI registration facility. For example Project.getLookup or Lookup.getDefault. And there are no problems with that. SPI is different from API and using directly SPI does not make sense.

> I changed the solution to one I considered originally. Now the only way to get
> JpaSupport is via lookup and the fallback is injected to it if there is no
> other JpaSupport.

This will work. I would question though whether added code complexity to make API idiot-proof and/or more "pure" is worth it. The code will be harder to understand for a newcomer (as compared to for example widespread API pattern used for Queries) because server plugin lookup has to be modified and extra items has to be added to it only for the sake of API design pattern. Why otherwise you would add Lookups.fixed(this) and "J2EE/Platform/Lookup" to it? To me simplicity of the code is more important. If some people want to abuse something you will never stop them anyway. But paradox is that most of the people never abuse API if it gives them all they need. My two cents again! If you want to do it this way go ahead. :-)
Comment 46 Petr Hejl 2011-05-26 20:47:06 UTC
I'll think about that. My concern is that many clients of j2eeserver used to be really clueless. Queries are bit different case. I'm afraid of J2eeModuleProvider (SPI or SPI/API combination) case in project lookup - lot of troubles with maintenance.

What I meant with "even worse" is that we had two way of getting API (lookup and static method) and with SPI and static method there would be one way (lookup) you would get SPI and the other with static method you would get an API. Perhaps I'm missing something. In such case I would be really scared clients may use the first solution they will find - SPI in lookup - and it would "somehow work for them" until they would hit the plugin without JpaSupport or until we would change something in that static method.
Comment 47 David Konecny 2011-05-26 22:32:08 UTC
We cannot save users who do not read Javadoc. In queries API pattern SPI  always says to never look it up directly but use API call xyz instead to access API. Simple like that.

(In reply to comment #46)
> What I meant with "even worse" is that we had two way of getting API (lookup
> and static method) 

What I was referring to is that looking up the API (instead of using static method) would give you *broken* API versus

> and with SPI and static method there would be one way
> (lookup) you would get SPI and the other with static method you would get an
> API.

in this case it is actually better - you always get only one valid API via one way. If user lookups SPI it is there problem. It is SPI and not API.
Comment 48 Petr Hejl 2011-05-27 15:08:27 UTC
Accepting David's suggestion. web-main serverplugins-support 222e4207584c
Comment 49 Petr Hejl 2011-06-13 12:35:24 UTC
JPA calls to isToolSupported removed from EJB and maven.
web-main serverplugins-support 7b9cf3de0ead
Comment 50 Petr Hejl 2011-06-13 12:59:11 UTC
I would consider this done for JPA. Sergey, if you would like to change something feel free to propose an API change and we can review it. Otherwise I would suggest merging this to trunk and recommend this as a case study for other possible areas and corresponding APIs (JSF, WS, ...).
Comment 51 Petr Hejl 2011-06-15 13:35:02 UTC
FYI I refactored j2ee to javaee 141140164814.
Comment 52 Petr Hejl 2011-06-16 12:49:23 UTC
Merged to trunk 0e9752e9066a
Comment 53 Quality Engineering 2011-06-17 14:39:31 UTC
Integrated into 'main-golden'
Changeset: http://hg.netbeans.org/main-golden/rev/4ac505595e18
User: phejl@netbeans.org
Log: #196466 - Java EE server technologies APIs - initial prototyping