0

I have a webapi service and the controller of this service is calling a remote webapi to get data.

I have a sample controller which has the below test method :

    [HttpGet("activity")]
    public ActionResult<BoredAPIDM> GetActivity()
    {
        Processor pr = new Processor();
        object data = pr.GetActivity();
        Debug.WriteLine(data); // Breakpoint
        return Ok();
    }

and the Processor class has the below method :

    public async Task GetRemoteActivity()
    {
        string baseURL = $"https://www.boredapi.com/api/activity";

        using (var client = new HttpClient())
        {

            client.BaseAddress = new Uri(baseURL);
            client.DefaultRequestHeaders.Accept.Clear();
            client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));

            HttpResponseMessage response = await client.GetAsync("activity");

            if (response.IsSuccessStatusCode)
            {
                string result = response.Content.ReadAsStringAsync().Result;
                DataTable dt = JsonConvert.DeserializeObject<DataTable>(result);
            }
            else
            {
                Debug.WriteLine($"Error calling the API from {baseURL}");
            }
        }
    }

As a result of this, what I am expecting is the data content which comes to the GetRemoteActivity() method but it does not come to GetActivity() method and the breakpoint on the debug line shows :

Id = 117, Status = WaitingForActivation, Method = "{null}", Result = "{Not yet computed}"

I feel like I am missing a whole point; what is it ?

GeoSystems
  • 35
  • 7
  • should it maybe be `pr.GetRemoteActivity()`? – dekuShrub Mar 10 '22 at 22:15
  • Is `GetActivity` just returning `GetRemoteActivity`? Because that looks like the debugger display for a `Task`, which is what `GetRemoteActivity` returns. It never returns the `DataTable` that it deserializes from the `HttpClient`. – Joshua Robinson Mar 10 '22 at 22:18
  • 1
    Your `GetActivity` method needs to be `async`. – Dai Mar 10 '22 at 23:11
  • `DataTable dt = JsonConvert.DeserializeObject(result);` <-- **ew, ew, ew** (I know JSON.NET [special-cases](https://www.newtonsoft.com/json/help/html/SerializeDataSet.htm) this), but JSON is inappropriate for tabular data: there's _so much overhead_ caused by repetitive field names, and lacks schema+type-information. If possible I recommend you change your web-service API's design to something like an annotated CSV format with type-information in the header row - which is also inherently streamable (and you can open it directly in Excel). – Dai Mar 10 '22 at 23:14
  • Thanks guys, data and serializations was for me secondary, thanks a lot for the comments. making the "caller method" in the controller solved the issues ;) Looks like I need to solidify the async programming more. – GeoSystems Mar 11 '22 at 17:44

1 Answers1

2

There is a lot missing here so I’ll try to break it down for you:

  • object data = pr.GetActivity(); should this be calling GetRemoteActivity instead?
var data = await pr.GetRemoteActivity(); // prefix the current method with async
  • public async Task GetRemoteActivity() does not return a DataTable.
public async Task<DataTable> GetRemoteActivity()

  • using (var client = new HttpClient()) should be a singleton, never newed up per method call.

  • DataTable dt = JsonConvert.DeserializeObject<DataTable>(result); Are you sure this will bind to JSON correctly? I highly doubt it but inspect the API response. Likely this is going to need a separate DTO that matches the response.

  • The debugger returns Status = WaitingForActivation because you need to await the call. Awaitable calls lazy load and do not execute until you do a proper await.

beautifulcoder
  • 10,832
  • 3
  • 19
  • 29
  • 1
    [`HttpClient` **should not** be a singleton](https://stackoverflow.com/questions/48778580/singleton-httpclient-vs-creating-new-httpclient-request): use `IHttpClientFactory`. Or if that's not an option, the next-best-thing is a `static HttpClient` that gets automatically replaced if older than, say, 5 minutes. – Dai Mar 10 '22 at 23:58
  • `DataTable dt = JsonConvert.DeserializeObject(result);` [will work because Newtonsoft.Json has special-case handling for `DataTable` and `DataSet`](https://www.newtonsoft.com/json/help/html/SerializeDataSet.htm) - however I really don't think JSON should be used for row-oriented data for reasons I mentioned in my comment-reply to the OP (also, Microsoft's first-party `System.Text.Json` library [does not support `DataTable`](https://github.com/dotnet/docs/issues/21366), so there's a potential maintainability issue there too, should the OP ever need to migrate). – Dai Mar 10 '22 at 23:59
  • All good points guys ! I will also look into IHttpClientFactory for a more efficient usage of the HttpClient. Changing the controller's caller method to async solved my issue. Would you say (as a general rule), all async calls should be linked ? i.e. if the child is async, the parent should be also async to make sure that it "awaits" the answer ? – GeoSystems Mar 11 '22 at 17:49