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

Bug 308125

Summary: [API] Support more flexible facet group enlistment
Product: [WebTools] WTP Common Tools Reporter: Tim deBoer <deboer>
Component: Faceted Project FrameworkAssignee: Konstantin Komissarchik <konstantin>
Status: CLOSED FIXED QA Contact: Konstantin Komissarchik <konstantin>
Severity: enhancement    
Priority: P3 CC: paul.fullbright, raghunathan.srinivasan
Version: unspecifiedFlags: konstantin: pmc_approved? (david_williams)
raghunathan.srinivasan: pmc_approved+
konstantin: pmc_approved? (naci.dai)
konstantin: pmc_approved? (deboer)
konstantin: pmc_approved? (neil.hauge)
konstantin: pmc_approved? (kaloyan)
Target Milestone: 3.2 M7   
Hardware: All   
OS: All   
Whiteboard: PMC_approved
Attachments:
Description Flags
Patch none

Description Tim deBoer CLA 2010-04-05 18:08:46 EDT
See bug 234383 (https://bugs.eclipse.org/bugs/show_bug.cgi?id=234383) for some background or an example of why this might be necessary, but I'll phrase this as a general 'adopter' issue.

When people define a new project facet, it usually looks something like this:
  <project-facet-version facet="my.facet" version="1.0">
    <constraint>
      <and>
        <requires facet="jst.java" version="[5.0"/>
        <or>
          <requires facet="jst.utility" version="[1.0"/>
          <requires facet="jst.appclient" version="[1.2"/>
          <requires facet="jst.ejb" version="[1.1"/>
          ...
        </or>
      </and>
    </constraint>
  </project-facet-version>

i.e. this new facet requires Java, and there is a set of other facets that it works with.

By necessity, the list of facets that it works with (the "<or>s") are *known* facets - i.e. facets in the same Eclipse project or from its dependencies. However, Eclipse is by design an extremely extendable IDE. It is quite common for an adopter to extend it and define new facet types, or specialize existing function into a new facet.

In cases like this, adopters essentially need a mechanism to add their new facets into the list of <or>s. One possibility that has been discussed is the ability to replace the <or> list with a group, and for the adopter to extend that group. This would be fine, although I wonder if it implies that facet creators should always use groups and never the syntax above, since it would still inherently block adopters.

Any mechanism/implementation is fine of course; preferrably something that can be used in the Eclipse 3.6/WTP 3.2 stream.
Comment 1 Konstantin Komissarchik CLA 2010-04-22 15:32:06 EDT
Changing the bug title to focus on the chosen implementation strategy.
Comment 2 Konstantin Komissarchik CLA 2010-04-22 15:36:21 EDT
Created attachment 165836 [details]
Patch

This patch implements more flexible facility for enlisting facets into groups. Previously, facets could only be enlisted into groups at the time there were defined, which limited group enlistment to facet authors. This patch retains the existing facility, but also makes it possible for third parties to enlist facets into groups.

The following example taken from the documentation for the new extension point shows the syntax and semantics.

<extension point="org.eclipse.wst.common.project.facet.core.groups">

  <!-- 
    Groups do not have to be explicitly defined. They come into existence when the
    first member is added, however explicit definition is recommended as that's the
    only way to specify a label and a description for the group. You can also 
    enlist facets into the group as part of that group's definition.
  -->
  
  <group id="gr">
    <label>Test Group</label>
    <description>Test group of facets.</description>
    <include facet="a"/>
    <include facet="b" versions="1.2"/>
    <include facet="c" versions="[1.3-2.0)"/>
  </group>
  
  <!--
    This is the most flexible way of enlisting facets into the group. This
    declaration can be made by a third party that is neither the facet author
    nor the group creator.
  -->
  
  <members group="gr">
    <include facet="d" versions="2.3,2.4,2.7"/>
    <include facet="e"/>
  </members>
  
</extension>

<!-- Facet can be enlisted into one or more group as part of facet definition. -->

<extension point="org.eclipse.wst.common.project.facet.core.facets">
  <project-facet id="f">
    ...
  </project-facet>
  <project-facet-version facet="f" version="1.0">
    ...
    <group-member id="gr"/>
    ...
  </project-facet-version>
</extension>
Comment 3 Konstantin Komissarchik CLA 2010-04-22 15:45:07 EDT
This change adds new API and deprecates a portion of existing API, so raising for PMC review. There is no impact on UI.

============

    * 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 functionality has been requested by an adopter to solve an adopter product use case.

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

No.

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

The attached patch includes a new unit test specifically designed to test the new feature. The existing suite of unit tests also touches on the affected area. Unit tests and manual smoke tests pass.

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

See Comment #2.

    * What is the risk associated with this fix? 

Low risk.
Comment 4 Konstantin Komissarchik CLA 2010-04-22 16:08:58 EDT
Released equivalent changes to fproj code stream. WTP PMC approval is necessary to release to WTP 3.2 code stream.
Comment 5 Tim deBoer CLA 2010-04-22 23:25:51 EDT
Since I requested the change, I'll try to abstain from voting. :)

The fundamental problem is that facet dependencies are not extendible - if an adopter creates a new facet that will work with a WTP facet, there is usually no way to extend the requires 'one of' set. This extension provides a simple (and hence lower risk) mechanism for adopters to add facets into a group defined by WTP. I will start testing with the patch immediately, but what Konstantin describes solves the problem.

Neil may get a bit of tingling in the back of his head re: bug 234383. If the approval and testing goes well I'll submit a minor patch to Dali so that we could take advantage of this.
Comment 6 Tim deBoer CLA 2010-04-24 22:07:29 EDT
Testing complete - both elements of the new extension point are working great, as well as interaction with the existing group and constraints elements. In other words, everything I've tested is working perfectly and it solves the problem. Thanks, Konstantin!
Comment 7 Konstantin Komissarchik CLA 2010-04-26 13:47:26 EDT
Changes released to 3.2 M7 code stream.
Comment 8 Tim deBoer CLA 2010-05-17 10:15:32 EDT
(Re)verified in RC1, working great. Thanks again.