From 4e823982c737414c330ddbcd7e85d78d56236d4d Mon Sep 17 00:00:00 2001 From: Jonny Muir Date: Tue, 6 Dec 2022 07:17:58 +0000 Subject: [PATCH] Allow indexing variant nodes when not all variants are published - fixes issues 11383. (#12669) * This fixes issues 11383. The introduction of the new Examine library caused an unintended consequence that it stopped indexing of nodes with language variants on them when one of the variants was unpublished. These changes align ValueSetValidationStatus.Filtered to indicate that a node is intended as filtered out of a search, not that parts of its contents had been excluded from the result. This brings it inline with how it is used in Umbraco.Examine.Lucene/UmbracoContentIndex Unit tests changed to indicate the intent of ValueSetValidationStatus.Filtered Change to UmbracoViewPage to make model variable nullable (because the solution wouldn't build otherwise on 2022) * revert to use explicit type instead of var Co-authored-by: Nathan Woulfe Co-authored-by: Bjarke Berg --- .../Examine/ContentValueSetValidator.cs | 22 ++++++++++--------- .../Examine/ValueSetValidator.cs | 14 +++++++----- .../Views/UmbracoViewPage.cs | 2 +- .../UmbracoContentValueSetValidatorTests.cs | 16 ++++++++++---- 4 files changed, 33 insertions(+), 21 deletions(-) diff --git a/src/Umbraco.Infrastructure/Examine/ContentValueSetValidator.cs b/src/Umbraco.Infrastructure/Examine/ContentValueSetValidator.cs index 7120bd37d4..7f4f3cd50e 100644 --- a/src/Umbraco.Infrastructure/Examine/ContentValueSetValidator.cs +++ b/src/Umbraco.Infrastructure/Examine/ContentValueSetValidator.cs @@ -13,7 +13,7 @@ namespace Umbraco.Cms.Infrastructure.Examine; public class ContentValueSetValidator : ValueSetValidator, IContentValueSetValidator { private const string PathKey = "path"; - private static readonly IEnumerable ValidCategories = new[] {IndexTypes.Content, IndexTypes.Media}; + private static readonly IEnumerable ValidCategories = new[] { IndexTypes.Content, IndexTypes.Media }; private readonly IPublicAccessService? _publicAccessService; private readonly IScopeProvider? _scopeProvider; @@ -105,6 +105,12 @@ public class ContentValueSetValidator : ValueSetValidator, IContentValueSetValid public override ValueSetValidationResult Validate(ValueSet valueSet) { + // Notes on status on the result: + // A result status of filtered means that this whole value set result is to be filtered from the index + // For example the path is incorrect or it is in the recycle bin + // It does not mean that the values it contains have been through a filtering (for example if an language variant is not published) + // See notes on issue 11383 + ValueSetValidationResult baseValidate = base.Validate(valueSet); valueSet = baseValidate.ValueSet; if (baseValidate.Status == ValueSetValidationStatus.Failed) @@ -112,8 +118,6 @@ public class ContentValueSetValidator : ValueSetValidator, IContentValueSetValid return new ValueSetValidationResult(ValueSetValidationStatus.Failed, valueSet); } - var isFiltered = baseValidate.Status == ValueSetValidationStatus.Filtered; - var filteredValues = valueSet.Values.ToDictionary(x => x.Key, x => x.Value.ToList()); //check for published content if (valueSet.Category == IndexTypes.Content && PublishedValuesOnly) @@ -133,18 +137,15 @@ public class ContentValueSetValidator : ValueSetValidator, IContentValueSetValid && variesByCulture.Count > 0 && variesByCulture[0].Equals("y")) { //so this valueset is for a content that varies by culture, now check for non-published cultures and remove those values - foreach (KeyValuePair> publishField in valueSet.Values - .Where(x => x.Key.StartsWith($"{UmbracoExamineFieldNames.PublishedFieldName}_")).ToList()) + foreach (KeyValuePair> publishField in valueSet.Values.Where(x => x.Key.StartsWith($"{UmbracoExamineFieldNames.PublishedFieldName}_")).ToList()) { if (publishField.Value.Count <= 0 || !publishField.Value[0].Equals("y")) { //this culture is not published, so remove all of these culture values var cultureSuffix = publishField.Key.Substring(publishField.Key.LastIndexOf('_')); - foreach (KeyValuePair> cultureField in valueSet.Values - .Where(x => x.Key.InvariantEndsWith(cultureSuffix)).ToList()) + foreach (KeyValuePair> cultureField in valueSet.Values.Where(x => x.Key.InvariantEndsWith(cultureSuffix)).ToList()) { filteredValues.Remove(cultureField.Key); - isFiltered = true; } } } @@ -175,9 +176,11 @@ public class ContentValueSetValidator : ValueSetValidator, IContentValueSetValid var path = pathValues[0].ToString(); var filteredValueSet = new ValueSet(valueSet.Id, valueSet.Category, valueSet.ItemType, filteredValues.ToDictionary(x => x.Key, x => (IEnumerable)x.Value)); + // We need to validate the path of the content based on ParentId, protected content and recycle bin rules. // We cannot return FAILED here because we need the value set to get into the indexer and then deal with it from there // because we need to remove anything that doesn't pass by protected content in the cases that umbraco data is moved to an illegal parent. + // Therefore we return FILTERED to indicate this whole set needs to be filtered out if (!ValidatePath(path!, valueSet.Category) || !ValidateRecycleBin(path!, valueSet.Category) || !ValidateProtectedContent(path!, valueSet.Category)) @@ -185,7 +188,6 @@ public class ContentValueSetValidator : ValueSetValidator, IContentValueSetValid return new ValueSetValidationResult(ValueSetValidationStatus.Filtered, filteredValueSet); } - return new ValueSetValidationResult( - isFiltered ? ValueSetValidationStatus.Filtered : ValueSetValidationStatus.Valid, filteredValueSet); + return new ValueSetValidationResult(ValueSetValidationStatus.Valid, filteredValueSet); } } diff --git a/src/Umbraco.Infrastructure/Examine/ValueSetValidator.cs b/src/Umbraco.Infrastructure/Examine/ValueSetValidator.cs index 4931bd5220..0372df31f2 100644 --- a/src/Umbraco.Infrastructure/Examine/ValueSetValidator.cs +++ b/src/Umbraco.Infrastructure/Examine/ValueSetValidator.cs @@ -57,6 +57,12 @@ public class ValueSetValidator : IValueSetValidator public virtual ValueSetValidationResult Validate(ValueSet valueSet) { + /* Notes on status on the result: + * A result status of filtered means that this whole value set result is to be filtered from the index + * For example the path is incorrect or it is in the recycle bin + * It does not mean that the values it contains have been through a filtering (for example if an language variant is not published) + * See notes on issue 11383 */ + if (ValidIndexCategories != null && !ValidIndexCategories.InvariantContains(valueSet.Category)) { return new ValueSetValidationResult(ValueSetValidationStatus.Failed, valueSet); @@ -74,8 +80,6 @@ public class ValueSetValidator : IValueSetValidator return new ValueSetValidationResult(ValueSetValidationStatus.Failed, valueSet); } - var isFiltered = false; - var filteredValues = valueSet.Values.ToDictionary(x => x.Key, x => x.Value.ToList()); // filter based on the fields provided (if any) @@ -86,19 +90,17 @@ public class ValueSetValidator : IValueSetValidator if (IncludeFields != null && !IncludeFields.InvariantContains(key)) { filteredValues.Remove(key); // remove any value with a key that doesn't match the inclusion list - isFiltered = true; } if (ExcludeFields != null && ExcludeFields.InvariantContains(key)) { filteredValues.Remove(key); // remove any value with a key that matches the exclusion list - isFiltered = true; } + } } var filteredValueSet = new ValueSet(valueSet.Id, valueSet.Category, valueSet.ItemType, filteredValues.ToDictionary(x => x.Key, x => (IEnumerable)x.Value)); - return new ValueSetValidationResult( - isFiltered ? ValueSetValidationStatus.Filtered : ValueSetValidationStatus.Valid, filteredValueSet); + return new ValueSetValidationResult(ValueSetValidationStatus.Valid, filteredValueSet); } } diff --git a/src/Umbraco.Web.Common/Views/UmbracoViewPage.cs b/src/Umbraco.Web.Common/Views/UmbracoViewPage.cs index c9b64e006a..086a0b0c81 100644 --- a/src/Umbraco.Web.Common/Views/UmbracoViewPage.cs +++ b/src/Umbraco.Web.Common/Views/UmbracoViewPage.cs @@ -41,7 +41,7 @@ public abstract class UmbracoViewPage : RazorPage _helper = Context.RequestServices.GetRequiredService(); - TModel model = ViewData.Model; + TModel? model = ViewData.Model; var content = model as IPublishedContent; if (content is null && model is IContentModel contentModel) diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Examine/UmbracoContentValueSetValidatorTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Examine/UmbracoContentValueSetValidatorTests.cs index c3d357c477..e53cb0c04e 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Examine/UmbracoContentValueSetValidatorTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Examine/UmbracoContentValueSetValidatorTests.cs @@ -106,7 +106,9 @@ public class UmbracoContentValueSetValidatorTests "test-content", new { hello = "world", path = "-1,555", world = "your oyster" }); var result = validator.Validate(valueSet); - Assert.AreEqual(ValueSetValidationStatus.Filtered, result.Status); + + // Note - Result is still valid, excluded is not the same as filtered. + Assert.AreEqual(ValueSetValidationStatus.Valid, result.Status); Assert.IsFalse(result.ValueSet.Values.ContainsKey("path")); Assert.IsTrue(result.ValueSet.Values.ContainsKey("hello")); @@ -128,7 +130,9 @@ public class UmbracoContentValueSetValidatorTests "test-content", new { hello = "world", path = "-1,555", world = "your oyster" }); var result = validator.Validate(valueSet); - Assert.AreEqual(ValueSetValidationStatus.Filtered, result.Status); + + // Note - Result is still valid, excluded is not the same as filtered. + Assert.AreEqual(ValueSetValidationStatus.Valid, result.Status); Assert.IsTrue(result.ValueSet.Values.ContainsKey("path")); Assert.IsFalse(result.ValueSet.Values.ContainsKey("hello")); @@ -150,7 +154,9 @@ public class UmbracoContentValueSetValidatorTests "test-content", new { hello = "world", path = "-1,555", world = "your oyster" }); var result = validator.Validate(valueSet); - Assert.AreEqual(ValueSetValidationStatus.Filtered, result.Status); + + // Note - Result is still valid, excluded is not the same as filtered. + Assert.AreEqual(ValueSetValidationStatus.Valid, result.Status); Assert.IsFalse(result.ValueSet.Values.ContainsKey("path")); Assert.IsTrue(result.ValueSet.Values.ContainsKey("hello")); @@ -416,7 +422,9 @@ public class UmbracoContentValueSetValidatorTests Assert.IsTrue(valueSet.Values.ContainsKey("title_es-ES")); result = validator.Validate(valueSet); - Assert.AreEqual(ValueSetValidationStatus.Filtered, result.Status); + + // Note - Result is still valid, excluded is not the same as filtered. + Assert.AreEqual(ValueSetValidationStatus.Valid, result.Status); Assert.AreEqual(7, result.ValueSet.Values.Count()); // filtered to 7 values (removes es-es values) Assert.IsFalse(result.ValueSet.Values.ContainsKey($"{UmbracoExamineFieldNames.PublishedFieldName}_es-es"));