U4-10289 IdkMap cache can cause the entity service to return invalid results
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
using System;
|
||||
using System.Collections.Generic;
|
||||
using System.Linq;
|
||||
using System.Threading;
|
||||
using Umbraco.Core.Models;
|
||||
using Umbraco.Core.Persistence.UnitOfWork;
|
||||
@@ -10,8 +11,8 @@ namespace Umbraco.Core.Services
|
||||
{
|
||||
private readonly IDatabaseUnitOfWorkProvider _uowProvider;
|
||||
private readonly ReaderWriterLockSlim _locker = new ReaderWriterLockSlim();
|
||||
private readonly Dictionary<int, Guid> _id2Key = new Dictionary<int, Guid>();
|
||||
private readonly Dictionary<Guid, int> _key2Id = new Dictionary<Guid, int>();
|
||||
private readonly Dictionary<Id2KeyCompositeKey, Guid> _id2Key = new Dictionary<Id2KeyCompositeKey, Guid>();
|
||||
private readonly Dictionary<Key2IdCompositeKey, int> _key2Id = new Dictionary<Key2IdCompositeKey, int>();
|
||||
|
||||
public IdkMap(IDatabaseUnitOfWorkProvider uowProvider)
|
||||
{
|
||||
@@ -23,11 +24,12 @@ namespace Umbraco.Core.Services
|
||||
|
||||
public Attempt<int> GetIdForKey(Guid key, UmbracoObjectTypes umbracoObjectType)
|
||||
{
|
||||
var compositeKey = new Key2IdCompositeKey() { Key = key, UmbracoObjectType = umbracoObjectType };
|
||||
int id;
|
||||
try
|
||||
{
|
||||
_locker.EnterReadLock();
|
||||
if (_key2Id.TryGetValue(key, out id)) return Attempt.Succeed(id);
|
||||
if (_key2Id.TryGetValue(compositeKey, out id)) return Attempt.Succeed(id);
|
||||
}
|
||||
finally
|
||||
{
|
||||
@@ -48,9 +50,10 @@ namespace Umbraco.Core.Services
|
||||
|
||||
try
|
||||
{
|
||||
var reversedCompositeKey = new Id2KeyCompositeKey { Id = id, UmbracoObjectType = umbracoObjectType };
|
||||
_locker.EnterWriteLock();
|
||||
_id2Key[id] = key;
|
||||
_key2Id[key] = id;
|
||||
_id2Key[reversedCompositeKey] = key;
|
||||
_key2Id[compositeKey] = id;
|
||||
}
|
||||
finally
|
||||
{
|
||||
@@ -73,11 +76,12 @@ namespace Umbraco.Core.Services
|
||||
|
||||
public Attempt<Guid> GetKeyForId(int id, UmbracoObjectTypes umbracoObjectType)
|
||||
{
|
||||
var compositeKey = new Id2KeyCompositeKey() {Id = id, UmbracoObjectType = umbracoObjectType};
|
||||
Guid key;
|
||||
try
|
||||
{
|
||||
_locker.EnterReadLock();
|
||||
if (_id2Key.TryGetValue(id, out key)) return Attempt.Succeed(key);
|
||||
if (_id2Key.TryGetValue(compositeKey, out key)) return Attempt.Succeed(key);
|
||||
}
|
||||
finally
|
||||
{
|
||||
@@ -98,9 +102,10 @@ namespace Umbraco.Core.Services
|
||||
|
||||
try
|
||||
{
|
||||
var reversedCompositeKey = new Key2IdCompositeKey {Key = key, UmbracoObjectType = umbracoObjectType};
|
||||
_locker.EnterWriteLock();
|
||||
_id2Key[id] = key;
|
||||
_key2Id[key] = id;
|
||||
_id2Key[compositeKey] = key;
|
||||
_key2Id[reversedCompositeKey] = id;
|
||||
}
|
||||
finally
|
||||
{
|
||||
@@ -139,10 +144,12 @@ namespace Umbraco.Core.Services
|
||||
try
|
||||
{
|
||||
_locker.EnterWriteLock();
|
||||
Guid key;
|
||||
if (_id2Key.TryGetValue(id, out key) == false) return;
|
||||
_id2Key.Remove(id);
|
||||
_key2Id.Remove(key);
|
||||
var match = _id2Key.Keys.SingleOrDefault(x => x.Id == id);
|
||||
if (match == null) return;
|
||||
var key = _id2Key[match];
|
||||
var reversedCompositeKey = new Key2IdCompositeKey { Key = key, UmbracoObjectType = match.UmbracoObjectType };
|
||||
_id2Key.Remove(match);
|
||||
_key2Id.Remove(reversedCompositeKey);
|
||||
}
|
||||
finally
|
||||
{
|
||||
@@ -156,10 +163,12 @@ namespace Umbraco.Core.Services
|
||||
try
|
||||
{
|
||||
_locker.EnterWriteLock();
|
||||
int id;
|
||||
if (_key2Id.TryGetValue(key, out id) == false) return;
|
||||
_id2Key.Remove(id);
|
||||
_key2Id.Remove(key);
|
||||
var match = _key2Id.Keys.SingleOrDefault(x => x.Key == key);
|
||||
if (match == null) return;
|
||||
var id = _key2Id[match];
|
||||
var reversedCompositeKey = new Id2KeyCompositeKey {Id = id, UmbracoObjectType = match.UmbracoObjectType};
|
||||
_id2Key.Remove(reversedCompositeKey);
|
||||
_key2Id.Remove(match);
|
||||
}
|
||||
finally
|
||||
{
|
||||
@@ -167,5 +176,17 @@ namespace Umbraco.Core.Services
|
||||
_locker.ExitWriteLock();
|
||||
}
|
||||
}
|
||||
|
||||
internal class Id2KeyCompositeKey
|
||||
{
|
||||
public int Id { get; set; }
|
||||
public UmbracoObjectTypes UmbracoObjectType { get; set; }
|
||||
}
|
||||
|
||||
internal class Key2IdCompositeKey
|
||||
{
|
||||
public Guid Key { get; set; }
|
||||
public UmbracoObjectTypes UmbracoObjectType { get; set; }
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -547,9 +547,11 @@ namespace Umbraco.Tests.Services
|
||||
public void EntityService_Cannot_Get_Key_For_Id_With_Incorrect_Object_Type()
|
||||
{
|
||||
var service = ServiceContext.EntityService;
|
||||
var result = service.GetKeyForId(1060, UmbracoObjectTypes.MediaType);
|
||||
|
||||
Assert.IsFalse(result.Success);
|
||||
var result1 = service.GetKeyForId(1060, UmbracoObjectTypes.DocumentType);
|
||||
var result2 = service.GetKeyForId(1060, UmbracoObjectTypes.MediaType);
|
||||
|
||||
Assert.IsTrue(result1.Success);
|
||||
Assert.IsFalse(result2.Success);
|
||||
}
|
||||
|
||||
[Test]
|
||||
@@ -566,9 +568,11 @@ namespace Umbraco.Tests.Services
|
||||
public void EntityService_Cannot_Get_Id_For_Key_With_Incorrect_Object_Type()
|
||||
{
|
||||
var service = ServiceContext.EntityService;
|
||||
var result = service.GetIdForKey(Guid.Parse("1D3A8E6E-2EA9-4CC1-B229-1AEE19821522"), UmbracoObjectTypes.MediaType);
|
||||
var result1 = service.GetIdForKey(Guid.Parse("1D3A8E6E-2EA9-4CC1-B229-1AEE19821522"), UmbracoObjectTypes.DocumentType);
|
||||
var result2 = service.GetIdForKey(Guid.Parse("1D3A8E6E-2EA9-4CC1-B229-1AEE19821522"), UmbracoObjectTypes.MediaType);
|
||||
|
||||
Assert.IsFalse(result.Success);
|
||||
Assert.IsTrue(result1.Success);
|
||||
Assert.IsFalse(result2.Success);
|
||||
}
|
||||
|
||||
private static bool _isSetup = false;
|
||||
|
||||
Reference in New Issue
Block a user