Java Messenger data transmission

Question

This is a very simple concern, could somebody take a look at a few lines of my code? I'm writing a simple Java multi-user messenger, it is structured in a desktop client with a Swing GUI, a java server based on plain sockets and a service library holding general utilities... At the moment I transmit messages through a ObjectOutputStream/ObjectInputStream with a custom Packet class...

package org.x1c1b.carrierpigeon.service.packet;

import java.io.Serializable;
import java.util.Objects;

public class Packet implements Serializable
{
    protected String source;
    protected String destination;

    protected int type;
    protected String payload;

    public Packet(String source, String destination, PacketType type, String payload)
    {
        this.source = source;
        this.destination = destination;

        this.type = type.getIdentifier();
        this.payload = payload;
    }

    public Packet(Packet packet)
    {
        this.source = packet.source;
        this.destination = packet.destination;

        this.type = packet.type;
        this.payload = packet.payload;
    }

    protected Packet(PacketBuilder builer)
    {
        this.source = builer.source;
        this.destination = builer.destination;

        this.type = builer.type;
        this.payload = builer.payload;
    }

    public String getSource()
    {
        return this.source;
    }

    public String getDestination()
    {
        return this.destination;
    }

    public PacketType getType()
    {
        return PacketType.getByIdentifier(this.type);
    }

    public String getPayload()
    {
        return this.payload;
    }

    @Override public boolean equals(Object object)
    {
        if(this == object)
        {
            return true;
        }

        if(object == null || getClass() != object.getClass())
        {
            return false;
        }

        Packet packet = (Packet) object;

        return type == packet.type && Objects.equals(source, packet.source) && Objects
                .equals(destination, packet.destination) && Objects.equals(payload, packet.payload);
    }

    @Override public int hashCode()
    {
        return Objects.hash(this.source, this.destination, this.type, this.payload);
    }
}

This is the Packet class used to transmit the message, for identifying the message purpose I uses a Enum defining different PacketType's...

package org.x1c1b.carrierpigeon.service.packet;

import java.util.NoSuchElementException;

public enum PacketType
{
    HANDSHAKE_REQUEST(1),
    HANDSHAKE_REPLY(2),
    HANDSHAKE_ERROR(3),
    AUTHENTICATION_REQUEST(4),
    AUTHENTICATION_REPLY(5),
    AUTHENTICATION_ERROR(6),
    CONNECTIVITY_ESTABLISHED(7),
    CONNECTIVITY_HALTED(8),
    CONNECTIVITY_STATUS(9),
    CONNECTIVITY_SETUP(10),
    CONNECTIVITY_ERROR(11),
    DATA_TRANSFER(12),
    DATA_TRANSFER_ERROR(13);

    private int identifier;

    private PacketType(int identifier)
    {
        this.identifier = identifier;
    }

    public int getIdentifier()
    {
        return this.identifier;
    }

    public static PacketType getByIdentifier(int identifier)
    {
        for(PacketType type : PacketType.values())
        {
            if(type.identifier == identifier)
            {
                return type;
            }
        }

        throw new NoSuchElementException("Identifier: " + identifier);
    }
}

Maybe for taking a look at the whole project:

• Service Library Containing basic API and Packet implementations
• Server
• Desktop Client

I'm not that much expired in network development with java, so are there any improvements by transmission of messages? Is it common practice to use ObjectStream's with a custom "Protocol"/Packet to transmit data or should I use already existing protocols like HTTP instead? I tried to hold it simple without REST or huge webservers...

I will be glad about any improvements or tips for the data transmission...

EDIT

I also anticipate to write a basic API with the Remote Procedure Call pattern, here a basic example to illustrate what I mean:

public interface MessengerServiceAPI
{
    public abstract boolean login(String name);
}

Here the server-side implementation of the service API:

public class MessengerService implements MessengerServiceAPI
{
    @Override public boolean login(String name)
    {
        // Some login logic and database interaction on server-side
    }
}

Now I superior to call this server-side method with RPC through a ObjectStream:

// Method used on client-side to call remote method of service API

