Ensured container collections retrieved from content type services return empty rather than nullable. (#12360)

* Ensured container collections retrieved from content type services return empty rather than nullable.

* Reverted change to ILocalLinkParser.

* Reverted change to ILocalLinkParser (2).

* Update GetByParentOrChildId in IRelationService to return non-nullable result.

* Added obsoleted constructor due to change in MemberTypeService.

* Updated nullability of Compare method parameters in IServiceConnector.

* Adjusted nullability of method on IValueConnector.

* Removed nullability of ArtifactDeployState.
This commit is contained in:
Andy Butland
2022-05-05 12:56:44 +02:00
committed by GitHub
parent 735086a747
commit 5be156bb22
16 changed files with 95 additions and 62 deletions

View File

@@ -1,4 +1,4 @@
namespace Umbraco.Cms.Core.Deploy
namespace Umbraco.Cms.Core.Deploy
{
/// <summary>
/// Represent the state of an artifact being deployed.
@@ -24,10 +24,7 @@
/// <summary>
/// Gets the artifact.
/// </summary>
public IArtifact? Artifact
{
get { return GetArtifactAsIArtifact(); }
}
public IArtifact Artifact => GetArtifactAsIArtifact();
/// <summary>
/// Gets the artifact as an <see cref="IArtifact"/>.
@@ -35,7 +32,7 @@
/// <returns>The artifact, as an <see cref="IArtifact"/>.</returns>
/// <remarks>This is because classes that inherit from this class cannot override the Artifact property
/// with a property that specializes the return type, and so they need to 'new' the property.</remarks>
protected abstract IArtifact? GetArtifactAsIArtifact();
protected abstract IArtifact GetArtifactAsIArtifact();
/// <summary>
/// Gets or sets the service connector in charge of deploying the artifact.

View File

@@ -1,4 +1,4 @@
namespace Umbraco.Cms.Core.Deploy
namespace Umbraco.Cms.Core.Deploy
{
/// <summary>
/// Represent the state of an artifact being deployed.
@@ -8,12 +8,6 @@
public class ArtifactDeployState<TArtifact, TEntity> : ArtifactDeployState
where TArtifact : IArtifact
{
/// <summary>
/// Initializes a new instance of the <see cref="ArtifactDeployState{TArtifact,TEntity}"/> class.
/// </summary>
public ArtifactDeployState()
{ }
/// <summary>
/// Initializes a new instance of the <see cref="ArtifactDeployState{TArtifact,TEntity}"/> class.
/// </summary>
@@ -32,15 +26,15 @@
/// <summary>
/// Gets or sets the artifact.
/// </summary>
public new TArtifact? Artifact { get; set; }
public new TArtifact Artifact { get; set; }
/// <summary>
/// Gets or sets the entity.
/// </summary>
public TEntity? Entity { get; set; }
public TEntity Entity { get; set; }
/// <inheritdoc/>
protected sealed override IArtifact? GetArtifactAsIArtifact()
protected sealed override IArtifact GetArtifactAsIArtifact()
{
return Artifact;
}

View File

@@ -1,5 +1,3 @@
using System.Collections.Generic;
namespace Umbraco.Cms.Core.Deploy
{
/// <summary>

View File

@@ -78,7 +78,7 @@ namespace Umbraco.Cms.Core.Deploy
/// <param name="differences">A collection of differences to append to, if not null.</param>
/// <returns>A boolean value indicating whether the artifacts are identical.</returns>
/// <remarks>ServiceConnectorBase{TArtifact} provides a very basic default implementation.</remarks>
bool Compare(IArtifact art1, IArtifact art2, ICollection<Difference>? differences = null);
bool Compare(IArtifact? art1, IArtifact? art2, ICollection<Difference>? differences = null);
}
}

View File

@@ -32,6 +32,6 @@ namespace Umbraco.Cms.Core.Deploy
/// <param name="propertyType">The value property type<</param>
/// <param name="currentValue">The current content property value.</param>
/// <returns>The content property value.</returns>
object? FromArtifact(string? value, IPropertyType propertyType, object currentValue);
object? FromArtifact(string? value, IPropertyType propertyType, object? currentValue);
}
}

View File

@@ -0,0 +1,5 @@
namespace Umbraco.Cms.Core.Persistence.Repositories
{
public interface IMemberTypeContainerRepository : IEntityContainerRepository
{ }
}

View File

@@ -1,7 +1,4 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using Microsoft.Extensions.Logging;
using Umbraco.Cms.Core.Events;
using Umbraco.Cms.Core.Exceptions;
@@ -20,12 +17,12 @@ namespace Umbraco.Cms.Core.Services
where TItem : class, IContentTypeComposition
{
private readonly IAuditRepository _auditRepository;
private readonly IEntityContainerRepository? _containerRepository;
private readonly IEntityContainerRepository _containerRepository;
private readonly IEntityRepository _entityRepository;
private readonly IEventAggregator _eventAggregator;
protected ContentTypeServiceBase(ICoreScopeProvider provider, ILoggerFactory loggerFactory, IEventMessagesFactory eventMessagesFactory,
TRepository repository, IAuditRepository auditRepository, IEntityContainerRepository? containerRepository, IEntityRepository entityRepository,
TRepository repository, IAuditRepository auditRepository, IEntityContainerRepository containerRepository, IEntityRepository entityRepository,
IEventAggregator eventAggregator)
: base(provider, loggerFactory, eventMessagesFactory)
{
@@ -943,7 +940,7 @@ namespace Umbraco.Cms.Core.Services
{
scope.ReadLock(ReadLockIds); // also for containers
return _containerRepository?.Get(containerId);
return _containerRepository.Get(containerId);
}
}
@@ -953,42 +950,37 @@ namespace Umbraco.Cms.Core.Services
{
scope.ReadLock(ReadLockIds); // also for containers
return _containerRepository?.Get(containerId);
return _containerRepository.Get(containerId);
}
}
public IEnumerable<EntityContainer>? GetContainers(int[] containerIds)
public IEnumerable<EntityContainer> GetContainers(int[] containerIds)
{
using (var scope = ScopeProvider.CreateCoreScope(autoComplete: true))
{
scope.ReadLock(ReadLockIds); // also for containers
return _containerRepository?.GetMany(containerIds);
return _containerRepository.GetMany(containerIds);
}
}
public IEnumerable<EntityContainer>? GetContainers(TItem? item)
public IEnumerable<EntityContainer> GetContainers(TItem item)
{
var ancestorIds = item?.Path.Split(Constants.CharArrays.Comma, StringSplitOptions.RemoveEmptyEntries)
var ancestorIds = item.Path.Split(Constants.CharArrays.Comma, StringSplitOptions.RemoveEmptyEntries)
.Select(x => int.TryParse(x, NumberStyles.Integer, CultureInfo.InvariantCulture, out var asInt) ? asInt : int.MinValue)
.Where(x => x != int.MinValue && x != item.Id)
.ToArray();
if (ancestorIds is null)
{
return null;
}
return GetContainers(ancestorIds);
}
public IEnumerable<EntityContainer>? GetContainers(string name, int level)
public IEnumerable<EntityContainer> GetContainers(string name, int level)
{
using (var scope = ScopeProvider.CreateCoreScope(autoComplete: true))
{
scope.ReadLock(ReadLockIds); // also for containers
return _containerRepository?.Get(name, level);
return _containerRepository.Get(name, level);
}
}

View File

@@ -522,14 +522,11 @@ namespace Umbraco.Cms.Core.Services
if (contentType.Level != 1 && masterContentType == null)
{
//get URL encoded folder names
var folders = _contentTypeService.GetContainers(contentType)?
IOrderedEnumerable<EntityContainer> folders = _contentTypeService.GetContainers(contentType)
.OrderBy(x => x.Level);
if (folders is not null)
{
folderNames = string.Join("/", folders.Select(x => WebUtility.UrlEncode(x.Name)).ToArray());
folderKeys = string.Join("/", folders.Select(x => x.Key).ToArray());
}
folderNames = string.Join("/", folders.Select(x => WebUtility.UrlEncode(x.Name)).ToArray());
folderKeys = string.Join("/", folders.Select(x => x.Key).ToArray());
}
if (string.IsNullOrWhiteSpace(folderNames) == false)

View File

@@ -82,9 +82,9 @@ namespace Umbraco.Cms.Core.Services
Attempt<OperationResult?> SaveContainer(EntityContainer container, int userId = Constants.Security.SuperUserId);
EntityContainer? GetContainer(int containerId);
EntityContainer? GetContainer(Guid containerId);
IEnumerable<EntityContainer>? GetContainers(int[] containerIds);
IEnumerable<EntityContainer>? GetContainers(TItem? contentType);
IEnumerable<EntityContainer>? GetContainers(string folderName, int level);
IEnumerable<EntityContainer> GetContainers(int[] containerIds);
IEnumerable<EntityContainer> GetContainers(TItem contentType);
IEnumerable<EntityContainer> GetContainers(string folderName, int level);
Attempt<OperationResult?> DeleteContainer(int containerId, int userId = Constants.Security.SuperUserId);
Attempt<OperationResult<OperationResultType, EntityContainer>?> RenameContainer(int id, string name, int userId = Constants.Security.SuperUserId);

View File

@@ -129,9 +129,9 @@ namespace Umbraco.Cms.Core.Services
/// </summary>
/// <param name="id">Id of the child or parent to retrieve relations for</param>
/// <returns>An enumerable list of <see cref="IRelation"/> objects</returns>
IEnumerable<IRelation>? GetByParentOrChildId(int id);
IEnumerable<IRelation> GetByParentOrChildId(int id);
IEnumerable<IRelation>? GetByParentOrChildId(int id, string relationTypeAlias);
IEnumerable<IRelation> GetByParentOrChildId(int id, string relationTypeAlias);
/// <summary>
/// Gets a relation by the unique combination of parentId, childId and relationType.

View File

@@ -1,5 +1,4 @@
using System;
using System.Collections.Generic;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Umbraco.Cms.Core.Events;
using Umbraco.Cms.Core.Models;
@@ -7,6 +6,7 @@ using Umbraco.Cms.Core.Notifications;
using Umbraco.Cms.Core.Persistence.Repositories;
using Umbraco.Cms.Core.Scoping;
using Umbraco.Cms.Core.Services.Changes;
using Umbraco.Cms.Web.Common.DependencyInjection;
using Umbraco.Extensions;
namespace Umbraco.Cms.Core.Services
@@ -15,9 +15,16 @@ namespace Umbraco.Cms.Core.Services
{
private readonly IMemberTypeRepository _memberTypeRepository;
[Obsolete("Please use the constructor taking all parameters. This constructor will be removed in V12.")]
public MemberTypeService(ICoreScopeProvider provider, ILoggerFactory loggerFactory, IEventMessagesFactory eventMessagesFactory, IMemberService memberService,
IMemberTypeRepository memberTypeRepository, IAuditRepository auditRepository, IEntityRepository entityRepository, IEventAggregator eventAggregator)
: base(provider, loggerFactory, eventMessagesFactory, memberTypeRepository, auditRepository, null, entityRepository, eventAggregator)
: this(provider, loggerFactory, eventMessagesFactory, memberService, memberTypeRepository, auditRepository, StaticServiceProvider.Instance.GetRequiredService<IMemberTypeContainerRepository>(), entityRepository, eventAggregator)
{
}
public MemberTypeService(ICoreScopeProvider provider, ILoggerFactory loggerFactory, IEventMessagesFactory eventMessagesFactory, IMemberService memberService,
IMemberTypeRepository memberTypeRepository, IAuditRepository auditRepository, IMemberTypeContainerRepository entityContainerRepository, IEntityRepository entityRepository, IEventAggregator eventAggregator)
: base(provider, loggerFactory, eventMessagesFactory, memberTypeRepository, auditRepository, entityContainerRepository, entityRepository, eventAggregator)
{
MemberService = memberService;
_memberTypeRepository = memberTypeRepository;

View File

@@ -153,7 +153,7 @@ namespace Umbraco.Cms.Core.Services
public IEnumerable<IRelation> GetByChild(IUmbracoEntity child, string relationTypeAlias) => GetByChildId(child.Id, relationTypeAlias);
/// <inheritdoc />
public IEnumerable<IRelation>? GetByParentOrChildId(int id)
public IEnumerable<IRelation> GetByParentOrChildId(int id)
{
using (var scope = ScopeProvider.CreateCoreScope(autoComplete: true))
{
@@ -162,7 +162,7 @@ namespace Umbraco.Cms.Core.Services
}
}
public IEnumerable<IRelation>? GetByParentOrChildId(int id, string relationTypeAlias)
public IEnumerable<IRelation> GetByParentOrChildId(int id, string relationTypeAlias)
{
using (var scope = ScopeProvider.CreateCoreScope(autoComplete: true))
{

View File

@@ -43,6 +43,7 @@ namespace Umbraco.Cms.Infrastructure.DependencyInjection
builder.Services.AddUnique<IMediaTypeRepository, MediaTypeRepository>();
builder.Services.AddUnique<IMemberGroupRepository, MemberGroupRepository>();
builder.Services.AddUnique<IMemberRepository, MemberRepository>();
builder.Services.AddUnique<IMemberTypeContainerRepository, MemberTypeContainerRepository>();
builder.Services.AddUnique<IMemberTypeRepository, MemberTypeRepository>();
builder.Services.AddUnique<INotificationsRepository, NotificationsRepository>();
builder.Services.AddUnique<IPublicAccessRepository, PublicAccessRepository>();

View File

@@ -687,7 +687,7 @@ namespace Umbraco.Cms.Infrastructure.Packaging
// The folder might already exist, but with a different key, so check if it exists, even if there is a key.
// Level 1 = root level folders, there can only be one with the same name
current ??= _contentTypeService.GetContainers(rootFolder, 1)?.FirstOrDefault();
current ??= _contentTypeService.GetContainers(rootFolder, 1).FirstOrDefault();
if (current == null)
{

View File

@@ -0,0 +1,32 @@
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Persistence.Repositories;
namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement
{
/// <summary>
/// A no-op implementation of <see cref="IMemberTypeContainerRepository"/>, as containers aren't supported for members.
/// </summary>
/// <remarks>
/// Introduced to avoid inconsistencies with nullability of dependencies for type repositories for content, media and members.
/// </remarks>
internal class MemberTypeContainerRepository : IMemberTypeContainerRepository
{
public void Delete(EntityContainer entity)
{
}
public bool Exists(int id) => false;
public EntityContainer? Get(Guid id) => null;
public IEnumerable<EntityContainer> Get(string name, int level) => Enumerable.Empty<EntityContainer>();
public EntityContainer? Get(int id) => null;
public IEnumerable<EntityContainer> GetMany(params int[]? ids) => Enumerable.Empty<EntityContainer>();
public void Save(EntityContainer entity)
{
}
}
}

View File

@@ -165,9 +165,9 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
}
IContentTypeComposition? contentType = allContentTypes.FirstOrDefault(c => c.Key == x.Item1?.Key);
EntityContainer[]? containers = GetEntityContainers(contentType, type)?.ToArray();
EntityContainer[] containers = GetEntityContainers(contentType, type).ToArray();
var containerPath =
$"/{(containers != null && containers.Any() ? $"{string.Join("/", containers.Select(c => c.Name))}/" : null)}";
$"/{(containers.Any() ? $"{string.Join("/", containers.Select(c => c.Name))}/" : null)}";
if (x.Item1 is not null)
{
x.Item1.AdditionalData["containerPath"] = containerPath;
@@ -178,22 +178,32 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
.ToList();
}
private IEnumerable<EntityContainer>? GetEntityContainers(IContentTypeComposition? contentType,
private IEnumerable<EntityContainer> GetEntityContainers(IContentTypeComposition? contentType,
UmbracoObjectTypes type)
{
if (contentType == null)
{
return null;
return Enumerable.Empty<EntityContainer>();
}
switch (type)
{
case UmbracoObjectTypes.DocumentType:
return ContentTypeService.GetContainers(contentType as IContentType);
if (contentType is IContentType documentContentType)
{
return ContentTypeService.GetContainers(documentContentType);
}
return Enumerable.Empty<EntityContainer>();
case UmbracoObjectTypes.MediaType:
return MediaTypeService.GetContainers(contentType as IMediaType);
if (contentType is IMediaType mediaContentType)
{
return MediaTypeService.GetContainers(mediaContentType);
}
return Enumerable.Empty<EntityContainer>();
case UmbracoObjectTypes.MemberType:
return new EntityContainer[0];
return Enumerable.Empty<EntityContainer>();
default:
throw new ArgumentOutOfRangeException("The entity type was not a content type");
}