diff --git a/src/Umbraco.Core/Persistence/Mappers/MappingResolver.cs b/src/Umbraco.Core/Persistence/Mappers/MappingResolver.cs index 6909c77744..a4dee60ee9 100644 --- a/src/Umbraco.Core/Persistence/Mappers/MappingResolver.cs +++ b/src/Umbraco.Core/Persistence/Mappers/MappingResolver.cs @@ -31,7 +31,7 @@ namespace Umbraco.Core.Persistence.Mappers /// /// /// - internal BaseMapper ResolveMapperByType(Type type) + public virtual BaseMapper ResolveMapperByType(Type type) { return _mapperCache.GetOrAdd(type, type1 => { @@ -67,7 +67,7 @@ namespace Umbraco.Core.Persistence.Mappers return Attempt.Succeed(mapper); } - internal string GetMapping(Type type, string propertyName) + public virtual string GetMapping(Type type, string propertyName) { var mapper = ResolveMapperByType(type); var result = mapper.Map(propertyName); diff --git a/src/Umbraco.Core/Persistence/Querying/ExpressionVisitorBase.cs b/src/Umbraco.Core/Persistence/Querying/ExpressionVisitorBase.cs index 678ceb1d8e..0dc421045d 100644 --- a/src/Umbraco.Core/Persistence/Querying/ExpressionVisitorBase.cs +++ b/src/Umbraco.Core/Persistence/Querying/ExpressionVisitorBase.cs @@ -581,6 +581,18 @@ namespace Umbraco.Core.Persistence.Querying case "InvariantContains": case "InvariantEquals": + //special case, if it is 'Contains' and the argumet that Contains is being called on is + //Enumerable and the methodArgs is the actual member access, then it's an SQL IN claus + if (m.Object == null + && m.Arguments[0].Type != typeof(string) + && m.Arguments.Count == 2 + && methodArgs.Length == 1 + && methodArgs[0].NodeType == ExpressionType.MemberAccess + && TypeHelper.IsTypeAssignableFrom(m.Arguments[0].Type)) + { + goto case "SqlIn"; + } + string compareValue; if (methodArgs[0].NodeType != ExpressionType.Constant) @@ -597,13 +609,6 @@ namespace Umbraco.Core.Persistence.Querying compareValue = methodArgs[0].ToString(); } - //special case, if it is 'Contains' and the member that Contains is being called on is not a string, then - // we should be doing an 'In' clause - but we currently do not support this - if (methodArgs[0].Type != typeof(string) && TypeHelper.IsTypeAssignableFrom(methodArgs[0].Type)) - { - throw new NotSupportedException("An array Contains method is not supported"); - } - //default column type var colType = TextColumnType.NVarchar; @@ -705,29 +710,33 @@ namespace Umbraco.Core.Persistence.Querying // } // return string.Format("{0}{1}", r, s); - //case "In": + case "SqlIn": - // var member = Expression.Convert(m.Arguments[0], typeof(object)); - // var lambda = Expression.Lambda>(member); - // var getter = lambda.Compile(); + if (m.Object == null && methodArgs.Length == 1 && methodArgs[0].NodeType == ExpressionType.MemberAccess) + { + var memberAccess = VisitMemberAccess((MemberExpression) methodArgs[0]); + + var member = Expression.Convert(m.Arguments[0], typeof(object)); + var lambda = Expression.Lambda>(member); + var getter = lambda.Compile(); - // var inArgs = (object[])getter(); + var inArgs = (IEnumerable)getter(); - // var sIn = new StringBuilder(); - // foreach (var e in inArgs) - // { - // SqlParameters.Add(e); + var sIn = new StringBuilder(); + foreach (var e in inArgs) + { + SqlParameters.Add(e); - // sIn.AppendFormat("{0}{1}", - // sIn.Length > 0 ? "," : "", - // string.Format("@{0}", SqlParameters.Count - 1)); + sIn.AppendFormat("{0}{1}", + sIn.Length > 0 ? "," : "", + string.Format("@{0}", SqlParameters.Count - 1)); + } - // //sIn.AppendFormat("{0}{1}", - // // sIn.Length > 0 ? "," : "", - // // GetQuotedValue(e, e.GetType())); - // } + return string.Format("{0} IN ({1})", memberAccess, sIn); + } + + throw new NotSupportedException("SqlIn must contain the member being accessed"); - // return string.Format("{0} {1} ({2})", r, m.Method.Name, sIn.ToString()); //case "Desc": // return string.Format("{0} DESC", r); //case "Alias": diff --git a/src/Umbraco.Core/Persistence/Querying/ModelToSqlExpressionVisitor.cs b/src/Umbraco.Core/Persistence/Querying/ModelToSqlExpressionVisitor.cs index 7f5e479af6..c4029516a3 100644 --- a/src/Umbraco.Core/Persistence/Querying/ModelToSqlExpressionVisitor.cs +++ b/src/Umbraco.Core/Persistence/Querying/ModelToSqlExpressionVisitor.cs @@ -1,5 +1,7 @@ using System; +using System.Linq; using System.Linq.Expressions; +using Umbraco.Core.Models.EntityBase; using Umbraco.Core.Persistence.Mappers; using Umbraco.Core.Persistence.SqlSyntax; @@ -12,17 +14,19 @@ namespace Umbraco.Core.Persistence.Querying /// This object is stateful and cannot be re-used to parse an expression. internal class ModelToSqlExpressionVisitor : ExpressionVisitorBase { + private readonly MappingResolver _mappingResolver; private readonly BaseMapper _mapper; - public ModelToSqlExpressionVisitor(ISqlSyntaxProvider sqlSyntax, BaseMapper mapper) + public ModelToSqlExpressionVisitor(ISqlSyntaxProvider sqlSyntax, MappingResolver mappingResolver) : base(sqlSyntax) { - _mapper = mapper; + _mapper = mappingResolver.ResolveMapperByType(typeof(T)); + _mappingResolver = mappingResolver; } [Obsolete("Use the overload the specifies a SqlSyntaxProvider")] public ModelToSqlExpressionVisitor() - : this(SqlSyntaxContext.SqlSyntaxProvider, MappingResolver.Current.ResolveMapperByType(typeof(T))) + : this(SqlSyntaxContext.SqlSyntaxProvider, MappingResolver.Current) { } protected override string VisitMemberAccess(MemberExpression m) @@ -36,7 +40,7 @@ namespace Umbraco.Core.Persistence.Querying { var field = _mapper.Map(m.Member.Name, true); if (field.IsNullOrWhiteSpace()) - throw new InvalidOperationException("The mapper returned an empty field for the member name: " + m.Member.Name); + throw new InvalidOperationException(string.Format("The mapper returned an empty field for the member name: {0} for type: {1}", m.Member.Name, m.Expression.Type)); return field; } //already compiled, return @@ -50,13 +54,39 @@ namespace Umbraco.Core.Persistence.Querying { var field = _mapper.Map(m.Member.Name, true); if (field.IsNullOrWhiteSpace()) - throw new InvalidOperationException("The mapper returned an empty field for the member name: " + m.Member.Name); + throw new InvalidOperationException(string.Format("The mapper returned an empty field for the member name: {0} for type: {1}", m.Member.Name, m.Expression.Type)); return field; } //already compiled, return return string.Empty; } + if (m.Expression != null && m.Expression.Type != typeof(T) && TypeHelper.IsTypeAssignableFrom(m.Expression.Type)) + { + //if this is the case, it means we have a sub expression / nested property access, such as: x.ContentType.Alias == "Test"; + //and since the sub type (x.ContentType) is not the same as x, we need to resolve a mapper for x.ContentType to get it's mapped SQL column + + //don't execute if compiled + if (Visited == false) + { + var subMapper = _mappingResolver.ResolveMapperByType(m.Expression.Type); + if (subMapper == null) + throw new NullReferenceException("No mapper found for type " + m.Expression.Type); + var field = subMapper.Map(m.Member.Name, true); + if (field.IsNullOrWhiteSpace()) + throw new InvalidOperationException(string.Format("The mapper returned an empty field for the member name: {0} for type: {1}", m.Member.Name, m.Expression.Type)); + return field; + } + //already compiled, return + return string.Empty; + } + + //TODO: When m.Expression.NodeType == ExpressionType.Constant and it's an expression like: content => aliases.Contains(content.ContentType.Alias); + // then an SQL parameter will be added for aliases as an array, however in SqlIn on the subclass it will manually add these SqlParameters anyways, + // however the query will still execute because the SQL that is written will only contain the correct indexes of SQL parameters, this would be ignored, + // I'm just unsure right now due to time constraints how to make it correct. It won't matter right now and has been working already with this bug but I've + // only just discovered what it is actually doing. + var member = Expression.Convert(m, typeof(object)); var lambda = Expression.Lambda>(member); var getter = lambda.Compile(); diff --git a/src/Umbraco.Core/Persistence/Querying/SqlStringExtensions.cs b/src/Umbraco.Core/Persistence/Querying/SqlExpressionExtensions.cs similarity index 83% rename from src/Umbraco.Core/Persistence/Querying/SqlStringExtensions.cs rename to src/Umbraco.Core/Persistence/Querying/SqlExpressionExtensions.cs index cbecc0a591..5a0452c3aa 100644 --- a/src/Umbraco.Core/Persistence/Querying/SqlStringExtensions.cs +++ b/src/Umbraco.Core/Persistence/Querying/SqlExpressionExtensions.cs @@ -1,4 +1,6 @@ using System; +using System.Collections.Generic; +using System.Linq; using System.Text.RegularExpressions; namespace Umbraco.Core.Persistence.Querying @@ -6,8 +8,13 @@ namespace Umbraco.Core.Persistence.Querying /// /// String extension methods used specifically to translate into SQL /// - internal static class SqlStringExtensions + internal static class SqlExpressionExtensions { + public static bool SqlIn(this IEnumerable collection, T item) + { + return collection.Contains(item); + } + public static bool SqlWildcard(this string str, string txt, TextColumnType columnType) { var wildcardmatch = new Regex("^" + Regex.Escape(txt). diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs index 8488bdb65a..83c6095775 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -134,6 +134,9 @@ namespace Umbraco.Core.Persistence.Repositories .On(SqlSyntax, left => left.NodeId, right => right.NodeId) .InnerJoin(SqlSyntax) .On(SqlSyntax, left => left.NodeId, right => right.NodeId); + //TODO: IF we want to enable querying on content type information this will need to be joined + //.InnerJoin(SqlSyntax) + //.On(SqlSyntax, left => left.ContentTypeId, right => right.NodeId, SqlSyntax); if (queryType == BaseQueryType.FullSingle) { diff --git a/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs b/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs index 53b04e1322..3279571518 100644 --- a/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs @@ -94,6 +94,9 @@ namespace Umbraco.Core.Persistence.Repositories .On(SqlSyntax, left => left.NodeId, right => right.NodeId) .InnerJoin(SqlSyntax) .On(SqlSyntax, left => left.NodeId, right => right.NodeId, SqlSyntax) + //TODO: IF we want to enable querying on content type information this will need to be joined + //.InnerJoin(SqlSyntax) + //.On(SqlSyntax, left => left.ContentTypeId, right => right.NodeId, SqlSyntax); .Where(x => x.NodeObjectType == NodeObjectTypeId, SqlSyntax); return sql; } diff --git a/src/Umbraco.Core/Services/IMediaService.cs b/src/Umbraco.Core/Services/IMediaService.cs index d4218764b7..ac0ff94d3c 100644 --- a/src/Umbraco.Core/Services/IMediaService.cs +++ b/src/Umbraco.Core/Services/IMediaService.cs @@ -166,6 +166,22 @@ namespace Umbraco.Core.Services IEnumerable GetPagedChildren(int id, long pageIndex, int pageSize, out long totalRecords, string orderBy, Direction orderDirection, bool orderBySystemField, string filter); + /// + /// Gets a collection of objects by Parent Id + /// + /// Id of the Parent to retrieve Children from + /// Page number + /// Page size + /// Total records query would return without paging + /// Field to order by + /// Direction to order by + /// Flag to indicate when ordering by system field + /// Search text filter + /// A list of content type Ids to filter the list by + /// An Enumerable list of objects + IEnumerable GetPagedChildren(int id, long pageIndex, int pageSize, out long totalRecords, + string orderBy, Direction orderDirection, bool orderBySystemField, string filter, int[] contentTypeFilter); + [Obsolete("Use the overload with 'long' parameter types instead")] [EditorBrowsable(EditorBrowsableState.Never)] IEnumerable GetPagedDescendants(int id, int pageIndex, int pageSize, out int totalRecords, diff --git a/src/Umbraco.Core/Services/MediaService.cs b/src/Umbraco.Core/Services/MediaService.cs index a67ee8285d..d236dccc5a 100644 --- a/src/Umbraco.Core/Services/MediaService.cs +++ b/src/Umbraco.Core/Services/MediaService.cs @@ -20,6 +20,7 @@ using Umbraco.Core.Persistence.Repositories; using Umbraco.Core.Persistence.SqlSyntax; using Umbraco.Core.Persistence.UnitOfWork; using Umbraco.Core.Publishing; +using ContentType = System.Net.Mime.ContentType; namespace Umbraco.Core.Services { @@ -435,6 +436,25 @@ namespace Umbraco.Core.Services /// An Enumerable list of objects public IEnumerable GetPagedChildren(int id, long pageIndex, int pageSize, out long totalChildren, string orderBy, Direction orderDirection, bool orderBySystemField, string filter) + { + return GetPagedChildren(id, pageIndex, pageSize, out totalChildren, orderBy, orderDirection, true, filter, null); + } + + /// + /// Gets a collection of objects by Parent Id + /// + /// Id of the Parent to retrieve Children from + /// Page number + /// Page size + /// Total records query would return without paging + /// Field to order by + /// Direction to order by + /// Flag to indicate when ordering by system field + /// Search text filter + /// A list of content type Ids to filter the list by + /// An Enumerable list of objects + public IEnumerable GetPagedChildren(int id, long pageIndex, int pageSize, out long totalChildren, + string orderBy, Direction orderDirection, bool orderBySystemField, string filter, int[] contentTypeFilter) { Mandate.ParameterCondition(pageIndex >= 0, "pageIndex"); Mandate.ParameterCondition(pageSize > 0, "pageSize"); @@ -443,6 +463,11 @@ namespace Umbraco.Core.Services var query = Query.Builder; query.Where(x => x.ParentId == id); + if (contentTypeFilter != null && contentTypeFilter.Length > 0) + { + query.Where(x => contentTypeFilter.Contains(x.ContentTypeId)); + } + var medias = repository.GetPagedResultsByQuery(query, pageIndex, pageSize, out totalChildren, orderBy, orderDirection, orderBySystemField, filter); return medias; diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 2e8fc0fd39..4aca6bdf02 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -464,6 +464,7 @@ + @@ -649,7 +650,6 @@ - diff --git a/src/Umbraco.Tests.Benchmarks/ModelToSqlExpressionHelperBenchmarks.cs b/src/Umbraco.Tests.Benchmarks/ModelToSqlExpressionHelperBenchmarks.cs index 03aa3206ee..5fa395d2f1 100644 --- a/src/Umbraco.Tests.Benchmarks/ModelToSqlExpressionHelperBenchmarks.cs +++ b/src/Umbraco.Tests.Benchmarks/ModelToSqlExpressionHelperBenchmarks.cs @@ -16,6 +16,7 @@ using BenchmarkDotNet.Loggers; using BenchmarkDotNet.Reports; using BenchmarkDotNet.Running; using BenchmarkDotNet.Validators; +using Moq; using Umbraco.Core; using Umbraco.Core.Logging; using Umbraco.Core.Models; @@ -43,15 +44,18 @@ namespace Umbraco.Tests.Benchmarks public ModelToSqlExpressionHelperBenchmarks() { - _contentMapper = new ContentMapper(_syntaxProvider); - _contentMapper.BuildMap(); + var contentMapper = new ContentMapper(_syntaxProvider); + contentMapper.BuildMap(); _cachedExpression = new CachedExpression(); + var mappingResolver = new Mock(); + mappingResolver.Setup(resolver => resolver.ResolveMapperByType(It.IsAny())).Returns(contentMapper); + _mappingResolver = mappingResolver.Object; } private readonly ISqlSyntaxProvider _syntaxProvider = new SqlCeSyntaxProvider(); - private readonly BaseMapper _contentMapper; private readonly CachedExpression _cachedExpression; - + private readonly MappingResolver _mappingResolver; + [Benchmark(Baseline = true)] public void WithNonCached() { @@ -62,7 +66,7 @@ namespace Umbraco.Tests.Benchmarks Expression> predicate = content => content.Path.StartsWith("-1") && content.Published && (content.ContentTypeId == a || content.ContentTypeId == b); - var modelToSqlExpressionHelper = new ModelToSqlExpressionVisitor(_syntaxProvider, _contentMapper); + var modelToSqlExpressionHelper = new ModelToSqlExpressionVisitor(_syntaxProvider, _mappingResolver); var result = modelToSqlExpressionHelper.Visit(predicate); } @@ -80,7 +84,7 @@ namespace Umbraco.Tests.Benchmarks Expression> predicate = content => content.Path.StartsWith("-1") && content.Published && (content.ContentTypeId == a || content.ContentTypeId == b); - var modelToSqlExpressionHelper = new ModelToSqlExpressionVisitor(_syntaxProvider, _contentMapper); + var modelToSqlExpressionHelper = new ModelToSqlExpressionVisitor(_syntaxProvider, _mappingResolver); //wrap it! _cachedExpression.Wrap(predicate); diff --git a/src/Umbraco.Tests.Benchmarks/Umbraco.Tests.Benchmarks.csproj b/src/Umbraco.Tests.Benchmarks/Umbraco.Tests.Benchmarks.csproj index 50328b2ecf..46f9dfde10 100644 --- a/src/Umbraco.Tests.Benchmarks/Umbraco.Tests.Benchmarks.csproj +++ b/src/Umbraco.Tests.Benchmarks/Umbraco.Tests.Benchmarks.csproj @@ -49,6 +49,9 @@ ..\packages\BenchmarkDotNet.Toolchains.Roslyn.0.9.9\lib\net45\BenchmarkDotNet.Toolchains.Roslyn.dll + + ..\packages\Castle.Core.4.0.0\lib\net45\Castle.Core.dll + ..\packages\Microsoft.CodeAnalysis.Common.1.3.2\lib\net45\Microsoft.CodeAnalysis.dll @@ -58,6 +61,9 @@ ..\packages\Microsoft.Diagnostics.Tracing.TraceEvent.1.0.41\lib\net40\Microsoft.Diagnostics.Tracing.TraceEvent.dll + + ..\packages\Moq.4.7.0\lib\net45\Moq.dll + ..\packages\System.Collections.Immutable.1.1.37\lib\dotnet\System.Collections.Immutable.dll diff --git a/src/Umbraco.Tests.Benchmarks/packages.config b/src/Umbraco.Tests.Benchmarks/packages.config index c4d2ba1df2..ec400d2ae8 100644 --- a/src/Umbraco.Tests.Benchmarks/packages.config +++ b/src/Umbraco.Tests.Benchmarks/packages.config @@ -4,11 +4,13 @@ + + diff --git a/src/Umbraco.Tests/Persistence/Querying/ExpressionTests.cs b/src/Umbraco.Tests/Persistence/Querying/ExpressionTests.cs index 65a660146e..b9869005f7 100644 --- a/src/Umbraco.Tests/Persistence/Querying/ExpressionTests.cs +++ b/src/Umbraco.Tests/Persistence/Querying/ExpressionTests.cs @@ -1,5 +1,6 @@ using System; using System.Diagnostics; +using System.Linq; using System.Linq.Expressions; using Moq; using NUnit.Framework; @@ -16,19 +17,35 @@ namespace Umbraco.Tests.Persistence.Querying [TestFixture] public class ExpressionTests : BaseUsingSqlCeSyntax { - // [Test] - // public void Can_Query_With_Content_Type_Alias() - // { - // //Arrange - // Expression> predicate = content => content.ContentType.Alias == "Test"; - // var modelToSqlExpressionHelper = new ModelToSqlExpressionVisitor(); - // var result = modelToSqlExpressionHelper.Visit(predicate); + [Test] + public void Can_Query_With_Content_Type_Alias() + { + //Arrange + Expression> predicate = content => content.ContentType.Alias == "Test"; + var modelToSqlExpressionHelper = new ModelToSqlExpressionVisitor(); + var result = modelToSqlExpressionHelper.Visit(predicate); - // Debug.Print("Model to Sql ExpressionHelper: \n" + result); + Debug.Print("Model to Sql ExpressionHelper: \n" + result); - // Assert.AreEqual("[cmsContentType].[alias] = @0", result); - // Assert.AreEqual("Test", modelToSqlExpressionHelper.GetSqlParameters()[0]); - // } + Assert.AreEqual("([cmsContentType].[alias] = @0)", result); + Assert.AreEqual("Test", modelToSqlExpressionHelper.GetSqlParameters()[0]); + } + + [Test] + public void Can_Query_With_Content_Type_Aliases() + { + //Arrange + var aliases = new[] {"Test1", "Test2"}; + Expression> predicate = content => aliases.Contains(content.ContentType.Alias); + var modelToSqlExpressionHelper = new ModelToSqlExpressionVisitor(); + var result = modelToSqlExpressionHelper.Visit(predicate); + + Debug.Print("Model to Sql ExpressionHelper: \n" + result); + + Assert.AreEqual("[cmsContentType].[alias] IN (@1,@2)", result); + Assert.AreEqual("Test1", modelToSqlExpressionHelper.GetSqlParameters()[1]); + Assert.AreEqual("Test2", modelToSqlExpressionHelper.GetSqlParameters()[2]); + } [Test] public void CachedExpression_Can_Verify_Path_StartsWith_Predicate_In_Same_Result() diff --git a/src/Umbraco.Tests/Persistence/Repositories/MediaRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/MediaRepositoryTest.cs index 1566441d5a..70e72eca9b 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/MediaRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/MediaRepositoryTest.cs @@ -4,6 +4,7 @@ using System.Linq; using System.Xml.Linq; using Moq; using NUnit.Framework; +using Umbraco.Core; using Umbraco.Core.Configuration.UmbracoSettings; using Umbraco.Core.Logging; using Umbraco.Core.Models; @@ -345,6 +346,61 @@ namespace Umbraco.Tests.Persistence.Repositories } } + [Test] + public void Can_Perform_GetByQuery_On_MediaRepository_With_ContentType_Id_Filter() + { + // Arrange + var folderMediaType = ServiceContext.ContentTypeService.GetMediaType(1031); + var provider = new PetaPocoUnitOfWorkProvider(Logger); + var unitOfWork = provider.GetUnitOfWork(); + MediaTypeRepository mediaTypeRepository; + using (var repository = CreateRepository(unitOfWork, out mediaTypeRepository)) + { + // Act + for (int i = 0; i < 10; i++) + { + var folder = MockedMedia.CreateMediaFolder(folderMediaType, -1); + repository.AddOrUpdate(folder); + } + unitOfWork.Commit(); + + var types = new[] { 1031 }; + var query = Query.Builder.Where(x => types.Contains(x.ContentTypeId)); + var result = repository.GetByQuery(query); + + // Assert + Assert.That(result.Count(), Is.GreaterThanOrEqualTo(11)); + } + } + + [Ignore("We could allow this to work but it requires an extra join on the query used which currently we don't absolutely need so leaving this out for now")] + [Test] + public void Can_Perform_GetByQuery_On_MediaRepository_With_ContentType_Alias_Filter() + { + // Arrange + var folderMediaType = ServiceContext.ContentTypeService.GetMediaType(1031); + var provider = new PetaPocoUnitOfWorkProvider(Logger); + var unitOfWork = provider.GetUnitOfWork(); + MediaTypeRepository mediaTypeRepository; + using (var repository = CreateRepository(unitOfWork, out mediaTypeRepository)) + { + // Act + for (int i = 0; i < 10; i++) + { + var folder = MockedMedia.CreateMediaFolder(folderMediaType, -1); + repository.AddOrUpdate(folder); + } + unitOfWork.Commit(); + + var types = new[] { "Folder" }; + var query = Query.Builder.Where(x => types.Contains(x.ContentType.Alias)); + var result = repository.GetByQuery(query); + + // Assert + Assert.That(result.Count(), Is.GreaterThanOrEqualTo(11)); + } + } + [Test] public void Can_Perform_GetPagedResultsByQuery_ForFirstPage_On_MediaRepository() { @@ -470,7 +526,7 @@ namespace Umbraco.Tests.Persistence.Repositories Assert.That(result.First().Name, Is.EqualTo("Test File")); } } - + [Test] public void Can_Perform_GetPagedResultsByQuery_WithFilterMatchingAll_On_MediaRepository() { diff --git a/src/Umbraco.Tests/Services/MediaServiceTests.cs b/src/Umbraco.Tests/Services/MediaServiceTests.cs index 8193df911c..82738a26f4 100644 --- a/src/Umbraco.Tests/Services/MediaServiceTests.cs +++ b/src/Umbraco.Tests/Services/MediaServiceTests.cs @@ -7,6 +7,7 @@ using Umbraco.Core; using Umbraco.Core.Models; using Umbraco.Core.Models.Rdbms; using Umbraco.Core.Persistence; +using Umbraco.Core.Persistence.DatabaseModelDefinitions; using Umbraco.Core.Persistence.Repositories; using Umbraco.Core.Persistence.UnitOfWork; using Umbraco.Tests.TestHelpers; @@ -30,6 +31,33 @@ namespace Umbraco.Tests.Services base.TearDown(); } + [Test] + public void Get_Paged_Children_With_Media_Type_Filter() + { + var mediaService = ServiceContext.MediaService; + var mediaType1 = MockedContentTypes.CreateImageMediaType("Image2"); + ServiceContext.ContentTypeService.Save(mediaType1); + var mediaType2 = MockedContentTypes.CreateImageMediaType("Image3"); + ServiceContext.ContentTypeService.Save(mediaType2); + + for (int i = 0; i < 10; i++) + { + var m1 = MockedMedia.CreateMediaImage(mediaType1, -1); + mediaService.Save(m1); + var m2 = MockedMedia.CreateMediaImage(mediaType2, -1); + mediaService.Save(m2); + } + + long total; + var result = ServiceContext.MediaService.GetPagedChildren(-1, 0, 11, out total, "SortOrder", Direction.Ascending, true, null, new[] {mediaType1.Id, mediaType2.Id}); + Assert.AreEqual(11, result.Count()); + Assert.AreEqual(20, total); + + result = ServiceContext.MediaService.GetPagedChildren(-1, 1, 11, out total, "SortOrder", Direction.Ascending, true, null, new[] { mediaType1.Id, mediaType2.Id }); + Assert.AreEqual(9, result.Count()); + Assert.AreEqual(20, total); + } + [Test] public void Can_Move_Media() { diff --git a/src/Umbraco.Web.UI.Client/src/common/resources/media.resource.js b/src/Umbraco.Web.UI.Client/src/common/resources/media.resource.js index c0aee7280d..c41b7e91c7 100644 --- a/src/Umbraco.Web.UI.Client/src/common/resources/media.resource.js +++ b/src/Umbraco.Web.UI.Client/src/common/resources/media.resource.js @@ -427,7 +427,9 @@ function mediaResource($q, $http, umbDataFormatter, umbRequestHelper) { * Retrieves all media children with types used as folders. * Uses the convention of looking for media items with mediaTypes ending in * *Folder so will match "Folder", "bannerFolder", "secureFolder" etc, - * + * + * NOTE: This will return a max of 500 folders, if more is required it needs to be paged + * * ##usage *
           * mediaResource.getChildFolders(1234)
