U4-1754 - GetProperty() method and similar extension methods should take an INode, not Node

Created by Murray Roke 20 Feb 2013, 21:25:33 Updated by Morten Christensen 04 May 2013, 19:41:55

all the extension methods in umbraco.NodeExtensions should take an INode, not Node

3 Attachments

Download 4540.patch

Download 4540_2.patch

Download 4799.patch

Comments

Sebastiaan Janssen 11 Mar 2013, 17:52:23

Hey Steven, as I suspected, anything built against 4.11.x will start breaking if these replacements are done, consider this little class: using System.Linq; using Umbraco.Core; using Umbraco.Web; using umbraco; using umbraco.NodeFactory;

namespace ClassLibrary1 { public class Class1 : IApplicationEventHandler { public void OnApplicationInitialized(UmbracoApplication httpApplication, ApplicationContext applicationContext)

    public void OnApplicationStarting(UmbracoApplication httpApplication, ApplicationContext applicationContext)
    {
    }

    public void OnApplicationStarted(UmbracoApplication httpApplication, ApplicationContext applicationContext)
    {
        var node = new Node(1048);
        node.HasProperty("bodyText");
    }
}

}

If I copy the dll produced from this to your new version, we get this: Method not found: 'Boolean umbraco.NodeExtensions.HasProperty(umbraco.NodeFactory.Node, System.String)'.

Description: An unhandled exception occurred during the execution of the current web request. Please review the stack trace for more information about the error and where it originated in the code.

Exception Details: System.MissingMethodException: Method not found: 'Boolean umbraco.NodeExtensions.HasProperty(umbraco.NodeFactory.Node, System.String)'.

Source Error:

Line 21: var node = new Node(1048); Line 22: node.HasProperty("bodyText"); Line 23: } Line 24: } Line 25: }

Notice: ".HasProperty(umbraco.NodeFactory.Node, System.String". Since it's built against a dll that only knew about the Node class, it has no idea what to do with something that now accepts an INode class.

In order to make it backwards compatible, each of these methods should have an INode overload. Make sure not to duplicate code though, keep the logic in the INode variant and cast the Node variant to INode, example:

public static bool HasProperty(this Node node, string propertyAlias) { return ((INode)node).HasProperty(propertyAlias); }

    public static bool HasProperty(this INode node, string propertyAlias)
    {
        var property = node.GetProperty(propertyAlias);
        return (property != null);
    }


Steven Lemmens 11 Mar 2013, 20:26:00

There. I added a patch that should be better. Also updated the Itellisense-comments wherever they made sense.


Sebastiaan Janssen 25 Mar 2013, 10:49:06

Thanks Steven! I've fixed the conversions to IEnumerable as they didn't work as you implemented them. Also changed GetChildNodes(this INode node, Func<Node, bool> func) to be Func<INode, bool>. The problem is: I have no idea how to use Funcs so I'm assigning this to Shannon for review.

@Shan: Import all three patches, I forgot to import the first one and .. that didn't work :) Works on current tip of 4.11.6.


Shannon Deminick 26 Mar 2013, 18:04:23

@Sebastian, are you saying that you've already imported these or I should import from scratch ?


Sebastiaan Janssen 26 Mar 2013, 20:13:05

@Shannon I have, but only locally to run some tests (I've enabled the mq extension in tortoise which allows me to strip changesets again). So yes, import from scratch! :-)


Shannon Deminick 26 Mar 2013, 21:09:14

all seems good to me, i added a comment and removed a Node cast but that was about it. Just merging and pushing now.


Morten Christensen 04 May 2013, 19:41:55

Think you missed an invalid cast that has now surfaced in 4.11.8 and 6.0.5 - see U4-2179

return GetChildNodes((INode)node, (Func<INode, bool>) func).Cast();


Priority: Normal

Type: Usability Problem

State: Fixed

Assignee: Shannon Deminick

Difficulty: Very Easy

Category:

Backwards Compatible: True

Fix Submitted: Patch

Affected versions: 4.11.4

Due in version: 4.11.6, 6.0.3

Sprint:

Story Points:

Cycle: