payload: {"action":"created","issue":{"url":"https://api.github.com/repos/triplea-game/triplea/issues/4724","repository_url":"https://api.github.com/repos/triplea-game/triplea","labels_url":"https://api.github.com/repos/triplea-game/triplea/issues/4724/labels{/name}","comments_url":"https://api.github.com/repos/triplea-game/triplea/issues/4724/comments","events_url":"https://api.github.com/repos/triplea-game/triplea/issues/4724/events","html_url":"https://github.com/triplea-game/triplea/issues/4724","id":416904866,"node_id":"MDU6SXNzdWU0MTY5MDQ4NjY=","number":4724,"title":"Network Serialization and Sockets (Java Socket + Https)","user":{"login":"DanVanAtta","id":12397753,"node_id":"MDQ6VXNlcjEyMzk3NzUz","avatar_url":"https://avatars2.githubusercontent.com/u/12397753?v=4","gravatar_id":"","url":"https://api.github.com/users/DanVanAtta","html_url":"https://github.com/DanVanAtta","followers_url":"https://api.github.com/users/DanVanAtta/followers","following_url":"https://api.github.com/users/DanVanAtta/following{/other_user}","gists_url":"https://api.github.com/users/DanVanAtta/gists{/gist_id}","starred_url":"https://api.github.com/users/DanVanAtta/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/DanVanAtta/subscriptions","organizations_url":"https://api.github.com/users/DanVanAtta/orgs","repos_url":"https://api.github.com/users/DanVanAtta/repos","events_url":"https://api.github.com/users/DanVanAtta/events{/privacy}","received_events_url":"https://api.github.com/users/DanVanAtta/received_events","type":"User","site_admin":false},"labels":[{"id":1204439580,"node_id":"MDU6TGFiZWwxMjA0NDM5NTgw","url":"https://api.github.com/repos/triplea-game/triplea/labels/Design","name":"Design","color":"FFDD00","default":false},{"id":317148881,"node_id":"MDU6TGFiZWwzMTcxNDg4ODE=","url":"https://api.github.com/repos/triplea-game/triplea/labels/code","name":"code","color":"2066Fb","default":false},{"id":280035309,"node_id":"MDU6TGFiZWwyODAwMzUzMDk=","url":"https://api.github.com/repos/triplea-game/triplea/labels/discussion","name":"discussion","color":"fef2c0","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2019-03-04T17:14:12Z","updated_at":"2019-03-05T20:32:23Z","closed_at":null,"author_association":"MEMBER","body":"I'm opening this issue to discuss some ideas about networking layer and to align the team on future changes.\r\n\r\nI'm thinking our two biggest needs are:\r\n- (A) encrypted/secure communication channel, ie: https\r\n- (B) simpler/easier architecture so we can make changes and more easily build additional network-enabled functionality \r\n\r\nWebsocket gives us (A), but that is only by virtue of running over 'http' with a 'nginx' reverse proxy, we get the same thing with an 'http' server. Perhaps we can simply use the http(s) server for a secure communication channel where needed, and otherwise stick to java sockets.\r\n\r\nIf we drop the desire to encrypt *all* network traffic, websocket no longer provides much benefit, and has a few critical costs:\r\n- (1) operations and performance implications, notably getting a new server and network technology up and running, well, is likely going to be a bigger challenge then we realize. For performance, this [blog post](https://medium.com/kifi-engineering/websockets-vs-regular-sockets-b3b8e7ea0708) notes a large throughput difference between websocket and plain old sockets.\r\n- (2) Parallel architecture, migration and rewrite effort. Typically parallel projects do not go well, having two different infrastructures will also make things complex if we go with current approach of building into new and slow migrating from old to new. With the latter approach, I don't think we'll ever fully get rid of the legacy network tech and would likely suffer from [lava flow](http://antipatterns.com/lavaflow.htm).  \r\n\r\n\r\n ### Key Ideas\r\n\r\n(1) Perhaps we can just improve our current socket layer, and couple it with an https server to get secure communication as needed.\r\n\r\nI like this idea because I think it's a very feasible direction and targets both of our needs (A) and (B). We already have an http server with an https nginx proxy in front of it, so we have the second half far on its way.\r\n\r\nOne of the biggest challenges of websocket when I tried to code it up, I found was really to be defining an application level communication protocol. While we could spend solid time building something nice, I think we'll find our design could easily break down when we try to migrate all java socket code to it, and secondly the migration of existing code to new could become exceedingly painful. I'm much more convinced if we can find a pattern to convert and improve the existing code rather than rewriting it in parallel.\r\n\r\nAs noted, we are far along with the https server, but I think we are actually in a good position to convert most of the reflective code to something that is application driven. Please note the `Serializable` payload in [MessageHeader](https://github.com/triplea-game/triplea/blob/master/game-core/src/main/java/games/strategy/net/MessageHeader.java#L12). I'm thinking if we change the `Serializable` to a `String`, we can instead encode an 'envelope' class as a JSON string. That 'envelope' class would have at least these two fields:\r\n\r\n```\r\n  private final String jsonString;\r\n  private final String typeIdentifer;\r\n```\r\nFrom here, we can have a mapping of type identifier to actual types, and use JSON to decode to that type. This removes most reflection at the network layer, we'd have a java reflection dependency on `MessageHeader` only. Most network interactions could also then be traced much deeper in the code, to where we map a type identifier to a type, and that should make it much easier to build on new features. I'm also thinking we could introduce 'Version' at the next release to the JSON encoding and decoding to allow network types to handle version specific encodings. \r\n\r\nTo sum up key benefits:\r\n- remove most Java reflection used at the application network layer\r\n- get better compile time guarantees and can trace code to a deeper layer\r\n- more flexibility when handling version specific network traffic.\r\n- we'll be able to refactor code in place\r\n- avoids introducing a new, large architectural component, a whole new server for websockets.\r\n\r\nGiven the above, I think we may also want to discuss if the https server and java socket server should be on the same JVM or separate? Eventually we'll need to design how an https server would communicate to the socket server. \r\n\r\nPreviously, I was thinking it would be via DB state, with 'https server' writing to DB and 'socket server' (lobby) reading it. We could also have the communication be in-memory, though in general I think lobby would do well to read more state from DB rather than keep in-memory. Perhaps that could be okay still with https and socket server."},"comment":{"url":"https://api.github.com/repos/triplea-game/triplea/issues/comments/469846093","html_url":"https://github.com/triplea-game/triplea/issues/4724#issuecomment-469846093","issue_url":"https://api.github.com/repos/triplea-game/triplea/issues/4724","id":469846093,"node_id":"MDEyOklzc3VlQ29tbWVudDQ2OTg0NjA5Mw==","user":{"login":"RoiEXLab","id":8350879,"node_id":"MDQ6VXNlcjgzNTA4Nzk=","avatar_url":"https://avatars0.githubusercontent.com/u/8350879?v=4","gravatar_id":"","url":"https://api.github.com/users/RoiEXLab","html_url":"https://github.com/RoiEXLab","followers_url":"https://api.github.com/users/RoiEXLab/followers","following_url":"https://api.github.com/users/RoiEXLab/following{/other_user}","gists_url":"https://api.github.com/users/RoiEXLab/gists{/gist_id}","starred_url":"https://api.github.com/users/RoiEXLab/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/RoiEXLab/subscriptions","organizations_url":"https://api.github.com/users/RoiEXLab/orgs","repos_url":"https://api.github.com/users/RoiEXLab/repos","events_url":"https://api.github.com/users/RoiEXLab/events{/privacy}","received_events_url":"https://api.github.com/users/RoiEXLab/received_events","type":"User","site_admin":false},"created_at":"2019-03-05T20:32:23Z","updated_at":"2019-03-05T20:32:23Z","author_association":"MEMBER","body":"@DanVanAtta \r\n> I'm suggesting we should focus on this, leave websocket out of it for now, and where we need https, use it with the https server that is coming online. I thought our previous plan was to have both an https server and websocket (and that the HTTPs server was not temporary, but made more sense for classical server/client communication).\r\n\r\nI think there's a disconnect at some point. I wasn't suggesting that plain HTTPS should only exist temporary. It clearly makes more sense than websocket on certain occasions and vice-versa.\r\nThe reason why I explicitly stated websocket in this use case is that I'm concerned that using HTTPS stops working well enough when we get to multiplayer games, where a lot of data needs to be sent to different hosts in multiple directions, where as when only the host has a websocket server (I'm guessing that's what's currently happening with TCP anyways), only a single bi-directional connection needs to be created for every player. Note that my focus is on ***bi-directional*** here. I'm convinced that hosting a web server just to receive events (which would also cause problems with port forwarding etc.) isn't the best thing to do.\r\nI'll try to explain my idea differently: I was thinking about 1:1 replacing TCP with websocket for now to get the technology and necessary infrastructure in place (routes, certificates, port-, nginx configuration etc.).\r\nThis way we'd only need a single websocket for all connections to just work out of the box.\r\nWe could also base64 encode the bytes in order to send them as message.\r\nAfter that everything that can be implemented step-by-step in a better way using HTTPS and a REST call for example is just modifying existing classes instead of having to establish a completely new class model.\r\n\r\nDoes this male sense?"}}
other: {"actor":{"display_login":"RoiEXLab"}}