dotnet-standard-sdk: All services should expose Async methods instead of blocking calls that perform async operations internally.

Blocking async operations like HttpRequests or File IO is a bad idea. https://msdn.microsoft.com/en-us/magazine/jj991977.aspx

The internal code that uses IRequest supports Async but it is blocked right before returning to customer code. All APIs should expose Async through the API.

Example:

SpeechModel ISpeechToTextService.GetModel(string modelId, Dictionary<string, object> customData = null);

Should be:

Task<SpeechModel> ISpeechToTextService.GetModelAsync(string modelId, Dictionary<string, object> customData = null, CancellationToken cancellation = default(CancellationToken));

Implementation Example

TokenManager.GetTokenAsync example. https://github.com/watson-developer-cloud/dotnet-standard-sdk/blob/development/src/IBM.WatsonDeveloperCloud/Util/TokenManager.cs

public class TokenManager
{
	[Obsolete("This method is obsolete. Use GetTokenAsync instead.")]
	public string GetToken() => GetTokenAsync().Result;

	public async Task<string> GetTokenAsync(CancellationToken cancellation = default(CancellationToken))
	{
		if (!string.IsNullOrEmpty(_userAccessToken))
		{
			// 1. use user-managed token
			return _userAccessToken;
		}
		else if (!string.IsNullOrEmpty(_tokenInfo.AccessToken) || IsRefreshTokenExpired())
		{
			// 2. request an initial token
			var tokenInfo = await RequestTokenAsync(cancellation);
			SaveTokenInfo(tokenInfo);
			return _tokenInfo.AccessToken;
		}
		else if (this.IsTokenExpired())
		{
			// 3. refresh a token
			var tokenInfo = await RefreshTokenAsync(cancellation);
			SaveTokenInfo(tokenInfo);
			return _tokenInfo.AccessToken;
		}
		else
		{
			// 4. use valid managed token
			return _tokenInfo.AccessToken;
		}
	}

	private async Task<IamTokenData> RequestTokenAsync(CancellationToken cancellation)
	{
		IamTokenData result = null;

		try
		{
			if (string.IsNullOrEmpty(_iamApikey))
				throw new ArgumentNullException(nameof(_iamApikey));

			var request = this.Client.PostAsync(_iamUrl);
			request.WithHeader("Content-type", "application/x-www-form-urlencoded");
			request.WithHeader("Authorization", "Basic Yng6Yng=");

			List<KeyValuePair<string, string>> content = new List<KeyValuePair<string, string>>();
			KeyValuePair<string, string> grantType = new KeyValuePair<string, string>("grant_type", "urn:ibm:params:oauth:grant-type:apikey");
			KeyValuePair<string, string> responseType = new KeyValuePair<string, string>("response_type", "cloud_iam");
			KeyValuePair<string, string> apikey = new KeyValuePair<string, string>("apikey", _iamApikey);
			content.Add(grantType);
			content.Add(responseType);
			content.Add(apikey);

			var formData = new FormUrlEncodedContent(content);

			request.WithBodyContent(formData);

			result = await request.AsAsync<IamTokenData>(cancellation);

			if (result == null)
				result = new IamTokenData();
		}
		catch(AggregateException ae)
		{
			throw ae.Flatten();
		}

		return result;
	}
	//...
}

Workaround

None, the only workaround is to rewrite the services removing the blocking sections.

About this issue

Original URL
State: closed
Created 6 years ago
Reactions: 2
Comments: 22 (10 by maintainers)

Most upvoted comments

Here you go. I just added the Extensions so it would build locally.

namespace IBM.WatsonDeveloperCloud.ToneAnalyzer.v3
{
  // Extensions to get it to build, these would be removed when the base classes use Async
  internal static class Extensions
  {
    public static Task<string> GetTokenAsync(this TokenManager tokenManager, CancellationToken cancellation = default(CancellationToken))
    {
      throw new NotImplementedException("This method should be added to TokenManager.");
    }
    public static Task<T> AsAsync<T>(this IRequest request, CancellationToken cancellation = default(CancellationToken))
    {
      throw new NotImplementedException("This method should be added to IRequest.");
    }
  }

  public partial class ToneAnalyzerService : WatsonService, IToneAnalyzerService
  {
    const string SERVICE_NAME = "tone_analyzer";
    const string URL = "https://gateway.watsonplatform.net/tone-analyzer/api";
    private string _versionDate;
    public string VersionDate
    {
      get { return _versionDate; }
      set { _versionDate = value; }
    }

    public ToneAnalyzerService() : base(SERVICE_NAME, URL)
    {
      if (!string.IsNullOrEmpty(this.Endpoint))
        this.Endpoint = URL;
    }

    public ToneAnalyzerService(string userName, string password, string versionDate) : this()
    {
      if (string.IsNullOrEmpty(userName))
        throw new ArgumentNullException(nameof(userName));

      if (string.IsNullOrEmpty(password))
        throw new ArgumentNullException(nameof(password));

      this.SetCredential(userName, password);
      if (string.IsNullOrEmpty(versionDate))
        throw new ArgumentNullException("versionDate cannot be null.");

      VersionDate = versionDate;
    }

