3
\$\begingroup\$

I am calling a rest API as follows. I wonder if there are ways to improve/optimize the code. Should I add time-out or cancellation token? Any help would be appreciated.

public static async Task<HttpResponseMessage> CallTestRazer(GameRequest gameRequest, string url)
{
    //FormUrlEncodedContent content = null;
    HttpResponseMessage response = null;

    //Transform GameRequest into ProductDTO
    var config = new MapperConfiguration(cfg => { cfg.CreateMap<GameRequest, ProductRequestDto>(); });
    var iMapper = config.CreateMapper();
    var productRequest = iMapper.Map<GameRequest, ProductRequestDto>(gameRequest);

    if (url == "Product/")
    {
        try
        {

            //HTTPWebRequest
            var request = (HttpWebRequest) WebRequest.Create("http://test.com/store/" + url);
            request.ContentType = "application/x-www-form-urlencoded";
            request.Method = "POST";

            var keyValueContent = productRequest.ToKeyValue();
            var formUrlEncodedContent = new FormUrlEncodedContent(keyValueContent);
            var urlEncodedString = await formUrlEncodedContent.ReadAsStringAsync();

            using (var streamWriter = new StreamWriter(await request.GetRequestStreamAsync()))
            {
                streamWriter.Write(urlEncodedString);
            }

            HttpWebResponse httpResponse = (HttpWebResponse) (await request.GetResponseAsync());

            response = new HttpResponseMessage
            {
                StatusCode = httpResponse.StatusCode,
                Content = new StreamContent(httpResponse.GetResponseStream()),
            };



            return response;

        }
        catch (Exception ex)
        {
            Console.WriteLine(ex);
            throw ;
        }
    }
    else
    {
        try
        {

            //HTTPWebRequest
            var request = (HttpWebRequest)WebRequest.Create("http://test.com/store/" + url);
            request.ContentType = "application/x-www-form-urlencoded";
            request.Method = "POST";

            var keyValueContent = gameRequest.ToKeyValue();
            var formUrlEncodedContent = new FormUrlEncodedContent(keyValueContent);
            var urlEncodedString = await formUrlEncodedContent.ReadAsStringAsync();

            using (var streamWriter = new StreamWriter(await request.GetRequestStreamAsync()))
            {
                streamWriter.Write(urlEncodedString);
            }

            HttpWebResponse httpResponse = (HttpWebResponse)(await request.GetResponseAsync());
            string json;
            using (Stream responseStream = httpResponse.GetResponseStream())
            {
                json = new StreamReader(responseStream).ReadToEnd();
            }

            response = new HttpResponseMessage
            {
                StatusCode = httpResponse.StatusCode,
                Content = new StringContent(json, System.Text.Encoding.UTF8, "application/json"),
            };



            return response;
        }
        catch (Exception e)
        {
            Console.WriteLine(e);
            throw;
        }
    }
}

Here is the ToKeyValues:

public static IDictionary<string, string> ToKeyValues(this object metaToken)
{
    if (metaToken == null)
    {
        return null;
    }

    JToken token = metaToken as JToken;
    if (token == null)
    {
        return ToKeyValues(JObject.FromObject(metaToken));
    }

    if (token.HasValues)
    {
        var contentData = new Dictionary<string, string>();

        foreach (var child in token.Children().ToList())
        {
            var childContent = child.ToKeyValues();
            if (childContent != null)
            {
                contentData = contentData.Concat(childContent).ToDictionary(k => k.Key, v => v.Value);
            }
        }

        return contentData;
    }

    var jValue = token as JValue;
    if (jValue?.Value == null)
    {
        return null;
    }

    var value = jValue?.Type == JTokenType.Date
        ? jValue?.ToString("o", CultureInfo.InvariantCulture)
        : jValue?.ToString(CultureInfo.InvariantCulture);

    return new Dictionary<string, string> {{token.Path, value}};
}
\$\endgroup\$
4
  • \$\begingroup\$ What happened to the rest of the code? Can you post the entire method? \$\endgroup\$ – t3chb0t Aug 30 '19 at 14:58
  • 1
    \$\begingroup\$ added the whole method. \$\endgroup\$ – Fake Aug 30 '19 at 15:11
  • \$\begingroup\$ @Fake I am curious as to why you are using HttpWebRequest yet still using HttpResponseMessage? Why not HttpClient? \$\endgroup\$ – Nkosi Aug 30 '19 at 18:24
  • \$\begingroup\$ @Nkosi, I was using httpclient but I got errors (System.Threading.Tasks.TaskCanceledException: A task was canceled) when I was load testing. One of the forums, somebody suggested me to use HttpWebRequest. After changing the code, the error didn't show up anymore. Long story short, I am sending HttpResponseMessage from service layer to controller. I am still using this logic in my code. \$\endgroup\$ – Fake Aug 30 '19 at 18:35
