ITitleAspect is never used.

Topics: Announcements, Core, Customizing Orchard, Troubleshooting
Sep 3, 2012 at 9:26 PM

I am looking through the code and I see that ITitleAspect is never used. Technically, I see it used in a few places, but it is always accessed through an .Get<ITitleAspect> or an .As<ITitleAspect> which will never return what you expect. This is because, when you dig a little deaper, it uses a Type.IsInstanceOf().

On the .net 4.0 framework, IsInstanceOf does not work for interfaces. It is not documented but I have written simple tests to verify this. The following xunit tests pass.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using Xunit;
namespace Miscellaneous
    public class TypeComparisons : ITypeComparisons
        public void Type_IsInstanceOfType_Interface()
        public void Interface_IsInstanceOfType_Type()
    public interface ITypeComparisons { }


Sep 12, 2012 at 10:31 PM

I'm not following you. What's your point? ITitleAspect is implemented by TitlePart.

Sep 13, 2012 at 8:13 PM
Edited Sep 13, 2012 at 8:14 PM

It is not important; I suppose that this can be filed under code-review/code-clean-up/low-priority.

As you point out, TitlePart has a Title property, but this is true regardless of whether the interface ITitleAspect exists or not. Because ITitleAspect exists, I would expect it to have a purpose. Furthermore, because it is used in the context of contentItem.As<ITitleAspect>(), I would expect that method to return an ITitleAspect. But that method is incapable of returning ITitleAspect because it uses Type.IsInstanceOfType() and Type.IsInstanceOfType() always returns false for an interface. 

Consequently, I would argue that the methods .As<TContent>(), Is<>() and Has<>() should be restricted to not allow an interface to be passed in, because they will always return null/false.

Sep 14, 2012 at 1:37 AM

Maybe. Please file a bug.

Sep 14, 2012 at 9:19 AM
Edited Sep 14, 2012 at 9:19 AM

Your test is slightly incorrect.  Looking at the code in Orchard, I think a better representation of your tests would be:


    public class TypeComparisons : ITypeComparisons
        public void Type_IsInstanceOfType_Interface()
            var testObject = new TypeComparisons();
        public void Interface_IsInstanceOfType_Type()
            var testObject = new TypeComparisons();
    public interface ITypeComparisons { }

Otherwise you're testing the RuntimeType object against your interface and class.  This stackoverflow question might help.


As for me, .As<ITitleAspect>() does seem to return the TitlePart, though it would be worth noting that this is not the recommended way to get the title of a content item, but to use DisplayText from the item's metadata.

Sep 14, 2012 at 3:57 PM

Thank you David, 

That was the piece of wisdom that I was missing. I was under the assumption that it worked like Type.IsSubclassOf(). In retrospect, the word "instance" in the method name should have tipped me off. 

On a related note, I still believe that the As<>, Is<> and Has<> should only work against parts. Creating a content-part-interface just is not consistent with the Orchard way of doing things. To me, the Orchad way of getting the Title of an object would be to get it from the TitlePart.

Furthermore, getting the title this way is non-deterministic. Suppose I add my own content part that implements ITitleAspect, the system will run my content handler and weld my part onto a content item which already has a TitlePart. At some point later, someone calls GetItemMetadata().DisplayText and that may call this:

