Close IModel in RabbitMQHealthCheck (#867)

Xabaril · Feb 18, 2022 · d167815 · d167815
1 parent 0f29ea6
commit d167815
Show file tree

Hide file tree

Showing 18 changed files with 67 additions and 28 deletions.
diff --git a/.github/workflows/healthchecks_ibmmq_cd.yml b/.github/workflows/healthchecks_ibmmq_cd.yml
@@ -10,7 +10,7 @@ jobs:
       BUILD_CONFIG: Release
     runs-on: ubuntu-latest
     services:
-      consul:
+      ibmmq:
         image: ibmcom/mq
         ports:
           - 1414:1414

diff --git a/.github/workflows/healthchecks_ibmmq_cd_preview.yml b/.github/workflows/healthchecks_ibmmq_cd_preview.yml
@@ -11,7 +11,7 @@ jobs:
       VERSION_SUFFIX: -rc2.${{ github.run_number }}
     runs-on: ubuntu-latest
     services:
-      consul:
+      ibmmq:
         image: ibmcom/mq
         ports:
           - 1414:1414

diff --git a/.github/workflows/healthchecks_ibmmq_ci.yml b/.github/workflows/healthchecks_ibmmq_ci.yml
@@ -20,7 +20,7 @@ jobs:
   build:
     runs-on: ubuntu-latest
     services:
-      consul:
+      ibmmq:
         image: ibmcom/mq
         ports:
           - 1414:1414

diff --git a/.github/workflows/healthchecks_oracle_cd.yml b/.github/workflows/healthchecks_oracle_cd.yml
@@ -11,7 +11,7 @@ jobs:
       BUILD_CONFIG: Release
     runs-on: ubuntu-latest
     services:
-      sqlserver:
+      oracle:
         image: oracleinanutshell/oracle-xe-11g
         ports:
           - 1521:1521

diff --git a/.github/workflows/healthchecks_oracle_cd_preview.yml b/.github/workflows/healthchecks_oracle_cd_preview.yml
@@ -12,7 +12,7 @@ jobs:
       VERSION_SUFFIX: -rc2.${{ github.run_number }}
     runs-on: ubuntu-latest
     services:
-      sqlserver:
+      oracle:
         image: oracleinanutshell/oracle-xe-11g
         ports:
           - 1521:1521

diff --git a/.github/workflows/healthchecks_oracle_ci.yml b/.github/workflows/healthchecks_oracle_ci.yml
@@ -20,7 +20,7 @@ jobs:
   build:
     runs-on: ubuntu-latest
     services:
-      sqlserver:
+      oracle:
         image: oracleinanutshell/oracle-xe-11g
         ports:
           - 1521:1521

diff --git a/.github/workflows/healthchecks_rabbitmq_cd.yml b/.github/workflows/healthchecks_rabbitmq_cd.yml
@@ -2,7 +2,7 @@ name: HealthChecks RabbitMQ CD
 
 on:
   push:
-    tags:       
+    tags:
       - release-rabbitmq-*
 
 jobs:
@@ -11,7 +11,7 @@ jobs:
       BUILD_CONFIG: Release
     runs-on: ubuntu-latest
     services:
-      sqlserver:
+      rabbitmq:
         image: rabbitmq
         ports:
           - 5672:5672
@@ -28,7 +28,7 @@ jobs:
       run: dotnet build --no-restore ./src/HealthChecks.Rabbitmq/HealthChecks.Rabbitmq.csproj
     - name: Test
       run: dotnet test ./test/HealthChecks.RabbitMQ.Tests/HealthChecks.RabbitMQ.Tests.csproj --verbosity normal
-    - name: dotnet pack 
+    - name: dotnet pack
       run: dotnet pack ./src/HealthChecks.Rabbitmq/HealthChecks.Rabbitmq.csproj -c $BUILD_CONFIG --include-source --include-symbols -o ./artifacts
     - name: setup nuget
       uses: NuGet/setup-nuget@v1.0.2

diff --git a/.github/workflows/healthchecks_rabbitmq_cd_preview.yml b/.github/workflows/healthchecks_rabbitmq_cd_preview.yml
@@ -2,7 +2,7 @@ name: HealthChecks RabbitMQ Preview CD
 
 on:
   push:
-    tags:       
+    tags:
       - preview-rabbitmq-*
 
 jobs:
@@ -12,7 +12,7 @@ jobs:
       VERSION_SUFFIX: -rc2.${{ github.run_number }}
     runs-on: ubuntu-latest
     services:
-      sqlserver:
+      rabbitmq:
         image: rabbitmq
         ports:
           - 5672:5672
@@ -29,7 +29,7 @@ jobs:
       run: dotnet build --no-restore ./src/HealthChecks.Rabbitmq/HealthChecks.Rabbitmq.csproj
     - name: Test
       run: dotnet test ./test/HealthChecks.RabbitMQ.Tests/HealthChecks.RabbitMQ.Tests.csproj --verbosity normal
-    - name: dotnet pack 
+    - name: dotnet pack
       run: dotnet pack ./src/HealthChecks.Rabbitmq/HealthChecks.Rabbitmq.csproj --version-suffix $VERSION_SUFFIX -c $BUILD_CONFIG --include-source --include-symbols -o ./artifacts
     - name: setup nuget
       uses: NuGet/setup-nuget@v1.0.2

diff --git a/.github/workflows/healthchecks_rabbitmq_ci.yml b/.github/workflows/healthchecks_rabbitmq_ci.yml
@@ -20,7 +20,7 @@ jobs:
   build:
     runs-on: ubuntu-latest
     services:
-      sqlserver:
+      rabbitmq:
         image: rabbitmq
         ports:
           - 5672:5672

diff --git a/.github/workflows/healthchecks_redis_cd.yml b/.github/workflows/healthchecks_redis_cd.yml
@@ -10,7 +10,7 @@ jobs:
       BUILD_CONFIG: Release
     runs-on: ubuntu-latest
     services:
-      sqlserver:
+      redis:
         image: redis:alpine
         ports:
           - 6379:6379

diff --git a/.github/workflows/healthchecks_redis_ci.yml b/.github/workflows/healthchecks_redis_ci.yml
@@ -20,7 +20,7 @@ jobs:
   build:
     runs-on: ubuntu-latest
     services:
-      sqlserver:
+      redis:
         image: redis:alpine
         ports:
           - 6379:6379

diff --git a/.github/workflows/healthchecks_solr_cd.yml b/.github/workflows/healthchecks_solr_cd.yml
@@ -11,7 +11,7 @@ jobs:
       BUILD_CONFIG: Release
     runs-on: ubuntu-latest
     services:
-      consul:
+      solr:
         image: solr:8.4.1
         ports:
           - 8983:8983

diff --git a/.github/workflows/healthchecks_solr_cd_preview.yml b/.github/workflows/healthchecks_solr_cd_preview.yml
@@ -12,7 +12,7 @@ jobs:
       VERSION_SUFFIX: -rc2.${{ github.run_number }}
     runs-on: ubuntu-latest
     services:
-      consul:
+      solr:
         image: solr:8.4.1
         ports:
           - 8983:8983

diff --git a/.github/workflows/healthchecks_sqlserver_cd_preview.yml b/.github/workflows/healthchecks_sqlserver_cd_preview.yml
@@ -2,7 +2,7 @@ name: HealthChecks SQL Server DB Preview CD
 
 on:
   push:
-    tags:       
+    tags:
       - preview-sqlserver-*
 
 jobs:
@@ -16,7 +16,7 @@ jobs:
         image: mcr.microsoft.com/mssql/server
         ports:
           - 5433:1433
-        env: 
+        env:
           ACCEPT_EULA: Y
           SA_PASSWORD: Password12!
     steps:
@@ -32,7 +32,7 @@ jobs:
       run: dotnet build --no-restore ./src/HealthChecks.SqlServer/HealthChecks.SqlServer.csproj
     - name: Test
       run: dotnet test ./test/HealthChecks.SqlServer.Tests/HealthChecks.SqlServer.Tests.csproj --verbosity normal
-    - name: dotnet pack 
+    - name: dotnet pack
       run: dotnet pack ./src/HealthChecks.SqlServer/HealthChecks.SqlServer.csproj --version-suffix $VERSION_SUFFIX -c $BUILD_CONFIG --include-source --include-symbols -o ./artifacts
     - name: setup nuget
       uses: NuGet/setup-nuget@v1.0.2

diff --git a/AspNetCore.Diagnostics.HealthChecks.sln b/AspNetCore.Diagnostics.HealthChecks.sln
@@ -15,6 +15,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "build", "build", "{A5A8CE48
 		Directory.Build.targets = Directory.Build.targets
 		docker-compose.yml = docker-compose.yml
 		dotnet-install.ps1 = dotnet-install.ps1
+		global.json = global.json
 		install-sdk.ps1 = install-sdk.ps1
 	EndProjectSection
 EndProject

diff --git a/src/HealthChecks.Rabbitmq/DependencyInjection/RabbitMQHealthCheckBuilderExtensions.cs b/src/HealthChecks.Rabbitmq/DependencyInjection/RabbitMQHealthCheckBuilderExtensions.cs
@@ -6,6 +6,9 @@
 
 namespace Microsoft.Extensions.DependencyInjection
 {
+    /// <summary>
+    /// Extension methods to configure <see cref="RabbitMQHealthCheck"/>.
+    /// </summary>
     public static class RabbitMQHealthCheckBuilderExtensions
     {
         const string NAME = "rabbitmq";
@@ -65,7 +68,7 @@ public static IHealthChecksBuilder AddRabbitMQ(this IHealthChecksBuilder builder
         }
 
         /// <summary>
-        /// Add a health check for RabbitMQ services using <see cref="IConnection"/> from service provider 
+        /// Add a health check for RabbitMQ services using <see cref="IConnection"/> from service provider
         /// or <see cref="IConnectionFactory"/> from service provider if none is found. At least one must be configured.
         /// </summary>
         /// <param name="builder">The <see cref="IHealthChecksBuilder"/>.</param>

diff --git a/src/HealthChecks.Rabbitmq/RabbitMQHealthCheck.cs b/src/HealthChecks.Rabbitmq/RabbitMQHealthCheck.cs
@@ -7,27 +7,31 @@
 namespace HealthChecks.RabbitMQ
 {
     public class RabbitMQHealthCheck
-        : IHealthCheck
+        : IHealthCheck, IDisposable
     {
         private IConnection _connection;
-
         private IConnectionFactory _factory;
         private readonly Uri _rabbitConnectionString;
         private readonly SslOption _sslOption;
+        private readonly bool _ownsConnection;
+        private bool _disposed;
 
         public RabbitMQHealthCheck(IConnection connection)
         {
             _connection = connection ?? throw new ArgumentNullException(nameof(connection));
         }
+
         public RabbitMQHealthCheck(IConnectionFactory factory)
         {
             _factory = factory ?? throw new ArgumentNullException(nameof(factory));
+            _ownsConnection = true;
         }
 
         public RabbitMQHealthCheck(Uri rabbitConnectionString, SslOption ssl)
         {
             _rabbitConnectionString = rabbitConnectionString;
             _sslOption = ssl;
+            _ownsConnection = true;
         }
 
         public Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context, CancellationToken cancellationToken = default)
@@ -36,11 +40,8 @@ public Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context, Canc
             {
                 EnsureConnection();
 
-                using (_connection.CreateModel())
-                {
-                    return Task.FromResult(
-                        HealthCheckResult.Healthy());
-                }
+                using var model = _connection.CreateModel();
+                return Task.FromResult(HealthCheckResult.Healthy());
             }
             catch (Exception ex)
             {
@@ -51,6 +52,9 @@ public Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context, Canc
 
         private void EnsureConnection()
         {
+            if (_disposed)
+                throw new ObjectDisposedException(nameof(RabbitMQHealthCheck));
+
             if (_connection == null)
             {
                 if (_factory == null)
@@ -68,5 +72,25 @@ private void EnsureConnection()
                 _connection = _factory.CreateConnection();
             }
         }
+
+        protected virtual void Dispose(bool disposing)
+        {
+            if (disposing)
+            {
+                // dispose connection only if RabbitMQHealthCheck owns it
+                if (!_disposed && _connection != null && _ownsConnection)
+                {
+                    _connection.Dispose();
+                    _connection = null;
+                }
+                _disposed = true;
+            }
+        }
+
+        public void Dispose()
+        {
+            Dispose(true);
+            GC.SuppressFinalize(this);
+        }
     }
 }
diff --git a/test/HealthChecks.RabbitMQ.Tests/DependencyInjection/RegistrationTests.cs b/test/HealthChecks.RabbitMQ.Tests/DependencyInjection/RegistrationTests.cs
@@ -3,6 +3,7 @@
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Diagnostics.HealthChecks;
 using Microsoft.Extensions.Options;
+using System;
 using System.Linq;
 using Xunit;
 
@@ -28,6 +29,11 @@ public void add_health_check_when_properly_configured()
 
             registration.Name.Should().Be(_defaultCheckName);
             check.GetType().Should().Be(typeof(RabbitMQHealthCheck));
+
+            ((RabbitMQHealthCheck)check).Dispose();
+            var result = check.CheckHealthAsync(new HealthCheckContext { Registration = new HealthCheckRegistration("", check, null, null) }).Result;
+            result.Status.Should().Be(HealthStatus.Unhealthy);
+            result.Exception.GetType().Should().Be(typeof(ObjectDisposedException));
         }
 
         [Fact]
@@ -47,6 +53,11 @@ public void add_named_health_check_when_properly_configured()
 
             registration.Name.Should().Be(customCheckName);
             check.GetType().Should().Be(typeof(RabbitMQHealthCheck));
+
+            ((RabbitMQHealthCheck)check).Dispose();
+            var result = check.CheckHealthAsync(new HealthCheckContext { Registration = new HealthCheckRegistration("", check, null, null) }).Result;
+            result.Status.Should().Be(HealthStatus.Unhealthy);
+            result.Exception.GetType().Should().Be(typeof(ObjectDisposedException));
         }
     }
 }