Maven: refactored libs' urls configuration

diff --git a/plugins/maven/src/main/java/org/jetbrains/idea/maven/importing/MavenRootModelAdapter.java b/plugins/maven/src/main/java/org/jetbrains/idea/maven/importing/MavenRootModelAdapter.java
index 97d321bef7f166864abd1934efcd47a58120988b..5faa976b914be3f54a3a1bde8b31b5eb82e9ffcd 100644 (file)
--- a/plugins/maven/src/main/java/org/jetbrains/idea/maven/importing/MavenRootModelAdapter.java
+++ b/plugins/maven/src/main/java/org/jetbrains/idea/maven/importing/MavenRootModelAdapter.java
@@ -222,32 +222,28 @@ public class MavenRootModelAdapter {
     if (library == null) {
       library = provider.createLibrary(libraryName);
     }
-
     Library.ModifiableModel libraryModel = provider.getLibraryModel(library);
 
-    setUrl(libraryModel, OrderRootType.CLASSES, artifact, null);
-    setUrl(libraryModel, OrderRootType.SOURCES, artifact, MavenConstants.CLASSIFIER_SOURCES);
-    setUrl(libraryModel, JavadocOrderRootType.getInstance(), artifact, MavenConstants.CLASSIFIER_JAVADOC);
+    updateUrl(libraryModel, OrderRootType.CLASSES, artifact, null, true);
+    if (!MavenConstants.SCOPE_SYSTEM.equals(artifact.getScope())) {
+      updateUrl(libraryModel, OrderRootType.SOURCES, artifact, MavenConstants.CLASSIFIER_SOURCES, false);
+      updateUrl(libraryModel, JavadocOrderRootType.getInstance(), artifact, MavenConstants.CLASSIFIER_JAVADOC, false);
+    }
 
     LibraryOrderEntry e = myRootModel.addLibraryEntry(library);
     e.setExported(isExportable);
     e.setScope(scope);
-
-    removeOldLibraryDependency(artifact);
   }
 