protected override void GetItemMetadata(GetContentItemMetadataContext context) { 
  var part = context.ContentItem.As<ITitleAspect>();
  if (part != null) {               
    context.Metadata.DisplayText = part.Title;           

Which will call this:

public static T As<T>(this IContent content) where T : IContent {     
  return content == null ? default(T) : (T)content.ContentItem.Get(typeof(T));     

Which will ultimately call this:

public IContent Get(Type partType) {           
  if (partType == typeof(ContentItem))               
    return this;           
  return _parts.FirstOrDefault(partType.IsInstanceOfType);       

Whether what is returned is my custom part or the system's TitlePart is dependent upon which one is reflected on first, or last, or well, you get my point. 

More concretely, I also see that WidgetPart implements this. What happens if I have an item with both a WidgetPart and a TitlePart? Which value are we displaying?

Sep 14, 2012 at 4:39 PM

If there were multiple parts which implemented ITitleAspect, then yes, it would be non-deterministic as to which was selected (or rather, it would be the first one welded).  But a content item doesn't necessarily have a TitlePart, but may have another part which implements ITitleAspect which David Hayden has written a blog post about.

Sep 14, 2012 at 4:58 PM

Of course adding more than one part that handles the title aspect is a user mistake. There are many similar ways to shoot yourself in the foot, and this is one of the reasons why we encourage parts that are doing just one thing. Types are what aggregates content aspects that parts implement.

Sep 14, 2012 at 6:52 PM
Edited Sep 14, 2012 at 8:08 PM

So, is there no good way to enforce that all WidgetParts have a TitlePart? Perhaps something like the following pseudo code in a handler would do?

if (contentItem.Has<WidgetPart>())

This way we could further enforce that As<>, Is<> and Has<> only work in a proper Orchard fashion. I now understand that the ITitleAspect is functional, but it is inconsistent with the rest of the application. 

Sep 14, 2012 at 8:58 PM

What do you mean "anti-Orchard pattern"? How is it inconsistent? We do not necessarily want the TitlePart on all widgets, but we do want to have a friendly name for all content items.

Sep 14, 2012 at 9:37 PM
Edited Sep 14, 2012 at 9:42 PM
bertrandleroy wrote:

What do you mean "anti-Orchard pattern"? How is it inconsistent? We do not necessarily want the TitlePart on all widgets, but we do want to have a friendly name for all content items.

What is the functional difference between using ITitleAspect and attaching a TitlePart? As David Hayden's blog shows, Orchard could remove the ITitleAspect and do the same thing with a TitlePart. 

Having converted every handler in the application to a different model, I can say with some confidence that this is the only content-part-interface in the entire application. In every other case, we create a part and a handler. 

Now imagine if we started doing this for other things in the application. Why have a BodyPart, when you could create an IBodyPart which exposes the same information? You could get the same results. I put forth to you that the Orchard way of working with a body part would be to weld a BodyPart and that the non-Orchard way of working with a body part would be to implement an IBodyPart on your custom ContentParts.

As for widgets, you may not want a TitlePart on every widget, but since WidgetPart implements ITitleAspect, you have a virtual TitlePart on every widget.

I am just pointing out that this does not work like anything else in the application. 

Sep 15, 2012 at 12:19 AM

I just told you: the title does not necessarily come from a TitlePart. Here's a scenario: a content item is surfaced from data that comes from an external data store (web service, user directory, etc.). The Title part is clearly not the right solution as it's going to end up duplicating existing data, which will need to be kept in sync, etc. You can instead implement ITitleAspect and get the right display text without those problems.

What you are missing is that *you don't know the "entire application"*: different people are doing different things, with different business models, different data stores, different everything for all you know.

Body is an entirely different problem, because we don't need the body part for anything else than storing the body of a content item. There isn't a whole class of property that needs abstracting like it is the case for the display text of an item.

(and by the way we used to not have a title at all on widgets, that is a recent addition)

It doesn't work like anything else in the application because the requirements are not like anything else in the application.

Sep 15, 2012 at 12:27 AM
bertrandleroy wrote:

I just told you: the title does not necessarily come from a TitlePart. 

I seem to have missed that post. My apologies. 

Sep 15, 2012 at 12:32 AM

No no, it's me who should apologize. I thought I had said it but I haven't. Still, the argument stands.

Sep 15, 2012 at 1:09 AM

Forgiven, and I agree that the argument stands. 

Just to clarify, I did not say that I knew the entire application, I said that ITitleAspect is the only content-part-interface in the entire application, which I stand by. 

Sep 15, 2012 at 10:44 AM

Actually, there are more :-). There's IAliasAspect, ICommonPart, ILocalizableAspect and even more. This is a good pattern: by defining interfaces to implement you can use certain common content parts everywhere, without tight-coupling the consuming logic to the specific part. So e.g. instead of using CommonPart, you can define your own part implementing ICommonPart, attach it to a content type and all the logic previously using CommonPart will use your part without any modifications.

Sep 15, 2012 at 3:15 PM

Thank you Piedone. 

I stand corrected. 

I had completely overlooked that. My world view stems from rewriting the handlers and ITitleAspect is the only interface that we used in the standard cast-and-check-for-null fashion from handlers. It looks like in every other handler that works with an interface-content-part, we know what content we have before we access it.

Sep 15, 2012 at 10:48 PM

Good point, and the scenario for ICommonPart for example is exactly the same: the dates and owner on a content item can come from somewhere else.