Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 353775

Summary: Static web project facets should be allowed with Java facet
Product: [WebTools] WTP Common Tools Reporter: Jim Zhang <jzhang>
Component: Faceted Project FrameworkAssignee: Carl Anderson <ccc>
Status: RESOLVED FIXED QA Contact: Konstantin Komissarchik <konstantin>
Severity: enhancement    
Priority: P2 CC: cbridgha, ccc, kaloyan, kholdaway, neil.hauge, paul.fullbright, shaun.smith, shr31223, thatnitind
Version: 3.2.4Flags: ccc: review+
Target Milestone: 3.4 M4   
Hardware: All   
OS: All   
Whiteboard:
Attachments:
Description Flags
removed the conflict constraint for jst.java 1.6
none
Remove the wst.web conflict constraints from all jst.java facets
none
patch for WTP 3.2 none

Description Jim Zhang CLA 2011-08-03 10:38:23 EDT
Build Identifier: Eclipse 3.6.2

There are projects that contain both java and web resources but are not JavaEE web modules, such as Android and Blackberry projects using PhoneGap.  As such these projects can benefit from the web tools that are enabled in web projects.  However currently the static web project facet conflicts with Java facet.

Reproducible: Always
Comment 1 Jim Zhang CLA 2011-08-05 09:46:36 EDT
I experimented with this idea, by removing the conflict definition b/w wst.web and jst.java.  After installing the two facets together, project explorer was able to display the project model correctly, basically adding the static web presentation (Javascript Resources, WebContent) alongside java (src, JRE System Library).

I went through a simply scenario of adding web pages, editing them in HTML editor, running html on HTTP server, adding and editing java classes.  All seem to work fine with any problems.
Comment 2 Jim Zhang CLA 2011-08-11 15:28:56 EDT
Created attachment 201343 [details]
removed the conflict constraint for jst.java 1.6

attached the patch for the modified /org.eclipse.jst.common.project.facet.core/plugin.xml, which allowed me to add the static web project facet with jst.java 1.6
Comment 3 Carl Anderson CLA 2011-08-11 15:42:58 EDT
Konstantin, is there still any reason why jst.java excludes wst.web?  I know that, from a puritanical standpoint, "Static Web Projects" are not supposed to have Java content, but is there really any strict limitation as to why we would want to prohibit it?

Note that, back when Static and Dynamic Web projects were first designed, there were no mobile phones or anysuch that could understand both static content and some Java.
Comment 4 Konstantin Komissarchik CLA 2011-08-11 15:47:48 EDT
I was just going to ask you the same question. It would likely be fine to remove this restriction. Maybe make this change early in Juno and see if anyone identifies problems?
Comment 5 Jim Zhang CLA 2011-08-16 11:06:31 EDT
I just realized this configuration applies to OSGi as well.  you can have web resources (html/js/css) in an OSGi bundle (not a web bundle), and write a little bit of java code to register the resources with the HTTP services that is built-in to OSGi.  this way your OSGi container will serve up the web resources.  

in such a scenario the project should be just an OSGi (java) project with the static web facet installed.
Comment 6 Jim Zhang CLA 2011-08-16 11:08:46 EDT
Given how widely this configuration is applicable (hybrid mobile, OSGi, sMash, and could be more), can we have this change in 3.6.2 instead of waiting for 3.8?
Comment 7 Carl Anderson CLA 2011-08-16 13:17:29 EDT
Created attachment 201594 [details]
Remove the wst.web conflict constraints from all jst.java facets

This patch is created against HEAD for WTP 3.4.0.  I removed all of the conflicts constraints- I see no reason to limit this to only Java 1.6.
Comment 8 Carl Anderson CLA 2011-08-16 13:29:23 EDT
We just declared Juno M1, so I committed this change to HEAD for Juno M2.  I am keeping this bug open to see about backporting this change to previous streams, as per the originator's request.
Comment 9 Jim Zhang CLA 2011-08-16 15:38:13 EDT
Created attachment 201603 [details]
patch for WTP 3.2