2
\$\begingroup\$

Looking at the code I would try to, make the code a bit more robust. Experiences has thought me that a perfectly good working web service sometimes fails or times-out. To capture this you can update your web.config and add the following 2 sections.

You can use this section to log Trace.WriteLine, beats Console.WriteLine as normally no one is looking at a console on a webservice and if you would the trafic would make this go so fast that you'd get an expert in speed reading

<system.diagnostics>  
  <trace autoflush="true" indentsize="4">  
    <listeners>  
      <add name="file" type="System.Diagnostics.TextWriterTraceListener" initializeData="trace.log"/>  
    </listeners>   
  </trace>  
</system.diagnostics>

You can filter what you like to capture with this section

<configuration>  
  <system.diagnostics>  
    <sources>  
      <source name="System.Net" tracemode="includehex" maxdatasize="1024">  
        <listeners>  
          <add name="System.Net"/>  
        </listeners>  
      </source>  
      <source name="System.Net.Cache">  
        <listeners>  
          <add name="System.Net"/>  
        </listeners>  
      </source>  
      <source name="System.Net.Http">  
        <listeners>  
          <add name="System.Net"/>  
        </listeners>  
      </source>  
      <source name="System.Net.Sockets">  
        <listeners>  
          <add name="System.Net"/>  
        </listeners>  
      </source>  
      <source name="System.Net.WebSockets">  
        <listeners>  
          <add name="System.Net"/>  
        </listeners>  
      </source>  
    </sources>  
    <switches>  
      <add name="System.Net" value="Verbose"/>  
      <add name="System.Net.Cache" value="Verbose"/>  
      <add name="System.Net.Http" value="Verbose"/>  
      <add name="System.Net.Sockets" value="Verbose"/>  
      <add name="System.Net.WebSockets" value="Verbose"/>  
    </switches>  
    <sharedListeners>  
      <add name="System.Net"  
        type="System.Diagnostics.TextWriterTraceListener"  
        initializeData="network.log"
        traceOutputOptions="ProcessId, DateTime" 
      />  
    </sharedListeners>  
    <trace autoflush="true"/>  
  </system.diagnostics>  
</configuration>

Unfortunately I do not have the time to go over all your code however here are some thoughts, I added some comments.

//update to capture lower case url as well as speed up Comparison
if (url.Equals("Product/", StringComparison.OrdinalIgnoreCase))
{
    int retryCount = 0;
    RETRY:
    try
    {
        retryCount++;

        //HTTPWebRequest works better with a URI then with a string as it would make the site more secure avoiding URL exploits
        var request = (HttpWebRequest)WebRequest.Create("http://test.com/store/" + url);

        request.ContentType = "application/x-www-form-urlencoded";
        request.Method = "POST";

        var keyValueContent = productRequest.ToKeyValue();
        var formUrlEncodedContent = new FormUrlEncodedContent(keyValueContent);
        var urlEncodedString = await formUrlEncodedContent.ReadAsStringAsync();

        using (var streamWriter = new StreamWriter(await request.GetRequestStreamAsync()))
        {
            streamWriter.Write(urlEncodedString);
        }

        HttpWebResponse httpResponse = (HttpWebResponse)(await request.GetResponseAsync());

        response = new HttpResponseMessage
        {
            StatusCode = httpResponse.StatusCode,
            Content = new StreamContent(httpResponse.GetResponseStream()),
        };

        //make sure you release the resource
        httpResponse.Close();

        //make sure you dispose this!
        return response;

    }
    catch (Exception ex)
    {
        if (retryCount < 3)
            goto RETRY;

        Console.WriteLine(ex);
        throw;
    }

}
\$\endgroup\$
1
  • \$\begingroup\$ It would be better if I add Task.Delay(retryInterval).Wait(); some delay in case there might be some network issue or etc. \$\endgroup\$ – Fake Sep 5 '19 at 10:29
1
\$\begingroup\$

Some quick remarks:

  • There's no need to convert the gameRequest to a productRequest, so the three lines that do that should be inside the if (url == "Product/").

  • IMHO each of the if... else blocks should be a class of their own. Considering that significant parts of those blocks are identical, those separate classes should inherit from a base class where you have a method that receives the url and the keyValueContent as parameters, and returns a httpResponse.

  • Don't do Console.WriteLine(ex);. Instead, use Trace combined with NLog, or pass an ILogger.

\$\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.