Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HasValue1 returns false for ItemList of ListItems #419

Open
dermotblairca opened this issue Mar 16, 2022 · 2 comments
Open

HasValue1 returns false for ItemList of ListItems #419

dermotblairca opened this issue Mar 16, 2022 · 2 comments
Assignees
Labels
bug Issues describing a bug or pull requests fixing a bug.

Comments

@dermotblairca
Copy link
Contributor

Describe the bug

I'm not 100% sure if this is a bug or not but I have json which contains an ItemList of ListItems and HasValue1 returns false on the ItemListElement and HasValue3 returns true.
HasValue1 = Gets a value indicating whether the value of type IListItem has a value.
HasValue3 = Gets a value indicating whether the value of type IThing has a value.
Even though ListItem inherits from Thing, should HasValue1 not return true here since it the type of the objects is ListItem?

{
	"@context": "https://schema.org",
	"@type": "ItemList",
	"itemListElement": [
		{
			"@type": "ListItem",
			"description": "test1",
			"url": "http://test1.com",
			"position": 0
		},
		{
			"@type": "ListItem",
			"description": "test2",
			"url": "http://test2.com",
			"position": 1
		}
	]
}

Steps to reproduce

Run the following unit test:

[Fact]
    public void Deserializing_ItemListJsonLd_HasItemListReturnsTrue()
    {
        var json = "{\"@context\":\"https://schema.org\",\"@type\":\"ItemList\",\"itemListElement\":[{\"@type\":\"ListItem\",\"description\":\"test1\",\"url\":\"http://test1.com\",\"position\":0},{\"@type\":\"ListItem\",\"description\":\"test2\",\"url\":\"http://test2.com\",\"position\":1}]}";
        var itemList = SchemaSerializer.DeserializeObject<ItemList>(json);
        Assert.Equal(true, itemList?.ItemListElement.HasValue1);
    }

Expected behaviour

Not 100% sure but I thought HasValue1 would return true here.

Schema objects

https://schema.org/ItemList

@dermotblairca dermotblairca added the bug Issues describing a bug or pull requests fixing a bug. label Mar 16, 2022
@Turnerj Turnerj self-assigned this Mar 16, 2022
@Turnerj
Copy link
Collaborator

Turnerj commented Mar 19, 2022

It is technically behaving as intended though the intended behaviour isn't fantastic. Basically the ValueJsonConverter performs a "right-to-left" type check for any objects it tries to deserialize. The types for itemListElement are ListItem, Text or Thing from "left-to-right". This ordering is just from how we get the data in the schema.json files we process.

Because everything implements IThing, it succeeds with deserializing and doesn't try other types. What we could do is alter the logic to try a different algorithm for best fit - something that demotes Thing to the last possible choice.

It might seem somewhat obvious in this example that we could just switch from "right-to-left" to "left-to-right" however we'd need to confirm we aren't just creating the same problem for other schema properties.

@dermotblairca
Copy link
Contributor Author

dermotblairca commented Mar 22, 2022

Thanks for looking into it @Turnerj That makes sense.
A workaround I used was just to check if the current item "is" an IListItem, if HasValue3 returns true.

if (itemList.ItemListElement.HasValue3)
{
	foreach (var listItem in itemList.ItemListElement.Value3)
	{
		if (listItem is IListItem)
		{
			// do something with list item
		}
	}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues describing a bug or pull requests fixing a bug.
Projects
None yet
Development

No branches or pull requests

2 participants