merge with default branch

docs/dtd/plugin/2.0.0-01.dtd

@@ -23,7 +23,7 @@
 -->
 
 <!--- root element of the plugin descriptor -->
-<!ELEMENT plugin (scm-version|information|child-first-classloader|conditions|resources|dependencies|extension|extension-point|rest-resource|subscriber)*>
+<!ELEMENT plugin (scm-version|information|child-first-classloader|conditions|resources|dependencies|optional-dependencies|extension|extension-point|rest-resource|subscriber)*>
 
 <!--- major scm-manager version -->
 <!ELEMENT scm-version (#PCDATA)>
@@ -79,13 +79,19 @@
 <!--- contains plugin dependencies -->
 <!ELEMENT dependencies (dependency)*>
 
-<!--- sing plugin dependency -->
+<!--- contains optional plugin dependencies -->
+<!ELEMENT optional-dependencies (dependency)*>
+
+<!--- single plugin dependency -->
 <!ELEMENT dependency (#PCDATA)>
 
 <!-- generated entries -->
 
 <!--- extension -->
-<!ELEMENT extension (description|class)*>
+<!ELEMENT extension (description|class|requires)*>
+
+<!--- requires value -->
+<!ELEMENT requires (#PCDATA)>
 
 <!--- class value -->
 <!ELEMENT class (#PCDATA)>

package.json

@@ -20,7 +20,7 @@
   },
   "resolutions": {
     "babel-core": "7.0.0-bridge.0",
-    "gitdiff-parser": "https://github.com/scm-manager/gitdiff-parser#6baa7278824ecd17a199d842ca720d0453f68982"
+    "gitdiff-parser": "https://github.com/scm-manager/gitdiff-parser#ed3fe7de73dbb0a06c3e6adbbdf22dbae6e66351"
   },
   "babel": {
     "presets": [

pom.xml

@@ -284,6 +284,12 @@
         <version>${jackson.version}</version>
       </dependency>
 
+      <dependency>
+        <groupId>org.hibernate</groupId>
+        <artifactId>hibernate-validator</artifactId>
+	<version>${hibernate-validator.version}</version>
+      </dependency>
+
       <!-- JUnit 5 -->
 
       <dependency>
@@ -843,6 +849,7 @@
     <jackson.version>2.10.0</jackson.version>
     <guice.version>4.0</guice.version>
     <jaxb.version>2.3.0</jaxb.version>
+    <hibernate-validator.version>6.1.0.Final</hibernate-validator.version>
 
     <!-- event bus -->
     <legman.version>1.6.1</legman.version>

scm-annotations/src/main/java/sonia/scm/plugin/Extension.java

@@ -49,4 +49,15 @@ import java.lang.annotation.Target;
 @Target({ ElementType.TYPE })
 @PluginAnnotation("extension")
 @Retention(RetentionPolicy.RUNTIME)
-public @interface Extension {}
+public @interface Extension {
+  /**
+   * This extension is loaded only if all of the specified plugins are installed.
+   * The requires attribute can be used to implement optional extensions.
+   * A plugin author is able to implement an extension point of an optional plugin and the extension is only loaded if
+   * all of the specified plugins are installed.
+   *
+   * @since 2.0.0
+   * @return list of required plugins to load this extension
+   */
+  String[] requires() default {};
+}

scm-core/pom.xml

@@ -173,6 +173,13 @@
       <artifactId>activation</artifactId>
     </dependency>
 
+    <!-- validation -->
+
+    <dependency>
+      <groupId>org.hibernate</groupId>
+      <artifactId>hibernate-validator</artifactId>
+    </dependency>
+
     <!-- util -->
 
     <dependency>
@@ -214,12 +221,6 @@
       <artifactId>shiro-unit</artifactId>
       <scope>test</scope>
     </dependency>
-    <dependency>
-      <groupId>org.hibernate</groupId>
-      <artifactId>hibernate-validator</artifactId>
-      <version>5.3.6.Final</version>
-      <scope>compile</scope>
-    </dependency>
 
   </dependencies>
 

scm-core/src/main/java/sonia/scm/plugin/ExtensionElement.java

@@ -0,0 +1,26 @@
+package sonia.scm.plugin;
+
+import lombok.AllArgsConstructor;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import lombok.NoArgsConstructor;
+import lombok.ToString;
+
+import javax.xml.bind.annotation.XmlAccessType;
+import javax.xml.bind.annotation.XmlAccessorType;
+import javax.xml.bind.annotation.XmlElement;
+import java.util.HashSet;
+import java.util.Set;
+
+@Getter
+@ToString
+@NoArgsConstructor
+@EqualsAndHashCode
+@AllArgsConstructor
+@XmlAccessorType(XmlAccessType.FIELD)
+public class ExtensionElement {
+  @XmlElement(name = "class")
+  private String clazz;
+  private String description;
+  private Set<String> requires = new HashSet<>();
+}

scm-core/src/main/java/sonia/scm/plugin/InstalledPluginDescriptor.java

@@ -38,6 +38,7 @@ package sonia.scm.plugin;
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Objects;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
 
 import javax.xml.bind.annotation.XmlAccessType;
 import javax.xml.bind.annotation.XmlAccessorType;
@@ -76,7 +77,7 @@ public final class InstalledPluginDescriptor extends ScmModule implements Plugin
    */
   public InstalledPluginDescriptor(int scmVersion, PluginInformation information,
                                    PluginResources resources, PluginCondition condition,
-                                   boolean childFirstClassLoader, Set<String> dependencies)
+                                   boolean childFirstClassLoader, Set<String> dependencies, Set<String> optionalDependencies)
   {
     this.scmVersion = scmVersion;
     this.information = information;
@@ -84,6 +85,7 @@ public final class InstalledPluginDescriptor extends ScmModule implements Plugin
     this.condition = condition;
     this.childFirstClassLoader = childFirstClassLoader;
     this.dependencies = dependencies;
+    this.optionalDependencies = optionalDependencies;
   }
 
   //~--- methods --------------------------------------------------------------
@@ -116,7 +118,8 @@ public final class InstalledPluginDescriptor extends ScmModule implements Plugin
       && Objects.equal(information, other.information)
       && Objects.equal(resources, other.resources)
       && Objects.equal(childFirstClassLoader, other.childFirstClassLoader)
-      && Objects.equal(dependencies, other.dependencies);
+      && Objects.equal(dependencies, other.dependencies)
+      && Objects.equal(optionalDependencies, other.optionalDependencies);
   }
 
   /**
@@ -129,7 +132,7 @@ public final class InstalledPluginDescriptor extends ScmModule implements Plugin
   public int hashCode()
   {
     return Objects.hashCode(scmVersion, condition, information, resources,
-      childFirstClassLoader, dependencies);
+      childFirstClassLoader, dependencies, optionalDependencies);
   }
 
   /**
@@ -149,6 +152,7 @@ public final class InstalledPluginDescriptor extends ScmModule implements Plugin
                   .add("resources", resources)
                   .add("childFirstClassLoader", childFirstClassLoader)
                   .add("dependencies", dependencies)
+                  .add("optionalDependencies", optionalDependencies)
                   .toString();
     //J+
   }
@@ -186,6 +190,27 @@ public final class InstalledPluginDescriptor extends ScmModule implements Plugin
     return dependencies;
   }
 
+  /**
+   * Method description
+   *
+   *
+   * @return
+   *
+   * @since 2.0.0
+   */
+  public Set<String> getOptionalDependencies() {
+    if (optionalDependencies == null)
+    {
+      optionalDependencies = ImmutableSet.of();
+    }
+
+    return optionalDependencies;
+  }
+
+  public Set<String> getDependenciesInclusiveOptionals() {
+    return ImmutableSet.copyOf(Iterables.concat(getDependencies(), getOptionalDependencies()));
+  }
+
   /**
    * Method description
    *
@@ -246,6 +271,11 @@ public final class InstalledPluginDescriptor extends ScmModule implements Plugin
   @XmlElementWrapper(name = "dependencies")
   private Set<String> dependencies;
 
+  /** Field description */
+  @XmlElement(name = "dependency")
+  @XmlElementWrapper(name = "optional-dependencies")
+  private Set<String> optionalDependencies;
+
   /** Field description */
   @XmlElement(name = "information")
   private PluginInformation information;

scm-core/src/main/java/sonia/scm/plugin/ScmModule.java

@@ -89,9 +89,9 @@ public class ScmModule
    *
    * @return
    */
-  public Iterable<Class<?>> getExtensions()
+  public Iterable<ExtensionElement> getExtensions()
   {
-    return unwrap(extensions);
+    return nonNull(extensions);
   }
 
   /**
@@ -223,7 +223,7 @@ public class ScmModule
 
   /** Field description */
   @XmlElement(name = "extension")
-  private Set<ClassElement> extensions;
+  private Set<ExtensionElement> extensions;
 
   /** Field description */
   @XmlElement(name = "rest-provider")

scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java

@@ -4,6 +4,7 @@ import com.google.common.base.Preconditions;
 import sonia.scm.repository.Person;
 import sonia.scm.repository.spi.MergeCommand;
 import sonia.scm.repository.spi.MergeCommandRequest;
+import sonia.scm.repository.spi.MergeConflictResult;
 import sonia.scm.repository.util.AuthorUtil;
 
 import java.util.Set;
@@ -168,7 +169,7 @@ public class MergeCommandBuilder {
   }
 
   /**
-   * Use this to check whether the given branches can be merged autmatically. If this is possible,
+   * Use this to check whether the given branches can be merged automatically. If this is possible,
    * {@link MergeDryRunCommandResult#isMergeable()} will return <code>true</code>.
    *
    * @return The result whether the given branches can be merged automatically.
@@ -177,4 +178,14 @@ public class MergeCommandBuilder {
     Preconditions.checkArgument(request.isValid(), "revision to merge and target revision is required");
     return mergeCommand.dryRun(request);
   }
+
+  /**
+   * Use this to compute concrete conflicts for a merge.
+   *
+   * @return A result containing all conflicts for the merge.
+   */
+  public MergeConflictResult conflicts() {
+    Preconditions.checkArgument(request.isValid(), "revision to merge and target revision is required");
+    return mergeCommand.computeConflicts(request);
+  }
 }

scm-core/src/main/java/sonia/scm/repository/spi/DiffCommandRequest.java

@@ -101,7 +101,7 @@ public final class DiffCommandRequest extends FileBaseCommandRequest
    *
    *
    * @param format format of the diff output
-   * 
+   *
    * @since 1.34
    */
   public void setFormat(DiffFormat format)
@@ -119,7 +119,7 @@ public final class DiffCommandRequest extends FileBaseCommandRequest
    *
    *
    * @return output format
-   * 
+   *
    * @since 1.34
    */
   public DiffFormat getFormat()

scm-core/src/main/java/sonia/scm/repository/spi/MergeCommand.java

@@ -17,6 +17,8 @@ public interface MergeCommand {
 
   MergeDryRunCommandResult dryRun(MergeCommandRequest request);
 
+  MergeConflictResult computeConflicts(MergeCommandRequest request);
+
   boolean isSupported(MergeStrategy strategy);
 
   Set<MergeStrategy> getSupportedMergeStrategies();

scm-core/src/main/java/sonia/scm/repository/spi/MergeConflictResult.java

@@ -0,0 +1,63 @@
+package sonia.scm.repository.spi;
+
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
+
+import static sonia.scm.repository.spi.MergeConflictResult.ConflictTypes.ADDED_BY_BOTH;
+import static sonia.scm.repository.spi.MergeConflictResult.ConflictTypes.BOTH_MODIFIED;
+import static sonia.scm.repository.spi.MergeConflictResult.ConflictTypes.DELETED_BY_THEM;
+import static sonia.scm.repository.spi.MergeConflictResult.ConflictTypes.DELETED_BY_US;
+
+public class MergeConflictResult {
+
+  private final List<SingleMergeConflict> conflicts = new LinkedList<>();
+
+  public List<SingleMergeConflict> getConflicts() {
+    return Collections.unmodifiableList(conflicts);
+  }
+
+  public void addBothModified(String path, String diff) {
+    conflicts.add(new SingleMergeConflict(BOTH_MODIFIED, path, diff));
+  }
+
+  public void addDeletedByThem(String path) {
+    conflicts.add(new SingleMergeConflict(DELETED_BY_THEM, path, null));
+  }
+
+  public void addDeletedByUs(String path) {
+    conflicts.add(new SingleMergeConflict(DELETED_BY_US, path, null));
+  }
+
+  public void addAddedByBoth(String path) {
+    conflicts.add(new SingleMergeConflict(ADDED_BY_BOTH, path, null));
+  }
+
+  public static class SingleMergeConflict {
+    private final ConflictTypes type;
+    private final String path;
+    private final String diff;
+
+    private SingleMergeConflict(ConflictTypes type, String path, String diff) {
+      this.type = type;
+      this.path = path;
+      this.diff = diff;
+    }
+
+    public ConflictTypes getType() {
+      return type;
+    }
+
+    public String getPath() {
+      return path;
+    }
+
+    public String getDiff() {
+      return diff;
+    }
+  }
+
+  public enum ConflictTypes {
+    BOTH_MODIFIED, DELETED_BY_THEM, DELETED_BY_US, ADDED_BY_BOTH
+  }
+}

scm-core/src/test/java/sonia/scm/plugin/ScmModuleTest.java

@@ -33,6 +33,7 @@ package sonia.scm.plugin;
 
 //~--- non-JDK imports --------------------------------------------------------
 
+import com.google.common.collect.Iterables;
 import com.google.common.io.Resources;
 
 import org.junit.Test;
@@ -69,43 +70,43 @@ public class ScmModuleTest
 
     //J-
     assertThat(
-      module.getExtensions(), 
+      Iterables.transform(module.getExtensions(), ExtensionElement::getClazz),
       containsInAnyOrder(
-        String.class,
+        String.class.getName(),
-        Integer.class
+        Integer.class.getName()
       )
     );
     assertThat(
-      module.getExtensionPoints(), 
+      module.getExtensionPoints(),
       containsInAnyOrder(
-        new ExtensionPointElement(String.class, "ext01", true, true), 
+        new ExtensionPointElement(String.class, "ext01", true, true),
-        new ExtensionPointElement(Long.class, "ext02", true, true), 
+        new ExtensionPointElement(Long.class, "ext02", true, true),
         new ExtensionPointElement(Integer.class, "ext03", false, true)
       )
     );
     assertThat(
-      module.getEvents(), 
+      module.getEvents(),
       containsInAnyOrder(
         String.class,
         Boolean.class
       )
     );
     assertThat(
-      module.getSubscribers(), 
+      module.getSubscribers(),
       containsInAnyOrder(
-        new SubscriberElement(Long.class, Integer.class, "sub01"), 
+        new SubscriberElement(Long.class, Integer.class, "sub01"),
         new SubscriberElement(Double.class, Float.class, "sub02")
       )
     );
     assertThat(
-      module.getRestProviders(), 
+      module.getRestProviders(),
       containsInAnyOrder(
         Integer.class,
         Long.class
       )
     );
     assertThat(
-      module.getRestResources(), 
+      module.getRestResources(),
       containsInAnyOrder(
         Float.class,
         Double.class

scm-core/src/test/resources/sonia/scm/plugin/module.xml

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8" standalone="no"?>
 <module>
-  
+
   <!-- subscribers -->
   <subscriber>
     <class>java.lang.Long</class>
@@ -47,28 +47,32 @@
 
   <extension>
     <class>java.lang.String</class>
+    <requires>scm-mail-plugin</requires>
+    <requires>scm-review-plugin</requires>
+    <description>ext01</description>
   </extension>
 
   <extension>
     <class>java.lang.Integer</class>
+    <description>ext02</description>
   </extension>
-  
+
   <!-- rest providers -->
-  
+
   <rest-provider>
     <class>java.lang.Integer</class>
   </rest-provider>
-  
+
   <rest-provider>
     <class>java.lang.Long</class>
   </rest-provider>
-  
+
   <!-- jax-rs resources -->
-  
+
   <rest-resource>
     <class>java.lang.Float</class>
   </rest-resource>
-  
+
   <rest-resource>
     <class>java.lang.Double</class>
   </rest-resource>

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitGcTask.java

@@ -81,19 +81,19 @@ public class GitGcTask implements Runnable {
     {
       if (repository.isValid() && repository.isHealthy())
       {
-        logger.info("start git gc for repository {}", repository.getName());
+        logger.info("start git gc for repository {}", repository.getNamespaceAndName());
         Stopwatch sw = Stopwatch.createStarted();
         gc(repository);
-        logger.debug("gc of repository {} has finished after {}", repository.getName(), sw.stop());
+        logger.debug("gc of repository {} has finished after {}", repository.getNamespaceAndName(), sw.stop());
       } 
       else 
       {
-        logger.debug("skip non valid/healthy repository {}", repository.getName());
+        logger.debug("skip non valid/healthy repository {}", repository.getNamespaceAndName());
       }
     } 
     else 
     {
-      logger.trace("skip non git repository {}", repository.getName());
+      logger.trace("skip non git repository {}", repository.getNamespaceAndName());
     }
   }
 

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommand.java

@@ -1,8 +1,17 @@
 package sonia.scm.repository.spi;
 
 import com.google.common.collect.ImmutableSet;
+import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.MergeResult;
+import org.eclipse.jgit.api.Status;
+import org.eclipse.jgit.api.errors.GitAPIException;
+import org.eclipse.jgit.lib.IndexDiff;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.merge.ResolveMerger;
+import org.eclipse.jgit.treewalk.CanonicalTreeParser;
+import org.eclipse.jgit.treewalk.filter.PathFilter;
 import sonia.scm.repository.GitWorkdirFactory;
 import sonia.scm.repository.InternalRepositoryException;
 import sonia.scm.repository.api.MergeCommandResult;
@@ -10,10 +19,13 @@ import sonia.scm.repository.api.MergeDryRunCommandResult;
 import sonia.scm.repository.api.MergeStrategy;
 import sonia.scm.repository.api.MergeStrategyNotSupportedException;
 
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.util.Set;
 
 import static org.eclipse.jgit.merge.MergeStrategy.RECURSIVE;
+import static sonia.scm.ContextEntry.ContextBuilder.entity;
+import static sonia.scm.NotFoundException.notFound;
 
 public class GitMergeCommand extends AbstractGitCommand implements MergeCommand {
 
@@ -35,6 +47,11 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand
     return mergeWithStrategy(request);
   }
 
+  @Override
+  public MergeConflictResult computeConflicts(MergeCommandRequest request) {
+    return inClone(git -> new ConflictWorker(git, request), workdirFactory, request.getTargetBranch());
+  }
+
   private MergeCommandResult mergeWithStrategy(MergeCommandRequest request) {
     switch(request.getMergeStrategy()) {
       case SQUASH:
@@ -75,4 +92,91 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand
     return STRATEGIES;
   }
 
+  private class ConflictWorker extends GitCloneWorker<MergeConflictResult> {
+    private final String theirs;
+    private final String ours;
+    private final CanonicalTreeParser treeParser;
+    private final ObjectId treeId;
+    private final ByteArrayOutputStream diffBuffer;
+
+    private final MergeConflictResult result = new MergeConflictResult();
+
+
+    private ConflictWorker(Git git, MergeCommandRequest request) {
+      super(git, context, repository);
+      theirs = request.getBranchToMerge();
+      ours = request.getTargetBranch();
+
+      treeParser = new CanonicalTreeParser();
+      diffBuffer = new ByteArrayOutputStream();
+      try {
+        treeId = git.getRepository().resolve(ours + "^{tree}");
+      } catch (IOException e) {
+        throw notFound(entity("branch", ours).in(repository));
+      }
+    }
+
+    @Override
+    MergeConflictResult run() throws IOException {
+      MergeResult mergeResult = doTemporaryMerge();
+      if (mergeResult.getConflicts() != null) {
+        getStatus().getConflictingStageState().forEach(this::computeConflict);
+      }
+      return result;
+    }
+
+    private void computeConflict(String path, IndexDiff.StageState stageState) {
+      switch (stageState) {
+        case BOTH_MODIFIED:
+          diffBuffer.reset();
+          try (ObjectReader reader = getClone().getRepository().newObjectReader()) {
+            treeParser.reset(reader, treeId);
+            getClone()
+              .diff()
+              .setOldTree(treeParser)
+              .setPathFilter(PathFilter.create(path))
+              .setOutputStream(diffBuffer)
+              .call();
+            result.addBothModified(path, diffBuffer.toString());
+          } catch (GitAPIException | IOException e) {
+            throw new InternalRepositoryException(repository, "could not calculate diff for path " + path, e);
+          }
+          break;
+        case BOTH_ADDED:
+          result.addAddedByBoth(path);
+          break;
+        case DELETED_BY_THEM:
+          result.addDeletedByUs(path);
+          break;
+        case DELETED_BY_US:
+          result.addDeletedByThem(path);
+          break;
+        default:
+          throw new InternalRepositoryException(context.getRepository(), "unexpected conflict type: " + stageState);
+      }
+    }
+
+    private MergeResult doTemporaryMerge() throws IOException {
+      ObjectId sourceRevision = resolveRevision(theirs);
+      try {
+        return getClone().merge()
+          .setFastForward(org.eclipse.jgit.api.MergeCommand.FastForwardMode.NO_FF)
+          .setCommit(false)
+          .include(theirs, sourceRevision)
+          .call();
+      } catch (GitAPIException e) {
+        throw new InternalRepositoryException(context.getRepository(), "could not merge branch " + theirs + " into " + ours, e);
+      }
+    }
+
+    private Status getStatus() {
+      Status status;
+      try {
+        status = getClone().status().call();
+      } catch (GitAPIException e) {
+        throw new InternalRepositoryException(context.getRepository(), "could not get status", e);
+      }
+      return status;
+    }
+  }
 }

scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitDiffCommandTest.java

@@ -37,6 +37,20 @@ public class GitDiffCommandTest extends AbstractGitCommandTestBase {
     "+++ b/f.txt\n" +
     "@@ -0,0 +1 @@\n" +
     "+f\n";
+  public static final String DIFF_FILE_PARTIAL_MERGE = "diff --git a/a.txt b/a.txt\n" +
+    "index 7898192..8cd63ec 100644\n" +
+    "--- a/a.txt\n" +
+    "+++ b/a.txt\n" +
+    "@@ -1 +1,2 @@\n" +
+    " a\n" +
+    "+change\n" +
+    "diff --git a/b.txt b/b.txt\n" +
+    "index 6178079..09ccdf0 100644\n" +
+    "--- a/b.txt\n" +
+    "+++ b/b.txt\n" +
+    "@@ -1 +1,2 @@\n" +
+    " b\n" +
+    "+change\n";
 
   @Test
   public void diffForOneRevisionShouldCreateDiff() throws IOException {
@@ -91,4 +105,15 @@ public class GitDiffCommandTest extends AbstractGitCommandTestBase {
     gitDiffCommand.getDiffResult(diffCommandRequest).accept(output);
     assertEquals(DIFF_FILE_A_MULTIPLE_REVISIONS, output.toString());
   }
+
+  @Test
+  public void diffBetweenTwoBranchesWithMergedIntegrationBranchShouldCreateDiffOfAllIncomingChanges() throws IOException {
+    GitDiffCommand gitDiffCommand = new GitDiffCommand(createContext(), repository);
+    DiffCommandRequest diffCommandRequest = new DiffCommandRequest();
+    diffCommandRequest.setRevision("partially_merged");
+    diffCommandRequest.setAncestorChangeset("master");
+    ByteArrayOutputStream output = new ByteArrayOutputStream();
+    gitDiffCommand.getDiffResult(diffCommandRequest).accept(output);
+    assertEquals(DIFF_FILE_PARTIAL_MERGE, output.toString());
+  }
 }

scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeCommand_Conflict_Test.java

@@ -0,0 +1,81 @@
+package sonia.scm.repository.spi;
+
+import org.junit.Rule;
+import org.junit.Test;
+import sonia.scm.repository.spi.MergeConflictResult.SingleMergeConflict;
+import sonia.scm.repository.util.WorkdirProvider;
+
+import java.io.IOException;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static sonia.scm.repository.spi.MergeConflictResult.ConflictTypes.BOTH_MODIFIED;
+import static sonia.scm.repository.spi.MergeConflictResult.ConflictTypes.DELETED_BY_THEM;
+import static sonia.scm.repository.spi.MergeConflictResult.ConflictTypes.DELETED_BY_US;
+
+public class GitMergeCommand_Conflict_Test extends AbstractGitCommandTestBase {
+
+  static final String DIFF_HEADER = "diff --git a/Main.java b/Main.java";
+  static final String DIFF_FILE_CONFLICT = "--- a/Main.java\n" +
+    "+++ b/Main.java\n" +
+    "@@ -1,6 +1,13 @@\n" +
+    "+import java.util.Arrays;\n" +
+    "+\n" +
+    " class Main {\n" +
+    "     public static void main(String[] args) {\n" +
+    "         System.out.println(\"Expect nothing more to happen.\");\n" +
+    "+<<<<<<< HEAD\n" +
+    "         System.out.println(\"This is for demonstration, only.\");\n" +
+    "+=======\n" +
+    "+        System.out.println(\"Parameters:\");\n" +
+    "+        Arrays.stream(args).map(arg -> \"- \" + arg).forEach(System.out::println);\n" +
+    "+>>>>>>> feature/print_args\n" +
+    "     }\n" +
+    " }";
+
+  @Rule
+  public BindTransportProtocolRule transportProtocolRule = new BindTransportProtocolRule();
+
+  @Test
+  public void diffBetweenTwoBranchesWithoutConflict() throws IOException {
+    MergeConflictResult result = computeMergeConflictResult("feature/rename_variable", "integration");
+    assertThat(result.getConflicts()).isEmpty();
+  }
+
+  @Test
+  public void diffBetweenTwoBranchesWithSimpleConflict() throws IOException {
+    MergeConflictResult result = computeMergeConflictResult("feature/print_args", "integration");
+    SingleMergeConflict conflict = result.getConflicts().get(0);
+    assertThat(conflict.getType()).isEqualTo(BOTH_MODIFIED);
+    assertThat(conflict.getPath()).isEqualTo("Main.java");
+    assertThat(conflict.getDiff()).contains(DIFF_HEADER, DIFF_FILE_CONFLICT);
+  }
+
+  @Test
+  public void diffBetweenTwoBranchesWithDeletedByUs() throws IOException {
+    MergeConflictResult result = computeMergeConflictResult("feature/remove_class", "integration");
+    SingleMergeConflict conflict = result.getConflicts().get(0);
+    assertThat(conflict.getType()).isEqualTo(DELETED_BY_US);
+    assertThat(conflict.getPath()).isEqualTo("Main.java");
+  }
+
+  @Test
+  public void diffBetweenTwoBranchesWithDeletedByThem() throws IOException {
+    MergeConflictResult result = computeMergeConflictResult("integration", "feature/remove_class");
+    SingleMergeConflict conflict = result.getConflicts().get(0);
+    assertThat(conflict.getType()).isEqualTo(DELETED_BY_THEM);
+    assertThat(conflict.getPath()).isEqualTo("Main.java");
+  }
+
+  private MergeConflictResult computeMergeConflictResult(String branchToMerge, String targetBranch) {
+    GitMergeCommand gitMergeCommand = new GitMergeCommand(createContext(), repository, new SimpleGitWorkdirFactory(new WorkdirProvider()));
+    MergeCommandRequest mergeCommandRequest = new MergeCommandRequest();
+    mergeCommandRequest.setBranchToMerge(branchToMerge);
+    mergeCommandRequest.setTargetBranch(targetBranch);
+    return gitMergeCommand.computeConflicts(mergeCommandRequest);
+  }
+
+  @Override
+  protected String getZippedRepositoryResource() {
+    return "sonia/scm/repository/spi/scm-git-spi-merge-diff-test.zip";
+  }
+}

scm-ui/ui-components/.storybook/config.js

@@ -4,6 +4,8 @@ import { addDecorator, configure } from "@storybook/react";
 import { withI18next } from "storybook-addon-i18next";
 
 import "!style-loader!css-loader!sass-loader!../../ui-styles/src/scm.scss";
+import React, { ReactNode } from "react";
+import { MemoryRouter } from "react-router-dom";
 
 i18n.use(initReactI18next).init({
   whitelist: ["en", "de", "es"],
@@ -28,4 +30,7 @@ addDecorator(
   })
 );
 
+const RoutingDecorator = (story) => <MemoryRouter initialEntries={["/"]}>{story()}</MemoryRouter>;
+addDecorator(RoutingDecorator);
+
 configure(require.context("../src", true, /\.stories\.tsx?$/), module);

scm-ui/ui-components/package.json

@@ -50,13 +50,14 @@
     "event-source-polyfill": "^1.0.9",
     "query-string": "5",
     "react": "^16.8.6",
-    "react-diff-view": "^1.8.1",
+    "react-diff-view": "^2.4.1",
     "react-dom": "^16.8.6",
     "react-i18next": "^10.13.1",
     "react-markdown": "^4.0.6",
     "react-router-dom": "^5.1.2",
     "react-select": "^2.1.2",
-    "react-syntax-highlighter": "^11.0.2"
+    "react-syntax-highlighter": "^11.0.2",
+    "gitdiff-parser": "^0.1.2"
   },
   "babel": {
     "presets": [

scm-ui/ui-components/src/MarkdownView.stories.tsx

@@ -2,7 +2,6 @@ import React from "react";
 import { storiesOf } from "@storybook/react";
 import MarkdownView from "./MarkdownView";
 import styled from "styled-components";
-import { MemoryRouter } from "react-router-dom";
 
 import TestPage from "./__resources__/test-page.md";
 import MarkdownWithoutLang from "./__resources__/markdown-without-lang.md";
@@ -12,7 +11,6 @@ const Spacing = styled.div`
 `;
 
 storiesOf("MarkdownView", module)
-  .addDecorator(story => <MemoryRouter initialEntries={["/"]}>{story()}</MemoryRouter>)
   .add("Default", () => (
     <Spacing>
       <MarkdownView content={TestPage} skipHtml={false} />

scm-ui/ui-components/src/__resources__/Diff.binary.ts

@@ -0,0 +1,18 @@
+export default `diff --git a/Main.java b/Main.java
+index 9b5ca13..7ced845 100644
+--- a/Main.java
++++ b/Main.java
+@@ -1,5 +1,5 @@
+  class Main {
+  -    public static void main(String[] args) {
+  +    public static void main(String[] arguments) {
+    System.out.println("Expect nothing more to happen.");
+  }
+}
+diff --git a/conflict.png b/conflict.png
+new file mode 100644
+index 0000000..7c77c7f
+--- /dev/null
++++ b/conflict.png
+Binary files differ
+`;

scm-ui/ui-components/src/__resources__/Diff.hunks.ts

@@ -0,0 +1,36 @@
+export default `diff --git a/src/main/java/com/cloudogu/scm/review/pullrequest/service/DefaultPullRequestService.java b/src/main/java/com/cloudogu/scm/review/pullrequest/service/DefaultPullRequestService.java
+index 17c35f6..cdf70f1 100644
+--- a/src/main/java/com/cloudogu/scm/review/pullrequest/service/DefaultPullRequestService.java
++++ b/src/main/java/com/cloudogu/scm/review/pullrequest/service/DefaultPullRequestService.java
+@@ -25,6 +25,8 @@ import java.util.Optional;
+ import java.util.Set;
+ import java.util.stream.Collectors;
+ 
++import static com.cloudogu.scm.review.pullrequest.service.PullRequestApprovalEvent.ApprovalCause.APPROVAL_REMOVED;
++import static com.cloudogu.scm.review.pullrequest.service.PullRequestApprovalEvent.ApprovalCause.APPROVED;
+ import static com.cloudogu.scm.review.pullrequest.service.PullRequestStatus.MERGED;
+ import static com.cloudogu.scm.review.pullrequest.service.PullRequestStatus.OPEN;
+ import static com.cloudogu.scm.review.pullrequest.service.PullRequestStatus.REJECTED;
+@@ -200,6 +202,7 @@ public class DefaultPullRequestService implements PullRequestService {
+     PullRequest pullRequest = getPullRequestFromStore(repository, pullRequestId);
+     pullRequest.addApprover(user.getId());
+     getStore(repository).update(pullRequest);
++    eventBus.post(new PullRequestApprovalEvent(repository, pullRequest, APPROVED));
+   }
+ 
+   @Override
+@@ -211,8 +214,12 @@ public class DefaultPullRequestService implements PullRequestService {
+     approver.stream()
+       .filter(recipient -> user.getId().equals(recipient))
+       .findFirst()
+-      .ifPresent(pullRequest::removeApprover);
+-    getStore(repository).update(pullRequest);
++      .ifPresent(
++        approval -> {
++          pullRequest.removeApprover(approval);
++          getStore(repository).update(pullRequest);
++          eventBus.post(new PullRequestApprovalEvent(repository, pullRequest, APPROVAL_REMOVED));
++        });
+   }
+ 
+   @Override`;

scm-ui/ui-components/src/__resources__/Diff.simple.ts

@@ -0,0 +1,171 @@
+export default `diff --git a/src/main/java/com/cloudogu/scm/review/events/EventListener.java b/src/main/java/com/cloudogu/scm/review/events/EventListener.java
+index f889f9c..95e3b10 100644
+--- a/src/main/java/com/cloudogu/scm/review/events/EventListener.java
++++ b/src/main/java/com/cloudogu/scm/review/events/EventListener.java
+@@ -1,20 +1,12 @@
+ package com.cloudogu.scm.review.events;
+ 
+-import com.cloudogu.scm.review.comment.service.BasicComment;
+-import com.cloudogu.scm.review.comment.service.BasicCommentEvent;
+-import com.cloudogu.scm.review.comment.service.CommentEvent;
+-import com.cloudogu.scm.review.comment.service.ReplyEvent;
+ import com.cloudogu.scm.review.pullrequest.service.BasicPullRequestEvent;
+ import com.cloudogu.scm.review.pullrequest.service.PullRequest;
+-import com.cloudogu.scm.review.pullrequest.service.PullRequestEvent;
+ import com.github.legman.Subscribe;
+-import lombok.Data;
+ import org.apache.shiro.SecurityUtils;
+ import org.apache.shiro.subject.PrincipalCollection;
+ import org.apache.shiro.subject.Subject;
+ import sonia.scm.EagerSingleton;
+-import sonia.scm.HandlerEventType;
+-import sonia.scm.event.HandlerEvent;
+ import sonia.scm.plugin.Extension;
+ import sonia.scm.repository.Repository;
+ import sonia.scm.security.SessionId;
+diff --git a/src/main/js/ChangeNotification.tsx b/src/main/js/ChangeNotification.tsx
+index f6d61a9..5f371e4 100644
+--- a/src/main/js/ChangeNotification.tsx
++++ b/src/main/js/ChangeNotification.tsx
+@@ -2,6 +2,7 @@ import React, { FC, useEffect, useState } from "react";
+ import { Link } from "@scm-manager/ui-types";
+ import { apiClient, Toast, ToastButtons, ToastButton } from "@scm-manager/ui-components";
+ import { PullRequest } from "./types/PullRequest";
++import { useTranslation } from "react-i18next";
+ 
+ type HandlerProps = {
+   url: string;
+@@ -15,14 +16,19 @@ const EventNotificationHandler: FC<HandlerProps> = ({ url, reload }) => {
+       pullRequest: setEvent
+     });
+   }, [url]);
++  const { t } = useTranslation("plugins");
+   if (event) {
+     return (
+-      <Toast type="warning" title="New Changes">
+-        <p>The underlying Pull-Request has changed. Press reload to see the changes.</p>
+-        <p>Warning: Non saved modification will be lost.</p>
++      <Toast type="warning" title={t("scm-review-plugin.changeNotification.title")}>
++        <p>{t("scm-review-plugin.changeNotification.description")}</p>
++        <p>{t("scm-review-plugin.changeNotification.modificationWarning")}</p>
+         <ToastButtons>
+-          <ToastButton icon="redo" onClick={reload}>Reload</ToastButton>
+-          <ToastButton icon="times" onClick={() => setEvent(undefined)}>Ignore</ToastButton>
++          <ToastButton icon="redo" onClick={reload}>
++            {t("scm-review-plugin.changeNotification.buttons.reload")}
++          </ToastButton>
++          <ToastButton icon="times" onClick={() => setEvent(undefined)}>
++            {t("scm-review-plugin.changeNotification.buttons.ignore")}
++          </ToastButton>
+         </ToastButtons>
+       </Toast>
+     );
+diff --git a/src/main/resources/locales/de/plugins.json b/src/main/resources/locales/de/plugins.json
+index 80f84a1..2c63ab3 100644
+--- a/src/main/resources/locales/de/plugins.json
++++ b/src/main/resources/locales/de/plugins.json
+@@ -181,6 +181,15 @@
+           "titleClickable": "Der Kommentar bezieht sich auf eine ältere Version des Source- oder Target-Branches. Klicken Sie hier, um den ursprünglichen Kontext zu sehen."
+         }
+       }
++    },
++    "changeNotification": {
++      "title": "Neue Änderungen",
++      "description": "An diesem Pull Request wurden Änderungen vorgenommen. Laden Sie die Seite neu um diese anzuzeigen.",
++      "modificationWarning": "Warnung: Nicht gespeicherte Eingaben gehen verloren.",
++      "buttons": {
++        "reload": "Neu laden",
++        "ignore": "Ignorieren"
++      }
+     }
+   },
+   "permissions": {
+diff --git a/src/main/resources/locales/en/plugins.json b/src/main/resources/locales/en/plugins.json
+index d020fcd..e3c1630 100644
+--- a/src/main/resources/locales/en/plugins.json
++++ b/src/main/resources/locales/en/plugins.json
+@@ -181,6 +181,15 @@
+           "titleClickable": "The comment is related to an older of the source or target branch. Click here to see the original context."
+         }
+       }
++    },
++    "changeNotification": {
++      "title": "New Changes",
++      "description": "The underlying Pull-Request has changed. Press reload to see the changes.",
++      "modificationWarning": "Warning: Non saved modification will be lost.",
++      "buttons": {
++        "reload": "Reload",
++        "ignore": "Ignore"
++      }
+     }
+   },
+   "permissions": {
+diff --git a/src/test/java/com/cloudogu/scm/review/events/ClientTest.java b/src/test/java/com/cloudogu/scm/review/events/ClientTest.java
+index 889cc49..d5a4811 100644
+--- a/src/test/java/com/cloudogu/scm/review/events/ClientTest.java
++++ b/src/test/java/com/cloudogu/scm/review/events/ClientTest.java
+@@ -7,19 +7,16 @@ import org.mockito.Answers;
+ import org.mockito.Mock;
+ import org.mockito.junit.jupiter.MockitoExtension;
+ import sonia.scm.security.SessionId;
++
+ import javax.ws.rs.sse.OutboundSseEvent;
+ import javax.ws.rs.sse.SseEventSink;
+-
+ import java.time.Clock;
+ import java.time.Instant;
+ import java.time.LocalDateTime;
+ import java.time.ZoneOffset;
+ import java.time.temporal.ChronoField;
+-import java.time.temporal.ChronoUnit;
+-import java.time.temporal.TemporalField;
+ import java.util.concurrent.CompletableFuture;
+ import java.util.concurrent.CompletionStage;
+-import java.util.concurrent.atomic.AtomicLong;
+ import java.util.concurrent.atomic.AtomicReference;
+ 
+ import static java.time.temporal.ChronoUnit.MINUTES;
+@@ -83,7 +80,7 @@ class ClientTest {
+ 
+   @Test
+   @SuppressWarnings("unchecked")
+-  void shouldCloseEventSinkOnFailure() throws InterruptedException {
++  void shouldCloseEventSinkOnFailure() {
+     CompletionStage future = CompletableFuture.supplyAsync(() -> {
+       throw new RuntimeException("failed to send message");
+     });
+@@ -91,9 +88,7 @@ class ClientTest {
+ 
+     client.send(message);
+ 
+-    Thread.sleep(50L);
+-
+-    verify(eventSink).close();
++    verify(eventSink, timeout(50L)).close();
+   }
+ 
+   @Test
+diff --git a/Main.java b/Main.java
+index e77e6da..f183b7c 100644
+--- a/Main.java
++++ b/Main.java
+@@ -1,9 +1,18 @@
++import java.io.PrintStream;
+ import java.util.Arrays;
+ 
+ class Main {
++    private static final PrintStream OUT = System.out;
++
+     public static void main(String[] args) {
++<<<<<<< HEAD
+         System.out.println("Expect nothing more to happen.");
+         System.out.println("The command line parameters are:");
+         Arrays.stream(args).map(arg -> "- " + arg).forEach(System.out::println);
++=======
++        OUT.println("Expect nothing more to happen.");
++        OUT.println("Parameters:");
++        Arrays.stream(args).map(arg -> "- " + arg).forEach(OUT::println);
++>>>>>>> feature/use_constant
+     }
+ }
+`;

scm-ui/ui-components/src/buttons/index.stories.tsx

@@ -2,7 +2,6 @@ import React, { ReactNode } from "react";
 import Button from "./Button";
 import { storiesOf } from "@storybook/react";
 import styled from "styled-components";
-import { MemoryRouter } from "react-router-dom";
 import AddButton from "./AddButton";
 import CreateButton from "./CreateButton";
 import DeleteButton from "./DeleteButton";
@@ -17,14 +16,9 @@ const Spacing = styled.div`
   padding: 1em;
 `;
 
-type StoryFn = () => ReactNode;
+const SpacingDecorator = (story: () => ReactNode) => <Spacing>{story()}</Spacing>;
-
-const RoutingDecorator = (story: StoryFn) => <MemoryRouter initialEntries={["/"]}>{story()}</MemoryRouter>;
-
-const SpacingDecorator = (story: StoryFn) => <Spacing>{story()}</Spacing>;
 
 storiesOf("Buttons|Button", module)
-  .addDecorator(RoutingDecorator)
   .add("Colors", () => (
     <div>
       {colors.map(color => (
@@ -44,7 +38,6 @@ storiesOf("Buttons|Button", module)
 
 const buttonStory = (name: string, storyFn: () => ReactElement) => {
   return storiesOf("Buttons|" + name, module)
-    .addDecorator(RoutingDecorator)
     .addDecorator(SpacingDecorator)
     .add("Default", storyFn);
 };
@@ -53,7 +46,7 @@ buttonStory("CreateButton", () => <CreateButton>Create</CreateButton>);
 buttonStory("DeleteButton", () => <DeleteButton>Delete</DeleteButton>);
 buttonStory("DownloadButton", () => <DownloadButton displayName="Download" disabled={false} url="" />).add(
   "Disabled",
-  () => <DownloadButton displayName="Download" disabled={true} url=""></DownloadButton>
+  () => <DownloadButton displayName="Download" disabled={true} url="" />
 );
 buttonStory("EditButton", () => <EditButton>Edit</EditButton>);
 buttonStory("SubmitButton", () => <SubmitButton>Submit</SubmitButton>);

scm-ui/ui-components/src/repos/Diff.stories.tsx

@@ -0,0 +1,60 @@
+import React, { useEffect, useState } from "react";
+import { storiesOf } from "@storybook/react";
+import Diff from "./Diff";
+// @ts-ignore
+import parser from "gitdiff-parser";
+import simpleDiff from "../__resources__/Diff.simple";
+import hunksDiff from "../__resources__/Diff.hunks";
+import binaryDiff from "../__resources__/Diff.binary";
+import Button from "../buttons/Button";
+import { DiffEventContext } from "./DiffTypes";
+import Toast from "../toast/Toast";
+
+const diffFiles = parser.parse(simpleDiff);
+
+storiesOf("Diff", module)
+  .add("Default", () => <Diff diff={diffFiles} />)
+  .add("Side-By-Side", () => <Diff diff={diffFiles} sideBySide={true} />)
+  .add("Collapsed", () => <Diff diff={diffFiles} defaultCollapse={true} />)
+  .add("File Controls", () => <Diff diff={diffFiles} fileControlFactory={() => <Button>Custom Control</Button>} />)
+  .add("File Annotation", () => (
+    <Diff
+      diff={diffFiles}
+      fileAnnotationFactory={file => [<p key={file.newPath}>Custom File annotation for {file.newPath}</p>]}
+    />
+  ))
+  .add("Line Annotation", () => (
+    <Diff
+      diff={diffFiles}
+      annotationFactory={ctx => {
+        return {
+          N2: <p key="N2">Line Annotation</p>
+        };
+      }}
+    />
+  ))
+  .add("OnClick", () => {
+    const OnClickDemo = () => {
+      const [changeId, setChangeId] = useState();
+      useEffect(() => {
+        const interval = setInterval(() => setChangeId(undefined), 2000);
+        return () => clearInterval(interval);
+      });
+      const onClick = (context: DiffEventContext) => setChangeId(context.changeId);
+      return (
+        <>
+          {changeId && <Toast type="info" title={"Change " + changeId} />}
+          <Diff diff={diffFiles} onClick={onClick} />
+        </>
+      );
+    };
+    return <OnClickDemo />;
+  })
+  .add("Hunks", () => {
+    const hunkDiffFiles = parser.parse(hunksDiff);
+    return <Diff diff={hunkDiffFiles} />;
+  })
+  .add("Binaries", () => {
+    const binaryDiffFiles = parser.parse(binaryDiff);
+    return <Diff diff={binaryDiffFiles} />;
+  });

scm-ui/ui-components/src/repos/DiffFile.tsx

@@ -3,11 +3,13 @@ import { withTranslation, WithTranslation } from "react-i18next";
 import classNames from "classnames";
 import styled from "styled-components";
 // @ts-ignore
-import { Change, Diff as DiffComponent, getChangeKey, Hunk } from "react-diff-view";
+import { Diff as DiffComponent, getChangeKey, Hunk, Decoration } from "react-diff-view";
 import { Button, ButtonGroup } from "../buttons";
 import Tag from "../Tag";
 import Icon from "../Icon";
-import { File, Hunk as HunkType, DiffObjectProps } from "./DiffTypes";
+import { ChangeEvent, Change, File, Hunk as HunkType, DiffObjectProps } from "./DiffTypes";
+
+const EMPTY_ANNOTATION_FACTORY = {};
 
 type Props = DiffObjectProps &
   WithTranslation & {
@@ -20,7 +22,7 @@ type Collapsible = {
 };
 
 type State = Collapsible & {
-  sideBySide: boolean;
+  sideBySide?: boolean;
 };
 
 const DiffFilePanel = styled.div`
@@ -56,6 +58,10 @@ const ChangeTypeTag = styled(Tag)`
 `;
 
 const ModifiedDiffComponent = styled(DiffComponent)`
+  /* align line numbers */
+  & .diff-gutter {
+    text-align: right;
+  }
   /* column sizing */
   > colgroup .diff-gutter-col {
     width: 3.25rem;
@@ -80,14 +86,15 @@ const ModifiedDiffComponent = styled(DiffComponent)`
 
 class DiffFile extends React.Component<Props, State> {
   static defaultProps: Partial<Props> = {
-    defaultCollapse: false
+    defaultCollapse: false,
+    markConflicts: true
   };
 
   constructor(props: Props) {
     super(props);
     this.state = {
       collapsed: !!this.props.defaultCollapse,
-      sideBySide: false
+      sideBySide: props.sideBySide
     };
   }
 
@@ -103,7 +110,7 @@ class DiffFile extends React.Component<Props, State> {
 
   toggleCollapse = () => {
     const { file } = this.props;
-    if (file && !file.isBinary) {
+    if (this.hasContent(file)) {
       this.setState(state => ({
         collapsed: !state.collapsed
       }));
@@ -126,7 +133,8 @@ class DiffFile extends React.Component<Props, State> {
     if (i > 0) {
       return <HunkDivider />;
     }
-    return null;
+    // hunk header must be defined
+    return <span />;
   };
 
   collectHunkAnnotations = (hunk: HunkType) => {
@@ -136,6 +144,8 @@ class DiffFile extends React.Component<Props, State> {
         hunk,
         file
       });
+    } else {
+      return EMPTY_ANNOTATION_FACTORY;
     }
   };
 
@@ -152,29 +162,45 @@ class DiffFile extends React.Component<Props, State> {
     }
   };
 
-  createCustomEvents = (hunk: HunkType) => {
+  createGutterEvents = (hunk: HunkType) => {
     const { onClick } = this.props;
     if (onClick) {
       return {
-        gutter: {
+        onClick: (event: ChangeEvent) => {
-          onClick: (change: Change) => {
+          this.handleClickEvent(event.change, hunk);
-            this.handleClickEvent(change, hunk);
-          }
         }
       };
     }
   };
 
   renderHunk = (hunk: HunkType, i: number) => {
-    return (
+    if (this.props.markConflicts && hunk.changes) {
+      this.markConflicts(hunk);
+    }
+    return [
+      <Decoration key={"decoration-" + hunk.content}>{this.createHunkHeader(hunk, i)}</Decoration>,
       <Hunk
-        key={hunk.content}
+        key={"hunk-" + hunk.content}
         hunk={hunk}
-        header={this.createHunkHeader(hunk, i)}
         widgets={this.collectHunkAnnotations(hunk)}
-        customEvents={this.createCustomEvents(hunk)}
+        gutterEvents={this.createGutterEvents(hunk)}
       />
-    );
+    ];
+  };
+
+  markConflicts = (hunk: HunkType) => {
+    let inConflict = false;
+    for (let i = 0; i < hunk.changes.length; ++i) {
+      if (hunk.changes[i].content === "<<<<<<< HEAD") {
+        inConflict = true;
+      }
+      if (inConflict) {
+        hunk.changes[i].type = "conflict";
+      }
+      if (hunk.changes[i].content.startsWith(">>>>>>>")) {
+        inConflict = false;
+      }
+    }
   };
 
   renderFileTitle = (file: File) => {
@@ -215,6 +241,16 @@ class DiffFile extends React.Component<Props, State> {
     return <ChangeTypeTag className={classNames("is-rounded", "has-text-weight-normal")} color={color} label={value} />;
   };
 
+  concat = (array: object[][]) => {
+    if (array.length > 0) {
+      return array.reduce((a, b) => a.concat(b));
+    } else {
+      return [];
+    }
+  };
+
+  hasContent = (file: File) => file && !file.isBinary && file.hunks && file.hunks.length > 0;
+
   render() {
     const { file, fileControlFactory, fileAnnotationFactory, t } = this.props;
     const { collapsed, sideBySide } = this.state;
@@ -228,13 +264,13 @@ class DiffFile extends React.Component<Props, State> {
       body = (
         <div className="panel-block is-paddingless">
           {fileAnnotations}
-          <ModifiedDiffComponent className={viewType} viewType={viewType}>
+          <ModifiedDiffComponent className={viewType} viewType={viewType} hunks={file.hunks} diffType={file.type}>
-            {file.hunks.map(this.renderHunk)}
+            {(hunks: HunkType[]) => this.concat(hunks.map(this.renderHunk))}
           </ModifiedDiffComponent>
         </div>
       );
     }
-    const collapseIcon = file && !file.isBinary ? <Icon name={icon} color="inherit" /> : null;
+    const collapseIcon = this.hasContent(file) ? <Icon name={icon} color="inherit" /> : null;
 
     const fileControls = fileControlFactory ? fileControlFactory(file, this.setCollapse) : null;
     return (

scm-ui/ui-components/src/repos/DiffTypes.ts

@@ -27,7 +27,7 @@ export type Hunk = {
   content: string;
 };
 
-export type ChangeType = "insert" | "delete" | "normal";
+export type ChangeType = "insert" | "delete" | "normal" | "conflict";
 
 export type Change = {
   content: string;
@@ -40,6 +40,10 @@ export type Change = {
   type: ChangeType;
 };
 
+export type ChangeEvent = {
+  change: Change;
+};
+
 export type BaseContext = {
   hunk: Hunk;
   file: File;
@@ -66,9 +70,10 @@ export type DiffEventHandler = (context: DiffEventContext) => void;
 export type FileControlFactory = (file: File, setCollapseState: (p: boolean) => void) => ReactNode | null | undefined;
 
 export type DiffObjectProps = {
-  sideBySide: boolean;
+  sideBySide?: boolean;
   onClick?: DiffEventHandler;
   fileControlFactory?: FileControlFactory;
   fileAnnotationFactory?: FileAnnotationFactory;
   annotationFactory?: AnnotationFactory;
+  markConflicts?: boolean;
 };

scm-ui/ui-components/src/repos/LoadingDiff.tsx

@@ -9,12 +9,13 @@ import Diff from "./Diff";
 import { DiffObjectProps, File } from "./DiffTypes";
 import { NotFoundError } from "../errors";
 import { Notification } from "../index";
-import {withTranslation, WithTranslation} from "react-i18next";
+import { withTranslation, WithTranslation } from "react-i18next";
 
-type Props = WithTranslation & DiffObjectProps & {
+type Props = WithTranslation &
-  url: string;
+  DiffObjectProps & {
-  defaultCollapse?: boolean;
+    url: string;
-};
+    defaultCollapse?: boolean;
+  };
 
 type State = {
   diff?: File[];

scm-ui/ui-components/src/storyshots.test.ts

@@ -1,6 +1,22 @@
 import path from "path";
-import initStoryshots from "@storybook/addon-storyshots";
+import initStoryshots, { snapshotWithOptions } from "@storybook/addon-storyshots";
+
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+const createNodeMock = (element: any) => {
+  if (element.type === "tr") {
+    return {
+      // eslint-disable-next-line @typescript-eslint/no-empty-function
+      querySelector: (selector: string) => {}
+    };
+  }
+  return null;
+};
 
 initStoryshots({
-  configPath: path.resolve(__dirname, "..", ".storybook")
+  configPath: path.resolve(__dirname, "..", ".storybook"),
+  // fix snapshot tests with react-diff-view which uses a ref on tr
+  // @see https://github.com/storybookjs/storybook/pull/1090
+  test: snapshotWithOptions({
+    createNodeMock
+  })
 });

scm-ui/ui-styles/package.json

@@ -13,7 +13,7 @@
     "bulma": "^0.7.5",
     "bulma-popover": "^1.0.0",
     "bulma-tooltip": "^3.0.0",
-    "react-diff-view": "^1.8.1"
+    "react-diff-view": "^2.4.1"
   },
   "devDependencies": {
     "css-loader": "^3.2.0",

scm-ui/ui-styles/src/scm.scss

@@ -423,7 +423,7 @@ $danger-25: scale-color($danger, $lightness: 75%);
 $fa-font-path: "~@fortawesome/fontawesome-free/webfonts";
 @import "~@fortawesome/fontawesome-free/scss/solid";
 
-@import "~react-diff-view/index";
+@import "~react-diff-view/style/index";
 
 // NEW STYLES
 
@@ -829,4 +829,12 @@ form .field:not(.is-grouped) {
   font-weight: 500 !important;
 }
 
+.diff-gutter-conflict {
+  background: $warning-50;
+}
+
+.diff-code-conflict {
+  background: $warning-25;
+}
+
 @import "bulma-popover/css/bulma-popover";

scm-webapp/src/main/java/sonia/scm/plugin/DefaultPluginLoader.java

@@ -99,7 +99,7 @@ public class DefaultPluginLoader implements PluginLoader
 
       modules = getInstalled(parent, context, PATH_MODULECONFIG);
 
-      collector = new ExtensionCollector(Iterables.concat(modules, unwrap()));
+      collector = new ExtensionCollector(parent, modules, installedPlugins);
       extensionProcessor = new DefaultExtensionProcessor(collector);
     }
     catch (IOException | JAXBException ex)
@@ -170,19 +170,6 @@ public class DefaultPluginLoader implements PluginLoader
     return uberWebResourceLoader;
   }
 
-  //~--- methods --------------------------------------------------------------
-
-  /**
-   * Method description
-   *
-   *
-   * @return
-   */
-  private Iterable<InstalledPluginDescriptor> unwrap()
-  {
-    return PluginsInternal.unwrap(installedPlugins);
-  }
-
   //~--- get methods ----------------------------------------------------------
 
   /**

scm-webapp/src/main/java/sonia/scm/plugin/ExplodedSmp.java

@@ -98,8 +98,8 @@ public final class ExplodedSmp implements Comparable<ExplodedSmp>
   {
     int result;
 
-    Set<String> depends = plugin.getDependencies();
+    Set<String> depends = plugin.getDependenciesInclusiveOptionals();
-    Set<String> odepends = o.plugin.getDependencies();
+    Set<String> odepends = o.plugin.getDependenciesInclusiveOptionals();
 
     if (depends.isEmpty() && odepends.isEmpty())
     {

scm-webapp/src/main/java/sonia/scm/plugin/ExtensionBinder.java

@@ -55,63 +55,37 @@ import sonia.scm.util.Util;
 public final class ExtensionBinder
 {
 
-  /** Field description */
   private static final String TYPE_LOOSE_EXT = "loose extension";
-
-  /** Field description */
   private static final String TYPE_REST_RESOURCE = "rest resource";
+  private static final String AS_EAGER_SINGLETON = " as eager singleton";
 
   /**
    * the logger for ExtensionBinder
    */
-  private static final Logger logger =
+  private static final Logger logger = LoggerFactory.getLogger(ExtensionBinder.class);
-    LoggerFactory.getLogger(ExtensionBinder.class);
 
-  //~--- constructors ---------------------------------------------------------
-
-  /**
-   * Constructs ...
-   *
-   *
-   * @param binder
-   */
   public ExtensionBinder(Binder binder)
   {
     this.binder = binder;
   }
 
-  //~--- methods --------------------------------------------------------------
-
-  /**
-   * Method description
-   *
-   *
-   * @param collector
-   */
   public void bind(ExtensionCollector collector)
   {
-    logger.info("bind extensions to extension points");
+    logger.debug("bind extensions to extension points");
 
     for (ExtensionPointElement epe : collector.getExtensionPointElements())
     {
       bindExtensionPoint(collector, epe);
     }
 
-    logger.info("bind loose extensions");
+    logger.debug("bind loose extensions");
     bindLooseExtensions(collector.getLooseExtensions());
-    logger.info("bind rest providers");
+    logger.debug("bind rest providers");
     bindRestProviders(collector.getRestProviders());
-    logger.info("bind rest resources");
+    logger.debug("bind rest resources");
     bindRestResource(collector.getRestResources());
   }
 
-  /**
-   * Method description
-   *
-   *
-   * @param collector
-   * @param epe
-   */
   private void bindExtensionPoint(ExtensionCollector collector,
     ExtensionPointElement epe)
   {
@@ -142,12 +116,6 @@ public final class ExtensionBinder
     }
   }
 
-  /**
-   * Method description
-   *
-   *
-   * @param extensions
-   */
   private void bindLooseExtensions(Iterable<Class> extensions)
   {
     for (Class extension : extensions)
@@ -156,46 +124,27 @@ public final class ExtensionBinder
     }
   }
 
-  /**
+  private void bindMultiExtensionPoint(ExtensionPointElement extensionPoint, Iterable<Class> extensions)
-   * Method description
-   *
-   *
-   *
-   * @param found
-   *
-   * @param extensionPoint
-   *
-   * @param boundClasses
-   * @param extensionPointClass
-   * @param extensions
-   */
-  private void bindMultiExtensionPoint(ExtensionPointElement extensionPoint,
-    Iterable<Class> extensions)
   {
     Class extensionPointClass = extensionPoint.getClazz();
 
-    if (logger.isInfoEnabled())
+    logger.debug("create multibinder for {}", extensionPointClass.getName());
-    {
-      logger.info("create multibinder for {}", extensionPointClass.getName());
-    }
-
-    Multibinder multibinder = Multibinder.newSetBinder(binder,
-                                extensionPointClass);
 
+    Multibinder multibinder = Multibinder.newSetBinder(binder, extensionPointClass);
     for (Class extensionClass : extensions)
     {
       boolean eagerSingleton = isEagerSingleton(extensionClass);
 
-      if (logger.isInfoEnabled())
+      if (logger.isDebugEnabled())
       {
         String as = Util.EMPTY_STRING;
 
         if (eagerSingleton)
         {
-          as = " as eager singleton";
+          as = AS_EAGER_SINGLETON;
         }
 
-        logger.info("bind {} to multibinder of {}{}", extensionClass.getName(),
+        logger.debug("bind {} to multibinder of {}{}", extensionClass.getName(),
           extensionPointClass.getName(), as);
       }
 
@@ -208,27 +157,15 @@ public final class ExtensionBinder
     }
   }
 
-  /**
-   * Method description
-   *
-   *
-   * @param restProviders
-   */
   private void bindRestProviders(Iterable<Class> restProviders)
   {
     for (Class restProvider : restProviders)
     {
-      logger.info("bind rest provider {}", restProvider);
+      logger.debug("bind rest provider {}", restProvider);
       binder.bind(restProvider).in(Singleton.class);
     }
   }
 
-  /**
-   * Method description
-   *
-   *
-   * @param restResources
-   */
   private void bindRestResource(Iterable<Class> restResources)
   {
     for (Class restResource : restResources)
@@ -237,31 +174,22 @@ public final class ExtensionBinder
     }
   }
 
-  /**
-   * Method description
-   *
-   *
-   * @param extensionPointClass
-   *
-   * @param extensionPoint
-   * @param extensionClass
-   */
   private void bindSingleInstance(ExtensionPointElement extensionPoint,
     Class extensionClass)
   {
     Class extensionPointClass = extensionPoint.getClazz();
     boolean eagerSingleton = isEagerSingleton(extensionClass);
 
-    if (logger.isInfoEnabled())
+    if (logger.isDebugEnabled())
     {
       String as = Util.EMPTY_STRING;
 
       if (eagerSingleton)
       {
-        as = " as eager singleton";
+        as = AS_EAGER_SINGLETON;
       }
 
-      logger.info("bind {} to {}{}", extensionClass.getName(),
+      logger.debug("bind {} to {}{}", extensionClass.getName(),
         extensionPointClass.getName(), as);
     }
 
@@ -274,13 +202,6 @@ public final class ExtensionBinder
     }
   }
 
-  /**
-   * Method description
-   *
-   *
-   * @param type
-   * @param extension
-   */
   private void singleBind(String type, Class extension)
   {
     StringBuilder log = new StringBuilder();
@@ -291,30 +212,19 @@ public final class ExtensionBinder
 
     if (isEagerSingleton(extension))
     {
-      log.append(" as eager singleton");
+      log.append(AS_EAGER_SINGLETON);
       abb.asEagerSingleton();
     }
 
-    logger.info(log.toString());
+    if (logger.isDebugEnabled()) {
+      logger.debug(log.toString());
+    }
   }
 
-  //~--- get methods ----------------------------------------------------------
-
-  /**
-   * Method description
-   *
-   *
-   * @param extensionClass
-   *
-   * @return
-   */
   private boolean isEagerSingleton(Class extensionClass)
   {
     return extensionClass.isAnnotationPresent(EagerSingleton.class);
   }
 
-  //~--- fields ---------------------------------------------------------------
-
-  /** Field description */
   private final Binder binder;
 }

scm-webapp/src/main/java/sonia/scm/plugin/ExtensionCollector.java

@@ -39,6 +39,8 @@ import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Multimap;
 import com.google.common.collect.Sets;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 //~--- JDK imports ------------------------------------------------------------
 
@@ -47,6 +49,7 @@ import java.util.Collections;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 /**
  *
@@ -56,23 +59,33 @@ import java.util.Set;
 public final class ExtensionCollector
 {
 
-  /**
+  private static final Logger LOG = LoggerFactory.getLogger(ExtensionCollector.class);
-   * Constructs ...
+
-   *
+  private final Set<String> pluginIndex;
-   *
+
-   * @param modules
+  public ExtensionCollector(ClassLoader moduleClassLoader, Set<ScmModule> modules, Set<InstalledPlugin> installedPlugins) {
-   */
+    this.pluginIndex = createPluginIndex(installedPlugins);
-  ExtensionCollector(Iterable<ScmModule> modules)
+
-  {
+    for (ScmModule module : modules) {
-    for (ScmModule module : modules)
-    {
       collectRootElements(module);
     }
-
+    for (ScmModule plugin : PluginsInternal.unwrap(installedPlugins)) {
-    for (ScmModule module : modules)
+      collectRootElements(plugin);
-    {
-      collectExtensions(module);
     }
+
+    for (ScmModule module : modules) {
+      collectExtensions(moduleClassLoader, module);
+    }
+
+    for (InstalledPlugin plugin : installedPlugins) {
+      collectExtensions(plugin.getClassLoader(), plugin.getDescriptor());
+    }
+  }
+
+  private Set<String> createPluginIndex(Set<InstalledPlugin> installedPlugins) {
+    return installedPlugins.stream()
+      .map(p -> p.getDescriptor().getInformation().getName())
+      .collect(Collectors.toSet());
   }
 
   //~--- methods --------------------------------------------------------------
@@ -245,20 +258,35 @@ public final class ExtensionCollector
     }
   }
 
-  /**
+  private void collectExtensions(ClassLoader defaultClassLoader, ScmModule module) {
-   * Method description
+    for (ExtensionElement extension : module.getExtensions()) {
-   *
+      if (isRequirementFulfilled(extension)) {
-   *
+        Class<?> extensionClass = loadExtension(defaultClassLoader, extension);
-   * @param module
+        appendExtension(extensionClass);
-   */
+      }
-  private void collectExtensions(ScmModule module)
-  {
-    for (Class extension : module.getExtensions())
-    {
-      appendExtension(extension);
     }
   }
 
+  private Class<?> loadExtension(ClassLoader classLoader, ExtensionElement extension) {
+    try {
+      return classLoader.loadClass(extension.getClazz());
+    } catch (ClassNotFoundException ex) {
+      throw new PluginLoadException("failed to load clazz", ex);
+    }
+  }
+
+  private boolean isRequirementFulfilled(ExtensionElement extension) {
+    if (extension.getRequires() != null) {
+      for (String requiredPlugin : extension.getRequires()) {
+        if (!pluginIndex.contains(requiredPlugin)) {
+          LOG.debug("skip loading of extension {}, because the required plugin {} is not installed", extension.getClazz(), requiredPlugin);
+          return false;
+        }
+      }
+    }
+    return true;
+  }
+
   /**
    * Method description
    *

scm-webapp/src/main/java/sonia/scm/plugin/PluginProcessor.java

@@ -189,7 +189,7 @@ public final class PluginProcessor
 
     PluginTree pluginTree = new PluginTree(smps);
 
-    logger.trace("build plugin tree: {}", pluginTree);
+    logger.info("install plugin tree:\n{}", pluginTree);
 
     List<PluginNode> rootNodes = pluginTree.getRootNodes();
 

scm-webapp/src/main/java/sonia/scm/plugin/PluginTree.java

@@ -35,15 +35,13 @@ package sonia.scm.plugin;
 
 import com.google.common.collect.Lists;
 import com.google.common.collect.Ordering;
-
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-//~--- JDK imports ------------------------------------------------------------
-
 import java.util.Arrays;
 import java.util.List;
-import java.util.Set;
+
+//~--- JDK imports ------------------------------------------------------------
 
 /**
  *
@@ -104,9 +102,7 @@ public final class PluginTree
 
       if ((condition == null) || condition.isSupported())
       {
-        Set<String> dependencies = plugin.getDependencies();
+        if (plugin.getDependencies().isEmpty() && plugin.getOptionalDependencies().isEmpty())
-
-        if ((dependencies == null) || dependencies.isEmpty())
         {
           rootNodes.add(new PluginNode(smp));
         }
@@ -170,6 +166,20 @@ public final class PluginTree
         //J+
       }
     }
+
+    boolean rootNode = smp.getPlugin().getDependencies().isEmpty();
+    for (String dependency : smp.getPlugin().getOptionalDependencies()) {
+      if (appendNode(rootNodes, child, dependency)) {
+        rootNode = false;
+      } else {
+        logger.info("optional dependency {} of {} is not installed", dependency, child.getId());
+      }
+    }
+
+    if (rootNode) {
+      logger.info("could not find optional dependencies of {}, append it as root node", child.getId());
+      rootNodes.add(new PluginNode(smp));
+    }
   }
 
   /**
@@ -212,7 +222,18 @@ public final class PluginTree
 
   @Override
   public String toString() {
-    return "plugin tree: " + rootNodes.toString();
+    StringBuilder buffer = new StringBuilder();
+    for (PluginNode node : rootNodes) {
+      append(buffer, "", node);
+    }
+    return buffer.toString();
+  }
+
+  private void append(StringBuilder buffer, String indent, PluginNode node) {
+    buffer.append(indent).append("+- ").append(node.getId()).append("\n");
+    for (PluginNode child : node.getChildren()) {
+      append(buffer, indent + "   ", child);
+    }
   }
 //~--- fields ---------------------------------------------------------------
 

scm-webapp/src/test/java/sonia/scm/plugin/ExplodedSmpTest.java

@@ -134,7 +134,7 @@ public class ExplodedSmpTest
     info.setVersion(version);
 
     InstalledPluginDescriptor plugin = new InstalledPluginDescriptor(2, info, null, null, false,
-                      Sets.newSet(dependencies));
+                      Sets.newSet(dependencies), null);
 
     return new ExplodedSmp(null, plugin);
   }

scm-webapp/src/test/java/sonia/scm/plugin/PluginTreeTest.java

@@ -34,6 +34,7 @@ package sonia.scm.plugin;
 //~--- non-JDK imports --------------------------------------------------------
 
 import com.google.common.base.Function;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 
 import org.junit.Rule;
@@ -72,7 +73,7 @@ public class PluginTreeTest
     PluginCondition condition = new PluginCondition("999",
                                   new ArrayList<String>(), "hit");
     InstalledPluginDescriptor plugin = new InstalledPluginDescriptor(2, createInfo("a",  "1"), null, condition,
-                      false, null);
+                      false, null, null);
     ExplodedSmp smp = createSmp(plugin);
 
     new PluginTree(smp).getRootNodes();
@@ -115,7 +116,7 @@ public class PluginTreeTest
   public void testScmVersion() throws IOException
   {
     InstalledPluginDescriptor plugin = new InstalledPluginDescriptor(1, createInfo("a", "1"), null, null, false,
-                      null);
+                      null, null);
     ExplodedSmp smp = createSmp(plugin);
 
     new PluginTree(smp).getRootNodes();
@@ -131,10 +132,10 @@ public class PluginTreeTest
   public void testSimpleDependencies() throws IOException
   {
     //J-
-    ExplodedSmp[] smps = new ExplodedSmp[] { 
+    ExplodedSmp[] smps = new ExplodedSmp[] {
       createSmpWithDependency("a"),
       createSmpWithDependency("b", "a"),
-      createSmpWithDependency("c", "a", "b") 
+      createSmpWithDependency("c", "a", "b")
     };
     //J+
 
@@ -152,6 +153,61 @@ public class PluginTreeTest
     assertThat(unwrapIds(b.getChildren()), containsInAnyOrder("c"));
   }
 
+  @Test
+  public void testWithOptionalDependency() throws IOException {
+    ExplodedSmp[] smps = new ExplodedSmp[] {
+      createSmpWithDependency("a"),
+      createSmpWithDependency("b", null, ImmutableSet.of("a")),
+      createSmpWithDependency("c", null, ImmutableSet.of("a", "b"))
+    };
+
+    PluginTree tree = new PluginTree(smps);
+    List<PluginNode> rootNodes = tree.getRootNodes();
+
+    assertThat(unwrapIds(rootNodes), containsInAnyOrder("a"));
+
+    PluginNode a = rootNodes.get(0);
+    assertThat(unwrapIds(a.getChildren()), containsInAnyOrder("b", "c"));
+
+    PluginNode b = a.getChild("b");
+    assertThat(unwrapIds(b.getChildren()), containsInAnyOrder("c"));
+  }
+
+  @Test
+  public void testWithDeepOptionalDependency() throws IOException {
+    ExplodedSmp[] smps = new ExplodedSmp[] {
+      createSmpWithDependency("a"),
+      createSmpWithDependency("b", "a"),
+      createSmpWithDependency("c", null, ImmutableSet.of("b"))
+    };
+
+    PluginTree tree = new PluginTree(smps);
+
+    System.out.println(tree);
+
+    List<PluginNode> rootNodes = tree.getRootNodes();
+
+    assertThat(unwrapIds(rootNodes), containsInAnyOrder("a"));
+
+    PluginNode a = rootNodes.get(0);
+    assertThat(unwrapIds(a.getChildren()), containsInAnyOrder("b"));
+
+    PluginNode b = a.getChild("b");
+    assertThat(unwrapIds(b.getChildren()), containsInAnyOrder("c"));
+  }
+
+  @Test
+  public void testWithNonExistentOptionalDependency() throws IOException {
+    ExplodedSmp[] smps = new ExplodedSmp[] {
+      createSmpWithDependency("a", null, ImmutableSet.of("b"))
+    };
+
+    PluginTree tree = new PluginTree(smps);
+    List<PluginNode> rootNodes = tree.getRootNodes();
+
+    assertThat(unwrapIds(rootNodes), containsInAnyOrder("a"));
+  }
+
   /**
    * Method description
    *
@@ -200,7 +256,7 @@ public class PluginTreeTest
   private ExplodedSmp createSmp(String name) throws IOException
   {
     return createSmp(new InstalledPluginDescriptor(2, createInfo(name, "1.0.0"), null, null,
-      false, null));
+      false, null, null));
   }
 
   /**
@@ -224,10 +280,19 @@ public class PluginTreeTest
     {
       dependencySet.add(d);
     }
+    return createSmpWithDependency(name, dependencySet, null);
+  }
 
-    InstalledPluginDescriptor plugin = new InstalledPluginDescriptor(2, createInfo(name, "1"), null, null,
+  private ExplodedSmp createSmpWithDependency(String name, Set<String> dependencies, Set<String> optionalDependencies) throws IOException {
-                      false, dependencySet);
+    InstalledPluginDescriptor plugin = new InstalledPluginDescriptor(
-
+      2,
+      createInfo(name, "1"),
+      null,
+      null,
+      false,
+      dependencies,
+      optionalDependencies
+    );
     return createSmp(plugin);
   }