Skip to main content
Added example of server implementation
Source Link
almaz
  • 7.3k
  • 18
  • 30

Update 2

Example of how I would tackle this task. First of all we have a number of classes describing our interaction in a typed way:

[XmlRoot("request_authenticate")]
public class AuthenticateRequest
{
    [XmlElement("userid")]
    public int UserId { get; set; }

    [XmlElement("password")]
    public string Password { get; set; }
}

[XmlRoot("response_authenticate")]
public class AuthenticateResponse
{
    [XmlElement("status")]
    public AuthenticateStatus Status { get; set; }
}

public enum AuthenticateStatus
{
    [XmlEnum("OK")]
    Ok,
    [XmlEnum("FAILED")]
    Failed
}

Then we need some base classes to handle these requests:

public interface IHandler
{
    Type RequestType { get; }
    Type ResponseType { get; }

    object Handle(object request);
}

public abstract class BaseHandler<TRequest, TResponse> : IHandler
{
    public Type RequestType { get { return typeof(TRequest); } }

    public Type ResponseType { get { return typeof(TResponse); } }

    public abstract TResponse Handle(TRequest request);

    object IHandler.Handle(object request)
    {
        if (!(request is TRequest))
            throw new ArgumentException("request is expected to be of type ", "request");
        
        return Handle((TRequest)request);
    }
}

public interface IHandlerFactory
{
    IHandler GetHandler(string elementName);
}

public class HandlerFactory : IHandlerFactory
{
    private readonly Dictionary<string, Func<IHandler>> _mapping = new Dictionary<string, Func<IHandler>>();

    public void RegisterHandler(string requestRootName, Func<IHandler> creator)
    {
        if (creator == null)
            throw new ArgumentNullException("creator", "Factory method should be provided");

        _mapping.Add(requestRootName, creator);
    }

    public IHandler GetHandler(string elementName)
    {
        Func<IHandler> creator;
        return _mapping.TryGetValue(elementName, out creator) ? creator() : null;
    }
}

We can define handlers as following:

public class AuthenticationHandler : BaseHandler<AuthenticateRequest, AuthenticateResponse>
{
    public override AuthenticateResponse Handle(AuthenticateRequest request)
    {
        var authenticateStatus = (request.UserId == 1 && request.Password == "123456")
            ? AuthenticateStatus.Ok
            : AuthenticateStatus.Failed;

        return new AuthenticateResponse { Status = authenticateStatus };
    }
}

And finally the "server":

public class MySocketTcp : IDisposable
{
    private const string LocalHost = "127.0.0.1";

    private readonly IHandlerFactory _handlerFactory;

    private readonly IPAddress _myAddress;
    private readonly int _myPortNumber;
    private readonly TcpListener _myTcpListener;

    private bool _stopping;
    private readonly ManualResetEvent _stopHandle = new ManualResetEvent(false);

    private Thread _mainThread;

    public MySocketTcp(IHandlerFactory handlerFactory) : this(handlerFactory, IPAddress.Parse(LocalHost), 58889) { }

    public MySocketTcp(IHandlerFactory handlerFactory, IPAddress hostAddress, int portNumber)
    {
        _handlerFactory = handlerFactory;
        _myAddress = hostAddress;
        _myPortNumber = portNumber;
        _myTcpListener = new TcpListener(_myAddress, _myPortNumber);
    }

    public void Start()
    {
        if (_stopping)
            return;

        _mainThread = new Thread(Listen);
        _mainThread.Start();
    }

    private void Listen()
    {
        _myTcpListener.Start();

        while (!_stopping)
        {
            var asyncResult = _myTcpListener.BeginAcceptTcpClient(HandleIncomingTcpClient, null);
            //blocks until a client has connected to the server or stopping has been signalled
            WaitHandle.WaitAny(new[] { _stopHandle, asyncResult.AsyncWaitHandle });
        }

        _myTcpListener.Stop();
    }

    private void HandleIncomingTcpClient(IAsyncResult ar)
    {
        using (TcpClient tcpClient = _myTcpListener.EndAcceptTcpClient(ar))
        using (NetworkStream networkStream = tcpClient.GetStream())
        {
            try
            {
                XElement xml = XElement.Load(networkStream);
                ProcessXmlRequest(networkStream, xml);
            }
            catch (XmlException ex)
            {
                //TODO:log deserialization exception, notify client about error in request etc
            }
            catch (Exception ex)
            {
                //TODO:log exception, notify client about error in request processing
            }
        }
    }

