1
\$\begingroup\$

I am currently developing a REST service in ASP.NET Core 2.2 that acts as a wrapper upon a REST API provided by a reporting solution.

The reporting systems REST API authenticates the user based on a username and a password and subsequent requests must contain both a token and the cookies received during the authentication process.

My primary goal is optimize the traffic by caching tokens and cookies for similar requests (that use the same reporting project which has its own REST API instance) and reuse them until their expiration.

My secondary goal is to write a class that is very easy to consume by the controller which does not have even know about authentication and retrials.

I am using Api Client Generation Tools to automatically generate the code used for actual REST API calls (type MsiRestClient).

The client service

public class MsiRestClientService : IMsiRestClientService
{
    #region Constants
    private const int TokenRefreshCoolDownPeriod = 30;  // seconds
    #endregion

    #region Variables
    private static readonly object LockSync = new object();

    private static readonly SemaphoreSlim GetNewTokenSemaphore = new SemaphoreSlim(1, 1);

    /// <summary>
    /// stores all the tokens generated for each project source
    /// </summary>
    private static readonly Dictionary<string, TokenInfo> MsiProjectSourceToTokenMap = new Dictionary<string, TokenInfo>();

    private static readonly Dictionary<string, DateTime> LastSuccessfulTokenFetchTimestamps = new Dictionary<string, DateTime>();
    #endregion

    #region Properties

    // this is used to provide information about current reporting instance
    private IMsiProjectSourceService MsiProjectSourceService { get; }
    private ILoggingService Logger { get; }
    #endregion

    #region Constructor
    public MsiRestClientService(ILoggingService logger, IMsiProjectSourceService msiProjectSourceService)
    {
        Logger = logger;
        MsiProjectSourceService = msiProjectSourceService;
    }
    #endregion

    #region Private methods

    private TokenInfo GetProjectSourceTokenInfo(string projectSourceName)
    {
        lock (LockSync)
        {
            return MsiProjectSourceToTokenMap.ContainsKey(projectSourceName)
                ? MsiProjectSourceToTokenMap[projectSourceName]
                : null;
        }
    }

    private void SetProjectSourceTokenInfo(string projectSourceName, TokenInfo token)
    {
        lock (LockSync)
        {
            MsiProjectSourceToTokenMap[projectSourceName] = token;
        }
    }

    private DateTime? GetLastSuccessfulTokenFetchTimestamp(string projectSourceName)
    {
        lock (LockSync)
        {
            return LastSuccessfulTokenFetchTimestamps.ContainsKey(projectSourceName)
                ? LastSuccessfulTokenFetchTimestamps[projectSourceName]
                : (DateTime?) null;
        }
    }

    private void SetLastSuccessfulTokenFetchTimestamp(string projectSourceName, DateTime dateTime)
    {
        lock (LockSync)
        {
            LastSuccessfulTokenFetchTimestamps[projectSourceName] = dateTime;
        }
    }

    private (MsiRestClient, CookieContainer) GetMsiRestClientInfo(string projectSourceName, CookieContainer cookies = null)
    {
        // using provided cookies if any, otherwise creating new ones
        CookieContainer actualCookies = cookies ?? new CookieContainer();
        var msiHttpClientHandler = new HttpClientHandler { CookieContainer = actualCookies };
        var msiHttpClient = new HttpClient(msiHttpClientHandler);
        var msiRestClient = new MsiRestClient(msiHttpClient)
        {
            // this is required before the generator does not fetch the URL correctly
            BaseUrl = MsiProjectSourceService.GetMsiProjectSourceRestApiUrl(projectSourceName)
        };

        if (string.IsNullOrWhiteSpace(msiRestClient.BaseUrl))
            throw new ArgumentException($"No MSI Rest Api URL found for project source {projectSourceName}");

        return (msiRestClient, actualCookies);
    }

    private async Task<bool> RefreshToken(string projectSourceName)
    {
        // do not refresh if the token if it has just been successfully been refreshed recently
        // this is done to avoid mass-refresh when multiple clients want to use the service to query MSI

        var credentials = MsiProjectSourceService.GetMsiProjectSourceCredentials(projectSourceName);
        if (credentials == null)
        {
            Logger.LogError($"Failed to get credentials for project source {projectSourceName}");
            return false;
        }

        await GetNewTokenSemaphore.WaitAsync();
        try
        {
            DateTime lastSuccessfulTokenFetchTimestamp =
                GetLastSuccessfulTokenFetchTimestamp(projectSourceName) ?? new DateTime(2000, 1, 1);

            int interval = (int) (DateTime.Now - lastSuccessfulTokenFetchTimestamp).TotalSeconds;
            if (interval < TokenRefreshCoolDownPeriod)
                return true;

            var (msiRestClient, cookies) = GetMsiRestClientInfo(projectSourceName);
            var authData = new AuthRequest
            {
                Username = credentials.Username,
                Password = credentials.Password
            };

            try
            {
                await msiRestClient.PostLoginAsync(authData);
            }
            //TODO: replace with ApiException when NullReferenceException is solved
            catch (Exception exc)
            {
                Console.WriteLine("Failed to authenticate for MSI project source: " + exc);
                throw;
            }

            TokenInfo ti = new TokenInfo {Token = msiRestClient.Token, Cookies = cookies};
            SetProjectSourceTokenInfo(projectSourceName, ti);
            SetLastSuccessfulTokenFetchTimestamp(projectSourceName, DateTime.Now);
            return true;
        }
        finally
        {
            GetNewTokenSemaphore.Release();
        }
    }