public Object call(String name, Object [] params) throws Exception
{
    try(Socket socket = new Socket(this.address, this.port);
            ObjectOutputStream sout = new ObjectOutputStream(socket.getOutputStream());
            ObjectInputStream sin = new ObjectInputStream(socket.getInputStream()))
    {
        sout.writeObject(name);
        sout.writeObject(params);
        sout.flush();

        Object object = sin.readObject();

        if(object instanceof Exception)
        {
            throw (Exception) object;
        }

        return object;
    }
}

// The server-side handler to handle API requests, which execute the requested action on server-side and return the return value of it

@Override public void handle(Socket socket)
{
    try (ObjectInputStream sin = new ObjectInputStream(socket.getInputStream());
                ObjectOutputStream sout = new ObjectOutputStream(socket.getOutputStream()))
    {
        Object object;
        String name = (String) sin.readObject();
        Object [] params = (Object[]) sin.readObject();

        try
        {
            Class <?> [] paramTypes = null;

            if(null != params)
            {
                types = new Class[params.length];

                for(int index = 0; index < params.length; ++index)
                {
                    paramTypes[index] = params[index].getClass();
                }
            }

            Method method = this.service.getClass().getMethod(name, paramTypes);
            object = method.invoke(this.service, params);
        }
        catch(InvocationTargetException exc)
        {
            object = exc.getTargetException();
        }
        catch(Exception exc)
        {
            object = exc;
        }

        sout.writeObject(object);
        sout.flush();
    }
    catch(Exception exc)
    {
        exc.printStackTrace();
    }
}

This is one possible implementation of such a service handling, but is it recommended to write such a basic API and service? Or is it recommended to send a string or packet instead with a identifier and the server executes the action by parsing this identifier?

Sorry because it is also in big parts software design not just code review but I'm waiting for somebody who takes a look on my code to give tips or improvements...

Answer 1

Is it common practice to use ObjectStream's with a custom "Protocol"/Packet to transmit data or should I use already existing protocols like HTTP instead?

I am not en expert and would say that it depends of your needs. However if you want to have other clients, then you may consider another protocol that would be easier to implement in another language. In the past I made two applications like your, they were all using java.net.Socket with a custom protocol (one text based and another Xml based).

Plain HTTP may not be the best choice because it is stateless and synchronous. gRpc and other Rpc mechanism can be alternatives if you want to focus on the messages more than the tranport. But be sure to choose one that allow long-lived connection between your client and the server.

is it recommended to write such a basic API and service?

I would say "no". But for the exercise yes, it can be interesting. You can also take a look at XMPP or other successful communication protocols for your inspiration. Or just write another implementation.

Or is it recommended to send a string or packet instead with a identifier and the server executes the action by parsing this identifier?

Again, it depends of your needs. But personally, being a literal one, I prefer to have "text" messages instead of identifiers. It is a kind of self documented code that is more readable than a bunch of obscure identifiers. However, "text" messages will weight more than a few bytes and than can be a problem in real-time systems.

---

Edit, some remarks after deeper code review

I reviewed UserController in the server module (#df5ba13). Despite the fact that there is no tests, there is some thing that you can do to improve your code. I am specially thinking to the front controller and chain of responsibility patterns.

A front controller will be used to manage the authentication and setup steps then route the Packet to a dediacted controller.

The chain of responsibility is let you define a set of classes dedicated to handle one command. Your command is your Packet, and you can create one handler for each kind of command : broadcast, message and a final node for error but also for authentication, setup or anything else. You just have a set of handlers and you add them next to each other, some can be composite of others so that your handling is just a kind of hierarchy where the root node is the server. The power of this pattern is that each node in the chain can decide to stop the handling or let it continue to the next nodes.

With the functional interface from Java 8, one handler can be seen as a BiConsumer that receive a Packet and his sender (User). Some can have a guard that is a simple Predicate<Packet> and you can also compose guards with the and and or methods.

To simplify a bit your code (and tests) you should simplify the chain of call to reach a method. I.e: instead of doing user.getRoom().sendPrivatePacket(user.getName(), packet) you can hide this logic between a simpler user.reply(packet) method.