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

ahuber pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/causeway.git


The following commit(s) were added to refs/heads/main by this push:
     new 177269d8151 CAUSEWAY-3865: [Validation] don't allow Collection 
annotation on scalars and vice versa
177269d8151 is described below

commit 177269d8151e746561dd4bb63998e8908650f757
Author: Andi Huber <[email protected]>
AuthorDate: Fri Mar 14 07:35:37 2025 +0100

    CAUSEWAY-3865: [Validation] don't allow Collection annotation on scalars
    and vice versa
---
 .../progmodel/ProgrammingModelConstants.java       |  3 ++-
 .../CollectionAnnotationFacetFactory.java          | 20 +++++++++------
 .../property/PropertyAnnotationFacetFactory.java   | 20 +++++++++------
 .../postprocessors/all/SanityChecksValidator.java  |  2 +-
 .../validator/ValidationFailureUtils.java          | 28 +++++++++++++++------
 ...tion.java => InvalidAssociationAnnotation.java} | 21 +++++++++++++---
 .../domainmodel/AnnotationSyntesizerTest.java      |  4 +--
 .../DomainModelTest_usingBadDomain.java            | 29 +++++++++++++++-------
 8 files changed, 88 insertions(+), 39 deletions(-)

diff --git 
a/core/config/src/main/java/org/apache/causeway/core/config/progmodel/ProgrammingModelConstants.java
 
