Preserve existing Examine FieldDefinitionCollection if it already exists (#20931)

* Preserve existing Examine FieldDefinitionCollection if it already exists (#20267)

* Fix missing bracket

* Minor tidy/addition of comments; addition of unit tests.

---------

Co-authored-by: Andy Butland <abutland73@gmail.com>
(cherry picked from commit 908974c6ac)

src/Umbraco.Examine.Lucene/DependencyInjection/ConfigureIndexOptions.cs

@@ -10,7 +10,7 @@ using Umbraco.Cms.Core.Configuration.Models;
 namespace Umbraco.Cms.Infrastructure.Examine.DependencyInjection;
 
 /// <summary>
-///     Configures the index options to construct the Examine indexes
+///     Configures the index options to construct the Examine indexes.
 /// </summary>
 public sealed class ConfigureIndexOptions : IConfigureNamedOptions<LuceneDirectoryIndexOptions>
 {
@@ -18,6 +18,9 @@ public sealed class ConfigureIndexOptions : IConfigureNamedOptions<LuceneDirecto
     private readonly IUmbracoIndexConfig _umbracoIndexConfig;
     private readonly IDeliveryApiContentIndexFieldDefinitionBuilder _deliveryApiContentIndexFieldDefinitionBuilder;
 
+    /// <summary>
+    /// Initializes a new instance of the <see cref="ConfigureIndexOptions"/> class.
+    /// </summary>
     public ConfigureIndexOptions(
         IUmbracoIndexConfig umbracoIndexConfig,
         IOptions<IndexCreatorSettings> settings,
@@ -28,24 +31,27 @@ public sealed class ConfigureIndexOptions : IConfigureNamedOptions<LuceneDirecto
         _deliveryApiContentIndexFieldDefinitionBuilder = deliveryApiContentIndexFieldDefinitionBuilder;
     }
 
+    /// <inheritdoc/>
     public void Configure(string? name, LuceneDirectoryIndexOptions options)
     {
+        // When creating FieldDefinitions with Umbraco defaults, pass in any already defined to avoid overwriting
+        // those added via a package or custom code.
         switch (name)
         {
             case Constants.UmbracoIndexes.InternalIndexName:
                 options.Analyzer = new CultureInvariantWhitespaceAnalyzer();
                 options.Validator = _umbracoIndexConfig.GetContentValueSetValidator();
-                options.FieldDefinitions = new UmbracoFieldDefinitionCollection();
+                options.FieldDefinitions = new UmbracoFieldDefinitionCollection(options.FieldDefinitions);
                 break;
             case Constants.UmbracoIndexes.ExternalIndexName:
                 options.Analyzer = new StandardAnalyzer(LuceneInfo.CurrentVersion);
                 options.Validator = _umbracoIndexConfig.GetPublishedContentValueSetValidator();
-                options.FieldDefinitions = new UmbracoFieldDefinitionCollection();
+                options.FieldDefinitions = new UmbracoFieldDefinitionCollection(options.FieldDefinitions);
                 break;
             case Constants.UmbracoIndexes.MembersIndexName:
                 options.Analyzer = new CultureInvariantWhitespaceAnalyzer();
                 options.Validator = _umbracoIndexConfig.GetMemberValueSetValidator();
-                options.FieldDefinitions = new UmbracoFieldDefinitionCollection();
+                options.FieldDefinitions = new UmbracoFieldDefinitionCollection(options.FieldDefinitions);
                 break;
             case Constants.UmbracoIndexes.DeliveryApiContentIndexName:
                 options.Analyzer = new StandardAnalyzer(LuceneInfo.CurrentVersion);
@@ -64,6 +70,7 @@ public sealed class ConfigureIndexOptions : IConfigureNamedOptions<LuceneDirecto
         }
     }
 
+    /// <inheritdoc/>
     public void Configure(LuceneDirectoryIndexOptions options)
         => throw new NotImplementedException("This is never called and is just part of the interface");
 }

src/Umbraco.Infrastructure/Examine/UmbracoFieldDefinitionCollection.cs

@@ -30,11 +30,29 @@ public class UmbracoFieldDefinitionCollection : FieldDefinitionCollection
         new(UmbracoExamineFieldNames.VariesByCultureFieldName, FieldDefinitionTypes.Raw),
     };
 
+    /// <summary>
+    /// Initializes a new instance of the <see cref="UmbracoFieldDefinitionCollection"/> class containing
+    /// the default Umbraco field definitions.
+    /// </summary>
     public UmbracoFieldDefinitionCollection()
         : base(UmbracoIndexFieldDefinitions)
     {
     }
 
+    /// <summary>
+    /// Initializes a new instance of the <see cref="UmbracoFieldDefinitionCollection"/> class containing the containing
+    /// the default Umbraco field definitions, augmented or overridden by the provided definitions.
+    /// </summary>
+    /// <param name="definitions">Existing collection of field definitions.</param>
+    public UmbracoFieldDefinitionCollection(FieldDefinitionCollection definitions)
+        : base(UmbracoIndexFieldDefinitions)
+    {
+        foreach (FieldDefinition definition in definitions)
+        {
+            AddOrUpdate(definition);
+        }
+    }
+
     /// <summary>
     ///     Overridden to dynamically add field definitions for culture variations
     /// </summary>

tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Examine/UmbracoFieldDefinitionCollectionTests.cs

@@ -0,0 +1,75 @@
+using Examine;
+using NUnit.Framework;
+using Umbraco.Cms.Infrastructure.Examine;
+
+namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Examine;
+
+[TestFixture]
+internal class UmbracoFieldDefinitionCollectionTests
+{
+    [Test]
+    public void Create_Contains_Expected_Fields()
+    {
+        var collection = new UmbracoFieldDefinitionCollection();
+        AssertDefaultField(collection);
+    }
+
+    [Test]
+    public void Create_New_Contains_Expected_Fields()
+    {
+        var collection = new UmbracoFieldDefinitionCollection();
+        collection.AddOrUpdate(new FieldDefinition("customField", "string"));
+        var collectionCount = collection.Count;
+
+        collection = new UmbracoFieldDefinitionCollection();
+        Assert.AreEqual(collectionCount - 1, collection.Count);
+        AssertDefaultField(collection);
+        AssertCustomField(collection, expectExists: false);
+    }
+
+    [Test]
+    public void Create_With_Existing_Contains_Expected_Fields()
+    {
+        var collection = new UmbracoFieldDefinitionCollection();
+        collection.AddOrUpdate(new FieldDefinition("customField", "string"));
+        var collectionCount = collection.Count;
+
+        collection = new UmbracoFieldDefinitionCollection(collection);
+        Assert.AreEqual(collectionCount, collection.Count);
+        AssertDefaultField(collection);
+        AssertCustomField(collection, expectExists: true);
+    }
+
+    [Test]
+    public void Create_With_Existing_Retains_Override_Of_DefaultField()
+    {
+        var collection = new UmbracoFieldDefinitionCollection();
+        collection.AddOrUpdate(new FieldDefinition("parentID", "string"));
+
+        collection = new UmbracoFieldDefinitionCollection(collection);
+        AssertDefaultField(collection, "string");
+    }
+
+    private static void AssertDefaultField(UmbracoFieldDefinitionCollection collection, string expectedType = "int")
+    {
+        var field = collection.SingleOrDefault(x => x.Name == "parentID");
+        Assert.IsNotNull(field);
+        Assert.AreEqual("parentID", field.Name);
+        Assert.AreEqual(expectedType, field.Type);
+    }
+
+    private static void AssertCustomField(UmbracoFieldDefinitionCollection collection, bool expectExists)
+    {
+        var field = collection.SingleOrDefault(x => x.Name == "customField");
+        if (expectExists is false)
+        {
+            Assert.IsNull(field.Name);
+            Assert.IsNull(field.Type);
+            return;
+        }
+
+        Assert.IsNotNull(field);
+        Assert.AreEqual("customField", field.Name);
+        Assert.AreEqual("string", field.Type);
+    }
+}