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() {
//