Some improvements to inspections - messages, ordering of inspections,…

… support for specifying relationship inspection severities by relationship src/dest, etc.
structurizr · Feb 21, 2024 · 19a4246 · 19a4246
1 parent 0588f7c
commit 19a4246
Show file tree

Hide file tree

Showing 35 changed files with 188 additions and 122 deletions.
diff --git a/structurizr-core/src/main/java/com/structurizr/model/Model.java b/structurizr-core/src/main/java/com/structurizr/model/Model.java
@@ -16,7 +16,10 @@ public final class Model implements PropertyHolder {
 
     private IdGenerator idGenerator = new SequentialIntegerIdGeneratorStrategy();
 
+    private final Set<Element> elements = new LinkedHashSet<>();
     private final Map<String, Element> elementsById = new HashMap<>();
+
+    private final Set<Relationship> relationships = new LinkedHashSet<>();
     private final Map<String, Relationship> relationshipsById = new HashMap<>();
 
     private Enterprise enterprise;
@@ -277,6 +280,7 @@ private void addElementToInternalStructures(Element element) {
         }
 
         elementsById.put(element.getId(), element);
+        elements.add(element);
         element.setModel(this);
         idGenerator.found(element.getId());
     }
@@ -288,12 +292,14 @@ private void addRelationshipToInternalStructures(Relationship relationship) {
         }
 
         relationshipsById.put(relationship.getId(), relationship);
