Skip to content

Commit 70d527c

Browse files
Disable append-hashcode when not built for netFx
Under .Net Core, hashcodes are not stable, causing distributed cache to be segregated by process
1 parent d19bfe7 commit 70d527c

File tree

6 files changed

+101
-34
lines changed

6 files changed

+101
-34
lines changed

CoreDistributedCache/NHibernate.Caches.CoreDistributedCache.Tests/CoreDistributedCacheProviderFixture.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ public void TestExpiration()
6868
[Test]
6969
public void TestAppendHashcodeToKey()
7070
{
71+
#if NETFX
7172
Assert.That(CoreDistributedCacheProvider.AppendHashcodeToKey, Is.True, "Default is not true");
7273

7374
var cache = DefaultProvider.BuildCache("foo", null) as CoreDistributedCache;
@@ -83,6 +84,23 @@ public void TestAppendHashcodeToKey()
8384
{
8485
CoreDistributedCacheProvider.AppendHashcodeToKey = true;
8586
}
87+
#else
88+
Assert.That(CoreDistributedCacheProvider.AppendHashcodeToKey, Is.False, "Default is not false");
89+
90+
var cache = DefaultProvider.BuildCache("foo", null) as CoreDistributedCache;
91+
Assert.That(cache.AppendHashcodeToKey, Is.False, "First built cache not correctly set");
92+
93+
CoreDistributedCacheProvider.AppendHashcodeToKey = true;
94+
try
95+
{
96+
cache = DefaultProvider.BuildCache("foo", null) as CoreDistributedCache;
97+
Assert.That(cache.AppendHashcodeToKey, Is.True, "Second built cache not correctly set");
98+
}
99+
finally
100+
{
101+
CoreDistributedCacheProvider.AppendHashcodeToKey = false;
102+
}
103+
#endif
86104
}
87105
}
88106
}

CoreDistributedCache/NHibernate.Caches.CoreDistributedCache.Tests/CoreDistributedCacheSectionHandlerFixture.cs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,23 @@ public void TestGetConfigNullSection()
4848
Assert.That(config.Properties.Count, Is.EqualTo(0), "Properties count");
4949
Assert.That(config.Regions, Is.Not.Null, "Regions");
5050
Assert.That(config.Regions.Length, Is.EqualTo(0));
51-
Assert.That(config.AppendHashcodeToKey, Is.True);
51+
Assert.That(config.AppendHashcodeToKey,
52+
#if NETFX
53+
Is.True
54+
#else
55+
Is.False
56+
#endif
57+
);
5258
}
5359

5460
[Test]
5561
public void TestGetConfigFromFile()
5662
{
57-
const string xmlSimple = "<coredistributedcache factory-class=\"factory1\" append-hashcode=\"false\"><properties><property name=\"prop1\">Value1</property></properties><cache region=\"foo\" expiration=\"500\" sliding=\"true\" /></coredistributedcache>";
63+
const string xmlSimple =
64+
"<coredistributedcache factory-class=\"factory1\" append-hashcode=\"false\">" +
65+
"<properties><property name=\"prop1\">Value1</property></properties>" +
66+
"<cache region=\"foo\" expiration=\"500\" sliding=\"true\" append-hashcode=\"true\" />" +
67+
"</coredistributedcache>";
5868

5969
var handler = new CoreDistributedCacheSectionHandler();
6070
var section = GetConfigurationSection(xmlSimple);
@@ -77,6 +87,8 @@ public void TestGetConfigFromFile()
7787
Assert.That(config.Regions[0].Properties["cache.use_sliding_expiration"], Is.EqualTo("true"));
7888
Assert.That(config.Regions[0].Properties, Does.ContainKey("expiration"));
7989
Assert.That(config.Regions[0].Properties["expiration"], Is.EqualTo("500"));
90+
Assert.That(config.Regions[0].Properties, Does.ContainKey("cache.use_sliding_expiration"));
91+
Assert.That(config.Regions[0].Properties["cache.use_sliding_expiration"], Is.EqualTo("true"));
8092

8193
Assert.That(config.AppendHashcodeToKey, Is.False);
8294
}