@@ -438,21 +440,22 @@ function mediaResource($q, $http, umbDataFormatter, umbRequestHelper) {
           *
           * @param {int} parentId Id of the media item to query for child folders    
           * @returns {Promise} resourcePromise object.
-          * @deprecated This method is no longer used and shouldn't be because it performs poorly when there are a lot of media items
+          *
           */
         getChildFolders: function (parentId) {
             if (!parentId) {
                 parentId = -1;
             }
 
+            //NOTE: This will return a max of 500 folders, if more is required it needs to be paged
             return umbRequestHelper.resourcePromise(
                   $http.get(
                         umbRequestHelper.getApiUrl(
                               "mediaApiBaseUrl",
                               "GetChildFolders",
-                              [
-                                    { id: parentId }
-                              ])),
+                            {
+                                id: parentId
+                            })),
                   'Failed to retrieve child folders for media item ' + parentId);
         },
 
diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/listview/listview.controller.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/listview/listview.controller.js
index 40ab5ce35f..f8887c6c44 100644
--- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/listview/listview.controller.js
+++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/listview/listview.controller.js
@@ -1,4 +1,4 @@
-function listViewController($rootScope, $scope, $routeParams, $injector, $cookieStore, mediaTypeHelper, notificationsService, iconHelper, dialogService, editorState, localizationService, $location, appState, $timeout, $q, mediaResource, listViewHelper, userService, navigationService, treeService) {
+function listViewController($rootScope, $scope, $routeParams, $injector, $cookieStore, notificationsService, iconHelper, dialogService, editorState, localizationService, $location, appState, $timeout, $q, mediaResource, listViewHelper, userService, navigationService, treeService) {
 
    //this is a quick check to see if we're in create mode, if so just exit - we cannot show children for content
    // that isn't created yet, if we continue this will use the parent id in the route params which isn't what
@@ -53,7 +53,9 @@ function listViewController($rootScope, $scope, $routeParams, $injector, $cookie
    $scope.isNew = false;
    $scope.actionInProgress = false;
    $scope.selection = [];
-   $scope.folders = [];
+   $scope.folders = [];   
+   //tracks if we've already loaded the folders for the current node
+   var foldersLoaded = false;
    $scope.listViewResultSet = {
       totalPages: 0,
       items: []
@@ -261,25 +263,25 @@ function listViewController($rootScope, $scope, $routeParams, $injector, $cookie
          $scope.actionInProgress = false;
          $scope.listViewResultSet = data;
 
-          //reset
-         $scope.folders = [];
-
-          //update all values for display
+         //update all values for display
          if ($scope.listViewResultSet.items) {
             _.each($scope.listViewResultSet.items, function (e, index) {
-                setPropertyValues(e);
-
-                //special case, we need to check if any of these types are folder types 
-                //and add them to the folders collection
-                if ($scope.entityType === 'media') {
-                    if (mediaTypeHelper.isFolderType(e)) {
-                        $scope.folders.push(e);
-                    }
-                }
+               setPropertyValues(e);
             });
          }
 
-         $scope.viewLoaded = true;
+         if (!foldersLoaded && $scope.entityType === 'media') {
+            //The folders aren't loaded - we only need to do this once since we're never changing node ids
+            mediaResource.getChildFolders($scope.contentId)
+                    .then(function (folders) {
+                       $scope.folders = folders;
+                       $scope.viewLoaded = true;
+                       foldersLoaded = true;
+                    });
+
+         } else {
+            $scope.viewLoaded = true;
+         }
 
          //NOTE: This might occur if we are requesting a higher page number than what is actually available, for example
          // if you have more than one page and you delete all items on the last page. In this case, we need to reset to the last
diff --git a/src/Umbraco.Web/Editors/MediaController.cs b/src/Umbraco.Web/Editors/MediaController.cs
index d00a26333e..2f3fe5bd94 100644
--- a/src/Umbraco.Web/Editors/MediaController.cs
+++ b/src/Umbraco.Web/Editors/MediaController.cs
@@ -167,18 +167,41 @@ namespace Umbraco.Web.Editors
         [Obsolete("This is no longer used and shouldn't be because it performs poorly when there are a lot of media items")]
         [FilterAllowedOutgoingMedia(typeof(IEnumerable>))]
         public IEnumerable> GetChildFolders(int id = -1)
+        {
+            //we are only allowing a max of 500 to be returned here, if more is required it needs to be paged
+            var result = GetChildFolders(id, 1, 500);
+            return result.Items;
+        }
+
+        /// 
+        /// Returns a paged result of media items known to be of a "Folder" type
+        /// 
+        /// 
+        /// 
+        /// 
+        /// 
+        public PagedResult> GetChildFolders(int id, int pageNumber, int pageSize)
         {
             //Suggested convention for folder mediatypes - we can make this more or less complicated as long as we document it...
             //if you create a media type, which has an alias that ends with ...Folder then its a folder: ex: "secureFolder", "bannerFolder", "Folder"
             var folderTypes = Services.ContentTypeService
-                .GetAllContentTypeAliases(Constants.ObjectTypes.MediaTypeGuid)
-                .Where(x => x.EndsWith("Folder"));
-            
-            var children = (id < 0) 
-                ? Services.MediaService.GetRootMedia()
-                : Services.MediaService.GetChildren(id);
+                .GetAllMediaTypes()
+                .Where(x => x.Alias.EndsWith("Folder"))
+                .Select(x => x.Id)
+                .ToArray();
 
-            return children.Where(x => folderTypes.Contains(x.ContentType.Alias)).Select(Mapper.Map>);
+            if (folderTypes.Length == 0)
+            {
+                return new PagedResult>(0, pageNumber, pageSize);
+            }
+
+            long total;
+            var children = Services.MediaService.GetPagedChildren(id, pageNumber - 1, pageSize, out total, "Name", Direction.Ascending, true, null, folderTypes.ToArray());
+            
+            return new PagedResult>(total, pageNumber, pageSize)
+            {
+                Items = children.Select(Mapper.Map>)
+            };
         }
 
         ///