You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@isis.apache.org by ah...@apache.org on 2021/04/14 11:15:29 UTC
[isis] branch master updated: ISIS-2493: adds tests;
also fixes overload detection
This is an automated email from the ASF dual-hosted git repository.
ahuber pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/isis.git
The following commit(s) were added to refs/heads/master by this push:
new 0ef966a ISIS-2493: adds tests; also fixes overload detection
0ef966a is described below
commit 0ef966a92cb9f862377ee2bafe4e1a1eb8dc32ff
Author: Andi Huber <ah...@apache.org>
AuthorDate: Wed Apr 14 13:15:18 2021 +0200
ISIS-2493: adds tests; also fixes overload detection
---
.../apache/isis/commons/collections/CanTest.java | 17 ++++++++-
.../actions/action/ActionOverloadingValidator.java | 39 +++++++++------------
.../spec/feature/ObjectActionContainer.java | 12 ++++++-
.../specloader/specimpl/ObjectMemberContainer.java | 13 +++++--
.../testspec/ObjectSpecificationStub.java | 6 +++-
.../model/bad/InvalidActionOverloading.java | 40 ++++++++++++++++++++++
.../DomainModelTest_usingBadDomain.java | 8 +++++
.../rendering/service/swagger/internal/Util.java | 4 +--
8 files changed, 109 insertions(+), 30 deletions(-)
diff --git a/commons/src/test/java/org/apache/isis/commons/collections/CanTest.java b/commons/src/test/java/org/apache/isis/commons/collections/CanTest.java
index 7430eb3..6095a05 100644
--- a/commons/src/test/java/org/apache/isis/commons/collections/CanTest.java
+++ b/commons/src/test/java/org/apache/isis/commons/collections/CanTest.java
@@ -19,6 +19,7 @@
package org.apache.isis.commons.collections;
import java.io.IOException;
+import java.util.Set;
import java.util.stream.Stream;
import org.junit.jupiter.api.Test;
@@ -27,6 +28,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import org.apache.isis.commons.internal.collections._Sets;
import org.apache.isis.commons.internal.testing._SerializationTester;
import lombok.val;
@@ -151,9 +153,22 @@ class CanTest {
}
+ // -- TO SET CONVERSION
+ @Test
+ void multiCan_toSet_should_find_duplicates() {
+ val expectedSet = _Sets.of("a", "b", "c");
+ val duplicates = _Sets.<String>newHashSet();
+
+ assertSetEquals(expectedSet, Can.<String>of("a", "c", "b", "a").toSet());
+ assertSetEquals(expectedSet, Can.<String>of("a", "c", "b", "a").toSet(duplicates::add));
+ assertSetEquals(_Sets.of("a"), duplicates);
+ }
-
+ private static <T> void assertSetEquals(Set<T> a, Set<T> b) {
+ assertTrue(_Sets.minus(a, b).isEmpty());
+ assertTrue(_Sets.minus(b, a).isEmpty());
+ }
}
diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/actions/action/ActionOverloadingValidator.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/actions/action/ActionOverloadingValidator.java
index b802775..e9ee60f 100644
--- a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/actions/action/ActionOverloadingValidator.java
+++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/actions/action/ActionOverloadingValidator.java
@@ -18,13 +18,11 @@
*/
package org.apache.isis.core.metamodel.facets.actions.action;
-import org.apache.isis.applib.Identifier;
import org.apache.isis.applib.services.metamodel.BeanSort;
-import org.apache.isis.commons.collections.Can;
import org.apache.isis.commons.internal.collections._Sets;
+import org.apache.isis.core.metamodel.spec.ActionType;
import org.apache.isis.core.metamodel.spec.ObjectSpecification;
import org.apache.isis.core.metamodel.spec.feature.MixedIn;
-import org.apache.isis.core.metamodel.spec.feature.ObjectAction;
import org.apache.isis.core.metamodel.specloader.validator.MetaModelVisitingValidatorAbstract;
import org.apache.isis.core.metamodel.specloader.validator.ValidationFailure;
@@ -45,29 +43,24 @@ extends MetaModelVisitingValidatorAbstract {
@Override
public void validate(@NonNull ObjectSpecification spec) {
- if(spec.getBeanSort()==BeanSort.UNKNOWN
+ if(spec.getBeanSort()!=BeanSort.UNKNOWN
&& !spec.isAbstract()) {
- val actionMemberNames = spec.streamActions(MixedIn.INCLUDED)
- .map(ObjectAction::getIdentifier)
- .map(Identifier::getMemberName)
- .collect(Can.toCan());
+ val overloadedNames = _Sets.<String>newHashSet();
- if (actionMemberNames.isCardinalityMultiple()) {
-
- val overloadedNames = _Sets.<String>newHashSet();
-
- actionMemberNames.toSet(overloadedNames::add);
-
- if(!overloadedNames.isEmpty()) {
-
- ValidationFailure.raiseFormatted(
- spec,
- "Action method overloading is not allowed, "
- + "yet %s has action(s) that have a the same member name: %s",
- spec.getCorrespondingClass().getName(),
- overloadedNames);
- }
+ spec.streamActions(ActionType.ANY, MixedIn.INCLUDED, oa->{
+ overloadedNames.add(oa.getIdentifier().getMemberName());
+ })
+ .count(); // consumer the stream
+
+ if(!overloadedNames.isEmpty()) {
+
+ ValidationFailure.raiseFormatted(
+ spec,
+ "Action method overloading is not allowed, "
+ + "yet %s has action(s) that have a the same member name: %s",
+ spec.getCorrespondingClass().getName(),
+ overloadedNames);
}
}
diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/spec/feature/ObjectActionContainer.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/spec/feature/ObjectActionContainer.java
index eff0ef5..6084ac1 100644
--- a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/spec/feature/ObjectActionContainer.java
+++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/spec/feature/ObjectActionContainer.java
@@ -20,6 +20,7 @@
package org.apache.isis.core.metamodel.spec.feature;
import java.util.Optional;
+import java.util.function.Consumer;
import java.util.stream.Stream;
import javax.annotation.Nullable;
@@ -85,7 +86,16 @@ public interface ObjectActionContainer {
// -- ACTION STREAM (W/ INHERITANCE)
- Stream<ObjectAction> streamActions(ImmutableEnumSet<ActionType> types, MixedIn contributed);
+ Stream<ObjectAction> streamActions(
+ ImmutableEnumSet<ActionType> types,
+ MixedIn contributed,
+ Consumer<ObjectAction> onActionOverloaded);
+
+ default Stream<ObjectAction> streamActions(
+ ImmutableEnumSet<ActionType> types,
+ MixedIn contributed) {
+ return streamActions(types, contributed, __->{});
+ }
default Stream<ObjectAction> streamActions(ActionType type, MixedIn contributed) {
return streamActions(ImmutableEnumSet.of(type), contributed);
diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/specloader/specimpl/ObjectMemberContainer.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/specloader/specimpl/ObjectMemberContainer.java
index 779465a..76337ff 100644
--- a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/specloader/specimpl/ObjectMemberContainer.java
+++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/specloader/specimpl/ObjectMemberContainer.java
@@ -19,6 +19,7 @@
package org.apache.isis.core.metamodel.specloader.specimpl;
import java.util.Optional;
+import java.util.function.Consumer;
import java.util.stream.Stream;
import javax.annotation.Nullable;
@@ -77,7 +78,8 @@ implements
@Override
public Stream<ObjectAction> streamActions(
final ImmutableEnumSet<ActionType> types,
- final MixedIn contributed) {
+ final MixedIn contributed,
+ final Consumer<ObjectAction> onActionOverloaded) {
if(isTypeHierarchyRoot()) {
return streamDeclaredActions(contributed); // stop going deeper
@@ -89,7 +91,14 @@ implements
streamDeclaredActions(contributed),
superclass().streamActions(contributed)
)
- .filter(action->ids.add(action.getId())); // ensure we don't emit duplicates
+ .filter(action->{
+ // ensure we don't emit duplicates
+ val isUnique = ids.add(action.getId());
+ if(!isUnique) {
+ onActionOverloaded.accept(action);
+ }
+ return isUnique;
+ });
}
// -- ASSOCIATIONS
diff --git a/core/metamodel/src/test/java/org/apache/isis/core/metamodel/testspec/ObjectSpecificationStub.java b/core/metamodel/src/test/java/org/apache/isis/core/metamodel/testspec/ObjectSpecificationStub.java
index 2fefd9b..06f9e04 100644
--- a/core/metamodel/src/test/java/org/apache/isis/core/metamodel/testspec/ObjectSpecificationStub.java
+++ b/core/metamodel/src/test/java/org/apache/isis/core/metamodel/testspec/ObjectSpecificationStub.java
@@ -24,6 +24,7 @@ import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.Set;
+import java.util.function.Consumer;
import java.util.stream.Stream;
import org.apache.isis.applib.Identifier;
@@ -358,7 +359,10 @@ implements ObjectSpecification {
}
@Override
- public Stream<ObjectAction> streamActions(ImmutableEnumSet<ActionType> types, MixedIn contributed) {
+ public Stream<ObjectAction> streamActions(
+ ImmutableEnumSet<ActionType> types,
+ MixedIn contributed,
+ final Consumer<ObjectAction> onActionOverloaded) {
// poorly implemented, inheritance not supported
return streamDeclaredActions(contributed);
}
diff --git a/regressiontests/stable/src/main/java/org/apache/isis/testdomain/model/bad/InvalidActionOverloading.java b/regressiontests/stable/src/main/java/org/apache/isis/testdomain/model/bad/InvalidActionOverloading.java
new file mode 100644
index 0000000..fcbd57f
--- /dev/null
+++ b/regressiontests/stable/src/main/java/org/apache/isis/testdomain/model/bad/InvalidActionOverloading.java
@@ -0,0 +1,40 @@
+/*
+ * 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.isis.testdomain.model.bad;
+
+import org.apache.isis.applib.annotation.Action;
+import org.apache.isis.applib.annotation.DomainObject;
+import org.apache.isis.applib.annotation.Nature;
+
+@DomainObject(nature = Nature.VIEW_MODEL)
+public class InvalidActionOverloading {
+
+ // overloading not allowed: should fail
+ @Action
+ public boolean anAction() {
+ return false;
+ }
+
+ // overloading not allowed: should fail
+ @Action
+ public String anAction(String param) {
+ return param;
+ }
+
+}
diff --git a/regressiontests/stable/src/test/java/org/apache/isis/testdomain/domainmodel/DomainModelTest_usingBadDomain.java b/regressiontests/stable/src/test/java/org/apache/isis/testdomain/domainmodel/DomainModelTest_usingBadDomain.java
index 083a140..2c0b5e4 100644
--- a/regressiontests/stable/src/test/java/org/apache/isis/testdomain/domainmodel/DomainModelTest_usingBadDomain.java
+++ b/regressiontests/stable/src/test/java/org/apache/isis/testdomain/domainmodel/DomainModelTest_usingBadDomain.java
@@ -53,6 +53,7 @@ import org.apache.isis.testdomain.conf.Configuration_headless;
import org.apache.isis.testdomain.model.bad.AmbiguousMixinAnnotations;
import org.apache.isis.testdomain.model.bad.AmbiguousTitle;
import org.apache.isis.testdomain.model.bad.Configuration_usingInvalidDomain;
+import org.apache.isis.testdomain.model.bad.InvalidActionOverloading;
import org.apache.isis.testdomain.model.bad.InvalidOrphanedActionSupport;
import org.apache.isis.testdomain.model.bad.InvalidOrphanedCollectionSupport;
import org.apache.isis.testdomain.model.bad.InvalidOrphanedPropertySupport;
@@ -121,6 +122,13 @@ class DomainModelTest_usingBadDomain {
"is assumed to support"));
}
+ @Test
+ void actionOverloading_shouldFail() {
+ assertTrue(validator.anyMatchesContaining(
+ Identifier.classIdentifier(LogicalType.fqcn(InvalidActionOverloading.class)),
+ "Action method overloading is not allowed"));
+ }
+
@ParameterizedTest
@MethodSource("provideAmbiguousMixins")
void ambiguousMixinAnnotions_shouldFailValidation(
diff --git a/viewers/restfulobjects/rendering/src/main/java/org/apache/isis/viewer/restfulobjects/rendering/service/swagger/internal/Util.java b/viewers/restfulobjects/rendering/src/main/java/org/apache/isis/viewer/restfulobjects/rendering/service/swagger/internal/Util.java
index 65a4cb0..772b613 100644
--- a/viewers/restfulobjects/rendering/src/main/java/org/apache/isis/viewer/restfulobjects/rendering/service/swagger/internal/Util.java
+++ b/viewers/restfulobjects/rendering/src/main/java/org/apache/isis/viewer/restfulobjects/rendering/service/swagger/internal/Util.java
@@ -145,8 +145,8 @@ public final class Util {
return objectSpec.streamActions(actionTypes, MixedIn.INCLUDED)
.filter(objectAction->
!classExcluder.exclude(objectAction)
- && !visibility.isPublic()
- || isVisibleForPublic(objectAction) )
+ && !visibility.isPublic()
+ || isVisibleForPublic(objectAction) )
.collect(Collectors.toList());
}