NH-4003 - porting ISessionCreationOptions #624
Conversation
| Never, | ||
|
|
||
| [System.Xml.Serialization.XmlEnumAttribute("manual")] | ||
| Manual, |
There was a problem hiding this comment.
Manually adjusted. The Never mode has been renamed Manual on Hibernate side, which looks more accurate to me. Still supporting Never for backward compatibility.
| @@ -81,6 +81,8 @@ public Settings() | |||
|
|
|||
| public bool IsIdentifierRollbackEnabled { get; internal set; } | |||
|
|
|||
| // Since v5 | |||
| [Obsolete("Please use DefaultFlushMode instead.")] | |||
| public bool IsFlushBeforeCompletionEnabled { get; internal set; } | |||
There was a problem hiding this comment.
It was already having no usages. Now maybe will it have some once Hibernate transaction management will be ported.
| /// <summary> | ||
| /// The singleton reference. | ||
| /// </summary> | ||
| public static readonly EmptyInterceptor Instance = new EmptyInterceptor(); |
There was a problem hiding this comment.
Realigning on Hibernate pattern. Creating it at each usage was not making much sens.
| /// <summary> | ||
| /// Represents a consolidation of all session creation options into a builder style delegate. | ||
| /// </summary> | ||
| public interface ISessionBuilder<T> where T : ISessionBuilder<T> |
There was a problem hiding this comment.
Java session builder interface uses covariant return type: its methods are fluent, and return the most derived interface (ISharedSessionBuilder by example). It is a bit messy to achieve in .Net.
| /// </summary> | ||
| /// <param name="conn">A connection provided by the application</param> | ||
| /// <returns>The session builder.</returns> | ||
| ISessionBuilder WithOptions(); |
There was a problem hiding this comment.
The factory has now only one non obsolete method for opening stateful session, and it returns a session builder. A session builder can be used for opening as many session as we wish.
| ISession _childSession; | ||
|
|
||
| [NonSerialized] | ||
| private readonly bool flushBeforeCompletionEnabled; |
There was a problem hiding this comment.
Fields removed.
| private readonly ConnectionReleaseMode connectionReleaseMode; | ||
| [NonSerialized] | ||
| private readonly bool _transactionCoordinatorShared; |
There was a problem hiding this comment.
NH addition, I expect this to be no more needed once transaction handling will be ported.
| @@ -140,15 +126,19 @@ void ISerializable.GetObjectData(SerializationInfo info, StreamingContext contex | |||
| { | |||
| throw new InvalidOperationException("Cannot serialize a Session while connected"); | |||
| } | |||
| if (_transactionCoordinatorShared) | |||
| { | |||
| throw new InvalidOperationException("Cannot serialize a Session sharing its transaction coordinator"); | |||
There was a problem hiding this comment.
After deserialization, the shared thing (currently connection manager) would be no more shared.
src/NHibernate/Impl/SessionImpl.cs
Outdated
| return interceptor.EntityName; | ||
| // NH: support of field-Interceptor-proxy | ||
| IFieldInterceptor Interceptor = FieldInterceptionHelper.ExtractFieldInterceptor(entity); | ||
| return Interceptor.EntityName; |
There was a problem hiding this comment.
Bad replace all, fixed locally, not yet pushed. (Fixed in comments/string too.)
| .ConnectionReleaseMode() | ||
| .FlushMode() | ||
| .Interceptor() | ||
| .OpenSession(); |
There was a problem hiding this comment.
Have to do all that with the new method if you want to be as close as possible... But we have no covering tests for that indeed: in the tests, I have just call Connection(), and that is enough for all of them.
|
|
||
| /// <remarks/> | ||
| [System.Xml.Serialization.XmlEnumAttribute("never")] | ||
| Never, |
There was a problem hiding this comment.
Shall be Never = Manual, (as in another place)
There was a problem hiding this comment.
Semantically cleaner, but not sure the generator from xsd would do that, it has no mean to know it should (or I do not know how to tell it in xsd).
|
It feels like some of the methods need to be CSharpified (eg. replaced with properties). |
|
LGTM |
At first I wanted to do that for So I have just decided that converting them to properties was not really worth the trouble in that case. Written as comment in its implementing class: I will add that comment on About Would you rather handle Have you seen other missed case of property conversion? |
|
(Pushed the fixes I had announced in review.) |
|
Found a bug, will fix and add test cases. |
|
Bugs fixed (a |
…squashed or dropped.
NH-4003 - Refactor session constructor
I consider this as a prerequisite for porting the current transaction handling from Hibernate. This handling especially relies on the
ISharedSessionBuilder, introduced by this PR.I have dropped the obsolete parameter
InterceptorsBeforeTransactionCompletionIgnoreExceptionsin the process.The
GetChildSessionintroduced in 5.0 by the removal of entity mode switching is removed in favor of HibernateISession.WithSessionOptions()which yield a builder of session, allowing to build new sessions taking a part of their stuff (like the connection manager currently) from the original session. These new sessions are not closed or flushed from the original session.I have reintroduced as obsolete the old
ISession.GetSession(), for hinting at usingISession.WithSessionOptions().All that by the way obsoletes NH-3985 (already resolved, #600, about disposed child session being returned) and NH-3986 (Enable multiple child sessions per root session: the new semantic allows this, though there are no more actually children, the "parent" has no knowledge of them currently).
Some stuff has moved around, Hibernate having put many more things in the AbstractSessionImpl.