You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@isis.apache.org by da...@apache.org on 2021/09/06 15:46:20 UTC

[isis] 01/02: ISIS-2867: adds arch tests 'unique logical type name'; 'logicalTypeName matches JDO discriminator'

This is an automated email from the ASF dual-hosted git repository.

danhaywood pushed a commit to branch ISIS-2867
in repository https://gitbox.apache.org/repos/asf/isis.git

commit 56cfee96f573542cceb57d2d15927e2b9ca97187
Author: danhaywood <da...@haywood-associates.co.uk>
AuthorDate: Mon Sep 6 16:45:14 2021 +0100

    ISIS-2867: adds arch tests 'unique logical type name'; 'logicalTypeName matches JDO discriminator'
---
 .../adoc/modules/archtestsupport/pages/about.adoc  | 12 ++++
 .../applib/classrules/ArchitectureDomainRules.java | 66 +++++++++++++++++++---
 .../applib/classrules/ArchitectureJdoRules.java    | 65 +++++++++++++++++----
 .../applib/classrules/CommonPredicates.java        | 24 ++++++++
 4 files changed, 149 insertions(+), 18 deletions(-)

diff --git a/testing/archtestsupport/adoc/modules/archtestsupport/pages/about.adoc b/testing/archtestsupport/adoc/modules/archtestsupport/pages/about.adoc
index d472aeb..3530f0e 100644
--- a/testing/archtestsupport/adoc/modules/archtestsupport/pages/about.adoc
+++ b/testing/archtestsupport/adoc/modules/archtestsupport/pages/about.adoc
@@ -113,6 +113,12 @@ This ensures that persisted/serialized bookmarks of domain objects are stable ev
 *** `every_DomainObject_must_specify_logicalTypeName()`
 *** `every_DomainService_must_specify_logicalTypeName()`
 
+** unique logical type name
++
+This ensures that the logical type name is also unique across ``@DomainObject``s and ``@DomainService``s.
++
+Note: this rule ignores any abstract classes because strictly the framework only requires that instantiatable classes have unique logical type names.
+
 ** JAXB view models are annotated with `@DomainObject(nature=VIEW_MODEL)`
 +
 This ensures that the framework is able to correctly detect the view models in the metamodel:
@@ -193,6 +199,12 @@ Enforces a very common convention for JPA entities:
 
 *** `every_jpa_Entity_must_have_an_id_field()`
 
+** ensure JDO entity's logicalTypeName matches its discriminator (if any)
++
+Both the logical type name and the `@Discriminator` for subclasses are a unique identifier of a type; this rule says they should be the same value.
+
+*** `every_logicalTypeName_and_jdo_discriminator_must_be_same()`
+
 
 * Recommended
 
diff --git a/testing/archtestsupport/applib/src/main/java/org/apache/isis/testing/archtestsupport/applib/classrules/ArchitectureDomainRules.java b/testing/archtestsupport/applib/src/main/java/org/apache/isis/testing/archtestsupport/applib/classrules/ArchitectureDomainRules.java
index 4eefef9..ddbc14b 100644
--- a/testing/archtestsupport/applib/src/main/java/org/apache/isis/testing/archtestsupport/applib/classrules/ArchitectureDomainRules.java
+++ b/testing/archtestsupport/applib/src/main/java/org/apache/isis/testing/archtestsupport/applib/classrules/ArchitectureDomainRules.java
@@ -20,6 +20,8 @@ package org.apache.isis.testing.archtestsupport.applib.classrules;
 
 import java.io.Serializable;
 import java.lang.annotation.Annotation;
+import java.util.Map;
+import java.util.TreeMap;
 import java.util.function.Function;
 import java.util.function.Predicate;
 
@@ -33,7 +35,6 @@ import com.tngtech.archunit.core.domain.JavaClass;
 import com.tngtech.archunit.core.domain.JavaCodeUnit;
 import com.tngtech.archunit.core.domain.JavaMethod;
 import com.tngtech.archunit.core.domain.JavaModifier;
-import com.tngtech.archunit.junit.ArchTest;
 import com.tngtech.archunit.lang.ArchCondition;
 import com.tngtech.archunit.lang.ArchRule;
 import com.tngtech.archunit.lang.ConditionEvents;
@@ -56,6 +57,7 @@ import org.apache.isis.applib.annotation.DomainService;
 import org.apache.isis.applib.annotation.DomainServiceLayout;
 import org.apache.isis.applib.annotation.Nature;
 import org.apache.isis.applib.annotation.Property;