    public ToneAnalyzerService(TokenOptions options, string versionDate) : this()
    {
      if (string.IsNullOrEmpty(options.IamApiKey) && string.IsNullOrEmpty(options.IamAccessToken))
        throw new ArgumentNullException(nameof(options.IamAccessToken) + ", " + nameof(options.IamApiKey));
      if (string.IsNullOrEmpty(versionDate))
        throw new ArgumentNullException("versionDate cannot be null.");

      VersionDate = versionDate;

      if (!string.IsNullOrEmpty(options.ServiceUrl))
      {
        this.Endpoint = options.ServiceUrl;
      }
      else
      {
        options.ServiceUrl = this.Endpoint;
      }

      _tokenManager = new TokenManager(options);
    }

    public ToneAnalyzerService(IClient httpClient) : this()
    {
      if (httpClient == null)
        throw new ArgumentNullException(nameof(httpClient));

      this.Client = httpClient;
    }

    /// <summary>
    /// Analyze general tone.
    ///
    /// Use the general purpose endpoint to analyze the tone of your input content. The service analyzes the content
    /// for emotional and language tones. The method always analyzes the tone of the full document; by default, it
    /// also analyzes the tone of each individual sentence of the content.
    ///
    /// You can submit no more than 128 KB of total input content and no more than 1000 individual sentences in
    /// JSON, plain text, or HTML format. The service analyzes the first 1000 sentences for document-level analysis
    /// and only the first 100 sentences for sentence-level analysis.
    ///
    /// Per the JSON specification, the default character encoding for JSON content is effectively always UTF-8; per
    /// the HTTP specification, the default encoding for plain text and HTML is ISO-8859-1 (effectively, the ASCII
    /// character set). When specifying a content type of plain text or HTML, include the `charset` parameter to
    /// indicate the character encoding of the input text; for example: `Content-Type: text/plain;charset=utf-8`.
    /// For `text/html`, the service removes HTML tags and analyzes only the textual content.
    /// </summary>
    /// <param name="toneInput">JSON, plain text, or HTML input that contains the content to be analyzed. For JSON
    /// input, provide an object of type `ToneInput`.</param>
    /// <param name="contentType">The type of the input. A character encoding can be specified by including a
    /// `charset` parameter. For example, 'text/plain;charset=utf-8'.</param>
    /// <param name="sentences">Indicates whether the service is to return an analysis of each individual sentence
    /// in addition to its analysis of the full document. If `true` (the default), the service returns results for
    /// each sentence. (optional, default to true)</param>
    /// <param name="tones">**`2017-09-21`:** Deprecated. The service continues to accept the parameter for
    /// backward-compatibility, but the parameter no longer affects the response.
    ///
    /// **`2016-05-19`:** A comma-separated list of tones for which the service is to return its analysis of the
    /// input; the indicated tones apply both to the full document and to individual sentences of the document. You
    /// can specify one or more of the valid values. Omit the parameter to request results for all three tones.
    /// (optional)</param>
    /// <param name="contentLanguage">The language of the input text for the request: English or French. Regional
    /// variants are treated as their parent language; for example, `en-US` is interpreted as `en`. The input
    /// content must match the specified language. Do not submit content that contains both languages. You can use
    /// different languages for **Content-Language** and **Accept-Language**.
    /// * **`2017-09-21`:** Accepts `en` or `fr`.
    /// * **`2016-05-19`:** Accepts only `en`. (optional, default to en)</param>
    /// <param name="acceptLanguage">The desired language of the response. For two-character arguments, regional
    /// variants are treated as their parent language; for example, `en-US` is interpreted as `en`. You can use
    /// different languages for **Content-Language** and **Accept-Language**. (optional, default to en)</param>
    /// <param name="customData">Custom data object to pass data including custom request headers.</param>
    /// <returns><see cref="ToneAnalysis" />ToneAnalysis</returns>
    public async Task<ToneAnalysis> ToneAsync(ToneInput toneInput, string contentType, bool? sentences = null, List<string> tones = null, string contentLanguage = null, string acceptLanguage = null, Dictionary<string, object> customData = null, CancellationToken cancellation = default(CancellationToken))
    {
      if (toneInput == null)
        throw new ArgumentNullException(nameof(toneInput));
      if (string.IsNullOrEmpty(contentType))
        throw new ArgumentNullException(nameof(contentType));

      if (string.IsNullOrEmpty(VersionDate))
        throw new ArgumentNullException("versionDate cannot be null.");

      ToneAnalysis result = null;

      try
      {
        IClient client;
        if (_tokenManager == null)
        {
          client = this.Client.WithAuthentication(this.UserName, this.Password);
        }
        else
        {
          client = this.Client.WithAuthentication(await _tokenManager.GetTokenAsync(cancellation));
        }
        var restRequest = client.PostAsync($"{this.Endpoint}/v3/tone");

        restRequest.WithArgument("version", VersionDate);
        restRequest.WithHeader("Content-Type", contentType);
        restRequest.WithHeader("Content-Language", contentLanguage);
        restRequest.WithHeader("Accept-Language", acceptLanguage);
        if (sentences != null)
          restRequest.WithArgument("sentences", sentences);
        restRequest.WithArgument("tones", tones != null && tones.Count > 0 ? string.Join(",", tones.ToArray()) : null);
        restRequest.WithBody<ToneInput>(toneInput);
        if (customData != null)
          restRequest.WithCustomData(customData);
        result = await restRequest.AsAsync<ToneAnalysis>(cancellation);
        if (result == null)
          result = new ToneAnalysis();
        result.CustomData = restRequest.CustomData;
      }
      catch (AggregateException ae)
      {
        throw ae.Flatten();
      }

      return result;
    }

