You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by hl...@apache.org on 2007/03/10 19:17:17 UTC
svn commit: r516760 - in
/tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src:
main/java/org/apache/tapestry/ioc/ main/java/org/apache/tapestry/ioc/def/
main/java/org/apache/tapestry/ioc/internal/
test/java/org/apache/tapestry/ioc/...
Author: hlship
Date: Sat Mar 10 10:17:16 2007
New Revision: 516760
URL: http://svn.apache.org/viewvc?view=rev&rev=516760
Log:
TAPESTRY-1339: Simplify the construction of service decorators and access to Log instances for modules and for services within modules.
Modified:
tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/RegistryBuilder.java
tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/def/ModuleDef.java
tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/DefaultModuleDefImpl.java
tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/InterceptorStackBuilder.java
tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/InternalRegistry.java
tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/Module.java
tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/ModuleImpl.java
tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/RegistryImpl.java
tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/test/java/org/apache/tapestry/ioc/internal/ModuleImplTest.java
Modified: tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/RegistryBuilder.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/RegistryBuilder.java?view=diff&rev=516760&r1=516759&r2=516760
==============================================================================
--- tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/RegistryBuilder.java (original)
+++ tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/RegistryBuilder.java Sat Mar 10 10:17:16 2007
@@ -127,8 +127,12 @@
{
_lock.lock();
- return new RegistryWrapper(new RegistryImpl(_modules, _classLoader, _logSource,
- _serviceOverrides));
+ RegistryImpl registry = new RegistryImpl(_modules, _classLoader, _logSource,
+ _serviceOverrides);
+
+ registry.eagerLoadServices();
+
+ return new RegistryWrapper(registry);
}
public ClassLoader getClassLoader()
Modified: tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/def/ModuleDef.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/def/ModuleDef.java?view=diff&rev=516760&r1=516759&r2=516760
==============================================================================
--- tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/def/ModuleDef.java (original)
+++ tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/def/ModuleDef.java Sat Mar 10 10:17:16 2007
@@ -16,6 +16,8 @@
import java.util.Set;
+import org.apache.commons.logging.Log;
+
/**
* Defines the contents of a module. In the default case, this is information about the services
* provided by the module builder class.
@@ -50,4 +52,10 @@
* services.
*/
Class getBuilderClass();
+
+ /**
+ * Returns the name used to create a {@link Log} instance. This is typically the builder class
+ * name.
+ */
+ String getLogName();
}
Modified: tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/DefaultModuleDefImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/DefaultModuleDefImpl.java?view=diff&rev=516760&r1=516759&r2=516760
==============================================================================
--- tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/DefaultModuleDefImpl.java (original)
+++ tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/DefaultModuleDefImpl.java Sat Mar 10 10:17:16 2007
@@ -309,4 +309,10 @@
{
return _contributionDefs;
}
+
+ public String getLogName()
+ {
+ return _builderClass.getName();
+ }
+
}
Modified: tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/InterceptorStackBuilder.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/InterceptorStackBuilder.java?view=diff&rev=516760&r1=516759&r2=516760
==============================================================================
--- tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/InterceptorStackBuilder.java (original)
+++ tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/InterceptorStackBuilder.java Sat Mar 10 10:17:16 2007
@@ -1,4 +1,4 @@
-// Copyright 2006 The Apache Software Foundation
+// Copyright 2006 2007 The Apache Software Foundation
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
@@ -12,71 +12,69 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package org.apache.tapestry.ioc.internal;
-
-import java.util.Collections;
-import java.util.List;
-
-import org.apache.tapestry.ioc.ObjectCreator;
-import org.apache.tapestry.ioc.ServiceDecorator;
-
-/**
- * Responsible for constructing the interceptor stack, on demand, by invoking an ordered series of
- * decorators ({@link org.apache.tapestry.ioc.def.DecoratorDef}.
- *
- *
- */
-public class InterceptorStackBuilder implements ObjectCreator
-{
- private final String _serviceId;
-
- private final ObjectCreator _coreServiceCreator;
-
- private final Module _module;
-
- /**
- * @param module
- * the module containing the decorator method
- * @param serviceId
- * identifies the service to be decorated
- * @param coreServiceCreator
- * responsible for creating the core service which is then decorated with a stack of
- * interceptors
- */
- public InterceptorStackBuilder(Module module, String serviceId, ObjectCreator coreServiceCreator)
- {
- _module = module;
- _serviceId = serviceId;
- _coreServiceCreator = coreServiceCreator;
- }
-
- public Object createObject()
- {
- Object current = _coreServiceCreator.createObject();
-
- List<ServiceDecorator> decorators = _module.findDecoratorsForService(_serviceId);
-
- // We get the decorators ordered according to their dependencies. However, we want to
- // process from the last interceptor to the first, so we reverse the list.
-
- Collections.reverse(decorators);
-
- for (ServiceDecorator decorator : decorators)
- {
- Object interceptor = decorator.createInterceptor(current);
-
- // Decorator methods may return null; this indicates that the decorator chose not to
- // decorate.
-
- if (interceptor != null)
- current = interceptor;
- }
-
- // The stack of interceptors (plus the core service implementation) are "represented" to the
- // outside world
- // as the outermost interceptor. That will still be buried inside the service proxy.
-
- return current;
- }
-
-}
+package org.apache.tapestry.ioc.internal;
+
+import java.util.Collections;
+import java.util.List;
+
+import org.apache.tapestry.ioc.ObjectCreator;
+import org.apache.tapestry.ioc.ServiceDecorator;
+
+/**
+ * Responsible for constructing the interceptor stack, on demand, by invoking an ordered series of
+ * decorators ({@link org.apache.tapestry.ioc.def.DecoratorDef} (which are converted into
+ * {@link ServiceDecorator}s).
+ */
+public class InterceptorStackBuilder implements ObjectCreator
+{
+ private final String _serviceId;
+
+ private final ObjectCreator _coreServiceCreator;
+
+ private final Module _module;
+
+ /**
+ * @param module
+ * the module containing the decorator method
+ * @param serviceId
+ * identifies the service to be decorated
+ * @param coreServiceCreator
+ * responsible for creating the core service which is then decorated with a stack of
+ * interceptors
+ */
+ public InterceptorStackBuilder(Module module, String serviceId, ObjectCreator coreServiceCreator)
+ {
+ _module = module;
+ _serviceId = serviceId;
+ _coreServiceCreator = coreServiceCreator;
+ }
+
+ public Object createObject()
+ {
+ Object current = _coreServiceCreator.createObject();
+
+ List<ServiceDecorator> decorators = _module.findDecoratorsForService(_serviceId);
+
+ // We get the decorators ordered according to their dependencies. However, we want to
+ // process from the last interceptor to the first, so we reverse the list.
+
+ Collections.reverse(decorators);
+
+ for (ServiceDecorator decorator : decorators)
+ {
+ Object interceptor = decorator.createInterceptor(current);
+
+ // Decorator methods may return null; this indicates that the decorator chose not to
+ // decorate.
+
+ if (interceptor != null) current = interceptor;
+ }
+
+ // The stack of interceptors (plus the core service implementation) are "represented" to the
+ // outside world
+ // as the outermost interceptor. That will still be buried inside the service proxy.
+
+ return current;
+ }
+
+}
Modified: tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/InternalRegistry.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/InternalRegistry.java?view=diff&rev=516760&r1=516759&r2=516760
==============================================================================
--- tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/InternalRegistry.java (original)
+++ tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/InternalRegistry.java Sat Mar 10 10:17:16 2007
@@ -18,7 +18,7 @@
import java.util.List;
import java.util.Map;
-import org.apache.tapestry.ioc.LogSource;
+import org.apache.commons.logging.Log;
import org.apache.tapestry.ioc.ObjectProvider;
import org.apache.tapestry.ioc.Registry;
import org.apache.tapestry.ioc.ServiceDecorator;
@@ -31,7 +31,7 @@
/**
* Internal view of the module registry, adding additional methods needed by modules.
*/
-public interface InternalRegistry extends Registry, LogSource, RegistryShutdownHub
+public interface InternalRegistry extends Registry, RegistryShutdownHub
{
/**
* Locates a service given a service id and the corresponding service interface type.
@@ -158,4 +158,13 @@
* @return expanded input
*/
String expandSymbols(String input);
+
+ /**
+ * Returns a log for the service, which consists of the Module's
+ * {@link Module#getLogName() log name} suffixed with a period and the service id.
+ *
+ * @param serviceId
+ * @return the log instance for the service
+ */
+ Log logForService(String serviceId);
}
Modified: tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/Module.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/Module.java?view=diff&rev=516760&r1=516759&r2=516760
==============================================================================
--- tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/Module.java (original)
+++ tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/Module.java Sat Mar 10 10:17:16 2007
@@ -94,4 +94,12 @@
* @return the service definition or null
*/
ServiceDef getServiceDef(String serviceId);
+
+ /**
+ * Returns the name used to obtain a logger for the module. Services within the module suffix
+ * this with a period and the service id.
+ *
+ * @return module log name
+ */
+ String getLogName();
}
Modified: tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/ModuleImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/ModuleImpl.java?view=diff&rev=516760&r1=516759&r2=516760
==============================================================================
--- tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/ModuleImpl.java (original)
+++ tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/ModuleImpl.java Sat Mar 10 10:17:16 2007
@@ -200,7 +200,7 @@
{
String serviceId = def.getServiceId();
- Log log = _registry.getLog(serviceId);
+ Log log = _registry.logForService(serviceId);
if (log.isDebugEnabled()) log.debug(IOCMessages.creatingService(serviceId));
@@ -447,6 +447,11 @@
public ServiceDef getServiceDef(String serviceId)
{
return _moduleDef.getServiceDef(serviceId);
+ }
+
+ public String getLogName()
+ {
+ return _moduleDef.getLogName();
}
}
Modified: tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/RegistryImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/RegistryImpl.java?view=diff&rev=516760&r1=516759&r2=516760
==============================================================================
--- tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/RegistryImpl.java (original)
+++ tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/main/java/org/apache/tapestry/ioc/internal/RegistryImpl.java Sat Mar 10 10:17:16 2007
@@ -86,9 +86,6 @@
/** Map from service id to the Module that contains the service. */
private final Map<String, Module> _serviceIdToModule = newCaseInsensitiveMap();
- /** Map from decorator id to thge Module that contains the decorator. */
- private final Map<String, Module> _decoratorIdToModule = newCaseInsensitiveMap();
-
private final Map<String, ServiceLifecycle> _lifecycles = newCaseInsensitiveMap();
/**
@@ -161,28 +158,23 @@
_serviceIdToModule.put(serviceId, module);
}
-
- for (DecoratorDef dd : def.getDecoratorDefs())
- {
- _decoratorIdToModule.put(dd.getDecoratorId(), module);
- }
}
addBuiltin(LOG_SOURCE_SERVICE_ID, LogSource.class, _logSource);
- Log log = logForService(TapestryIOCModule.class, CLASS_FACTORY_SERVICE_ID);
+ Log log = logForBuiltinService(CLASS_FACTORY_SERVICE_ID);
_classFactory = new ClassFactoryImpl(contextClassLoader, log);
addBuiltin(CLASS_FACTORY_SERVICE_ID, ClassFactory.class, _classFactory);
- log = logForService(TapestryIOCModule.class, THREAD_CLEANUP_HUB_SERVICE_ID);
+ log = logForBuiltinService(THREAD_CLEANUP_HUB_SERVICE_ID);
_cleanupHub = new ThreadCleanupHubImpl(log);
addBuiltin(THREAD_CLEANUP_HUB_SERVICE_ID, ThreadCleanupHub.class, _cleanupHub);
- log = logForService(TapestryIOCModule.class, REGISTRY_SHUTDOWN_HUB_SERVICE_ID);
+ log = logForBuiltinService(REGISTRY_SHUTDOWN_HUB_SERVICE_ID);
_registryShutdownHub = new RegistryShutdownHubImpl(log);
@@ -192,16 +184,32 @@
_registryShutdownHub);
_lifecycles.put("singleton", new SingletonServiceLifecycle());
+ }
- // Ask all modules to eager-load any services marked with @EagerLoad
-
+ /**
+ * It's not unreasonable for an eagerly-loaded service to decide to start a thread, at which
+ * point we raise issues about improper publishing of the Registry instance from the
+ * RegistryImpl constructor. Moving eager loading of services out to its own method should
+ * ensure thread safety.
+ */
+ public void eagerLoadServices()
+ {
for (Module m : _modules)
m.eagerLoadServices();
}
- private Log logForService(Class moduleClass, String serviceId)
+ public Log logForService(String serviceId)
+ {
+ Module module = _serviceIdToModule.get(serviceId);
+
+ assert module != null;
+
+ return _logSource.getLog(module.getLogName() + "." + serviceId);
+ }
+
+ private Log logForBuiltinService(String serviceId)
{
- return _logSource.getLog(moduleClass.getName() + "." + serviceId);
+ return _logSource.getLog(TapestryIOCModule.class + "." + serviceId);
}
private <T> void addBuiltin(String serviceId, Class<T> serviceInterface, T service)
@@ -314,7 +322,7 @@
{
_lock.check();
- Log log = getLog(serviceDef.getServiceId());
+ Log log = null;
final Orderer<T> orderer = new Orderer<T>(log);
@@ -368,7 +376,8 @@
if (contributions.isEmpty()) return;
- Log log = getLog(serviceId);
+ Log log = logForService(serviceId);
+
boolean debug = log.isDebugEnabled();
ServiceLocator locator = new ServiceResourcesImpl(this, module, serviceDef, log);
@@ -393,7 +402,8 @@
if (contributions.isEmpty()) return;
- Log log = getLog(serviceId);
+ Log log = logForService(serviceId);
+
boolean debug = log.isDebugEnabled();
ServiceLocator locator = new ServiceResourcesImpl(this, module, serviceDef, log);
@@ -417,7 +427,7 @@
if (contributions.isEmpty()) return;
- Log log = getLog(serviceId);
+ Log log = logForService(serviceId);
boolean debug = log.isDebugEnabled();
ServiceLocator locator = new ServiceResourcesImpl(this, module, serviceDef, log);
@@ -489,73 +499,29 @@
{
_lock.check();
- // This simply should never happen, yet we're seeing it. This falls into the
- // category of "scary things that happen in multi-threaded applications".
-
assert serviceDef != null;
- Log log = getLog(serviceDef.getServiceId());
-
- Orderer<DecoratorDef> orderer = new Orderer<DecoratorDef>(log);
-
- addDecoratorDefsToOrderer(orderer, serviceDef);
+ Log log = logForService(serviceDef.getServiceId());
- List<DecoratorDef> ordered = orderer.getOrdered();
+ Orderer<ServiceDecorator> orderer = new Orderer<ServiceDecorator>(log);
- return convertDecoratorDefsToServiceDecorators(ordered, serviceDef, log);
- }
-
- private List<ServiceDecorator> convertDecoratorDefsToServiceDecorators(
- List<DecoratorDef> ordered, ServiceDef serviceDef, Log log)
- {
- List<ServiceDecorator> result = newList();
-
- for (DecoratorDef dd : ordered)
+ for (Module module : _modules)
{
- Module module = _decoratorIdToModule.get(dd.getDecoratorId());
+ Set<DecoratorDef> decorators = module.findMatchingDecoratorDefs(serviceDef);
- // TODO: If null sanity check? Not really needed.
+ if (decorators.isEmpty()) continue;
ServiceResources resources = new ServiceResourcesImpl(this, module, serviceDef, log);
- ServiceDecorator decorator = dd.createDecorator(module, resources);
-
- result.add(decorator);
- }
-
- return result;
- }
-
- private void addDecoratorDefsToOrderer(Orderer<DecoratorDef> orderer, ServiceDef serviceDef)
- {
-
- for (Module m : _modules)
- {
- Set<DecoratorDef> moduleDecorators = m.findMatchingDecoratorDefs(serviceDef);
- addToOrderer(orderer, moduleDecorators);
- }
- }
+ for (DecoratorDef dd : decorators)
+ {
+ ServiceDecorator sd = dd.createDecorator(module, resources);
- private void addToOrderer(Orderer<DecoratorDef> orderer, Set<DecoratorDef> decorators)
- {
- for (DecoratorDef df : decorators)
- {
- orderer.add(df.getDecoratorId(), df, df.getConstraints());
+ orderer.add(dd.getDecoratorId(), sd, dd.getConstraints());
+ }
}
- }
-
- public Log getLog(Class clazz)
- {
- _lock.check();
-
- return _logSource.getLog(clazz);
- }
- public Log getLog(String name)
- {
- _lock.check();
-
- return _logSource.getLog(name);
+ return orderer.getOrdered();
}
public ClassFab newClass(Class serviceInterface)
@@ -593,6 +559,8 @@
public String expandSymbols(String input)
{
+ _lock.check();
+
// Again, a bit of work to avoid instantiating the SymbolSource until absolutely necessary.
if (!InternalUtils.containsSymbols(input)) return input;
Modified: tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/test/java/org/apache/tapestry/ioc/internal/ModuleImplTest.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/test/java/org/apache/tapestry/ioc/internal/ModuleImplTest.java?view=diff&rev=516760&r1=516759&r2=516760
==============================================================================
--- tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/test/java/org/apache/tapestry/ioc/internal/ModuleImplTest.java (original)
+++ tapestry/tapestry5/tapestry-ioc/branches/hlship-20070309-simplifyioc/src/test/java/org/apache/tapestry/ioc/internal/ModuleImplTest.java Sat Mar 10 10:17:16 2007
@@ -45,7 +45,7 @@
Module module = new ModuleImpl(registry, moduleDef, log);
- train_getLog(registry, "Upcase", log);
+ expect(registry.logForService("Upcase")).andReturn(log);
train_isDebugEnabled(log, true);
log.debug("Creating service 'Upcase'.");