Umbraco-cms: Not all PVCs return a correct XPath value

[ronaldbarendse]

In Umbraco 7 I'm using the following XPath query to fetch the 'Not found' error page: //error[statusCode='404'], where the statusCode property is selected from a dropdown (Umbraco.DropDown.Flexible, enable multiple choice disabled and prevalues: 404 and 500).

This XPath query doesn't work anymore in Umbraco 8, although //error does (so it's something with the statusCode='505' part). After checking the source of FlexibleDropdownPropertyValueConverter, it always returns System.String[] (the literal string value) as XPath value, because the intermediate value is a string[]:

https://github.com/umbraco/Umbraco-CMS/blob/3bfd9b71e290354744d677440983ecd06c5c0788/src/Umbraco.Web/PropertyEditors/ValueConverters/FlexibleDropdownPropertyValueConverter.cs#L19-L25

And the XPath value is returned from PropertyValueConverterBase by just doing a ToString() on it:
https://github.com/umbraco/Umbraco-CMS/blob/3bfd9b71e290354744d677440983ecd06c5c0788/src/Umbraco.Core/PropertyEditors/PropertyValueConverterBase.cs#L45-L46

Looking at some other Property Value Converters, these all seem to return wrong XPath values:

• FlexibleDropdownPropertyValueConverter returns System.String[] instead of the comma seperated values
• MediaPickerValueConverter returns Umbraco.Core.Udi[] instead of the comma seperated UDI values
• MultiNodeTreePickerValueConverter returns Umbraco.Core.Udi[] instead of the comma seperated UDI values

Reproduction

Bug summary

UmbracoContext.Cache.GetByXPath("//error[statusCode='404']") does not return content, because the XPath value from the FlexibleDropdownPropertyValueConverter isn't 404, but System.String[].

Specifics

Latest version: Umbraco 8.1.5

Steps to reproduce

• Create a document type with alias error
• Add a property statusCode using Umbraco.DropDown.Flexible, enable multiple choice disabled and prevalue: 404
• Add content using the document type, select 404 in the dropdown, publish
• Try to query by XPath: //error[statusCode='404']

Expected result

Return the content matched by the XPath query.

Actual result

Nothing returned, as the statusCode value doesn't match.

[ronaldbarendse]

As a workaround, overriding the default PVCs and returning a correct XPath value fixes the problem. So for the FlexibleDropdownPropertyValueConverter, adding the following class to your project is enough:

```c#
public class XPathFlexibleDropdownPropertyValueConverter : FlexibleDropdownPropertyValueConverter
{
public override object ConvertIntermediateToXPath(IPublishedElement owner, IPublishedPropertyType propertyType, PropertyCacheLevel referenceCacheLevel, object inter, bool preview)
{
if (inter == null)
return null;

    var selectedValues = (string[])inter;
    return string.Join(",", selectedValues);
}

}
```

[ronaldbarendse]

Maybe the easiest fix is to detect whether the intermediate value is an array/enumerable and return comma seperated values in the PropertyValueConverterBase:

c# public override object ConvertIntermediateToXPath(IPublishedElement owner, IPublishedPropertyType propertyType, PropertyCacheLevel referenceCacheLevel, object inter, bool preview) { if (inter is string) return inter; // string is an IEnumerable<char>, which we don't want to comma seperate if (inter is IEnumerable enumerable) return string.Join(",", enumerable); return inter?.ToString() ?? string.Empty; }

This might get tricky because some types implement IEnumerable, such as System.String, but that's quite similair to how its currently just doing a ToString() on the value. And PVCs can always override the default implementation anyway... Thoughts?

[zpqrtbnk]

Note: I am pretty sure that we never took the time to implement the XPath conversion for most converters. So yes, it would be plainly missing. Improving the default conversion is a good step, yes.

Note that the converter can return an XPath-navigable object, so you can either return a single string, or an Xml fragment.

[ronaldbarendse]

