Skip to content

Commit

Permalink
Few more NRT tweaks (#12323)
Browse files Browse the repository at this point in the history
* Amended GetAll() on IDataTypeService to return an empty collection rather than null.

* Added a ClearSessionValue method to ISessionManager (given you can no longer set a value to null).

* Allow for null values in a StatefulNotification.

* Removed obsoletion of synchronous messages on TreeControllerBase.

* Fixed further CS8620 warnings in core project.

* Further fix to nullable warning.

* Aligned nullablility of retreiving tree nodes and menus, synchronously or asynchronously (such that we no longer can get null values, always empty collection objects).
  • Loading branch information
AndyButland committed May 1, 2022
1 parent d47ae68 commit 96d3320
Show file tree
Hide file tree
Showing 31 changed files with 102 additions and 77 deletions.
2 changes: 1 addition & 1 deletion src/Umbraco.Core/Events/QueuingEventDispatcherBase.cs
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.Linq;
using Umbraco.Cms.Core.Collections;
Expand Down
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.Linq;
using Umbraco.Cms.Core.PropertyEditors;
Expand Down Expand Up @@ -84,7 +84,7 @@ public PublishedDataType GetDataType(int id)
if (_publishedDataTypes == null)
{
var dataTypes = _dataTypeService.GetAll();
_publishedDataTypes = dataTypes?.ToDictionary(x => x.Id, CreatePublishedDataType);
_publishedDataTypes = dataTypes.ToDictionary(x => x.Id, CreatePublishedDataType);
}

publishedDataTypes = _publishedDataTypes;
Expand All @@ -104,19 +104,16 @@ public void NotifyDataTypeChanges(int[] ids)
if (_publishedDataTypes == null)
{
var dataTypes = _dataTypeService.GetAll();
_publishedDataTypes = dataTypes?.ToDictionary(x => x.Id, CreatePublishedDataType);
_publishedDataTypes = dataTypes.ToDictionary(x => x.Id, CreatePublishedDataType);
}
else
{
foreach (var id in ids)
_publishedDataTypes.Remove(id);

var dataTypes = _dataTypeService.GetAll(ids);
if (dataTypes is not null)
{
foreach (var dataType in dataTypes)
_publishedDataTypes[dataType.Id] = CreatePublishedDataType(dataType);
}
foreach (var dataType in dataTypes)
_publishedDataTypes[dataType.Id] = CreatePublishedDataType(dataType);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Umbraco.Core/Notifications/IStatefulNotification.cs
Expand Up @@ -4,6 +4,6 @@ namespace Umbraco.Cms.Core.Notifications
{
public interface IStatefulNotification : INotification
{
IDictionary<string, object> State { get; set; }
IDictionary<string, object?> State { get; set; }
}
}
2 changes: 1 addition & 1 deletion src/Umbraco.Core/Notifications/NotificationExtensions.cs
Expand Up @@ -4,7 +4,7 @@ namespace Umbraco.Cms.Core.Notifications
{
public static class NotificationExtensions
{
public static T WithState<T>(this T notification, IDictionary<string, object>? state) where T : IStatefulNotification
public static T WithState<T>(this T notification, IDictionary<string, object?>? state) where T : IStatefulNotification
{
notification.State = state!;
return notification;
Expand Down
6 changes: 3 additions & 3 deletions src/Umbraco.Core/Notifications/StatefulNotification.cs
Expand Up @@ -6,15 +6,15 @@ namespace Umbraco.Cms.Core.Notifications
{
public abstract class StatefulNotification : IStatefulNotification
{
private IDictionary<string, object>? _state;
private IDictionary<string, object?>? _state;

/// <summary>
/// This can be used by event subscribers to store state in the notification so they easily deal with custom state data between
/// a starting ("ing") and an ending ("ed") notification
/// </summary>
public IDictionary<string, object> State
public IDictionary<string, object?> State
{
get => _state ??= new Dictionary<string, object>();
get => _state ??= new Dictionary<string, object?>();
set => _state = value;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/Umbraco.Core/Services/ContentService.cs
Expand Up @@ -1377,7 +1377,7 @@ public OperationResult Save(IEnumerable<IContent> contents, int userId = Constan
/// </remarks>
private PublishResult CommitDocumentChangesInternal(ICoreScope scope, IContent content,
EventMessages eventMessages, IReadOnlyCollection<ILanguage> allLangs,
IDictionary<string, object>? notificationState,
IDictionary<string, object?>? notificationState,
int userId = Constants.Security.SuperUserId,
bool branchOne = false, bool branchRoot = false)
{
Expand Down Expand Up @@ -3036,7 +3036,7 @@ internal IEnumerable<IContent> GetPublishedDescendantsLocked(IContent content)
private PublishResult StrategyCanPublish(ICoreScope scope, IContent content, bool checkPath,
IReadOnlyList<string>? culturesPublishing,
IReadOnlyCollection<string>? culturesUnpublishing, EventMessages evtMsgs,
IReadOnlyCollection<ILanguage> allLangs, IDictionary<string, object>? notificationState)
IReadOnlyCollection<ILanguage> allLangs, IDictionary<string, object?>? notificationState)
{
// raise Publishing notification
if (scope.Notifications.PublishCancelable(
Expand Down
6 changes: 1 addition & 5 deletions src/Umbraco.Core/Services/DataTypeService.cs
Expand Up @@ -364,15 +364,11 @@ public IEnumerable<EntityContainer> GetContainers(string name, int level)
/// </summary>
/// <param name="ids">Optional array of Ids</param>
/// <returns>An enumerable list of <see cref="IDataType"/> objects</returns>
public IEnumerable<IDataType>? GetAll(params int[] ids)
public IEnumerable<IDataType> GetAll(params int[] ids)
{
using (var scope = ScopeProvider.CreateCoreScope(autoComplete: true))
{
var dataTypes = _dataTypeRepository.GetMany(ids);
if (dataTypes is null)
{
return null;
}

ConvertMissingEditorsOfDataTypesToLabels(dataTypes);
return dataTypes;
Expand Down
2 changes: 1 addition & 1 deletion src/Umbraco.Core/Services/IDataTypeService.cs
Expand Up @@ -53,7 +53,7 @@ public interface IDataTypeService : IService
/// </summary>
/// <param name="ids">Optional array of Ids</param>
/// <returns>An enumerable list of <see cref="IDataType"/> objects</returns>
IEnumerable<IDataType>? GetAll(params int[] ids);
IEnumerable<IDataType> GetAll(params int[] ids);

/// <summary>
/// Saves an <see cref="IDataType"/>
Expand Down
2 changes: 1 addition & 1 deletion src/Umbraco.Core/Services/MemberService.cs
Expand Up @@ -852,7 +852,7 @@ public void Delete(IMember member)
}
}

private void DeleteLocked(ICoreScope scope, IMember member, EventMessages evtMsgs, IDictionary<string, object>? notificationState = null)
private void DeleteLocked(ICoreScope scope, IMember member, EventMessages evtMsgs, IDictionary<string, object?>? notificationState = null)
{
// a member has no descendants
_memberRepository.Delete(member);
Expand Down
7 changes: 5 additions & 2 deletions src/Umbraco.Core/Web/ISessionManager.cs
Expand Up @@ -2,7 +2,10 @@ namespace Umbraco.Cms.Core.Web
{
public interface ISessionManager
{
string? GetSessionValue(string sessionName);
void SetSessionValue(string sessionName, string value);
string? GetSessionValue(string key);

void SetSessionValue(string key, string value);

void ClearSessionValue(string key);
}
}
Expand Up @@ -90,7 +90,7 @@ private OldRoutesDictionary GetOldRoutes(IStatefulNotification notification)
notification.State[NotificationStateKey] = new OldRoutesDictionary();
}

return (OldRoutesDictionary)notification.State[NotificationStateKey];
return (OldRoutesDictionary?)notification.State[NotificationStateKey] ?? new OldRoutesDictionary();
}

private void StoreOldRoute(IContent entity, OldRoutesDictionary oldRoutes)
Expand Down
8 changes: 4 additions & 4 deletions src/Umbraco.Web.BackOffice/Controllers/DataTypeController.cs
Expand Up @@ -424,8 +424,8 @@ public DataTypeReferences GetReferences(int id)
[Authorize(Policy = AuthorizationPolicies.SectionAccessForDataTypeReading)]
public IEnumerable<DataTypeBasic>? GetAll()
{
return _dataTypeService?
.GetAll()?
return _dataTypeService
.GetAll()
.Select(_umbracoMapper.Map<IDataType, DataTypeBasic>).WhereNotNull().Where(x => x.IsSystemDataType == false);
}

Expand All @@ -439,8 +439,8 @@ public DataTypeReferences GetReferences(int id)
[Authorize(Policy = AuthorizationPolicies.SectionAccessForDataTypeReading)]
public IDictionary<string, IEnumerable<DataTypeBasic>>? GetGroupedDataTypes()
{
var dataTypes = _dataTypeService?
.GetAll()?
var dataTypes = _dataTypeService
.GetAll()
.Select(_umbracoMapper.Map<IDataType, DataTypeBasic>)
.ToArray();

Expand Down
Expand Up @@ -73,7 +73,7 @@ public class ContentBlueprintTreeController : TreeController

return root;
}
protected override ActionResult<TreeNodeCollection?> GetTreeNodes(string id, FormCollection queryStrings)
protected override ActionResult<TreeNodeCollection> GetTreeNodes(string id, FormCollection queryStrings)
{
var nodes = new TreeNodeCollection();

Expand Down
8 changes: 4 additions & 4 deletions src/Umbraco.Web.BackOffice/Trees/ContentTreeControllerBase.cs
Expand Up @@ -187,7 +187,7 @@ private void GetUserStartNodes(out int[]? startNodeIds, out string[]? startNodeP
/// </summary>
protected abstract int[] UserStartNodes { get; }

protected virtual ActionResult<TreeNodeCollection?> PerformGetTreeNodes(string id, FormCollection queryStrings)
protected virtual ActionResult<TreeNodeCollection> PerformGetTreeNodes(string id, FormCollection queryStrings)
{
var nodes = new TreeNodeCollection();

Expand Down Expand Up @@ -345,7 +345,7 @@ protected bool HasPathAccess(IUmbracoEntity? entity, FormCollection queryStrings
/// <remarks>
/// This method is overwritten strictly to render the recycle bin, it should serve no other purpose
/// </remarks>
protected sealed override ActionResult<TreeNodeCollection?> GetTreeNodes(string id, FormCollection queryStrings)
protected sealed override ActionResult<TreeNodeCollection> GetTreeNodes(string id, FormCollection queryStrings)
{
//check if we're rendering the root
if (id == Constants.System.RootString && UserStartNodes.Contains(Constants.System.Root))
Expand Down Expand Up @@ -384,7 +384,7 @@ protected bool HasPathAccess(IUmbracoEntity? entity, FormCollection queryStrings
queryStrings.GetRequiredValue<string>("application") + TreeAlias.EnsureStartsWith('/') + "/recyclebin"));
}

return nodes;
return nodes ?? new TreeNodeCollection();
}

return GetTreeNodesInternal(id, queryStrings);
Expand Down Expand Up @@ -435,7 +435,7 @@ protected bool ShouldRenderChildrenOfContainer(IEntitySlim e)
/// <remarks>
/// Currently this just checks if it is a container type, if it is we cannot render children. In the future this might check for other things.
/// </remarks>
private ActionResult<TreeNodeCollection?> GetTreeNodesInternal(string id, FormCollection queryStrings)
private ActionResult<TreeNodeCollection> GetTreeNodesInternal(string id, FormCollection queryStrings)
{
var current = GetEntityFromId(id);

Expand Down
Expand Up @@ -58,7 +58,7 @@ public ContentTypeTreeController(ILocalizedTextService localizedTextService, Umb
return root;
}

protected override ActionResult<TreeNodeCollection?> GetTreeNodes(string id, FormCollection queryStrings)
protected override ActionResult<TreeNodeCollection> GetTreeNodes(string id, FormCollection queryStrings)
{
if (!int.TryParse(id, NumberStyles.Integer, CultureInfo.InvariantCulture, out var intId))
{
Expand Down
4 changes: 2 additions & 2 deletions src/Umbraco.Web.BackOffice/Trees/DataTypeTreeController.cs
Expand Up @@ -41,7 +41,7 @@ public DataTypeTreeController(ILocalizedTextService localizedTextService, Umbrac
_dataTypeService = dataTypeService;
}

protected override ActionResult<TreeNodeCollection?> GetTreeNodes(string id, FormCollection queryStrings)
protected override ActionResult<TreeNodeCollection> GetTreeNodes(string id, FormCollection queryStrings)
{
if (!int.TryParse(id, NumberStyles.Integer, CultureInfo.InvariantCulture, out var intId))
{
Expand Down Expand Up @@ -71,7 +71,7 @@ public DataTypeTreeController(ILocalizedTextService localizedTextService, Umbrac
var systemListViewDataTypeIds = GetNonDeletableSystemListViewDataTypeIds();

var children = _entityService.GetChildren(intId, UmbracoObjectTypes.DataType).ToArray();
var dataTypes = Enumerable.ToDictionary(_dataTypeService.GetAll(children.Select(c => c.Id).ToArray()) ?? Enumerable.Empty<IDataType>(), dt => dt.Id);
var dataTypes = Enumerable.ToDictionary(_dataTypeService.GetAll(children.Select(c => c.Id).ToArray()), dt => dt.Id);

nodes.AddRange(
children
Expand Down
Expand Up @@ -71,7 +71,7 @@ public DictionaryTreeController(ILocalizedTextService localizedTextService, Umbr
/// We are allowing an arbitrary number of query strings to be passed in so that developers are able to persist custom data from the front-end
/// to the back end to be used in the query for model data.
/// </remarks>
protected override ActionResult<TreeNodeCollection?> GetTreeNodes(string id, FormCollection queryStrings)
protected override ActionResult<TreeNodeCollection> GetTreeNodes(string id, FormCollection queryStrings)
{
if (!int.TryParse(id, NumberStyles.Integer, CultureInfo.InvariantCulture, out var intId))
{
Expand Down
Expand Up @@ -49,7 +49,7 @@ IEventAggregator eventAggregator
treeNode.AdditionalData["jsClickCallback"] = "javascript:void(0);";
}

protected override ActionResult<TreeNodeCollection?> GetTreeNodes(string id, FormCollection queryStrings)
protected override ActionResult<TreeNodeCollection> GetTreeNodes(string id, FormCollection queryStrings)
{
var path = string.IsNullOrEmpty(id) == false && id != Constants.System.RootString
? WebUtility.UrlDecode(id).TrimStart("/")
Expand Down
12 changes: 8 additions & 4 deletions src/Umbraco.Web.BackOffice/Trees/LanguageTreeController.cs
Expand Up @@ -17,24 +17,28 @@ namespace Umbraco.Cms.Web.BackOffice.Trees
[CoreTree]
public class LanguageTreeController : TreeController
{
private readonly IMenuItemCollectionFactory _menuItemCollectionFactory;

public LanguageTreeController(
ILocalizedTextService textService,
UmbracoApiControllerTypeCollection umbracoApiControllerTypeCollection,
IEventAggregator eventAggregator)
IEventAggregator eventAggregator,
IMenuItemCollectionFactory menuItemCollectionFactory)
: base(textService, umbracoApiControllerTypeCollection, eventAggregator)
{
_menuItemCollectionFactory = menuItemCollectionFactory;
}
protected override ActionResult<TreeNodeCollection?> GetTreeNodes(string id, FormCollection queryStrings)

protected override ActionResult<TreeNodeCollection> GetTreeNodes(string id, FormCollection queryStrings)
{
//We don't have any child nodes & only use the root node to load a custom UI
return new TreeNodeCollection();
}

protected override ActionResult<MenuItemCollection>? GetMenuForNode(string id, FormCollection queryStrings)
protected override ActionResult<MenuItemCollection> GetMenuForNode(string id, FormCollection queryStrings)
{
//We don't have any menu item options (such as create/delete/reload) & only use the root node to load a custom UI
return null;
return _menuItemCollectionFactory.Create();
}

/// <summary>
Expand Down
12 changes: 8 additions & 4 deletions src/Umbraco.Web.BackOffice/Trees/LogViewerTreeController.cs
Expand Up @@ -17,24 +17,28 @@ namespace Umbraco.Cms.Web.BackOffice.Trees
[CoreTree]
public class LogViewerTreeController : TreeController
{
private readonly IMenuItemCollectionFactory _menuItemCollectionFactory;

public LogViewerTreeController(
ILocalizedTextService localizedTextService,
UmbracoApiControllerTypeCollection umbracoApiControllerTypeCollection,
IEventAggregator eventAggregator)
IEventAggregator eventAggregator,
IMenuItemCollectionFactory menuItemCollectionFactory)
: base(localizedTextService, umbracoApiControllerTypeCollection, eventAggregator)
{
_menuItemCollectionFactory = menuItemCollectionFactory;
}

protected override ActionResult<TreeNodeCollection?> GetTreeNodes(string id, FormCollection queryStrings)
protected override ActionResult<TreeNodeCollection> GetTreeNodes(string id, FormCollection queryStrings)
{
//We don't have any child nodes & only use the root node to load a custom UI
return new TreeNodeCollection();
}

protected override ActionResult<MenuItemCollection>? GetMenuForNode(string id, FormCollection queryStrings)
protected override ActionResult<MenuItemCollection> GetMenuForNode(string id, FormCollection queryStrings)
{
//We don't have any menu item options (such as create/delete/reload) & only use the root node to load a custom UI
return null;
return _menuItemCollectionFactory.Create();
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion src/Umbraco.Web.BackOffice/Trees/MacrosTreeController.cs
Expand Up @@ -48,7 +48,7 @@ public MacrosTreeController(ILocalizedTextService localizedTextService, UmbracoA
return root;
}

protected override ActionResult<TreeNodeCollection?> GetTreeNodes(string id, FormCollection queryStrings)
protected override ActionResult<TreeNodeCollection> GetTreeNodes(string id, FormCollection queryStrings)
{
var nodes = new TreeNodeCollection();

Expand Down
Expand Up @@ -40,7 +40,7 @@ public MediaTypeTreeController(ILocalizedTextService localizedTextService, Umbra
_entityService = entityService;
}

protected override ActionResult<TreeNodeCollection?> GetTreeNodes(string id, FormCollection queryStrings)
protected override ActionResult<TreeNodeCollection> GetTreeNodes(string id, FormCollection queryStrings)
{
if (!int.TryParse(id, NumberStyles.Integer, CultureInfo.InvariantCulture, out var intId))
{
Expand Down
2 changes: 1 addition & 1 deletion src/Umbraco.Web.BackOffice/Trees/MemberTreeController.cs
Expand Up @@ -104,7 +104,7 @@ public class MemberTreeController : TreeController, ISearchableTree, ITreeNodeCo
return node;
}

protected override ActionResult<TreeNodeCollection?> GetTreeNodes(string id, FormCollection queryStrings)
protected override ActionResult<TreeNodeCollection> GetTreeNodes(string id, FormCollection queryStrings)
{
var nodes = new TreeNodeCollection();

Expand Down
Expand Up @@ -52,7 +52,7 @@ public abstract class MemberTypeAndGroupTreeControllerBase : TreeController
{
}

protected override ActionResult<TreeNodeCollection?> GetTreeNodes(string id, FormCollection queryStrings)
protected override ActionResult<TreeNodeCollection> GetTreeNodes(string id, FormCollection queryStrings)
{
var nodes = new TreeNodeCollection();

Expand Down
2 changes: 1 addition & 1 deletion src/Umbraco.Web.BackOffice/Trees/PackagesTreeController.cs
Expand Up @@ -56,7 +56,7 @@ public class PackagesTreeController : TreeController
}


protected override ActionResult<TreeNodeCollection?> GetTreeNodes(string id, FormCollection queryStrings)
protected override ActionResult<TreeNodeCollection> GetTreeNodes(string id, FormCollection queryStrings)
{
//full screen app without tree nodes
return TreeNodeCollection.Empty;
Expand Down
Expand Up @@ -63,7 +63,7 @@ protected override ActionResult<MenuItemCollection> GetMenuForNode(string id, Fo
return menu;
}

protected override ActionResult<TreeNodeCollection?> GetTreeNodes(string id, FormCollection queryStrings)
protected override ActionResult<TreeNodeCollection> GetTreeNodes(string id, FormCollection queryStrings)
{
var nodes = new TreeNodeCollection();

Expand Down

0 comments on commit 96d3320

Please sign in to comment.