Possible null from GetJsonAsync<T>(string uri) when T is a List and the response has no data using Postgresql.

[rcpacheco]

From the revision done for issue #3041 with the help of @leigh-pointer we realized that a crash was happening while trying to display a newly activated module while trying to display its index page. The problem was that the call of GetJsonAsync(string uri), from the modules client service, did not return records and was returning a null value.
This was present only while using Postgresql as a database but not with Sqlite or Sql Server (didn't test Mysql).

My initial suggestion to close the issue was just to add some validations in the template used to create the service for modules:

But then I decided to research a bit more and made a small program to test Npgsql and EF to understand if the null result where coming from the persistence request. They both seem to work perfect with postgresql, returning an empty list or informing that no records where found. But then decided to put the EF code inside a method returning the list of results or default if no records where found, and clearly if no results are found a null is returned if T is a list, but not an empty List :

And so I realized that even if we add the validation in the Template for the GetVerdes Service, at the end the problem remains in the ServiceBase's GetJsonAsync(string uri), because the "return default" will be returning null for all T's that are List.

There are a lot of such cases in the code:

Some have a validation (like in GetSettingsAsync), but some don't, and in many cases a method is called even when the object may be null

It is clear that many of those call will never be empty under normal conditions, nevertheless may be adequate to add the validations as a defensive measure either with an if or symbols ( ? and ?? new()).
So the suggestion that I put on the table for your consideration is to add the validation in the Client template, but also to consider adding additional validations in the places where GetJsonAsync(string uri) is invoked with a type List.

[sbwalker]

@rcpacheco what I would like to understand better is this initial comment from your post above "This was present only while using Postgresql as a database but not with Sqlite or Sql Server (didn't test Mysql)." Considering that the whole point of using an ORM like EF Core is to abstract away the differences of various databases, it concerns me that PostgreSQL appears to be returning a different result from the Repository to the Controller - which is then retrieved by the Client using GetJsonAsync() - which throws an error. If the source of the problem was in GetJsonAsync() then the error should be occurring when running on all database types - not just PostgreSQL. So I would look at the Repository to try and see the differences between the results returned by EF Core.

[sbwalker]

@rcpacheco ... @leigh-pointer was able to confirm that this issue was not related to PostgreSQL - it was a problem on all databases and is related to OrderBy() throwing an exception on a null collection.

[sbwalker]

@rcpacheco I agree with your observation that anytime a GetJsonAsync() call is made and the return type is a List, then it would be a best practice to perform validation on the result to prevent potential errors.

The approach taken by @leigh-pointer in #3041 performs the validation in the Service class instance - which works fine and eliminates the Linq OrderBy() System.ArgumentNullException: Value cannot be null. (Parameter 'source') error.

I also researched how to potentially handle this in GetJsonAsync() within ServiceBase however I was not able to get the C# compiler to return an empty List within an async generic method. An alternative would be to introduce a new GetJsonAsync() method which allows the caller to pass a default value:

        protected async Task<T> GetJsonAsync<T>(string uri, T defaultvalue)
        {
            var response = await GetHttpClient().GetAsync(uri, HttpCompletionOption.ResponseHeadersRead, CancellationToken.None);
            if (await CheckResponse(response, uri) && ValidateJsonContent(response.Content))
            {
                return await response.Content.ReadFromJsonAsync<T>();
            }
            return defaultvalue;
        }

This would allow the caller to use this method when returning a list - which would ensure it always returns a consistent result type.

await GetJsonAsync<List<YourModel>>(ApiUrl, new List<YourModel>());

I am open to your thoughts if this is a good idea or not.

[leigh-pointer]

@sbwalker the problem is really with the orderBy. The way it is now is good.

We could remove the orderby and then return default. If the mod developer wants the data ordered than that can be done in Index and the template could also handle the return.

I will work on this.

[sbwalker]

@leigh-pointer the GetJsonAsync() method I included above would also solve the issue, as it would return an empty list rather than null... which means the OrderBy() would work fine and not throw an error... and there would be no need for extra null handling in the caller.

[leigh-pointer]

@sbwalker ok, cool!

[vnetonline]

I also agree supplying a default value would be cool

[sbwalker]

PR #3109 adds the new method and includes an example of how to use it (after more research it is more efficient to IEnumerable.Empty rather than new List as it avoids unnecessary heap allocations and GC):

    List<Alias> aliases = await GetJsonAsync<List<Alias>>(ApiUrl, Enumerable.Empty<Alias>().ToList());

[rcpacheco]

I really like the idea of being able to pass a default value. It allows, not only to solve this issue in an elegant way, it also avoids the need to do the extra null validation.

[sbwalker]

@leigh-pointer PR #3112 implements the new GetJsonAsync method