Different Approach to the following ASP.NET Web API Action

Question

Maybe someone here can help me sort out this implementation and make it better.

First the following constraints:

• 1 Controller & Action that accepts all post requests, these post requests will always be XML data but can be different in the sense that the elements and the element count can differ.
• Each type of incoming XML has different rules that apply to what the system should do once a request comes in with that type of XML

Here is some code as an example, my implementation is ugly and I am hoping someone can help with making it better, and or showing me an ASP.NET Web API feature that could help with this.

public async Task<HttpResponseMessage> Post(HttpRequestMessage message)
{
 try
 {
 var content = message.Content;
 string messageContent = await content.ReadAsStringAsync();
 _logger.Info("Incomming Request: " + messageContent);
 var testSer = new XmlSerializer(typeof(MessageType));
 XmlReader testReader = XmlReader.Create(new StringReader(messageContent));
 var msgType = (MessageType)testSer.Deserialize(testReader);
 var serializer = new XmlSerializer(typeof(Strikemedia.Api.WeChat.Message.Common.TextMessage));
 var customer = _uow._customer.Single(u => u.OAWeChatID.Equals(msgType.ToUserName, StringComparison.InvariantCultureIgnoreCase));
 if (customer == null)
 {
 var error = _uow._errorLogs.Create(new DAL.Model.ErrorLog { Message = string.Format("Customer with OA ID {0} could not be found", msgType.ToUserName), Type = "Not found" });
 _uow.commit();
 return Request.CreateResponse(HttpStatusCode.BadRequest);
 }
 if (customer.CustomAssembly == null)
 {
 handler = new Handler(_logger, Request, _uow);
 }
 else
 {
 handler = ExtractPluginFromAssembly(customer.CustomAssembly);
 }
 XmlReader reader = XmlReader.Create(new StringReader(messageContent));
 if (msgType.MsgType.Equals("Text", StringComparison.OrdinalIgnoreCase))
 {
 var textMessage = (Strikemedia.Api.WeChat.Message.Common.TextMessage)serializer.Deserialize(reader);
 return await handler.HandleTextMessage(textMessage);
 }
 else
 {
 serializer = new XmlSerializer(typeof(Strikemedia.Api.WeChat.Message.Common.ImageMessage));
 if (msgType.MsgType.Equals("Image", StringComparison.OrdinalIgnoreCase))
 {
 var imageMessage = (Strikemedia.Api.WeChat.Message.Common.ImageMessage)serializer.Deserialize(reader);
 }
 else
 {
 serializer = new XmlSerializer(typeof(Strikemedia.Api.WeChat.Message.Common.AudioMessage));
 if (msgType.MsgType.Equals("Voice", StringComparison.OrdinalIgnoreCase))
 {
 var audioMessage = (Strikemedia.Api.WeChat.Message.Common.AudioMessage)serializer.Deserialize(reader);
 }
 else
 {
 serializer = new XmlSerializer(typeof(Strikemedia.Api.WeChat.Message.Common.VideoMessage));
 if (msgType.MsgType.Equals("Video", StringComparison.OrdinalIgnoreCase))
 {
 var videoMessage = (Strikemedia.Api.WeChat.Message.Common.VideoMessage)serializer.Deserialize(reader);
 }
 else
 {
 serializer = new XmlSerializer(typeof(LocationDataMessage));
 if (msgType.MsgType.Equals("Location", StringComparison.OrdinalIgnoreCase))
 {
 var locationDataMessage = (LocationDataMessage)serializer.Deserialize(reader);
 }
 else
 {
 serializer = new XmlSerializer(typeof(LinkMessage));
 if (msgType.MsgType.Equals("Link", StringComparison.OrdinalIgnoreCase))
 {
 var linkMessage = (LinkMessage)serializer.Deserialize(reader);
 }
 else
 {
 serializer = new XmlSerializer(typeof(FollowingEvent));
 if (msgType.MsgType.Equals("Event", StringComparison.OrdinalIgnoreCase))
 {
 var eventSer = new XmlSerializer(typeof(EventType));
 XmlReader eventReader = XmlReader.Create(new StringReader(messageContent));
 var eventType = (EventType)eventSer.Deserialize(eventReader);
 if (eventType.Event.Equals("subscribe", StringComparison.OrdinalIgnoreCase))
 {
 var followingEvent = (FollowingEvent)serializer.Deserialize(reader);
 return await handler.HandleFollowingEvent(followingEvent);
 }
 else if (eventType.Event.Equals("unsubscribe", StringComparison.OrdinalIgnoreCase))
 {
 var followingEvent = (FollowingEvent)serializer.Deserialize(reader);
 return await handler.HandleFollowingEvent(followingEvent);
 }
 else if (eventType.Event.Equals("location", StringComparison.OrdinalIgnoreCase))
 {
 serializer = new XmlSerializer(typeof(ReportingLocationEvent));
 if (serializer.CanDeserialize(reader))
 {
 var reportingLocationEvent = (ReportingLocationEvent)serializer.Deserialize(reader);
 }
 }
 else if (eventType.Event.Equals("click", StringComparison.OrdinalIgnoreCase))
 {
 serializer = new XmlSerializer(typeof(TextMessageEvent));
 if (serializer.CanDeserialize(reader))
 {
 var textMessageEvent = (TextMessageEvent)serializer.Deserialize(reader);
 return await handler.HandleClickEvent(textMessageEvent);
 }
 }
 }
 else
 {
 return message.CreateResponse(HttpStatusCode.BadRequest);
 }
 }
 }
 }
 }
 }
 }
 return message.CreateResponse(HttpStatusCode.OK);
 }
 catch (Exception ex)
 {
 logError(new DAL.Model.ErrorLog
 {
 Message = ex.ToString(),
 Type = "Exception"
 });
 _logger.Error(ex.ToString());
 return Request.CreateResponse(HttpStatusCode.BadRequest);
 }
}

