U4-3581 - Make sure advanced v7 prop editors can also be used from xslt

Created by Tim Geyssens 19 Nov 2013, 08:54:37 Updated by Dan Okkels Brendstrup 11 Dec 2014, 10:52:23

Currently if an advanced prop editor (where value type is JSON) doesn't have the ConvertDbToXml it will store it's data as json in the xml cache and this is then useless when trying to output from xslt...

Suggestion: if the value type == JSON , just use the converter by default to inject into xml cache, people can then override if they don't like the automatic way

Comments

Stephan 19 Nov 2013, 09:10:33

But... if we inject XML into the XML cache instead of JSON then the PropertyValueConverters will receive the XML text instead of the JSON value, which is not very efficient? Don't we want the new PropertyValueConverters to parse JSON rather than XML?

Guess we have to choose who we're most friendly with... if we want to be friendly with people writing simple and efficient converters to CLR types, we should use JSON, then ask XSL people to do something such as

<xsl:variable select="umbraco:library.parseJson(value)" /> ?


Tim Geyssens 19 Nov 2013, 09:16:36

Ok then we'll add an xslt extension...


Shannon Deminick 19 Nov 2013, 09:16:43

That's correct. Will need to just create an xslt extension (until the new cache is available)


Chriztian Steinmeier 19 Nov 2013, 09:26:28

I'm perfectly fine with an extension for that - a couple of things to keep in mind:

  1. In a view/template it's much easier to handle an {{}} element than any exception it may throw
  2. The extension should return the root ''element'' of the XML - e.g., by using something like {{value.Select("/*")}} instead of the ''document node''


Shannon Deminick 19 Nov 2013, 09:37:56

Can't set to feature planned as that makes it a swimlane on the agile board


Lee Kelleher 19 Nov 2013, 10:37:30

+1 @greystate on returning the root element


Shannon Deminick 20 Nov 2013, 04:15:53

I feel so dirty adding new methods to 'library' :P


Shannon Deminick 20 Nov 2013, 04:55:22

This is completed with unit tests in rev: 0c226d1a2c170e7f3888aae67dd3de85af9e2a79

You can see the tests and their outcome.


Chriztian Steinmeier 20 Nov 2013, 10:01:59

Cool, thanks - looks like it's returning the document though (as mentioned earlier) - the problem with that is that it will match the "/" template when using apply-templates (that's what the cool XSLT guys do :-) which isn't the optimal solution. Selecting '''"/*"''' instead of '''"/"''' is way better for this, because then you've got the {{}} element directly, and you can select stuff just by simple XPaths instead of having to use {{$data/json/xxx}}.

Anyway, it's great that this is made possible in a familiar way (using an extension).


Shannon Deminick 20 Nov 2013, 10:05:39

I'm happy to change whatever you'd like, just let me know! I'm no (longer) an xslt expert so can't remember how best to work with it anymore :-)


Tim Geyssens 20 Nov 2013, 11:41:01

@Chriztian So in this example http://www.nibble.be/?p=341 what would change then? It wouldn't have the json element?


Chriztian Steinmeier 20 Nov 2013, 12:09:02

@Tim - No changes necessary - copy-of will give you the same result; but it leads you to think that you can do this:

<xsl:variable name="data" select="umbraco.library:JsonToXml($currentPage/matrix)" /> <xsl:value-of select="$data/arrayitem/arrayitem[1]" />

.. and then get "xslt" as the output - but you can't - you have to use:

$data/json/arrayitem/arrayitem[1]

to get that output.

Since the {{}} element is something we wrap (to always have a single root element), it would be nicer to have the "pointer" on that by default, so we dont need to explicitly use it in every XPath expression.

@Shannon: If I submit a pull-request with the changes, we can work it out on GitHub, right?


esunxray 20 Nov 2013, 12:14:37

using item instead of arrayitem is enough.json/item/item[1]


Chriztian Steinmeier 20 Nov 2013, 12:27:09

@esunxray: That totally depends on the XML nodes' names - they are called in Tim's example, so the XPath has to use that.


Tim Geyssens 20 Nov 2013, 12:48:34

yup true, don't have a name for my json objects so it's just an array, guess that's what gets generated by default during the conversion


Shannon Deminick 20 Nov 2013, 13:22:32

Pull request would be awesome, but has to be within 24 hr with tests passing. Just keep in mind that we need to convert both objects and array


Dan Okkels Brendstrup 11 Dec 2014, 10:52:23

This extension works, but severely limits the usefulness of XPath traversing (since extension methods can't be called in {{match}} statements and in {{key}} definitions and many other places, as well as carrying a potential performance penalty). That problem can be solved the {{}} feature proposed in [pull request #588|https://github.com/umbraco/Umbraco-CMS/pull/588].


Priority: Normal

Type: Feature (request)

State: Fixed

Assignee: Shannon Deminick

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions:

Due in version: 7.0.0

Sprint:

Story Points:

Cycle: