diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs index 33dce9be3b..cd1793f4d5 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -273,12 +273,13 @@ namespace Umbraco.Core.Persistence.Repositories var sqlFull = translate(GetBaseQuery(BaseQueryType.FullMultiple)); var sqlIds = translate(GetBaseQuery(BaseQueryType.Ids)); - return ProcessQuery(sqlFull, new PagingSqlQuery(sqlIds), true); + return ProcessQuery(sqlFull, new PagingSqlQuery(sqlIds), true, includeAllVersions:true); } public override IContent GetByVersion(Guid versionId) { var sql = GetBaseQuery(BaseQueryType.FullSingle); + //TODO: cmsContentVersion.VersionId has a Unique Index constraint applied, seems silly then to also add OrderByDescending since it would be impossible to return more than one. sql.Where("cmsContentVersion.VersionId = @VersionId", new { VersionId = versionId }); sql.OrderByDescending(x => x.VersionDate, SqlSyntax); @@ -297,7 +298,8 @@ namespace Umbraco.Core.Persistence.Repositories var sql = new Sql() .Select("*") .From(SqlSyntax) - .InnerJoin(SqlSyntax).On(SqlSyntax, left => left.VersionId, right => right.VersionId) + .InnerJoin(SqlSyntax) + .On(SqlSyntax, left => left.VersionId, right => right.VersionId) .Where(x => x.VersionId == versionId, SqlSyntax) .Where(x => x.Newest != true, SqlSyntax); var dto = Database.Fetch(sql).FirstOrDefault(); @@ -317,7 +319,8 @@ namespace Umbraco.Core.Persistence.Repositories var sql = new Sql() .Select("*") .From() - .InnerJoin().On(left => left.VersionId, right => right.VersionId) + .InnerJoin() + .On(left => left.VersionId, right => right.VersionId) .Where(x => x.NodeId == id) .Where(x => x.VersionDate < versionDate) .Where(x => x.Newest != true); @@ -897,7 +900,7 @@ order by umbracoNode.{2}, umbracoNode.parentID, umbracoNode.sortOrder", return base.GetDatabaseFieldNameForOrderBy(orderBy); } - + /// /// This is the underlying method that processes most queries for this repository /// @@ -908,8 +911,12 @@ order by umbracoNode.{2}, umbracoNode.parentID, umbracoNode.sortOrder", /// The Id SQL without the outer join to just return all document ids - used to process the properties for the content item /// /// + /// + /// Generally when querying for content we only want to return the most recent version of the content item, however in some cases like when + /// we want to return all versions of a content item, we can't simply return the latest + /// /// - private IEnumerable ProcessQuery(Sql sqlFull, PagingSqlQuery pagingSqlQuery, bool withCache = false) + private IEnumerable ProcessQuery(Sql sqlFull, PagingSqlQuery pagingSqlQuery, bool withCache = false, bool includeAllVersions = false) { // fetch returns a list so it's ok to iterate it in this method var dtos = Database.Fetch(sqlFull); @@ -952,7 +959,7 @@ ORDER BY doc2.updateDate DESC var content = new List(); - var defs = new DocumentDefinitionCollection(); + var defs = new DocumentDefinitionCollection(includeAllVersions); var templateIds = new List(); //track the looked up content types, even though the content types are cached @@ -969,9 +976,8 @@ ORDER BY doc2.updateDate DESC if (withCache) { var cached = RuntimeCache.GetCacheItem(GetCacheIdKey(dto.NodeId)); - //only use this cached version if the dto returned is also the publish version, they must match - //TODO: Shouldn't this also match on version!? - if (cached != null && cached.Published && dto.Published) + //only use this cached version if the dto returned is also the publish version, they must match and be teh same version + if (cached != null && cached.Version == dto.VersionId && cached.Published && dto.Published) { content.Add(cached); continue; @@ -991,16 +997,30 @@ ORDER BY doc2.updateDate DESC contentType = _contentTypeRepository.Get(dto.ContentVersionDto.ContentDto.ContentTypeId); contentTypes[dto.ContentVersionDto.ContentDto.ContentTypeId] = contentType; } - - // track the definition and if it's successfully added or updated then processed - if (defs.AddOrUpdate(new DocumentDefinition(dto, contentType))) + + if (includeAllVersions) { - // need template + // track the definition + defs.Add(new DocumentDefinition(dto, contentType)); + + // assign template if (dto.TemplateId.HasValue && dto.TemplateId.Value > 0) templateIds.Add(dto.TemplateId.Value); content.Add(ContentFactory.BuildEntity(dto, contentType, publishedDto)); } + else + { + // track the definition and if it's successfully added or updated then processed + if (defs.AddOrUpdate(new DocumentDefinition(dto, contentType))) + { + // assign template + if (dto.TemplateId.HasValue && dto.TemplateId.Value > 0) + templateIds.Add(dto.TemplateId.Value); + + content.Add(ContentFactory.BuildEntity(dto, contentType, publishedDto)); + } + } } // load all required templates in 1 query @@ -1013,7 +1033,7 @@ ORDER BY doc2.updateDate DESC // assign template and property data foreach (var cc in content) { - var def = defs[cc.Id]; + var def = defs[includeAllVersions ? (ValueType)cc.Version : cc.Id]; ITemplate template = null; if (def.DocumentDto.TemplateId.HasValue) diff --git a/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs b/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs index 3f0d1d2486..272ea4334e 100644 --- a/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs @@ -175,8 +175,9 @@ namespace Umbraco.Core.Persistence.Repositories if (withCache) { var cached = RuntimeCache.GetCacheItem(GetCacheIdKey(dto.NodeId)); - //TODO: Shouldn't this also match on version!? - if (cached != null) + //only use this cached version if the dto returned is the same version - this is just a safety check, media doesn't + //store different versions, but just in case someone corrupts some data we'll double check to be sure. + if (cached != null && cached.Version == dto.VersionId) { content.Add(cached); continue; diff --git a/src/Umbraco.Core/Persistence/Repositories/MemberRepository.cs b/src/Umbraco.Core/Persistence/Repositories/MemberRepository.cs index 23b42108bc..f81424c99d 100644 --- a/src/Umbraco.Core/Persistence/Repositories/MemberRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/MemberRepository.cs @@ -690,7 +690,6 @@ namespace Umbraco.Core.Persistence.Repositories if (withCache) { var cached = RuntimeCache.GetCacheItem(GetCacheIdKey(dto.NodeId)); - //TODO: Shouldn't this also match on version!? if (cached != null) { content.Add(cached); diff --git a/src/Umbraco.Core/Persistence/Repositories/VersionableRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/VersionableRepositoryBase.cs index 0d9d0bf3d2..f96b49c1db 100644 --- a/src/Umbraco.Core/Persistence/Repositories/VersionableRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/VersionableRepositoryBase.cs @@ -773,11 +773,22 @@ ORDER BY contentNodeId, propertytypeid /// protected abstract Sql GetBaseQuery(BaseQueryType queryType); - internal class DocumentDefinitionCollection : KeyedCollection + internal class DocumentDefinitionCollection : KeyedCollection { - protected override int GetKeyForItem(DocumentDefinition item) + private readonly bool _includeAllVersions; + + /// + /// Constructor specifying if all versions should be allowed, in that case the key for the collection becomes the versionId (GUID) + /// + /// + public DocumentDefinitionCollection(bool includeAllVersions = false) { - return item.Id; + _includeAllVersions = includeAllVersions; + } + + protected override ValueType GetKeyForItem(DocumentDefinition item) + { + return _includeAllVersions ? (ValueType)item.Version : item.Id; } /// @@ -817,7 +828,7 @@ ORDER BY contentNodeId, propertytypeid return true; } - public bool TryGetValue(int key, out DocumentDefinition val) + public bool TryGetValue(ValueType key, out DocumentDefinition val) { if (Dictionary == null) { diff --git a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs index cf2347c9e9..04f8944232 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs @@ -121,6 +121,14 @@ namespace Umbraco.Tests.Persistence.Repositories var content = repository.GetByQuery(new Query().Where(c => c.Id == content1.Id)).ToArray(); Assert.AreEqual(1, content.Length); 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); + + 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))); + Assert.IsTrue(allVersions.Select(x => x.Version).ContainsAll(allKnownVersions)); } }