+import org.apache.isis.commons.internal.base._Strings;
 
 import lombok.val;
 import lombok.experimental.UtilityClass;
@@ -90,6 +92,56 @@ public class ArchitectureDomainRules {
                 .should().beAnnotatedWith(DomainService_logicalTypeName());
     }
 
+    private static Map<String, JavaClass> javaClassByLogicalTypeName = new TreeMap<>();
+
+    /**
+     * This rule requires that classes annotated the <code>logicalTypeName</code> must be unique across all
+     * non-abstract {@link DomainObject}s and {@link DomainService}s
+     */
+    public static ArchRule every_logicalTypeName_must_be_unique() {
+        return classes()
+                .that().areAnnotatedWith(DomainObject.class)
+                .or().areAnnotatedWith(DomainService.class)
+                .and(new DescribedPredicate<>("have an logicalTypeName") {
+                    @Override
+                    public boolean apply(JavaClass javaClass) {
+                        val domainObjectIfAny = javaClass.tryGetAnnotationOfType(DomainObject.class);
+                        if (domainObjectIfAny.isPresent() && !_Strings.isNullOrEmpty(domainObjectIfAny.get().logicalTypeName())) {
+                            return true;
+                        }
+                        val domainServiceIfAny = javaClass.tryGetAnnotationOfType(DomainService.class);
+                        if (domainServiceIfAny.isPresent() && !_Strings.isNullOrEmpty(domainServiceIfAny.get().logicalTypeName())) {
+                            return true;
+                        }
+
+                        return false;
+                    }
+                })
+                .should(new ArchCondition<>("be unique") {
+                    @Override
+                    public void check(JavaClass javaClass, ConditionEvents conditionEvents) {
+                        val domainObjectIfAny = javaClass.tryGetAnnotationOfType(DomainObject.class);
+                        String logicalTypeName = null;
+                        if (domainObjectIfAny.isPresent()) {
+                            logicalTypeName = domainObjectIfAny.get().logicalTypeName();
+                        } else {
+                            val domainServiceIfAny = javaClass.tryGetAnnotationOfType(DomainService.class);
+                            if (domainServiceIfAny.isPresent()) {
+                                logicalTypeName = domainServiceIfAny.get().logicalTypeName();
+                            }
+                        }
+                        final JavaClass existing = javaClassByLogicalTypeName.get(logicalTypeName);
+                        if (existing != null) {
+                            conditionEvents.add(
+                                    new SimpleConditionEvent(javaClass, false,
+                                            String.format("Classes '%s' and '%s' have the same logicalTypeName '%s'", javaClass.getName(), existing.getName(), logicalTypeName)));
+                        } else {
+                            javaClassByLogicalTypeName.put(logicalTypeName, javaClass);
+                        }
+                    }
+                });
+    }
+
     /**
      * This rule requires that classes annotated with the {@link DomainObject} annotation must also be
      * annotated with the {@link DomainObjectLayout} annotation.
@@ -134,7 +186,7 @@ public class ArchitectureDomainRules {
 
     /**
      * This rule requires that classes annotated with the {@link XmlRootElement} annotation must also be
-     * annotated with the {@link DomainObject} annotation specifying a {@link Nature nature} of {@link Nature#MIXIN MIXIN}.
+     * annotated with the {@link DomainObject} annotation specifying a {@link Nature nature} of {@link Nature#VIEW_MODEL VIEW_MODEL}.
      *
      * <p>
      *     This is required because the framework uses Spring to detect entities and view models (the
@@ -142,11 +194,11 @@ public class ArchitectureDomainRules {
      *     {@link org.springframework.stereotype.Component} annotation.
      * </p>
      */
