IContentmentDataSource.GetItems does not support Async

[warrenbuckley]

Which Contentment version are you using?

6.0.0-alpha006

Which Umbraco version are you using? For example: 13.2.1 - don't just write v13

15.3.1

Bug summary

I am trying to port some Custom Data Source over as getting the deprecation warning from Umbraco that ILocalizationService is being deprecated and that replacements of ILanguageService and IDictionaryItemService methods are async.

Steps to reproduce

Newly ported code

Note the .Result in the GetItems method

using Umbraco.Cms.Core.PropertyEditors;
using Umbraco.Community.Contentment.DataEditors;
using Umbraco.Community.Contentment.Services;

namespace Consilium.Core.Entities.DataSources;

public class GenericLinkDictionaryEntries : IContentmentDataSource
{
    private readonly IContentmentContentContext _contentmentContentContext;
    private readonly ILanguageService _languageService;
    private readonly IDictionaryItemService _dictionaryItemService;

    public string Name => "Generic link dictionary entries";

    public string Description => "Get all generic links dictionary entries based on the editing document type";

    public string Icon => "icon-book-alt";

    public string Group => "Consilium";

    public OverlaySize OverlaySize => OverlaySize.Small;

    public Dictionary<string, object> DefaultValues => default;

    public IEnumerable<ContentmentConfigurationField> Fields => default;

    public GenericLinkDictionaryEntries(
        IContentmentContentContext contentmentContentContext,
        ILanguageService languageService,
        IDictionaryItemService dictionaryItemService)
    {
        _contentmentContentContext = contentmentContentContext;
        _languageService = languageService;
        _dictionaryItemService = dictionaryItemService;
    }

    public IEnumerable<DataListItem> GetItems(Dictionary<string, object> config)
    {
        // TODO: Raise an issue that GetItems does not support async :(
        // Appologies for all the .Results

        var defaultLanguage = _languageService.GetDefaultLanguageAsync().Result;
        var currentContent = _contentmentContentContext.GetCurrentContent(out bool isParent);

        var rootDictionaryItem = _dictionaryItemService.GetAsync(Constants.Core.DictionaryEntryGenericLinksPrefix + currentContent?.GetTemplateAlias()).Result;
        if (rootDictionaryItem == null)
        {
            rootDictionaryItem = _dictionaryItemService.GetAsync(Constants.Core.DictionaryEntryGenericLinksRoot).Result;
        }

        var children = _dictionaryItemService.GetChildrenAsync(rootDictionaryItem?.Key ?? Guid.Empty).Result;

        return children?.Select(entry => new DataListItem
        {
            Value = entry.ItemKey,
            Name = entry.GetTranslatedValue(defaultLanguage?.IsoCode),
        }).Where(x => !string.IsNullOrEmpty(x.Name)) ?? new List<DataListItem>();
    }
}

Expected result / actual result

For it to allow me to support working with Async code easier

Do you have Umbraco ModelsBuilder enabled?

• Yes, it is enabled.

What browsers are you seeing the problem on?

No response

[leekelleher]

@warrenbuckley Thanks for raising this.

I've moved it over to a discussion, as it's not a bug per se, so it'd be good to explore how this feature request could be implemented.

I know were in an age of "async all the things", and I'm happy to add support for this, but trying to understand the balance of those scenarios that don't require the data-source to be async (just regular sync). Otherwise the opposite of what you outline would happen, with people wrapping their non-async code with Task.FromResult() or Task.Run().

Maybe the answer is having both sync/async C# interfaces, then people can pick whichever they want and the Data List code can figure out which one to use? 🤔

[warrenbuckley]

Yes in an ideal world you have both @leekelleher then down to us implementors to pick and choose what is best. However if you are using any Umbraco services they are all seem to be going Async so it would be nice if you can support this.

Again no big deal breaker just a quality of life thing to consider perhaps?