Skip to content

Commit 8ad1c88

Browse files
bradwilsonbradwilson
committed
Remove IHttpControllerActivator.Release()
1 parent 0602800 commit 8ad1c88

File tree

14 files changed

+197
-274
lines changed

14 files changed

+197
-274
lines changed

‎src/System.Web.Http/ApiController.cs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,15 @@ public virtual Task<HttpResponseMessage> ExecuteAsync(HttpControllerContext cont
124124
}
125125

126126
Initialize(controllerContext);
127-
HttpControllerDescriptor controllerDescriptor = controllerContext.ControllerDescriptor;
128127

128+
// We can't be reused, and we know we're disposable, so make sure we go away when
129+
// the request has been completed.
130+
if (_request != null)
131+
{
132+
_request.RegisterForDispose(this);
133+
}
134+
135+
HttpControllerDescriptor controllerDescriptor = controllerContext.ControllerDescriptor;
129136
HttpActionDescriptor actionDescriptor = controllerDescriptor.HttpActionSelector.SelectAction(controllerContext);
130137
HttpActionContext actionContext = new HttpActionContext(controllerContext, actionDescriptor);
131138

‎src/System.Web.Http/Controllers/HttpControllerDescriptor.cs‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -172,27 +172,6 @@ public virtual IHttpController CreateController(HttpRequestMessage request)
172172
return instance;
173173
}
174174

175-
/// <summary>
176-
/// Releases an <see cref="IHttpController"/> instance.
177-
/// </summary>
178-
/// <param name="controllerContext">The controller context.</param>
179-
/// <param name="controller">The controller.</param>
180-
public virtual void ReleaseController(IHttpController controller, HttpControllerContext controllerContext)
181-
{
182-
if (controller == null)
183-
{
184-
throw Error.ArgumentNull("controller");
185-
}
186-
187-
if (controllerContext == null)
188-
{
189-
throw Error.ArgumentNull("controllerContext");
190-
}
191-
192-
// just delegate the work to the activator
193-
HttpControllerActivator.Release(controller, controllerContext);
194-
}
195-
196175
/// <summary>
197176
/// Returns the collection of <see cref="IFilter">filters</see> associated with this descriptor's controller.
198177
/// </summary>

‎src/System.Web.Http/Dispatcher/DefaultHttpControllerActivator.cs‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -85,29 +85,5 @@ public IHttpController Create(HttpRequestMessage request, HttpControllerDescript
8585
throw Error.InvalidOperation(ex, SRResources.DefaultControllerFactory_ErrorCreatingController, controllerType.Name);
8686
}
8787
}
88-
89-
/// <summary>
90-
/// Releases the <paramref name="controller"/> instance
91-
/// </summary>
92-
/// <param name="controllerContext">The <see cref="HttpControllerContext"/> containing </param>
93-
/// <param name="controller">The <see cref="IHttpController"/> that is to be released</param>
94-
public void Release(IHttpController controller, HttpControllerContext controllerContext)
95-
{
96-
if (controller == null)
97-
{
98-
throw Error.ArgumentNull("controller");
99-
}
100-
101-
if (controllerContext == null)
102-
{
103-
throw Error.ArgumentNull("controllerContext");
104-
}
105-
106-
IDisposable disposable = controller as IDisposable;
107-
if (disposable != null)
108-
{
109-
disposable.Dispose();
110-
}
111-
}
11288
}
11389
}

‎src/System.Web.Http/Dispatcher/HttpControllerDispatcher.cs‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -157,18 +157,7 @@ private Task<HttpResponseMessage> SendAsyncInternal(HttpRequestMessage request,
157157
controllerContext.Controller = httpController;
158158
controllerContext.ControllerDescriptor = httpControllerDescriptor;
159159

160-
try
161-
{
162-
return httpController.ExecuteAsync(controllerContext, cancellationToken).Finally(() =>
163-
{
164-
httpControllerDescriptor.ReleaseController(httpController, controllerContext);
165-
});
166-
}
167-
catch
168-
{
169-
httpControllerDescriptor.ReleaseController(httpController, controllerContext);
170-
throw;
171-
}
160+
return httpController.ExecuteAsync(controllerContext, cancellationToken);
172161
}
173162

174163
[SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "Caller owns HttpResponseMessage instance.")]

