C# Async Oauth API Wrapper Class

Question

Two quick notes: I'm targeting 4.5, and the server already has newtownsoft's json library. I'd prefer not installing anything additional on the server.

using Newtonsoft.Json.Linq;
using System;
using System.Text;
using System.Collections.Generic;
using System.Net.Http;
using System.Net.Http.Headers;
using System.Threading.Tasks;
using System.Web;
using System.Web.Script.Serialization;
using System.Collections.Generic;
using System.Configuration;

namespace Sierra
{
    public class API
    {
        private string _uri, _key, _secret, _token;
        private DateTime _token_expires;
        public bool isInitialized = false;
        private static HttpClient Client = new HttpClient();

        #region Classes for json serializer

        //POCO classes for serializer

        #endregion

        /// <summary>
        /// initialize the API
        /// </summary>
        /// <param name="uri">Base uri to API endpoint</param>
        /// <param name="key">Key value </param>
        /// <param name="args">Secret value</param>
        public API(string uri, string key, string secret)
        {
            _uri = uri;
            _key = key;
            _secret = secret;
            isInitialized = GetBearerTokenAsync().Result;
        }

        public Patron GetPatronById(string barcode)
        {
            string uri = String.Format("patrons/find?barcode={0}", barcode);
            var Patron = GetAsync<Patron>(uri).Result;

            return Patron;
        }

        public Boolean PlaceHold(int patronID, string recordType, int recordNumber, string pickupLocation)
        {
            string uri = String.Format("patrons/{0}/holds/requests", patronID);

            Hold hold = new Hold();
            hold.recordType = recordType;
            hold.recordNumber = recordNumber;
            hold.pickupLocation = pickupLocation;
            string json = Newtonsoft.Json.JsonConvert.SerializeObject(hold);

            HttpResponseMessage response = PostAsync(uri, json).Result;

            return response.IsSuccessStatusCode == true;
        }

        public async Task<T> GetAsync<T>(string uri)
        {
            CheckToken();
            var content = await Client.GetStringAsync(uri);
            var serializer = new JavaScriptSerializer();
            var t = serializer.Deserialize<T>(content);

            return t;
        }

        public async Task<HttpResponseMessage> PostAsync(string uri, string json)
        {
            CheckToken();
            var response = await Client.PostAsync(uri, new StringContent(json, Encoding.UTF8, "application/json"));
            return response;
        }

        private async Task<bool> GetBearerTokenAsync()
        {
            var client      = new HttpClient();
            client.BaseAddress = new Uri(_uri);
            var request     = new HttpRequestMessage(HttpMethod.Post, "/iii/sierra-api/v2/token");

            var byteArray   = new UTF8Encoding().GetBytes(_key + ":" + _secret);
            client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Basic", Convert.ToBase64String(byteArray));

            var formData    = new List<KeyValuePair<string, string>>();
            formData.Add(new KeyValuePair<string, string>("grant_type", "client_credentials"));
            request.Content = new FormUrlEncodedContent(formData);

            var response    = await client.SendAsync(request);
            var payload     = await response.Content.ReadAsStringAsync();
            JavaScriptSerializer ser = new JavaScriptSerializer();
            SierraResponse SierraResponse = ser.Deserialize<SierraResponse>(payload);

            _token_expires  = DateTime.Now.AddSeconds(SierraResponse.expires_in);
            _token          = SierraResponse.access_token;
            client.Dispose();
            //use static HttpClient for all subsequent requests
            InitializeHttpClient();

            return true;
        }

        internal void InitializeHttpClient()
        {
            Client.BaseAddress = new Uri(_uri);
            Client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
            Client.DefaultRequestHeaders.Add("Authorization", "Bearer " + _token);
        }

        internal void CheckToken()
        {
           if (DateTime.Now >= _token_expires)
            {
                isInitialized = GetBearerTokenAsync().Result;
            }
        }

    }
}

Concerns

1. Is async done correctly

Answer 1

API class should be Api and probably sealed. Also Api isn't a meaningful name, it's an API for...what?

Fields _uri, _key and _secret are initialized only in ctor, make them readonly.

HttpClient is not thread-safe unless you call static methods, do not make it static (do not forget to implement IDisposable in your class). If you're concerned about resources usage then add a lock. .

No need to set _isInitialized to false because inside constructor you already set it.

You have a local variable named Patron, it's confusing because it has same name as Patron class and usually C# local variables are camelCase.

I'm not sure about barcode parameter content but it's not escaped, are you sure it contains only valid URL characters?

== true in response.IsSuccessStatusCode == true is unnecessary.

Why do you have a local HttpClient variable? Don't you use class field? Also do not call Dispose() directly but use using(), it will correctly dispose resources also in case of errors.

GetBearerTokenAsync() always returns true, no need for a return value then make it Task.

Is there a reason some methods are internal instead of private?

GetBearerTokenAsync() does too many things, I'd refactor out some methods (convert key, send request, parse and so on).

Also you're using DateTime.Now as expiration, did you consider that time may change during the day (for example when changing to Summer Time), you'd better to use UTC time (which is unaffected by these changes) or your token may have birth already expired (or expire one hour later than expected).

Answer 2

Do not use a public field; instead, make it a property:

public bool isInitialized = false;

---

Your code is barely above 100 lines, you don't need regions and you shouldn't use them anyway.

---

Local variables like var Patron should be camelCase.

---

Don't use Boolean, etc., instead use the alias (bool,...).

---

This:

        Hold hold = new Hold();
        hold.recordType = recordType;
        hold.recordNumber = recordNumber;
        hold.pickupLocation = pickupLocation;

... can be compacted to:

        var hold = new Hold 
        {
           recordType = recordType,
           recordNumber = recordNumber,
           pickupLocation = pickupLocation
        }

---

You're inconsistent in using var. For instance:

JavaScriptSerializer ser = new JavaScriptSerializer();

If you'd called it javaScriptSerializer instead:

var javaScriptSerializer = new JavaScriptSerializer();

---

The flow of GetBearerTokenAsync() jumps around a lot. You define request on line 3, yet don't use until line 10. Instead, you continue building client.