thanks Carl, for your convenience, here's the patch against WTP 3.2 stream.
Comment 10 Carl Anderson CLA 2011-08-30 12:12:41 EDT
I have run the entire Java EE Tools JUnit bucket against this, with no errors.  I have also tested this by hand.

The next step to get this into WTP 3.2.5 will be to get PMC approval, barring any reason why that should not be done.
Comment 11 Carl Anderson CLA 2011-09-13 14:51:06 EDT
PMC Review requested due to behavior change

Explain why you believe this is a stop-ship defect. Or, if it is a "hotbug" (requested by an adopter) please document it as such.

This is requested by an adopter - IBM

Is there a work-around? If so, why do you believe the work-around is insufficient?

There is no work-around.

How has the fix been tested? Is there a test case attached to the bugzilla record? Has a JUnit Test been added?

As documented in comment #10, this has been tested by hand and by running the Java EE Tools JUnit bucket.  No JUnit Test has been added.

Give a brief technical overview. Who has reviewed this fix?

Konstantin and myself have reviewed this fix.  This fix simply removes the constraint that wst.web faceted projects cannot have the jst.java facet.  There is no technical reason for wst.web faceted projects to not have applets or other similar Java classes in them... in fact, they should be allowed to.

What is the risk associated with this fix? 

Low - this is just a plugin.xml/behavior change, it is the removal of an unnecessary limitation.  It is already tested and working in WTP 3.4.0
Comment 12 Kaloyan Raev CLA 2011-09-14 02:44:22 EDT
I am really confused from this change. I understand the business need and I agree that we must address it. But I am not satisfied with the way we do this. 

I reject this change for two reasons: 
  1) This is quite a core change that may affect other WTP projects and adopter. Undertaking such change requires open discussion which I haven't seen happened. 
  2) I don't like the technical solution - looks like a quick and dirty fix that does not address the problem in a sustainable way for the long term. 

IMHO, after this change having "Static Web Module" and "Dynamic Web Module" facets becomes semantically meaningless. I would be more in favor of redesigning a little bit these two facet so they complement each other instead of being in parallel worlds. 

One possible approach is to rename the "Static Web Module" facet to just "Web" facet. It will add the web capabilities (WebContent, etc) to the project. And it will not conflict with the Java facet. Just like suggested in this bug. However, the "Dynamic Web Module" facet should become an add-on to the Web facet that adds the Java EE Servlet capabilities to the web project. This means to rename the "Dynamic Web Module" to something like "Servlet" and make it *require* the "Web" facet. Of course, such change would require some adjustments in dependent artifacts: wizards, other facets, etc. 

I believe the above approach addresses this new business need much better for the long term. And this is just an example. They could be other suggestion if we ask for them. This is why we need a discussion about this change in a broader round.
Comment 13 Konstantin Komissarchik CLA 2011-09-14 07:30:02 EDT
> I would be more in favor of redesigning a little bit these two facet so they 
> complement each other instead of being in parallel worlds. 

The idea of layering static/dynamic web facets is tracked by Bug 173901. It is a better solution, but it isn't clear how to do that in a backwards compatible manner, as such no one has been willing to expend the energy to push that forward.

