From e7da7b8ef69651487afabcfe7a2bf8132c2eca9c Mon Sep 17 00:00:00 2001 From: "Denis.Zhdanov" Date: Fri, 17 Feb 2012 16:12:33 +0400 Subject: [PATCH] IDEA-76142: Gradle support - cannot update IDEA projects once one of build.gradle files changes 1. Library path conflict change now aggregates all mismatched paths; 2. 'Local change' markup is used for 'conflict changes' now; 3. Corresonding tests are added; --- ...adleLibraryStructureChangesCalculator.java | 9 +-- .../GradleMismatchedLibraryPathChange.java | 8 ++- .../sync/GradleProjectStructureTreeModel.java | 3 +- .../gradle/ui/GradleProjectStructureNode.java | 7 ++- ...dleProjectStructureChangesModelTest.groovy | 58 +++++++++++++++++-- .../gradle/testutil/AbstractGradleTest.groovy | 33 +++++++++-- .../gradle/testutil/ChangeBuilder.groovy | 17 +++--- 7 files changed, 106 insertions(+), 29 deletions(-) diff --git a/plugins/gradle/src/org/jetbrains/plugins/gradle/diff/GradleLibraryStructureChangesCalculator.java b/plugins/gradle/src/org/jetbrains/plugins/gradle/diff/GradleLibraryStructureChangesCalculator.java index f746329331bb..ec95f20ffd1f 100644 --- a/plugins/gradle/src/org/jetbrains/plugins/gradle/diff/GradleLibraryStructureChangesCalculator.java +++ b/plugins/gradle/src/org/jetbrains/plugins/gradle/diff/GradleLibraryStructureChangesCalculator.java @@ -29,16 +29,17 @@ public class GradleLibraryStructureChangesCalculator implements GradleStructureC @NotNull Set currentChanges) { final Set gradleBinaryPaths = new HashSet(gradleEntity.getPaths(LibraryPathType.BINARY)); + final Set intellijBinaryPaths = new HashSet(); for (VirtualFile file : intellijEntity.getFiles(OrderRootType.CLASSES)) { final String path = myPlatformFacade.getLocalFileSystemPath(file); if (!gradleBinaryPaths.remove(path)) { - currentChanges.add(new GradleMismatchedLibraryPathChange(intellijEntity, null, path)); + intellijBinaryPaths.add(path); } } - for (String binaryPath : gradleBinaryPaths) { - currentChanges.add(new GradleMismatchedLibraryPathChange(intellijEntity, binaryPath, null)); - } + if (!gradleBinaryPaths.equals(intellijBinaryPaths)) { + currentChanges.add(new GradleMismatchedLibraryPathChange(intellijEntity, gradleBinaryPaths, intellijBinaryPaths)); + } } @NotNull diff --git a/plugins/gradle/src/org/jetbrains/plugins/gradle/diff/GradleMismatchedLibraryPathChange.java b/plugins/gradle/src/org/jetbrains/plugins/gradle/diff/GradleMismatchedLibraryPathChange.java index c6dbdbcf2d9b..9853e006c3a6 100644 --- a/plugins/gradle/src/org/jetbrains/plugins/gradle/diff/GradleMismatchedLibraryPathChange.java +++ b/plugins/gradle/src/org/jetbrains/plugins/gradle/diff/GradleMismatchedLibraryPathChange.java @@ -5,17 +5,19 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.plugins.gradle.util.GradleBundle; +import java.util.Set; + /** * @author Denis Zhdanov * @since 2/2/12 1:32 PM */ -public class GradleMismatchedLibraryPathChange extends GradleAbstractConflictingPropertyChange { +public class GradleMismatchedLibraryPathChange extends GradleAbstractConflictingPropertyChange> { private final String myLibraryName; public GradleMismatchedLibraryPathChange(@NotNull Library entity, - @Nullable String gradleValue, - @Nullable String intellijValue) + @Nullable Set gradleValue, + @Nullable Set intellijValue) throws IllegalArgumentException { super(GradleBundle.message("gradle.sync.change.library.path", entity.getName()), gradleValue, intellijValue); diff --git a/plugins/gradle/src/org/jetbrains/plugins/gradle/sync/GradleProjectStructureTreeModel.java b/plugins/gradle/src/org/jetbrains/plugins/gradle/sync/GradleProjectStructureTreeModel.java index f0dc67d9692d..e99f44b6b11c 100644 --- a/plugins/gradle/src/org/jetbrains/plugins/gradle/sync/GradleProjectStructureTreeModel.java +++ b/plugins/gradle/src/org/jetbrains/plugins/gradle/sync/GradleProjectStructureTreeModel.java @@ -222,9 +222,8 @@ public class GradleProjectStructureTreeModel extends DefaultTreeModel { } } GradleProjectStructureNode newNode = buildNode(id, id.getLibraryName()); - newNode.getDescriptor().setAttributes(attributes); + newNode.setAttributes(attributes); dependenciesNode.add(newNode); - nodeStructureChanged(dependenciesNode); } private void processNewModulePresenceChange(@NotNull GradleModulePresenceChange change) { diff --git a/plugins/gradle/src/org/jetbrains/plugins/gradle/ui/GradleProjectStructureNode.java b/plugins/gradle/src/org/jetbrains/plugins/gradle/ui/GradleProjectStructureNode.java index 37dcd05b960c..00d7209ec8b1 100644 --- a/plugins/gradle/src/org/jetbrains/plugins/gradle/ui/GradleProjectStructureNode.java +++ b/plugins/gradle/src/org/jetbrains/plugins/gradle/ui/GradleProjectStructureNode.java @@ -145,7 +145,12 @@ public class GradleProjectStructureNode extends Defaul */ public void addConflictChange(@NotNull GradleProjectStructureChange change) { myConflictChanges.add(change); - if (myConflictChanges.size() == 1) { + if (myConflictChanges.size() != 1) { + return; + } + final TextAttributesKey key = myDescriptor.getAttributes(); + boolean localNode = key == GradleTextAttributes.GRADLE_LOCAL_CHANGE || key == GradleTextAttributes.INTELLIJ_LOCAL_CHANGE; + if (!localNode) { myDescriptor.setAttributes(GradleTextAttributes.GRADLE_CHANGE_CONFLICT); onNodeChanged(this); } diff --git a/plugins/gradle/testSources/org/jetbrains/plugins/gradle/sync/GradleProjectStructureChangesModelTest.groovy b/plugins/gradle/testSources/org/jetbrains/plugins/gradle/sync/GradleProjectStructureChangesModelTest.groovy index 897969814943..3ad729659a6c 100644 --- a/plugins/gradle/testSources/org/jetbrains/plugins/gradle/sync/GradleProjectStructureChangesModelTest.groovy +++ b/plugins/gradle/testSources/org/jetbrains/plugins/gradle/sync/GradleProjectStructureChangesModelTest.groovy @@ -6,6 +6,8 @@ import org.jetbrains.plugins.gradle.testutil.AbstractGradleTest import org.junit.Test import static org.junit.Assert.assertEquals +import org.jetbrains.plugins.gradle.diff.GradleMismatchedLibraryPathChange +import org.jetbrains.plugins.gradle.diff.GradleLibraryDependencyPresenceChange /** * @author Denis Zhdanov @@ -15,7 +17,7 @@ import static org.junit.Assert.assertEquals public class GradleProjectStructureChangesModelTest extends AbstractGradleTest { @Test - public void processObsoleteGradleLocalChange() { + public void "obsolete gradle-local modules"() { // Configure initial projects state. init( gradle: { @@ -93,7 +95,7 @@ public class GradleProjectStructureChangesModelTest extends AbstractGradleTest { } @Test - public void libraryDependenciesWithDifferentPaths() { + public void "library dependencies on binary paths"() { // Let the model has two differences in a library setup initially. init( gradle: { @@ -114,8 +116,7 @@ public class GradleProjectStructureChangesModelTest extends AbstractGradleTest { checkChanges { libraryConflict(entity: intellij.libraries['lib2']) { - binaryPath(gradle: '1', intellij: null) - binaryPath(gradle: null, intellij: '3') + binaryPath(gradle: '1', intellij: ['3']) } } checkTree { project { @@ -182,7 +183,7 @@ public class GradleProjectStructureChangesModelTest extends AbstractGradleTest { } @Test - public void intellijModuleRemoval() { + public void "intellij module removal"() { Closure initialClosure = { project { module('module1') @@ -222,7 +223,7 @@ public class GradleProjectStructureChangesModelTest extends AbstractGradleTest { } @Test - public void gradleModuleIsImported() { + public void "gradle-local module is not treated as 'local' after import"() { init( gradle: { project { @@ -257,4 +258,49 @@ public class GradleProjectStructureChangesModelTest extends AbstractGradleTest { module2() // Imported module node is not highlighted anymore. } } } + + @Test + public void "gradle local library dependency outweighs library path conflict"() { + init( + gradle: { + project { + module('module1') { + dependencies { + library('lib1', bin: ['1']) + } } + module('module2') { + dependencies { + library('lib1') + } } } }, + intellij: { + project { + module('module1') { + dependencies { + library('lib1', bin: ['2']) + } } } }, + changesSorter: changeByClassSorter([ + (GradleMismatchedLibraryPathChange) : 2, + (GradleLibraryDependencyPresenceChange) : 1 + ]) + ) + + checkChanges { + presence { + module(gradle: gradle.modules.find { it.name == 'module2' }) + libraryDependency(gradle: gradle.dependencies[gradle.modules.find { it.name == 'module2' }].first()) + } + libraryConflict(entity: intellij.libraries['lib1']) { + binaryPath(gradle: ['1'], intellij: ['2']) + } } + checkTree { + project { + module1() { + dependencies { + lib1('conflict') + } } + module2('gradle') { + dependencies { + lib1('gradle') // This is the point of the test. We don't expect to see 'conflict' here. + } } } } + } } diff --git a/plugins/gradle/testSources/org/jetbrains/plugins/gradle/testutil/AbstractGradleTest.groovy b/plugins/gradle/testSources/org/jetbrains/plugins/gradle/testutil/AbstractGradleTest.groovy index 99f9fc4f7076..a6aa6d127d44 100644 --- a/plugins/gradle/testSources/org/jetbrains/plugins/gradle/testutil/AbstractGradleTest.groovy +++ b/plugins/gradle/testSources/org/jetbrains/plugins/gradle/testutil/AbstractGradleTest.groovy @@ -29,15 +29,16 @@ public abstract class AbstractGradleTest { GradleProjectStructureTreeModel treeModel def gradle def intellij - def changes + def changesBuilder def treeChecker def container + private Closure changesComparator @Before public void setUp() { gradle = new GradleProjectBuilder() intellij = new IntellijProjectBuilder() - changes = new ChangeBuilder() + changesBuilder = new ChangeBuilder() treeChecker = new ProjectStructureChecker() container = new DefaultPicoContainer() container.registerComponentInstance(Project, intellij.project) @@ -62,26 +63,35 @@ public abstract class AbstractGradleTest { protected def init(map = [:]) { treeModel = container.getComponentInstance(GradleProjectStructureTreeModel) as GradleProjectStructureTreeModel changesModel.addListener({ old, current -> - treeModel.update(current) - treeModel.processObsoleteChanges(ContainerUtil.subtract(old, current)); + treeModel.update(sortChanges(current)) + treeModel.processObsoleteChanges(sortChanges(ContainerUtil.subtract(old, current))); } as GradleProjectStructureChangeListener) setState(map, false) treeModel.rebuild() changesModel.update(gradle.project) } + def sortChanges(changes) { + if (changesComparator) { + return changes.toList().sort(changesComparator) + } + return changes + } + protected def setState(map, update = true) { map.intellij?.delegate = intellij map.intellij?.call() map.gradle?.delegate = gradle map.gradle?.call() + changesComparator = map.changesSorter if (update) { changesModel.update(gradle.project) } } protected def checkChanges(Closure c) { - c.delegate = changes + changesBuilder.changes.clear() + c.delegate = changesBuilder def expected = c() if (!expected) { expected = [].toSet() @@ -95,4 +105,17 @@ public abstract class AbstractGradleTest { def expected = c() treeChecker.check(expected, treeModel.root) } + + protected Closure changeByClassSorter(Map, Integer> rules) { + { a, b -> + def weightA = rules[a.class] ?: Integer.MAX_VALUE + def weightB = rules[b.class] ?: Integer.MAX_VALUE + if (weightA == weightB) { + return a.hashCode() - b.hashCode() + } + else { + return weightA - weightB + } + } + } } diff --git a/plugins/gradle/testSources/org/jetbrains/plugins/gradle/testutil/ChangeBuilder.groovy b/plugins/gradle/testSources/org/jetbrains/plugins/gradle/testutil/ChangeBuilder.groovy index 0a840bdb4674..2f6fc75a8862 100644 --- a/plugins/gradle/testSources/org/jetbrains/plugins/gradle/testutil/ChangeBuilder.groovy +++ b/plugins/gradle/testSources/org/jetbrains/plugins/gradle/testutil/ChangeBuilder.groovy @@ -29,9 +29,6 @@ public class ChangeBuilder extends BuilderSupport { @Override protected Object createNode(Object name, Map attributes) { - if (current == null) { - changes = [] - } switch (name) { case "module": changes.addAll attributes.gradle.collect { new GradleModulePresenceChange(it, null)} @@ -50,14 +47,11 @@ public class ChangeBuilder extends BuilderSupport { if (!library) { throw new IllegalArgumentException("No entity is defined for the library conflict change. Known attributes: $attributes") } - if (attributes.gradle) { - return register(new GradleMismatchedLibraryPathChange(library, attributes.gradle, attributes.intellij)) - } return library case "binaryPath": // Assuming that we're processing library binary path conflict here register(new GradleMismatchedLibraryPathChange( - current as Library, toCanonicalPath(attributes.gradle), toCanonicalPath(attributes.intellij) + current as Library, collectPaths(attributes.gradle), collectPaths(attributes.intellij) )) } changes @@ -73,9 +67,16 @@ public class ChangeBuilder extends BuilderSupport { protected def register(change) { changes << change - change + changes } + private def collectPaths(paths) { + if (!paths) { + return [].toSet() + } + paths.collect { toCanonicalPath(it) }.toSet() + } + private def toCanonicalPath(String path) { path ? GradleUtil.toCanonicalPath(path) : path }