-  private void setUrl(Library.ModifiableModel libraryModel,
-                      OrderRootType type,
-                      MavenArtifact artifact,
-                      String classifier) {
+  private void updateUrl(Library.ModifiableModel library, OrderRootType type, MavenArtifact artifact, String classifier, boolean clearAll) {
     String newUrl = artifact.getUrlForExtraArtifact(classifier);
-    for (String url : libraryModel.getUrls(type)) {
+    for (String url : library.getUrls(type)) {
       if (newUrl.equals(url)) return;
-      if (MavenConstants.SCOPE_SYSTEM.equals(artifact.getScope()) || isRepositoryUrl(artifact, url, classifier)) {
-        libraryModel.removeRoot(url, type);
+      if (clearAll || isRepositoryUrl(artifact, url, classifier)) {
+        library.removeRoot(url, type);
       }
     }
-    libraryModel.addRoot(newUrl, type);
+    library.addRoot(newUrl, type);
   }
 
   private boolean isRepositoryUrl(MavenArtifact artifact, String url, String classifier) {
@@ -266,33 +262,25 @@ public class MavenRootModelAdapter {
     if (dotPos == -1) return true;
     String pathToJar = classes.substring(0, dotPos);
 
-    String[] sources = library.getUrls(OrderRootType.SOURCES);
-    if (sources.length != 1 || !FileUtil.startsWith(sources[0], pathToJar)) return true;
-
-    String[] javadoc = library.getUrls(JavadocOrderRootType.getInstance());
-    if (javadoc.length != 1 || !FileUtil.startsWith(javadoc[0], pathToJar)) return true;
+    if (hasUserPaths(OrderRootType.SOURCES, library, pathToJar)) return true;
+    if (hasUserPaths(JavadocOrderRootType.getInstance(), library, pathToJar)) return true;
 
     return false;
   }
 
-  private void removeOldLibraryDependency(MavenArtifact artifact) {
-    Library lib = findLibrary(artifact, false);
-    if (lib == null) return;
-    LibraryOrderEntry entry = myRootModel.findLibraryOrderEntry(lib);
-    if (entry == null) return;
-
-    myRootModel.removeOrderEntry(entry);
-  }
-
-  public Library findLibrary(MavenArtifact artifact) {
-    return findLibrary(artifact, true);
+  private static boolean hasUserPaths(OrderRootType rootType, Library library, String pathToJar) {
+    String[] sources = library.getUrls(rootType);
+    for (String each : sources) {
+      if (!FileUtil.startsWith(each, pathToJar)) return true;
+    }
+    return false;
   }
 
-  private Library findLibrary(final MavenArtifact artifact, final boolean newType) {
+  public Library findLibrary(final MavenArtifact artifact) {
     return myRootModel.processOrder(new RootPolicy<Library>() {
       @Override
       public Library visitLibraryOrderEntry(LibraryOrderEntry e, Library result) {
-        String name = newType ? makeLibraryName(artifact) : artifact.getDisplayStringForLibraryName();
+        String name = makeLibraryName(artifact);
         return name.equals(e.getLibraryName()) ? e.getLibrary() : result;
       }
     }, null);

diff --git a/plugins/maven/src/test/java/org/jetbrains/idea/maven/importing/DependenciesImportingTest.java b/plugins/maven/src/test/java/org/jetbrains/idea/maven/importing/DependenciesImportingTest.java
index 2316d1e5473c491b1ca054f6f513dc0d6980dfe6..3afb802ec337dcb26f9362e1bc989c39044d9738 100644 (file)
--- a/plugins/maven/src/test/java/org/jetbrains/idea/maven/importing/DependenciesImportingTest.java
+++ b/plugins/maven/src/test/java/org/jetbrains/idea/maven/importing/DependenciesImportingTest.java
@@ -29,6 +29,7 @@ import org.jetbrains.idea.maven.project.MavenProject;
 
 import java.io.File;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 
 public class DependenciesImportingTest extends MavenImportingTestCase {
@@ -69,9 +70,8 @@ public class DependenciesImportingTest extends MavenImportingTestCase {
 
     assertModules("project");
     assertModuleLibDep("project", "Maven: junit:junit:4.0",
-                       "jar://" + getRepositoryPath() + "/junit/junit/4.0/junit-4.0.jar!/",
-                       "jar://" + getRepositoryPath() + "/junit/junit/4.0/junit-4.0-sources.jar!/",
-                       "jar://" + getRepositoryPath() + "/junit/junit/4.0/junit-4.0-javadoc.jar!/");
+                       Arrays.asList("jar://" + getRepositoryPath() + "/junit/junit/4.0/junit-4.0.jar!/"),
+                       Collections.<String>emptyList(), Collections.<String>emptyList());
   }
 
   public void testTestJarDependencies() throws Exception {
@@ -110,9 +110,9 @@ public class DependenciesImportingTest extends MavenImportingTestCase {
                   "</dependencies>");
     assertModules("project");
     assertModuleLibDep("project", "Maven: junit:junit:bar:4.0",
-                        "jar://" + getRepositoryPath() + "/junit/junit/4.0/junit-4.0-bar.jar!/",
-                        "jar://" + getRepositoryPath() + "/junit/junit/4.0/junit-4.0-sources.jar!/",
-                        "jar://" + getRepositoryPath() + "/junit/junit/4.0/junit-4.0-javadoc.jar!/");
+                       "jar://" + getRepositoryPath() + "/junit/junit/4.0/junit-4.0-bar.jar!/",
+                       "jar://" + getRepositoryPath() + "/junit/junit/4.0/junit-4.0-sources.jar!/",
+                       "jar://" + getRepositoryPath() + "/junit/junit/4.0/junit-4.0-javadoc.jar!/");
   }
 
   public void testSystemDependencyWithoutPath() throws Exception {
@@ -1256,35 +1256,6 @@ public class DependenciesImportingTest extends MavenImportingTestCase {
     assertModuleLibDeps("project", "My Library", "Maven: junit:junit:4.0");
   }
 
-  public void testRemoveOldTypeLibraryDependencies() throws Exception {
-    importProject("<groupId>test</groupId>" +
-                  "<artifactId>project</artifactId>" +
-                  "<version>1</version>");
-
-    ModifiableRootModel rootModel = ModuleRootManager.getInstance(getModule("project")).getModifiableModel();
-    LibraryTable.ModifiableModel tableModel = rootModel.getModuleLibraryTable().getModifiableModel();
-    tableModel.createLibrary("junit:junit:4.0");
-    tableModel.commit();
-    rootModel.commit();
-
-    assertModuleLibDeps("project", "junit:junit:4.0");
-
-    createProjectPom("<groupId>test</groupId>" +
-                     "<artifactId>project</artifactId>" +
-                     "<version>1</version>" +
-
-                     "<dependencies>" +
-                     "  <dependency>" +
-                     "    <groupId>junit</groupId>" +
-                     "    <artifactId>junit</artifactId>" +
-                     "    <version>4.0</version>" +
-                     "  </dependency>" +
-                     "</dependencies>");
-    importProject();
-
-    assertModuleLibDeps("project", "Maven: junit:junit:4.0");
-  }
-
   public void testDoNotResetUserModuleDependencies() throws Exception {
     VirtualFile m1 = createModulePom("m1",
                                      "<groupId>test</groupId>" +
@@ -1380,6 +1351,55 @@ public class DependenciesImportingTest extends MavenImportingTestCase {
                        Arrays.asList("jar://" + getRepositoryPath() + "/junit/junit/4.0/junit-4.0-javadoc.jar!/", "file://foo.javadoc"));
   }
 
+  public void testDoNotResetCustomRootForSystemLibraries() throws Exception {
+    importProject("<groupId>test</groupId>" +
+                  "<artifactId>project</artifactId>" +
+                  "<version>1</version>" +
+
+                  "<dependencies>" +
+                  "  <dependency>" +
+                  "    <groupId>xxx</groupId>" +
+                  "    <artifactId>yyy</artifactId>" +
+                  "    <version>1</version>" +
+                  "    <scope>system</scope>" +
+                  "    <systemPath>c:/foo/foo.jar</systemPath>" +
+                  "  </dependency>" +
+                  "</dependencies>");
+
+    assertProjectLibraries("Maven: xxx:yyy:1");
+    assertModuleLibDeps("project", "Maven: xxx:yyy:1");
+
+    // add sources and javadoc near the jar, just like in the local repository
+    addLibraryRoot("Maven: xxx:yyy:1", OrderRootType.SOURCES, "jar://c:/foo/foo-sources.jar!/");
+    addLibraryRoot("Maven: xxx:yyy:1", JavadocOrderRootType.getInstance(), "jar://c:/foo/foo-javadoc.jar!/");
+
+    assertModuleLibDep("project", "Maven: xxx:yyy:1",
+                       Arrays.asList("jar://c:/foo/foo.jar!/"),
+                       Arrays.asList("jar://c:/foo/foo-sources.jar!/"),
+                       Arrays.asList("jar://c:/foo/foo-javadoc.jar!/"));
+
+    createProjectPom("<groupId>test</groupId>" +
+                     "<artifactId>project</artifactId>" +
+                     "<version>1</version>" +
+
+                     "<dependencies>" +
+                     "  <dependency>" +
+                     "    <groupId>xxx</groupId>" +
+                     "    <artifactId>yyy</artifactId>" +
+                     "    <version>1</version>" +
+                     "    <scope>system</scope>" +
+                     "    <systemPath>c:/bar/bar.jar</systemPath>" +
+                     "  </dependency>" +
+                     "</dependencies>");
+    scheduleResolveAll();
+    resolveDependenciesAndImport();
+
+    assertModuleLibDep("project", "Maven: xxx:yyy:1",
+                       Arrays.asList("jar://c:/bar/bar.jar!/"),
+                       Arrays.asList("jar://c:/foo/foo-sources.jar!/"),
+                       Arrays.asList("jar://c:/foo/foo-javadoc.jar!/"));
+  }
+
   public void testUpdateRootEntriesWithActualPath() throws Exception {
     importProject("<groupId>test</groupId>" +
                   "<artifactId>project</artifactId>" +
@@ -1492,8 +1512,8 @@ public class DependenciesImportingTest extends MavenImportingTestCase {
 
     assertModuleLibDep("project", "Maven: xxx:yyy:1",
                        Arrays.asList("jar://c:/foo/bar.jar!/"),
-                       Arrays.asList("jar://c:/foo/bar-sources.jar!/"),
-                       Arrays.asList("jar://c:/foo/bar-javadoc.jar!/"));
+                       Collections.<String>emptyList(),
+                       Collections.<String>emptyList());
 
     scheduleResolveAll();
     resolveDependenciesAndImport();
@@ -1502,8 +1522,8 @@ public class DependenciesImportingTest extends MavenImportingTestCase {
 
     assertModuleLibDep("project", "Maven: xxx:yyy:1",
                        Arrays.asList("jar://c:/foo/bar.jar!/"),
-                       Arrays.asList("jar://c:/foo/bar-sources.jar!/"),
-                       Arrays.asList("jar://c:/foo/bar-javadoc.jar!/"));
+                       Collections.<String>emptyList(),
+                       Collections.<String>emptyList());
   }
 
   public void testRemovingPreviousSystemPathForForSystemLibraries() throws Exception {
@@ -1523,8 +1543,8 @@ public class DependenciesImportingTest extends MavenImportingTestCase {
 
     assertModuleLibDep("project", "Maven: xxx:yyy:1",
                        Arrays.asList("jar://c:/foo/bar.jar!/"),
-                       Arrays.asList("jar://c:/foo/bar-sources.jar!/"),
-                       Arrays.asList("jar://c:/foo/bar-javadoc.jar!/"));
+                       Collections.<String>emptyList(),
+                       Collections.<String>emptyList());
 
     createProjectPom("<groupId>test</groupId>" +
                      "<artifactId>project</artifactId>" +
@@ -1545,8 +1565,8 @@ public class DependenciesImportingTest extends MavenImportingTestCase {
 
     assertModuleLibDep("project", "Maven: xxx:yyy:1",
                        Arrays.asList("jar://c:/foo/xxx.jar!/"),
-                       Arrays.asList("jar://c:/foo/xxx-sources.jar!/"),
-                       Arrays.asList("jar://c:/foo/xxx-javadoc.jar!/"));
+                       Collections.<String>emptyList(),
+                       Collections.<String>emptyList());
   }
 
   public void testRemovingUnusedLibraries() throws Exception {