b/core/config/src/main/java/org/apache/causeway/core/config/progmodel/ProgrammingModelConstants.java
index ce3546c81a2..54da4c83252 100644
--- 
a/core/config/src/main/java/org/apache/causeway/core/config/progmodel/ProgrammingModelConstants.java
+++ 
b/core/config/src/main/java/org/apache/causeway/core/config/progmodel/ProgrammingModelConstants.java
@@ -518,7 +518,8 @@ public static enum MessageTemplate {
                 + "Can use @ParameterTuple only on parameter of a single arg 
action."),
         PARAMETER_TUPLE_TYPE_WITH_AMBIGUOUS_CONSTRUCTORS("${type}#${member}: "
                 + "Tuple type ${patType} referenced by @ParameterTuple 
annotated parameter has no or more than one public constructor."),
-        INVALID_MEMBER_ELEMENT_TYPE("${type}: has a member with vetoed, mixin 
or managed "
+        MEMBER_INVALID_ANNOTATION("Use of ${annot} is not allowed on 
${member}."),
+        MEMBER_INVALID_ELEMENT_TYPE("${type}: has a member with vetoed, mixin 
or managed "
                 + "element-type ${elementType}, which is not allowed; (allowed 
types are abstract, value, viewmodel and entity)"),
         MEMBER_ID_CLASH("${type}: has members using the same member-id "
                 + "'${memberId}', which is not allowed; 
clashes:\n\t[1]${member1}\n\t[2]${member2}"),
diff --git 
a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/collections/collection/CollectionAnnotationFacetFactory.java
 
b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/collections/collection/CollectionAnnotationFacetFactory.java
index 38a4336fae5..83c0766c7ba 100644
--- 
a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/collections/collection/CollectionAnnotationFacetFactory.java
+++ 
b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/collections/collection/CollectionAnnotationFacetFactory.java
@@ -42,18 +42,24 @@ public class CollectionAnnotationFacetFactory
 
     @Inject
     public CollectionAnnotationFacetFactory(final MetaModelContext mmc) {
-        super(mmc, FeatureType.COLLECTIONS_AND_ACTIONS);
+        super(mmc, FeatureType.MEMBERS);
     }
 
     @Override
     public void process(final ProcessMethodContext processMethodContext) {
-
         var collectionIfAny = collectionIfAny(processMethodContext);
 
-        if(processMethodContext.isMixinMain()) {
-            collectionIfAny.ifPresent(collection->{
-                inferMixinSort(collection, 
processMethodContext.getFacetHolder());
-            });
+        if(processMethodContext.getFeatureType().isProperty()) {
+            if(collectionIfAny.isPresent()) {
+                ValidationFailureUtils
+                    
.raiseMemberInvalidAnnotation(processMethodContext.getFacetHolder(), 
Collection.class);
+            }
+            return;
+        }
+
+        if(processMethodContext.isMixinMain()
+                && collectionIfAny.isPresent()) {
+            inferMixinSort(processMethodContext.getFacetHolder());
         }
 
         processDomainEvent(processMethodContext, collectionIfAny);
@@ -68,7 +74,7 @@ Optional<Collection> collectionIfAny(final 
ProcessMethodContext processMethodCon
                     
.raiseAmbiguousMixinAnnotations(processMethodContext.getFacetHolder(), 
Collection.class));
     }
 
-    void inferMixinSort(final Collection collection, final FacetedMethod 
facetedMethod) {
+    void inferMixinSort(final FacetedMethod facetedMethod) {
         /* if @Collection detected on method or type level infer:
          * @Action(semantics=SAFE) */
         addFacet(new ActionSemanticsFacet("InferSafeForMixedInCollection", 
SemanticsOf.SAFE, facetedMethod));
diff --git 
a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/properties/property/PropertyAnnotationFacetFactory.java
 
b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/properties/property/PropertyAnnotationFacetFactory.java
index 8a6cf0e8e32..1e1847acc32 100644
--- 
a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/properties/property/PropertyAnnotationFacetFactory.java
+++ 
b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/properties/property/PropertyAnnotationFacetFactory.java
@@ -60,18 +60,24 @@ public class PropertyAnnotationFacetFactory
 
     @Inject
     public PropertyAnnotationFacetFactory(final MetaModelContext mmc) {
-        super(mmc, FeatureType.PROPERTIES_AND_ACTIONS);
+        super(mmc, FeatureType.MEMBERS);
     }
 
     @Override
     public void process(final ProcessMethodContext processMethodContext) {
-
         var propertyIfAny = propertyIfAny(processMethodContext);
 
-        if(processMethodContext.isMixinMain()) {
-            propertyIfAny.ifPresent(property->{
-                inferMixinSort(property, 
processMethodContext.getFacetHolder());
-            });
+        if(processMethodContext.getFeatureType().isCollection()) {
+            if(propertyIfAny.isPresent()) {
+                ValidationFailureUtils
+                    
.raiseMemberInvalidAnnotation(processMethodContext.getFacetHolder(), 
Property.class);
+            }
+            return;
+        }
+
+        if(processMethodContext.isMixinMain()
+                && propertyIfAny.isPresent()) {
+            inferMixinSort(processMethodContext.getFacetHolder());
         }
 
         processDomainEvent(processMethodContext, propertyIfAny);
@@ -96,7 +102,7 @@ Optional<Property> propertyIfAny(final ProcessMethodContext 
processMethodContext
                         
.raiseAmbiguousMixinAnnotations(processMethodContext.getFacetHolder(), 
Property.class));
     }
 
-    void inferMixinSort(final Property property, final FacetedMethod 
facetedMethod) {
+    void inferMixinSort(final FacetedMethod facetedMethod) {
         /* if @Property detected on method or type level infer:
          * @Action(semantics=SAFE) */
         addFacet(new ActionSemanticsFacet("InferSafeForMixedInProperty", 
SemanticsOf.SAFE, facetedMethod));
diff --git 
a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/SanityChecksValidator.java
 
b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/SanityChecksValidator.java
index d51959c2afc..854e96c6d04 100644
--- 
a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/SanityChecksValidator.java
+++ 
b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/SanityChecksValidator.java
@@ -98,7 +98,7 @@ private void checkElementType(
         if(elementType == null
                 || 
!elementType.getBeanSort().policy().isAllowedAsMemberElementType()) {
 
-            ValidationFailureUtils.raiseInvalidMemberElementType(facetHolder, 
declaringType, elementType);
+            ValidationFailureUtils.raiseMemberInvalidElementType(facetHolder, 
declaringType, elementType);
         }
     }
 
diff --git 
a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/validator/ValidationFailureUtils.java
 
b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/validator/ValidationFailureUtils.java
index 2dd20d4bcc7..441f79de816 100644
--- 
a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/validator/ValidationFailureUtils.java
+++ 
b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/validator/ValidationFailureUtils.java
@@ -20,6 +20,7 @@
 
 import java.lang.annotation.Annotation;
 
+import org.apache.causeway.applib.Identifier;
 import org.apache.causeway.core.config.progmodel.ProgrammingModelConstants;
 import org.apache.causeway.core.metamodel.facetapi.FacetHolder;
 import org.apache.causeway.core.metamodel.facets.FacetedMethod;
@@ -34,8 +35,7 @@ public final class ValidationFailureUtils {
     public <A extends Annotation> void raiseAmbiguousMixinAnnotations(
             final FacetedMethod holder,
             final Class<A> annotationType) {
-
-        ValidationFailure.raiseFormatted(holder,
+        ValidationFailure.raise(holder,
                 
ProgrammingModelConstants.MessageTemplate.AMBIGUOUS_MIXIN_ANNOTATIONS
                     .builder()
                     .addVariable("annot", "@" + annotationType.getSimpleName())
@@ -47,8 +47,7 @@ public void raiseMemberIdClash(
             final ObjectSpecification declaringType,
             final ObjectMember memberA,
             final ObjectMember memberB) {
-
-        ValidationFailure.raiseFormatted(memberB,
+        ValidationFailure.raise(memberB,
                 ProgrammingModelConstants.MessageTemplate.MEMBER_ID_CLASH
                     .builder()
                     .addVariable("type", declaringType.fqcn())
@@ -58,16 +57,29 @@ public void raiseMemberIdClash(
                     .buildMessage());
     }
 
-    public void raiseInvalidMemberElementType(
+    public void raiseMemberInvalidElementType(
             final FacetHolder facetHolder,
             final ObjectSpecification declaringType,
             final ObjectSpecification elementType) {
-        ValidationFailure.raiseFormatted(facetHolder,
-                
ProgrammingModelConstants.MessageTemplate.INVALID_MEMBER_ELEMENT_TYPE
-                    .builder()
+        ValidationFailure.raise(facetHolder,
+                
ProgrammingModelConstants.MessageTemplate.MEMBER_INVALID_ELEMENT_TYPE.builder()
                     .addVariable("type", declaringType.fqcn())
                     .addVariable("elementType", ""+elementType)
                     .buildMessage());
     }
 
+    public <A extends Annotation> void raiseMemberInvalidAnnotation(
+            final FacetedMethod facetedMethod,
+            final Class<A> annotationType) {
+        ValidationFailure.raise(facetedMethod, 
formatMemberInvalidAnnotation(facetedMethod.getFeatureIdentifier(), 
annotationType));
+    }
+    public <A extends Annotation> String formatMemberInvalidAnnotation(
+            final Identifier identifier,
+            final Class<A> annotationType) {
+        return 
ProgrammingModelConstants.MessageTemplate.MEMBER_INVALID_ANNOTATION.builder()
+            .addVariable("member", identifier.getFullIdentityString())
+            .addVariable("annot", "@" + annotationType.getSimpleName())
+            .buildMessage();
+    }
+
 }
diff --git 
a/regressiontests/base/src/main/java/org/apache/causeway/testdomain/model/bad/InvalidPropertyAnnotationOnAction.java
 
b/regressiontests/base/src/main/java/org/apache/causeway/testdomain/model/bad/InvalidAssociationAnnotation.java
similarity index 73%
rename from 
regressiontests/base/src/main/java/org/apache/causeway/testdomain/model/bad/InvalidPropertyAnnotationOnAction.java
rename to 
regressiontests/base/src/main/java/org/apache/causeway/testdomain/model/bad/InvalidAssociationAnnotation.java
index e8ad49b33eb..2bfaa814b53 100644
--- 
a/regressiontests/base/src/main/java/org/apache/causeway/testdomain/model/bad/InvalidPropertyAnnotationOnAction.java
+++ 
b/regressiontests/base/src/main/java/org/apache/causeway/testdomain/model/bad/InvalidAssociationAnnotation.java
@@ -18,20 +18,33 @@
  */
 package org.apache.causeway.testdomain.model.bad;
 
+import java.util.List;
+
 import org.apache.causeway.applib.annotation.Action;
+import org.apache.causeway.applib.annotation.Collection;
 import org.apache.causeway.applib.annotation.DomainObject;
 import org.apache.causeway.applib.annotation.Nature;
 import org.apache.causeway.applib.annotation.Property;
 import org.apache.causeway.applib.value.Blob;
 
-@DomainObject(nature = Nature.VIEW_MODEL)
-public class InvalidPropertyAnnotationOnAction {
+import lombok.Getter;
+import lombok.Setter;
 
-    // TODO as this is no getter representing a property, @Property should not 
be allowed here
+@DomainObject(nature = Nature.VIEW_MODEL)
+public class InvalidAssociationAnnotation {
 
-    @Action @Property(fileAccept=".xlsx")
+    @Property(fileAccept=".xlsx") // TODO as this is no getter representing a 
property, @Property should not be allowed here
+    @Action
     public Blob exportToJson() {
         return null;
     }
 
+    @Collection // invalid
+    @Getter @Setter
+    private String singular;
+
+    @Property // invalid
+    @Getter @Setter
+    private List<String> plural;
+
 }
diff --git 
a/regressiontests/domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/AnnotationSyntesizerTest.java
 
b/regressiontests/domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/AnnotationSyntesizerTest.java
index b4728016024..3549fc1ff23 100644
--- 
a/regressiontests/domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/AnnotationSyntesizerTest.java
+++ 
b/regressiontests/domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/AnnotationSyntesizerTest.java
@@ -39,7 +39,7 @@
 import org.apache.causeway.schema.metamodel.v2.DomainClassDto;
 import org.apache.causeway.testdomain.conf.Configuration_headless;
 import 
org.apache.causeway.testdomain.model.bad.Configuration_usingInvalidDomain;
-import 
org.apache.causeway.testdomain.model.bad.InvalidPropertyAnnotationOnAction;
+import org.apache.causeway.testdomain.model.bad.InvalidAssociationAnnotation;
 import 
org.apache.causeway.testdomain.model.good.Configuration_usingValidDomain;
 
 @SpringBootTest(
@@ -65,7 +65,7 @@ class AnnotationSyntesizerTest {
     @Test
     void propertyAnnotationOnAction_shouldNotContributeToSynthesizedAction() {
 
-        var actionMethod = 
ReflectionUtils.findMethod(InvalidPropertyAnnotationOnAction.class, 
"exportToJson");
+        var actionMethod = 
ReflectionUtils.findMethod(InvalidAssociationAnnotation.class, "exportToJson");
         assertNotNull(actionMethod);
 
         var action = _Annotations.synthesize(actionMethod, Action.class).get();
diff --git 
a/regressiontests/domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/DomainModelTest_usingBadDomain.java
 
b/regressiontests/domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/DomainModelTest_usingBadDomain.java
index 3fcd5518320..25dc7621af4 100644
--- 
a/regressiontests/domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/DomainModelTest_usingBadDomain.java
+++ 
b/regressiontests/domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/DomainModelTest_usingBadDomain.java
@@ -24,7 +24,6 @@
 import jakarta.inject.Inject;
 
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
@@ -56,6 +55,7 @@
 import 
org.apache.causeway.core.config.progmodel.ProgrammingModelConstants.MessageTemplate;
 import org.apache.causeway.core.metamodel.spec.feature.MixedIn;
 import org.apache.causeway.core.metamodel.specloader.SpecificationLoader;
+import 
org.apache.causeway.core.metamodel.specloader.validator.ValidationFailureUtils;
 import org.apache.causeway.testdomain.conf.Configuration_headless;
 import org.apache.causeway.testdomain.model.bad.AmbiguousMixinAnnotations;
 import org.apache.causeway.testdomain.model.bad.AmbiguousTitle;
@@ -71,7 +71,7 @@
 import org.apache.causeway.testdomain.model.bad.InvalidOrphanedActionSupport;
 import 
org.apache.causeway.testdomain.model.bad.InvalidOrphanedCollectionSupport;
 import org.apache.causeway.testdomain.model.bad.InvalidOrphanedPropertySupport;
-import 
org.apache.causeway.testdomain.model.bad.InvalidPropertyAnnotationOnAction;
+import org.apache.causeway.testdomain.model.bad.InvalidAssociationAnnotation;
 import org.apache.causeway.testdomain.model.bad.InvalidServiceWithAlias;
 import org.apache.causeway.testdomain.model.bad.OrphanedMemberSupportDetection;
 import 
org.apache.causeway.testdomain.util.interaction.DomainObjectTesterFactory;
@@ -389,6 +389,24 @@ void memberIdClash() {
             validator.assertAnyFailuresContaining(InvalidMemberIdClash.class, 
expectedMessageChunk));
     }
 
+    // -- MEMBER-FEATURE-TYPE
+
+    @Test
+    void invalidPropertyAnnotationOnCollection_shouldFail() {
+        var origin = 
Identifier.propertyIdentifier(LogicalType.fqcn(InvalidAssociationAnnotation.class),
 "singular");
+        validator.assertAnyFailuresContaining(
+            origin,
+            ValidationFailureUtils.formatMemberInvalidAnnotation(origin, 
Collection.class));
+    }
+
+    @Test
+    void invalidCollectionAnnotationOnProperty_shouldFail() {
+        var origin = 
Identifier.propertyIdentifier(LogicalType.fqcn(InvalidAssociationAnnotation.class),
 "plural");
+        validator.assertAnyFailuresContaining(
+            origin,
+            ValidationFailureUtils.formatMemberInvalidAnnotation(origin, 
Property.class));
+    }
+
     // -- ELEMENT-TYPE
 
     @ParameterizedTest
@@ -434,13 +452,6 @@ void invalidMixinDeclaration(final Class<?> 
classUnderTest) {
 
     // -- INCUBATING
 
-    @Test @Disabled("this case has no vaildation refiner yet")
-    void invalidPropertyAnnotationOnAction_shouldFail() {
-        validator.assertAnyFailuresContaining(
-                
Identifier.classIdentifier(LogicalType.fqcn(InvalidPropertyAnnotationOnAction.class)),
-                "TODO");
-    }
-
 //    @Test
 //    void orphanedActionSupportNotEnforced_shouldFail() {
 //

Reply via email to