Skip to content

fixup! IQueryCache constraint should be an IQueryCacheFactory constraint #1

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

Merged
merged 1 commit into from
Sep 17, 2018
Merged

fixup! IQueryCache constraint should be an IQueryCacheFactory constraint #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 6 additions & 0 deletions src/NHibernate.Test/CfgTest/Loquacious/ConfigurationFixture.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,12 @@ public void CompleteConfiguration()
Assert.That(cfg.Properties[Environment.MaxFetchDepth], Is.EqualTo("11"));
Assert.That(cfg.Properties[Environment.QuerySubstitutions], Is.EqualTo("true 1, false 0, yes 'Y', no 'N'"));
Assert.That(cfg.Properties[Environment.Hbm2ddlAuto], Is.EqualTo("validate"));

// Keywords import and auto-validation require a valid connection string, disable them before checking
// the session factory can be built.
cfg.SetProperty(Environment.Hbm2ddlKeyWords, "none");
cfg.SetProperty(Environment.Hbm2ddlAuto, null);
Assert.That(() => cfg.BuildSessionFactory().Dispose(), Throws.Nothing);
}

[Test]
Expand Down
9 changes: 8 additions & 1 deletion src/NHibernate.Test/CfgTest/Loquacious/LambdaConfigurationFixture.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using NHibernate.Bytecode;
using NHibernate.Cache;
using NHibernate.Cfg;
using NHibernate.Cfg.Loquacious;
using NHibernate.Dialect;
using NHibernate.Driver;
using NHibernate.Hql.Ast.ANTLR;
Expand All @@ -27,7 +28,7 @@ public void FullConfiguration()
c.DefaultExpiration = 15;
c.RegionsPrefix = "xyz";
c.Provider<HashtableCacheProvider>();
c.QueryCache<StandardQueryCacheFactory>();
c.QueryCacheFactory<StandardQueryCacheFactory>();
});
configure.CollectionTypeFactory<DefaultCollectionTypeFactory>();
configure.HqlQueryTranslator<ASTQueryTranslatorFactory>();
Expand Down Expand Up @@ -106,6 +107,12 @@ public void FullConfiguration()
Assert.That(configure.Properties[Environment.QuerySubstitutions], Is.EqualTo("true 1, false 0, yes 'Y', no 'N'"));
Assert.That(configure.Properties[Environment.Hbm2ddlAuto], Is.EqualTo("validate"));
Assert.That(configure.Properties[Environment.LinqToHqlGeneratorsRegistry], Is.EqualTo(typeof(DefaultLinqToHqlGeneratorsRegistry).AssemblyQualifiedName));

// Keywords import and auto-validation require a valid connection string, disable them before checking
// the session factory can be built.
configure.SetProperty(Environment.Hbm2ddlKeyWords, "none");
configure.SetProperty(Environment.Hbm2ddlAuto, null);
Assert.That(() => configure.BuildSessionFactory().Dispose(), Throws.Nothing);
}
}
}
15 changes: 13 additions & 2 deletions src/NHibernate/Cfg/Loquacious/CacheConfiguration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using NHibernate.Cache;

namespace NHibernate.Cfg.Loquacious
Expand Down Expand Up @@ -39,7 +40,13 @@ public void Provider<TProvider>() where TProvider : ICacheProvider
cfg.SetProperty(Environment.CacheProvider, typeof(TProvider).AssemblyQualifiedName);
}

public void QueryCache<TFactory>() where TFactory : IQueryCacheFactory
[Obsolete("This method is invalid and should not be used. Use QueryCacheFactory method instead.", true)]
public void QueryCache<TFactory>() where TFactory : IQueryCache
{
throw new InvalidOperationException("This method is invalid and should not be used. Use QueryCacheFactory method instead.");
}

public void QueryCacheFactory<TFactory>() where TFactory : IQueryCacheFactory
Copy link
Author

Choose a reason for hiding this comment

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

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.

{
UseSecondLevelCache = true;
UseQueryCache = true;
Expand Down Expand Up @@ -111,8 +118,12 @@ public QueryCacheConfiguration(CacheConfiguration cc)

#region Implementation of IQueryCacheConfiguration

public ICacheConfiguration Through<TFactory>() where TFactory : IQueryCacheFactory
// 6.0 TODO: enable constraint and remove runtime type check
public ICacheConfiguration Through<TFactory>() // where TFactory : IQueryCacheFactory
{
if (!typeof(IQueryCacheFactory).IsAssignableFrom(typeof(TFactory)))
throw new ArgumentException($"{nameof(TFactory)} must be an {nameof(IQueryCacheFactory)}", nameof(TFactory));

cc.Configuration.SetProperty(Environment.UseSecondLevelCache, "true");
cc.Configuration.SetProperty(Environment.UseQueryCache, "true");
cc.Configuration.SetProperty(Environment.QueryCacheFactory, typeof(TFactory).AssemblyQualifiedName);
Expand Down
17 changes: 16 additions & 1 deletion src/NHibernate/Cfg/Loquacious/ICacheConfiguration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
using System;
using NHibernate.Cache;
using NHibernate.Util;

namespace NHibernate.Cfg.Loquacious
{
public interface ICacheConfiguration
Expand All @@ -17,6 +20,18 @@ public interface ICacheConfigurationProperties
string RegionsPrefix { set; }
int DefaultExpiration { set; }
void Provider<TProvider>() where TProvider : ICacheProvider;
void QueryCache<TFactory>() where TFactory : IQueryCacheFactory;
[Obsolete("This method is invalid and should not be used. Use the QueryCacheFactory extension method instead.", true)]
void QueryCache<TFactory>() where TFactory : IQueryCache;
}

// 6.0 TODO: merge into ICacheConfigurationProperties
public static class CacheConfigurationPropertiesExtensions
{
public static void QueryCacheFactory<TFactory>(this ICacheConfigurationProperties config) where TFactory : IQueryCacheFactory
{
ReflectHelper
.CastOrThrow<CacheConfigurationProperties>(config, "Setting the query cache factory with Loquacious")
.QueryCacheFactory<TFactory>();
Copy link
Author

Choose a reason for hiding this comment

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

@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.)

}
}
}
5 changes: 2 additions & 3 deletions src/NHibernate/Cfg/Loquacious/IQueryCacheConfiguration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
using NHibernate.Cache;

namespace NHibernate.Cfg.Loquacious
{
public interface IQueryCacheConfiguration
{
ICacheConfiguration Through<TFactory>() where TFactory : IQueryCacheFactory;
// 6.0 TODO: enable constraint
ICacheConfiguration Through<TFactory>(); // where TFactory : IQueryCacheFactory;
Copy link
Author

Choose a reason for hiding this comment

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

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.

}
}