    private void ProcessXmlRequest(NetworkStream networkStream, XElement xml)
    {
        var handler = _handlerFactory.GetHandler(xml.Name.LocalName);
        if (handler == null)
        {
            //TODO: notify customer about missing handler, sort of 404 error
            return;
        }

        try
        {
            XmlSerializer serializer = new XmlSerializer(handler.RequestType);
            object requestObject = serializer.Deserialize(xml.CreateReader());
            object responseObject = handler.Handle(requestObject);
            serializer = new XmlSerializer(handler.ResponseType);
            serializer.Serialize(networkStream, responseObject);
        }
        catch (InvalidOperationException ex)
        {
            //TODO: add analysis of the error
        }
    }

    public void Close()
    {
        _stopping = true;
        _stopHandle.Set();
        _mainThread.Join();
    }

    public void Dispose()
    {
        //TODO: dispose all IDisposable properties
    }
}

Usage would look like:

void Main()
{
    HandlerFactory factory = new HandlerFactory();
    factory.RegisterHandler("request_authenticate", () => new AuthenticationHandler());
    //TODO: register all other handlers here

    MySocketTcp server = new MySocketTcp(factory);
    server.Start();
}

I haven't tested this code and can't guarantee it will work on WinMobile 6.5. The main purpose of it is to demonstrate how serialization/deserialization is abstracted from request handling, and how to decouple different request processing.

Update 2

Example of how I would tackle this task. First of all we have a number of classes describing our interaction in a typed way:

[XmlRoot("request_authenticate")]
public class AuthenticateRequest
{
    [XmlElement("userid")]
    public int UserId { get; set; }

    [XmlElement("password")]
    public string Password { get; set; }
}

[XmlRoot("response_authenticate")]
public class AuthenticateResponse
{
    [XmlElement("status")]
    public AuthenticateStatus Status { get; set; }
}

public enum AuthenticateStatus
{
    [XmlEnum("OK")]
    Ok,
    [XmlEnum("FAILED")]
    Failed
}

Then we need some base classes to handle these requests:

public interface IHandler
{
    Type RequestType { get; }
    Type ResponseType { get; }

    object Handle(object request);
}

public abstract class BaseHandler<TRequest, TResponse> : IHandler
{
    public Type RequestType { get { return typeof(TRequest); } }

    public Type ResponseType { get { return typeof(TResponse); } }

    public abstract TResponse Handle(TRequest request);

    object IHandler.Handle(object request)
    {
        if (!(request is TRequest))
            throw new ArgumentException("request is expected to be of type ", "request");
        
        return Handle((TRequest)request);
    }
}

public interface IHandlerFactory
{
    IHandler GetHandler(string elementName);
}

public class HandlerFactory : IHandlerFactory
{
    private readonly Dictionary<string, Func<IHandler>> _mapping = new Dictionary<string, Func<IHandler>>();

    public void RegisterHandler(string requestRootName, Func<IHandler> creator)
    {
        if (creator == null)
            throw new ArgumentNullException("creator", "Factory method should be provided");

        _mapping.Add(requestRootName, creator);
    }

    public IHandler GetHandler(string elementName)
    {
        Func<IHandler> creator;
        return _mapping.TryGetValue(elementName, out creator) ? creator() : null;
    }
}

We can define handlers as following:

public class AuthenticationHandler : BaseHandler<AuthenticateRequest, AuthenticateResponse>
{
    public override AuthenticateResponse Handle(AuthenticateRequest request)
    {
        var authenticateStatus = (request.UserId == 1 && request.Password == "123456")
            ? AuthenticateStatus.Ok
            : AuthenticateStatus.Failed;

        return new AuthenticateResponse { Status = authenticateStatus };
    }
}

And finally the "server":

public class MySocketTcp : IDisposable
{
    private const string LocalHost = "127.0.0.1";

    private readonly IHandlerFactory _handlerFactory;

    private readonly IPAddress _myAddress;
    private readonly int _myPortNumber;
    private readonly TcpListener _myTcpListener;

    private bool _stopping;
    private readonly ManualResetEvent _stopHandle = new ManualResetEvent(false);

