Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

NH-3402 - Refactor ThreadLocalSessionContext #292

Closed
wants to merge 2 commits into from
Closed

NH-3402 - Refactor ThreadLocalSessionContext #292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7f04562
NH-3402
rjperes Aug 13, 2014
272f985
Fixed compilation bugs.
rjperes Nov 18, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/NHibernate.Test/ConnectionTest/ThreadLocalCurrentSessionTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,13 @@ public TestableThreadLocalContext(ISessionFactoryImplementor factory)

public static bool IsSessionBound(ISession session)
{
return context != null && context.ContainsKey(me.factory)
&& context[me.factory] == session;

return session.SessionFactory.GetCurrentSession() == session;
}

public static bool HasBind()
{
return context != null && context.ContainsKey(me.factory);
return me.Session != null;
}
}
}
153 changes: 7 additions & 146 deletions src/NHibernate/Context/ThreadLocalSessionContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
using System;
using System.Collections.Generic;

using System.Threading;
using NHibernate;
using NHibernate.Engine;

namespace NHibernate.Context
{
//TODO: refactoring on this class. Maybe using MapBasedSessionContext.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good move to have ignored and removed this comment, since the MapBasedSessionContext implementation is flawed (thread safety issues, going to Jira that), and unneeded with your implementation.

/// <summary>
/// A <see cref="ICurrentSessionContext"/> impl which scopes the notion of current
/// session by the current thread of execution. Threads do not give us a
Expand All @@ -20,164 +19,26 @@ namespace NHibernate.Context
/// has been called. If <tt>Close()</tt> is called on a session managed by
/// this class, it will be automatically unbound.
/// <p/>
/// Additionally, the static <see cref="Bind"/> and <see cref="Unbind"/> methods are
/// Additionally, the static <see cref="CurrentSessionContext.Bind"/> and <see cref="CurrentSessionContext.Unbind"/> methods are
/// provided to allow application code to explicitly control opening and
/// closing of these sessions. This, with some from of interception,
/// is the preferred approach. It also allows easy framework integration
/// and one possible approach for implementing long-sessions.
/// <p/>
/// </summary>
Copy link
Member

@fredericDelaporte fredericDelaporte Apr 4, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change, most of this xml comment is obsolete. The previous implementation was generating a session if there was no current session, which this new implementation does not.
The comment should be updated accordingly.

Worst, this comment looks obsolete from the start. The mechanism for auto-unbinding a closed session seems to have never been implemented though documented in this xml doc. Same for the mechanism which was supposed to render the session unusable till a transaction is started.
The Ignore on his test class confirms it: Ignore("Not yet supported. Need AutoClosed feature.(TransactionContext)"). This ThreadLocalCurrentSessionTest test class looks quite crappy: it is like it assumes OpenSession on factory use the session context, it overrides a method pointlessly (called neither by base implementation nor by the test class), ... It should be deleted I think.

[Serializable]
public class ThreadLocalSessionContext : ICurrentSessionContext
public class ThreadLocalSessionContext : CurrentSessionContext
{
private static readonly IInternalLogger log = LoggerProvider.LoggerFor(typeof(ThreadLocalSessionContext));

[ThreadStatic]
protected static IDictionary<ISessionFactory, ISession> context;

protected readonly ISessionFactoryImplementor factory;

private ThreadLocal<ISession> _session = new ThreadLocal<ISession>();

public ThreadLocalSessionContext(ISessionFactoryImplementor factory)
{
this.factory = factory;
}

#region ICurrentSessionContext Members

public ISession CurrentSession()
{
ISession current = ExistingSession(factory);
if (current == null)
{
current = BuildOrObtainSession();

// wrap the session in the transaction-protection proxy
if (NeedsWrapping(current))
{
current = Wrap(current);
}
// then bind it
DoBind(current, factory);
}
return current;
}

#endregion

private static void CleanupAnyOrphanedSession(ISessionFactory factory)
{
ISession orphan = DoUnbind(factory, false);

if (orphan != null)
{
log.Warn("Already session bound on call to bind(); make sure you clean up your sessions!");

try
{
if (orphan.Transaction != null && orphan.Transaction.IsActive)
{
try
{
orphan.Transaction.Rollback();
}
catch (Exception ex)
{
log.Debug("Unable to rollback transaction for orphaned session", ex);
}
}
orphan.Close();
}
catch (Exception ex)
{
log.Debug("Unable to close orphaned session", ex);
}
}
}

public static void Bind(ISession session)
{
ISessionFactory factory = session.SessionFactory;
CleanupAnyOrphanedSession(factory);
DoBind(session, factory);
}

/// <summary>
/// Unassociate a previously bound session from the current thread of execution.
/// </summary>
/// <param name="factory"></param>
/// <returns></returns>
public static ISession Unbind(ISessionFactory factory)
{
return DoUnbind(factory, true);
}

private static void DoBind(ISession current, ISessionFactory factory)
{
context = context ?? new Dictionary<ISessionFactory, ISession>();

context.Add(factory, current);
}

private static ISession DoUnbind(ISessionFactory factory, bool releaseMapIfEmpty)
{
ISession session = null;

if (context != null)
{
if (context.TryGetValue(factory, out session))
{
context.Remove(factory);
}

if (releaseMapIfEmpty && context.Count == 0)
context = null;
}
return session;
}

private ISession Wrap(ISession current)
{
return current;
}

private bool NeedsWrapping(ISession current)
{
return false;
}

protected ISession BuildOrObtainSession()
{
return factory.OpenSession(
null,
IsAutoFlushEnabled(),
IsAutoCloseEnabled(),
GetConnectionReleaseMode());
}

private ConnectionReleaseMode GetConnectionReleaseMode()
{
return factory.Settings.ConnectionReleaseMode;
}

protected virtual bool IsAutoCloseEnabled()
{
return true;
}

protected virtual bool IsAutoFlushEnabled()
{
return true;
}

private static ISession ExistingSession(ISessionFactory factory)
protected override ISession Session
{
if (context == null)
return null;

ISession result;
context.TryGetValue(factory, out result);
return result;
get { return (this._session.Value); }
set { this._session.Value = value; }
}
}
}