On 21/01/2016 3:35am, Dzmitry Kazimirchyk wrote: > At this point the implementation is quite crude and lacks proper unit tests, > but it shows the general direction we could follow. Comments and suggestions > are welcome.
I think it looks really clear. I'm going to try and assemble a bunch of questions, some small and some big, to help my understanding. And some is just about code which was there from before. BaseConnection.sendMessage(message) does pretty much nothing but call beforeSendMessage(message), then doSendMessage(message) with a bunch of logging. It all seems a bit pointless. ---- ROPConnector could do with some docs to explain the difference between a sharedSession and a Session. ---- So DefaultClientConnectionProvider is what creates a ProxyRemoteService and passes to it the serializationService and the ropConnector. Why is the serializationService injected here but the ropConnector hardcoded to HttpROPConnector? Doesn't that effectively make this a HttpClientConnectionProvider rather than a DefaultClientConnectionProvider? Should DefaultClientConnectionProvider.createHttpRopConnector() be removed? ---- For that matter, do we need quite so many layers? Could the functionality of ProxyRemoteService just be brought into DefaultClientConnectionProvider? Maybe I'm just not quite understanding the need for both of these things separately. ------ HttpROPConnector might do with some way to set the cookie name. You made SESSION_COOKIE_NAME final for some reason. ----- Instead of reinventing HttpROPConnector.base64(), can we use this: https://docs.oracle.com/javase/8/docs/api/java/util/Base64.html I'll keep reading, but this is already a major step forward. Nicely done. Ari -- --------------------------> Aristedes Maniatis GPG fingerprint CBFB 84B4 738D 4E87 5E5C 5EFA EF6A 7D2E 3E49 102A
