cayenne-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Aristedes Maniatis <...@maniatis.org>
Subject Re: work on ROP
Date Thu, 21 Jan 2016 10:15:41 GMT
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

Mime
View raw message