Skip to content
This repository has been archived by the owner on Dec 13, 2021. It is now read-only.

CurrentContent attribute and value-resolver #79

Merged
merged 14 commits into from Jun 17, 2015

Conversation

leekelleher
Copy link
Collaborator

(Closes #78)

Introduces new attribute and value-resolver to apply the current IPublishedContent to a POCO property.

An example...

public class MyModel
{
    [CurrentContent]
    public MyOtherModel MyProperty { get; set; }
}

This would be the equivalent of: foo.MyProperty = content.As<MyOtherModel>().

result = ((IPublishedContent)propertyValue).As(propertyInfo.PropertyType);

// TODO: [LK] Could this be also used to handle IEnumerable<IPublishedContent>?
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add additional check for IEnumerable< IPublishedContent> where to result type is IEnumerable< Entity> and do ditto cast there too

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this as simple as checking for propertyValue is IEnumerable<IPublishedContent>? or do we have to get funky with all the .IsGenericType checks?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, I think might be able to do IsAssignableTo

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

propertyValue.IsEnumerableOfType(typeof(IPublishedContent))

Honeymoon is next week 😉

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @JimBobSquarePants!

(See you next week - does it sound weird that we'll see you on your honeymoon?)

@mattbrailsford
Copy link
Collaborator

Just to clarify, there are two things going on here.

The [CurrentContent] attribute sets the properties resolved value as the current contents IPublishedContent. The things that then converts the value into the end entity type (MyOtherModels in Lee's example) is because a new clause has been added to the main ditto conversion code that if the raw value type is IPublishedContent and no better converter could be found for the resulting type, then it falls back to doing a Ditto .As() call.

A couple of things that play on my mind are, should the CurrentContent attribute be something more like [CurrentContentAs] and have that call .As() internally, such that it's all self contained and explicit, and doesn't rely on a "magic" fall-back (one argument could be, just using CurrentContent could result in another type converter being found to be a better match and being used instead of .As() thus producing unexpected results?, so CurrentContentAs is explicit that .As() is used).

Maybe we have two attributes

[CurrentContent]
public IPublishedContent Node { get; set; }

[CurrentContentAs]
public MyModel MyProp { get; set; }

The other thing is that I can't decide if the magic .As() fall-back in the ditto conversion code should be there? It does make sense, that if it can't find a better way to convert, and the type is IPublishedContent, then it does it's best to return the type you asked for, it's just you didn't explicitly tell it to use Ditto to resolve it.

@@ -545,6 +545,13 @@ public static class PublishedContentExtensions
// Simple types
result = propertyValue;
}
else if (propertyValue is IPublishedContent)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(from Skype) @mattbrailsford suggests that we change to:

else if (propertyValue is IPublishedContent && propertyInfo.PropertyType.IsClass)

making a note here so I don't forget later

mattbrailsford added a commit that referenced this pull request Jun 17, 2015
CurrentContent attribute and value-resolver
@mattbrailsford mattbrailsford merged commit 8c7f7bc into develop Jun 17, 2015
@mattbrailsford mattbrailsford deleted the feature/current-content-resolver branch June 17, 2015 08:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

Re-applying IPublishedContent to a POCO property
3 participants