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/06/04 06:45:22 UTC

[isis] 01/02: ISIS-1960: Adding background error handling

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

commit 272a6198a28d643a8f5ef8e0519ee6a154caecbe
Author: Andi Huber <ah...@apache.org>
AuthorDate: Mon Jun 4 08:36:54 2018 +0200

    ISIS-1960: Adding background error handling
    
    also init thread-pool only if required
    
    Task-Url: https://issues.apache.org/jira/browse/ISIS-1960
---
 .../background/BackgroundServiceDefault.java       | 222 ++++++++++++---------
 .../background/ForkingInvocationHandler.java       |  15 +-
 2 files changed, 138 insertions(+), 99 deletions(-)

diff --git a/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/BackgroundServiceDefault.java b/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/BackgroundServiceDefault.java
index 14cf750..75d20b3 100644
--- a/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/BackgroundServiceDefault.java
+++ b/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/BackgroundServiceDefault.java
@@ -16,6 +16,8 @@
  */
 package org.apache.isis.core.runtime.services.background;
 
+import static org.apache.isis.commons.internal.base._Casts.uncheckedCast;
+
 import java.lang.reflect.InvocationHandler;
 import java.util.Map;
 import java.util.concurrent.ExecutorService;
@@ -33,7 +35,6 @@ import org.apache.isis.applib.services.background.BackgroundService2;
 import org.apache.isis.applib.services.command.CommandContext;
 import org.apache.isis.applib.services.factory.FactoryService;
 import org.apache.isis.commons.internal._Constants;
-import org.apache.isis.commons.internal.base._Casts;
 import org.apache.isis.core.commons.lang.ArrayExtensions;
 import org.apache.isis.core.metamodel.adapter.mgr.AdapterManager;
 import org.apache.isis.core.metamodel.services.command.CommandDtoServiceInternal;
@@ -42,92 +43,117 @@ import org.apache.isis.core.metamodel.specloader.SpecificationLoader;
 import org.apache.isis.core.metamodel.specloader.classsubstitutor.ProxyEnhanced;
 import org.apache.isis.core.plugins.codegen.ProxyFactory;
 import org.apache.isis.core.runtime.system.session.IsisSessionFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
