Skip to content

Commit 7d83fcf

Browse files
committed
Simplify filter logic
Based on the fact that SessionAccessor is required to be scoped service code became much simplier
1 parent d4c8420 commit 7d83fcf

File tree

1 file changed

+35
-26
lines changed

1 file changed

+35
-26
lines changed

Extensions/Xtensive.Orm.Web/Filters/SessionActionFilter.cs

Lines changed: 35 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,8 @@ namespace Xtensive.Orm.Web.Filters
1515
/// </summary>
1616
public class SessionActionFilter : IActionFilter, IAsyncActionFilter
1717
{
18-
private SessionAccessor sessionAccessor;
1918
private IDisposable contextBindResource;
20-
private bool skipResourcesRelease;
19+
private bool skipResourcesDisposal;
2120

2221
/// <inheritdoc/>
2322
public void OnActionExecuting(ActionExecutingContext context) =>
@@ -119,52 +118,59 @@ protected virtual void OnSessionDisposed(ActionExecutedContext context)
119118
/// <returns><see langword="true"/> for complete transaction scope, otherwise, <see langword="false"/>.</returns>
120119
protected virtual bool CompleteTransactionOnException(Exception exception, ActionExecutedContext context) => false;
121120

122-
//internal void Persist(PersistReason reason) => Persist(reason, false).GetAwaiter().GetResult();
123121
private async ValueTask ExecuteBeforeAction(ActionExecutingContext context, bool isAsync)
124122
{
125123
var actionParameters = context.ActionDescriptor.Parameters;
126124

127-
skipResourcesRelease = true;
125+
skipResourcesDisposal = true;
126+
127+
// accessor is always in services as scoped instance (one per request)
128+
// so, if there is no session opened we need to open it,
129+
// put it to the httpcontext and bind it to the first mention of SessionAccessor
128130

129131
//this search can probably be improved by caching action names with needed parameters
130132
foreach (var p in actionParameters) {
131133
if (p.ParameterType == WellKnownTypes.SessionAccessorType) {
132134
// trying to get registered accessor as service
133135
var accessor = GetSessionAccesorFromServices(context.HttpContext);
134-
if (accessor != null) {
135-
// this is registered as service and probably filled with middleware
136-
skipResourcesRelease = true;
137-
sessionAccessor = accessor;
138-
if (accessor.ContextIsBound) {
139-
return;// middleware is in pipeline, it did the work
136+
if (!accessor.ContextIsBound) {
137+
if (SessionInContextExists(context.HttpContext)) {
138+
// something opened session but does not bind context to accessor
139+
// bind context and let outer code manage Session instance.
140+
contextBindResource = accessor.BindHttpContext(context.HttpContext);
141+
skipResourcesDisposal = true;
140142
}
141143
else {
142-
contextBindResource = await CreateSessionAndBindContext(sessionAccessor, context, isAsync);
143-
skipResourcesRelease = false;
144+
// nothing is in context, create and bind,
145+
// also remember that we need to dispose opened session and transaction scope
146+
contextBindResource = await CreateSessionAndBindContext(accessor, context, isAsync);
147+
skipResourcesDisposal = false;
144148
}
145149
}
146-
else {
147-
//we're using the instance Asp.Net infrastructure created for us
148-
sessionAccessor = (SessionAccessor) context.ActionArguments[p.Name];
149-
contextBindResource = await CreateSessionAndBindContext(sessionAccessor, context, isAsync);
150-
skipResourcesRelease = false;
151-
}
152150
break;
153151
}
154152
}
155153
}
156154

157155
private async ValueTask ExecuteAfterAction(ActionExecutedContext context, bool isAsync)
158156
{
159-
if (skipResourcesRelease) {
157+
contextBindResource.DisposeSafely();
158+
if (skipResourcesDisposal) {
160159
return;
161160
}
161+
162162
// session and tx is created here
163163
// so we need to clean HttpContext.
164-
var session = sessionAccessor.Session;
165-
var tx = sessionAccessor.TransactionScope;
166164

167165
var httpContext = context.HttpContext;
166+
var session = (Session) httpContext.Items[SessionAccessor.SessionIdentifier];
167+
var tx = (TransactionScope) httpContext.Items[SessionAccessor.ScopeIdentifier];
168+
169+
if (session == null || tx == null) {
170+
// just in case user clears items, this will cause session leakage.
171+
throw new InvalidOperationException("Session or TransactionScope no longer exists in HttpContext. Do not remove it from HttpContext manually.");
172+
}
173+
168174
_ = httpContext.Items.Remove(SessionAccessor.ScopeIdentifier);
169175
_ = httpContext.Items.Remove(SessionAccessor.SessionIdentifier);
170176

@@ -185,9 +191,6 @@ private async ValueTask ExecuteAfterAction(ActionExecutedContext context, bool i
185191
session.Dispose();
186192
}
187193
OnSessionDisposed(context);
188-
189-
contextBindResource.DisposeSafely(); // unbind httpcontext
190-
sessionAccessor = null;
191194
}
192195

193196
private async ValueTask<IDisposable> CreateSessionAndBindContext(SessionAccessor accessor, ActionExecutingContext context, bool isAsync)
@@ -216,7 +219,13 @@ private static Domain GetDomainFromServices(HttpContext context)
216219
return domain ?? throw new InvalidOperationException("Domain is not found among registered services.");
217220
}
218221

219-
private static SessionAccessor GetSessionAccesorFromServices(HttpContext context) =>
220-
(SessionAccessor) context.RequestServices.GetService(WellKnownTypes.SessionAccessorType);
222+
private static SessionAccessor GetSessionAccesorFromServices(HttpContext context)
223+
{
224+
var sessionAccessor = (SessionAccessor) context.RequestServices.GetService(WellKnownTypes.SessionAccessorType);
225+
return sessionAccessor ?? throw new InvalidOperationException("SessionAccessor should be registered as Scoped service.");
226+
}
227+
228+
private static bool SessionInContextExists(HttpContext context) =>
229+
context.Items.TryGetValue(SessionAccessor.SessionIdentifier, out var sessionRaw) && sessionRaw != null;
221230
}
222231
}

0 commit comments

Comments
 (0)