Fixes U4-9456, ensures that property sets are mapped to a content version instead of an Id, adds more unit tests, ensures that if this warning ever occurs in a test that the test will fail.

This commit is contained in:
Shannon
2017-02-24 13:51:14 +11:00
parent 266f82b6c5
commit 28f0ab1001
6 changed files with 99 additions and 16 deletions

View File

@@ -1039,7 +1039,7 @@ ORDER BY doc2.updateDate DESC
if (def.DocumentDto.TemplateId.HasValue)
templates.TryGetValue(def.DocumentDto.TemplateId.Value, out template); // else null
cc.Template = template;
cc.Properties = propertyData[cc.Id];
cc.Properties = propertyData[cc.Version];
//on initial construction we don't want to have dirty properties tracked
// http://issues.umbraco.org/issue/U4-1946
@@ -1071,7 +1071,7 @@ ORDER BY doc2.updateDate DESC
var properties = GetPropertyCollection(docSql, new[] { docDef });
content.Properties = properties[dto.NodeId];
content.Properties = properties[dto.VersionId];
//on initial construction we don't want to have dirty properties tracked
// http://issues.umbraco.org/issue/U4-1946

View File

@@ -211,7 +211,7 @@ namespace Umbraco.Core.Persistence.Repositories
// assign property data
foreach (var cc in content)
{
cc.Properties = propertyData[cc.Id];
cc.Properties = propertyData[cc.Version];
//on initial construction we don't want to have dirty properties tracked
// http://issues.umbraco.org/issue/U4-1946
@@ -529,7 +529,7 @@ namespace Umbraco.Core.Persistence.Repositories
var properties = GetPropertyCollection(new PagingSqlQuery(docSql), new[] { docDef });
media.Properties = properties[dto.NodeId];
media.Properties = properties[dto.VersionId];
//on initial construction we don't want to have dirty properties tracked
// http://issues.umbraco.org/issue/U4-1946

View File

@@ -452,7 +452,7 @@ namespace Umbraco.Core.Persistence.Repositories
var properties = GetPropertyCollection(new PagingSqlQuery(sql), new[] { new DocumentDefinition(dto.ContentVersionDto, memberType) });
member.Properties = properties[dto.NodeId];
member.Properties = properties[dto.ContentVersionDto.VersionId];
//on initial construction we don't want to have dirty properties tracked
// http://issues.umbraco.org/issue/U4-1946
@@ -690,7 +690,9 @@ namespace Umbraco.Core.Persistence.Repositories
if (withCache)
{
var cached = RuntimeCache.GetCacheItem<IMember>(GetCacheIdKey<IMember>(dto.NodeId));
if (cached != null)
//only use this cached version if the dto returned is the same version - this is just a safety check, members dont
//store different versions, but just in case someone corrupts some data we'll double check to be sure.
if (cached != null && cached.Version == dto.ContentVersionDto.VersionId)
{
content.Add(cached);
continue;
@@ -714,7 +716,7 @@ namespace Umbraco.Core.Persistence.Repositories
// assign property data
foreach (var cc in content)
{
cc.Properties = propertyData[cc.Id];
cc.Properties = propertyData[cc.Version];
//on initial construction we don't want to have dirty properties tracked
// http://issues.umbraco.org/issue/U4-1946
@@ -741,7 +743,7 @@ namespace Umbraco.Core.Persistence.Repositories
var properties = GetPropertyCollection(docSql, new[] { docDef });
member.Properties = properties[dto.ContentVersionDto.NodeId];
member.Properties = properties[dto.ContentVersionDto.VersionId];
//on initial construction we don't want to have dirty properties tracked
// http://issues.umbraco.org/issue/U4-1946

View File

@@ -32,6 +32,11 @@ namespace Umbraco.Core.Persistence.Repositories
{
private readonly IContentSection _contentSection;
/// <summary>
/// This is used for unit tests ONLY
/// </summary>
internal static bool ThrowOnWarning = false;
protected VersionableRepositoryBase(IDatabaseUnitOfWork work, CacheHelper cache, ILogger logger, ISqlSyntaxProvider sqlSyntax, IContentSection contentSection)
: base(work, cache, logger, sqlSyntax)
{
@@ -502,7 +507,7 @@ namespace Umbraco.Core.Persistence.Repositories
/// <param name="sql"></param>
/// <param name="documentDefs"></param>
/// <returns></returns>
protected IDictionary<int, PropertyCollection> GetPropertyCollection(
protected IDictionary<Guid, PropertyCollection> GetPropertyCollection(
Sql sql,
IReadOnlyCollection<DocumentDefinition> documentDefs)
{
@@ -515,11 +520,11 @@ namespace Umbraco.Core.Persistence.Repositories
/// <param name="pagingSqlQuery"></param>
/// <param name="documentDefs"></param>
/// <returns></returns>
protected IDictionary<int, PropertyCollection> GetPropertyCollection(
protected IDictionary<Guid, PropertyCollection> GetPropertyCollection(
PagingSqlQuery pagingSqlQuery,
IReadOnlyCollection<DocumentDefinition> documentDefs)
{
if (documentDefs.Count == 0) return new Dictionary<int, PropertyCollection>();
if (documentDefs.Count == 0) return new Dictionary<Guid, PropertyCollection>();
//initialize to the query passed in
var docSql = pagingSqlQuery.PrePagedSql;
@@ -584,7 +589,7 @@ ORDER BY contentNodeId, propertytypeid
// from SQL server otherwise we'll get an exception.
var allPropertyData = Database.Query<PropertyDataDto>(propSql);
var result = new Dictionary<int, PropertyCollection>();
var result = new Dictionary<Guid, PropertyCollection>();
var propertiesWithTagSupport = new Dictionary<string, SupportTagsAttribute>();
//used to track the resolved composition property types per content type so we don't have to re-resolve (ToArray) the list every time
var resolvedCompositionProperties = new Dictionary<int, PropertyType[]>();
@@ -661,11 +666,19 @@ ORDER BY contentNodeId, propertytypeid
}
}
if (result.ContainsKey(def.Id))
if (result.ContainsKey(def.Version))
{
Logger.Warn<VersionableRepositoryBase<TId, TEntity>>("The query returned multiple property sets for document definition " + def.Id + ", " + def.Composition.Name);
var msg = string.Format("The query returned multiple property sets for document definition {0}, {1}, {2}", def.Id, def.Version, def.Composition.Name);
if (ThrowOnWarning)
{
throw new InvalidOperationException(msg);
}
else
{
Logger.Warn<VersionableRepositoryBase<TId, TEntity>>(msg);
}
}
result[def.Id] = new PropertyCollection(properties);
result[def.Version] = new PropertyCollection(properties);
}
}
finally

View File

@@ -36,11 +36,15 @@ namespace Umbraco.Tests.Persistence.Repositories
base.Initialize();
CreateTestData();
VersionableRepositoryBase<int, IContent>.ThrowOnWarning = true;
}
[TearDown]
public override void TearDown()
{
VersionableRepositoryBase<int, IContent>.ThrowOnWarning = false;
base.TearDown();
}
@@ -66,7 +70,61 @@ namespace Umbraco.Tests.Persistence.Repositories
var repository = new ContentRepository(unitOfWork, CacheHelper, Logger, SqlSyntax, contentTypeRepository, templateRepository, tagRepository, Mock.Of<IContentSection>());
return repository;
}
[Test]
public void Get_Always_Returns_Latest_Version()
{
// Arrange
var provider = new PetaPocoUnitOfWorkProvider(Logger);
var unitOfWork = provider.GetUnitOfWork();
ContentTypeRepository contentTypeRepository;
IContent content1;
var versions = new List<Guid>();
using (var repository = CreateRepository(unitOfWork, out contentTypeRepository))
{
var hasPropertiesContentType = MockedContentTypes.CreateSimpleContentType("umbTextpage1", "Textpage");
content1 = MockedContent.CreateSimpleContent(hasPropertiesContentType);
//save version
contentTypeRepository.AddOrUpdate(hasPropertiesContentType);
repository.AddOrUpdate(content1);
unitOfWork.Commit();
versions.Add(content1.Version);
//publish version
content1.ChangePublishedState(PublishedState.Published);
repository.AddOrUpdate(content1);
unitOfWork.Commit();
versions.Add(content1.Version);
//change something and make a pending version
content1.Name = "new name";
content1.ChangePublishedState(PublishedState.Saved);
repository.AddOrUpdate(content1);
unitOfWork.Commit();
versions.Add(content1.Version);
}
// Assert
Assert.AreEqual(3, versions.Distinct().Count());
using (var repository = CreateRepository(unitOfWork, out contentTypeRepository))
{
var content = repository.GetByQuery(new Query<IContent>().Where(c => c.Id == content1.Id)).ToArray()[0];
Assert.AreEqual(versions[2], content.Version);
content = repository.Get(content1.Id);
Assert.AreEqual(versions[2], content.Version);
foreach (var version in versions)
{
content = repository.GetByVersion(version);
Assert.IsNotNull(content);
Assert.AreEqual(version, content.Version);
}
}
}
[Test]
public void Deal_With_Corrupt_Duplicate_Newest_Published_Flags()
{
@@ -120,11 +178,17 @@ namespace Umbraco.Tests.Persistence.Repositories
{
var content = repository.GetByQuery(new Query<IContent>().Where(c => c.Id == content1.Id)).ToArray();
Assert.AreEqual(1, content.Length);
Assert.AreEqual(content[0].Version, versionDtos.Single(x => x.Id == versionDtos.Max(y => y.Id)).VersionId);
Assert.AreEqual(content[0].UpdateDate.ToString(CultureInfo.InvariantCulture), versionDtos.Single(x => x.Id == versionDtos.Max(y => y.Id)).VersionDate.ToString(CultureInfo.InvariantCulture));
var contentItem = repository.GetByVersion(content1.Version);
Assert.IsNotNull(contentItem);
contentItem = repository.Get(content1.Id);
Assert.IsNotNull(contentItem);
Assert.AreEqual(contentItem.UpdateDate.ToString(CultureInfo.InvariantCulture), versionDtos.Single(x => x.Id == versionDtos.Max(y => y.Id)).VersionDate.ToString(CultureInfo.InvariantCulture));
Assert.AreEqual(contentItem.Version, versionDtos.Single(x => x.Id == versionDtos.Max(y => y.Id)).VersionId);
var allVersions = repository.GetAllVersions(content[0].Id);
var allKnownVersions = versionDtos.Select(x => x.VersionId).Union(new[]{ content1.Version }).ToArray();
Assert.IsTrue(allKnownVersions.ContainsAll(allVersions.Select(x => x.Version)));

View File

@@ -36,11 +36,15 @@ namespace Umbraco.Tests.Services
public override void Initialize()
{
base.Initialize();
VersionableRepositoryBase<int, IContent>.ThrowOnWarning = true;
}
[TearDown]
public override void TearDown()
{
VersionableRepositoryBase<int, IContent>.ThrowOnWarning = false;
base.TearDown();
}