This bug tracks the pragmatic change that is backwards compatible and while it doesn't resolve the larger usability issues with having conflicting static/dynamic web facets, it does allow for important user scenarios in a fairly cheap manner.
Comment 14 Jim Zhang CLA 2011-09-14 09:10:29 EDT
(In reply to comment #13)
I completely agree with Konstantin here.

(In reply to comment #12)
I also agree with your proposal, in fact I had the same thoughts around making jst.web additive to wst.web, but like Konstantin said, it's a much bigger undertaking and is really parallel to this current change.
Comment 15 Kaloyan Raev CLA 2011-09-14 11:15:42 EDT
I don't agree that this change is backward compatible. We must treat facets as API. And this API consists of several attributes: facet id, facet version and facet constraints. Removing a facet constraint is breaking the API. WTP is pretty famous project and is adopted in many use cases. We cannot be sure that there isn't any adopter that is relying on the existence of the constraint that the wst.web facet conflicts with the jst.java facet. 

I suggest that for Juno we address this problem in the scope of bug 173901. 

For Helios and Indigo stream we can release the change proposed in the current bug as a feature patch, so the adopters that need this change *intentionally* install the patch in their products. But we should keep this change out of the official 3.2.x and 3.3.x release, so we don't break any adopter. 

Does this sound acceptable?

And we should not forget the most important thing: public discussion in a broader round.
Comment 16 Nitin Dahyabhai CLA 2011-09-14 12:00:26 EDT
(In reply to comment #15)
Kaloyen, I do not know of a situation in which removing a conflicting Facet constraint would constitute a breaking change.  Konstantin has said it is compatible in comment 13, and existing workspace projects will neither be modified nor marked as erroneous because of this change.
Comment 17 Chuck Bridgham CLA 2011-09-14 14:42:12 EDT
The Facet framework was designed to allow individual facets the ability to declare supporting artifacts (classpath, libraries), and a lifecycle api that interacts with these relevent libraries or domains.  In almost all instances... individual facets should not be aware or concerned with other defined facets.

The wst.web restriction from jst.java always seemed broken to me, because these unrelated facets were artificially prevented from being installed on the same project, and in essence, prevented wst.web artifacts from ever being installed on a faceted java project.

In the case of projects using the jst.java facet, I don't see how any api would be affected with the inclusion of the wst.web facet, that has no awareness of java api's or artifacts.

The same holds true for projects using wst.web, but the ability to be installed on a java project does introduce an added environment, but all of wst.web api's would not be affected.

I also agree, as I stated in 2007, with layering the static and dynamic web facets.  But over the past few releases, priorities have kept this enhancement from being fulfilled, and without a significant commitment to restructuring these facets, we may not get to it in Juno either.   

This is why I do think this solution should be considered until we get to this currently uncommitted enhancement.

I also think procedure has been followed in making sure full PMC review has been done in this case, but perhaps more public review of this proposed change is warranted, and we can post this to wtp-dev.
Comment 18 Carl Anderson CLA 2011-09-14 14:58:53 EDT
I think we are all in agreement - this issue needs to be raised in wtp-dev.  However, I wanted to chime in on a couple of issues, to make sure that this is clear:

We are not making the Static Web facet anywhere near the same as the Dynamic Web Facet.  You still need a Dynamic Web project for:  JSPs, Servlets, a classpath container that includes the contents of WEB-INF/lib, a WEB-INF/web.xml, packaging as a WAR, etc.

We are allowing for Static Web projects to have Applets and other Java classes in them (rather than having a Java project map to the Static Web project, ending up with a .jar inside of it).  For devices such as mobile phones (which can have html, css, JavaScript, and Java classes), this is a necessity- you can't have the classes in a jar.  When I first developed a web site in 1995, it consisted of an index.html that had a Java applet in it (not in a jar) that would redirect you to other html files according to what was displayed at the time.  Sadly, I cannot create that same Static Web project in older versions of WTP, since I cannot create a Java Applet in my Static Web project.

And please note that, as I stated in comment #8, this change went into WTP 3.4.0 already.  I was seeking approval to backport it to WTP 3.2.5 since an adopter has that need.
Comment 19 Kaloyan Raev CLA 2011-09-15 09:03:02 EDT
Let me clarify what I mean by "breaking". I treat the facets as API - I've already explained this in comment 12. This API has a behavior: if the wst.web facet is installed in a faceted project, then the jst.java facet cannot be installed. This change breaks this behavior. We cannot be sure that this change in the behavior will not affect any adopter. And we cannot estimate the impact of such potential problems. We simply don't know - and this is the biggest problem with this change. We even don't remember why this constraint has been introduced. This makes me even more concerned. 

I see this as a high risk change. Therefore, I believe it's safer to release it as a feature patch than including it in the maintenance releases. 

Yes, we need this topic raised in a more public channel. I would say that wtp-dev is too narrow. We often explain to user and adopters that wtp-dev is only for WTP committers and forward them to the WTP forums. If we want to reach these people we need to have this discussion in the forum too.
Comment 20 Nitin Dahyabhai CLA 2011-09-15 13:33:33 EDT
(In reply to comment #19)
> We often explain to user and adopters that wtp-dev is
> only for WTP committers and forward them to the WTP forums.

I don't think we've ever said that to adopters; at least not ones that have identified themselves as such.
Comment 21 Kaloyan Raev CLA 2011-09-15 13:42:28 EDT
It took me less than a minute to find one example: http://dev.eclipse.org/mhonarc/lists/wtp-dev/msg08049.html

We can never be sure which user is actually an adopter. There are adopters than are not so close to the core of the Eclipse community to know that they have to identify themselves as "adopters".
Comment 22 Paul Fullbright CLA 2011-09-15 14:27:29 EDT
Need to be kept in the loop on this.  As JPA is an SE/EE crossover technology (and one that *does* depend in several aspects on what other facets are installed) it's important to understand how this will impact our configuration.
Comment 23 Paul Fullbright CLA 2011-09-15 15:34:21 EDT
I was just testing this out, and we do have a bit of a configuration problem.

If you install java, static web, and jpa into a project, we can't find our resources.  The project has a ModuleCoreNature, but doesn't use it to map the src folder to the root deploy location, so we can't use it to map runtime paths to devtime resources.  We would have to use the simple java project method of mapping runtime paths to devtime resources, but in order to do that we'd have to change our test for which mapping mechanism to use.

Is this intended?  Or should in fact an assembly mapping be provided for source folders to the runtime root path?
Comment 24 Kaloyan Raev CLA 2011-09-16 08:48:01 EDT
Carl, thanks for posting a note for this change to the wtp-dev mailing list and the forum.
Comment 25 Jim Zhang CLA 2011-09-16 14:13:50 EDT
(In reply to comment #23)
hi Paul, can you describe the scenario where you saw the problems?  Is this an existing JPA use case that used to work fine and now is broken?  

Or the problem arises only when one adds wst.web facets to an existing JPA project, or the other way around maybe for using JPA inside applets?  This would be new scenarios that each tooling area can decide whether to support or not.  Just because it's now possible to do doesn't mean it is actually supported right?

On the other hand, I would think there is value from JPA tools view point that this change enables JPA tools team to consider supporting JPA + web scenarios (not sure what the real world scenarios are, except for applets using JPA).
Comment 26 Paul Fullbright CLA 2011-09-16 15:27:37 EDT
Since JPA requires java, this would be a new use case, although not radically new.  JPA works perfectly well in all current scenarios that support java.  I don't know if it even makes sense, so one option of course would be to have JPA and wst.web conflict.  (Adding Shaun so he can comment.)

This occurs whether one installs JPA into a static web project, or installs static web content into a JPA project.  It occurs, because once the project has a module core nature, we depend on that nature to map runtime paths to resource locations for us.  When there is no module core nature, we make do with several assumptions about how a typical java project is constructed.

So there are several ways to address this problem:
- make JPA and wst.web conflict (as described above)
- change our rules for when the module core nature is used to map runtime paths to resource locations
- add deployment assembly mappings for java source locations in a wst.web project.  (Still waiting on a explanation about whether this should or should not be done.)
- Others may be possible, specific to wst.web w/ JPA

At any rate, here we are one example of how this change can affect downstream plugins in unpredictable ways. :)
Comment 27 Chuck Bridgham CLA 2011-09-27 10:29:22 EDT
After much discussion in the PMC - It has been determined that all OSGI extension points related to a Java API  (facets) is also API(including behavior change), and cannot be changed in a maintenance release per our API policy.  An adopter could provide a feature patch to override this behavior, but the maintenance stream will not accept this change.

Evolving an API is acceptable in the current release with proper project review and discussion with the exception of breaking changes that do require a minimum 2 year deprecation period.
Comment 28 Chuck Bridgham CLA 2011-09-27 10:30:24 EDT
resolving as won't fix in maintenance release
Comment 29 Paul Fullbright CLA 2011-10-06 12:02:27 EDT
added bug 360122 to address java resource/runtime path mapping.
Comment 30 Carl Anderson CLA 2012-03-08 11:43:55 EST
In reviewing this, this bug was used to commit the change to HEAD.  As such, I am changing the target to reflect its actual commit time, and I am removing the PMC request and flags, and changing the status to RESOLVED/FIXED.