Broken if NHibernate version is greater than 5.1

[EnemyArea]

Hey guys,

there seems to be some changes, that prevents the dialect to be createt. If you test it with Nhibernate 5.1.2 (latest 5.1) it works well. But under Nhibernate 5.2 and greater, you got an error: ": A GeometryType column has been declared, but there is no spatial dialect configured"

[peetw]

Hi @EnemyArea thanks for reporting this. I've confirmed that this issue occurs when running the NHibernate.Spatial test cases too.

It appears to be due to changes made in nhibernate/nhibernate-core#1703. In particular, the following changes to the Configuration.AddDeserializedMapping method: nhibernate/nhibernate-core@5dae5f1#diff-68f5f80acf91dec19fb1198a5985ef63

Those changes mean that the specified Dialect is no longer initialized when configuring the mappings, which means that SpatialDialect.LastInstantiated is null when the GeometryType class is instantiated, thus resulting in the exception described above.

@hazzik any thoughts on this? The remarks on the SpatialDialect.LastInstantiated field suggest something could be done, but I'm not quite sure how to interpret them...

As a workaround for now, I'll release a new 5.1.1 version of NHibernate.Spatial that limits the NHibernate version dependency to 5.1.x.

[hazzik]

Sorry, I've missed the notification.

[hazzik]

@fredericDelaporte can you take a look? I tried to make customUserType of GeometryType lazy, but it did not help as CustomType accesses SqlTypes immediately after load.

[fredericDelaporte]

I will try to have a look tomorrow. My current understanding of this is that NHibernate somewhat lacks a feature required by Spatial, and that a hack was used by Spatial to work around this lack. But this hack is broken with latest NHibernate.

If someone could PR a test case of that hack on NHibernate side, it would surely help investigating what can/should be done. Preferably Spatial contributors should also submit a new feature issue on NHibernate side for asking for the proper feature which would allow avoiding hacking around its lack. (Unless there is already such a pending issue, but then please point us to it.)

[fredericDelaporte]

I propose nhibernate/nhibernate-core#2011 for allowing restoring the Spatial hack.

Still I think Spatial contributors should ask or contribute some better way of handling this situation.

It could be something like adding an IConfigurable interface with a void Configure(IDictionary<string, string> settings) method. When a custom type implements it, this method would be called upon creation, with the dictionary of NHibernate settings.

This would allow the GeometryType to instantiate its dialect rather than relying on the hack. (It would just have to call Dialect.GetDialect(settings).)

[peetw]

First of all, thank you both for looking into this - much appreciated! I was away for the weekend so apologies for not replying sooner. I should have some time today to test whether the PR fixes the issue.

Still I think Spatial contributors should ask or contribute some better way of handling this situation.

It could be something like adding an IConfigurable interface with a void Configure(IDictionary<string, string> settings) method. When a custom type implements it, this method would be called upon creation, with the dictionary of NHibernate settings.

This would allow the GeometryType to instantiate its dialect rather than relying on the hack. (It would just have to call Dialect.GetDialect(settings).)

I agree - I'm not too familiar with the current implementation in NHibernate.Spatial (it was before my time), but judging by the comments in the source code, it definitely seems like it was meant to be a temporary "kludge". I will try to find some time to implement something along the lines of what you propose and see if it works.

[EnemyArea]

Thank you guys! Great job!

[andrerav]

Great job @peetw, thank you very much! I did a smoke test with a few of my current projects, and everything seems to run fine so far :)