-    @ArchTest
-    public static ArchRule every_jaxb_view_model_must_also_be_annotated_with_DomainObject_nature_MIXIN =
-            ArchRuleDefinition.classes().that()
-                    .areAnnotatedWith(XmlRootElement.class)
-                    .should().beAnnotatedWith(CommonPredicates.DomainObject_nature_MIXIN());
+    public static ArchRule every_jaxb_view_model_must_also_be_annotated_with_DomainObject_nature_VIEW_MODEL() {
+        return ArchRuleDefinition.classes().that()
+                .areAnnotatedWith(XmlRootElement.class)
+                .should().beAnnotatedWith(CommonPredicates.DomainObject_nature_VIEW_MODEL());
+    }
 
     /**
      * This rule requires that action mixin classes should follow the naming convention
diff --git a/testing/archtestsupport/applib/src/main/java/org/apache/isis/testing/archtestsupport/applib/classrules/ArchitectureJdoRules.java b/testing/archtestsupport/applib/src/main/java/org/apache/isis/testing/archtestsupport/applib/classrules/ArchitectureJdoRules.java
index 112be23..bde6633 100644
--- a/testing/archtestsupport/applib/src/main/java/org/apache/isis/testing/archtestsupport/applib/classrules/ArchitectureJdoRules.java
+++ b/testing/archtestsupport/applib/src/main/java/org/apache/isis/testing/archtestsupport/applib/classrules/ArchitectureJdoRules.java
@@ -18,7 +18,10 @@
  */
 package org.apache.isis.testing.archtestsupport.applib.classrules;
 
+import java.util.Objects;
+
 import javax.inject.Inject;
+import javax.jdo.annotations.Discriminator;
 import javax.jdo.annotations.NotPersistent;
 import javax.jdo.annotations.PersistenceCapable;
 
@@ -26,15 +29,20 @@ import com.tngtech.archunit.base.DescribedPredicate;
 import com.tngtech.archunit.core.domain.JavaAnnotation;
 import com.tngtech.archunit.core.domain.JavaClass;
 import com.tngtech.archunit.core.domain.JavaEnumConstant;
+import com.tngtech.archunit.lang.ArchCondition;
 import com.tngtech.archunit.lang.ArchRule;
-
-import org.apache.isis.applib.annotation.DomainObject;
+import com.tngtech.archunit.lang.ConditionEvents;
+import com.tngtech.archunit.lang.SimpleConditionEvent;
 
 import static com.tngtech.archunit.base.DescribedPredicate.not;
 import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes;
 import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.fields;
-import lombok.experimental.UtilityClass;
+
+import org.apache.isis.applib.annotation.DomainObject;
+import org.apache.isis.commons.internal.base._Strings;
+
 import lombok.val;
