fixup! IQueryCache constraint should be an IQueryCacheFactory constraint #1
Conversation
Reduce breaking changes and test more thoroughly
| throw new InvalidOperationException("This method is invalid and should not be used. Use QueryCacheFactory method instead."); | ||
| } | ||
|
|
||
| public void QueryCacheFactory<TFactory>() where TFactory : IQueryCacheFactory |
There was a problem hiding this comment.
The general wording for method setting factory seems to include Factory as a suffix to the method name. So here we can dodge the overload trouble by renaming the method.
| { | ||
| ReflectHelper | ||
| .CastOrThrow<CacheConfigurationProperties>(config, "Setting the query cache factory with Loquacious") | ||
| .QueryCacheFactory<TFactory>(); |
There was a problem hiding this comment.
@InlineAsm, that is how we handle simulating addition of an interface method: only the NHibernate implementation will be supported, so long for external implementations. This is a valid stance for things not expected to be implemented outside of NHibernate itself, which is the case for the loquacious configuration. (It is likely not even injectable with custom implementations anyway.)
(For cases which are expected to be implemented outside of NHibernate, we additionally introduce for the cast a brand new interface till the next major version, to be implemented by who wants to support the feature.)
| namespace NHibernate.Cfg.Loquacious | ||
| { | ||
| public interface IQueryCacheConfiguration | ||
| { | ||
| ICacheConfiguration Through<TFactory>() where TFactory : IQueryCacheFactory; | ||
| // 6.0 TODO: enable constraint | ||
| ICacheConfiguration Through<TFactory>(); // where TFactory : IQueryCacheFactory; |
There was a problem hiding this comment.
Removing a constraint breaks compilation on implementing classes, at least for implicit implementations. I guess it is not a binary breaking change though, I do not think the IL cares for constraint.
This case is unfortunately not documented in the breaking change guidelines of core fx.
Since this interface is not supposed to be implemented by anything outside of NHibernate code itself, I think it is acceptable in a minor release.
Reduce breaking changes and test more thoroughly