Describable will give a warning on multiple implementations of a symbol (#674)

Author: dgarzon

plugin/src/main/java/io/jenkins/plugins/casc/ConfigurationContext.java

@@ -67,13 +67,13 @@ public RootElementConfigurator lookupRootElement(String name) {
 
     @Override
     @NonNull
-    public Configurator lookupOrFail(Type type) throws ConfiguratorException {
+    public <T> Configurator<T> lookupOrFail(Type type) throws ConfiguratorException {
         return registry.lookupOrFail(type);
     }
 
     @Override
     @CheckForNull
-    public Configurator lookup(Type type) {
+    public <T> Configurator<T> lookup(Type type) {
         return registry.lookup(type);
     }
 

plugin/src/main/java/io/jenkins/plugins/casc/Configurator.java

@@ -96,11 +96,11 @@ default Class getImplementedAPI() {
 
 
     /**
-     * @return list of {@link }Configurator}s to be considered so one can fully configure this component.
+     * @return list of {@link Configurator<T>}s to be considered so one can fully configure this component.
      * Typically, configurator for an abstract extension point will return Configurators for available implementations.
      */
     @NonNull
-    default List<Configurator> getConfigurators(ConfigurationContext context) {
+    default List<Configurator<T>> getConfigurators(ConfigurationContext context) {
         return Collections.singletonList(this);
     }
 

plugin/src/main/java/io/jenkins/plugins/casc/impl/configurators/HeteroDescribableConfigurator.java

@@ -1,5 +1,8 @@
 package io.jenkins.plugins.casc.impl.configurators;
 
+import static io.jenkins.plugins.casc.model.CNode.Type.MAPPING;
+import static io.vavr.API.unchecked;
+
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.model.Describable;
@@ -9,12 +12,16 @@
 import io.jenkins.plugins.casc.Attribute;
 import io.jenkins.plugins.casc.ConfigurationContext;
 import io.jenkins.plugins.casc.Configurator;
-import io.jenkins.plugins.casc.ConfiguratorException;
 import io.jenkins.plugins.casc.ObsoleteConfigurationMonitor;
 import io.jenkins.plugins.casc.impl.attributes.DescribableAttribute;
 import io.jenkins.plugins.casc.model.CNode;
 import io.jenkins.plugins.casc.model.Mapping;
 import io.jenkins.plugins.casc.model.Scalar;
+import io.vavr.Tuple;
+import io.vavr.Tuple2;
+import io.vavr.collection.HashMap;
+import io.vavr.collection.Stream;
+import io.vavr.control.Option;
 import jenkins.model.Jenkins;
 import org.jenkinsci.Symbol;
 import org.kohsuke.accmod.Restricted;
@@ -24,8 +31,8 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Predicate;
 import java.util.logging.Logger;
-import java.util.stream.Collectors;
 
 /**
  * {@link Configurator} that works with {@link Describable} subtype as a {@link #target}.
@@ -42,11 +49,10 @@
  * @author <a href="mailto:[email protected]">Nicolas De Loof</a>
  */
 @Restricted(NoExternalUse.class)
-public class HeteroDescribableConfigurator<T extends Describable> implements Configurator<T> {
+public class HeteroDescribableConfigurator<T extends Describable<T>> implements Configurator<T> {
 
     private static final Logger LOGGER = Logger.getLogger(HeteroDescribableConfigurator.class.getName());
 
-
     private final Class<T> target;
 
     public HeteroDescribableConfigurator(Class<T> clazz) {
@@ -59,119 +65,167 @@ public Class<T> getTarget() {
     }
 
     @NonNull
-    public List<Configurator> getConfigurators(ConfigurationContext context) {
-        final List<Descriptor> candidates = Jenkins.getInstance().getDescriptorList(target);
-        final List<Configurator> configurators = candidates.stream()
-                .map(d -> context.lookup(d.getKlass().toJavaClass()))
-                .filter(c -> c != null)
-                .collect(Collectors.toList());
-        configurators.add(this);
-        return configurators;
+    @Override
+    public List<Configurator<T>> getConfigurators(ConfigurationContext context) {
+        return getDescriptors()
+            .flatMap(d -> lookupConfigurator(context, descriptorClass(d)))
+            .append(this)
+            .toJavaList();
     }
 
     @NonNull
     @Override
-    public T configure(CNode config, ConfigurationContext context) throws ConfiguratorException {
-        String shortname;
-        CNode subconfig = null;
-        switch (config.getType()) {
-            case SCALAR:
-                shortname = config.asScalar().getValue();
-                break;
-            case MAPPING:
-                Mapping map = config.asMapping();
-                if (map.size() != 1) {
-                    throw new IllegalArgumentException("single entry map expected to configure a "+target.getName());
-                }
-                final Map.Entry<String, CNode> next = map.entrySet().iterator().next();
-                shortname = next.getKey();
-                subconfig = next.getValue();
-                break;
-            default:
-                throw new IllegalArgumentException("Unexpected configuration type "+config);
-        }
+    public T configure(CNode config, ConfigurationContext context) {
+        return preConfigure(config).apply((shortName, subConfig) ->
+            lookupDescriptor(shortName, config)
+                .map(descriptor -> forceLookupConfigurator(context, descriptor))
+                .map(configurator -> doConfigure(context, configurator, subConfig.getOrNull())))
+            .getOrNull();
+    }
 
-        final List<Descriptor> candidates = Jenkins.getInstance().getDescriptorList(target);
+    @Override
+    public T check(CNode config, ConfigurationContext context) {
+        return configure(config, context);
+    }
 
-        Class<? extends T> k = findDescribableBySymbol(config, shortname, candidates);
-        final Configurator<T> configurator = context.lookup(k);
-        if (configurator == null) throw new IllegalStateException("No configurator implementation to manage "+k);
-        return configurator.configure(subconfig, context);
+    @NonNull
+    @Override
+    public Set<Attribute<T, ?>> describe() {
+        return Collections.emptySet();
     }
 
+    @CheckForNull
     @Override
-    public T check(CNode config, ConfigurationContext context) throws ConfiguratorException {
-        return configure(config, context);
+    public CNode describe(T instance, ConfigurationContext context) {
+        Predicate<CNode> isScalar = node -> node.getType().equals(MAPPING) && unchecked(node::asMapping).apply().size() == 0;
+        return lookupConfigurator(context, instance.getClass())
+                .map(configurator -> convertToNode(context, configurator, instance))
+                .map(node -> {
+                    if (isScalar.test(node)) {
+                        return new Scalar(preferredSymbol(instance.getDescriptor()));
+                    } else {
+                        return new Mapping().put(preferredSymbol(instance.getDescriptor()), node);
+                    }
+                }).getOrNull();
+    }
+
+    @SuppressWarnings("unused")
+    public Map<String, Class<T>> getImplementors() {
+        return getDescriptors()
+                .map(descriptor -> Tuple.of(preferredSymbol(descriptor), descriptor))
+                .foldLeft(HashMap.empty(), this::handleDuplicateSymbols)
+                .mapValues(this::descriptorClass)
+                .toJavaMap();
+    }
+
+    @SuppressWarnings("unchecked")
+    private Option<Configurator<T>> lookupConfigurator(ConfigurationContext context, Class<?> descriptor) {
+        return Option.of(context.lookup(descriptor));
     }
 
-    public Map<String, Class> getImplementors() {
-        final Class api = getImplementedAPI();
-        final List<Descriptor> descriptors = Jenkins.getInstance().getDescriptorList(target);
-        return descriptors.stream()
-            .collect(Collectors.toMap(
-                    d -> DescribableAttribute.getSymbols(d, api, target).get(0),
-                    d -> d.getKlass().toJavaClass()));
+    private Configurator<T> forceLookupConfigurator(ConfigurationContext context, Descriptor<T> descriptor) {
+        Class<T> klazz = descriptorClass(descriptor);
+        return lookupConfigurator(context, klazz)
+                .getOrElseThrow(() -> new IllegalStateException("No configurator implementation to manage " + klazz));
     }
 
+    private Stream<Descriptor<T>> getDescriptors() {
+        return Stream.ofAll(Jenkins.getInstance().getDescriptorList(target));
+    }
 
-    /**
-     *
-     * @return true if the support plugin is installed, false otherwise.
-     */
-    private boolean _hasSupportPluginInstalled() {
-        return Jenkins.getInstance().getPlugin("configuration-as-code-support") != null;
+    @SuppressWarnings("unchecked")
+    private Class<T> descriptorClass(Descriptor<T> descriptor) {
+        return descriptor.getKlass().toJavaClass();
     }
 
-    private Class<T> findDescribableBySymbol(CNode node, String shortname, List<Descriptor> candidates) {
+    private Option<Descriptor<T>> lookupDescriptor(String symbol, CNode config) {
+        return getDescriptors()
+                .filter(descriptor -> findByPreferredSymbol(descriptor, symbol) || findBySymbols(descriptor, symbol, config))
+                .map(descriptor -> Tuple.of(preferredSymbol(descriptor), descriptor))
+                .foldLeft(HashMap.empty(), this::handleDuplicateSymbols)
+                .values()
+                .headOption()
+                .orElse(() -> {
+                    String message = "No " + target.getName() + " implementation found for " + symbol;
+                    String possible = lookupPlugin("configuration-as-code-support") ? "" : System.lineSeparator() + "Possible solution: Try to install 'configuration-as-code-support' plugin";
+                    throw new IllegalArgumentException(Stream.of(message, possible)
+                            .intersperse("")
+                            .foldLeft(new StringBuilder(), StringBuilder::append)
+                            .toString());
+                });
+    }
 
-        final Class api = getImplementedAPI();
-        // Search for @Symbol annotation on Descriptor to match shortName
+    private Boolean findByPreferredSymbol(Descriptor<T> descriptor, String symbol) {
+        return preferredSymbol(descriptor).equalsIgnoreCase(symbol);
+    }
 
-        for (Descriptor d : candidates) {
-            final List<String> symbols = DescribableAttribute.getSymbols(d, api, target);
-            final String preferred = symbols.get(0);
-            if (preferred.equalsIgnoreCase(shortname)) {
-                return d.getKlass().toJavaClass();
-            }
+    private Boolean findBySymbols(Descriptor<T> descriptor, String symbol, CNode node) {
+        return getSymbols(descriptor)
+                .find(actual -> actual.equalsIgnoreCase(symbol))
+                .map(actual -> {
+                    ObsoleteConfigurationMonitor.get().record(node, "'" + symbol + "' is obsolete, please use '" + preferredSymbol(descriptor) + "'");
+                    return descriptorClass(descriptor);
+                }).isDefined();
+    }
+
+    private Stream<String> getSymbols(Descriptor<T> descriptor) {
+        return Stream.ofAll(DescribableAttribute.getSymbols(descriptor, target, target));
+    }
+
+    private String preferredSymbol(Descriptor<?> descriptor) {
+        return DescribableAttribute.getPreferredSymbol(descriptor, target, target);
+    }
+
+    private Boolean lookupPlugin(String name) {
+        return Option.of(Jenkins.getInstance().getPlugin(name)).isDefined();
+    }
+
+    private HashMap<String, Descriptor<T>> handleDuplicateSymbols(HashMap<String, Descriptor<T>> r, Tuple2<String, Descriptor<T>> t) {
+        if (r.containsKey(t._1)) {
+            String message = String.format("Found multiple implementations for symbol = %s: [%s, %s]. Please report to plugin maintainer.", t._1, r.get(t._1).get(), t._2);
+            LOGGER.warning(message);
+            return r;
+        } else {
+            return r.put(t);
         }
+    }
 
-        // Not found by preferred symbol, try other ones
-        for (Descriptor d : candidates) {
-            final List<String> symbols = DescribableAttribute.getSymbols(d, api, target);
-            for (String symbol : symbols) {
-                if (symbol.equalsIgnoreCase(shortname)) {
-                    ObsoleteConfigurationMonitor.get().record(node, "'"+shortname+"' is obsolete, please use '" + symbols.get(0) + "'");
-                    return d.getKlass().toJavaClass();
-                }
-            }
+    private Tuple2<String, Option<CNode>> preConfigure(CNode config) {
+        switch (config.getType()) {
+            case SCALAR:
+                return configureScalar(config);
+            case MAPPING:
+                return configureMapping(config);
+            default:
+                return configureUnexpected(config);
         }
+    }
 
-        final String errSupport = !_hasSupportPluginInstalled() ? "\nPossible solution: Try to install 'configuration-as-code-support' plugin" : "";
-        final String msg = "No "+target.getName()+ " implementation found for "+shortname;
-        throw new IllegalArgumentException(String.format("%s%s", msg, errSupport));
+    private Tuple2<String, Option<CNode>> configureUnexpected(CNode config) {
+        throw new IllegalArgumentException("Unexpected configuration type " + config);
     }
 
-    @NonNull
-    @Override
-    public Set<Attribute<T,?>> describe() {
-        return Collections.emptySet();
+    private Tuple2<String, Option<CNode>> configureScalar(CNode config) {
+        Scalar scalar = unchecked(config::asScalar).apply();
+        return Tuple.of(scalar.getValue(), Option.none());
     }
 
-    @CheckForNull
-    @Override
-    public CNode describe(Describable instance, ConfigurationContext context) throws Exception {
-        final String symbol = DescribableAttribute.getPreferredSymbol(instance.getDescriptor(), getTarget(), instance.getClass());
-        final Configurator c = context.lookupOrFail(instance.getClass());
-        final CNode describe = c.describe(instance, context);
-        if (describe == null) {
-            return null;
-        }
-        if (describe.getType() == CNode.Type.MAPPING && describe.asMapping().size() == 0) {
-            return new Scalar(symbol);
+    private Tuple2<String, Option<CNode>> configureMapping(CNode config) {
+        Mapping mapping = unchecked(config::asMapping).apply();
+        if (mapping.size() != 1) {
+            throw new IllegalArgumentException("Single entry map expected to configure a " + target.getName());
+        } else {
+            Map.Entry<String, CNode> next = mapping.entrySet().iterator().next();
+            return Tuple.of(next.getKey(), Option.some(next.getValue()));
         }
+    }
+
+    private T doConfigure(ConfigurationContext context, Configurator<T> configurator, CNode subConfig) {
+        return unchecked(() -> configurator.configure(subConfig, context)).apply();
+    }
 
-        Mapping mapping = new Mapping();
-        mapping.put(symbol, describe);
-        return mapping;
+    @SuppressWarnings("unchecked")
+    private CNode convertToNode(ConfigurationContext context, Configurator configurator, Describable instance) {
+        return unchecked(() -> configurator.describe(instance, context)).apply();
     }
 }