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/12/20 15:24:59 UTC

[isis] 01/02: ISIS-2050: fixes suspected race condition in metamodel validation of DomainObjectAnnotationFacetFactory

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

danhaywood pushed a commit to branch ISIS-2050
in repository https://gitbox.apache.org/repos/asf/isis.git

commit 5f32ad6dde30cffdecbd6bec4873db0b64e961de
Author: danhaywood <da...@haywood-associates.co.uk>
AuthorDate: Thu Dec 20 15:20:13 2018 +0000

    ISIS-2050: fixes suspected race condition in metamodel validation of DomainObjectAnnotationFacetFactory
    
    was using servicesInjector#isRegisteredService to determine if a referenced domain class was a repository.  However, this uses a flag that is only set once a PersistenceSession has been created, so (I think) resulted in a race condition.  The new implementation only looks at the DomainServiceFacet of the ObjectSpecification.
    
    Also:
    - promoted IntrospectionMode to a top-level class
---
 .../DomainObjectAnnotationFacetFactory.java        |  4 +-
 .../metamodel/specloader/IntrospectionMode.java    | 52 ++++++++++++++++++++
 .../metamodel/specloader/SpecificationLoader.java  | 56 ++++++++++------------
 3 files changed, 78 insertions(+), 34 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 77e6a32..af2e630 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
@@ -621,8 +621,8 @@ public class DomainObjectAnnotationFacetFactory extends FacetFactoryAbstract
                 if(autoCompleteFacet != null && !autoCompleteFacet.isNoop() && autoCompleteFacet instanceof AutoCompleteFacetAbstract) {
                     final AutoCompleteFacetAbstract facet = (AutoCompleteFacetForDomainObjectAnnotation) autoCompleteFacet;
                     final Class<?> repositoryClass = facet.getRepositoryClass();
-                    final boolean isRegistered = servicesInjector.isRegisteredService(repositoryClass);
-                    if(!isRegistered) {
+                    final boolean isService = getSpecificationLoader().isServiceClass(repositoryClass);
+                    if(!isService) {
                         validationFailures.add(
                                 "@DomainObject annotation on %s specifies unknown repository '%s'",
                                 thisSpec.getFullIdentifier(), repositoryClass.getName());
diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/specloader/IntrospectionMode.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/specloader/IntrospectionMode.java
new file mode 100644
index 0000000..aafdfd3
--- /dev/null
+++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/specloader/IntrospectionMode.java
@@ -0,0 +1,52 @@
+/*
+ *  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.core.metamodel.specloader;
+
+import org.apache.isis.core.metamodel.deployment.DeploymentCategory;
+
+public enum IntrospectionMode {
+    /**
+     * Lazy (don't introspect members for most classes unless required), irrespective of the deployment mode.
+     */
+    LAZY{
+        @Override
+        public boolean isFullIntrospect(final DeploymentCategory category) {
+            return false;
+        }
+    },
+    /**
+     * If production deployment mode, then full, otherwise lazy.
+     */
+    LAZY_UNLESS_PRODUCTION {
+        @Override public boolean isFullIntrospect(final DeploymentCategory category) {
+            return category.isProduction();
+        }
+    },
+    /**
+     * Full introspection, irrespective of deployment mode.
+     */
+    FULL {
+        @Override
+        public boolean isFullIntrospect(final DeploymentCategory category) {
+            return true;
+        }
+    };
+
+    public abstract boolean isFullIntrospect(final DeploymentCategory category);
+}
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 a8ffae0..67a5906 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
@@ -42,8 +42,8 @@ import org.apache.isis.core.commons.config.IsisConfiguration;
 import org.apache.isis.core.commons.ensure.Assert;
 import org.apache.isis.core.commons.exceptions.IsisException;
 import org.apache.isis.core.commons.lang.ClassUtil;
-import org.apache.isis.core.metamodel.deployment.DeploymentCategory;
 import org.apache.isis.core.metamodel.facetapi.Facet;
+import org.apache.isis.core.metamodel.facets.object.domainservice.DomainServiceFacet;
 import org.apache.isis.core.metamodel.facets.object.objectspecid.ObjectSpecIdFacet;
 import org.apache.isis.core.metamodel.layoutmetadata.LayoutMetadataReader;
 import org.apache.isis.core.metamodel.progmodel.ProgrammingModel;
@@ -96,36 +96,6 @@ public class SpecificationLoader implements ApplicationScopedComponent {
     public static final ConfigPropertyBoolean CONFIG_PROPERTY_PARALLELIZE =
             new ConfigPropertyBoolean("isis.reflector.introspector.parallelize", true);
 
-    public static enum IntrospectionMode {
-        /**
-         * Lazy (don't introspect members for most classes unless required), irrespective of the deployment mode.
-         */
-        LAZY{
-            @Override
-            public boolean isFullIntrospect(final DeploymentCategory category) {
-                return false;
-            }
-        },
-        /**
-         * If production deployment mode, then full, otherwise lazy.
-         */
-        LAZY_UNLESS_PRODUCTION {
-            @Override public boolean isFullIntrospect(final DeploymentCategory category) {
-                return category.isProduction();
-            }
-        },
-        /**
-         * Full introspection, irrespective of deployment mode.
-         */
-        FULL {
-            @Override
-            public boolean isFullIntrospect(final DeploymentCategory category) {
-                return true;
-            }
-        };
-
-        public abstract boolean isFullIntrospect(final DeploymentCategory category);
-    }
     public static final ConfigPropertyEnum<IntrospectionMode> CONFIG_PROPERTY_MODE =
             new ConfigPropertyEnum<>("isis.reflector.introspector.mode", IntrospectionMode.LAZY_UNLESS_PRODUCTION);
 
@@ -476,11 +446,32 @@ public class SpecificationLoader implements ApplicationScopedComponent {
     }
 
     @Programmatic
+    public ObjectSpecification peekSpecification(final Class<?> type) {
+
+        final Class<?> substitutedType = classSubstitutor.getClass(type);
+        if (substitutedType == null) {
+            return null;
+        }
+
+        final String typeName = substitutedType.getName();
+        ObjectSpecification spec = cache.get(typeName);
+        if (spec != null) {
+            return spec;
+        }
+
+        return null;
+    }
+
+    @Programmatic
     public ObjectSpecification loadSpecification(final Class<?> type, final IntrospectionState upTo) {
         final ObjectSpecification spec = internalLoadSpecification(type, null, upTo);
         if(spec == null) {
             return null;
         }
+
+        // TODO: review, is this now needed?
+        //  We now create the ObjectSpecIdFacet immediately after creating the ObjectSpecification,
+        //  so the cache shouldn't need updating here also.
         if(cache.isInitialized()) {
             // umm.  It turns out that anonymous inner classes (eg org.estatio.dom.WithTitleGetter$ToString$1)
             // don't have an ObjectSpecId; hence the guard.
@@ -638,7 +629,8 @@ public class SpecificationLoader implements ApplicationScopedComponent {
 
     @Programmatic
     public boolean isServiceClass(Class<?> cls) {
-        return this.servicesInjector.isRegisteredService(cls);
+        final ObjectSpecification objectSpecification = peekSpecification(cls);
+        return objectSpecification != null && objectSpecification.containsDoOpFacet(DomainServiceFacet.class);
     }
 
     //endregion