CoreDistributedCache/NHibernate.Caches.CoreDistributedCache/CacheConfig.cs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,7 @@ public CacheConfig(
4646

4747
/// <summary>Should the keys be appended with their hashcode?</summary>
4848
/// <remarks>This option is a workaround for distinguishing composite-id missing an
49-
/// <see cref="object.ToString"/> override. It may causes trouble if the cache is shared
50-
/// between processes running different runtimes.</remarks>
49+
/// <see cref="object.ToString"/> override.</remarks>
5150
public bool AppendHashcodeToKey { get; }
5251

5352
/// <summary>The configured cache regions.</summary>
@@ -68,14 +67,29 @@ public class RegionConfig
6867
/// <param name="region">The configured cache region.</param>
6968
/// <param name="expiration">The expiration for the region.</param>
7069
/// <param name="sliding">Whether the expiration should be sliding or not.</param>
71-
public RegionConfig(string region, string expiration, string sliding)
70+
// Since 5.5
71+
[Obsolete("Use overload with appendHashcodeToKey additional parameter")]
72+
public RegionConfig(string region, string expiration, string sliding) : this(region, expiration, sliding, null)
73+
{
74+
}
75+
76+
/// <summary>
77+
/// Build a cache region configuration.
78+
/// </summary>
79+
/// <param name="region">The configured cache region.</param>
80+
/// <param name="expiration">The expiration for the region.</param>
81+
/// <param name="sliding">Whether the expiration should be sliding or not.</param>
82+
/// <param name="appendHashcodeToKey">Should the keys be appended with their hashcode?</param>
83+
public RegionConfig(string region, string expiration, string sliding, string appendHashcodeToKey)
7284
{
7385
Region = region;
7486
Properties = new Dictionary<string, string>();
7587
if (!string.IsNullOrEmpty(expiration))
7688
Properties["expiration"] = expiration;
7789
if (!string.IsNullOrEmpty(sliding))
7890
Properties["cache.use_sliding_expiration"] = sliding;
91+
if (!string.IsNullOrEmpty(appendHashcodeToKey))
92+
Properties["cache.append_hashcode_to_key"] = appendHashcodeToKey;
7993
}
8094

8195
/// <summary>The region name.</summary>

CoreDistributedCache/NHibernate.Caches.CoreDistributedCache/CoreDistributedCache.cs

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ public partial class CoreDistributedCache : ICache
4444

4545
private static readonly TimeSpan DefaultExpiration = TimeSpan.FromSeconds(300);
4646
private const bool _defaultUseSlidingExpiration = false;
47+
private const bool _defaultAppendHashcodeToKey = false;
4748
private static readonly string DefaultRegionPrefix = string.Empty;
4849
private const string _cacheKeyPrefix = "NHibernate-Cache:";
4950

@@ -96,11 +97,20 @@ public CoreDistributedCache(
9697
public bool UseSlidingExpiration { get; private set; }
9798

9899
/// <summary>Should the keys be appended with their hashcode?</summary>
99-
/// <remarks>This option is a workaround for distinguishing composite-id missing an
100+
/// <remarks>
101+
/// <para>
102+
/// This option is a workaround for distinguishing composite-id missing an
100103
/// <see cref="object.ToString"/> override. It may causes trouble if the cache is shared
101-
/// between processes running different runtimes. Configure it through
102-
/// <see cref="CoreDistributedCacheProvider.AppendHashcodeToKey"/>.</remarks>
103-
public bool AppendHashcodeToKey { get; internal set; }
104+
/// between processes running another runtime than .Net Framework, or with future versions
105+
/// of .Net Framework: the hascode is not guaranteed to be stable.
106+
/// </para>
107+
/// <para>
108+
/// The value of this property can be set with the attribute <c>append-hashcode</c> of the
109+
/// region configuration node, or globally through
110+
/// <see cref="CoreDistributedCacheProvider.AppendHashcodeToKey"/>.
111+
/// </para>
112+
/// </remarks>
113+
public bool AppendHashcodeToKey { get; private set; }
104114

105115
private void Configure(IDictionary<string, string> props)
106116
{
@@ -110,12 +120,14 @@ private void Configure(IDictionary<string, string> props)
110120
Log.Warn("Configuring cache with default values");
111121
Expiration = DefaultExpiration;
112122
UseSlidingExpiration = _defaultUseSlidingExpiration;
123+
AppendHashcodeToKey = _defaultAppendHashcodeToKey;
113124
}
114125
else
115126
{
116127
Expiration = GetExpiration(props);
117128
UseSlidingExpiration = GetUseSlidingExpiration(props);
118129
regionPrefix = GetRegionPrefix(props);
130+
AppendHashcodeToKey = GetAppendHashcodeToKey(props);
119131
}
120132

121133
_fullRegion = regionPrefix + RegionName;
@@ -172,6 +184,13 @@ private static bool GetUseSlidingExpiration(IDictionary<string, string> props)
172184
return sliding;
173185
}
174186

187+
private static bool GetAppendHashcodeToKey(IDictionary<string, string> props)
188+
{
189+
var append = PropertiesHelper.GetBoolean("cache.append_hashcode_to_key", props, _defaultAppendHashcodeToKey);
190+
Log.Debug("Use append hashcode to key value: {0}", append);
191+
return append;
192+
}
193+
175194
private string GetCacheKey(object key)
176195
{
177196
var baseKey = _cacheKeyPrefix + _fullRegion + ":" + key;

CoreDistributedCache/NHibernate.Caches.CoreDistributedCache/CoreDistributedCacheProvider.cs

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,18 @@ public class CoreDistributedCacheProvider : ICacheProvider
5151
public static IDistributedCacheFactory CacheFactory { get; set; }
5252

5353
/// <summary>Should the keys be appended with their hashcode?</summary>
54-
/// <value><see langword="true" /> by default.</value>
54+
/// <value>By defauly <see langword="true" /> for the .Net Framework build,
55+
/// <see langword="false" /> otherwise. This setting will be <c>false</c> by default
56+
/// for all runtime in the next major release (6.0).</value>
5557
/// <remarks>
5658
/// <para>This option is a workaround for distinguishing composite-id missing an
5759
/// <see cref="object.ToString"/> override. It may causes trouble if the cache is shared
58-
/// between processes running different runtimes.
60+
/// between processes running another runtime than .Net Framework, or with future versions
61+
/// of .Net Framework: the hascode is not guaranteed to be stable.
5962
/// </para>
6063
/// <para>
61-
/// Changes to this property affect only caches built after the change.
64+
/// Changes to this property affect only caches built after the change, and whose configuration node
65+
/// does not define their own <c>append-hashcode</c> attribute.
6266
/// </para>
6367
/// <para>
6468
/// The value of this property can be set with the attribute <c>append-hashcode</c> of the
@@ -120,28 +124,22 @@ public ICache BuildCache(string regionName, IDictionary<string, string> properti
120124
regionName = string.Empty;
121125
}
122126

127+
properties = properties != null
128+
// Duplicate it for not altering the global configuration
129+
? new Dictionary<string, string>(properties)
130+
: new Dictionary<string, string>();
131+
132+
properties["cache.append_hashcode_to_key"] = AppendHashcodeToKey.ToString();
133+
123134
if (ConfiguredCachesProperties.TryGetValue(regionName, out var configuredProperties) && configuredProperties.Count > 0)
124135
{
125-
if (properties != null)
126-
{
127-
// Duplicate it for not altering the global configuration
128-
properties = new Dictionary<string, string>(properties);
129-
foreach (var prop in configuredProperties)
130-
{
131-
properties[prop.Key] = prop.Value;
132-
}
133-
}
134-
else
136+
foreach (var prop in configuredProperties)
135137
{
136-
properties = configuredProperties;
138+
properties[prop.Key] = prop.Value;
137139
}
138140
}
139141

140142
// create cache
141-
if (properties == null)
142-
{
143-
properties = new Dictionary<string, string>(1);
144-
}
145143

146144
if (Log.IsDebugEnabled())
147145
{
@@ -159,10 +157,7 @@ public ICache BuildCache(string regionName, IDictionary<string, string> properti
159157
Log.Debug("building cache with region: {0}, properties: {1}, factory: {2}" , regionName, sb.ToString(), CacheFactory.GetType().FullName);
160158
}
161159
return
162-
new CoreDistributedCache(CacheFactory.BuildCache(), CacheFactory.Constraints, regionName, properties)
163-
{
164-
AppendHashcodeToKey = AppendHashcodeToKey
165-
};
160+
new CoreDistributedCache(CacheFactory.BuildCache(), CacheFactory.Constraints, regionName, properties);
166161
}
167162

168163
/// <inheritdoc />

CoreDistributedCache/NHibernate.Caches.CoreDistributedCache/CoreDistributedCacheSectionHandler.cs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,10 @@ public object Create(object parent, object configContext, XmlNode section)
2626
var region = node.Attributes["region"]?.Value;
2727
var expiration = node.Attributes["expiration"]?.Value;
2828
var sliding = node.Attributes["sliding"]?.Value;
29+
var appendHashcode = node.Attributes["append-hashcode"]?.Value;
2930
if (region != null)
3031
{
31-
caches.Add(new RegionConfig(region, expiration, sliding));
32+
caches.Add(new RegionConfig(region, expiration, sliding, appendHashcode));
3233
}
3334
else
3435
{
@@ -53,8 +54,16 @@ public object Create(object parent, object configContext, XmlNode section)
5354
node.OuterXml);
5455
}
5556
}
56-
var appendHashcodeToKey = !StringComparer.OrdinalIgnoreCase.Equals(
57-
section.Attributes?["append-hashcode"]?.Value, "false");
57+
58+
var appendHashcodeToKey =
59+
// 6.0 TODO: disable for all cases by default (so keep only the else code)
60+
#if NETFX
61+
!StringComparer.OrdinalIgnoreCase.Equals(
62+
section.Attributes?["append-hashcode"]?.Value, "false");
63+
#else
64+
StringComparer.OrdinalIgnoreCase.Equals(
65+
section.Attributes?["append-hashcode"]?.Value, "true");
66+
#endif
5867

5968
return new CacheConfig(factoryClass, properties, caches.ToArray(), appendHashcodeToKey);
6069
}

0 commit comments

Comments
 (0)