From 1d30c731e6c7a31efd95030fb125720c54a9ddf6 Mon Sep 17 00:00:00 2001 From: Andi Huber Date: Thu, 25 May 2023 10:15:25 +0200 Subject: [PATCH 1/6] CAUSEWAY-3444: refactor MixinFacet to also provide info whether act/pro/coll (WIP) --- .../applib/services/metamodel/BeanSort.java | 8 +- .../facets/object/mixin/MixinFacet.java | 34 ++++++- .../object/mixin/MixinFacetAbstract.java | 25 ++--- .../MixinFacetForDomainObjectAnnotation.java | 2 +- .../all/SanityChecksValidator2.java | 91 +++++++++++++++++++ .../dflt/ProgrammingModelFacetsJava11.java | 3 + .../metamodel/spec/ObjectSpecification.java | 8 +- .../specimpl/ObjectSpecificationAbstract.java | 26 +++++- .../validator/MetaModelValidator.java | 6 +- .../object/mixin/MixinFacetAbstract_Test.java | 4 +- .../DomainModelTest_usingBadDomain.java | 25 +++-- .../model/bad/InvalidMixinDeclarations.java | 56 ++++++++++++ 12 files changed, 259 insertions(+), 29 deletions(-) create mode 100644 core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/SanityChecksValidator2.java create mode 100644 regressiontests/stable/src/main/java/org/apache/causeway/testdomain/model/bad/InvalidMixinDeclarations.java diff --git a/api/applib/src/main/java/org/apache/causeway/applib/services/metamodel/BeanSort.java b/api/applib/src/main/java/org/apache/causeway/applib/services/metamodel/BeanSort.java index 08c7cfe0f76..5490a6e4bc3 100644 --- a/api/applib/src/main/java/org/apache/causeway/applib/services/metamodel/BeanSort.java +++ b/api/applib/src/main/java/org/apache/causeway/applib/services/metamodel/BeanSort.java @@ -20,7 +20,6 @@ import org.springframework.context.annotation.Profile; -import org.apache.causeway.applib.annotation.Domain; import org.apache.causeway.applib.annotation.Programmatic; /** @@ -59,8 +58,9 @@ public enum BeanSort { */ MANAGED_BEAN_NOT_CONTRIBUTING, /** - * Object associated with an 'entity' or 'bean' to act as contributer of - * domain actions or properties. Might also be stateful similar to VIEW_MODEL. + * Object associated with an entity, viewmodel or domain-service + * to act as contributer of a single domain-action or + * domain-property or domain-collection. */ MIXIN, /** @@ -77,7 +77,7 @@ public enum BeanSort { ABSTRACT, /** * Type must not be added to the meta-model, eg. by means of - * {@link Domain#Exclude}, {@link Profile} or {@link Programmatic} + * {@link org.apache.causeway.applib.annotation.Domain.Exclude}, {@link Profile} or {@link Programmatic} */ VETOED, UNKNOWN; diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacet.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacet.java index 50fc64d3649..965ab2247eb 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacet.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacet.java @@ -25,7 +25,7 @@ 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.core.metamodel.facets.SingleValueFacet; +import org.apache.causeway.core.metamodel.facetapi.Facet; import org.apache.causeway.core.metamodel.spec.ObjectSpecification; /** @@ -40,7 +40,37 @@ * the type recognized as a mix-in. These are {@link Action}, {@link Property} and * {@link Collection}. */ -public interface MixinFacet extends SingleValueFacet { +public interface MixinFacet extends Facet { + + public enum MixinSort { + /** + * FIXME refactoring step + */ + @Deprecated + AUTO, + /** + * Object associated with an entity, viewmodel or domain-service + * to act as contributer of a single domain-action. + */ + MIXIN_FOR_ACT, + /** + * Object associated with an entity, viewmodel or domain-service + * to act as contributer of a single domain-property. + */ + MIXIN_PROP, + /** + * Object associated with an entity, viewmodel or domain-service + * to act as contributer of a single domain-collection. + */ + MIXIN_COLL, + } + + MixinSort getMixinSort(); + + /** + * The mixin's main method name. + */ + String getMainMethodName(); boolean isMixinFor(Class candidateDomainType); diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract.java index 26f06d0acd1..c708b2871e8 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract.java @@ -25,17 +25,23 @@ import org.apache.causeway.commons.internal.exceptions._Exceptions; import org.apache.causeway.core.metamodel.facetapi.Facet; +import org.apache.causeway.core.metamodel.facetapi.FacetAbstract; import org.apache.causeway.core.metamodel.facetapi.FacetHolder; -import org.apache.causeway.core.metamodel.facets.SingleValueFacetAbstract; +import lombok.Getter; import lombok.NonNull; import lombok.val; //@Log4j2 public abstract class MixinFacetAbstract -extends SingleValueFacetAbstract +extends FacetAbstract implements MixinFacet { + @Getter(onMethod_={@Override}) + private final @NonNull String mainMethodName; + @Getter(onMethod_={@Override}) + private final @NonNull MixinSort mixinSort; + private final @NonNull Class mixinType; private final @NonNull Class holderType; private final @NonNull Constructor constructor; @@ -46,12 +52,15 @@ private static final Class type() { protected MixinFacetAbstract( final Class mixinType, + final MixinSort mixinSort, final String mainMethodName, final Constructor constructor, final FacetHolder holder) { - super(type(), mainMethodName, holder); + super(type(), holder); + this.mainMethodName = mainMethodName; this.mixinType = mixinType; + this.mixinSort = mixinSort; this.constructor = constructor; // by mixin convention: first constructor argument is identified as the holder type this.holderType = constructor.getParameterTypes()[0]; @@ -113,15 +122,9 @@ public boolean isCandidateForMain(final Method method) { public void visitAttributes(final BiConsumer visitor) { super.visitAttributes(visitor); visitor.accept("mixinType", mixinType); + visitor.accept("mixinSort", mixinSort); + visitor.accept("mainMethodName", mainMethodName); visitor.accept("holderType", holderType); } - /** - * The mixin's main method name. - * @implNote as stored in the SingleValueFacetAbstract's value field - */ - public String getMainMethodName() { - return super.value(); - } - } diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetForDomainObjectAnnotation.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetForDomainObjectAnnotation.java index 6578a62ce83..bd18176f0a4 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetForDomainObjectAnnotation.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetForDomainObjectAnnotation.java @@ -60,6 +60,6 @@ private MixinFacetForDomainObjectAnnotation( final String mixinMethodName, final Constructor constructor, final FacetHolder holder) { - super(mixinClass, mixinMethodName, constructor, holder); + super(mixinClass, MixinSort.AUTO, mixinMethodName, constructor, holder); } } diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/SanityChecksValidator2.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/SanityChecksValidator2.java new file mode 100644 index 00000000000..ca0af7e6d98 --- /dev/null +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/SanityChecksValidator2.java @@ -0,0 +1,91 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.causeway.core.metamodel.postprocessors.all; + +import javax.inject.Inject; + +import org.apache.causeway.core.metamodel.context.MetaModelContext; +import org.apache.causeway.core.metamodel.spec.ObjectSpecification; +import org.apache.causeway.core.metamodel.spec.feature.ObjectAction; +import org.apache.causeway.core.metamodel.spec.feature.OneToManyAssociation; +import org.apache.causeway.core.metamodel.spec.feature.OneToOneAssociation; +import org.apache.causeway.core.metamodel.specloader.validator.MetaModelValidator; +import org.apache.causeway.core.metamodel.specloader.validator.MetaModelValidatorAbstract; + +/** + * Checks various preconditions for a sane meta-model. + *
    + * + *
  • Guard against members that contribute vetoed or managed types. + * Those are not allowed as member/return/param.
    • + *
+ */ +public class SanityChecksValidator2 +extends MetaModelValidatorAbstract +implements + MetaModelValidator.ActionValidator, + MetaModelValidator.PropertyValidator, + MetaModelValidator.CollectionValidator { + + @Inject + public SanityChecksValidator2(final MetaModelContext mmc) { + super(mmc, MIXINS); + } + + @Override + public void validateObjectEnter(final ObjectSpecification objSpec) { + System.err.printf("mixin %s%n", objSpec); + } + + @Override + public void validateObjectExit(final ObjectSpecification objSpec) { + } + + @Override + public void validateAction(final ObjectSpecification objectSpecification, final ObjectAction objectAction) { + + } + + @Override + public void validateProperty(final ObjectSpecification objectSpecification, final OneToOneAssociation prop) { + + } + + @Override + public void validateCollection(final ObjectSpecification objectSpecification, final OneToManyAssociation coll) { + + } + + // -- HELPER + +// private void checkElementType( +// final FacetHolder facetHolder, +// final ObjectSpecification declaringType, +// final ObjectSpecification elementType) { +// +// if(elementType == null +// || elementType.getBeanSort().isManagedBeanAny() +// || elementType.getBeanSort().isMixin() +// || elementType.getBeanSort().isVetoed()) { +// +// ValidationFailureUtils.raiseInvalidMemberElementType(facetHolder, declaringType, elementType); +// } +// } + +} diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/progmodels/dflt/ProgrammingModelFacetsJava11.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/progmodels/dflt/ProgrammingModelFacetsJava11.java index 4c4028518fc..39667a9a0e1 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/progmodels/dflt/ProgrammingModelFacetsJava11.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/progmodels/dflt/ProgrammingModelFacetsJava11.java @@ -86,6 +86,7 @@ import org.apache.causeway.core.metamodel.postprocessors.all.CssOnActionFromConfiguredRegexPostProcessor; import org.apache.causeway.core.metamodel.postprocessors.all.DescribedAsFromTypePostProcessor; import org.apache.causeway.core.metamodel.postprocessors.all.SanityChecksValidator; +import org.apache.causeway.core.metamodel.postprocessors.all.SanityChecksValidator2; import org.apache.causeway.core.metamodel.postprocessors.all.i18n.SynthesizeObjectNamingPostProcessor; import org.apache.causeway.core.metamodel.postprocessors.all.i18n.TranslationPostProcessor; import org.apache.causeway.core.metamodel.postprocessors.allbutparam.authorization.AuthorizationPostProcessor; @@ -272,6 +273,8 @@ private void addValidators() { addValidator(ValidationOrder.A1_BUILTIN, new ActionAnnotationShouldEnforceConcreteTypeToBeIncludedWithMetamodelValidator(mmc)); addValidator(ValidationOrder.A1_BUILTIN, new ActionOverloadingValidator(mmc)); addValidator(ValidationOrder.A1_BUILTIN, new LogicalTypeMalformedValidator(mmc)); + + addValidator(ValidationOrder.A1_BUILTIN, new SanityChecksValidator2(mmc)); } } diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/spec/ObjectSpecification.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/spec/ObjectSpecification.java index ea564878f30..b2175fb5d84 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/spec/ObjectSpecification.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/spec/ObjectSpecification.java @@ -59,6 +59,7 @@ import org.apache.causeway.core.metamodel.facets.object.icon.IconFacet; import org.apache.causeway.core.metamodel.facets.object.icon.ObjectIcon; import org.apache.causeway.core.metamodel.facets.object.immutable.ImmutableFacet; +import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet.MixinSort; import org.apache.causeway.core.metamodel.facets.object.parented.ParentedCollectionFacet; import org.apache.causeway.core.metamodel.facets.object.title.TitleFacet; import org.apache.causeway.core.metamodel.facets.object.title.TitleRenderRequest; @@ -285,11 +286,16 @@ public default Optional lookupMixedInAction(final ObjectSpe Optional getElementSpecification(); /** - * * @since 2.0 */ BeanSort getBeanSort(); + /** + * @since 2.0 + */ + Optional getMixinSort(); + + // ////////////////////////////////////////////////////////////// // TitleContext // ////////////////////////////////////////////////////////////// diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/ObjectSpecificationAbstract.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/ObjectSpecificationAbstract.java index e19a25dc6e1..0f7e2278cb5 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/ObjectSpecificationAbstract.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/ObjectSpecificationAbstract.java @@ -67,6 +67,7 @@ import org.apache.causeway.core.metamodel.facets.object.immutable.ImmutableFacet; import org.apache.causeway.core.metamodel.facets.object.logicaltype.AliasedFacet; import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet; +import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet.MixinSort; import org.apache.causeway.core.metamodel.facets.object.navparent.NavigableParentFacet; import org.apache.causeway.core.metamodel.facets.object.parented.ParentedCollectionFacet; import org.apache.causeway.core.metamodel.facets.object.title.TitleFacet; @@ -93,6 +94,7 @@ import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.val; +import lombok.experimental.Accessors; import lombok.extern.log4j.Log4j2; @EqualsAndHashCode(of = "correspondingClass", callSuper = false) @@ -533,6 +535,11 @@ public String getHelp() { return helpFacet == null ? null : helpFacet.value(); } + @Override + public final Optional getMixinSort() { + return lookupMixinFacet() + .map(MixinFacet::getMixinSort); + } // -- FACET HANDLING @@ -565,6 +572,21 @@ public Q getFacet(final Class facetType) { } } + @Getter(lazy = true) @Accessors(fluent=true) + private final Optional mixinFacet = lookupMixinFacet(); + + private Optional lookupMixinFacet() { + if(!isMixin()) { + return Optional.empty(); + } + val mixinFacet = getFacet(MixinFacet.class); + if(mixinFacet==null) { + throw _Exceptions.illegalState("type %s has BeanSort MIXIN but ended up NOT having a MixinFacet", + getCorrespondingClass()); + } + return Optional.of(mixinFacet); + } + @Domain.Exclude private static class NotANoopFacetFilter implements Predicate { Q noopFacet; @@ -736,7 +758,7 @@ private Stream createMixedInAssociation(final Class mixinT if(!mixinFacet.isMixinFor(getCorrespondingClass())) { return Stream.empty(); } - val mixinMethodName = mixinFacet.value(); + val mixinMethodName = mixinFacet.getMainMethodName(); return specification.streamActions(ActionScope.ANY, MixedIn.INCLUDED) .filter(_SpecPredicates::isMixedInAssociation) @@ -775,7 +797,7 @@ private Stream createMixedInAction(final Class mixinType return Stream.empty(); } - val mixinMethodName = mixinFacet.value(); + val mixinMethodName = mixinFacet.getMainMethodName(); return mixinSpec.streamActions(ActionScope.ANY, MixedIn.EXCLUDED) // value types only support constructor mixins diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/validator/MetaModelValidator.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/validator/MetaModelValidator.java index 34a4360a2e4..93588249562 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/validator/MetaModelValidator.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/validator/MetaModelValidator.java @@ -99,10 +99,14 @@ static interface CollectionValidator { public final static Predicate SKIP_MANAGED_BEANS = spec->!spec.isInjectable(); - /** types pass this filter, if not a mixin */ + /** types pass this filter, if is NOT a mixin */ public final static Predicate SKIP_MIXINS = spec->!spec.isMixin(); + /** types pass this filter, if IS a mixin */ + public final static Predicate MIXINS = + spec->spec.isMixin(); + /** types pass this filter, if either not {@link ObjectSpecificationAbstract} or member-annotation is not required */ public final static Predicate SKIP_WHEN_MEMBER_ANNOT_REQUIRED = spec->(!(spec instanceof ObjectSpecificationAbstract) diff --git a/core/metamodel/src/test/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract_Test.java b/core/metamodel/src/test/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract_Test.java index 00a8036cfd2..de751d38a37 100644 --- a/core/metamodel/src/test/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract_Test.java +++ b/core/metamodel/src/test/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract_Test.java @@ -21,6 +21,8 @@ import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; +import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet.MixinSort; + import lombok.val; class MixinFacetAbstract_Test { @@ -41,7 +43,7 @@ void happy_case() throws Exception { // given val constructor = Collection_numberOfChildren.class.getConstructor(Object.class); val facet = new MixinFacetAbstract( - Collection_numberOfChildren.class, "prop", constructor, null) {}; + Collection_numberOfChildren.class, MixinSort.MIXIN_PROP, "prop", constructor, null) {}; val propMethodInSubclass = SimpleObject_numberOfChildren.class.getMethod("prop"); diff --git a/regressiontests/stable-domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/DomainModelTest_usingBadDomain.java b/regressiontests/stable-domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/DomainModelTest_usingBadDomain.java index 3e781a7c4ed..c738f02adce 100644 --- a/regressiontests/stable-domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/DomainModelTest_usingBadDomain.java +++ b/regressiontests/stable-domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/DomainModelTest_usingBadDomain.java @@ -65,6 +65,7 @@ import org.apache.causeway.testdomain.model.bad.InvalidElementTypes; import org.apache.causeway.testdomain.model.bad.InvalidMemberIdClash; import org.apache.causeway.testdomain.model.bad.InvalidMemberOverloadingWhenInherited; +import org.apache.causeway.testdomain.model.bad.InvalidMixinDeclarations; import org.apache.causeway.testdomain.model.bad.InvalidObjectWithAlias; import org.apache.causeway.testdomain.model.bad.InvalidOrphanedActionSupport; import org.apache.causeway.testdomain.model.bad.InvalidOrphanedCollectionSupport; @@ -126,7 +127,7 @@ void ambiguousTitle_shouldFail() { void orphanedActionSupport_shouldFail() { validator.assertAnyFailuresContaining( Identifier.classIdentifier(LogicalType.fqcn(InvalidOrphanedActionSupport.class)), - validationMessage( + unsatisfiedDomainIncludeSemantics( "InvalidOrphanedActionSupport", "hideOrphaned()")); @@ -141,7 +142,7 @@ void orphanedActionSupport_shouldFail() { void orphanedPropertySupport_shouldFail() { validator.assertAnyFailuresContaining( Identifier.classIdentifier(LogicalType.fqcn(InvalidOrphanedPropertySupport.class)), - validationMessage( + unsatisfiedDomainIncludeSemantics( "InvalidOrphanedPropertySupport", "hideMyProperty()")); @@ -155,7 +156,7 @@ void orphanedPropertySupport_shouldFail() { void orphanedCollectionSupport_shouldFail() { validator.assertAnyFailuresContaining( Identifier.classIdentifier(LogicalType.fqcn(InvalidOrphanedCollectionSupport.class)), - validationMessage( + unsatisfiedDomainIncludeSemantics( "InvalidOrphanedCollectionSupport", "hideMyCollection()")); @@ -208,14 +209,14 @@ void actionOverloading_shouldFail() { validator.assertAnyFailuresContaining( Identifier.classIdentifier(LogicalType.fqcn( InvalidMemberOverloadingWhenInherited.WhenAnnotationRequired.class)), - validationMessage( + unsatisfiedDomainIncludeSemantics( "", "isActive()")); validator.assertAnyFailuresContaining( Identifier.classIdentifier(LogicalType.fqcn( InvalidMemberOverloadingWhenInherited.WhenEncapsulationEnabled.class)), - validationMessage( + unsatisfiedDomainIncludeSemantics( "", "isActive()")); } @@ -405,6 +406,18 @@ void invalidElementType(final Class classUnderTest) { "has a member with vetoed, mixin or managed element-type"); } + // -- MIXINS + + @ParameterizedTest + @ValueSource(classes = { + InvalidMixinDeclarations.ActionMixinWithProp.class, + }) + void invalidMixinDeclaration(final Class classUnderTest) { + validator.assertAnyFailuresContaining( + classUnderTest, + "has a member with vetoed, mixin or managed element-type"); + } + // -- INCUBATING @Test @Disabled("this case has no vaildation refiner yet") @@ -427,7 +440,7 @@ void invalidPropertyAnnotationOnAction_shouldFail() { // -- HELPER - private String validationMessage( + private String unsatisfiedDomainIncludeSemantics( final String className, final String memberName) { return Violation.UNSATISFIED_DOMAIN_INCLUDE_SEMANTICS diff --git a/regressiontests/stable/src/main/java/org/apache/causeway/testdomain/model/bad/InvalidMixinDeclarations.java b/regressiontests/stable/src/main/java/org/apache/causeway/testdomain/model/bad/InvalidMixinDeclarations.java new file mode 100644 index 00000000000..b0a196185be --- /dev/null +++ b/regressiontests/stable/src/main/java/org/apache/causeway/testdomain/model/bad/InvalidMixinDeclarations.java @@ -0,0 +1,56 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.causeway.testdomain.model.bad; + +import javax.inject.Named; + +import org.apache.causeway.applib.annotation.Action; +import org.apache.causeway.applib.annotation.ActionLayout; +import org.apache.causeway.applib.annotation.DomainObject; +import org.apache.causeway.applib.annotation.Introspection; +import org.apache.causeway.applib.annotation.MemberSupport; +import org.apache.causeway.applib.annotation.Nature; + +import lombok.RequiredArgsConstructor; + +@DomainObject(nature = Nature.VIEW_MODEL, introspection = Introspection.ANNOTATION_REQUIRED) +@Named("testdomain.InvalidMixinDeclarations") +public class InvalidMixinDeclarations { + + // -- INVALID SCENARIOS + + @Action + @ActionLayout(named = "someActionProp") + @RequiredArgsConstructor + public static class ActionMixinWithProp { + @SuppressWarnings("unused") + private final InvalidMixinDeclarations invalidElementTypes; + @MemberSupport public String prop() { return ""; } + } +/* + @Action + @ActionLayout(named = "someActionColl") + @RequiredArgsConstructor + public static class ActionMixinWithColl { + @SuppressWarnings("unused") + private final InvalidMixinDeclarations invalidElementTypes; + @MemberSupport public String coll() { return ""; } + }*/ + +} From ef54e9880d0ef64142022bc07c6a88a99c2debc4 Mon Sep 17 00:00:00 2001 From: Andi Huber Date: Thu, 25 May 2023 13:27:48 +0200 Subject: [PATCH 2/6] CAUSEWAY-3444: implements MixinSanityChecksValidator --- .../progmodel/ProgrammingModelConstants.java | 4 + .../facets/object/mixin/MixinFacet.java | 11 +- .../object/mixin/MixinFacetAbstract.java | 33 ++--- .../MixinFacetForDomainObjectAnnotation.java | 3 +- .../all/MixinSanityChecksValidator.java | 119 ++++++++++++++++++ .../all/SanityChecksValidator2.java | 91 -------------- .../dflt/ProgrammingModelFacetsJava11.java | 4 +- .../specimpl/ObjectSpecificationAbstract.java | 49 ++++---- .../dflt/ObjectSpecificationDefault.java | 18 +++ .../object/mixin/MixinFacetAbstract_Test.java | 4 +- .../DomainModelTest_usingBadDomain.java | 4 +- 11 files changed, 197 insertions(+), 143 deletions(-) create mode 100644 core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/MixinSanityChecksValidator.java delete mode 100644 core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/SanityChecksValidator2.java 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 7d43a134d77..c2346a735ac 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 @@ -537,6 +537,10 @@ public static enum Violation { + "'${memberId}', which is not allowed; clashes:\n\t[1]${member1}\n\t[2]${member2}"), AMBIGUOUS_MIXIN_ANNOTATIONS("Annotation ${annot} on both method and type level is not allowed, " + "it must be one or the other. Found with mixin: ${mixinType}"), + INVALID_MIXIN_TYPE("Mixin ${type} could not be identified as action, property or collection."), + INVALID_MIXIN_MAIN("Mixin ${type} does declare method name '${expectedMethodName}' as" + + " the mixin main method to use," + + " but introspection did pick up method '${actualMethodName}' instead."), ; private final String template; diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacet.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacet.java index 965ab2247eb..4987e95152d 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacet.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacet.java @@ -44,10 +44,9 @@ public interface MixinFacet extends Facet { public enum MixinSort { /** - * FIXME refactoring step + * Initial state early during introspection. */ - @Deprecated - AUTO, + UNSPECIFIED, /** * Object associated with an entity, viewmodel or domain-service * to act as contributer of a single domain-action. @@ -57,12 +56,14 @@ public enum MixinSort { * Object associated with an entity, viewmodel or domain-service * to act as contributer of a single domain-property. */ - MIXIN_PROP, + MIXIN_FOR_PROP, /** * Object associated with an entity, viewmodel or domain-service * to act as contributer of a single domain-collection. */ - MIXIN_COLL, + MIXIN_FOR_COLL; + + public boolean isUnspecified() { return this==UNSPECIFIED; } } MixinSort getMixinSort(); diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract.java index c708b2871e8..50cbd58dfe0 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract.java @@ -40,7 +40,7 @@ public abstract class MixinFacetAbstract @Getter(onMethod_={@Override}) private final @NonNull String mainMethodName; @Getter(onMethod_={@Override}) - private final @NonNull MixinSort mixinSort; + private @NonNull MixinSort mixinSort = MixinSort.UNSPECIFIED; private final @NonNull Class mixinType; private final @NonNull Class holderType; @@ -52,7 +52,6 @@ private static final Class type() { protected MixinFacetAbstract( final Class mixinType, - final MixinSort mixinSort, final String mainMethodName, final Constructor constructor, final FacetHolder holder) { @@ -60,7 +59,6 @@ protected MixinFacetAbstract( super(type(), holder); this.mainMethodName = mainMethodName; this.mixinType = mixinType; - this.mixinSort = mixinSort; this.constructor = constructor; // by mixin convention: first constructor argument is identified as the holder type this.holderType = constructor.getParameterTypes()[0]; @@ -68,11 +66,9 @@ protected MixinFacetAbstract( @Override public boolean isMixinFor(final Class candidateDomainType) { - if (candidateDomainType == null) { - return false; - } - - return holderType.isAssignableFrom(candidateDomainType); + return candidateDomainType == null + ? false + : holderType.isAssignableFrom(candidateDomainType); } @Override @@ -106,13 +102,12 @@ public Object instantiate(final Object mixee) { @Override public boolean isCandidateForMain(final Method method) { - - // include methods from super classes or interfaces - // - // it is sufficient to detect any match; - // mixin invocation will take care of calling the right method, - // that is in terms of type-hierarchy the 'nearest' to this mixin - + /* include methods from super classes or interfaces + * + * it is sufficient to detect any match; + * mixin invocation will take care of calling the right method, + * that is in terms of type-hierarchy the 'nearest' to this mixin; + */ return method.getName().equals(getMainMethodName()) && method.getDeclaringClass() .isAssignableFrom(constructor.getDeclaringClass()); @@ -127,4 +122,12 @@ public void visitAttributes(final BiConsumer visitor) { visitor.accept("holderType", holderType); } + /** + * Framework internal. + */ + public MixinFacetAbstract initMixinSort(final @NonNull MixinSort mixinSort) { + this.mixinSort = mixinSort; + return this; + } + } diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetForDomainObjectAnnotation.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetForDomainObjectAnnotation.java index bd18176f0a4..081f58ff9ec 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetForDomainObjectAnnotation.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetForDomainObjectAnnotation.java @@ -60,6 +60,7 @@ private MixinFacetForDomainObjectAnnotation( final String mixinMethodName, final Constructor constructor, final FacetHolder holder) { - super(mixinClass, MixinSort.AUTO, mixinMethodName, constructor, holder); + super(mixinClass, mixinMethodName, constructor, holder); } + } diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/MixinSanityChecksValidator.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/MixinSanityChecksValidator.java new file mode 100644 index 00000000000..43dc2f0b545 --- /dev/null +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/MixinSanityChecksValidator.java @@ -0,0 +1,119 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.causeway.core.metamodel.postprocessors.all; + +import javax.inject.Inject; + +import org.apache.causeway.applib.Identifier; +import org.apache.causeway.applib.services.metamodel.BeanSort; +import org.apache.causeway.core.config.progmodel.ProgrammingModelConstants; +import org.apache.causeway.core.metamodel.context.MetaModelContext; +import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet.MixinSort; +import org.apache.causeway.core.metamodel.spec.ObjectSpecification; +import org.apache.causeway.core.metamodel.spec.feature.ObjectAction; +import org.apache.causeway.core.metamodel.spec.feature.OneToManyAssociation; +import org.apache.causeway.core.metamodel.spec.feature.OneToOneAssociation; +import org.apache.causeway.core.metamodel.specloader.specimpl.ObjectSpecificationAbstract; +import org.apache.causeway.core.metamodel.specloader.validator.MetaModelValidator; +import org.apache.causeway.core.metamodel.specloader.validator.MetaModelValidatorAbstract; +import org.apache.causeway.core.metamodel.specloader.validator.ValidationFailure; + +import lombok.val; + +/** + * Checks various preconditions for a sane meta-model. + *
    + *
  • Guard against types that are identified as {@link BeanSort#MIXIN}, but don't contribute any member.
    • + *
  • Make sure mixins contribute either an action or a property or a collection.
    • + *
  • Make sure if a mixin main method name is specified (eg 'act', 'prop', 'coll'), + * that introspection was able to pick it up.
    • + *
+ */ +public class MixinSanityChecksValidator +extends MetaModelValidatorAbstract +implements + MetaModelValidator.ActionValidator, + MetaModelValidator.PropertyValidator, + MetaModelValidator.CollectionValidator { + + private MixinSort mixinSort; + + @Inject + public MixinSanityChecksValidator(final MetaModelContext mmc) { + super(mmc, MIXINS); + } + + @Override + public void validateObjectEnter(final ObjectSpecification objSpec) { + final MixinSort mixinSort = objSpec.getMixinSort().orElse(null); + if(mixinSort==null + || mixinSort.isUnspecified()) { + ValidationFailure.raiseFormatted(objSpec, + ProgrammingModelConstants.Violation.INVALID_MIXIN_TYPE + .builder() + .addVariable("type", objSpec.getCorrespondingClass().getName()) + .buildMessage()); + return; + } + this.mixinSort = mixinSort; + } + + @Override + public void validateObjectExit(final ObjectSpecification objSpec) { + this.mixinSort = null; + } + + @Override + public void validateAction(final ObjectSpecification objSpec, final ObjectAction act) { + if(mixinSort==null) return; // skip if already failed earlier + checkMixinMainMethod(objSpec, act.getFeatureIdentifier()); + } + + @Override + public void validateProperty(final ObjectSpecification objSpec, final OneToOneAssociation prop) { + if(mixinSort==null) return; // skip if already failed earlier + checkMixinMainMethod(objSpec, prop.getFeatureIdentifier()); + } + + @Override + public void validateCollection(final ObjectSpecification objSpec, final OneToManyAssociation coll) { + if(mixinSort==null) return; // skip if already failed earlier + checkMixinMainMethod(objSpec, coll.getFeatureIdentifier()); + } + + // -- HELPER + + private void checkMixinMainMethod(final ObjectSpecification objSpec, final Identifier memberIdentifier) { + val mixinFacet = ((ObjectSpecificationAbstract)objSpec).mixinFacet().orElseThrow(); + + val expectedMethodName = mixinFacet.getMainMethodName(); + val actualMethodName = memberIdentifier.getMemberLogicalName(); + + if(!expectedMethodName.equals(actualMethodName)) { + ValidationFailure.raiseFormatted(objSpec, + ProgrammingModelConstants.Violation.INVALID_MIXIN_MAIN + .builder() + .addVariable("type", objSpec.getCorrespondingClass().getName()) + .addVariable("expectedMethodName", expectedMethodName) + .addVariable("actualMethodName", actualMethodName) + .buildMessage()); + } + } + +} diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/SanityChecksValidator2.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/SanityChecksValidator2.java deleted file mode 100644 index ca0af7e6d98..00000000000 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/SanityChecksValidator2.java +++ /dev/null @@ -1,91 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.causeway.core.metamodel.postprocessors.all; - -import javax.inject.Inject; - -import org.apache.causeway.core.metamodel.context.MetaModelContext; -import org.apache.causeway.core.metamodel.spec.ObjectSpecification; -import org.apache.causeway.core.metamodel.spec.feature.ObjectAction; -import org.apache.causeway.core.metamodel.spec.feature.OneToManyAssociation; -import org.apache.causeway.core.metamodel.spec.feature.OneToOneAssociation; -import org.apache.causeway.core.metamodel.specloader.validator.MetaModelValidator; -import org.apache.causeway.core.metamodel.specloader.validator.MetaModelValidatorAbstract; - -/** - * Checks various preconditions for a sane meta-model. - *
    - * - *
  • Guard against members that contribute vetoed or managed types. - * Those are not allowed as member/return/param.
    • - *
- */ -public class SanityChecksValidator2 -extends MetaModelValidatorAbstract -implements - MetaModelValidator.ActionValidator, - MetaModelValidator.PropertyValidator, - MetaModelValidator.CollectionValidator { - - @Inject - public SanityChecksValidator2(final MetaModelContext mmc) { - super(mmc, MIXINS); - } - - @Override - public void validateObjectEnter(final ObjectSpecification objSpec) { - System.err.printf("mixin %s%n", objSpec); - } - - @Override - public void validateObjectExit(final ObjectSpecification objSpec) { - } - - @Override - public void validateAction(final ObjectSpecification objectSpecification, final ObjectAction objectAction) { - - } - - @Override - public void validateProperty(final ObjectSpecification objectSpecification, final OneToOneAssociation prop) { - - } - - @Override - public void validateCollection(final ObjectSpecification objectSpecification, final OneToManyAssociation coll) { - - } - - // -- HELPER - -// private void checkElementType( -// final FacetHolder facetHolder, -// final ObjectSpecification declaringType, -// final ObjectSpecification elementType) { -// -// if(elementType == null -// || elementType.getBeanSort().isManagedBeanAny() -// || elementType.getBeanSort().isMixin() -// || elementType.getBeanSort().isVetoed()) { -// -// ValidationFailureUtils.raiseInvalidMemberElementType(facetHolder, declaringType, elementType); -// } -// } - -} diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/progmodels/dflt/ProgrammingModelFacetsJava11.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/progmodels/dflt/ProgrammingModelFacetsJava11.java index 39667a9a0e1..07b3561412c 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/progmodels/dflt/ProgrammingModelFacetsJava11.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/progmodels/dflt/ProgrammingModelFacetsJava11.java @@ -86,7 +86,7 @@ import org.apache.causeway.core.metamodel.postprocessors.all.CssOnActionFromConfiguredRegexPostProcessor; import org.apache.causeway.core.metamodel.postprocessors.all.DescribedAsFromTypePostProcessor; import org.apache.causeway.core.metamodel.postprocessors.all.SanityChecksValidator; -import org.apache.causeway.core.metamodel.postprocessors.all.SanityChecksValidator2; +import org.apache.causeway.core.metamodel.postprocessors.all.MixinSanityChecksValidator; import org.apache.causeway.core.metamodel.postprocessors.all.i18n.SynthesizeObjectNamingPostProcessor; import org.apache.causeway.core.metamodel.postprocessors.all.i18n.TranslationPostProcessor; import org.apache.causeway.core.metamodel.postprocessors.allbutparam.authorization.AuthorizationPostProcessor; @@ -274,7 +274,7 @@ private void addValidators() { addValidator(ValidationOrder.A1_BUILTIN, new ActionOverloadingValidator(mmc)); addValidator(ValidationOrder.A1_BUILTIN, new LogicalTypeMalformedValidator(mmc)); - addValidator(ValidationOrder.A1_BUILTIN, new SanityChecksValidator2(mmc)); + addValidator(ValidationOrder.A1_BUILTIN, new MixinSanityChecksValidator(mmc)); } } diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/ObjectSpecificationAbstract.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/ObjectSpecificationAbstract.java index 0f7e2278cb5..76f2630ad77 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/ObjectSpecificationAbstract.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/ObjectSpecificationAbstract.java @@ -91,6 +91,7 @@ import static org.apache.causeway.commons.internal.base._NullSafe.stream; +import lombok.AccessLevel; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.val; @@ -537,7 +538,7 @@ public String getHelp() { @Override public final Optional getMixinSort() { - return lookupMixinFacet() + return mixinFacet() .map(MixinFacet::getMixinSort); } @@ -572,21 +573,6 @@ public Q getFacet(final Class facetType) { } } - @Getter(lazy = true) @Accessors(fluent=true) - private final Optional mixinFacet = lookupMixinFacet(); - - private Optional lookupMixinFacet() { - if(!isMixin()) { - return Optional.empty(); - } - val mixinFacet = getFacet(MixinFacet.class); - if(mixinFacet==null) { - throw _Exceptions.illegalState("type %s has BeanSort MIXIN but ended up NOT having a MixinFacet", - getCorrespondingClass()); - } - return Optional.of(mixinFacet); - } - @Domain.Exclude private static class NotANoopFacetFilter implements Predicate { Q noopFacet; @@ -745,14 +731,14 @@ private Stream createMixedInAssociations() { private Stream createMixedInAssociation(final Class mixinType) { - val specification = getSpecificationLoader().loadSpecification(mixinType, + val mixinSpec = getSpecificationLoader().loadSpecification(mixinType, IntrospectionState.FULLY_INTROSPECTED); - if (specification == this) { + if (mixinSpec == this) { return Stream.empty(); } - val mixinFacet = specification.getFacet(MixinFacet.class); + val mixinFacet = mixinSpec.getFacet(MixinFacet.class); if(mixinFacet == null) { - // this shouldn't happen; perhaps it would be more correct to throw an exception? + // this shouldn't happen; to be covered by meta-model validation later return Stream.empty(); } if(!mixinFacet.isMixinFor(getCorrespondingClass())) { @@ -760,7 +746,7 @@ private Stream createMixedInAssociation(final Class mixinT } val mixinMethodName = mixinFacet.getMainMethodName(); - return specification.streamActions(ActionScope.ANY, MixedIn.INCLUDED) + return mixinSpec.streamActions(ActionScope.ANY, MixedIn.EXCLUDED) .filter(_SpecPredicates::isMixedInAssociation) .map(ObjectActionDefault.class::cast) .map(_MixedInMemberFactory.mixedInAssociation(this, mixinType, mixinMethodName)) @@ -785,10 +771,7 @@ private Stream createMixedInAction(final Class mixinType } val mixinFacet = mixinSpec.getFacet(MixinFacet.class); if(mixinFacet == null) { - // this shouldn't happen; perhaps it would be more correct to throw an exception? - return Stream.empty(); - } - if(!mixinFacet.isMixinFor(getCorrespondingClass())) { + // this shouldn't happen; to be covered by meta-model validation later return Stream.empty(); } // don't mixin Object_ mixins to domain services @@ -921,6 +904,22 @@ private void createMixedInAssociationsAndResort() { mixedInAssociations.stream())); } + // -- MIXIN FACET LOOKUP - WITH CACHING + + @Getter(lazy = true, value = AccessLevel.PUBLIC) @Accessors(fluent=true) + private final Optional mixinFacet = lookupMixinFacet(); + private Optional lookupMixinFacet() { + if(!isMixin()) { + return Optional.empty(); + } + val mixinFacet = getFacet(MixinFacet.class); + if(mixinFacet==null) { + throw _Exceptions.illegalState("type %s has BeanSort MIXIN but ended up NOT having a MixinFacet", + getCorrespondingClass()); + } + return Optional.of(mixinFacet); + } + @Getter(lazy = true) private final CausewayBeanTypeRegistry causewayBeanTypeRegistry = getServiceRegistry() diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/dflt/ObjectSpecificationDefault.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/dflt/ObjectSpecificationDefault.java index b11227a75a1..ecb97a4c6e6 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/dflt/ObjectSpecificationDefault.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/dflt/ObjectSpecificationDefault.java @@ -32,6 +32,7 @@ import org.apache.causeway.applib.annotation.Introspection.IntrospectionPolicy; import org.apache.causeway.commons.collections.Can; import org.apache.causeway.commons.collections.ImmutableEnumSet; +import org.apache.causeway.commons.internal.base._Casts; import org.apache.causeway.commons.internal.base._Lazy; import org.apache.causeway.commons.internal.base._NullSafe; import org.apache.causeway.commons.internal.base._Strings; @@ -49,6 +50,8 @@ import org.apache.causeway.core.metamodel.facets.all.named.MemberNamedFacet; import org.apache.causeway.core.metamodel.facets.all.named.MemberNamedFacetForStaticMemberName; import org.apache.causeway.core.metamodel.facets.object.introspection.IntrospectionPolicyFacet; +import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet.MixinSort; +import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacetAbstract; import org.apache.causeway.core.metamodel.object.ManagedObject; import org.apache.causeway.core.metamodel.services.classsubstitutor.ClassSubstitutorRegistry; import org.apache.causeway.core.metamodel.spec.ActionScope; @@ -204,8 +207,18 @@ private Stream createAssociations() { private ObjectAssociation createAssociation(final FacetedMethod facetMethod) { if (facetMethod.getFeatureType().isCollection()) { + + mixinFacet() + .flatMap(mixinFacet->_Casts.castTo(MixinFacetAbstract.class, mixinFacet)) + .ifPresent(mixinFacetAbstract->mixinFacetAbstract.initMixinSort(MixinSort.MIXIN_FOR_COLL)); + return OneToManyAssociationDefault.forMethod(facetMethod); } else if (facetMethod.getFeatureType().isProperty()) { + + mixinFacet() + .flatMap(mixinFacet->_Casts.castTo(MixinFacetAbstract.class, mixinFacet)) + .ifPresent(mixinFacetAbstract->mixinFacetAbstract.initMixinSort(MixinSort.MIXIN_FOR_PROP)); + return OneToOneAssociationDefault.forMethod(facetMethod); } else { return null; @@ -221,6 +234,11 @@ private Stream createActions() { private ObjectAction createAction(final FacetedMethod facetedMethod) { if (facetedMethod.getFeatureType().isAction()) { + + mixinFacet() + .flatMap(mixinFacet->_Casts.castTo(MixinFacetAbstract.class, mixinFacet)) + .ifPresent(mixinFacetAbstract->mixinFacetAbstract.initMixinSort(MixinSort.MIXIN_FOR_ACT)); + return this.isMixin() ? ObjectActionDefault.forMixinMain(facetedMethod) : ObjectActionDefault.forMethod(facetedMethod); diff --git a/core/metamodel/src/test/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract_Test.java b/core/metamodel/src/test/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract_Test.java index de751d38a37..00a8036cfd2 100644 --- a/core/metamodel/src/test/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract_Test.java +++ b/core/metamodel/src/test/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract_Test.java @@ -21,8 +21,6 @@ import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; -import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet.MixinSort; - import lombok.val; class MixinFacetAbstract_Test { @@ -43,7 +41,7 @@ void happy_case() throws Exception { // given val constructor = Collection_numberOfChildren.class.getConstructor(Object.class); val facet = new MixinFacetAbstract( - Collection_numberOfChildren.class, MixinSort.MIXIN_PROP, "prop", constructor, null) {}; + Collection_numberOfChildren.class, "prop", constructor, null) {}; val propMethodInSubclass = SimpleObject_numberOfChildren.class.getMethod("prop"); diff --git a/regressiontests/stable-domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/DomainModelTest_usingBadDomain.java b/regressiontests/stable-domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/DomainModelTest_usingBadDomain.java index c738f02adce..7b7ed516823 100644 --- a/regressiontests/stable-domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/DomainModelTest_usingBadDomain.java +++ b/regressiontests/stable-domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/DomainModelTest_usingBadDomain.java @@ -415,7 +415,9 @@ void invalidElementType(final Class classUnderTest) { void invalidMixinDeclaration(final Class classUnderTest) { validator.assertAnyFailuresContaining( classUnderTest, - "has a member with vetoed, mixin or managed element-type"); + "Mixin org.apache.causeway.testdomain.model.bad.InvalidMixinDeclarations$ActionMixinWithProp" + + " does declare method name 'act' as the mixin main method to use," + + " but introspection did pick up method 'prop' instead."); } // -- INCUBATING From 5c6c24101fa9dc591a957eaef5caa3d69c035dec Mon Sep 17 00:00:00 2001 From: Andi Huber Date: Thu, 25 May 2023 15:13:38 +0200 Subject: [PATCH 3/6] CAUSEWAY-3444: consolidate enums MixinSort and Contributing --- .../progmodel/ProgrammingModelConstants.java | 2 + .../contributing/ContributingFacet.java | 36 +++++----- .../ContributingFacetAbstract.java | 19 +++--- ...ontributingFacetFromMixinFacetFactory.java | 2 +- .../CollectionAnnotationFacetFactory.java | 28 ++++---- .../facets/object/mixin/MixinFacet.java | 10 +-- .../object/mixin/MixinFacetAbstract.java | 11 ++-- .../PropertyAnnotationFacetFactory.java | 27 ++++---- .../all/MixinSanityChecksValidator.java | 65 +++++++++++-------- .../metamodel/spec/ObjectSpecification.java | 4 +- .../specimpl/ObjectSpecificationAbstract.java | 9 ++- .../dflt/ObjectSpecificationDefault.java | 19 ++---- 12 files changed, 118 insertions(+), 114 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 c2346a735ac..21962c7d060 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 @@ -541,6 +541,8 @@ public static enum Violation { INVALID_MIXIN_MAIN("Mixin ${type} does declare method name '${expectedMethodName}' as" + " the mixin main method to use," + " but introspection did pick up method '${actualMethodName}' instead."), + INVALID_MIXIN_SORT("Mixin ${type} is declared as contributing '${expectedContributing}'" + + " but introspection did pick it up as '${actualContributing}' instead."), ; private final String template; diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/actions/contributing/ContributingFacet.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/actions/contributing/ContributingFacet.java index 00db80f9f38..ed337ebd124 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/actions/contributing/ContributingFacet.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/actions/contributing/ContributingFacet.java @@ -19,43 +19,43 @@ package org.apache.causeway.core.metamodel.facets.actions.contributing; import org.apache.causeway.core.metamodel.facetapi.Facet; +import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet.Contributing; import org.apache.causeway.core.metamodel.spec.feature.ObjectAction; import lombok.val; /** * Indicates that the action should be contributed to objects either - * as ACTION or ASSOCIATION. + * as Action, Property or Collection. *

* Since v2 only ever used for mixed in actions. * @since 2.0 */ public interface ContributingFacet extends Facet { - enum Contributing { - - /** - * Contributed as an action but not as an association. - */ - AS_ACTION, - - /** - * (If takes a single argument and has safe semantics) then is contributed as an association - * (contributed property if returns a single value, contributed collection if returns a collection) but not - * as an action. - */ - AS_ASSOCIATION, - } +// enum Contributing { +// /** +// * Contributed as an Action. +// */ +// AS_ACTION, +// /** +// * Contributed as a Property. +// */ +// AS_PROPERTY, +// /** +// * Contributed as a Collection. +// */ +// AS_COLLECTION +// } public Contributing contributed(); default boolean isActionContributionVetoed() { - // not contributed to actions if... - return contributed() == Contributing.AS_ASSOCIATION; + return contributed() == Contributing.AS_PROPERTY + || contributed() == Contributing.AS_COLLECTION; } default boolean isAssociationContributionVetoed() { - // not contributed to associations if... return contributed() == Contributing.AS_ACTION; } diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/actions/contributing/ContributingFacetAbstract.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/actions/contributing/ContributingFacetAbstract.java index f717726b0a5..127737c2eb6 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/actions/contributing/ContributingFacetAbstract.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/actions/contributing/ContributingFacetAbstract.java @@ -23,33 +23,34 @@ import org.apache.causeway.core.metamodel.facetapi.Facet; import org.apache.causeway.core.metamodel.facetapi.FacetAbstract; import org.apache.causeway.core.metamodel.facetapi.FacetHolder; +import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet.Contributing; + +import lombok.Getter; +import lombok.NonNull; +import lombok.experimental.Accessors; public abstract class ContributingFacetAbstract extends FacetAbstract implements ContributingFacet { - private final Contributing contributing; + @Getter(onMethod_={@Override}) @Accessors(fluent=true) + private final @NonNull Contributing contributed; private static final Class type() { return ContributingFacet.class; } public ContributingFacetAbstract( - final Contributing contributing, + final Contributing contributed, final FacetHolder holder) { super(type(), holder); - this.contributing = contributing; - } - - @Override - public Contributing contributed() { - return contributing; + this.contributed = contributed; } @Override public void visitAttributes(final BiConsumer visitor) { super.visitAttributes(visitor); - visitor.accept("contributing", contributing); + visitor.accept("contributing", contributed()); } } diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/actions/contributing/derived/ContributingFacetFromMixinFacetFactory.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/actions/contributing/derived/ContributingFacetFromMixinFacetFactory.java index 45add474d40..c03b88f84d7 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/actions/contributing/derived/ContributingFacetFromMixinFacetFactory.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/actions/contributing/derived/ContributingFacetFromMixinFacetFactory.java @@ -26,9 +26,9 @@ import org.apache.causeway.core.metamodel.facetapi.FacetUtil; import org.apache.causeway.core.metamodel.facetapi.FeatureType; import org.apache.causeway.core.metamodel.facets.FacetFactoryAbstract; -import org.apache.causeway.core.metamodel.facets.actions.contributing.ContributingFacet.Contributing; import org.apache.causeway.core.metamodel.facets.actions.contributing.ContributingFacetAbstract; import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet; +import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet.Contributing; import lombok.val; 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 ef6b82a4f55..e570f3c8256 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 @@ -28,13 +28,14 @@ import org.apache.causeway.core.metamodel.context.MetaModelContext; import org.apache.causeway.core.metamodel.facetapi.FeatureType; import org.apache.causeway.core.metamodel.facets.FacetFactoryAbstract; +import org.apache.causeway.core.metamodel.facets.FacetedMethod; import org.apache.causeway.core.metamodel.facets.actcoll.typeof.TypeOfFacet; -import org.apache.causeway.core.metamodel.facets.actions.contributing.ContributingFacet.Contributing; import org.apache.causeway.core.metamodel.facets.actions.contributing.ContributingFacetAbstract; import org.apache.causeway.core.metamodel.facets.actions.semantics.ActionSemanticsFacetAbstract; import org.apache.causeway.core.metamodel.facets.collections.collection.hidden.HiddenFacetForCollectionAnnotation; import org.apache.causeway.core.metamodel.facets.collections.collection.modify.CollectionDomainEventFacet; import org.apache.causeway.core.metamodel.facets.collections.collection.typeof.TypeOfFacetForCollectionAnnotation; +import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet.Contributing; import org.apache.causeway.core.metamodel.facets.propcoll.accessor.PropertyOrCollectionAccessorFacet; import org.apache.causeway.core.metamodel.specloader.validator.ValidationFailureUtils; @@ -53,7 +54,11 @@ public void process(final ProcessMethodContext processMethodContext) { val collectionIfAny = collectionIfAny(processMethodContext); - inferIntentWhenOnTypeLevel(processMethodContext, collectionIfAny); + if(processMethodContext.isMixinMain()) { + collectionIfAny.ifPresent(collection->{ + inferMixinSort(collection, processMethodContext.getFacetHolder()); + }); + } processDomainEvent(processMethodContext, collectionIfAny); processHidden(processMethodContext, collectionIfAny); @@ -68,22 +73,11 @@ Optional collectionIfAny(final ProcessMethodContext processMethodCon .raiseAmbiguousMixinAnnotations(processMethodContext.getFacetHolder(), Collection.class)); } - void inferIntentWhenOnTypeLevel(final ProcessMethodContext processMethodContext, final Optional collectionIfAny) { - if(!processMethodContext.isMixinMain() || !collectionIfAny.isPresent()) { - return; // no @Collection found neither type nor method - } - - // XXX[1998] this condition would allow 'intent inference' only when @Property is found at type level - // val isPropertyMethodLevel = processMethodContext.synthesizeOnMethod(Property.class).isPresent(); - // if(isPropertyMethodLevel) return; - - //[1998] if @Collection detected on method or type level infer: - //@Action(semantics=SAFE) - //@ActionLayout(contributed=ASSOCIATION) ... it seems, is already allowed for mixins - val facetedMethod = processMethodContext.getFacetHolder(); + void inferMixinSort(final Collection collection, final FacetedMethod facetedMethod) { + /* if @Collection detected on method or type level infer: + * @Action(semantics=SAFE) */ addFacet(new ActionSemanticsFacetAbstract(SemanticsOf.SAFE, facetedMethod) {}); - addFacet(new ContributingFacetAbstract(Contributing.AS_ASSOCIATION, facetedMethod) {}); - + addFacet(new ContributingFacetAbstract(Contributing.AS_COLLECTION, facetedMethod) {}); } void processDomainEvent(final ProcessMethodContext processMethodContext, final Optional collectionIfAny) { diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacet.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacet.java index 4987e95152d..1b93f6962e1 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacet.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacet.java @@ -42,7 +42,7 @@ */ public interface MixinFacet extends Facet { - public enum MixinSort { + public enum Contributing { /** * Initial state early during introspection. */ @@ -51,22 +51,22 @@ public enum MixinSort { * Object associated with an entity, viewmodel or domain-service * to act as contributer of a single domain-action. */ - MIXIN_FOR_ACT, + AS_ACTION, /** * Object associated with an entity, viewmodel or domain-service * to act as contributer of a single domain-property. */ - MIXIN_FOR_PROP, + AS_PROPERTY, /** * Object associated with an entity, viewmodel or domain-service * to act as contributer of a single domain-collection. */ - MIXIN_FOR_COLL; + AS_COLLECTION; public boolean isUnspecified() { return this==UNSPECIFIED; } } - MixinSort getMixinSort(); + Contributing contributing(); /** * The mixin's main method name. diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract.java index 50cbd58dfe0..5b856beae57 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract.java @@ -31,6 +31,7 @@ import lombok.Getter; import lombok.NonNull; import lombok.val; +import lombok.experimental.Accessors; //@Log4j2 public abstract class MixinFacetAbstract @@ -39,8 +40,8 @@ public abstract class MixinFacetAbstract @Getter(onMethod_={@Override}) private final @NonNull String mainMethodName; - @Getter(onMethod_={@Override}) - private @NonNull MixinSort mixinSort = MixinSort.UNSPECIFIED; + @Getter(onMethod_={@Override}) @Accessors(fluent=true) + private @NonNull Contributing contributing = Contributing.UNSPECIFIED; private final @NonNull Class mixinType; private final @NonNull Class holderType; @@ -117,7 +118,7 @@ public boolean isCandidateForMain(final Method method) { public void visitAttributes(final BiConsumer visitor) { super.visitAttributes(visitor); visitor.accept("mixinType", mixinType); - visitor.accept("mixinSort", mixinSort); + visitor.accept("contributing", contributing); visitor.accept("mainMethodName", mainMethodName); visitor.accept("holderType", holderType); } @@ -125,8 +126,8 @@ public void visitAttributes(final BiConsumer visitor) { /** * Framework internal. */ - public MixinFacetAbstract initMixinSort(final @NonNull MixinSort mixinSort) { - this.mixinSort = mixinSort; + public MixinFacetAbstract initMixinSort(final @NonNull Contributing contributing) { + this.contributing = contributing; return this; } 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 2864b238cda..bed15f00d33 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 @@ -29,11 +29,12 @@ import org.apache.causeway.core.metamodel.context.MetaModelContext; import org.apache.causeway.core.metamodel.facetapi.FeatureType; import org.apache.causeway.core.metamodel.facets.FacetFactoryAbstract; -import org.apache.causeway.core.metamodel.facets.actions.contributing.ContributingFacet.Contributing; +import org.apache.causeway.core.metamodel.facets.FacetedMethod; import org.apache.causeway.core.metamodel.facets.actions.contributing.ContributingFacetAbstract; import org.apache.causeway.core.metamodel.facets.actions.semantics.ActionSemanticsFacetAbstract; import org.apache.causeway.core.metamodel.facets.members.publish.command.CommandPublishingFacetForPropertyAnnotation; import org.apache.causeway.core.metamodel.facets.members.publish.execution.ExecutionPublishingPropertyFacetForPropertyAnnotation; +import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet.Contributing; import org.apache.causeway.core.metamodel.facets.propcoll.accessor.PropertyOrCollectionAccessorFacet; import org.apache.causeway.core.metamodel.facets.properties.projection.ProjectingFacetFromPropertyAnnotation; import org.apache.causeway.core.metamodel.facets.properties.property.disabled.DisabledFacetForPropertyAnnotation; @@ -69,7 +70,11 @@ public void process(final ProcessMethodContext processMethodContext) { val propertyIfAny = propertyIfAny(processMethodContext); - inferIntentWhenOnTypeLevel(processMethodContext, propertyIfAny); + if(processMethodContext.isMixinMain()) { + propertyIfAny.ifPresent(property->{ + inferMixinSort(property, processMethodContext.getFacetHolder()); + }); + } processDomainEvent(processMethodContext, propertyIfAny); processHidden(processMethodContext, propertyIfAny); @@ -94,21 +99,11 @@ Optional propertyIfAny(final ProcessMethodContext processMethodContext .raiseAmbiguousMixinAnnotations(processMethodContext.getFacetHolder(), Property.class)); } - void inferIntentWhenOnTypeLevel(final ProcessMethodContext processMethodContext, final Optional propertyIfAny) { - if(!processMethodContext.isMixinMain() || !propertyIfAny.isPresent()) { - return; // no @Property found neither type nor method - } - - // XXX[1998] this condition would allow 'intent inference' only when @Property is found at type level - // val isPropertyMethodLevel = processMethodContext.synthesizeOnMethod(Property.class).isPresent(); - // if(isPropertyMethodLevel) return; - - //[1998] if @Property detected on method or type level infer: - //@Action(semantics=SAFE) - //@ActionLayout(contributed=ASSOCIATION) ... it seems, is already allowed for mixins - val facetedMethod = processMethodContext.getFacetHolder(); + void inferMixinSort(final Property property, final FacetedMethod facetedMethod) { + /* if @Property detected on method or type level infer: + * @Action(semantics=SAFE) */ addFacet(new ActionSemanticsFacetAbstract(SemanticsOf.SAFE, facetedMethod) {}); - addFacet(new ContributingFacetAbstract(Contributing.AS_ASSOCIATION, facetedMethod) {}); + addFacet(new ContributingFacetAbstract(Contributing.AS_PROPERTY, facetedMethod) {}); } void processDomainEvent(final ProcessMethodContext processMethodContext, final Optional propertyIfAny) { diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/MixinSanityChecksValidator.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/MixinSanityChecksValidator.java index 43dc2f0b545..57db3b51bc0 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/MixinSanityChecksValidator.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/MixinSanityChecksValidator.java @@ -21,14 +21,17 @@ import javax.inject.Inject; import org.apache.causeway.applib.Identifier; +import org.apache.causeway.applib.annotation.Action; +import org.apache.causeway.applib.annotation.Collection; +import org.apache.causeway.applib.annotation.Property; import org.apache.causeway.applib.services.metamodel.BeanSort; import org.apache.causeway.core.config.progmodel.ProgrammingModelConstants; import org.apache.causeway.core.metamodel.context.MetaModelContext; -import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet.MixinSort; +import org.apache.causeway.core.metamodel.facets.FacetedMethod; +import org.apache.causeway.core.metamodel.facets.actions.contributing.ContributingFacet; +import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet.Contributing; import org.apache.causeway.core.metamodel.spec.ObjectSpecification; import org.apache.causeway.core.metamodel.spec.feature.ObjectAction; -import org.apache.causeway.core.metamodel.spec.feature.OneToManyAssociation; -import org.apache.causeway.core.metamodel.spec.feature.OneToOneAssociation; import org.apache.causeway.core.metamodel.specloader.specimpl.ObjectSpecificationAbstract; import org.apache.causeway.core.metamodel.specloader.validator.MetaModelValidator; import org.apache.causeway.core.metamodel.specloader.validator.MetaModelValidatorAbstract; @@ -40,19 +43,19 @@ * Checks various preconditions for a sane meta-model. *

    *
  • Guard against types that are identified as {@link BeanSort#MIXIN}, but don't contribute any member.
    • - *
  • Make sure mixins contribute either an action or a property or a collection.
    • - *
  • Make sure if a mixin main method name is specified (eg 'act', 'prop', 'coll'), + *
  • Make sure mixins contribute either an action or a property or a collection, + * and if declared via {@link Action}, {@link Property} or {@link Collection} annotation, + * that the {@link Contributing} is correct.
    • + *
  • Make sure if a mixin main method name is specified (eg. 'act', 'prop', 'coll'), * that introspection was able to pick it up.
    • *
*/ public class MixinSanityChecksValidator extends MetaModelValidatorAbstract implements - MetaModelValidator.ActionValidator, - MetaModelValidator.PropertyValidator, - MetaModelValidator.CollectionValidator { + MetaModelValidator.ActionValidator { - private MixinSort mixinSort; + private Contributing contributing; @Inject public MixinSanityChecksValidator(final MetaModelContext mmc) { @@ -61,9 +64,9 @@ public MixinSanityChecksValidator(final MetaModelContext mmc) { @Override public void validateObjectEnter(final ObjectSpecification objSpec) { - final MixinSort mixinSort = objSpec.getMixinSort().orElse(null); - if(mixinSort==null - || mixinSort.isUnspecified()) { + final Contributing contributing = objSpec.getMixinSort().orElse(null); + if(contributing==null + || contributing.isUnspecified()) { ValidationFailure.raiseFormatted(objSpec, ProgrammingModelConstants.Violation.INVALID_MIXIN_TYPE .builder() @@ -71,33 +74,43 @@ public void validateObjectEnter(final ObjectSpecification objSpec) { .buildMessage()); return; } - this.mixinSort = mixinSort; + this.contributing = contributing; } @Override public void validateObjectExit(final ObjectSpecification objSpec) { - this.mixinSort = null; + this.contributing = null; } + /* + * (introspected) mixins have no properties nor collections; instead the single member is always + * an action that either contributes as action, property or collection + */ @Override public void validateAction(final ObjectSpecification objSpec, final ObjectAction act) { - if(mixinSort==null) return; // skip if already failed earlier + if(contributing==null) return; // skip if already failed earlier checkMixinMainMethod(objSpec, act.getFeatureIdentifier()); + checkMixinSort(objSpec, (FacetedMethod) act.getFacetHolder()); } - @Override - public void validateProperty(final ObjectSpecification objSpec, final OneToOneAssociation prop) { - if(mixinSort==null) return; // skip if already failed earlier - checkMixinMainMethod(objSpec, prop.getFeatureIdentifier()); - } + // -- HELPER - @Override - public void validateCollection(final ObjectSpecification objSpec, final OneToManyAssociation coll) { - if(mixinSort==null) return; // skip if already failed earlier - checkMixinMainMethod(objSpec, coll.getFeatureIdentifier()); - } + private void checkMixinSort(final ObjectSpecification objSpec, final FacetedMethod facetedMethod) { + val expectedContributing = facetedMethod.lookupFacet(ContributingFacet.class) + .map(ContributingFacet::contributed) + .orElse(null); + val actualContributing = this.contributing; - // -- HELPER + if(actualContributing!=expectedContributing) { + ValidationFailure.raiseFormatted(objSpec, + ProgrammingModelConstants.Violation.INVALID_MIXIN_SORT + .builder() + .addVariable("type", objSpec.getCorrespondingClass().getName()) + .addVariable("expectedContributing", expectedContributing.name()) + .addVariable("actualContributing", actualContributing.name()) + .buildMessage()); + } + } private void checkMixinMainMethod(final ObjectSpecification objSpec, final Identifier memberIdentifier) { val mixinFacet = ((ObjectSpecificationAbstract)objSpec).mixinFacet().orElseThrow(); diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/spec/ObjectSpecification.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/spec/ObjectSpecification.java index b2175fb5d84..b13f6b426d4 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/spec/ObjectSpecification.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/spec/ObjectSpecification.java @@ -59,7 +59,7 @@ import org.apache.causeway.core.metamodel.facets.object.icon.IconFacet; import org.apache.causeway.core.metamodel.facets.object.icon.ObjectIcon; import org.apache.causeway.core.metamodel.facets.object.immutable.ImmutableFacet; -import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet.MixinSort; +import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet.Contributing; import org.apache.causeway.core.metamodel.facets.object.parented.ParentedCollectionFacet; import org.apache.causeway.core.metamodel.facets.object.title.TitleFacet; import org.apache.causeway.core.metamodel.facets.object.title.TitleRenderRequest; @@ -293,7 +293,7 @@ public default Optional lookupMixedInAction(final ObjectSpe /** * @since 2.0 */ - Optional getMixinSort(); + Optional getMixinSort(); // ////////////////////////////////////////////////////////////// diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/ObjectSpecificationAbstract.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/ObjectSpecificationAbstract.java index 76f2630ad77..a9325ec4f7c 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/ObjectSpecificationAbstract.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/ObjectSpecificationAbstract.java @@ -67,7 +67,7 @@ import org.apache.causeway.core.metamodel.facets.object.immutable.ImmutableFacet; import org.apache.causeway.core.metamodel.facets.object.logicaltype.AliasedFacet; import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet; -import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet.MixinSort; +import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet.Contributing; import org.apache.causeway.core.metamodel.facets.object.navparent.NavigableParentFacet; import org.apache.causeway.core.metamodel.facets.object.parented.ParentedCollectionFacet; import org.apache.causeway.core.metamodel.facets.object.title.TitleFacet; @@ -537,9 +537,9 @@ public String getHelp() { } @Override - public final Optional getMixinSort() { + public final Optional getMixinSort() { return mixinFacet() - .map(MixinFacet::getMixinSort); + .map(MixinFacet::contributing); } // -- FACET HANDLING @@ -774,6 +774,9 @@ private Stream createMixedInAction(final Class mixinType // this shouldn't happen; to be covered by meta-model validation later return Stream.empty(); } + if(!mixinFacet.isMixinFor(getCorrespondingClass())) { + return Stream.empty(); + } // don't mixin Object_ mixins to domain services if(getBeanSort().isManagedBeanContributing() && mixinFacet.isMixinFor(java.lang.Object.class)) { diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/dflt/ObjectSpecificationDefault.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/dflt/ObjectSpecificationDefault.java index ecb97a4c6e6..d57c21ba8ec 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/dflt/ObjectSpecificationDefault.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/dflt/ObjectSpecificationDefault.java @@ -47,10 +47,10 @@ import org.apache.causeway.core.metamodel.facets.FacetedMethod; import org.apache.causeway.core.metamodel.facets.ImperativeFacet; import org.apache.causeway.core.metamodel.facets.actcoll.typeof.TypeOfFacet; +import org.apache.causeway.core.metamodel.facets.actions.contributing.ContributingFacet; import org.apache.causeway.core.metamodel.facets.all.named.MemberNamedFacet; import org.apache.causeway.core.metamodel.facets.all.named.MemberNamedFacetForStaticMemberName; import org.apache.causeway.core.metamodel.facets.object.introspection.IntrospectionPolicyFacet; -import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet.MixinSort; import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacetAbstract; import org.apache.causeway.core.metamodel.object.ManagedObject; import org.apache.causeway.core.metamodel.services.classsubstitutor.ClassSubstitutorRegistry; @@ -207,18 +207,8 @@ private Stream createAssociations() { private ObjectAssociation createAssociation(final FacetedMethod facetMethod) { if (facetMethod.getFeatureType().isCollection()) { - - mixinFacet() - .flatMap(mixinFacet->_Casts.castTo(MixinFacetAbstract.class, mixinFacet)) - .ifPresent(mixinFacetAbstract->mixinFacetAbstract.initMixinSort(MixinSort.MIXIN_FOR_COLL)); - return OneToManyAssociationDefault.forMethod(facetMethod); } else if (facetMethod.getFeatureType().isProperty()) { - - mixinFacet() - .flatMap(mixinFacet->_Casts.castTo(MixinFacetAbstract.class, mixinFacet)) - .ifPresent(mixinFacetAbstract->mixinFacetAbstract.initMixinSort(MixinSort.MIXIN_FOR_PROP)); - return OneToOneAssociationDefault.forMethod(facetMethod); } else { return null; @@ -235,9 +225,14 @@ private Stream createActions() { private ObjectAction createAction(final FacetedMethod facetedMethod) { if (facetedMethod.getFeatureType().isAction()) { + // assuming, that facetedMethod was already populated with ContributingFacet mixinFacet() .flatMap(mixinFacet->_Casts.castTo(MixinFacetAbstract.class, mixinFacet)) - .ifPresent(mixinFacetAbstract->mixinFacetAbstract.initMixinSort(MixinSort.MIXIN_FOR_ACT)); + .ifPresent(mixinFacetAbstract->{ + facetedMethod.lookupFacet(ContributingFacet.class) + .map(ContributingFacet::contributed) + .ifPresent(mixinFacetAbstract::initMixinSort); + }); return this.isMixin() ? ObjectActionDefault.forMixinMain(facetedMethod) From e7808b004230bc17b9c454665a42f12e5ef84b57 Mon Sep 17 00:00:00 2001 From: Andi Huber Date: Thu, 25 May 2023 15:13:53 +0200 Subject: [PATCH 4/6] CAUSEWAY-3444: test approvals --- ...omainWithPdfJsViewer_IntegTest.dump_facets.approved.xml | 7 ++++--- ...jsViewer_MixinDomain_IntegTest.dump_facets.approved.xml | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/extensions/vw/pdfjs/metamodel/src/test/java/org/apache/causeway/extensions/pdfjs/metamodel/PdfjsViewer_MixinDomainWithPdfJsViewer_IntegTest.dump_facets.approved.xml b/extensions/vw/pdfjs/metamodel/src/test/java/org/apache/causeway/extensions/pdfjs/metamodel/PdfjsViewer_MixinDomainWithPdfJsViewer_IntegTest.dump_facets.approved.xml index c52610eed40..46d6d3a7e90 100644 --- a/extensions/vw/pdfjs/metamodel/src/test/java/org/apache/causeway/extensions/pdfjs/metamodel/PdfjsViewer_MixinDomainWithPdfJsViewer_IntegTest.dump_facets.approved.xml +++ b/extensions/vw/pdfjs/metamodel/src/test/java/org/apache/causeway/extensions/pdfjs/metamodel/PdfjsViewer_MixinDomainWithPdfJsViewer_IntegTest.dump_facets.approved.xml @@ -128,7 +128,7 @@ - + @@ -1019,11 +1019,12 @@ + + - @@ -1080,7 +1081,7 @@ - + diff --git a/extensions/vw/pdfjs/metamodel/src/test/java/org/apache/causeway/extensions/pdfjs/metamodel/PdfjsViewer_MixinDomain_IntegTest.dump_facets.approved.xml b/extensions/vw/pdfjs/metamodel/src/test/java/org/apache/causeway/extensions/pdfjs/metamodel/PdfjsViewer_MixinDomain_IntegTest.dump_facets.approved.xml index 80beffd31c8..9b9b3792811 100644 --- a/extensions/vw/pdfjs/metamodel/src/test/java/org/apache/causeway/extensions/pdfjs/metamodel/PdfjsViewer_MixinDomain_IntegTest.dump_facets.approved.xml +++ b/extensions/vw/pdfjs/metamodel/src/test/java/org/apache/causeway/extensions/pdfjs/metamodel/PdfjsViewer_MixinDomain_IntegTest.dump_facets.approved.xml @@ -128,7 +128,7 @@ - + @@ -1012,11 +1012,12 @@ + + - @@ -1073,7 +1074,7 @@ - + From f857ebcad80d4d35d84c16d6938c0f8ed4276589 Mon Sep 17 00:00:00 2001 From: Andi Huber Date: Thu, 25 May 2023 15:41:38 +0200 Subject: [PATCH 5/6] CAUSEWAY-3444: on mixins, in absence of explicit Property or Collection annot. assume -> contributes as Action --- .../contributing/ContributingFacet.java | 15 ---- .../all/MixinSanityChecksValidator.java | 4 +- .../metamodel/spec/ObjectSpecification.java | 4 +- .../specimpl/ObjectSpecificationAbstract.java | 2 +- .../dflt/ObjectSpecificationDefault.java | 6 +- .../DomainModelTest_usingBadDomain.java | 5 +- ...etaModelRegressionTest.verify.approved.xml | 72 +++++++++++-------- 7 files changed, 59 insertions(+), 49 deletions(-) diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/actions/contributing/ContributingFacet.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/actions/contributing/ContributingFacet.java index ed337ebd124..2482f2aaf32 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/actions/contributing/ContributingFacet.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/actions/contributing/ContributingFacet.java @@ -33,21 +33,6 @@ */ public interface ContributingFacet extends Facet { -// enum Contributing { -// /** -// * Contributed as an Action. -// */ -// AS_ACTION, -// /** -// * Contributed as a Property. -// */ -// AS_PROPERTY, -// /** -// * Contributed as a Collection. -// */ -// AS_COLLECTION -// } - public Contributing contributed(); default boolean isActionContributionVetoed() { diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/MixinSanityChecksValidator.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/MixinSanityChecksValidator.java index 57db3b51bc0..07d0f7062fc 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/MixinSanityChecksValidator.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/postprocessors/all/MixinSanityChecksValidator.java @@ -64,7 +64,7 @@ public MixinSanityChecksValidator(final MetaModelContext mmc) { @Override public void validateObjectEnter(final ObjectSpecification objSpec) { - final Contributing contributing = objSpec.getMixinSort().orElse(null); + final Contributing contributing = objSpec.contributing().orElse(null); if(contributing==null || contributing.isUnspecified()) { ValidationFailure.raiseFormatted(objSpec, @@ -98,7 +98,7 @@ public void validateAction(final ObjectSpecification objSpec, final ObjectAction private void checkMixinSort(final ObjectSpecification objSpec, final FacetedMethod facetedMethod) { val expectedContributing = facetedMethod.lookupFacet(ContributingFacet.class) .map(ContributingFacet::contributed) - .orElse(null); + .orElse(Contributing.AS_ACTION); // if not specified, defaults to action val actualContributing = this.contributing; if(actualContributing!=expectedContributing) { diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/spec/ObjectSpecification.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/spec/ObjectSpecification.java index b13f6b426d4..d3b415e5f64 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/spec/ObjectSpecification.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/spec/ObjectSpecification.java @@ -291,9 +291,11 @@ public default Optional lookupMixedInAction(final ObjectSpe BeanSort getBeanSort(); /** + * Optionally the mixin sort {@link Contributing}, + * based on whether the corresponding class is a mixin type. * @since 2.0 */ - Optional getMixinSort(); + Optional contributing(); // ////////////////////////////////////////////////////////////// diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/ObjectSpecificationAbstract.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/ObjectSpecificationAbstract.java index a9325ec4f7c..0bf5b9c88b8 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/ObjectSpecificationAbstract.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/ObjectSpecificationAbstract.java @@ -537,7 +537,7 @@ public String getHelp() { } @Override - public final Optional getMixinSort() { + public final Optional contributing() { return mixinFacet() .map(MixinFacet::contributing); } diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/dflt/ObjectSpecificationDefault.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/dflt/ObjectSpecificationDefault.java index d57c21ba8ec..71c2db8198b 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/dflt/ObjectSpecificationDefault.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/dflt/ObjectSpecificationDefault.java @@ -51,6 +51,7 @@ import org.apache.causeway.core.metamodel.facets.all.named.MemberNamedFacet; import org.apache.causeway.core.metamodel.facets.all.named.MemberNamedFacetForStaticMemberName; import org.apache.causeway.core.metamodel.facets.object.introspection.IntrospectionPolicyFacet; +import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet.Contributing; import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacetAbstract; import org.apache.causeway.core.metamodel.object.ManagedObject; import org.apache.causeway.core.metamodel.services.classsubstitutor.ClassSubstitutorRegistry; @@ -231,7 +232,10 @@ private ObjectAction createAction(final FacetedMethod facetedMethod) { .ifPresent(mixinFacetAbstract->{ facetedMethod.lookupFacet(ContributingFacet.class) .map(ContributingFacet::contributed) - .ifPresent(mixinFacetAbstract::initMixinSort); + .ifPresentOrElse( + mixinFacetAbstract::initMixinSort, + // if not specified, default to ACTION + ()->mixinFacetAbstract.initMixinSort(Contributing.AS_ACTION)); }); return this.isMixin() diff --git a/regressiontests/stable-domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/DomainModelTest_usingBadDomain.java b/regressiontests/stable-domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/DomainModelTest_usingBadDomain.java index 7b7ed516823..627f19b63db 100644 --- a/regressiontests/stable-domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/DomainModelTest_usingBadDomain.java +++ b/regressiontests/stable-domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/DomainModelTest_usingBadDomain.java @@ -415,9 +415,12 @@ void invalidElementType(final Class classUnderTest) { void invalidMixinDeclaration(final Class classUnderTest) { validator.assertAnyFailuresContaining( classUnderTest, +// "Mixin org.apache.causeway.testdomain.model.bad.InvalidMixinDeclarations$ActionMixinWithProp" +// + " could not be identified as action, property or collection." "Mixin org.apache.causeway.testdomain.model.bad.InvalidMixinDeclarations$ActionMixinWithProp" + " does declare method name 'act' as the mixin main method to use," - + " but introspection did pick up method 'prop' instead."); + + " but introspection did pick up method 'prop' instead." + ); } // -- INCUBATING diff --git a/regressiontests/stable-domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/MetaModelRegressionTest.verify.approved.xml b/regressiontests/stable-domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/MetaModelRegressionTest.verify.approved.xml index 6c1e39ae9b4..7284cfae99e 100644 --- a/regressiontests/stable-domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/MetaModelRegressionTest.verify.approved.xml +++ b/regressiontests/stable-domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/MetaModelRegressionTest.verify.approved.xml @@ -54,11 +54,12 @@ + + - @@ -2761,11 +2762,12 @@ + + - @@ -3879,11 +3881,12 @@ + + - @@ -14212,7 +14215,7 @@ - + @@ -14352,7 +14355,7 @@ - + @@ -14492,7 +14495,7 @@ - + @@ -14606,7 +14609,7 @@ - + @@ -14737,7 +14740,7 @@ - + @@ -14868,7 +14871,7 @@ - + @@ -20915,11 +20918,12 @@ + + - @@ -21238,11 +21242,12 @@ + + - @@ -21550,11 +21555,12 @@ + + - @@ -21876,11 +21882,12 @@ + + - @@ -22202,11 +22209,12 @@ + + - @@ -22387,11 +22395,12 @@ + + - @@ -22701,11 +22710,12 @@ + + - @@ -22765,7 +22775,7 @@ - + @@ -22924,11 +22934,12 @@ + + - @@ -22988,7 +22999,7 @@ - + @@ -23147,11 +23158,12 @@ + + - @@ -23211,7 +23223,7 @@ - + @@ -23349,11 +23361,12 @@ + + - @@ -23406,7 +23419,7 @@ - + @@ -23581,11 +23594,12 @@ + + - @@ -23638,7 +23652,7 @@ - + @@ -23813,11 +23827,12 @@ + + - @@ -23870,7 +23885,7 @@ - + @@ -26732,11 +26747,12 @@ + + - From 95b00f24a1655efc1b3fb98f8e910683ebd98844 Mon Sep 17 00:00:00 2001 From: Andi Huber Date: Thu, 25 May 2023 16:16:41 +0200 Subject: [PATCH 6/6] CAUSEWAY-3444: adds tests for ill-formed prop and coll mixins --- .../object/mixin/MixinFacetAbstract.java | 15 +++++--- .../dflt/ObjectSpecificationDefault.java | 17 +++------ .../DomainModelTest_usingBadDomain.java | 21 ++++++++--- .../model/bad/InvalidMixinDeclarations.java | 36 +++++++++++++++---- 4 files changed, 62 insertions(+), 27 deletions(-) diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract.java index 5b856beae57..d481bdbbc57 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/facets/object/mixin/MixinFacetAbstract.java @@ -27,6 +27,8 @@ import org.apache.causeway.core.metamodel.facetapi.Facet; import org.apache.causeway.core.metamodel.facetapi.FacetAbstract; import org.apache.causeway.core.metamodel.facetapi.FacetHolder; +import org.apache.causeway.core.metamodel.facets.FacetedMethod; +import org.apache.causeway.core.metamodel.facets.actions.contributing.ContributingFacet; import lombok.Getter; import lombok.NonNull; @@ -124,11 +126,16 @@ public void visitAttributes(final BiConsumer visitor) { } /** - * Framework internal. + * Framework internal: copy the mixin-sort ({@link MixinFacet.Contributing}) + * information from the {@link FacetedMethod} + * (as eg. associated with mixin main method 'act') + * to the {@link MixinFacet} that is held by the mixin's type spec. */ - public MixinFacetAbstract initMixinSort(final @NonNull Contributing contributing) { - this.contributing = contributing; - return this; + public void initMixinSortFrom(final FacetedMethod facetedMethod) { + this.contributing = facetedMethod + .lookupFacet(ContributingFacet.class) + .map(ContributingFacet::contributed) + .orElse(Contributing.AS_ACTION); // if not specified, defaults to ACTION } } diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/dflt/ObjectSpecificationDefault.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/dflt/ObjectSpecificationDefault.java index 71c2db8198b..d8866d7cd41 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/dflt/ObjectSpecificationDefault.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/specloader/specimpl/dflt/ObjectSpecificationDefault.java @@ -47,11 +47,9 @@ import org.apache.causeway.core.metamodel.facets.FacetedMethod; import org.apache.causeway.core.metamodel.facets.ImperativeFacet; import org.apache.causeway.core.metamodel.facets.actcoll.typeof.TypeOfFacet; -import org.apache.causeway.core.metamodel.facets.actions.contributing.ContributingFacet; import org.apache.causeway.core.metamodel.facets.all.named.MemberNamedFacet; import org.apache.causeway.core.metamodel.facets.all.named.MemberNamedFacetForStaticMemberName; import org.apache.causeway.core.metamodel.facets.object.introspection.IntrospectionPolicyFacet; -import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacet.Contributing; import org.apache.causeway.core.metamodel.facets.object.mixin.MixinFacetAbstract; import org.apache.causeway.core.metamodel.object.ManagedObject; import org.apache.causeway.core.metamodel.services.classsubstitutor.ClassSubstitutorRegistry; @@ -225,18 +223,13 @@ private Stream createActions() { private ObjectAction createAction(final FacetedMethod facetedMethod) { if (facetedMethod.getFeatureType().isAction()) { - - // assuming, that facetedMethod was already populated with ContributingFacet + /* Assuming, that facetedMethod was already populated with ContributingFacet, + * we copy the mixin-sort information from the FacetedMethod to the MixinFacet + * that is held by the mixin's type spec. */ mixinFacet() .flatMap(mixinFacet->_Casts.castTo(MixinFacetAbstract.class, mixinFacet)) - .ifPresent(mixinFacetAbstract->{ - facetedMethod.lookupFacet(ContributingFacet.class) - .map(ContributingFacet::contributed) - .ifPresentOrElse( - mixinFacetAbstract::initMixinSort, - // if not specified, default to ACTION - ()->mixinFacetAbstract.initMixinSort(Contributing.AS_ACTION)); - }); + .ifPresent(mixinFacetAbstract-> + mixinFacetAbstract.initMixinSortFrom(facetedMethod)); return this.isMixin() ? ObjectActionDefault.forMixinMain(facetedMethod) diff --git a/regressiontests/stable-domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/DomainModelTest_usingBadDomain.java b/regressiontests/stable-domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/DomainModelTest_usingBadDomain.java index 627f19b63db..ec157f0a143 100644 --- a/regressiontests/stable-domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/DomainModelTest_usingBadDomain.java +++ b/regressiontests/stable-domainmodel/src/test/java/org/apache/causeway/testdomain/domainmodel/DomainModelTest_usingBadDomain.java @@ -411,15 +411,26 @@ void invalidElementType(final Class classUnderTest) { @ParameterizedTest @ValueSource(classes = { InvalidMixinDeclarations.ActionMixinWithProp.class, + InvalidMixinDeclarations.ActionMixinWithColl.class, + InvalidMixinDeclarations.PropertyMixinWithOther.class, + InvalidMixinDeclarations.CollectionMixinWithOther.class, }) void invalidMixinDeclaration(final Class classUnderTest) { + + // just by convention of these test scenarios ... + final String expectedMethodName = classUnderTest.getSimpleName().startsWith("Property") + ? "prop" + : classUnderTest.getSimpleName().startsWith("Collection") + ? "coll" + : "act"; + validator.assertAnyFailuresContaining( classUnderTest, -// "Mixin org.apache.causeway.testdomain.model.bad.InvalidMixinDeclarations$ActionMixinWithProp" -// + " could not be identified as action, property or collection." - "Mixin org.apache.causeway.testdomain.model.bad.InvalidMixinDeclarations$ActionMixinWithProp" - + " does declare method name 'act' as the mixin main method to use," - + " but introspection did pick up method 'prop' instead." + ProgrammingModelConstants.Violation.INVALID_MIXIN_MAIN.builder() + .addVariable("type", classUnderTest.getName()) + .addVariable("expectedMethodName", expectedMethodName) + .addVariable("actualMethodName", "other") + .buildMessage() ); } diff --git a/regressiontests/stable/src/main/java/org/apache/causeway/testdomain/model/bad/InvalidMixinDeclarations.java b/regressiontests/stable/src/main/java/org/apache/causeway/testdomain/model/bad/InvalidMixinDeclarations.java index b0a196185be..1ecf65dbec5 100644 --- a/regressiontests/stable/src/main/java/org/apache/causeway/testdomain/model/bad/InvalidMixinDeclarations.java +++ b/regressiontests/stable/src/main/java/org/apache/causeway/testdomain/model/bad/InvalidMixinDeclarations.java @@ -18,14 +18,20 @@ */ package org.apache.causeway.testdomain.model.bad; +import java.util.List; + import javax.inject.Named; import org.apache.causeway.applib.annotation.Action; import org.apache.causeway.applib.annotation.ActionLayout; +import org.apache.causeway.applib.annotation.Collection; +import org.apache.causeway.applib.annotation.CollectionLayout; import org.apache.causeway.applib.annotation.DomainObject; import org.apache.causeway.applib.annotation.Introspection; import org.apache.causeway.applib.annotation.MemberSupport; import org.apache.causeway.applib.annotation.Nature; +import org.apache.causeway.applib.annotation.Property; +import org.apache.causeway.applib.annotation.PropertyLayout; import lombok.RequiredArgsConstructor; @@ -40,17 +46,35 @@ public class InvalidMixinDeclarations { @RequiredArgsConstructor public static class ActionMixinWithProp { @SuppressWarnings("unused") - private final InvalidMixinDeclarations invalidElementTypes; - @MemberSupport public String prop() { return ""; } + private final InvalidMixinDeclarations mixee; + @MemberSupport public String other() { return ""; } } -/* + @Action @ActionLayout(named = "someActionColl") @RequiredArgsConstructor public static class ActionMixinWithColl { @SuppressWarnings("unused") - private final InvalidMixinDeclarations invalidElementTypes; - @MemberSupport public String coll() { return ""; } - }*/ + private final InvalidMixinDeclarations mixee; + @MemberSupport public List other() { return List.of(); } + } + + @Property + @PropertyLayout(named = "someProperty") + @RequiredArgsConstructor + public static class PropertyMixinWithOther { + @SuppressWarnings("unused") + private final InvalidMixinDeclarations mixee; + @MemberSupport public void other() { } + } + + @Collection + @CollectionLayout(named = "someCollection") + @RequiredArgsConstructor + public static class CollectionMixinWithOther { + @SuppressWarnings("unused") + private final InvalidMixinDeclarations mixee; + @MemberSupport public void other() { } + } }