implab/gradle-common Commit - r28:1a0b4caf9976

Variants validation + fail-fast

cin -

r28:1a0b4caf9976 default

parent child

Context file:

r28:1a0b4caf9976

Collapse all files

common/src/main/java/org/implab/gradle/common/sources/BuildVariant.java +11 -1

                  public LayerLink link(String from, String to, String kind) {
                      ensureMutable("add links");
-                     var link = new LayerLink(from, to, kind);
+                     var link = new LayerLink(
+                             requireLinkValue("from", from),
+                             requireLinkValue("to", to),
+                             requireLinkValue("kind", kind));
                      links.add(link);
                      return link;
                  }
                          throw new InvalidUserDataException("Variant '" + name + "' is finalized and cannot " + operation);
                  }
+                 private static String requireLinkValue(String field, String value) {
+                     if (value == null || value.trim().isEmpty())
+                         throw new InvalidUserDataException("Link '" + field + "' must not be null or blank");
+                     return value.trim();
+                 }
                  public final class RolesSpec {
                      public BuildRole role(String name, Action<? super BuildRole> configure) {
                          return BuildVariant.this.role(name, configure);

common/src/main/java/org/implab/gradle/common/sources/BuildVariantsExtension.java +57 -6

                      var errors = new ArrayList<String>();
                      var layersByName = new LinkedHashMap<String, BuildLayer>();
-                     for (var layer : layers)
-                         layersByName.put(layer.getName(), layer);
+                     for (var layer : layers) {
+                         var layerName = normalize(layer.getName());
+                         if (layerName == null) {
+                             errors.add("Layer name must not be blank");
+                             continue;
+                         }
+                         var previous = layersByName.putIfAbsent(layerName, layer);
+                         if (previous != null) {
+                             errors.add("Layer '" + layerName + "' is declared more than once");
+                         }
+                     }
                      for (var variant : variants)
                          validateVariant(variant, layersByName, errors);
                  }
                  private static void validateVariant(BuildVariant variant, Map<String, BuildLayer> layersByName, List<String> errors) {
+                     var variantName = normalize(variant.getName());
+                     if (variantName == null) {
+                         errors.add("Variant name must not be blank");
+                         return;
+                     }
+                     validateRoleAndArtifactNames(variant, errors);
                      var variantLayers = validateRoleMappings(variant, layersByName, errors);
                      validateLinks(variant, variantLayers, errors);
                  }
+                 private static void validateRoleAndArtifactNames(BuildVariant variant, List<String> errors) {
+                     var roleNames = new LinkedHashSet<String>();
+                     for (var role : variant.getRoles()) {
+                         var roleName = normalize(role.getName());
+                         if (roleName == null) {
+                             errors.add("Variant '" + variant.getName() + "' contains blank role name");
+                             continue;
+                         }
+                         if (!roleNames.add(roleName)) {
+                             errors.add("Variant '" + variant.getName() + "' contains duplicated role name '" + roleName + "'");
+                         }
+                     }
+                     var slotNames = new LinkedHashSet<String>();
+                     for (var slot : variant.getArtifactSlots()) {
+                         var slotName = normalize(slot.getName());
+                         if (slotName == null) {
+                             errors.add("Variant '" + variant.getName() + "' contains blank artifact slot name");
+                             continue;
+                         }
+                         if (!slotNames.add(slotName)) {
+                             errors.add("Variant '" + variant.getName() + "' contains duplicated artifact slot name '" + slotName + "'");
+                         }
+                     }
+                 }
                  private static Set<String> validateRoleMappings(BuildVariant variant, Map<String, BuildLayer> layersByName,
                          List<String> errors) {
                      var variantLayers = new LinkedHashSet<String>();
                      for (var role : variant.getRoles()) {
+                         var seenLayers = new LinkedHashSet<String>();
                          for (var layerName : role.getLayers().getOrElse(List.of())) {
-                             if (isBlank(layerName)) {
+                             var normalizedLayerName = normalize(layerName);
+                             if (normalizedLayerName == null) {
                                  errors.add("Variant '" + variant.getName() + "', role '" + role.getName() + "' contains blank layer name");
                                  continue;
                              }
-                             var layer = layersByName.get(layerName);
+                             var layer = layersByName.get(normalizedLayerName);
                              if (layer == null) {
-                                 errors.add("Variant '" + variant.getName() + "' references unknown layer '" + layerName + "'");
+                                 errors.add("Variant '" + variant.getName() + "' references unknown layer '" + normalizedLayerName + "'");
                                  continue;
                              }
-                             variantLayers.add(layerName);
+                             if (!seenLayers.add(normalizedLayerName)) {
+                                 errors.add("Variant '" + variant.getName() + "', role '" + role.getName()
+                                         + "' contains duplicated layer reference '" + normalizedLayerName + "'");
+                                 continue;
+                             }
+                             variantLayers.add(normalizedLayerName);
                          }
                      }

common/src/main/java/org/implab/gradle/common/sources/VariantSourcesExtension.java +7 0

                  private final List<SourceSetContext> boundContexts = new ArrayList<>();
                  private final LinkedHashMap<String, NamedDomainObjectProvider<GenericSourceSet>> sourceSetsByName = new LinkedHashMap<>();
                  private final LinkedHashMap<String, String> sourceSetLayersByName = new LinkedHashMap<>();
+                 private boolean sourceSetsRegistered;
                  @Inject
                  public VariantSourcesExtension(ObjectFactory objects) {
                  }
                  void registerSourceSets(BuildVariantsExtension variants, NamedDomainObjectContainer<GenericSourceSet> sources) {
+                     if (sourceSetsRegistered) {
+                         throw new InvalidUserDataException("variantSources source sets are already registered");
+                     }
                      validateBindings(variants);
                      var usages = layerUsages(variants).toList();
                              registeredContexts.size() - registeredBefore,
                              boundContexts.size() - boundBefore,
                              sourceSetsByName.size());
+                     sourceSetsRegistered = true;
                  }
                  private Stream<LayerUsage> layerUsages(BuildVariantsExtension variants) {

common/src/test/java/org/implab/gradle/common/sources/VariantsPluginFunctionalTest.java +20 -1

                                  link('a', 'b', null)
                              }
                          }
-                         """, "has incomplete link (from/to/kind are required)");
+                         """, "Link 'kind' must not be null or blank");
                  }
                  @Test
                  }
                  @Test
+                 void failsOnDuplicatedLayerReferenceInRole() throws Exception {
+                     assertBuildFails("""
+                         plugins {
+                             id 'org.implab.gradle-variants'
+                         }
+                         variants {
+                             layer('a')
+                             variant('browser') {
+                                 role('main') {
+                                     layers('a', 'a')
+                                 }
+                             }
+                         }
+                         """, "contains duplicated layer reference 'a'");
+                 }
+                 @Test
                  void failsOnLateLayerMutationAfterFinalize() throws Exception {
                      assertBuildFails("""
                          plugins {

General Comments 0

Write
Preview

You need to be logged in to leave comments. Login now

No TODOs yet

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages