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 2018/09/28 20:16:16 UTC

[isis] 02/02: ISIS-1976: facet lookup: cleanup some code smell

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

ahuber pushed a commit to branch v2
in repository https://gitbox.apache.org/repos/asf/isis.git

commit f9ebfa2adc414d69d602d76d59fdf875f192dd26
Author: Andi Huber <ah...@apache.org>
AuthorDate: Fri Sep 28 22:12:10 2018 +0200

    ISIS-1976: facet lookup: cleanup some code smell
    
    Task-Url: https://issues.apache.org/jira/browse/ISIS-1976
---
 .../core/metamodel/spec/ObjectSpecification.java   |  2 +
 .../specimpl/ObjectSpecificationAbstract.java      | 74 +++++++++++-----------
 2 files changed, 40 insertions(+), 36 deletions(-)

diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/spec/ObjectSpecification.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/spec/ObjectSpecification.java
index 9a99c37..33d2fc3 100644
--- a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/spec/ObjectSpecification.java
+++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/spec/ObjectSpecification.java
@@ -342,5 +342,7 @@ ObjectAssociationContainer, Hierarchical,  DefaultProvider {
     boolean isPersistenceCapable();
     boolean isPersistenceCapableOrViewModel();
 
+    
+
 
 }
diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/specloader/specimpl/ObjectSpecificationAbstract.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/specloader/specimpl/ObjectSpecificationAbstract.java
index ba8c158..22c9711 100644
--- a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/specloader/specimpl/ObjectSpecificationAbstract.java
+++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/specloader/specimpl/ObjectSpecificationAbstract.java
@@ -26,6 +26,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Function;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -37,6 +38,7 @@ import org.slf4j.LoggerFactory;
 import org.apache.isis.applib.AppManifest;
 import org.apache.isis.applib.Identifier;
 import org.apache.isis.applib.annotation.Where;
+import org.apache.isis.commons.internal.base._NullSafe;
 import org.apache.isis.commons.internal.base._Strings;
 import org.apache.isis.commons.internal.collections._Lists;
 import org.apache.isis.commons.internal.collections._Maps;
@@ -468,46 +470,46 @@ public abstract class ObjectSpecificationAbstract extends FacetHolderImpl implem
 
     @Override
     public <Q extends Facet> Q getFacet(final Class<Q> facetType) {
-        final Q facet = super.getFacet(facetType);
-        if (isNotANoopFacet(facet)) {
-            return facet;
-        }
-        Q noopFacet = facet; // might be null
-        if (interfaces() != null) {
-            final List<ObjectSpecification> interfaces = interfaces();
-            for (int i = 0; i < interfaces.size(); i++) {
-                final ObjectSpecification interfaceSpec = interfaces.get(i);
-                if (interfaceSpec == null) {
-                    // HACK: shouldn't happen, but occurring on occasion when
-                    // running
-                    // XATs under JUnit4. Some sort of race condition?
-                    continue;
-                }
-                final Q interfaceFacet = interfaceSpec.getFacet(facetType);
-                if (isNotANoopFacet(interfaceFacet)) {
-                    return interfaceFacet;
-                }
-                if (noopFacet == null) {
-                    noopFacet = interfaceFacet; // might be null
-                }
-            }
-        }
+
+        // lookup facet holder's facet
+        final Stream<Q> facets1 = _NullSafe.streamNullable(super.getFacet(facetType));
+        
+        // lookup all interfaces
+        final Stream<Q> facets2 = _NullSafe.stream(interfaces)
+                .filter(_NullSafe::isPresent) // just in case
+                .map(interfaceSpec->interfaceSpec.getFacet(facetType));
+        
         // search up the inheritance hierarchy
-        final ObjectSpecification superSpec = superclass();
-        if (superSpec != null) {
-            final Q superClassFacet = superSpec.getFacet(facetType);
-            if (isNotANoopFacet(superClassFacet)) {
-                return superClassFacet;
-            }
-            // TODO: should we perhaps default the noopFacet here as we do in the previous two cases?
-        }
-        return noopFacet;
+        final Stream<Q> facets3 = _NullSafe.streamNullable(superclass()) 
+                .map(superSpec->superSpec.getFacet(facetType));
+        
+        final Stream<Q> facetsCombined = Stream.concat(Stream.concat(facets1, facets2), facets3);
+        
+        final NotANoopFacetFilter<Q> notANoopFacetFilter = new NotANoopFacetFilter<>();
+        
+        return facetsCombined
+        .filter(notANoopFacetFilter)
+        .findFirst()
+        .orElse(notANoopFacetFilter.noopFacet);
     }
 
-    private boolean isNotANoopFacet(final Facet facet) {
-        return facet != null && !facet.isNoop();
-    }
+    private static class NotANoopFacetFilter<Q extends Facet> implements Predicate<Q> {
+        Q noopFacet;
 
+        @Override
+        public boolean test(Q facet) {
+            if(facet==null) {
+                return false;
+            }
+            if(!facet.isNoop()) {
+                return true;
+            }
+            if(noopFacet == null) {
+                noopFacet = facet;
+            }
+            return false;
+        }
+    }
 
 
     // -- DefaultValue - unused