yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fixes: Consistency for Nullable reference types in LINQ extensions methods #12692 (#12703)

Browse Source 
* Ensure Name and Children can never return null.

* Name should be empty not null

* Ensure Name and Children type queries never return null

Co-authored-by: Sebastiaan Janssen <sebastiaan@umbraco.com>
This commit is contained in:
Dan Booth
2022-10-19 09:32:09 +01:00
committed by GitHub
parent 27e129a6b4
commit df19d4fc6d
8 changed files with 57 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

69
src/Umbraco.Core/Extensions/PublishedContentExtensions.cs
View File

@@ -25,7 +25,7 @@ public static class PublishedContentExtensions
/// The specific culture to get the name for. If null is used the current culture is used (Default is
/// null).
/// </param>
public static string? Name(this IPublishedContent content, IVariationContextAccessor? variationContextAccessor, string? culture = null)
public static string Name(this IPublishedContent content, IVariationContextAccessor? variationContextAccessor, string? culture = null)
{
if (content == null)
{
@@ -37,7 +37,7 @@ public static class PublishedContentExtensions
{
return content.Cultures.TryGetValue(string.Empty, out PublishedCultureInfo? invariantInfos)
? invariantInfos.Name
: null;
: string.Empty;
}
// handle context culture for variant
@@ -49,7 +49,7 @@ public static class PublishedContentExtensions
// get
return culture != string.Empty && content.Cultures.TryGetValue(culture, out PublishedCultureInfo? infos)
? infos.Name
: null;
: string.Empty;
}
#endregion
@@ -683,8 +683,8 @@ public static class PublishedContentExtensions
/// <param name="maxLevel">The level.</param>
/// <returns>The content or its nearest (in down-top order) ancestor, at a level lesser or equal to the specified level.</returns>
/// <remarks>May or may not return the content itself depending on its level. May return <c>null</c>.</remarks>
public static IPublishedContent? AncestorOrSelf(this IPublishedContent content, int maxLevel) =>
content.EnumerateAncestors(true).FirstOrDefault(x => x.Level <= maxLevel);
public static IPublishedContent AncestorOrSelf(this IPublishedContent content, int maxLevel) =>
content.EnumerateAncestors(true).FirstOrDefault(x => x.Level <= maxLevel) ?? content;
/// <summary>
/// Gets the content or its nearest ancestor, of a specified content type.
@@ -693,8 +693,8 @@ public static class PublishedContentExtensions
/// <param name="contentTypeAlias">The content type.</param>
/// <returns>The content or its nearest (in down-top order) ancestor, of the specified content type.</returns>
/// <remarks>May or may not return the content itself depending on its content type. May return <c>null</c>.</remarks>
public static IPublishedContent? AncestorOrSelf(this IPublishedContent content, string contentTypeAlias) => content
.EnumerateAncestors(true).FirstOrDefault(x => x.ContentType.Alias.InvariantEquals(contentTypeAlias));
public static IPublishedContent AncestorOrSelf(this IPublishedContent content, string contentTypeAlias) => content
.EnumerateAncestors(true).FirstOrDefault(x => x.ContentType.Alias.InvariantEquals(contentTypeAlias)) ?? content;
/// <summary>
/// Gets the content or its nearest ancestor, of a specified content type.
@@ -1012,7 +1012,7 @@ public static class PublishedContentExtensions
/// However, if an empty string is specified only invariant children are returned.
/// </para>
/// </remarks>
public static IEnumerable<IPublishedContent>? Children(this IPublishedContent content, IVariationContextAccessor? variationContextAccessor, string? culture = null)
public static IEnumerable<IPublishedContent> Children(this IPublishedContent content, IVariationContextAccessor? variationContextAccessor, string? culture = null)
{
// handle context culture for variant
if (culture == null)
@@ -1021,9 +1021,9 @@ public static class PublishedContentExtensions
}
IEnumerable<IPublishedContent>? children = content.ChildrenForAllCultures;
return culture == "*"
? children
: children?.Where(x => x.IsInvariantOrHasCulture(culture));
return (culture == "*"
? children : children?.Where(x => x.IsInvariantOrHasCulture(culture)))
?? Enumerable.Empty<IPublishedContent>();
}
/// <summary>
@@ -1040,12 +1040,12 @@ public static class PublishedContentExtensions
/// <remarks>
/// <para>Children are sorted by their sortOrder.</para>
/// </remarks>
public static IEnumerable<IPublishedContent>? Children(
public static IEnumerable<IPublishedContent> Children(
this IPublishedContent content,
IVariationContextAccessor variationContextAccessor,
Func<IPublishedContent, bool> predicate,
string? culture = null) =>
content.Children(variationContextAccessor, culture)?.Where(predicate);
content.Children(variationContextAccessor, culture).Where(predicate);
/// <summary>
/// Gets the children of the content, of any of the specified types.
@@ -1058,7 +1058,7 @@ public static class PublishedContentExtensions
/// </param>
/// <param name="contentTypeAlias">The content type alias.</param>
/// <returns>The children of the content, of any of the specified types.</returns>
public static IEnumerable<IPublishedContent>? ChildrenOfType(this IPublishedContent content, IVariationContextAccessor variationContextAccessor, string? contentTypeAlias, string? culture = null) =>
public static IEnumerable<IPublishedContent> ChildrenOfType(this IPublishedContent content, IVariationContextAccessor variationContextAccessor, string? contentTypeAlias, string? culture = null) =>
content.Children(variationContextAccessor, x => x.ContentType.Alias.InvariantEquals(contentTypeAlias), culture);
/// <summary>
@@ -1075,9 +1075,9 @@ public static class PublishedContentExtensions
/// <remarks>
/// <para>Children are sorted by their sortOrder.</para>
/// </remarks>
public static IEnumerable<T>? Children<T>(this IPublishedContent content, IVariationContextAccessor variationContextAccessor, string? culture = null)
public static IEnumerable<T> Children<T>(this IPublishedContent content, IVariationContextAccessor variationContextAccessor, string? culture = null)
where T : class, IPublishedContent =>
content.Children(variationContextAccessor, culture)?.OfType<T>();
content.Children(variationContextAccessor, culture).OfType<T>();
public static IPublishedContent? FirstChild(this IPublishedContent content, IVariationContextAccessor variationContextAccessor, string? culture = null) =>
content.Children(variationContextAccessor, culture)?.FirstOrDefault();
@@ -1119,12 +1119,13 @@ public static class PublishedContentExtensions
/// <remarks>
/// <para>Note that in V7 this method also return the content node self.</para>
/// </remarks>
public static IEnumerable<IPublishedContent>? Siblings(
public static IEnumerable<IPublishedContent> Siblings(
this IPublishedContent content,
IPublishedSnapshot? publishedSnapshot,
IVariationContextAccessor variationContextAccessor,
string? culture = null) =>
SiblingsAndSelf(content, publishedSnapshot, variationContextAccessor, culture)?.Where(x => x.Id != content.Id);
SiblingsAndSelf(content, publishedSnapshot, variationContextAccessor, culture)
?.Where(x => x.Id != content.Id) ?? Enumerable.Empty<IPublishedContent>();
/// <summary>
/// Gets the siblings of the content, of a given content type.
@@ -1141,14 +1142,14 @@ public static class PublishedContentExtensions
/// <remarks>
/// <para>Note that in V7 this method also return the content node self.</para>
/// </remarks>
public static IEnumerable<IPublishedContent>? SiblingsOfType(
public static IEnumerable<IPublishedContent> SiblingsOfType(
this IPublishedContent content,
IPublishedSnapshot? publishedSnapshot,
IVariationContextAccessor variationContextAccessor,
string contentTypeAlias,
string? culture = null) =>
SiblingsAndSelfOfType(content, publishedSnapshot, variationContextAccessor, contentTypeAlias, culture)
?.Where(x => x.Id != content.Id);
?.Where(x => x.Id != content.Id) ?? Enumerable.Empty<IPublishedContent>();
/// <summary>
/// Gets the siblings of the content, of a given content type.
@@ -1165,10 +1166,10 @@ public static class PublishedContentExtensions
/// <remarks>
/// <para>Note that in V7 this method also return the content node self.</para>
/// </remarks>
public static IEnumerable<T>? Siblings<T>(this IPublishedContent content, IPublishedSnapshot? publishedSnapshot, IVariationContextAccessor variationContextAccessor, string? culture = null)
public static IEnumerable<T> Siblings<T>(this IPublishedContent content, IPublishedSnapshot? publishedSnapshot, IVariationContextAccessor variationContextAccessor, string? culture = null)
where T : class, IPublishedContent =>
SiblingsAndSelf<T>(content, publishedSnapshot, variationContextAccessor, culture)
?.Where(x => x.Id != content.Id);
?.Where(x => x.Id != content.Id) ?? Enumerable.Empty<T>();
/// <summary>
/// Gets the siblings of the content including the node itself to indicate the position.
@@ -1203,16 +1204,17 @@ public static class PublishedContentExtensions
/// </param>
/// <param name="contentTypeAlias">The content type alias.</param>
/// <returns>The siblings of the content including the node itself, of the given content type.</returns>
public static IEnumerable<IPublishedContent>? SiblingsAndSelfOfType(
public static IEnumerable<IPublishedContent> SiblingsAndSelfOfType(
this IPublishedContent content,
IPublishedSnapshot? publishedSnapshot,
IVariationContextAccessor variationContextAccessor,
string contentTypeAlias,
string? culture = null) =>
content.Parent != null
(content.Parent != null
? content.Parent.ChildrenOfType(variationContextAccessor, contentTypeAlias, culture)
: publishedSnapshot?.Content?.GetAtRoot(culture).OfTypes(contentTypeAlias)
.WhereIsInvariantOrHasCulture(variationContextAccessor, culture);
.WhereIsInvariantOrHasCulture(variationContextAccessor, culture))
?? Enumerable.Empty<IPublishedContent>();
/// <summary>
/// Gets the siblings of the content including the node itself to indicate the position, of a given content type.
@@ -1226,16 +1228,17 @@ public static class PublishedContentExtensions
/// null)
/// </param>
/// <returns>The siblings of the content including the node itself, of the given content type.</returns>
public static IEnumerable<T>? SiblingsAndSelf<T>(
public static IEnumerable<T> SiblingsAndSelf<T>(
this IPublishedContent content,
IPublishedSnapshot? publishedSnapshot,
IVariationContextAccessor variationContextAccessor,
string? culture = null)
where T : class, IPublishedContent =>
content.Parent != null
(content.Parent != null
? content.Parent.Children<T>(variationContextAccessor, culture)
: publishedSnapshot?.Content?.GetAtRoot(culture).OfType<T>()
.WhereIsInvariantOrHasCulture(variationContextAccessor, culture);
.WhereIsInvariantOrHasCulture(variationContextAccessor, culture))
?? Enumerable.Empty<T>();
#endregion
@@ -1253,7 +1256,7 @@ public static class PublishedContentExtensions
/// <see cref="Umbraco.Web.PublishedContentExtensions.AncestorOrSelf(IPublishedContent, int)" /> with <c>maxLevel</c>
/// set to 1.
/// </remarks>
public static IPublishedContent? Root(this IPublishedContent content) => content.AncestorOrSelf(1);
public static IPublishedContent Root(this IPublishedContent content) => content.AncestorOrSelf(1);
/// <summary>
/// Gets the root content (ancestor or self at level 1) for the specified <paramref name="content" /> if it's of the
@@ -1278,16 +1281,16 @@ public static class PublishedContentExtensions
#region Writer and creator
public static string? GetCreatorName(this IPublishedContent content, IUserService userService)
public static string GetCreatorName(this IPublishedContent content, IUserService userService)
{
IProfile? user = userService.GetProfileById(content.CreatorId);
return user?.Name;
return user?.Name ?? string.Empty;
}
public static string? GetWriterName(this IPublishedContent content, IUserService userService)
public static string GetWriterName(this IPublishedContent content, IUserService userService)
{
IProfile? user = userService.GetProfileById(content.WriterId);
return user?.Name;
return user?.Name ?? string.Empty;
}
#endregion

6
src/Umbraco.Core/Models/PublishedContent/IPublishedContent.cs
View File

@@ -23,7 +23,7 @@ public interface IPublishedContent : IPublishedElement
/// <summary>
/// Gets the name of the content item for the current culture.
/// </summary>
string? Name { get; }
string Name { get; }
/// <summary>
/// Gets the URL segment of the content item for the current culture.
@@ -141,10 +141,10 @@ public interface IPublishedContent : IPublishedElement
/// <summary>
/// Gets the children of the content item that are available for the current culture.
/// </summary>
IEnumerable<IPublishedContent>? Children { get; }
IEnumerable<IPublishedContent> Children { get; }
/// <summary>
/// Gets all the children of the content item, regardless of whether they are available for the current culture.
/// </summary>
IEnumerable<IPublishedContent>? ChildrenForAllCultures { get; }
IEnumerable<IPublishedContent> ChildrenForAllCultures { get; }
}

4
src/Umbraco.Core/Models/PublishedContent/PublishedContentBase.cs
View File

@@ -24,7 +24,7 @@ namespace Umbraco.Cms.Core.Models.PublishedContent
public abstract int Id { get; }
/// <inheritdoc />
public virtual string? Name => this.Name(_variationContextAccessor);
public virtual string Name => this.Name(_variationContextAccessor);
/// <inheritdoc />
public virtual string? UrlSegment => this.UrlSegment(_variationContextAccessor);
@@ -69,7 +69,7 @@ namespace Umbraco.Cms.Core.Models.PublishedContent
public abstract IPublishedContent? Parent { get; }
/// <inheritdoc />
public virtual IEnumerable<IPublishedContent>? Children => this.Children(_variationContextAccessor);
public virtual IEnumerable<IPublishedContent> Children => this.Children(_variationContextAccessor);
/// <inheritdoc />
public abstract IEnumerable<IPublishedContent> ChildrenForAllCultures { get; }

6
src/Umbraco.Core/Models/PublishedContent/PublishedContentWrapped.cs
View File

@@ -54,7 +54,7 @@ public abstract class PublishedContentWrapped : IPublishedContent
public IPublishedContent Unwrap() => _content;
/// <inheritdoc />
public virtual string? Name => _content.Name;
public virtual string Name => _content.Name;
/// <inheritdoc />
public virtual string? UrlSegment => _content.UrlSegment;
@@ -99,10 +99,10 @@ public abstract class PublishedContentWrapped : IPublishedContent
public virtual bool IsPublished(string? culture = null) => _content.IsPublished(culture);
/// <inheritdoc />
public virtual IEnumerable<IPublishedContent>? Children => _content.Children;
public virtual IEnumerable<IPublishedContent> Children => _content.Children;
/// <inheritdoc />
public virtual IEnumerable<IPublishedContent>? ChildrenForAllCultures => _content.ChildrenForAllCultures;
public virtual IEnumerable<IPublishedContent> ChildrenForAllCultures => _content.ChildrenForAllCultures;
/// <inheritdoc cref="IPublishedElement.Properties" />
public virtual IEnumerable<IPublishedProperty> Properties => _content.Properties;

9
src/Umbraco.Core/PublishedCache/Internal/InternalPublishedContent.cs
View File

@@ -20,13 +20,14 @@ public sealed class InternalPublishedContent : IPublishedContent
Path = string.Empty;
ContentType = contentType;
Properties = Enumerable.Empty<IPublishedProperty>();
Name = string.Empty;
}
public Guid Version { get; set; }
public int ParentId { get; set; }
public IEnumerable<int>? ChildIds { get; set; }
public IEnumerable<int> ChildIds { get; set; } = Enumerable.Empty<int>();
public int Id { get; set; }
@@ -45,7 +46,7 @@ public sealed class InternalPublishedContent : IPublishedContent
public int SortOrder { get; set; }
public string? Name { get; set; }
public string Name { get; set; }
public IReadOnlyDictionary<string, PublishedCultureInfo> Cultures => _cultures ??= GetCultures();
@@ -71,9 +72,9 @@ public sealed class InternalPublishedContent : IPublishedContent
public bool IsPublished(string? culture = null) => true;
public IEnumerable<IPublishedContent>? Children { get; set; }
public IEnumerable<IPublishedContent> Children { get; set; } = Enumerable.Empty<IPublishedContent>();
public IEnumerable<IPublishedContent>? ChildrenForAllCultures => Children;
public IEnumerable<IPublishedContent> ChildrenForAllCultures => Children;
public IPublishedContentType ContentType { get; set; }

2
src/Umbraco.Web.Common/Extensions/FriendlyPublishedContentExtensions.cs
View File

@@ -330,7 +330,7 @@ public static class FriendlyPublishedContentExtensions
/// However, if an empty string is specified only invariant children are returned.
/// </para>
/// </remarks>
public static IEnumerable<IPublishedContent>? Children(this IPublishedContent content, string? culture = null)
public static IEnumerable<IPublishedContent> Children(this IPublishedContent content, string? culture = null)
=> content.Children(VariationContextAccessor, culture);
/// <summary>

10
tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/PublishedCache/PublishedSnapshotServiceCollectionTests.cs
View File

@@ -719,25 +719,25 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT
AssertDocuments(documents);
documents = snapshot.Content.GetById(1).Children(VariationContextAccessor, "*").ToArray();
AssertDocuments(documents, "N4-fr-FR", null, "N6-fr-FR");
AssertDocuments(documents, "N4-fr-FR", string.Empty, "N6-fr-FR");
AssertDocuments("en-US", documents, "N4-en-US", "N5-en-US", "N6-en-US");
documents = snapshot.Content.GetById(1).Children(VariationContextAccessor, "en-US").ToArray();
AssertDocuments(documents, "N4-fr-FR", null, "N6-fr-FR");
AssertDocuments(documents, "N4-fr-FR", string.Empty, "N6-fr-FR");
AssertDocuments("en-US", documents, "N4-en-US", "N5-en-US", "N6-en-US");
documents = snapshot.Content.GetById(1).ChildrenForAllCultures.ToArray();
AssertDocuments(documents, "N4-fr-FR", null, "N6-fr-FR");
AssertDocuments(documents, "N4-fr-FR", string.Empty, "N6-fr-FR");
AssertDocuments("en-US", documents, "N4-en-US", "N5-en-US", "N6-en-US");
documents = snapshot.Content.GetAtRoot("*").ToArray();
AssertDocuments(documents, "N1-fr-FR", null, "N3-fr-FR");
AssertDocuments(documents, "N1-fr-FR", string.Empty, "N3-fr-FR");
documents = snapshot.Content.GetById(1).DescendantsOrSelf(VariationContextAccessor).ToArray();
AssertDocuments(documents, "N1-fr-FR", "N4-fr-FR", "N12-fr-FR", "N6-fr-FR");
documents = snapshot.Content.GetById(1).DescendantsOrSelf(VariationContextAccessor, "*").ToArray();
AssertDocuments(documents, "N1-fr-FR", "N4-fr-FR", null /*11*/, "N12-fr-FR", null /*5*/, "N6-fr-FR");
AssertDocuments(documents, "N1-fr-FR", "N4-fr-FR", string.Empty /*11*/, "N12-fr-FR", string.Empty /*5*/, "N6-fr-FR");
}
[Test]

4
tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/PublishedCache/PublishedSnapshotServiceContentTests.cs
View File

@@ -120,7 +120,7 @@ public class PublishedSnapshotServiceContentTests : PublishedSnapshotServiceTest
Assert.AreEqual("val-fr1", publishedContent.Value<string>(Mock.Of<IPublishedValueFallback>(), "prop", "fr-FR"));
Assert.AreEqual("val-uk1", publishedContent.Value<string>(Mock.Of<IPublishedValueFallback>(), "prop", "en-UK"));
Assert.IsNull(publishedContent.Name(VariationContextAccessor)); // no invariant name for varying content
Assert.AreEqual(publishedContent.Name(VariationContextAccessor), string.Empty); // no invariant name for varying content
Assert.AreEqual("name-fr1", publishedContent.Name(VariationContextAccessor, "fr-FR"));
Assert.AreEqual("name-uk1", publishedContent.Name(VariationContextAccessor, "en-UK"));
@@ -129,7 +129,7 @@ public class PublishedSnapshotServiceContentTests : PublishedSnapshotServiceTest
Assert.AreEqual("val-fr2", draftContent.Value<string>(Mock.Of<IPublishedValueFallback>(), "prop", "fr-FR"));
Assert.AreEqual("val-uk2", draftContent.Value<string>(Mock.Of<IPublishedValueFallback>(), "prop", "en-UK"));
Assert.IsNull(draftContent.Name(VariationContextAccessor)); // no invariant name for varying content
Assert.AreEqual(draftContent.Name(VariationContextAccessor), string.Empty); // no invariant name for varying content
Assert.AreEqual("name-fr2", draftContent.Name(VariationContextAccessor, "fr-FR"));
Assert.AreEqual("name-uk2", draftContent.Name(VariationContextAccessor, "en-UK"));