+import lombok.experimental.UtilityClass;
 
 /**
  * A library of architecture tests to ensure coding conventions are followed for classes annotated with
@@ -45,6 +53,38 @@ import lombok.val;
 @UtilityClass
 public class ArchitectureJdoRules {
 
+    public static ArchRule every_logicalTypeName_and_jdo_discriminator_must_be_same() {
+        return classes()
+                .that().areAnnotatedWith(DomainObject.class)
+                .and(new DescribedPredicate<>("have a logicalTypeName") {
+                    @Override
+                    public boolean apply(JavaClass javaClass) {
+                        val domainObjectIfAny = javaClass.tryGetAnnotationOfType(DomainObject.class);
+                        return domainObjectIfAny.isPresent() && !_Strings.isNullOrEmpty(domainObjectIfAny.get().logicalTypeName());
+                    }
+                })
+                .and(new DescribedPredicate<>("have a @Discriminator") {
+                    @Override
+                    public boolean apply(JavaClass javaClass) {
+                        val discriminatorIfAny = javaClass.tryGetAnnotationOfType(Discriminator.class);
+                        return discriminatorIfAny.isPresent();
+                    }
+                })
+                .should(new ArchCondition<>("be the same") {
+                    @Override
+                    public void check(JavaClass javaClass, ConditionEvents conditionEvents) {
+                        val logicalTypeName = javaClass.getAnnotationOfType(DomainObject.class).logicalTypeName();
+                        val discriminatorValue = javaClass.getAnnotationOfType(Discriminator.class).value();
+                        if (!Objects.equals(logicalTypeName, discriminatorValue)) {
+                            conditionEvents.add(
+                                    new SimpleConditionEvent(javaClass, false,
+                                    String.format("@DomainObject(logicalTypeName = '%s') vs @Discriminator('%s')",
+                                            logicalTypeName, discriminatorValue)));
+                        }
+                    }
+                });
+    }
+
     /**
      * This rule requires that classes annotated with the JDO {@link javax.jdo.annotations.PersistenceCapable} annotation
      * must also be annotated with the Apache Isis {@link DomainObject} annotation specifying that its
@@ -154,8 +194,9 @@ public class ArchitectureJdoRules {
 
 
     static DescribedPredicate<JavaAnnotation<?>> PersistenceCapable_schema() {
-        return new DescribedPredicate<JavaAnnotation<?>>("@PersistenceCapable(schema=...)") {
-            @Override public boolean apply(final JavaAnnotation<?> javaAnnotation) {
+        return new DescribedPredicate<>("@PersistenceCapable(schema=...)") {
+            @Override
+            public boolean apply(final JavaAnnotation<?> javaAnnotation) {
                 if (!javaAnnotation.getRawType().isAssignableTo(PersistenceCapable.class)) {
                     return false;
                 }
@@ -168,8 +209,9 @@ public class ArchitectureJdoRules {
     }
 
     static DescribedPredicate<JavaAnnotation<?>> PersistenceCapable_with_DATASTORE_identityType() {
-        return new DescribedPredicate<JavaAnnotation<?>>("@PersistenceCapable(identityType=DATASTORE)") {
-            @Override public boolean apply(final JavaAnnotation<?> javaAnnotation) {
+        return new DescribedPredicate<>("@PersistenceCapable(identityType=DATASTORE)") {
+            @Override
+            public boolean apply(final JavaAnnotation<?> javaAnnotation) {
                 if (!javaAnnotation.getRawType().isAssignableTo(PersistenceCapable.class)) {
                     return false;
                 }
@@ -203,7 +245,7 @@ public class ArchitectureJdoRules {
     }
 
     static DescribedPredicate<JavaClass> areEntities() {
-        return new DescribedPredicate<JavaClass>("are entities") {
+        return new DescribedPredicate<>("are entities") {
             @Override
             public boolean apply(JavaClass input) {
                 return input.isAnnotatedWith(PersistenceCapable.class);
@@ -212,10 +254,11 @@ public class ArchitectureJdoRules {
     }
 
     static DescribedPredicate<? super JavaClass> areSubtypeEntities() {
-        return new DescribedPredicate<JavaClass>("are subtype entities ") {
-            @Override public boolean apply(final JavaClass input) {
+        return new DescribedPredicate<>("are subtype entities ") {
+            @Override
+            public boolean apply(final JavaClass input) {
                 val superclassIfAny = input.getSuperclass();
-                if(!superclassIfAny.isPresent()) {
+                if (!superclassIfAny.isPresent()) {
                     return false;
                 }
                 val superType = superclassIfAny.get();
diff --git a/testing/archtestsupport/applib/src/main/java/org/apache/isis/testing/archtestsupport/applib/classrules/CommonPredicates.java b/testing/archtestsupport/applib/src/main/java/org/apache/isis/testing/archtestsupport/applib/classrules/CommonPredicates.java
index 33b4cc5..dacf942 100644
--- a/testing/archtestsupport/applib/src/main/java/org/apache/isis/testing/archtestsupport/applib/classrules/CommonPredicates.java
+++ b/testing/archtestsupport/applib/src/main/java/org/apache/isis/testing/archtestsupport/applib/classrules/CommonPredicates.java
@@ -42,6 +42,16 @@ import lombok.experimental.UtilityClass;
 @UtilityClass
 class CommonPredicates {
 
+    static DescribedPredicate<JavaClass> notAbstract() {
+        return new DescribedPredicate<>("not abstract") {
+            @Override
+            public boolean apply(JavaClass javaClass) {
+                val isAbstract = javaClass.getModifiers().stream().anyMatch((JavaModifier x) -> x == JavaModifier.ABSTRACT);
+                return !isAbstract;
+            }
+        };
+    }
+
     static DescribedPredicate<JavaAnnotation<?>> DomainObject_nature_ENTITY() {
         return DomainObject_nature(Nature.ENTITY);
     }
@@ -50,6 +60,10 @@ class CommonPredicates {
         return DomainObject_nature(Nature.MIXIN);
     }
 
+    static DescribedPredicate<JavaAnnotation<?>> DomainObject_nature_VIEW_MODEL() {
+        return DomainObject_nature(Nature.VIEW_MODEL);
+    }
+
     static DescribedPredicate<JavaAnnotation<?>> DomainObject_nature(final Nature expectedNature) {
         return new DescribedPredicate<>(String.format("@DomainObject(nature=%s)", expectedNature.name())) {
             @Override
@@ -131,4 +145,14 @@ class CommonPredicates {
         };
     }
 
+    static DescribedPredicate<JavaClass> areNotAbstract() {
+        return new DescribedPredicate<JavaClass>("are not abstract") {
+            @Override
+            public boolean apply(JavaClass javaClass) {
+                val isAbstract = javaClass.getModifiers().stream().anyMatch((JavaModifier x) -> x == JavaModifier.ABSTRACT);
+                return !isAbstract;
+            }
+        };
+    }
+
 }