Example xml:

<xml>
 <enentType>click</eventType>
 <fromUserId></fromUserId>
 <toUserId></toUserId>
</xml>

The classes used for the de-serialize are all sizable with the XML root being specified.

Answer 1

First you need a serializer class for serialize and deserializer method

public class Serializer
{
 public static T Deserialize<T>(string message)
 {
 var xmlSerializer = new XmlSerializer(typeof(T));
 using (var stringReader = new StringReader(message))
 {
 using (XmlReader reader = XmlReader.Create(stringReader))
 {
 return (T)xmlSerializer.Deserialize(reader);
 }
 }
 }
}

Then you need to ContentHandler which will handle all the call (above if else is not required)

public class ContentHandler
{
 public enum MessageTypes
 {
 Text,
 Image,
 Voice,
 Video,
 Location,
 Link,
 Suscribe
 }
 public async Task<Content> Process(MessageTypes messageType)
 {
 switch (messageType)
 {
 case MessageTypes.Text:
 var textMessage=Serializer.Deserialize<Strikemedia.Api.WeChat.Message.Common.TextMessage>(messageContent);
 return await handler.HandleImageMessage(textMessage);
 case MessageTypes.Voice:
 var voiceMessage=Serializer.Deserialize<Strikemedia.Api.WeChat.Message.Common.VoiceMessage>(messageContent);
 return await handler.HandleVoiceMessage(voiceMessage);
 //TODO other cases
 }
 throw new NotImplementedException();
 }
}

Then your refactored method, I am processing on the basis of the message types.

 public async Task<HttpResponseMessage> Post(HttpRequestMessage message)
 {
 var content = message.Content;
 string messageContent = await content.ReadAsStringAsync();
 var msgType = Serializer.Deserialize<MessageType>(messageContent);
 var customer = _uow._customer.Single(u => u.OAWeChatID.Equals(msgType.ToUserName, StringComparison.InvariantCultureIgnoreCase));
 if (customer == null)
 {
 var error = _uow._errorLogs.Create(new DAL.Model.ErrorLog { Message = string.Format("Customer with OA ID {0} could not be found", msgType.ToUserName), Type = "Not found" });
 _uow.commit();
 return Request.CreateResponse(HttpStatusCode.BadRequest);
 }
 if (customer.CustomAssembly == null)
 handler = new Handler(_logger, Request, _uow);
 else
 handler = ExtractPluginFromAssembly(customer.CustomAssembly);
 var contentHandler = new ContentHandler();
 var response= await contentHandler.Process((ContentHandler.MessageTypes) msgType.MsgType);
 return message.CreateResponse(HttpStatusCode.OK);
 }

One more tip for , reading the content from the HttpRequestMessage you could user some model binder , I did not find any async version of the code though so you might need to look for it.

• Jesse C. Slicer: StringReader and XmlReader both implement IDisposable and therefore should be properly deterministically disposed by employing using constructs.
• Paritosh: true ,sorry forgot to implement dispose thing

Answer 2

Much of your posted code is just unneeded. Note I am refering to the else part of theis if statement

if (msgType.MsgType.Equals("Text", StringComparison.OrdinalIgnoreCase))

You are creating objects and you are doing nothing with them like

var imageMessage = ..
var audioMessage = ..
var videoMessage = .. 

As I guess MessageType is an enum, why don't you use the enum and a switch statement ?
Also, create the serializer object at the time you know that the condition is true.
You should refactor this whole if..else monster to a separate method using the described way above.