    // checks if token information is available and reauthenticates if needed. Also, allows to forcefully reauthenticate (e.g. caller knows about a failure)
    private async Task<TokenInfo> EnsureTokenInfo(string projectSourceName, bool force)
    {
        var tokenInfo = GetProjectSourceTokenInfo(projectSourceName);
        if (force || tokenInfo == null)
        {
            await RefreshToken(projectSourceName);
            tokenInfo = GetProjectSourceTokenInfo(projectSourceName);
        }
        if (tokenInfo == null)
        {
            throw new ApplicationException(
                $"Failed to get cached info for project source {projectSourceName}. Should not happen since token info was just refreshed");
        }

        return tokenInfo;
    }

    private string HandleRestApiCallException(Exception e, int trial)
    {
        string errorMessage = $"Unexpected exception during Rest Api Call {trial}";
        if (e is ApiException apiExc)
        {
            return $"ExecuteWithTokenRefresh failed #{trial}: Response = {apiExc.Response}, Code = {apiExc.StatusCode}";
        }
        if (e is ArgumentNullException)
        {
            errorMessage = $"ExecuteWithTokenRefresh failed #{trial}: {e.Message}";
            Logger.LogInfo(errorMessage);
            return errorMessage;
        }
        if (e is NullReferenceException)
        {
            errorMessage = "Null reference exception received while executing ExecuteWithTokenRefresh - did you fix ApiException code?";
            Logger.LogError(errorMessage);
            return errorMessage;
        }

        return errorMessage;
    }

    private async Task<ValidationResult<TRes>> ExecuteWithTokenRefresh<TRes>(string projectSourceName, 
        Func<MsiRestClient, string, Task<TRes>> requestFunc)
    {
        var tokenInfo = await EnsureTokenInfo(projectSourceName, false);

        // creating a REST client based on data got from authentication (including cookies)
        var (msiRestClient, _) = GetMsiRestClientInfo(projectSourceName, tokenInfo.Cookies);

        try
        {
            var result = await requestFunc(msiRestClient, tokenInfo.Token);

            // no exception means that it successfully completed
            return new ValidationResult<TRes> {Payload = result};
        }
        catch (Exception e)
        {
            HandleRestApiCallException(e, 1);
        }

        // error is most probably caused by an authentication / transient REST service -> retrying
        tokenInfo = await EnsureTokenInfo(projectSourceName, true);
        var (msiRestClientBis, _) = GetMsiRestClientInfo(projectSourceName, tokenInfo.Cookies);

        string errorMessage;
        try
        {
            var result = await requestFunc(msiRestClientBis, tokenInfo.Token);

            // no exception means that it successfully completed
            return new ValidationResult<TRes> { Payload = result };
        }
        catch (Exception e)
        {
            errorMessage = HandleRestApiCallException(e, 2);
        }

        return new ValidationResult<TRes>
        {
            IsError = true,
            Message = errorMessage
        };
    }
    #endregion

    #region Public methods

    public async Task<TokenInfo> GetTokenInfo(string projectSourceName)
    {
        bool refreshResult = await RefreshToken(projectSourceName);
        if (!refreshResult)
            return null;

        return GetProjectSourceTokenInfo(projectSourceName);
    }

    // all actual calls that do not deal with authentication or retrial simply use ExecuteWithTokenRefresh to wrap the actual call
    public async Task<ValidationResult<SessionInfo>> GetCurrentUserSessionInfo(string projectSourceName)
    {
        return await ExecuteWithTokenRefresh(projectSourceName, 
            async (msiRestClient, token) => await msiRestClient.SessionSessionIdUserInfoGetAsync(token));
    }

    #endregion
}

Usage

[HttpGet("[action]")]
public async Task<ActionResult<ValidationResult<SessionInfo>>> GetCurrentUserSessionInfo(string localName = null)
{
    Logger.LogInfo("Test/GetCurrentUserSessionInfo called");

    var result = await MsiRestClientService.GetCurrentUserSessionInfo(localName);
    return new ActionResult<ValidationResult<SessionInfo>>(result);
}

Am I on the right track?

\$\endgroup\$
2
\$\begingroup\$

Review

  • Don't use regions to group members by type. This is redundant grouping. (Regions pattern or anti-pattern?)
  • Use proper naming conventions and casing of variables. LockSync is generally called syncRoot. GetNewTokenSemaphore indicates a method name, rename it to newTokenMutex. It's a mutex because you use the semaphore as a mutex.
  • Prefer TryGetValue over the two-phase ContainsKey + Indexer lookup on a Dictionary. Refactor GetProjectSourceTokenInfo and GetLastSuccessfulTokenFetchTimestamp to use this method instead.
  • GetMsiRestClientInfo is a factory method, so rename it to CreateMsiRestClientInfo.
  • GetMsiRestClientInfo creates instances of HttpClient. This class uses a socket connection and is IDisposable to manage its connection with it. But you never dispose instances of this class. Also, creating instances all the time might lead to an influx in socket connections. (HttpClient Considerations) I suggest to also provide a cache of instances, given the cookies and a dispose strategy.
  • RefreshToken mixes sandbox (return false) with error-prone (throw) statements. There is no clear specification what this method should return when. It seems a mess.
  • Using DateTime.Now to validate cache expiration is bad practice. Prefer a strategy that does not rely on your system's local time. An option is to consider StopWatch.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.