‎src/System.Web.Http/Dispatcher/IHttpControllerActivator.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,5 @@ namespace System.Web.Http.Dispatcher
99
public interface IHttpControllerActivator
1010
{
1111
IHttpController Create(HttpRequestMessage request, HttpControllerDescriptor controllerDescriptor, Type controllerType);
12-
13-
void Release(IHttpController controller, HttpControllerContext controllerContext);
1412
}
1513
}

‎src/System.Web.Http/Tracing/Tracers/HttpControllerActivatorTracer.cs‎

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ namespace System.Web.Http.Tracing.Tracers
1111
internal class HttpControllerActivatorTracer : IHttpControllerActivator
1212
{
1313
private const string CreateMethodName = "Create";
14-
private const string ReleaseMethodName = "Release";
1514

1615
private readonly IHttpControllerActivator _innerActivator;
1716
private readonly ITraceWriter _traceWriter;
@@ -46,30 +45,10 @@ IHttpController IHttpControllerActivator.Create(HttpRequestMessage request, Http
4645

4746
if (controller != null && !(controller is HttpControllerTracer))
4847
{
49-
controller = new HttpControllerTracer(controller, _traceWriter);
48+
controller = new HttpControllerTracer(request, controller, _traceWriter);
5049
}
5150

5251
return controller;
5352
}
54-
55-
void IHttpControllerActivator.Release(IHttpController controller, HttpControllerContext controllerContext)
56-
{
57-
_traceWriter.TraceBeginEnd(
58-
controllerContext.Request,
59-
TraceCategories.ControllersCategory,
60-
TraceLevel.Info,
61-
_innerActivator.GetType().Name,
62-
ReleaseMethodName,
63-
beginTrace: (tr) =>
64-
{
65-
tr.Message = controller == null ? SRResources.TraceNoneObjectMessage : controller.GetType().FullName;
66-
},
67-
execute: () =>
68-
{
69-
_innerActivator.Release(controller, controllerContext);
70-
},
71-
endTrace: null,
72-
errorTrace: null);
73-
}
7453
}
7554
}

‎src/System.Web.Http/Tracing/Tracers/HttpControllerDescriptorTracer.cs‎

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
using System.Net.Http;
1+
using System.Diagnostics.CodeAnalysis;
2+
using System.Net.Http;
23
using System.Web.Http.Controllers;
34
using System.Web.Http.Properties;
45

@@ -10,7 +11,6 @@ namespace System.Web.Http.Tracing.Tracers
1011
internal class HttpControllerDescriptorTracer : HttpControllerDescriptor
1112
{
1213
private const string CreateControllerMethodName = "CreateController";
13-
private const string ReleaseControllerMethodName = "ReleaseController";
1414

1515
private readonly HttpControllerDescriptor _innerDescriptor;
1616
private readonly ITraceWriter _traceWriter;
@@ -22,6 +22,7 @@ public HttpControllerDescriptorTracer(HttpConfiguration configuration, string co
2222
_traceWriter = traceWriter;
2323
}
2424

25+
[SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "This object is returned back to the caller")]
2526
public override IHttpController CreateController(HttpRequestMessage request)
2627
{
2728
IHttpController controller = null;
@@ -47,31 +48,10 @@ public override IHttpController CreateController(HttpRequestMessage request)
4748

4849
if (controller != null && !(controller is HttpControllerTracer))
4950
{
50-
return new HttpControllerTracer(controller, _traceWriter);
51+
return new HttpControllerTracer(request, controller, _traceWriter);
5152
}
5253

5354
return controller;
5455
}
55-
56-
public override void ReleaseController(IHttpController controller, HttpControllerContext controllerContext)
57-
{
58-
_traceWriter.TraceBeginEnd(
59-
controllerContext.Request,
60-
TraceCategories.ControllersCategory,
61-
TraceLevel.Info,
62-
_innerDescriptor.GetType().Name,
63-
ReleaseControllerMethodName,
64-
beginTrace: (tr) =>
65-
{
66-
tr.Message = HttpControllerTracer.ActualControllerType(controller).FullName;
67-
},
68-
execute: () =>
69-
{
70-
IHttpController actualController = HttpControllerTracer.ActualController(controller);
71-
_innerDescriptor.ReleaseController(actualController, controllerContext);
72-
},
73-
endTrace: null,
74-
errorTrace: null);
75-
}
7656
}
7757
}

