CXuesong / WikiClientLibrary

/*🌻*/ Wiki Client Library is an asynchronous MediaWiki API client library targeting modern .NET platforms
https://github.com/CXuesong/WikiClientLibrary/wiki
Apache License 2.0
80 stars 16 forks source link

ArgumentNullException in WikiPagePropertyList<T> #67

Closed Torvin closed 4 years ago

Torvin commented 4 years ago

WikiPagePropertyList<T> doesn't correctly handle paging. Consider this request in API Sandbox for example: link.

Once the default page limit size (10 categories) is exceeded, the rest of the pages entries are returned without the categories property and WikiPagePropertyList<T>.EnumItemsAsync() fails on line 112 with System.ArgumentNullException: 'Value cannot be null. (Parameter 'source')' because of jprop being null.

Code that reproduces the issue:

static async Task Main()
{
    using var client = new WikiClient();
    var site = new WikiSite(client, "https://en.wikipedia.org/w/api.php");
    await site.Initialization;

    var items = await new CategoriesGenerator(site)
    {
        PageTitle = MediaWikiHelper.JoinValues(new[] { "Test", ".test", "Test_(Unix)", "Test_(assessment)" }),
    }.EnumItemsAsync().ToListAsync(); // throws System.ArgumentNullException
}
CXuesong commented 4 years ago

PageTitle is not designed to accept multiple pages, and I suspect I won't change my initial design to PageTitle property in the near future, as there may be more scenarios to consider, like "should I return duplicate items, if there is same category among multiple pages". Still, I'm open to discussion on this part.

That's being said, the parsing logic assumes you can have multiple pages in the response https://github.com/CXuesong/WikiClientLibrary/blob/bc52979e3d4a2db47e8d47b38e77d00f29f70702/WikiClientLibrary/Generators/Primitive/WikiPagePropertyList.cs#L108-L112 So I'll check for null here, but will keep "JoinValues on PageTitle" as some hidden magic in API.

Any comments?

Torvin commented 4 years ago

Thank you for a quick reply! Sorry I didn't know multiple titles weren't supported. I hope it will eventually be supported as it seems to be more efficient than issuing separate requests for each title? Although I don't quite understand why MediaWiki API returns all titles at once and only pages properties - this seem to only unnecessarily increase the overhead.

I guess checking for null is the right thing to do, thank you!

CXuesong commented 4 years ago

Released v0.7.0. Now you should not see this Exception anymore.