Merge pull request #3256 from daniel-beck/better-dependency-errors

Separate original and downstream dependency errors

core/src/main/java/hudson/PluginWrapper.java

@@ -37,6 +37,9 @@ import hudson.model.UpdateCenter;
 import hudson.model.UpdateSite;
 import hudson.util.VersionNumber;
 import org.jvnet.localizer.ResourceBundleHolder;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.DoNotUse;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.HttpResponses;
 import org.kohsuke.stapler.StaplerRequest;
@@ -66,10 +69,12 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Predicate;
 import java.util.jar.JarFile;
 import java.util.jar.Manifest;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.stream.Collectors;
 
 import static java.util.logging.Level.WARNING;
 import static org.apache.commons.io.FilenameUtils.getBaseName;
@@ -159,10 +164,34 @@ public class PluginWrapper implements Comparable<PluginWrapper>, ModelObject {
     private final List<Dependency> optionalDependencies;
 
     public List<String> getDependencyErrors() {
-        return Collections.unmodifiableList(dependencyErrors);
+        return Collections.unmodifiableList(new ArrayList<>(dependencyErrors.keySet()));
     }
 
-    private final transient List<String> dependencyErrors = new ArrayList<>();
+    @Restricted(NoExternalUse.class) // Jelly use
+    public List<String> getOriginalDependencyErrors() {
+        Predicate<Map.Entry<String, Boolean>> p = Map.Entry::getValue;
+        return dependencyErrors.entrySet().stream().filter(p.negate()).map(Map.Entry::getKey).collect(Collectors.toList());
+    }
+
+    @Restricted(NoExternalUse.class) // Jelly use
+    public boolean hasOriginalDependencyErrors() {
+        return !getOriginalDependencyErrors().isEmpty();
+    }
+
+    @Restricted(NoExternalUse.class) // Jelly use
+    public List<String> getDerivedDependencyErrors() {
+        return dependencyErrors.entrySet().stream().filter(Map.Entry::getValue).map(Map.Entry::getKey).collect(Collectors.toList());
+    }
+
+    @Restricted(NoExternalUse.class) // Jelly use
+    public boolean hasDerivedDependencyErrors() {
+        return !getDerivedDependencyErrors().isEmpty();
+    }
+
+    /**
+     * A String error message, and a boolean indicating whether it's an original error (false) or downstream from an original one (true)
+     */
+    private final transient Map<String, Boolean> dependencyErrors = new HashMap<>();
 
     /**
      * Is this plugin bundled in jenkins.war?
@@ -571,7 +600,7 @@ public class PluginWrapper implements Comparable<PluginWrapper>, ModelObject {
             } else {
                 VersionNumber actualVersion = Jenkins.getVersion();
                 if (actualVersion.isOlderThan(new VersionNumber(requiredCoreVersion))) {
-                    dependencyErrors.add(Messages.PluginWrapper_obsoleteCore(Jenkins.getVersion().toString(), requiredCoreVersion));
+                    dependencyErrors.put(Messages.PluginWrapper_obsoleteCore(Jenkins.getVersion().toString(), requiredCoreVersion), false);
                 }
             }
         }
@@ -581,21 +610,21 @@ public class PluginWrapper implements Comparable<PluginWrapper>, ModelObject {
             if (dependency == null) {
                 PluginWrapper failedDependency = NOTICE.getPlugin(d.shortName);
                 if (failedDependency != null) {
-                    dependencyErrors.add(Messages.PluginWrapper_failed_to_load_dependency(failedDependency.getLongName(), failedDependency.getVersion()));
+                    dependencyErrors.put(Messages.PluginWrapper_failed_to_load_dependency(failedDependency.getLongName(), failedDependency.getVersion()), true);
                     break;
                 } else {
-                    dependencyErrors.add(Messages.PluginWrapper_missing(d.shortName, d.version));
+                    dependencyErrors.put(Messages.PluginWrapper_missing(d.shortName, d.version), false);
                 }
             } else {
                 if (dependency.isActive()) {
                     if (isDependencyObsolete(d, dependency)) {
-                        dependencyErrors.add(Messages.PluginWrapper_obsolete(dependency.getLongName(), dependency.getVersion(), d.version));
+                        dependencyErrors.put(Messages.PluginWrapper_obsolete(dependency.getLongName(), dependency.getVersion(), d.version), false);
                     }
                 } else {
                     if (isDependencyObsolete(d, dependency)) {
-                        dependencyErrors.add(Messages.PluginWrapper_disabledAndObsolete(dependency.getLongName(), dependency.getVersion(), d.version));
+                        dependencyErrors.put(Messages.PluginWrapper_disabledAndObsolete(dependency.getLongName(), dependency.getVersion(), d.version), false);
                     } else {
-                        dependencyErrors.add(Messages.PluginWrapper_disabled(dependency.getLongName()));
+                        dependencyErrors.put(Messages.PluginWrapper_disabled(dependency.getLongName()), false);
                     }
                 }
 
@@ -606,7 +635,7 @@ public class PluginWrapper implements Comparable<PluginWrapper>, ModelObject {
             PluginWrapper dependency = parent.getPlugin(d.shortName);
             if (dependency != null && dependency.isActive()) {
                 if (isDependencyObsolete(d, dependency)) {
-                    dependencyErrors.add(Messages.PluginWrapper_obsolete(dependency.getLongName(), dependency.getVersion(), d.version));
+                    dependencyErrors.put(Messages.PluginWrapper_obsolete(dependency.getLongName(), dependency.getVersion(), d.version), false);
                 } else {
                     dependencies.add(d);
                 }
@@ -616,7 +645,7 @@ public class PluginWrapper implements Comparable<PluginWrapper>, ModelObject {
             NOTICE.addPlugin(this);
             StringBuilder messageBuilder = new StringBuilder();
             messageBuilder.append(Messages.PluginWrapper_failed_to_load_plugin(getLongName(), getVersion())).append(System.lineSeparator());
-            for (Iterator<String> iterator = dependencyErrors.iterator(); iterator.hasNext(); ) {
+            for (Iterator<String> iterator = dependencyErrors.keySet().iterator(); iterator.hasNext(); ) {
                 String dependencyError = iterator.next();
                 messageBuilder.append(" - ").append(dependencyError);
                 if (iterator.hasNext()) {
@@ -750,6 +779,11 @@ public class PluginWrapper implements Comparable<PluginWrapper>, ModelObject {
             return !plugins.isEmpty();
         }
 
+        @Restricted(DoNotUse.class) // Jelly
+        public boolean hasAnyDerivedDependencyErrors() {
+            return plugins.values().stream().anyMatch(PluginWrapper::hasDerivedDependencyErrors);
+        }
+
         @Override
         public String getDisplayName() {
             return Messages.PluginWrapper_PluginWrapperAdministrativeMonitor_DisplayName();

core/src/main/resources/hudson/PluginWrapper/PluginWrapperAdministrativeMonitor/message.jelly

@@ -4,14 +4,31 @@
     <form method="post" action="${rootURL}/${it.url}/act" name="${it.id}">
       <f:submit name="correct" value="${%Correct}"/>
     </form>
-    ${%Dependency errors}
+    <p><strong>${%Dependency errors}:</strong></p>
+    <p>${%blurbOriginal}</p>
     <j:forEach items="${it.plugins}" var="plugin">
-    <dl>
-      <dt>${plugin.longName} v${plugin.version}</dt>
-      <j:forEach items="${plugin.dependencyErrors}" var="d">
-        <dd>${d}</dd>
-      </j:forEach>
-    </dl>
+      <j:if test="${plugin.hasOriginalDependencyErrors()}">
+        <dl>
+          <dt>${plugin.longName} version ${plugin.version}</dt>
+          <j:forEach items="${plugin.originalDependencyErrors}" var="d">
+            <dd>${d}</dd>
+          </j:forEach>
+        </dl>
+      </j:if>
     </j:forEach>
+    <j:if test="${it.hasAnyDerivedDependencyErrors()}">
+      <p><strong>${%Downstream dependency errors}:</strong></p>
+      <p>${%blurbDerived}</p>
+      <dl>
+        <j:forEach items="${it.plugins}" var="plugin">
+          <j:if test="${plugin.hasDerivedDependencyErrors()}">
+            <dt>${plugin.displayName} version ${plugin.version}</dt>
+            <j:forEach items="${plugin.derivedDependencyErrors}" var="d">
+              <dd>${d}</dd>
+            </j:forEach>
+          </j:if>
+        </j:forEach>
+      </dl>
+    </j:if>
   </div>
 </j:jelly>

core/src/main/resources/hudson/PluginWrapper/PluginWrapperAdministrativeMonitor/message.properties

@@ -19,4 +19,5 @@
 # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 # THE SOFTWARE.
-Dependency\ errors=There are dependency errors loading some plugins.
+blurbOriginal=Some plugins could not be loaded due to unsatisfied dependencies. Fix these issues and restart Jenkins to restore the functionality provided by these plugins.
+blurbDerived=These plugins failed to load because of one or more of the errors above. Fix those and these plugins will load again.