Fixed a couple of issues with conflict detection code not picking up on indirect conflicts in all cases. Added test cases.

4 files changed, 195 insertions, 3 deletions

diff --git a/plugins/org.eclipse.wst.common.project.facet.core/src/org/eclipse/wst/common/project/facet/core/internal/ProjectFacetVersion.java b/plugins/org.eclipse.wst.common.project.facet.core/src/org/eclipse/wst/common/project/facet/core/internal/ProjectFacetVersion.java
index 7e7a87a..a85cc89 100644
--- a/plugins/org.eclipse.wst.common.project.facet.core/src/org/eclipse/wst/common/project/facet/core/internal/ProjectFacetVersion.java
+++ b/plugins/org.eclipse.wst.common.project.facet.core/src/org/eclipse/wst/common/project/facet/core/internal/ProjectFacetVersion.java
@@ -314,7 +314,7 @@ public final class ProjectFacetVersion
                 final IProjectFacetVersion fv 
                     = (IProjectFacetVersion) itr2.next();
                 
-                if( ! ( this.conflictsWith( fv ) || fv.conflictsWith( this ) ) )
+                if( ! conflictsWith( fv ) )
                 {
                     conflictsWithAllVersions = false;
                     break;
@@ -342,7 +342,8 @@ public final class ProjectFacetVersion
         }
         else
         {
-            return conflictsWith( fv, getConstraint() );
+            return conflictsWith( fv, getConstraint() ) || 
+                   conflictsWith( this, fv.getConstraint() );
         }
     }
     
@@ -422,7 +423,7 @@ public final class ProjectFacetVersion
         
             if( soft )
             {
-                return true;
+                return false;
             }
             else
             {
diff --git a/tests/org.eclipse.wst.common.project.facet.core.tests/plugin.xml b/tests/org.eclipse.wst.common.project.facet.core.tests/plugin.xml
index 710cc2d..39e976b 100644
--- a/tests/org.eclipse.wst.common.project.facet.core.tests/plugin.xml
+++ b/tests/org.eclipse.wst.common.project.facet.core.tests/plugin.xml
@@ -169,4 +169,47 @@
     
   </extension>
 
+  <!-- Facet Constraint Tests -->
+  
+  <extension point="org.eclipse.wst.common.project.facet.core.facets">
+  
+    <project-facet id="fct_f1"/>
+    
+    <project-facet-version facet="fct_f1" version="1.0">
+      <constraint>
+        <conflicts facet="fct_f2"/>
+      </constraint>
+    </project-facet-version>
+    
+    <project-facet id="fct_f2"/>
+    
+    <project-facet-version facet="fct_f2" version="1.0">
+      <constraint>
+        <conflicts facet="fct_f4"/>
+      </constraint>
+    </project-facet-version>
+  
+    <project-facet id="fct_f3"/>
+    
+    <project-facet-version facet="fct_f3" version="1.0">
+      <constraint>
+        <requires facet="fct_f2" version="1.0"/>
+      </constraint>
+    </project-facet-version>
+
+    <project-facet id="fct_f4"/>
+    
+    <project-facet-version facet="fct_f4" version="1.0">
+    </project-facet-version>
+  
+    <project-facet id="fct_f5"/>
+    
+    <project-facet-version facet="fct_f5" version="1.0">
+      <constraint>
+        <requires facet="fct_f2" version="1.0" soft="true"/>
+      </constraint>
+    </project-facet-version>
+
+  </extension>
+
 </plugin>
diff --git a/tests/org.eclipse.wst.common.project.facet.core.tests/src/org/eclipse/wst/common/project/facet/core/tests/AllTests.java b/tests/org.eclipse.wst.common.project.facet.core.tests/src/org/eclipse/wst/common/project/facet/core/tests/AllTests.java
index 9421a00..2dc4ce0 100644
--- a/tests/org.eclipse.wst.common.project.facet.core.tests/src/org/eclipse/wst/common/project/facet/core/tests/AllTests.java
+++ b/tests/org.eclipse.wst.common.project.facet.core.tests/src/org/eclipse/wst/common/project/facet/core/tests/AllTests.java
@@ -20,6 +20,7 @@ public class AllTests
         suite.addTest( BasicFacetActionTests.suite() );
         suite.addTest( FacetActionSortTests.suite() );
         suite.addTest( ProjectChangeReactionTests.suite() );
+        suite.addTest( FacetConstraintsTests.suite() );
         
         return suite;
     }
