search

umbraco / Umbraco.Cms.Integrations

MIT License
31 stars 20 forks source link

Algolia Search - Application crashes with Stack Overflow exception when indexing media picker properties #150

Closed geann closed 7 months ago

geann commented 7 months ago

Whenever I select a property of type Umbraco.MediaPicker3 for an Algolia index and try to save changes on an indexable page, the application crashes with the following error:

Stack overflow.
Repeat 23771 times:
--------------------------------
   at Umbraco.Cms.Core.PropertyEditors.NoopPropertyIndexValueFactory.GetIndexValues(Umbraco.Cms.Core.Models.IProperty, System.String, System.String, Boolean)
--------------------------------
   at Umbraco.Cms.Integrations.Search.Algolia.Services.AlgoliaSearchPropertyIndexValueFactory.GetValue(Umbraco.Cms.Core.Models.IProperty, System.String)
   at Umbraco.Cms.Integrations.Search.Algolia.Builders.ContentRecordBuilder.BuildFromContent(Umbraco.Cms.Core.Models.IContent, System.Func`2<Umbraco.Cms.Core.Models.IProperty,Boolean>)
   at Umbraco.Cms.Integrations.Search.Algolia.Handlers.AlgoliaContentCacheRefresherHandler+<RebuildIndex>d__10.MoveNext()
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[[Umbraco.Cms.Integrations.Search.Algolia.Handlers.AlgoliaContentCacheRefresherHandler+<RebuildIndex>d__10, Umbraco.Cms.Integrations.Search.Algolia, Version=1.5.0.0, Culture=neutral, PublicKeyToken=null]](<RebuildIndex>d__10 ByRef)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder.Start[[Umbraco.Cms.Integrations.Search.Algolia.Handlers.AlgoliaContentCacheRefresherHandler+<RebuildIndex>d__10, Umbraco.Cms.Integrations.Search.Algolia, Version=1.5.0.0, Culture=neutral, PublicKeyToken=null]](<RebuildIndex>d__10 ByRef)
   at Umbraco.Cms.Integrations.Search.Algolia.Handlers.AlgoliaContentCacheRefresherHandler.RebuildIndex(System.Collections.Generic.IEnumerable`1<Umbraco.Cms.Core.Models.IContent>)
   at Umbraco.Cms.Integrations.Search.Algolia.Handlers.AlgoliaContentCacheRefresherHandler+<HandleAsync>d__9.MoveNext()
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[[Umbraco.Cms.Integrations.Search.Algolia.Handlers.AlgoliaContentCacheRefresherHandler+<HandleAsync>d__9, Umbraco.Cms.Integrations.Search.Algolia, Version=1.5.0.0, Culture=neutral, PublicKeyToken=null]](<HandleAsync>d__9 ByRef)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder.Start[[Umbraco.Cms.Integrations.Search.Algolia.Handlers.AlgoliaContentCacheRefresherHandler+<HandleAsync>d__9, Umbraco.Cms.Integrations.Search.Algolia, Version=1.5.0.0, Culture=neutral, PublicKeyToken=null]](<HandleAsync>d__9 ByRef)
   at Umbraco.Cms.Integrations.Search.Algolia.Handlers.AlgoliaContentCacheRefresherHandler.HandleAsync(Umbraco.Cms.Core.Notifications.ContentCacheRefresherNotification, System.Threading.CancellationToken)
   at Umbraco.Cms.Core.Events.INotificationAsyncHandler`1+<HandleAsync>d__1[[System.__Canon, System.Private.CoreLib, Version=7.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].MoveNext()
...

I found that the problem is in the NoopPropertyIndexValueFactory class that is used by MediaPicker3PropertyEditor. Its method GetIndexValues() just makes a recursive call instead of calling the overloaded method with the availableCultures parameter:

image

I've tried creating my own implementation of the AlgoliaSearchPropertyIndexValueFactory as described on this page to call the other method like this:

public override KeyValuePair<string, string> GetValue(IProperty property, string culture)
{
    var dataType = _dataTypeService.GetByEditorAlias(property.PropertyType.PropertyEditorAlias)
        .FirstOrDefault(p => p.Id == property.PropertyType.DataTypeId);

    if (dataType == null) return default;

    var indexValues = dataType.Editor.PropertyIndexValueFactory.GetIndexValues(property, culture, string.Empty, true, new [] {culture});

    if (indexValues == null || !indexValues.Any()) return new KeyValuePair<string, string>(property.Alias, string.Empty);
...
}

The application is no longer crashing on page save, but there is another problem - the NoopPropertyIndexValueFactory always returns an empty array as indexValues, so the method GetValue() never reaches the place where it should check available Convertors and never calls the method ConvertMediaPicker().

As a temporary workaround, I'm getting values from the Umbraco.MediaPicker3 properties using code from the DefaultPropertyIndexValueFactory:

indexValues = new [] {new KeyValuePair<string, IEnumerable<object?>>(property.Alias,new []{property.GetValue(culture, string.Empty, true)})};

All of the above is tested with Umbraco versions 12.2.0 and 12.3.0 and Algolia version 1.5.0.

So I have a couple of questions related to this issue:

This item has been added to our backlog AB#34720

acoumb commented 7 months ago

Hi @geann ,

Please check my comment here as it provides some background. I will address this with the upcoming version of Algolia.

Thank you, Adrian

acoumb commented 7 months ago

Hi @geann , Please try with the newest version of the package, using this PR as guidance.

Umbraco Docs will be updated soon as well.

Regards, Adrian