    /// <summary>
    /// Analyze customer engagement tone.
    ///
    /// Use the customer engagement endpoint to analyze the tone of customer service and customer support
    /// conversations. For each utterance of a conversation, the method reports the most prevalent subset of the
    /// following seven tones: sad, frustrated, satisfied, excited, polite, impolite, and sympathetic.
    ///
    /// If you submit more than 50 utterances, the service returns a warning for the overall content and analyzes
    /// only the first 50 utterances. If you submit a single utterance that contains more than 500 characters, the
    /// service returns an error for that utterance and does not analyze the utterance. The request fails if all
    /// utterances have more than 500 characters.
    ///
    /// Per the JSON specification, the default character encoding for JSON content is effectively always UTF-8.
    /// </summary>
    /// <param name="utterances">An object that contains the content to be analyzed.</param>
    /// <param name="contentLanguage">The language of the input text for the request: English or French. Regional
    /// variants are treated as their parent language; for example, `en-US` is interpreted as `en`. The input
    /// content must match the specified language. Do not submit content that contains both languages. You can use
    /// different languages for **Content-Language** and **Accept-Language**.
    /// * **`2017-09-21`:** Accepts `en` or `fr`.
    /// * **`2016-05-19`:** Accepts only `en`. (optional, default to en)</param>
    /// <param name="acceptLanguage">The desired language of the response. For two-character arguments, regional
    /// variants are treated as their parent language; for example, `en-US` is interpreted as `en`. You can use
    /// different languages for **Content-Language** and **Accept-Language**. (optional, default to en)</param>
    /// <param name="customData">Custom data object to pass data including custom request headers.</param>
    /// <returns><see cref="UtteranceAnalyses" />UtteranceAnalyses</returns>
    public async Task<UtteranceAnalyses> ToneChatAsync(ToneChatInput utterances, string contentLanguage = null, string acceptLanguage = null, Dictionary<string, object> customData = null, CancellationToken cancellation = default(CancellationToken))
    {
      if (utterances == null)
        throw new ArgumentNullException(nameof(utterances));

      if (string.IsNullOrEmpty(VersionDate))
        throw new ArgumentNullException("versionDate cannot be null.");

      UtteranceAnalyses result = null;

      try
      {
        IClient client;
        if (_tokenManager == null)
        {
          client = this.Client.WithAuthentication(this.UserName, this.Password);
        }
        else
        {
          client = this.Client.WithAuthentication(await _tokenManager.GetTokenAsync(cancellation));
        }
        var restRequest = client.PostAsync($"{this.Endpoint}/v3/tone_chat");

        restRequest.WithArgument("version", VersionDate);
        restRequest.WithHeader("Content-Language", contentLanguage);
        restRequest.WithHeader("Accept-Language", acceptLanguage);
        restRequest.WithBody<ToneChatInput>(utterances);
        if (customData != null)
          restRequest.WithCustomData(customData);
        result = await restRequest.AsAsync<UtteranceAnalyses>(cancellation);
        if (result == null)
          result = new UtteranceAnalyses();
        result.CustomData = restRequest.CustomData;
      }
      catch (AggregateException ae)
      {
        throw ae.Flatten();
      }

      return result;
    }

    [Obsolete("This method is obsolete. Use ToneAsync instead.")]
    public ToneAnalysis Tone(ToneInput toneInput, string contentType, bool? sentences = null, List<string> tones = null, string contentLanguage = null, string acceptLanguage = null, Dictionary<string, object> customData = null)
      => ToneAsync(toneInput, contentType, sentences, tones, contentLanguage, acceptLanguage, customData).Result;

    [Obsolete("This method is obsolete. Use ToneChatAsync instead.")]
    public UtteranceAnalyses ToneChat(ToneChatInput utterances, string contentLanguage = null, string acceptLanguage = null, Dictionary<string, object> customData = null)
      => ToneChatAsync(utterances, contentLanguage, acceptLanguage, customData).Result;
  }
}

acollard on Jul 12, 2018

@atilatosta can you comment on this as well?

mediumTaj on Jul 12, 2018

dotnet-standard-sdk: All services should expose Async methods instead of blocking calls that perform async operations internally.

Suggested Solution

Implementation Example

Workaround

About this issue

Most upvoted comments