diff --git a/tests/org.eclipse.wst.common.project.facet.core.tests/src/org/eclipse/wst/common/project/facet/core/tests/FacetConstraintsTests.java b/tests/org.eclipse.wst.common.project.facet.core.tests/src/org/eclipse/wst/common/project/facet/core/tests/FacetConstraintsTests.java
new file mode 100644
index 0000000..d61f0dd
--- /dev/null
+++ b/tests/org.eclipse.wst.common.project.facet.core.tests/src/org/eclipse/wst/common/project/facet/core/tests/FacetConstraintsTests.java
@@ -0,0 +1,147 @@
+package org.eclipse.wst.common.project.facet.core.tests;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+
+import junit.framework.Test;
+import junit.framework.TestCase;
+import junit.framework.TestSuite;
+
+import org.eclipse.core.runtime.CoreException;
+import org.eclipse.wst.common.project.facet.core.IProjectFacet;
+import org.eclipse.wst.common.project.facet.core.IProjectFacetVersion;
+import org.eclipse.wst.common.project.facet.core.ProjectFacetsManager;
+import org.eclipse.wst.common.project.facet.core.IFacetedProject.Action;
+import org.eclipse.wst.common.project.facet.core.tests.support.TestUtils;
+
+public class FacetConstraintsTests
+
+    extends TestCase
+    
+{
+    private static IProjectFacet f1;
+    private static IProjectFacetVersion f1v10;
+    
+    private static IProjectFacet f2;
+    private static IProjectFacetVersion f2v10;
+    
+    private static IProjectFacet f3;
+    private static IProjectFacetVersion f3v10;
+
+    private static IProjectFacet f4;
+    private static IProjectFacetVersion f4v10;
+
+    private static IProjectFacet f5;
+    private static IProjectFacetVersion f5v10;
+    
+    static
+    {
+        f1 = ProjectFacetsManager.getProjectFacet( "fct_f1" );
+        f1v10 = f1.getVersion( "1.0" );
+
+        f2 = ProjectFacetsManager.getProjectFacet( "fct_f2" );
+        f2v10 = f2.getVersion( "1.0" );
+
+        f3 = ProjectFacetsManager.getProjectFacet( "fct_f3" );
+        f3v10 = f3.getVersion( "1.0" );
+
+        f4 = ProjectFacetsManager.getProjectFacet( "fct_f4" );
+        f4v10 = f4.getVersion( "1.0" );
+
+        f5 = ProjectFacetsManager.getProjectFacet( "fct_f5" );
+        f5v10 = f5.getVersion( "1.0" );
+    }
+    
+    private FacetConstraintsTests( final String name )
+    {
+        super( name );
+    }
+    
+    public static Test suite()
+    {
+        final TestSuite suite = new TestSuite();
+        
+        suite.setName( "Facet Constraint Tests" );
+
+        suite.addTest( new FacetConstraintsTests( "testIndirectConflict1" ) );
+        suite.addTest( new FacetConstraintsTests( "testIndirectConflict2" ) );
+        suite.addTest( new FacetConstraintsTests( "testIndirectConflict3" ) );
+        suite.addTest( new FacetConstraintsTests( "testIndirectConflict4" ) );
+        suite.addTest( new FacetConstraintsTests( "testIndirectConflict5" ) );
+        suite.addTest( new FacetConstraintsTests( "testIndirectConflict6" ) );
+        suite.addTest( new FacetConstraintsTests( "testIndirectConflict7" ) );
+        suite.addTest( new FacetConstraintsTests( "testIndirectConflict8" ) );
+        
+        return suite;
+    }
+    
+    /*
+     * Tests whether the conflict detection code picks up on an indirect
+     * conflict. Also tests that a soft constraint is not used to flag a
+     * conflict.
+     * 
+     * Here is the relationship diagram between the five facets involved in 
+     * this test: 
+     * 
+     *           conflicts            requires
+     *   f1 ----------------> f2 <---------------- f3
+     *   f4 <----------------    <---------------- f5
+     *           conflicts         soft requires
+     * 
+     * These case should come back positive for conflict:
+     * 
+     *   f1 with f3
+     *   f3 with f1
+     *   f4 with f3
+     *   f3 with f4
+     *   
+     * These case should come back negative for conflict:
+     * 
+     *   f1 with f5
+     *   f5 with f1
+     *   f4 with f5
+     *   f5 with f4
+     */
+    
+    public void testIndirectConflict1()
+    {
+        assertTrue( f1v10.conflictsWith( f3v10 ) );
+    }
+    
+    public void testIndirectConflict2()
+    {
+        assertTrue( f3v10.conflictsWith( f1v10 ) );
+    }
+
+    public void testIndirectConflict3()
+    {
+        assertTrue( f4v10.conflictsWith( f3v10 ) );
+    }
+
+    public void testIndirectConflict4()
+    {
+        assertTrue( f3v10.conflictsWith( f4v10 ) );
+    }
+    
+    public void testIndirectConflict5()
+    {
+        assertFalse( f1v10.conflictsWith( f5v10 ) );
+    }
+    
+    public void testIndirectConflict6()
+    {
+        assertFalse( f5v10.conflictsWith( f1v10 ) );
+    }
+
+    public void testIndirectConflict7()
+    {
+        assertFalse( f4v10.conflictsWith( f5v10 ) );
+    }
+
+    public void testIndirectConflict8()
+    {
+        assertFalse( f5v10.conflictsWith( f4v10 ) );
+    }
+    
+}