Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix media tracking of items added via macro parameters in RTE and Grid #12139

Merged
merged 15 commits into from
Mar 21, 2022
Merged
9 changes: 8 additions & 1 deletion src/Umbraco.Core/Cache/MacroCacheRefresher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ public override void Refresh(string json)
{
var payloads = Deserialize(json);

Refresh(payloads);
}

public override void Refresh(JsonPayload[] payloads)
{
foreach (var payload in payloads)
{
foreach (var alias in GetCacheKeysForAlias(payload.Alias))
Expand All @@ -55,11 +60,13 @@ public override void Refresh(string json)
if (macroRepoCache)
{
macroRepoCache.Result.Clear(RepositoryCacheKeys.GetKey<IMacro, int>(payload.Id));
macroRepoCache.Result.Clear(RepositoryCacheKeys.GetKey<IMacro, string>(payload.Alias)); // Repository caching of macro definition by alias
}
}

base.Refresh(json);
base.Refresh(payloads);
}

#endregion

#region Json
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
using System;
using System.Collections.Generic;
using Umbraco.Cms.Core.Models;

namespace Umbraco.Cms.Core.Persistence.Repositories
{
[Obsolete("This interface will be merged with IMacroRepository in Umbraco 11")]
public interface IMacroWithAliasRepository : IMacroRepository
{
IMacro GetByAlias(string alias);

IEnumerable<IMacro> GetAllByAlias(string[] aliases);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using Microsoft.Extensions.Logging;
using Umbraco.Cms.Core.Hosting;
using Umbraco.Cms.Core.IO;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Serialization;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Strings;
Expand Down Expand Up @@ -28,5 +28,17 @@ public MultipleContentPickerParameterEditor(
DefaultConfiguration.Add("minNumber",0 );
DefaultConfiguration.Add("maxNumber", 0);
}

protected override IDataValueEditor CreateValueEditor() => DataValueEditorFactory.Create<MultipleContentPickerParameterEditor.MultipleContentPickerParamateterValueEditor>(Attribute);

internal class MultipleContentPickerParamateterValueEditor : MultiplePickerParamateterValueEditorBase
{
public MultipleContentPickerParamateterValueEditor(ILocalizedTextService localizedTextService, IShortStringHelper shortStringHelper, IJsonSerializer jsonSerializer, IIOHelper ioHelper, DataEditorAttribute attribute, IEntityService entityService) : base(localizedTextService, shortStringHelper, jsonSerializer, ioHelper, attribute, entityService)
{
}

public override string UdiEntityType { get; } = Constants.UdiEntityType.Document;
public override UmbracoObjectTypes UmbracoObjectType { get; } = UmbracoObjectTypes.Document;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
using Microsoft.Extensions.Logging;
using Umbraco.Cms.Core.Hosting;
using System;
using System.Collections.Generic;
using System.Reflection.Metadata;
using Umbraco.Cms.Core.IO;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.Editors;
using Umbraco.Cms.Core.Serialization;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Strings;
Expand All @@ -26,5 +30,17 @@ public MultipleMediaPickerParameterEditor(
{
DefaultConfiguration.Add("multiPicker", "1");
}

protected override IDataValueEditor CreateValueEditor() => DataValueEditorFactory.Create<MultipleMediaPickerPropertyValueEditor>(Attribute);

internal class MultipleMediaPickerPropertyValueEditor : MultiplePickerParamateterValueEditorBase
{
public MultipleMediaPickerPropertyValueEditor(ILocalizedTextService localizedTextService, IShortStringHelper shortStringHelper, IJsonSerializer jsonSerializer, IIOHelper ioHelper, DataEditorAttribute attribute, IEntityService entityService) : base(localizedTextService, shortStringHelper, jsonSerializer, ioHelper, attribute, entityService)
{
}

public override string UdiEntityType { get; } = Constants.UdiEntityType.Media;
public override UmbracoObjectTypes UmbracoObjectType { get; } = UmbracoObjectTypes.Media;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
using System;
using System.Collections.Generic;
using Umbraco.Cms.Core.IO;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.Editors;
using Umbraco.Cms.Core.Serialization;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Strings;

namespace Umbraco.Cms.Core.PropertyEditors.ParameterEditors
{
internal abstract class MultiplePickerParamateterValueEditorBase : DataValueEditor, IDataValueReference
{
private readonly IEntityService _entityService;

public MultiplePickerParamateterValueEditorBase(
ILocalizedTextService localizedTextService,
IShortStringHelper shortStringHelper,
IJsonSerializer jsonSerializer,
IIOHelper ioHelper,
DataEditorAttribute attribute,
IEntityService entityService)
: base(localizedTextService, shortStringHelper, jsonSerializer, ioHelper, attribute)
{
_entityService = entityService;
}

public abstract string UdiEntityType { get; }
public abstract UmbracoObjectTypes UmbracoObjectType { get; }
public IEnumerable<UmbracoEntityReference> GetReferences(object value)
{
var asString = value is string str ? str : value?.ToString();

if (string.IsNullOrEmpty(asString))
{
yield break;
}

foreach (var udiStr in asString.Split(','))
{
if (UdiParser.TryParse(udiStr, out Udi udi))
{
yield return new UmbracoEntityReference(udi);
}

// this is needed to support the legacy case when the multiple media picker parameter editor stores ints not udis
if (int.TryParse(udiStr, out var id))
{
Attempt<Guid> guidAttempt = _entityService.GetKey(id, UmbracoObjectType);
Guid guid = guidAttempt.Success ? guidAttempt.Result : Guid.Empty;

if (guid != Guid.Empty)
{
yield return new UmbracoEntityReference(new GuidUdi(Constants.UdiEntityType.Media, guid));
}

}
}
}
}
}
9 changes: 1 addition & 8 deletions src/Umbraco.Core/Services/IMacroService.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using Umbraco.Cms.Core.Models;

Expand All @@ -17,13 +17,6 @@ public interface IMacroService : IService
/// <returns>An <see cref="IMacro"/> object</returns>
IMacro GetByAlias(string alias);

///// <summary>
///// Gets a list all available <see cref="IMacro"/> objects
///// </summary>
///// <param name="aliases">Optional array of aliases to limit the results</param>
///// <returns>An enumerable list of <see cref="IMacro"/> objects</returns>
//IEnumerable<IMacro> GetAll(params string[] aliases);

IEnumerable<IMacro> GetAll();

IEnumerable<IMacro> GetAll(params int[] ids);
Expand Down
17 changes: 17 additions & 0 deletions src/Umbraco.Core/Services/IMacroWithAliasService.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
using System;
using System.Collections.Generic;
using Umbraco.Cms.Core.Models;

namespace Umbraco.Cms.Core.Services
{
[Obsolete("This interface will be merged with IMacroService in Umbraco 11")]
public interface IMacroWithAliasService : IMacroService
{
/// <summary>
/// Gets a list of available <see cref="IMacro"/> objects by alias.
/// </summary>
/// <param name="aliases">Optional array of aliases to limit the results</param>
/// <returns>An enumerable list of <see cref="IMacro"/> objects</returns>
IEnumerable<IMacro> GetAll(params string[] aliases);
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using Microsoft.Extensions.DependencyInjection;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Persistence.Repositories;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement;
using Umbraco.Extensions;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Services.Implement;
using Umbraco.Cms.Infrastructure.Packaging;
using Umbraco.Cms.Infrastructure.Persistence;
using Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement;
using Umbraco.Cms.Infrastructure.Services.Implement;
using Umbraco.Cms.Infrastructure.Templates;
using Umbraco.Extensions;

namespace Umbraco.Cms.Infrastructure.DependencyInjection
Expand Down Expand Up @@ -92,6 +92,7 @@ internal static IUmbracoBuilder AddServices(this IUmbracoBuilder builder)
builder.Services.AddUnique<ICreatedPackagesRepository, CreatedPackageSchemaRepository>();
builder.Services.AddSingleton<PackageDataInstallation>();
builder.Services.AddUnique<IPackageInstallation, PackageInstallation>();
builder.Services.AddUnique<IHtmlMacroParameterParser, HtmlMacroParameterParser>();

return builder;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,16 @@

namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement
{
internal class MacroRepository : EntityRepositoryBase<int, IMacro>, IMacroRepository
internal class MacroRepository : EntityRepositoryBase<int, IMacro>, IMacroWithAliasRepository
{
private readonly IShortStringHelper _shortStringHelper;
private readonly IRepositoryCachePolicy<IMacro, string> _macroByAliasCachePolicy;

public MacroRepository(IScopeAccessor scopeAccessor, AppCaches cache, ILogger<MacroRepository> logger, IShortStringHelper shortStringHelper)
: base(scopeAccessor, cache, logger)
{
_shortStringHelper = shortStringHelper;
_macroByAliasCachePolicy = new DefaultRepositoryCachePolicy<IMacro, string>(GlobalIsolatedCache, ScopeAccessor, DefaultOptions);
}

protected override IMacro PerformGet(int id)
Expand Down Expand Up @@ -68,6 +70,38 @@ public bool Exists(Guid id)
return Get(id) != null;
}

public IMacro GetByAlias(string alias)
{
return _macroByAliasCachePolicy.Get(alias, PerformGetByAlias, PerformGetAllByAlias);
}

public IEnumerable<IMacro> GetAllByAlias(string[] aliases)
{
if (aliases.Any() is false)
{
return base.GetMany();
}
Copy link
Contributor

@bjarnef bjarnef Mar 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Zeegaan shouldn't it just use Length here since the aliases is string[].

for example:

if (aliases?.Length == 0)
{
    return base.GetMany();
}

As far I know Any() check first item if collection, but with Array, List, ICollection etc, Length or Count property already know the size of the array/collection.

Copy link
Member

@Zeegaan Zeegaan Mar 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @bjarnef, thanks for your comment, i had to check how this works 👀 When it is an array, Any() will also use the Count property like this:
image
And because of that, i think its more readable if we use Any() instead of Length


return _macroByAliasCachePolicy.GetAll(aliases, PerformGetAllByAlias);
}

private IMacro PerformGetByAlias(string alias)
{
var query = Query<IMacro>().Where(x => x.Alias.Equals(alias));
return PerformGetByQuery(query).FirstOrDefault();
}

private IEnumerable<IMacro> PerformGetAllByAlias(params string[] aliases)
{
if (aliases.Any() is false)
{
return base.GetMany();
}

var query = Query<IMacro>().Where(x => aliases.Contains(x.Alias));
return PerformGetByQuery(query);
}

protected override IEnumerable<IMacro> PerformGetAll(params int[] ids)
{
return ids.Length > 0 ? ids.Select(Get) : GetAllNoIds();
Expand Down
Loading