    private Thread _mainThread;

    public MySocketTcp(IHandlerFactory handlerFactory) : this(handlerFactory, IPAddress.Parse(LocalHost), 58889) { }

    public MySocketTcp(IHandlerFactory handlerFactory, IPAddress hostAddress, int portNumber)
    {
        _handlerFactory = handlerFactory;
        _myAddress = hostAddress;
        _myPortNumber = portNumber;
        _myTcpListener = new TcpListener(_myAddress, _myPortNumber);
    }

    public void Start()
    {
        if (_stopping)
            return;

        _mainThread = new Thread(Listen);
        _mainThread.Start();
    }

    private void Listen()
    {
        _myTcpListener.Start();

        while (!_stopping)
        {
            var asyncResult = _myTcpListener.BeginAcceptTcpClient(HandleIncomingTcpClient, null);
            //blocks until a client has connected to the server or stopping has been signalled
            WaitHandle.WaitAny(new[] { _stopHandle, asyncResult.AsyncWaitHandle });
        }

        _myTcpListener.Stop();
    }

    private void HandleIncomingTcpClient(IAsyncResult ar)
    {
        using (TcpClient tcpClient = _myTcpListener.EndAcceptTcpClient(ar))
        using (NetworkStream networkStream = tcpClient.GetStream())
        {
            try
            {
                XElement xml = XElement.Load(networkStream);
                ProcessXmlRequest(networkStream, xml);
            }
            catch (XmlException ex)
            {
                //TODO:log deserialization exception, notify client about error in request etc
            }
            catch (Exception ex)
            {
                //TODO:log exception, notify client about error in request processing
            }
        }
    }

    private void ProcessXmlRequest(NetworkStream networkStream, XElement xml)
    {
        var handler = _handlerFactory.GetHandler(xml.Name.LocalName);
        if (handler == null)
        {
            //TODO: notify customer about missing handler, sort of 404 error
            return;
        }

        try
        {
            XmlSerializer serializer = new XmlSerializer(handler.RequestType);
            object requestObject = serializer.Deserialize(xml.CreateReader());
            object responseObject = handler.Handle(requestObject);
            serializer = new XmlSerializer(handler.ResponseType);
            serializer.Serialize(networkStream, responseObject);
        }
        catch (InvalidOperationException ex)
        {
            //TODO: add analysis of the error
        }
    }

    public void Close()
    {
        _stopping = true;
        _stopHandle.Set();
        _mainThread.Join();
    }

    public void Dispose()
    {
        //TODO: dispose all IDisposable properties
    }
}

Usage would look like:

void Main()
{
    HandlerFactory factory = new HandlerFactory();
    factory.RegisterHandler("request_authenticate", () => new AuthenticationHandler());
    //TODO: register all other handlers here

    MySocketTcp server = new MySocketTcp(factory);
    server.Start();
}

I haven't tested this code and can't guarantee it will work on WinMobile 6.5. The main purpose of it is to demonstrate how serialization/deserialization is abstracted from request handling, and how to decouple different request processing.

Added explanation according to comments
Source Link
almaz
  • 7.3k
  • 18
  • 30

There are a number of naming issues here so it's quite hard to understand the code, I'll start from those:

  • public properties/events/methods should be named PascalCase (receiveMessageHandler => ReceiveMessageHandler)
  • events are named according to the time they occur. If event describes that some fact occurred, it should be named in passive voice without "Event" suffix (socketReadCompleteEvent => SocketReadCompleted)
  • private fields should be name camelCase (TimeStart => timeStart), I prefer to have underscore prefix
  • classes derived from EventArgs should have EventArgs suffix by convention
  • Async suffix is used to define methods that run asynchronously and return either Task or Task<TResult> objects. In your case ListenAsync just enters infinite loop.

Now let's move to logical issues:

  • if you catch exceptions you should do something rather than just re-throwing them.
  • re-throwing exceptions should be done with throw;, not throw ex;. You loose the stack trace if you do the latter.
  • locking should be done on private fields that are not accessible from outside (see remarks section)
  • You shouldn't expose internal streams to clients (as you do via ClientConnection.NetworkStream)
  • Why do you expose an event that returns raw byte[] and have a separate method that converts this data to string? If you plan to receive strings only just convert array to bytes straight away, otherwise leave the task to others.