- * Depends on an implementation of {@link org.apache.isis.applib.services.background.BackgroundCommandService} to
+ * For command-reification depends on an implementation of 
+ * {@link org.apache.isis.applib.services.background.BackgroundCommandService} to
  * be configured.
  */
 @DomainService(
-        nature = NatureOfService.DOMAIN,
-        menuOrder = "" + Integer.MAX_VALUE
-)
+		nature = NatureOfService.DOMAIN,
+		menuOrder = "" + Integer.MAX_VALUE
+		)
 public class BackgroundServiceDefault implements BackgroundService2 {
-
-	private final int threadCount = Runtime.getRuntime().availableProcessors();
 	
-	private final ExecutorService backgroundExecutorService = 
-			Executors.newFixedThreadPool(threadCount);
-	
-    @Programmatic
-    @PostConstruct
-    public void init(Map<String,String> props) {
-    }
-
-    @Programmatic
-    @PreDestroy
-    public void shutdown() {
-    	backgroundExecutorService.shutdownNow();
-    }
-    
-    ObjectSpecification getSpecification(final Class<?> type) {
-        return specificationLoader.loadSpecification(type);
-    }
-
-    // //////////////////////////////////////
-
-    @Programmatic
-    @Override
-    public <T> T execute(final T domainObject) {
-        final Class<T> cls = _Casts.uncheckedCast(domainObject.getClass());
-        final InvocationHandler methodHandler = newMethodHandler(domainObject, null);
-        return newProxy(cls, null, methodHandler);
-    }
-
-    @Override
-    public <T> T executeMixin(Class<T> mixinClass, Object mixedIn) {
-        final T mixin = factoryService.mixin(mixinClass, mixedIn);
-        final InvocationHandler methodHandler = newMethodHandler(mixin, mixedIn);
-        return newProxy(mixinClass, mixedIn, methodHandler);
-    }
-
-    private <T> T newProxy(
-            final Class<T> cls,
-            final Object mixedInIfAny,
-            final InvocationHandler methodHandler) {
-
-    	final Class<?>[] interfaces = ArrayExtensions.combine(
-    			cls.getInterfaces(), 
-    			new Class<?>[] { ProxyEnhanced.class }); 
-    	
-    	final boolean initialize = mixedInIfAny!=null;
-    	
-    	
-        final Class<?>[] constructorArgTypes = initialize ? new Class<?>[] {mixedInIfAny.getClass()} : _Constants.emptyClasses;
-        final Object[] constructorArgs = initialize ? new Object[] {mixedInIfAny} : _Constants.emptyObjects;
-        
-        final ProxyFactory<T> proxyFactory = ProxyFactory.builder(cls)
-        		.interfaces(interfaces)
-        		.constructorArgTypes(constructorArgTypes)
-        		.build();
-        
-        return initialize 
-        		? proxyFactory.createInstance(methodHandler, constructorArgs)  
-        		: proxyFactory.createInstance(methodHandler, false)
-        		;
-    }
-
-    /**
-     *
-     * @param target - the object that is proxied, either a domain object or a mixin around a domain object
-     * @param mixedInIfAny - if target is a mixin, then this is the domain object that is mixed-in to.
-     */
-    private <T> InvocationHandler newMethodHandler(final T target, final Object mixedInIfAny) {
-    	
-    	if(backgroundCommandService==null) {
-    		return new ForkingInvocationHandler<T>(target, mixedInIfAny, backgroundExecutorService);
-    	}
-    	
-    	return new CommandInvocationHandler<T>(
+	static final Logger LOG = LoggerFactory.getLogger(BackgroundServiceDefault.class);
+
+	/*
+	 * For the fixed thread-pool let there be 1-4 concurrent threads,
+	 * limited by the number of available (logical) processor cores.
+	 * 
+	 * Note: Future improvements might make these values configurable, 
+	 * but for now lets try to be reasonably nice here.
+	 * 
+	 */
+	private final int minThreadCount = 1; // only used if there is no BackgroundCommandService
+	private final int maxThreadCount = 4; // only used if there is no BackgroundCommandService
+
+	private final int threadCount = // only used if there is no BackgroundCommandService
+			Math.max(minThreadCount, 
+					Math.min(maxThreadCount,
+							Runtime.getRuntime().availableProcessors()));
+
+	// only used if there is no BackgroundCommandService
+	private ExecutorService backgroundExecutorService; 
+
+	@Programmatic
+	@PostConstruct
+	public void init(Map<String,String> props) {
+		if(backgroundCommandService==null) {
+			backgroundExecutorService = Executors.newFixedThreadPool(threadCount);	
+		}
+	}
+
+	@Programmatic
+	@PreDestroy
+	public void shutdown() {
+		if(backgroundExecutorService!=null) {
+			backgroundExecutorService.shutdownNow();
+			backgroundExecutorService = null;
+		}
+	}
+
+	ObjectSpecification getSpecification(final Class<?> type) {
+		return specificationLoader.loadSpecification(type);
+	}
+
+	// //////////////////////////////////////
+
+	@Programmatic
+	@Override
+	public <T> T execute(final T domainObject) {
+		final Class<T> cls = uncheckedCast(domainObject.getClass());
+		final InvocationHandler methodHandler = newMethodHandler(domainObject, null);
+		return newProxy(cls, null, methodHandler);
+	}
+
+	@Override
+	public <T> T executeMixin(Class<T> mixinClass, Object mixedIn) {
+		final T mixin = factoryService.mixin(mixinClass, mixedIn);
+		final InvocationHandler methodHandler = newMethodHandler(mixin, mixedIn);
+		return newProxy(mixinClass, mixedIn, methodHandler);
+	}
+
+	private <T> T newProxy(
+			final Class<T> cls,
+			final Object mixedInIfAny,
+			final InvocationHandler methodHandler) {
+
+		final Class<?>[] interfaces = ArrayExtensions.combine(
+				cls.getInterfaces(), 
+				new Class<?>[] { ProxyEnhanced.class }); 
+
+		final boolean initialize = mixedInIfAny!=null;
+
+
+		final Class<?>[] constructorArgTypes = initialize ? new Class<?>[] {mixedInIfAny.getClass()} : _Constants.emptyClasses;
+		final Object[] constructorArgs = initialize ? new Object[] {mixedInIfAny} : _Constants.emptyObjects;
+
+		final ProxyFactory<T> proxyFactory = ProxyFactory.builder(cls)
+				.interfaces(interfaces)
+				.constructorArgTypes(constructorArgTypes)
+				.build();
+
+		return initialize 
+				? proxyFactory.createInstance(methodHandler, constructorArgs)  
+						: proxyFactory.createInstance(methodHandler, false)
+						;
+	}
+
+	/**
+	 *
+	 * @param target - the object that is proxied, either a domain object or a mixin around a domain object
+	 * @param mixedInIfAny - if target is a mixin, then this is the domain object that is mixed-in to.
+	 */
+	private <T> InvocationHandler newMethodHandler(final T target, final Object mixedInIfAny) {
+
+		if(backgroundCommandService==null) {
+			return new ForkingInvocationHandler<T>(target, mixedInIfAny, backgroundExecutorService);
+		}
+
+		return new CommandInvocationHandler<T>(
 				(BackgroundCommandService2) backgroundCommandService, 
 				target, 
 				mixedInIfAny, 
@@ -135,32 +161,32 @@ public class BackgroundServiceDefault implements BackgroundService2 {
 				commandDtoServiceInternal,
 				commandContext,
 				this::getAdapterManager);
-    	
-    }
+
+	}
 
 
-    // //////////////////////////////////////
+	// //////////////////////////////////////
 
-    @javax.inject.Inject
-    private BackgroundCommandService backgroundCommandService;
+	@javax.inject.Inject
+	private BackgroundCommandService backgroundCommandService;
 
-    @javax.inject.Inject
-    private CommandDtoServiceInternal commandDtoServiceInternal;
+	@javax.inject.Inject
+	private CommandDtoServiceInternal commandDtoServiceInternal;
 
-    @javax.inject.Inject
-    private CommandContext commandContext;
+	@javax.inject.Inject
+	private CommandContext commandContext;
 
-    @javax.inject.Inject
-    private FactoryService factoryService;
+	@javax.inject.Inject
+	private FactoryService factoryService;
 
-    @javax.inject.Inject
-    private SpecificationLoader specificationLoader;
+	@javax.inject.Inject
+	private SpecificationLoader specificationLoader;
 
-    @javax.inject.Inject
-    private IsisSessionFactory isisSessionFactory;
+	@javax.inject.Inject
+	private IsisSessionFactory isisSessionFactory;
 
-    protected AdapterManager getAdapterManager() {
-        return isisSessionFactory.getCurrentSession().getPersistenceSession();
-    }
+	protected AdapterManager getAdapterManager() {
+		return isisSessionFactory.getCurrentSession().getPersistenceSession();
+	}
 
 }
diff --git a/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/ForkingInvocationHandler.java b/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/ForkingInvocationHandler.java
index 9ac2593..aa4bbc3 100644
--- a/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/ForkingInvocationHandler.java
+++ b/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/ForkingInvocationHandler.java
@@ -62,7 +62,20 @@ class ForkingInvocationHandler<T> implements InvocationHandler {
         }
         
         backgroundExecutorService.submit(()->{
-    		IsisContext.getSessionFactory().doInSession(()->proxyMethod.invoke(domainObject, args));
+        	
+        	try {
+        		
+        		IsisContext.getSessionFactory().doInSession(
+        			()->proxyMethod.invoke(domainObject, args));
+        		
+        	} catch (Exception e) {
+        		// log in caller's context        		
+        		BackgroundServiceDefault.LOG.error(
+        				String.format("Background execution of action '%s' on object '%s' failed.", 
+        						proxyMethod.getName(),
+        						domainObject.getClass().getName()),
+        				e);
+			}
     	}); 
 
         return null;

-- 
To stop receiving notification emails like this one, please contact
ahuber@apache.org.