@zpqrtbnk Using LINQ to query content is a lot easier and with the new NuCache also more performant (from what I've heard), so I get why XPath support/compatibility was pushed back.

However, XPath is used quite a lot, e.g. for finding the error page or root node in the MNTP. In most cases it's a great way to store the query as a configuration value.

Some PVCs would indeed benefit from returning the XPath-navigable object, e.g. the Multi Url Picker or Nested Content. Do you have an example of this?

Also, what's the difference between al these methods that return an XPath value?

https://github.com/umbraco/Umbraco-CMS/blob/3bfd9b71e290354744d677440983ecd06c5c0788/src/Umbraco.Core/Models/PublishedContent/IPublishedProperty.cs#L52-L60
https://github.com/umbraco/Umbraco-CMS/blob/3bfd9b71e290354744d677440983ecd06c5c0788/src/Umbraco.Core/Models/PublishedContent/IPublishedPropertyType.cs#L76-L87
https://github.com/umbraco/Umbraco-CMS/blob/3bfd9b71e290354744d677440983ecd06c5c0788/src/Umbraco.Core/PropertyEditors/IPropertyValueConverter.cs#L88-L110

[zpqrtbnk]

Methods:

• IPropertyValueConverter.ConvertIntermediateToXPath(...) converts the "intermediate" value into an XPath value (a string or an Xml fragment, more on this later) - this is where the actual work happens.
• IPublishedPropertyType.ConvertInterToXPath(...) uses the above method from the converter for that type to convert the "intermediate" value into the XPath value, and manages the caching of the result.
• IPublishedProperty.GetXPathValue(...) returns the XPath value (a string or an Xml fragment) of a property - which is obtained via the above methods.

Which means that the only thing you need to bother implementing is the first one, the rest is plumbing that you should not need to bother about.

The method can return either a string, which will then be the property value in the Xml view of the cache, eg <firstName>John</firstName>. But it can also return an Xml fragment, as an XPathNavigator. The MultipleTextStringValueConverter in Core implements such a conversion, and returns something that, in the Xml view of the cache, will look like

<firstNames>
    <value>John</value>
    <value>Barbara</value>
</firstNames>

[ronaldbarendse]

Thanks for the explanations @zpqrtbnk! So it would actually be better to return an XPathNavigator instead of the comma seperated string, right? That would also keep it inline with the MultipleTextStringValueConverter.

The logic for creating the navigator could be moved into a helper function (protected virtual XPathNavigator ConvertToXPathNavigator(IEnumerable values)), so all PVCs can use this to return the correct value... I will start a PR for this 👍

[ronaldbarendse]

Looking at the following code from MultipleTextStringValueConverter:

https://github.com/umbraco/Umbraco-CMS/blob/3bfd9b71e290354744d677440983ecd06c5c0788/src/Umbraco.Core/PropertyEditors/ValueConverters/MultipleTextStringValueConverter.cs#L61-L76

It actually creates the following XML (for a property with alias firstNames):

<firstNames>
    <values>
        <value>John</value>
        <value>Barbara</value>
    </values>
</firstNames>

Querying would thus require the following XPath: //*[firstNames/values/value='John'] (although you can also search on node text using //*[firstNames='JohnBarbara']). The values element seems redundant and is probably just added because when creating the XPathNavigator, you require a single root element. Just changing return d.CreateNavigator(); to return e.CreateNavigator(); would fix this and only return the value elements.

So the following PVC would make querying on //*[firstNames/value='John'] work (and when only a single value can/is selected, also using //*[firstNames='John']):

```c#
public class XPathFlexibleDropdownPropertyValueConverter : FlexibleDropdownPropertyValueConverter
{
public override object ConvertIntermediateToXPath(IPublishedElement owner, IPublishedPropertyType propertyType, PropertyCacheLevel referenceCacheLevel, object inter, bool preview)
{
// The intermediate value is always an array
return ConvertToXPathNavigator((string[])inter);
}

// This method should be added to PropertyValueConverterBase
protected virtual XPathNavigator ConvertToXPathNavigator(IEnumerable<string> values)
{
    var doc = new XmlDocument();
    var root = doc.CreateElement("values");
    doc.AppendChild(root);

    foreach (var value in values)
    {
        var el = doc.CreateElement("value");
        el.InnerText = value;
        root.AppendChild(el);
    }

    return root.CreateNavigator();
}

}
```

[zpqrtbnk]

Yup, valid comment about returning values or not - and then, only the elements. Glad it works!