Replaced problematic CacheAttribute interlocked initialization (#46)

ybqwerty · Yagel · web-flow · commit 4b275509dc25 · 2021-01-19T01:32:08.000+02:00
Also increased PostSharp version to support net5

Co-authored-by: Yagel &lt;yagelba@payoneer.com&gt;
diff --git a/AopCaching.UnitTests/AopCacheTests.cs b/AopCaching.UnitTests/AopCacheTests.cs
@@ -51,6 +51,26 @@ public void TestCacheWithImplicitName()
             Assert.AreEqual(2, cache1.Misses);
         }
 
+        [TestMethod]
+        public void TestMissingNameCache()
+        {
+            var service = new Service2();
+            var result = service.MethodToCache0();
+            Assert.AreEqual(1, result);
+            result = service.MethodToCache0();
+            Assert.AreEqual(2, result);
+        }
+
+        [TestMethod]
+        public async Task TestMissingNameCacheAsync()
+        {
+            var service = new Service2();
+            var result = await service.MethodToCache0Async();
+            Assert.AreEqual(1, result);
+            result = await service.MethodToCache0Async();
+            Assert.AreEqual(2, result);
+        }
+
         [TestMethod]
         public void TestNamedCache1()
         {
diff --git a/AopCaching.UnitTests/AopCaching.UnitTests.csproj b/AopCaching.UnitTests/AopCaching.UnitTests.csproj
@@ -7,7 +7,7 @@
   <ItemGroup>
     <PackageReference Include="MSTest.TestAdapter" Version="1.2.0" />
     <PackageReference Include="MSTest.TestFramework" Version="1.2.0" />
-    <PackageReference Include="PostSharp" Version="6.0.28" />
+    <PackageReference Include="PostSharp" Version="6.8.7" />
     <PackageReference Include="System.Runtime.Caching" Version="4.5.0" />
   </ItemGroup>
   <ItemGroup>
diff --git a/AopCaching.UnitTests/Mocks/Service2.cs b/AopCaching.UnitTests/Mocks/Service2.cs
@@ -6,6 +6,21 @@ namespace PubComp.Caching.AopCaching.UnitTests.Mocks
 {
     public class Service2
     {
+        private int methodToCache0Counter;
+
+        [Cache("CacheMissing")]
+        public int MethodToCache0()
+        {
+            return ++methodToCache0Counter;
+        }
+
+        [Cache("CacheMissing")]
+        public async Task<int> MethodToCache0Async()
+        {
+            await Task.Delay(10);
+            return ++methodToCache0Counter;
+        }
+
         [Cache("localCache")]
         public IEnumerable<string> MethodToCache1()
         {
diff --git a/AopCaching/AopCaching.csproj b/AopCaching/AopCaching.csproj
@@ -5,9 +5,9 @@
     <AssemblyName>PubComp.Caching.AopCaching</AssemblyName>
     <RootNamespace>PubComp.Caching.AopCaching</RootNamespace>
     <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
-    <Version>5.0.0</Version>
+    <Version>5.0.1</Version>
     <AssemblyVersion>5.0.0.0</AssemblyVersion>
-    <FileVersion>5.0.0.0</FileVersion>
+    <FileVersion>5.0.1.0</FileVersion>
   </PropertyGroup>
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'">
     <NoWarn>1701;1702;1591</NoWarn>
@@ -16,7 +16,8 @@
     <NoWarn>1701;1702;1591</NoWarn>
   </PropertyGroup>
   <ItemGroup>
-    <PackageReference Include="PostSharp" Version="6.0.28" />
+    <PackageReference Include="NLog" Version="4.5.10" />
+    <PackageReference Include="PostSharp" Version="6.8.7" />
   </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="..\Core\Core.csproj" />
diff --git a/AopCaching/CacheAttribute.cs b/AopCaching/CacheAttribute.cs
@@ -1,4 +1,5 @@
-﻿using PostSharp.Aspects;
+﻿using NLog;
+using PostSharp.Aspects;
 using PubComp.Caching.Core;
 using PubComp.Caching.Core.Attributes;
 using System;
@@ -15,7 +16,6 @@ public class CacheAttribute : MethodInterceptionAspect
     {
         private string cacheName;
         private ICache cache;
-        private long initialized = 0L;
         private string className;
         private string methodName;
         private string[] parameterTypeNames;
@@ -32,7 +32,7 @@ public CacheAttribute(string cacheName)
             this.cacheName = cacheName;
         }
 
-        public sealed override void CompileTimeInitialize(System.Reflection.MethodBase method, AspectInfo aspectInfo)
+        public sealed override void CompileTimeInitialize(MethodBase method, AspectInfo aspectInfo)
         {
             var type = method.DeclaringType;
 
@@ -64,10 +64,13 @@ public sealed override void CompileTimeInitialize(System.Reflection.MethodBase m
 
         public sealed override void OnInvoke(MethodInterceptionArgs args)
         {
-            if (Interlocked.Read(ref initialized) == 0L)
+            if (this.cache == null)
             {
                 this.cache = CacheManager.GetCache(this.cacheName);
-                Interlocked.Exchange(ref initialized, 1L);
+                if (this.cache == null)
+                {
+                    LogManager.GetCurrentClassLogger().Warn($"AOP cache [{this.cacheName}] is not initialized, define NoCache if needed!");
+                }
             }
 
             var cacheToUse = this.cache;
@@ -86,10 +89,13 @@ public sealed override void OnInvoke(MethodInterceptionArgs args)
         /// <inheritdoc />
         public sealed override async Task OnInvokeAsync(MethodInterceptionArgs args)
         {
-            if (Interlocked.Read(ref initialized) == 0L)
+            if (this.cache == null)
             {
                 this.cache = CacheManager.GetCache(this.cacheName);
-                Interlocked.Exchange(ref initialized, 1L);
+                if (this.cache == null)
+                {
+                    LogManager.GetCurrentClassLogger().Warn($"AOP cache [{this.cacheName}] is not initialized, define NoCache if needed!");
+                }
             }
 
             var cacheToUse = this.cache;
diff --git a/Core.UnitTests/Core.UnitTests.csproj b/Core.UnitTests/Core.UnitTests.csproj
@@ -16,7 +16,7 @@
     <PackageReference Include="MSTest.TestFramework" Version="1.2.0" />
     <PackageReference Include="Newtonsoft.Json" Version="11.0.2" />
     <PackageReference Include="NLog" Version="4.5.10" />
-    <PackageReference Include="PostSharp" Version="6.0.28" />
+    <PackageReference Include="PostSharp" Version="6.8.7" />
     <PackageReference Include="System.Runtime.Caching" Version="4.5.0" />
   </ItemGroup>
   <ItemGroup>
diff --git a/RedisCaching.UnitTests/RedisCaching.UnitTests.csproj b/RedisCaching.UnitTests/RedisCaching.UnitTests.csproj
@@ -7,7 +7,7 @@
   <ItemGroup>
     <PackageReference Include="MSTest.TestAdapter" Version="1.2.0" />
     <PackageReference Include="MSTest.TestFramework" Version="1.2.0" />
-    <PackageReference Include="PostSharp" Version="6.0.28" />
+    <PackageReference Include="PostSharp" Version="6.8.7" />
   </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="..\AopCaching\AopCaching.csproj" />

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`		`-using PostSharp.Aspects;`
	`1`	`+using NLog;`
	`2`	`+using PostSharp.Aspects;`
`2`	`3`	`using PubComp.Caching.Core;`
`3`	`4`	`using PubComp.Caching.Core.Attributes;`
`4`	`5`	`using System;`
`@@ -15,7 +16,6 @@ public class CacheAttribute : MethodInterceptionAspect`
`15`	`16`	`{`
`16`	`17`	`private string cacheName;`
`17`	`18`	`private ICache cache;`
`18`		`- private long initialized = 0L;`
`19`	`19`	`private string className;`
`20`	`20`	`private string methodName;`
`21`	`21`	`private string[] parameterTypeNames;`
`@@ -32,7 +32,7 @@ public CacheAttribute(string cacheName)`
`32`	`32`	`this.cacheName = cacheName;`
`33`	`33`	`}`
`34`	`34`
`35`		`- public sealed override void CompileTimeInitialize(System.Reflection.MethodBase method, AspectInfo aspectInfo)`
	`35`	`+ public sealed override void CompileTimeInitialize(MethodBase method, AspectInfo aspectInfo)`
`36`	`36`	`{`
`37`	`37`	`var type = method.DeclaringType;`
`38`	`38`
`@@ -64,10 +64,13 @@ public sealed override void CompileTimeInitialize(System.Reflection.MethodBase m`
`64`	`64`
`65`	`65`	`public sealed override void OnInvoke(MethodInterceptionArgs args)`
`66`	`66`	`{`
`67`		`- if (Interlocked.Read(ref initialized) == 0L)`
	`67`	`+ if (this.cache == null)`
`68`	`68`	`{`
`69`	`69`	`this.cache = CacheManager.GetCache(this.cacheName);`
`70`		`- Interlocked.Exchange(ref initialized, 1L);`
	`70`	`+ if (this.cache == null)`
	`71`	`+ {`
	`72`	`+ LogManager.GetCurrentClassLogger().Warn($"AOP cache [{this.cacheName}] is not initialized, define NoCache if needed!");`
	`73`	`+ }`
`71`	`74`	`}`
`72`	`75`
`73`	`76`	`var cacheToUse = this.cache;`
`@@ -86,10 +89,13 @@ public sealed override void OnInvoke(MethodInterceptionArgs args)`
`86`	`89`	`/// <inheritdoc />`
`87`	`90`	`public sealed override async Task OnInvokeAsync(MethodInterceptionArgs args)`
`88`	`91`	`{`
`89`		`- if (Interlocked.Read(ref initialized) == 0L)`
	`92`	`+ if (this.cache == null)`
`90`	`93`	`{`
`91`	`94`	`this.cache = CacheManager.GetCache(this.cacheName);`
`92`		`- Interlocked.Exchange(ref initialized, 1L);`
	`95`	`+ if (this.cache == null)`
	`96`	`+ {`
	`97`	`+ LogManager.GetCurrentClassLogger().Warn($"AOP cache [{this.cacheName}] is not initialized, define NoCache if needed!");`
	`98`	`+ }`
`93`	`99`	`}`
`94`	`100`
`95`	`101`	`var cacheToUse = this.cache;`