Maven: correctly set sources/javadoc urls for artifacts with classifiers

diff --git a/platform/util/src/com/intellij/openapi/util/io/FileUtil.java b/platform/util/src/com/intellij/openapi/util/io/FileUtil.java
index 0ff6f7cd39520f300e3100d06ea7e531d9aa9ad1..aa58b0a89bd72707371786045acf45f0eaf03e00 100644 (file)
--- a/platform/util/src/com/intellij/openapi/util/io/FileUtil.java
+++ b/platform/util/src/com/intellij/openapi/util/io/FileUtil.java
@@ -659,24 +659,24 @@ public class FileUtil {
     delete(source);
   }
 
-  public static boolean startsWith(@NonNls String path1, @NonNls String path2) {
-    return startsWith(path1, path2, SystemInfo.isFileSystemCaseSensitive);
+  public static boolean startsWith(@NonNls String path, @NonNls String start) {
+    return startsWith(path, start, SystemInfo.isFileSystemCaseSensitive);
   }
 
-  public static boolean startsWith(final String path1, final String path2, final boolean caseSensitive) {
-    final int length1 = path1.length();
-    final int length2 = path2.length();
+  public static boolean startsWith(final String path, final String start, final boolean caseSensitive) {
+    final int length1 = path.length();
+    final int length2 = start.length();
     if (length2 == 0) return true;
     if (length2 > length1) return false;
-    if (!path1.regionMatches(!caseSensitive, 0, path2, 0, length2)) return false;
+    if (!path.regionMatches(!caseSensitive, 0, start, 0, length2)) return false;
     if (length1 == length2) return true;
-    char last2 = path2.charAt(length2 - 1);
+    char last2 = start.charAt(length2 - 1);
     char next1;
     if (last2 == '/' || last2 == File.separatorChar) {
-      next1 = path1.charAt(length2 -1);
+      next1 = path.charAt(length2 -1);
     }
     else {
-      next1 = path1.charAt(length2);
+      next1 = path.charAt(length2);
     }
     return next1 == '/' || next1 == File.separatorChar;
   }

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 6b2db908180b152d6e316bfa4531a0ae226e9e93..97d321bef7f166864abd1934efcd47a58120988b 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
@@ -240,7 +240,7 @@ public class MavenRootModelAdapter {
                       OrderRootType type,
                       MavenArtifact artifact,
                       String classifier) {
-    String newUrl = artifact.getUrlForClassifier(classifier);
+    String newUrl = artifact.getUrlForExtraArtifact(classifier);
     for (String url : libraryModel.getUrls(type)) {
       if (newUrl.equals(url)) return;
       if (MavenConstants.SCOPE_SYSTEM.equals(artifact.getScope()) || isRepositoryUrl(artifact, url, classifier)) {
@@ -251,7 +251,7 @@ public class MavenRootModelAdapter {
   }
 
   private boolean isRepositoryUrl(MavenArtifact artifact, String url, String classifier) {
-    return url.endsWith(artifact.getRelativePathForClassifier(classifier) + JarFileSystem.JAR_SEPARATOR);
+    return url.endsWith(artifact.getRelativePathForExtraArtifact(classifier) + JarFileSystem.JAR_SEPARATOR);
   }
 
   public static boolean isChangedByUser(Library library) {
@@ -260,20 +260,18 @@ public class MavenRootModelAdapter {
 
     String classes = classRoots[0];
 
-    int dotPos = classes.lastIndexOf(".");
+    if (!classes.endsWith("!/")) return true;
+
+    int dotPos = classes.lastIndexOf("/", classes.length() - 2 /* trim ending !/ */);
     if (dotPos == -1) return true;
-    String path = classes.substring(0, dotPos);
+    String pathToJar = classes.substring(0, dotPos);
 
-    String jarSuffix = ".jar" + JarFileSystem.JAR_SEPARATOR;
-    String sourcesPath = path + "-" + MavenConstants.CLASSIFIER_SOURCES + jarSuffix;
-    String javadocPath = path + "-" + MavenConstants.CLASSIFIER_JAVADOC + jarSuffix;
+    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;
 
-    for (String each : library.getUrls(OrderRootType.SOURCES)) {
-      if (!FileUtil.pathsEqual(each, sourcesPath)) return true;
-    }
-    for (String each : library.getUrls(JavadocOrderRootType.getInstance())) {
-      if (!FileUtil.pathsEqual(each, javadocPath)) return true;
-    }
     return false;
   }
 

diff --git a/plugins/maven/src/main/java/org/jetbrains/idea/maven/project/MavenArtifact.java b/plugins/maven/src/main/java/org/jetbrains/idea/maven/project/MavenArtifact.java
index 0c047302dd7ccd9be5f2926aa3fadf70d07d0e31..6e0c06a45d73d503f3640228d3e8d5ddef108ca8 100644 (file)
--- a/plugins/maven/src/main/java/org/jetbrains/idea/maven/project/MavenArtifact.java
+++ b/plugins/maven/src/main/java/org/jetbrains/idea/maven/project/MavenArtifact.java
@@ -19,15 +19,11 @@ package org.jetbrains.idea.maven.project;
 import com.intellij.openapi.util.io.FileUtil;
 import com.intellij.openapi.util.text.StringUtil;
 import com.intellij.openapi.vfs.JarFileSystem;
-import com.intellij.openapi.vfs.LocalFileSystem;
-import com.intellij.openapi.vfs.VirtualFile;
 import com.intellij.openapi.vfs.VirtualFileManager;
 import org.apache.maven.artifact.Artifact;
 import org.apache.maven.artifact.handler.ArtifactHandler;
 import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
 import org.jetbrains.idea.maven.embedder.CustomArtifact;
-import static org.jetbrains.idea.maven.project.MavenId.append;
 import org.jetbrains.idea.maven.utils.MavenConstants;
 
 import java.io.File;
@@ -36,6 +32,8 @@ import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.List;
 
+import static org.jetbrains.idea.maven.project.MavenId.append;
+
 public class MavenArtifact implements Serializable {
   private String myGroupId;
   private String myArtifactId;
@@ -148,10 +146,10 @@ public class MavenArtifact implements Serializable {
   }
 
   public String getRelativePath() {
-    return getRelativePathForClassifier(null);
+    return getRelativePathForExtraArtifact(null);
   }
 
-  public String getRelativePathForClassifier(String extraClassifier) {
+  public String getRelativePathForExtraArtifact(String extraArtifactClassifier) {
     StringBuilder result = new StringBuilder();
     result.append(myGroupId.replace('.', '/'));
     result.append('/');
@@ -162,17 +160,20 @@ public class MavenArtifact implements Serializable {
     result.append(myArtifactId);
     result.append('-');
     result.append(myVersion);
-    if (!StringUtil.isEmptyOrSpaces(myClassifier)) {
-      result.append('-');
-      result.append(myClassifier);
-    }
 
-    if (!StringUtil.isEmptyOrSpaces(extraClassifier)) {
+    if (!StringUtil.isEmptyOrSpaces(extraArtifactClassifier)) {
+      if (MavenConstants.TYPE_TEST_JAR.equals(myType)) {
+        result.append("-test");
+      }
       result.append('-');
-      result.append(extraClassifier);
+      result.append(extraArtifactClassifier);
       result.append(".jar");
     }
     else {
+      if (!StringUtil.isEmptyOrSpaces(myClassifier)) {
+        result.append('-');
+        result.append(myClassifier);
+      }
       result.append(".");
       result.append(myExtension);
     }
@@ -180,17 +181,25 @@ public class MavenArtifact implements Serializable {
   }
 
   public String getUrl() {
-    return getUrlForClassifier(null);
+    return getUrlForExtraArtifact(null);
   }
 
-  public String getUrlForClassifier(String extraClassifier) {
+  public String getUrlForExtraArtifact(String extraArtifactClassifier) {
     String path = getPath();
 
-    if (!StringUtil.isEmptyOrSpaces(extraClassifier)) {
-      int dotPos = path.lastIndexOf(".");
-      if (dotPos != -1) {// sometimes path doesn't contain '.'; but i can't find any reason.
-        String withoutExtension = path.substring(0, dotPos);
-        path = MessageFormat.format("{0}-{1}.jar", withoutExtension, extraClassifier);
+    if (!StringUtil.isEmptyOrSpaces(extraArtifactClassifier)) {
+      int repoEnd = path.lastIndexOf(getRelativePath());
+
+      if (repoEnd == -1) {
+        // unknown path format: try to add a classified at the end of the filename
+        int dotPos = path.lastIndexOf(".");
+        if (dotPos != -1) {// sometimes path doesn't contain '.'; but i can't find any reason why.
+          String withoutExtension = path.substring(0, dotPos);
+          path = MessageFormat.format("{0}-{1}.jar", withoutExtension, extraArtifactClassifier);
+        }
+      } else {
+        String repoPath = path.substring(0, repoEnd);
+        path = repoPath + getRelativePathForExtraArtifact(extraArtifactClassifier);
       }
     }
     return VirtualFileManager.constructUrl(JarFileSystem.PROTOCOL, path) + JarFileSystem.JAR_SEPARATOR;

diff --git a/plugins/maven/src/main/java/org/jetbrains/idea/maven/project/MavenProject.java b/plugins/maven/src/main/java/org/jetbrains/idea/maven/project/MavenProject.java
index a6d37c007e87d4a92bfe7c46d00befd8b634f457..c5bcb1a3e88a80f6e3cc0e3ed314b4ba727fb7fe 100644 (file)
--- a/plugins/maven/src/main/java/org/jetbrains/idea/maven/project/MavenProject.java
+++ b/plugins/maven/src/main/java/org/jetbrains/idea/maven/project/MavenProject.java
@@ -671,10 +671,10 @@ public class MavenProject {
 
   public boolean isSupportedDependency(MavenArtifact artifact) {
     String t = artifact.getType();
-    if (t.equalsIgnoreCase(MavenConstants.TYPE_JAR)
-        || t.equalsIgnoreCase("test-jar")
-        || t.equalsIgnoreCase("ejb")
-        || t.equalsIgnoreCase("ejb-client")) {
+    if (MavenConstants.TYPE_JAR.equalsIgnoreCase(t)
+        || MavenConstants.TYPE_TEST_JAR.equalsIgnoreCase(t)
+        || "ejb".equalsIgnoreCase(t)
+        || "ejb-client".equalsIgnoreCase(t)) {
       return true;
     }
 

diff --git a/plugins/maven/src/main/java/org/jetbrains/idea/maven/utils/MavenConstants.java b/plugins/maven/src/main/java/org/jetbrains/idea/maven/utils/MavenConstants.java
index f031e6b6c8f671ebe272a6fbe5c3ec6b9533c97c..c1be345b500ff3de448dbe78eed95a816c4b55d9 100644 (file)
--- a/plugins/maven/src/main/java/org/jetbrains/idea/maven/utils/MavenConstants.java
+++ b/plugins/maven/src/main/java/org/jetbrains/idea/maven/utils/MavenConstants.java
@@ -23,6 +23,7 @@ public class MavenConstants {
   public static final String SETTINGS_XML = "settings.xml";
 
   public static final String TYPE_JAR = "jar";
+  public static final String TYPE_TEST_JAR = "test-jar";
   public static final String TYPE_WAR = "war";
 
   public static final String CLASSIFIER_JAVADOC = "javadoc";

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 40c8ae8a07c8b6997892bc3cee28a8b106991877..a64e540a805b2830da37f047e8b51595aced37ad 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
@@ -74,6 +74,47 @@ public class DependenciesImportingTest extends MavenImportingTestCase {
                        "jar://" + getRepositoryPath() + "/junit/junit/4.0/junit-4.0-javadoc.jar!/");
   }
 
+  public void testTestJarDependencies() throws Exception {
+    importProject("<groupId>test</groupId>" +
+                  "<artifactId>project</artifactId>" +
+                  "<version>1</version>" +
+
+                  "<dependencies>" +
+                  "  <dependency>" +
+                  "    <groupId>junit</groupId>" +
+                  "    <artifactId>junit</artifactId>" +
+                  "    <version>4.0</version>" +
+                  "    <type>test-jar</type>" +
+                  "  </dependency>" +
+                  "</dependencies>");
+
+    assertModules("project");
+    assertModuleLibDep("project", "Maven: junit:junit:test-jar:tests:4.0",
+                       "jar://" + getRepositoryPath() + "/junit/junit/4.0/junit-4.0-tests.jar!/",
+                       "jar://" + getRepositoryPath() + "/junit/junit/4.0/junit-4.0-test-sources.jar!/",
+                       "jar://" + getRepositoryPath() + "/junit/junit/4.0/junit-4.0-test-javadoc.jar!/");
+  }
+
+  public void testDependencyWithClassifier() throws Exception {
+    importProject("<groupId>test</groupId>" +
+                  "<artifactId>project</artifactId>" +
+                  "<version>1</version>" +
+
+                  "<dependencies>" +
+                  "  <dependency>" +
+                  "    <groupId>junit</groupId>" +
+                  "    <artifactId>junit</artifactId>" +
+                  "    <version>4.0</version>" +
+                  "    <classifier>bar</classifier>" +
+                  "  </dependency>" +
+                  "</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!/");
+  }
+
   public void testSystemDependencyWithoutPath() throws Exception {
     importProject("<groupId>test</groupId>" +
                   "<artifactId>project</artifactId>" +
@@ -718,41 +759,6 @@ public class DependenciesImportingTest extends MavenImportingTestCase {
                        "jar://" + envDir + "/lib/tools.jar!/");
   }
 
-  public void testTestJarDependencies() throws Exception {
-    importProject("<groupId>test</groupId>" +
-                  "<artifactId>project</artifactId>" +
-                  "<version>1</version>" +
-
-                  "<dependencies>" +
-                  "  <dependency>" +
-                  "    <groupId>group</groupId>" +
-                  "    <artifactId>artifact</artifactId>" +
-                  "    <type>test-jar</type>" +
-                  "    <version>1</version>" +
-                  "  </dependency>" +
-                  "</dependencies>");
-
-    assertModules("project");
-    assertModuleLibDeps("project", "Maven: group:artifact:test-jar:tests:1");
-  }
-
-  public void testDependencyWithClassifier() throws Exception {
-    importProject("<groupId>test</groupId>" +
-                  "<artifactId>project</artifactId>" +
-                  "<version>1</version>" +
-
-                  "<dependencies>" +
-                  "  <dependency>" +
-                  "    <groupId>group</groupId>" +
-                  "    <artifactId>artifact</artifactId>" +
-                  "    <classifier>bar</classifier>" +
-                  "    <version>1</version>" +
-                  "  </dependency>" +
-                  "</dependencies>");
-    assertModules("project");
-    assertModuleLibDeps("project", "Maven: group:artifact:bar:1");
-  }
-
   public void testDependencyWithVersionRangeOnModule() throws Exception {
     createProjectPom("<groupId>test</groupId>" +
                      "<artifactId>project</artifactId>" +
@@ -1385,8 +1391,8 @@ public class DependenciesImportingTest extends MavenImportingTestCase {
     assertModuleLibDeps("project", "Maven: org.testng:testng:jdk15:5.8", "Maven: junit:junit:3.8.1");
     assertModuleLibDep("project", "Maven: org.testng:testng:jdk15:5.8",
                        "jar://" + getRepositoryPath() + "/org/testng/testng/5.8/testng-5.8-jdk15.jar!/",
-                       "jar://" + getRepositoryPath() + "/org/testng/testng/5.8/testng-5.8-jdk15-sources.jar!/",
-                       "jar://" + getRepositoryPath() + "/org/testng/testng/5.8/testng-5.8-jdk15-javadoc.jar!/");
+                       "jar://" + getRepositoryPath() + "/org/testng/testng/5.8/testng-5.8-sources.jar!/",
+                       "jar://" + getRepositoryPath() + "/org/testng/testng/5.8/testng-5.8-javadoc.jar!/");
 
     setRepositoryPath(new File(myDir, "__repo").getPath());
     myProjectsManager.getEmbeddersManagerInTests().reset(); // to recognize repository change
@@ -1397,8 +1403,8 @@ public class DependenciesImportingTest extends MavenImportingTestCase {
 
     assertModuleLibDep("project", "Maven: org.testng:testng:jdk15:5.8",
                        "jar://" + getRepositoryPath() + "/org/testng/testng/5.8/testng-5.8-jdk15.jar!/",
-                       "jar://" + getRepositoryPath() + "/org/testng/testng/5.8/testng-5.8-jdk15-sources.jar!/",
-                       "jar://" + getRepositoryPath() + "/org/testng/testng/5.8/testng-5.8-jdk15-javadoc.jar!/");
+                       "jar://" + getRepositoryPath() + "/org/testng/testng/5.8/testng-5.8-sources.jar!/",
+                       "jar://" + getRepositoryPath() + "/org/testng/testng/5.8/testng-5.8-javadoc.jar!/");
   }
 
   public void testDoNotPopulateSameRootEntriesOnEveryImport() throws Exception {