Getting content element by name and type can fail even if valid

[tropcicstefan]

Describe the bug

When you try to get some content part lets say BlogPart from content item you can use extension method ContentExtensions.Get . Bug occurs when there are multiple valid casts of content part.

To Reproduce

Steps to reproduce the behavior:

1. make a Blog content item with BlogPart
2. in code get content part as ContentPart (base class of every part) contentItem.Get<ContentPart>("BlogPart")
3. then try to get content part as BlogPart contentItem.Get<BlogPart>("BlogPart")
4. InvalidCastException

Expected behavior

casts are valid should recast object

[MikeAlhayek]

Why are you calling contentItem.Get<ContentPart>("BlogPart") to begin with?

shouldn’t you use this instead contentItem.As<BagPart>()?

[tropcicstefan]

I am trying to read some properties from basically all parts in content item but in graphql where those parts are already loaded with dynamic providers(e.g. this).

It's kind of shady and will understand if it's not a bug for you, but on the other side it's a public extension method that can cause unwanted effect

[gvkries]

I think this is currently by design, but quite unfortunate. The first usage of Get<TElement>() determines the type for deserialization. No way to get a more specific type afterwards. Maybe we should allow to get derived types after an initial deserialization.

[MikeAlhayek]

I think this is currently by design, but quite unfortunate. The first usage of Get() determines the type for deserialization. No way to get a more specific type afterwards. Maybe we should allow to get derived types after an initial deserialization.

Maybe add a new method that does not check the cache. At least whom ever consumes it, will hopefully have a good reason to use it.

[hishamco]

shouldn’t you use this instead contentItem.As()?

I still didn't understand what was wrong with As<TPart>() or you are trying to fix an existing bug

[tropcicstefan]

shouldn’t you use this instead contentItem.As()?

I still didn't understand what was wrong with As<TPart>() or you are trying to fix an existing bug

As<> in the end uses this Get and won't work if you ever read content with different type

[gvkries]

To make it more clear, the following test will throw. Not sure if we want to allow this:

[Fact]
public void ShouldDeserializeMoreSpecificTypes()
{
    var contentItem = new ContentItem();
    var myPart = new MyPart { Text = "test" };
    contentItem.Weld(myPart);

    var json = JConvert.SerializeObject(contentItem);

    var contentItem2 = JConvert.DeserializeObject<ContentItem>(json);

    // act
    var contentPart = contentItem2.Get<ContentPart>(nameof(MyPart));
    // This will throw
    var actualMyPart = contentItem2.Get<MyPart>(nameof(MyPart));

    // assert
    Assert.NotNull(actualMyPart);
}