‎src/System.Web.Http/Tracing/Tracers/HttpControllerTracer.cs‎

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,49 @@
1-
using System.Net.Http;
1+
using System.Collections.Generic;
2+
using System.Net.Http;
23
using System.Threading;
34
using System.Threading.Tasks;
45
using System.Web.Http.Controllers;
6+
using System.Web.Http.Hosting;
57

68
namespace System.Web.Http.Tracing.Tracers
79
{
810
/// <summary>
911
/// Tracer for <see cref="IHttpController"/>.
1012
/// </summary>
11-
internal class HttpControllerTracer : IHttpController
13+
internal class HttpControllerTracer : IHttpController, IDisposable
1214
{
15+
private const string DisposeMethodName = "Dispose";
1316
private const string ExecuteAsyncMethodName = "ExecuteAsync";
1417

1518
private readonly IHttpController _innerController;
19+
private readonly HttpRequestMessage _request;
1620
private readonly ITraceWriter _traceWriter;
1721

18-
public HttpControllerTracer(IHttpController innerController, ITraceWriter traceWriter)
22+
public HttpControllerTracer(HttpRequestMessage request, IHttpController innerController, ITraceWriter traceWriter)
1923
{
2024
_innerController = innerController;
25+
_request = request;
2126
_traceWriter = traceWriter;
2227
}
2328

29+
void IDisposable.Dispose()
30+
{
31+
IDisposable disposable = _innerController as IDisposable;
32+
if (disposable != null)
33+
{
34+
_traceWriter.TraceBeginEnd(
35+
_request,
36+
TraceCategories.ControllersCategory,
37+
TraceLevel.Info,
38+
_innerController.GetType().Name,
39+
DisposeMethodName,
40+
beginTrace: null,
41+
execute: disposable.Dispose,
42+
endTrace: null,
43+
errorTrace: null);
44+
}
45+
}
46+
2447
Task<HttpResponseMessage> IHttpController.ExecuteAsync(HttpControllerContext controllerContext, CancellationToken cancellationToken)
2548
{
2649
return _traceWriter.TraceBeginEndAsync<HttpResponseMessage>(
@@ -34,7 +57,25 @@ Task<HttpResponseMessage> IHttpController.ExecuteAsync(HttpControllerContext con
3457
{
3558
// Critical to allow wrapped controller to have itself in ControllerContext
3659
controllerContext.Controller = ActualController(controllerContext.Controller);
37-
return _innerController.ExecuteAsync(controllerContext, cancellationToken);
60+
return _innerController.ExecuteAsync(controllerContext, cancellationToken)
61+
.Finally(() =>
62+
{
63+
IDisposable disposable = _innerController as IDisposable;
64+
65+
if (disposable != null)
66+
{
67+
// Need to remove the original controller from the disposables list, if it's
68+
// there, and put ourselves in there instead, so we can trace the dispose.
69+
// This currently knows a little too much about how RegisterForDispose works,
70+
// but that's unavoidable unless we want to offer UnregisterForDispose.
71+
IList<IDisposable> disposables;
72+
if (_request.Properties.TryGetValue(HttpPropertyKeys.DisposableRequestResourcesKey, out disposables))
73+
{
74+
disposables.Remove(disposable);
75+
disposables.Add(this);
76+
}
77+
}
78+
});
3879
},
3980
endTrace: (tr, response) =>
4081
{

‎test/System.Web.Http.Integration.Test/Controllers/CustomControllerDescriptorTest.cs‎

Lines changed: 0 additions & 75 deletions
This file was deleted.

‎test/System.Web.Http.Integration.Test/System.Web.Http.Integration.Test.csproj‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,6 @@
105105
<Compile Include="Controllers\Apis\TestController.cs" />
106106
<Compile Include="Controllers\Apis\User.cs" />
107107
<Compile Include="Controllers\Apis\UserAddress.cs" />
108-
<Compile Include="Controllers\CustomControllerDescriptorTest.cs" />
109108
<Compile Include="Controllers\Helpers\ApiControllerHelper.cs" />
110109
<Compile Include="ExceptionHandling\DuplicateControllers.cs" />
111110
<Compile Include="ExceptionHandling\ExceptionController.cs" />

0 commit comments

Comments
 (0)