+        relationships.add(relationship);
         relationship.setModel(this);
         idGenerator.found(relationship.getId());
     }
 
     private void removeRelationshipFromInternalStructures(Relationship relationship) {
         relationshipsById.remove(relationship.getId());
+        relationships.remove(relationship);
     }
 
     /**
@@ -304,7 +310,7 @@ private void removeRelationshipFromInternalStructures(Relationship relationship)
     @JsonIgnore
     @Nonnull
     public Set<Element> getElements() {
-        return new HashSet<>(this.elementsById.values());
+        return new LinkedHashSet<>(elements);
     }
 
     /**
@@ -331,7 +337,7 @@ public Element getElement(@Nonnull String id) {
     @JsonIgnore
     @Nonnull
     public Set<Relationship> getRelationships() {
-        return new HashSet<>(this.relationshipsById.values());
+        return new LinkedHashSet<>(this.relationships);
     }
 
     /**
@@ -554,7 +560,7 @@ private void hydrateRelationships(Element element) {
      * @return true, if the element is contained in this model
      */
     public boolean contains(Element element) {
-        return elementsById.containsValue(element);
+        return elements.contains(element);
     }
 
     /**
@@ -564,7 +570,7 @@ public boolean contains(Element element) {
      * @return true, if the relationship is contained in this model
      */
     public boolean contains(Relationship relationship) {
-        return relationshipsById.containsValue(relationship);
+        return relationships.contains(relationship);
     }
 
     /**
@@ -1153,6 +1159,7 @@ private void removeElement(Element element) {
         }
 
         elementsById.remove(element.getId());
+        elements.remove(element);
     }
 
 }
diff --git a/structurizr-inspection/src/main/java/com/structurizr/inspection/DefaultInspector.java b/structurizr-inspection/src/main/java/com/structurizr/inspection/DefaultInspector.java
@@ -72,11 +72,11 @@ private void runModelInspections() {
 
             add(disconnectedElementCheck.run(element));
             add(elementNotIncludedInAnyViewsCheck.run(element));
-        }
 
-        for (Relationship relationship : getWorkspace().getModel().getRelationships()) {
-            add(new RelationshipDescriptionInspection(this).run(relationship));
-            add(new RelationshipTechnologyInspection(this).run(relationship));
+            for (Relationship relationship : element.getRelationships()) {
+                add(new RelationshipDescriptionInspection(this).run(relationship));
+                add(new RelationshipTechnologyInspection(this).run(relationship));
+            }
         }
     }
 

diff --git a/structurizr-inspection/src/main/java/com/structurizr/inspection/Inspector.java b/structurizr-inspection/src/main/java/com/structurizr/inspection/Inspector.java
@@ -10,6 +10,7 @@ public abstract class Inspector {
     private final Workspace workspace;
 
     private final List<Violation> violations = new ArrayList<>();
+    private int numberOfInspections = 0;
 
     protected Inspector(Workspace workspace) {
         this.workspace = workspace;
@@ -23,7 +24,13 @@ public List<Violation> getViolations() {
         return new ArrayList<>(violations);
     }
 
+    public int getNumberOfInspections() {
+        return numberOfInspections;
+    }
+
     protected void add(Violation violation) {
+        numberOfInspections++;
+
         if (violation != null) {
             violations.add(violation);
         }

diff --git a/...zr-inspection/src/main/java/com/structurizr/inspection/PropertyBasedSeverityStrategy.java b/...zr-inspection/src/main/java/com/structurizr/inspection/PropertyBasedSeverityStrategy.java
@@ -11,6 +11,7 @@
 import com.structurizr.view.ViewSet;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
@@ -23,27 +24,32 @@ public PropertyBasedSeverityStrategy() {
 
     @Override
     public Severity getSeverity(Inspection inspection, Workspace workspace) {
-        return getSeverityFromProperties(inspection, workspace);
+        Severity severity = getSeverityFromProperties(inspection.getType(), workspace);
+        return severity != null ? severity : Severity.ERROR;
     }
 
     @Override
     public Severity getSeverity(Inspection inspection, ViewSet viewSet) {
-        return getSeverityFromProperties(inspection, inspection.getWorkspace(), viewSet.getConfiguration());
+        Severity severity = getSeverityFromProperties(inspection.getType(), inspection.getWorkspace(), viewSet.getConfiguration());
+        return severity != null ? severity : Severity.ERROR;
     }
 
     @Override
     public Severity getSeverity(Inspection inspection, View view) {
-        return getSeverityFromProperties(inspection, inspection.getWorkspace(), inspection.getWorkspace().getViews().getConfiguration(), view);
+        Severity severity = getSeverityFromProperties(inspection.getType(), inspection.getWorkspace(), inspection.getWorkspace().getViews().getConfiguration(), view);
+        return severity != null ? severity : Severity.ERROR;
     }
 
     @Override
     public Severity getSeverity(Inspection inspection, ElementStyle elementStyle) {
-        return getSeverityFromProperties(inspection, inspection.getWorkspace(), inspection.getWorkspace().getViews().getConfiguration(), elementStyle);
+        Severity severity = getSeverityFromProperties(inspection.getType(), inspection.getWorkspace(), inspection.getWorkspace().getViews().getConfiguration(), elementStyle);
+        return severity != null ? severity : Severity.ERROR;
     }
 
     @Override
     public Severity getSeverity(Inspection inspection, Model model) {
-        return getSeverityFromProperties(inspection, inspection.getWorkspace(), model);
+        Severity severity = getSeverityFromProperties(inspection.getType(), inspection.getWorkspace(), model);
+        return severity != null ? severity : Severity.ERROR;
     }
 
     @Override
@@ -54,7 +60,8 @@ public Severity getSeverity(Inspection inspection, Element element) {
             grandParentElement = parentElement.getParent();
         }
 
-        return getSeverityFromProperties(inspection, inspection.getWorkspace(), inspection.getWorkspace().getModel(), grandParentElement, parentElement, element);
+        Severity severity = getSeverityFromProperties(inspection.getType(), inspection.getWorkspace(), inspection.getWorkspace().getModel(), grandParentElement, parentElement, element);
+        return severity != null ? severity : Severity.ERROR;
     }
 
     @Override
@@ -65,48 +72,78 @@ public Severity getSeverity(Inspection inspection, Relationship relationship) {
             inspection.getWorkspace().getModel().getRelationship(relationship.getLinkedRelationshipId());
         }
 
-        return getSeverityFromProperties(inspection, inspection.getWorkspace(), inspection.getWorkspace().getModel(), source.getParent(), source, linkedRelationship, relationship);
+        String allRelationshipsType = inspection.getType();
+        String specificRelationshipType = inspection.getType();
+
+        // convert model.relationship.description to model.relationship[sourceType->destinationType].description
+        String sourceType = relationship.getSource().getClass().getSimpleName().toLowerCase();
+        String destinationType = relationship.getDestination().getClass().getSimpleName().toLowerCase();
+        specificRelationshipType = allRelationshipsType.replaceFirst(
+                "\\.relationship\\.",
+                String.format(".relationship[%s->%s].", sourceType, destinationType)
+        );
+
+        Severity severity = getSeverityFromProperties(specificRelationshipType, inspection.getWorkspace(), inspection.getWorkspace().getModel(), source.getParent(), source, linkedRelationship, relationship);
+        if (severity == null) {
+            severity = getSeverityFromProperties(allRelationshipsType, inspection.getWorkspace(), inspection.getWorkspace().getModel(), source.getParent(), source, linkedRelationship, relationship);
+        }
+
+        return severity != null ? severity : Severity.ERROR;
     }
 
-    protected Severity getSeverityFromProperties(Inspection inspection, PropertyHolder... propertyHolders) {
-        List<String> types = generatePropertyNames(inspection.getType());
+    protected Severity getSeverityFromProperties(String type, PropertyHolder... propertyHolders) {
+        List<String> types = generatePropertyNames(type);
+        List<PropertyHolder> reversedPropertyHolders = Arrays.asList(propertyHolders);
+        Collections.reverse(reversedPropertyHolders);
 
-        for (String type : types) {
-            for (PropertyHolder propertyHolder : propertyHolders) {
-                if (propertyHolder != null) {
-                    if (propertyHolder.getProperties().containsKey(type)) {
-                        return Severity.valueOf(propertyHolder.getProperties().get(type).toUpperCase());
+        for (PropertyHolder propertyHolder : reversedPropertyHolders) {
+            if (propertyHolder != null) {
+                for (String t : types) {
+                    if (propertyHolder.getProperties().containsKey(t)) {
+                        return Severity.valueOf(propertyHolder.getProperties().get(t).toUpperCase());
                     }
                 }
             }
         }
 
-        return Severity.ERROR;
+        return null;
     }
 
-    protected List<String> generatePropertyNames(String inspection) {
+    protected List<String> generatePropertyNames(String type) {
         // example input:
-        // structurizr.inspection.model.component.description
+        // model.component.description
         //
         // example output:
         // structurizr.inspection.model.component.description
         // structurizr.inspection.model.component.*
         // structurizr.inspection.model.*
         // structurizr.inspection.*
 
+        // example input:
+        // model.relationship[component->component].technology
+        //
+        // example output:
+        // structurizr.inspection.model.relationship[component->component].technology
+        // structurizr.inspection.model.relationship[component->component].*
+
+        type = type.toLowerCase();
         List<String> types = new ArrayList<>();
 
-        String[] parts = inspection.split("\\.");
+        String[] parts = type.split("\\.");
         String buf = STRUCTURIZR_INSPECTION_PREFIX;
         types.add(buf + "*");
         for (int i = 0; i < parts.length-1; i++) {
             buf = buf + parts[i] + ".";
             types.add(buf + "*");
         }
 
-        types.add(STRUCTURIZR_INSPECTION_PREFIX + inspection);
+        types.add(STRUCTURIZR_INSPECTION_PREFIX + type);
         Collections.reverse(types);
 
+        if (type.contains(".relationship[")) {
+            return types.subList(0, types.size()-2);
+        }
+
         return types;
     }
 

diff --git a/...-inspection/src/main/java/com/structurizr/inspection/model/AbstractElementInspection.java b/...-inspection/src/main/java/com/structurizr/inspection/model/AbstractElementInspection.java
@@ -23,6 +23,11 @@ protected String terminologyFor(Element element) {
         return getWorkspace().getViews().getConfiguration().getTerminology().findTerminology(element).toLowerCase();
     }
 
+    protected String nameOf(Element element) {
+        String canonicalName = element.getCanonicalName();
+        return canonicalName.substring(canonicalName.indexOf("://") + 3);
+    }
+
     protected abstract Violation inspect(Element element);
 
 }
diff --git a/...ection/src/main/java/com/structurizr/inspection/model/AbstractRelationshipInspection.java b/...ection/src/main/java/com/structurizr/inspection/model/AbstractRelationshipInspection.java
@@ -24,6 +24,11 @@ protected String terminologyFor(Element element) {
         return getWorkspace().getViews().getConfiguration().getTerminology().findTerminology(element).toLowerCase();
     }
 
+    protected String nameOf(Element element) {
+        String canonicalName = element.getCanonicalName();
+        return canonicalName.substring(canonicalName.indexOf("://") + 3);
+    }
+
     protected abstract Violation inspect(Relationship relationship);
 
 }
diff --git a/...pection/src/main/java/com/structurizr/inspection/model/ComponentTechnologyInspection.java b/...pection/src/main/java/com/structurizr/inspection/model/ComponentTechnologyInspection.java
@@ -14,7 +14,7 @@ public ComponentTechnologyInspection(Inspector inspector) {
     @Override
     protected Violation inspect(Component component) {
         if (StringUtils.isNullOrEmpty(component.getDescription())) {
-            return violation("Add a technology to the " + terminologyFor(component).toLowerCase() + " named \"" + component.getName() + "\".");
+            return violation("The " + terminologyFor(component).toLowerCase() + " \"" + nameOf(component) + "\" is missing a technology.");
         }
 
         return noViolation();

diff --git a/...pection/src/main/java/com/structurizr/inspection/model/ContainerTechnologyInspection.java b/...pection/src/main/java/com/structurizr/inspection/model/ContainerTechnologyInspection.java
@@ -14,7 +14,7 @@ public ContainerTechnologyInspection(Inspector inspector) {
     @Override
     protected Violation inspect(Container container) {
         if (StringUtils.isNullOrEmpty(container.getTechnology())) {
-            return violation("Add a technology to the " + terminologyFor(container).toLowerCase() + " named \"" + container.getName() + "\".");
+            return violation("The " + terminologyFor(container).toLowerCase() + " \"" + nameOf(container) + "\" is missing a technology.");
         }
 
         return noViolation();

diff --git a/...on/src/main/java/com/structurizr/inspection/model/DeploymentNodeTechnologyInspection.java b/...on/src/main/java/com/structurizr/inspection/model/DeploymentNodeTechnologyInspection.java
@@ -14,7 +14,7 @@ public DeploymentNodeTechnologyInspection(Inspector inspector) {
     @Override
     protected Violation inspect(DeploymentNode deploymentNode) {
         if (StringUtils.isNullOrEmpty(deploymentNode.getDescription())) {
-            return violation("Add a technology to the " + terminologyFor(deploymentNode).toLowerCase() + " named \"" + deploymentNode.getName() + "\".");
+            return violation("The " + terminologyFor(deploymentNode).toLowerCase() + " \"" + nameOf(deploymentNode) + "\" is missing a technology.");
         }
 
         return noViolation();

diff --git a/...pection/src/main/java/com/structurizr/inspection/model/DisconnectedElementInspection.java b/...pection/src/main/java/com/structurizr/inspection/model/DisconnectedElementInspection.java
@@ -30,7 +30,7 @@ protected Violation inspect(Element element) {
         }
 
         if (!elementsWithRelationships.contains(element.getId())) {
-            return violation("The " + terminologyFor(element).toLowerCase() + " named \"" + element.getName() + "\" is disconnected - add a relationship to/from it, or consider removing it from the model.");
+            return violation("The " + terminologyFor(element).toLowerCase() + " \"" + nameOf(element) + "\" is disconnected - add a relationship to/from it, or consider removing it from the model.");
         }
 
         return noViolation();

diff --git a/...spection/src/main/java/com/structurizr/inspection/model/ElementDescriptionInspection.java b/...spection/src/main/java/com/structurizr/inspection/model/ElementDescriptionInspection.java
@@ -14,7 +14,7 @@ public ElementDescriptionInspection(Inspector inspector) {
     @Override
     protected Violation inspect(Element element) {
         if (StringUtils.isNullOrEmpty(element.getDescription())) {
-            return violation("Add a description to the " + terminologyFor(element).toLowerCase() + " named \"" + element.getName() + "\".");
+            return violation("The " + terminologyFor(element).toLowerCase() + " \"" + nameOf(element) + "\" is missing a description.");
         }
 
         return noViolation();

diff --git a/...pection/src/main/java/com/structurizr/inspection/model/EmptyDeploymentNodeInspection.java b/...pection/src/main/java/com/structurizr/inspection/model/EmptyDeploymentNodeInspection.java
@@ -13,7 +13,7 @@ public EmptyDeploymentNodeInspection(Inspector inspector) {
     @Override
     protected Violation inspect(DeploymentNode deploymentNode) {
         if (!deploymentNode.hasChildren() && !deploymentNode.hasSoftwareSystemInstances() && !deploymentNode.hasContainerInstances() && !deploymentNode.hasInfrastructureNodes()) {
-            return violation("The " + terminologyFor(deploymentNode).toLowerCase() + " named \"" + deploymentNode.getName() + "\" is empty.");
+            return violation("The " + terminologyFor(deploymentNode).toLowerCase() + " \"" + nameOf(deploymentNode) + "\" is empty.");
         }
 
         return noViolation();

diff --git a/...rc/main/java/com/structurizr/inspection/model/InfrastructureNodeTechnologyInspection.java b/...rc/main/java/com/structurizr/inspection/model/InfrastructureNodeTechnologyInspection.java
@@ -14,7 +14,7 @@ public InfrastructureNodeTechnologyInspection(Inspector inspector) {
     @Override
     protected Violation inspect(InfrastructureNode infrastructureNode) {
         if (StringUtils.isNullOrEmpty(infrastructureNode.getTechnology())) {
-            return violation("Add a technology to the " + terminologyFor(infrastructureNode).toLowerCase() + " named \"" + infrastructureNode.getName() + "\".");
+            return violation("The " + terminologyFor(infrastructureNode).toLowerCase() + " \"" + nameOf(infrastructureNode) + "\" is missing a technology.");
         }
 
         return noViolation();

diff --git a/...ion/src/main/java/com/structurizr/inspection/model/RelationshipDescriptionInspection.java b/...ion/src/main/java/com/structurizr/inspection/model/RelationshipDescriptionInspection.java
@@ -19,11 +19,7 @@ protected Violation inspect(Relationship relationship) {
         Element destination = relationship.getDestination();
 
         if (StringUtils.isNullOrEmpty(relationship.getDescription())) {
-            if (source instanceof Component && destination instanceof Component) {
-                return violation("Add a description to the relationship between the " + terminologyFor(source) + " named \"" + source.getName() + "\" and the " + terminologyFor(destination) + " named \"" + destination.getName() + "\".");
-            } else {
-                return violation("Add a description to the relationship between the " + terminologyFor(source) + " named \"" + source.getName() + "\" and the " + terminologyFor(destination) + " named \"" + destination.getName() + "\".");
-            }
+            return violation("The relationship between the " + terminologyFor(source) + " \"" + nameOf(source) + "\" and the " + terminologyFor(destination) + " \"" + nameOf(destination) + "\" is missing a description.");
         }
 
         return noViolation();

diff --git a/...tion/src/main/java/com/structurizr/inspection/model/RelationshipTechnologyInspection.java b/...tion/src/main/java/com/structurizr/inspection/model/RelationshipTechnologyInspection.java
@@ -19,11 +19,7 @@ protected Violation inspect(Relationship relationship) {
         Element destination = relationship.getDestination();
 
         if (StringUtils.isNullOrEmpty(relationship.getTechnology())) {
-            if (source instanceof Container && destination instanceof Container) {
-                return violation("Add a technology to the relationship between the " + terminologyFor(source) + " named \"" + source.getName() + "\" and the " + terminologyFor(destination) + " named \"" + destination.getName() + "\".");
-            } else {
-                return violation("Add a technology to the relationship between the " + terminologyFor(source) + " named \"" + source.getName() + "\" and the " + terminologyFor(destination) + " named \"" + destination.getName() + "\".");
-            }
+            return violation("The relationship between the " + terminologyFor(source) + " \"" + nameOf(source) + "\" and the " + terminologyFor(destination) + " \"" + nameOf(destination) + "\" is missing a technology.");
         }
 
         return noViolation();

diff --git a/...ion/src/main/java/com/structurizr/inspection/model/SoftwareSystemDecisionsInspection.java b/...ion/src/main/java/com/structurizr/inspection/model/SoftwareSystemDecisionsInspection.java
@@ -13,7 +13,7 @@ public SoftwareSystemDecisionsInspection(Inspector inspector) {
     @Override
     protected Violation inspect(SoftwareSystem softwareSystem) {
         if (softwareSystem.hasContainers() && softwareSystem.getDocumentation().getDecisions().isEmpty()) {
-            return violation("The " + terminologyFor(softwareSystem).toLowerCase() + " named \"" + softwareSystem.getName() + "\" has containers, but is missing decisions.");
+            return violation("The " + terminologyFor(softwareSystem).toLowerCase() + " \"" + nameOf(softwareSystem) + "\" has containers, but is missing decisions.");
         }
 
         return noViolation();

diff --git a/...src/main/java/com/structurizr/inspection/model/SoftwareSystemDocumentationInspection.java b/...src/main/java/com/structurizr/inspection/model/SoftwareSystemDocumentationInspection.java
@@ -13,7 +13,7 @@ public SoftwareSystemDocumentationInspection(Inspector inspector) {
     @Override
     protected Violation inspect(SoftwareSystem softwareSystem) {
         if (softwareSystem.hasContainers() && softwareSystem.getDocumentation().getSections().isEmpty()) {
-            return violation("The " + terminologyFor(softwareSystem).toLowerCase() + " named \"" + softwareSystem.getName() + "\" has containers, but is missing documentation.");
+            return violation("The " + terminologyFor(softwareSystem).toLowerCase() + " \"" + nameOf(softwareSystem) + "\" has containers, but is missing documentation.");
         }
 
         return noViolation();