You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@isis.apache.org by da...@apache.org on 2018/01/02 17:09:17 UTC
[isis] branch master updated: ISIS-1741: moves responsibility for
taking a protective list copy of all specifications obtained from
SpecificationLoader
This is an automated email from the ASF dual-hosted git repository.
danhaywood 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 42e9160 ISIS-1741: moves responsibility for taking a protective list copy of all specifications obtained from SpecificationLoader
42e9160 is described below
commit 42e9160fe1d445edf621a3c694bcd0bca20590cc
Author: Dan Haywood <da...@haywood-associates.co.uk>
AuthorDate: Tue Jan 2 17:09:04 2018 +0000
ISIS-1741: moves responsibility for taking a protective list copy of all specifications obtained from SpecificationLoader
previously all clients were taking a copy (and this bug was yet another case where a protective copy hadn't been taken); is now done by supplier
---
.../domainobject/DomainObjectAnnotationFacetFactory.java | 4 +---
.../metamodel/services/swagger/internal/Generation.java | 16 +++++++---------
.../core/metamodel/specloader/SpecificationLoader.java | 14 ++++++++++++--
.../specloader/validator/MetaModelValidatorVisiting.java | 7 ++++---
.../core/runtime/system/session/IsisSessionFactory.java | 9 +++++----
5 files changed, 29 insertions(+), 21 deletions(-)
diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/object/domainobject/DomainObjectAnnotationFacetFactory.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/object/domainobject/DomainObjectAnnotationFacetFactory.java
index 34a1811..3a60287 100644
--- a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/object/domainobject/DomainObjectAnnotationFacetFactory.java
+++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/object/domainobject/DomainObjectAnnotationFacetFactory.java
@@ -19,7 +19,6 @@
package org.apache.isis.core.metamodel.facets.object.domainobject;
import java.lang.reflect.Method;
-import java.util.Collection;
import java.util.Map;
import javax.annotation.PostConstruct;
@@ -551,8 +550,7 @@ public class DomainObjectAnnotationFacetFactory extends FacetFactoryAbstract
}
final Map<ObjectSpecId, ObjectSpecification> specById = Maps.newHashMap();
- final Collection<ObjectSpecification> allSpecifications = getSpecificationLoader().allSpecifications();
- for (final ObjectSpecification otherSpec : allSpecifications) {
+ for (final ObjectSpecification otherSpec : getSpecificationLoader().allSpecifications()) {
if(thisSpec == otherSpec) {
continue;
diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/services/swagger/internal/Generation.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/services/swagger/internal/Generation.java
index d76dfb9..604624b 100644
--- a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/services/swagger/internal/Generation.java
+++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/services/swagger/internal/Generation.java
@@ -31,7 +31,6 @@ import com.google.common.base.Joiner;
import com.google.common.base.Strings;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Lists;
import com.google.common.collect.Ordering;
import com.google.common.collect.Sets;
@@ -45,12 +44,12 @@ import org.apache.isis.core.metamodel.facets.object.mixin.MixinFacet;
import org.apache.isis.core.metamodel.facets.object.objectspecid.ObjectSpecIdFacet;
import org.apache.isis.core.metamodel.services.ServiceUtil;
import org.apache.isis.core.metamodel.spec.ObjectSpecification;
-import org.apache.isis.core.metamodel.specloader.SpecificationLoader;
import org.apache.isis.core.metamodel.spec.feature.Contributed;
import org.apache.isis.core.metamodel.spec.feature.ObjectAction;
import org.apache.isis.core.metamodel.spec.feature.ObjectActionParameter;
import org.apache.isis.core.metamodel.spec.feature.OneToManyAssociation;
import org.apache.isis.core.metamodel.spec.feature.OneToOneAssociation;
+import org.apache.isis.core.metamodel.specloader.SpecificationLoader;
import io.swagger.models.Info;
import io.swagger.models.ModelImpl;
@@ -129,10 +128,9 @@ class Generation {
}
void appendServicePathsAndDefinitions() {
- // take copy to avoid concurrent modification exception
- final Collection<ObjectSpecification> allSpecs = Lists.newArrayList(specificationLoader.allSpecifications());
-
- for (final ObjectSpecification objectSpec : allSpecs) {
+ // (previously we took a protective copy to avoid a concurrent modification exception,
+ // but this is now done by SpecificationLoader itself)
+ for (final ObjectSpecification objectSpec : specificationLoader.allSpecifications()) {
final DomainServiceFacet domainServiceFacet = objectSpec.getFacet(DomainServiceFacet.class);
if (domainServiceFacet == null) {
@@ -182,9 +180,9 @@ class Generation {
}
void appendObjectPathsAndDefinitions() {
- // take copy to avoid concurrent modification exception
- final Collection<ObjectSpecification> allSpecs = Lists.newArrayList(specificationLoader.allSpecifications());
- for (final ObjectSpecification objectSpec : allSpecs) {
+ // (previously we took a protective copy to avoid a concurrent modification exception,
+ // but this is now done by SpecificationLoader itself)
+ for (final ObjectSpecification objectSpec : specificationLoader.allSpecifications()) {
final DomainServiceFacet domainServiceFacet = objectSpec.getFacet(DomainServiceFacet.class);
if (domainServiceFacet != null) {
diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/specloader/SpecificationLoader.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/specloader/SpecificationLoader.java
index 1625116..f4a30ff 100644
--- a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/specloader/SpecificationLoader.java
+++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/specloader/SpecificationLoader.java
@@ -175,7 +175,7 @@ public class SpecificationLoader implements ApplicationScopedComponent {
private void cacheBySpecId() {
final Map<ObjectSpecId, ObjectSpecification> specById = Maps.newHashMap();
- for (final ObjectSpecification objSpec : allSpecifications()) {
+ for (final ObjectSpecification objSpec : allCachedSpecifications()) {
final ObjectSpecId objectSpecId = objSpec.getSpecId();
if (objectSpecId == null) {
continue;
@@ -460,10 +460,20 @@ public class SpecificationLoader implements ApplicationScopedComponent {
//region > allSpecifications
/**
- * Return all the loaded specifications.
+ * Returns (a new list holding a copy of) all the loaded specifications.
+ *
+ * <p>
+ * A new list is returned to avoid concurrent modification exceptions for if the caller then
+ * iterates over all the specifications and performs an activity that might give rise to new
+ * ObjectSpec's being discovered, eg performing metamodel validation.
+ * </p>
*/
@Programmatic
public Collection<ObjectSpecification> allSpecifications() {
+ return Lists.newArrayList(allCachedSpecifications());
+ }
+
+ private Collection<ObjectSpecification> allCachedSpecifications() {
return cache.allSpecifications();
}
diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/specloader/validator/MetaModelValidatorVisiting.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/specloader/validator/MetaModelValidatorVisiting.java
index 7edca60..6b2ff6b 100644
--- a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/specloader/validator/MetaModelValidatorVisiting.java
+++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/specloader/validator/MetaModelValidatorVisiting.java
@@ -19,6 +19,7 @@
package org.apache.isis.core.metamodel.specloader.validator;
+import java.util.Collection;
import java.util.List;
import com.google.common.collect.Lists;
@@ -65,9 +66,9 @@ public class MetaModelValidatorVisiting extends MetaModelValidatorAbstract {
final ValidationFailures validationFailures) {
// all currently known specs
- // (we take a protective copy in case any of the metamodel validators cause us to discover further object specs)
- final List<ObjectSpecification> specsToValidate =
- Lists.newArrayList(specificationLoader.allSpecifications());
+ // (previously we took a protective copy to avoid a concurrent modification exception,
+ // but this is now done by SpecificationLoader itself)
+ final Collection<ObjectSpecification> specsToValidate = specificationLoader.allSpecifications();
// don't validate any specs already processed
specsToValidate.removeAll(specsAlreadyValidated);
diff --git a/core/runtime/src/main/java/org/apache/isis/core/runtime/system/session/IsisSessionFactory.java b/core/runtime/src/main/java/org/apache/isis/core/runtime/system/session/IsisSessionFactory.java
index 368f44b..f7b48cf 100644
--- a/core/runtime/src/main/java/org/apache/isis/core/runtime/system/session/IsisSessionFactory.java
+++ b/core/runtime/src/main/java/org/apache/isis/core/runtime/system/session/IsisSessionFactory.java
@@ -155,16 +155,17 @@ public class IsisSessionFactory
//
final List<Object> services = servicesInjector.getRegisteredServices();
- // take a copy of all services to avoid occasionall concurrent modification exceptions
+ // take a copy of all services to avoid occasional concurrent modification exceptions
// that can sometimes occur in the loop
final List<Object> copyOfServices = Lists.newArrayList(services);
final TitleService titleService = servicesInjector.lookupServiceElseFail(TitleService.class);
for (Object service : copyOfServices) {
final String unused = titleService.titleOf(service);
}
- final List<ObjectSpecification> objectSpecsCopy =
- Lists.newArrayList(servicesInjector.getSpecificationLoader().allSpecifications());
- for (final ObjectSpecification objSpec : objectSpecsCopy) {
+
+ // (previously we took a protective copy to avoid a concurrent modification exception,
+ // but this is now done by SpecificationLoader itself)
+ for (final ObjectSpecification objSpec : servicesInjector.getSpecificationLoader().allSpecifications()) {
final Class<?> correspondingClass = objSpec.getCorrespondingClass();
if(correspondingClass.isEnum()) {
final Object[] enumConstants = correspondingClass.getEnumConstants();
--
To stop receiving notification emails like this one, please contact
['"commits@isis.apache.org" <co...@isis.apache.org>'].