Update Since you are creating a server, I would switch from event-based data handling to the structure that mimics IIS request handling via handlers: you can define an IHandler interface that allows customer to handle requests, and register a handler factory that can generate handler instance when you get a new client. That way you will call handler's method with payload and get response as a method result. Then you can serialise result and push it to the stream.

There are a number of naming issues here so it's quite hard to understand the code, I'll start from those:

  • public properties/events/methods should be named PascalCase (receiveMessageHandler => ReceiveMessageHandler)
  • events are named according to the time they occur. If event describes that some fact occurred, it should be named in passive voice without "Event" suffix (socketReadCompleteEvent => SocketReadCompleted)
  • private fields should be name camelCase (TimeStart => timeStart), I prefer to have underscore prefix
  • classes derived from EventArgs should have EventArgs suffix by convention
  • Async suffix is used to define methods that run asynchronously and return either Task or Task<TResult> objects. In your case ListenAsync just enters infinite loop.

Now let's move to logical issues:

  • if you catch exceptions you should do something rather than just re-throwing them.
  • re-throwing exceptions should be done with throw;, not throw ex;. You loose the stack trace if you do the latter.
  • locking should be done on private fields that are not accessible from outside (see remarks section)
  • You shouldn't expose internal streams to clients (as you do via ClientConnection.NetworkStream)
  • Why do you expose an event that returns raw byte[] and have a separate method that converts this data to string? If you plan to receive strings only just convert array to bytes straight away, otherwise leave the task to others.

There are a number of naming issues here so it's quite hard to understand the code, I'll start from those:

  • public properties/events/methods should be named PascalCase (receiveMessageHandler => ReceiveMessageHandler)
  • events are named according to the time they occur. If event describes that some fact occurred, it should be named in passive voice without "Event" suffix (socketReadCompleteEvent => SocketReadCompleted)
  • private fields should be name camelCase (TimeStart => timeStart), I prefer to have underscore prefix
  • classes derived from EventArgs should have EventArgs suffix by convention
  • Async suffix is used to define methods that run asynchronously and return either Task or Task<TResult> objects. In your case ListenAsync just enters infinite loop.

Now let's move to logical issues:

  • if you catch exceptions you should do something rather than just re-throwing them.
  • re-throwing exceptions should be done with throw;, not throw ex;. You loose the stack trace if you do the latter.
  • locking should be done on private fields that are not accessible from outside (see remarks section)
  • You shouldn't expose internal streams to clients (as you do via ClientConnection.NetworkStream)
  • Why do you expose an event that returns raw byte[] and have a separate method that converts this data to string? If you plan to receive strings only just convert array to bytes straight away, otherwise leave the task to others.

Update Since you are creating a server, I would switch from event-based data handling to the structure that mimics IIS request handling via handlers: you can define an IHandler interface that allows customer to handle requests, and register a handler factory that can generate handler instance when you get a new client. That way you will call handler's method with payload and get response as a method result. Then you can serialise result and push it to the stream.

Source Link
almaz
  • 7.3k
  • 18
  • 30

There are a number of naming issues here so it's quite hard to understand the code, I'll start from those:

  • public properties/events/methods should be named PascalCase (receiveMessageHandler => ReceiveMessageHandler)
  • events are named according to the time they occur. If event describes that some fact occurred, it should be named in passive voice without "Event" suffix (socketReadCompleteEvent => SocketReadCompleted)
  • private fields should be name camelCase (TimeStart => timeStart), I prefer to have underscore prefix
  • classes derived from EventArgs should have EventArgs suffix by convention
  • Async suffix is used to define methods that run asynchronously and return either Task or Task<TResult> objects. In your case ListenAsync just enters infinite loop.

Now let's move to logical issues:

  • if you catch exceptions you should do something rather than just re-throwing them.
  • re-throwing exceptions should be done with throw;, not throw ex;. You loose the stack trace if you do the latter.
  • locking should be done on private fields that are not accessible from outside (see remarks section)
  • You shouldn't expose internal streams to clients (as you do via ClientConnection.NetworkStream)
  • Why do you expose an event that returns raw byte[] and have a separate method that converts this data to string? If you plan to receive strings only just convert array to bytes straight away, otherwise leave the task to others.