diff --git a/src/Spring/Spring.Aop/Aspects/Cache/BaseCacheAdvice.cs b/src/Spring/Spring.Aop/Aspects/Cache/BaseCacheAdvice.cs index 6985b83d..56ce9df9 100644 --- a/src/Spring/Spring.Aop/Aspects/Cache/BaseCacheAdvice.cs +++ b/src/Spring/Spring.Aop/Aspects/Cache/BaseCacheAdvice.cs @@ -71,9 +71,24 @@ namespace Spring.Aspects.Cache /// Cache instance for the specified if one /// is registered in the application context, or null if it isn't. /// + /// + /// If there's no cache instance registered for the specified . + /// + /// + /// If the cache instance could not be created. + /// + /// + /// If the cache instance registered does not implement the interface. + /// public ICache GetCache(string name) { - return applicationContext.GetObject(name) as ICache; + ICache cache = applicationContext.GetObject(name) as ICache; + if (cache == null) + { + throw new ArgumentException(String.Format( + "Cache with the specified name [{0}] does not implement the 'Spring.Caching.ICache' interface.", name)); + } + return cache; } /// diff --git a/src/Spring/Spring.Aop/Aspects/Cache/CacheParameterAdvice.cs b/src/Spring/Spring.Aop/Aspects/Cache/CacheParameterAdvice.cs index 10a46b8b..713b345c 100644 --- a/src/Spring/Spring.Aop/Aspects/Cache/CacheParameterAdvice.cs +++ b/src/Spring/Spring.Aop/Aspects/Cache/CacheParameterAdvice.cs @@ -52,8 +52,10 @@ namespace Spring.Aspects.Cache /// /// /// Aleksandar Seovic - public class CacheParameterAdvice : BaseCacheAdvice, IAfterReturningAdvice - { + public class CacheParameterAdvice : BaseCacheAdvice, IAfterReturningAdvice + { + #region CacheParameterAttribute caching + private class CacheParameterInfo { public readonly ParameterInfo[] Parameters; @@ -66,7 +68,28 @@ namespace Spring.Aspects.Cache } } - private readonly Hashtable _cacheParameterInfoCache = new Hashtable(); + private readonly Hashtable _cacheParameterInfoCache = new Hashtable(); + + private CacheParameterInfo GetCacheParameterInfo(MethodInfo method) + { + CacheParameterInfo cpi = (CacheParameterInfo)_cacheParameterInfoCache[method]; + if (cpi == null) + { + ParameterInfo[] parameters = method.GetParameters(); + CacheParameterAttribute[][] parameterInfos = new CacheParameterAttribute[parameters.Length][]; + for (int i = 0; i < parameters.Length; i++) + { + ParameterInfo p = parameters[i]; + CacheParameterAttribute[] paramInfoArray = (CacheParameterAttribute[])GetCustomAttributes(p, typeof(CacheParameterAttribute)); + parameterInfos[i] = paramInfoArray; + } + cpi = new CacheParameterInfo(parameters, parameterInfos); + _cacheParameterInfoCache[method] = cpi; + } + return cpi; + } + + #endregion /// /// Executes after target @@ -89,9 +112,11 @@ namespace Spring.Aspects.Cache /// The intercepted method's arguments. /// The target object. /// - public void AfterReturning(object returnValue, MethodInfo method, object[] arguments, object target) - { + public void AfterReturning(object returnValue, MethodInfo method, object[] arguments, object target) + { + #region Instrumentation bool isLogDebugEnabled = logger.IsDebugEnabled; + #endregion CacheParameterInfo cpi = GetCacheParameterInfo(method); CacheParameterAttribute[][] cacheParameterAttributes = cpi.CacheParameterAttributes; @@ -103,16 +128,13 @@ namespace Spring.Aspects.Cache if (EvalCondition(paramInfo.Condition, paramInfo.ConditionExpression, arguments[i], null)) { ICache cache = GetCache(paramInfo.CacheName); - if (cache == null) - { - throw new ArgumentNullException("CacheName", string.Format("Parameter cache with the specified name [{0}] does not exist.", paramInfo.CacheName)); - } + object key = paramInfo.KeyExpression.GetValue(arguments[i]); #region Instrumentation if (isLogDebugEnabled) { - logger.Debug(string.Format("Caching parameter for key [{0}].", key)); + logger.Debug(string.Format("Caching parameter for key [{0}] into cache [{1}].", key, paramInfo.CacheName)); } #endregion @@ -121,24 +143,5 @@ namespace Spring.Aspects.Cache } } } - - private CacheParameterInfo GetCacheParameterInfo(MethodInfo method) - { - CacheParameterInfo cpi = (CacheParameterInfo) _cacheParameterInfoCache[method]; - if (cpi == null) - { - ParameterInfo[] parameters = method.GetParameters(); - CacheParameterAttribute[][] parameterInfos = new CacheParameterAttribute[parameters.Length][]; - for (int i = 0; i < parameters.Length; i++) - { - ParameterInfo p = parameters[i]; - CacheParameterAttribute[] paramInfoArray = (CacheParameterAttribute[])GetCustomAttributes(p, typeof(CacheParameterAttribute)); - parameterInfos[i] = paramInfoArray; - } - cpi = new CacheParameterInfo(parameters, parameterInfos); - _cacheParameterInfoCache[method] = cpi; - } - return cpi; - } } } \ No newline at end of file diff --git a/src/Spring/Spring.Aop/Aspects/Cache/CacheResultAdvice.cs b/src/Spring/Spring.Aop/Aspects/Cache/CacheResultAdvice.cs index 2e3aca93..f7cb963f 100644 --- a/src/Spring/Spring.Aop/Aspects/Cache/CacheResultAdvice.cs +++ b/src/Spring/Spring.Aop/Aspects/Cache/CacheResultAdvice.cs @@ -27,6 +27,7 @@ using Common.Logging; using Spring.Caching; using Spring.Expressions; using Spring.Util; +using System; #endregion @@ -50,8 +51,7 @@ namespace Spring.Aspects.Cache /// Aleksandar Seovic public class CacheResultAdvice : BaseCacheAdvice, IMethodInterceptor { - // NullValue - private static readonly object NullValue = new object(); + #region CacheResultAttribute & CacheResultItemsAttribute caching private class CacheResultInfo { @@ -67,6 +67,26 @@ namespace Spring.Aspects.Cache private readonly Hashtable _cacheResultAttributeCache = new Hashtable(); + private CacheResultInfo GetCacheResultInfo(MethodInfo method) + { + CacheResultInfo cacheResultInfo = (CacheResultInfo)_cacheResultAttributeCache[method]; + // no need for locking here - last one wins + if (cacheResultInfo == null) + { + CacheResultAttribute resultInfo = (CacheResultAttribute)GetCustomAttribute(method, typeof(CacheResultAttribute)); + CacheResultItemsAttribute[] itemInfoArray = (CacheResultItemsAttribute[])GetCustomAttributes(method, typeof(CacheResultItemsAttribute)); + + cacheResultInfo = new CacheResultInfo(resultInfo, itemInfoArray); + _cacheResultAttributeCache[method] = cacheResultInfo; + } + return cacheResultInfo; + } + + #endregion + + // NullValue + private static readonly object NullValue = new object(); + /// /// Applies caching around a method invocation. /// @@ -131,8 +151,11 @@ namespace Spring.Aspects.Cache { if (resultInfo != null) { - object returnValue = null; + #region Instrumentation bool isLogDebugEnabled = logger.IsDebugEnabled; + #endregion + + object returnValue = null; IDictionary vars = PrepareVariables(invocation.Method, invocation.Arguments); @@ -140,33 +163,26 @@ namespace Spring.Aspects.Cache "The cache attribute is missing the key definition."); object resultKey = resultInfo.KeyExpression.GetValue(null, vars); ICache cache = GetCache(resultInfo.CacheName); - AssertUtils.ArgumentNotNull(cache, "CacheName", - "Result cache with the specified name [" + resultInfo.CacheName + - "] does not exist."); returnValue = cache.Get(resultKey); cacheHit = (returnValue != null); if (!cacheHit) { #region Instrumentation - if (isLogDebugEnabled) { - logger.Debug("Object for key [" + resultKey + "] was not found in cache. Proceeding..."); + logger.Debug(String.Format("Object for key [{0}] was not found in cache [{1}]. Proceeding...", resultKey, resultInfo.CacheName)); } - #endregion returnValue = invocation.Proceed(); if (EvalCondition(resultInfo.Condition, resultInfo.ConditionExpression, returnValue, vars)) { #region Instrumentation - if (isLogDebugEnabled) { - logger.Debug("Caching object for key [" + resultKey + "]."); + logger.Debug(String.Format("Caching object for key [{0}] into cache [{1}].", resultKey, resultInfo.CacheName)); } - #endregion cache.Insert(resultKey, (returnValue == null) ? NullValue : returnValue, resultInfo.TimeToLiveTimeSpan); } @@ -174,12 +190,10 @@ namespace Spring.Aspects.Cache else { #region Instrumentation - if (isLogDebugEnabled) { - logger.Debug("Cache hit for [" + resultKey + "]. Aborting invocation..."); + logger.Debug(String.Format("Object for key [{0}] found in cache [{1}]. Aborting invocation...", resultKey, resultInfo.CacheName)); } - #endregion } @@ -203,47 +217,29 @@ namespace Spring.Aspects.Cache { foreach (CacheResultItemsAttribute itemInfo in itemInfoArray) { - ICache cache = GetCache(itemInfo.CacheName); - AssertUtils.ArgumentNotNull(cache, "CacheName", - "Result item cache with the specified name [" + itemInfo.CacheName + - "] does not exist."); - AssertUtils.ArgumentNotNull(itemInfo.KeyExpression, "KeyExpression", "The cache attribute is missing the key definition."); + ICache cache = GetCache(itemInfo.CacheName); + + #region Instrumentation bool isDebugEnabled = logger.IsDebugEnabled; + #endregion foreach (object item in items) { if (EvalCondition(itemInfo.Condition, itemInfo.ConditionExpression, item, null)) { object itemKey = itemInfo.KeyExpression.GetValue(item); #region Instrumentation - if (isDebugEnabled) { logger.Debug("Caching collection item for key [" + itemKey + "]."); } - #endregion cache.Insert(itemKey, (item == null ? NullValue : item), itemInfo.TimeToLiveTimeSpan); } } } } - - private CacheResultInfo GetCacheResultInfo(MethodInfo method) - { - CacheResultInfo cacheResultInfo = (CacheResultInfo)_cacheResultAttributeCache[method]; - // no need for locking here - last one wins - if (cacheResultInfo == null) - { - CacheResultAttribute resultInfo = (CacheResultAttribute)GetCustomAttribute(method, typeof(CacheResultAttribute)); - CacheResultItemsAttribute[] itemInfoArray = (CacheResultItemsAttribute[])GetCustomAttributes(method, typeof(CacheResultItemsAttribute)); - - cacheResultInfo = new CacheResultInfo(resultInfo, itemInfoArray); - _cacheResultAttributeCache[method] = cacheResultInfo; - } - return cacheResultInfo; - } } } \ No newline at end of file diff --git a/src/Spring/Spring.Aop/Aspects/Cache/InvalidateCacheAdvice.cs b/src/Spring/Spring.Aop/Aspects/Cache/InvalidateCacheAdvice.cs index f6c708ea..5f61a404 100644 --- a/src/Spring/Spring.Aop/Aspects/Cache/InvalidateCacheAdvice.cs +++ b/src/Spring/Spring.Aop/Aspects/Cache/InvalidateCacheAdvice.cs @@ -53,10 +53,25 @@ namespace Spring.Aspects.Cache /// /// /// Aleksandar Seovic - public class InvalidateCacheAdvice : BaseCacheAdvice, IAfterReturningAdvice - { - private readonly Hashtable _invalidateCacheAttributeCache = new Hashtable(); - + public class InvalidateCacheAdvice : BaseCacheAdvice, IAfterReturningAdvice + { + #region InvalidateCacheAttribute caching + + private readonly Hashtable _invalidateCacheAttributeCache = new Hashtable(); + + private InvalidateCacheAttribute[] GetInvalidateCacheInfo(MethodInfo method) + { + InvalidateCacheAttribute[] cacheInfoArray = (InvalidateCacheAttribute[])_invalidateCacheAttributeCache[method]; + if (cacheInfoArray == null) + { + cacheInfoArray = (InvalidateCacheAttribute[])GetCustomAttributes(method, typeof(InvalidateCacheAttribute)); + _invalidateCacheAttributeCache[method] = cacheInfoArray; + } + return cacheInfoArray; + } + + #endregion + /// /// Executes after /// returns successfully. @@ -79,7 +94,11 @@ namespace Spring.Aspects.Cache /// The target object. /// public void AfterReturning(object returnValue, MethodInfo method, object[] arguments, object target) - { + { + #region Instrumentation + bool isLogDebugEnabled = logger.IsDebugEnabled; + #endregion + InvalidateCacheAttribute[] cacheInfoArray = GetInvalidateCacheInfo(method); if (cacheInfoArray.Length > 0) @@ -90,40 +109,44 @@ namespace Spring.Aspects.Cache if (EvalCondition(cacheInfo.Condition, cacheInfo.ConditionExpression, returnValue, vars)) { ICache cache = GetCache(cacheInfo.CacheName); - AssertUtils.ArgumentNotNull(cache, "CacheName", - "Cache with the specified name [" + cacheInfo.CacheName + - "] does not exist."); if (cacheInfo.KeysExpression != null) - { + { object keys = cacheInfo.KeysExpression.GetValue(returnValue, vars); if (keys is ICollection) - { + { + #region Instrumentation + if (isLogDebugEnabled) + { + logger.Debug(string.Format("Removing objects for keys [{0}] from cache [{1}].", keys, cacheInfo.CacheName)); + } + #endregion cache.RemoveAll(keys as ICollection); } else - { + { + #region Instrumentation + if (isLogDebugEnabled) + { + logger.Debug(string.Format("Removing object for key [{0}] from cache [{1}].", keys, cacheInfo.CacheName)); + } + #endregion cache.Remove(keys); } } else - { + { + #region Instrumentation + if (isLogDebugEnabled) + { + logger.Debug(string.Format("Invalidate cache [{0}].", cacheInfo.CacheName)); + } + #endregion cache.Clear(); } } } } - } - - private InvalidateCacheAttribute[] GetInvalidateCacheInfo(MethodInfo method) - { - InvalidateCacheAttribute[] cacheInfoArray = (InvalidateCacheAttribute[]) _invalidateCacheAttributeCache[method]; - if (cacheInfoArray == null) - { - cacheInfoArray = (InvalidateCacheAttribute[])GetCustomAttributes(method, typeof(InvalidateCacheAttribute)); - _invalidateCacheAttributeCache[method] = cacheInfoArray; - } - return cacheInfoArray; } } } \ No newline at end of file diff --git a/test/Spring/Spring.Aop.Tests/Aspects/Cache/CacheAspectIntegrationTests.cs b/test/Spring/Spring.Aop.Tests/Aspects/Cache/CacheAspectIntegrationTests.cs index 54785125..188751fd 100644 --- a/test/Spring/Spring.Aop.Tests/Aspects/Cache/CacheAspectIntegrationTests.cs +++ b/test/Spring/Spring.Aop.Tests/Aspects/Cache/CacheAspectIntegrationTests.cs @@ -28,6 +28,7 @@ using Spring.Aop.Framework; using Spring.Caching; using Spring.Context; using Spring.Context.Support; +using Spring.Objects.Factory; #endregion @@ -86,7 +87,7 @@ namespace Spring.Aspects.Cache /// http://jira.springframework.org/browse/SPRNET-1226 /// [Test] - [ExpectedException(ExpectedException = typeof(ArgumentNullException))] + [ExpectedException(typeof(ArgumentNullException))] public void NoCacheKeySpecified() { ICache cache = new NonExpiringCache(); @@ -100,6 +101,33 @@ namespace Spring.Aspects.Cache Assert.IsNotNull(items); } + [Test] + [ExpectedException(typeof(NoSuchObjectDefinitionException))] + public void CacheDoesNotExist() + { + //ICache cache = new NonExpiringCache(); + //context.ObjectFactory.RegisterSingleton("losers", cache); + + ProxyFactory pf = new ProxyFactory(new InventorStore()); + pf.AddAdvisors(cacheAspect); + + IInventorStore store = (IInventorStore)pf.GetProxy(); + IList items = store.GetAll(); + } + + [Test] + [ExpectedException(typeof(ArgumentException))] + public void CacheDoesNotImplementICache() + { + context.ObjectFactory.RegisterSingleton("inventors", new Object()); + + ProxyFactory pf = new ProxyFactory(new InventorStore()); + pf.AddAdvisors(cacheAspect); + + IInventorStore store = (IInventorStore)pf.GetProxy(); + IList items = store.GetAll(); + } + #if NET_2_0 [Test(Description = "http://jira.springframework.org/browse/SPRNET-959")] public void UseMethodInfoForKeyGeneration()