Skip to content

Provide cacheable representations for all NHibernate built-in types #1765

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.

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

Merged
merged 1 commit into from
Jun 28, 2018

Conversation

fredericDelaporte
Copy link
Member

As written in nhibernate/NHibernate-Caches#45, some NHibernate built-in types do not have a proper cacheable representation. Many cache providers relies on cached objects being binary serializable (even some non-distributed ones), but some NHibernate types yields representations which are not, or which convey way more information than actually handled by NHibernate.
Moreover if the type can be a simple type, easing support of other kind of serializations (Json), that is better.

  • CultureInfoType yields a full CultureInfo object, while it only persists its Name.
  • TypeType yields a System.Type object, which is not binary serializable under .Net Core.
  • UriType yields an Uri object, while it only persists the uri.
  • XDocType yields a XDocument which is not binary serializable.
  • XmlDocType yields a XmlDocument which is not binary serializable.

These five types are all persisted as string in the database, so they already naturally have a cacheable representation. This PR uses that string representation.

Additionally better guidance has been added on IUserType Assemble/Disassemble methods.

This is a possible breaking change, because any out-of-process second level cache would have to be cleared after upgrading NHibernate, if some of those types were cached.

@hazzik
Copy link
Member

hazzik commented Jun 21, 2018

NullableType already has FromStringValue and ToString methods which have the same purpose. Why not using them? (I saw you've already delegated from these methods to a new one).

@fredericDelaporte
Copy link
Member Author

Their semantic seems to be xml bound. Their xml comment states:

A representation of the value to be embedded in an XML element

Parse the XML representation of an instance

That is why I have avoided re-using them. They do not look as being for general purpose. I have used them only in cases where they were already used for parsing/generating the string value from/for database.

@hazzik
Copy link
Member

hazzik commented Jun 21, 2018

The XML part is a remnant of never implemented EntityMode.Xml.

@fredericDelaporte
Copy link
Member Author

I had guessed, but that means two things:

  • they are untested (excepted those already used for another purpose), especially round-tripping is not tested.
  • they may potentially do some xml escaping/de-escaping we do not care of.

If re-using them, I would have better changing their implementations for these used for setting/getting the value to/from db.

@fredericDelaporte fredericDelaporte merged commit 27e47da into nhibernate:master Jun 28, 2018
@fredericDelaporte fredericDelaporte deleted the Disassemble branch June 28, 2018 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants