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

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

Closed
geann opened this issue Nov 7, 2023 · 2 comments

Comments

@geann
Copy link

geann commented Nov 7, 2023

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:

  • Is there a better solution?
  • Please can this be resolved in the Integrations.Search.Algolia library?

This item has been added to our backlog AB#34720

@acoumb
Copy link
Contributor

acoumb commented Nov 10, 2023

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
Copy link
Contributor

acoumb commented Nov 29, 2023

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

@acoumb acoumb closed this as completed Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants