rpc example for directory file listing

rpc example for directory file listing

BUG=
TEST=

Committed: https://code.google.com/p/dart/source/detail?r=1954

[mattsh, 2011-11-11 20:55:08]

(This is not for submission in its current form, but I wanted to show how the
RPC system and isolates can be used to expose an asynchronous API over a system
service that has a synchronous API.)

In this example, there is a class that has a synchronous API for listing files
in a directory (that calls into native code).

class DirectorySync {
  DirectorySync(String path);
  bool isDirectory();
  List<String> list();
}

Then, there is an RPC proxy for this class has corresponding methods where
return values are Future<X> instead of X.

class DirectoryProxy {
  Future<bool> isDirectory();
  Future<List<String>> list();
}

[Ben Laurie (Google), 2011-11-11 21:51:43]

http://codereview.chromium.org/8536006/diff/5001/corelib/src/proxy.dart
File corelib/src/proxy.dart (right):

http://codereview.chromium.org/8536006/diff/5001/corelib/src/proxy.dart#newco...
corelib/src/proxy.dart:14: final Future<SendPort> futurePort;
_futurePort?

http://codereview.chromium.org/8536006/diff/5001/corelib/src/proxy.dart#newco...
corelib/src/proxy.dart:25: *      isolate).
How does that work? I don't see code to send RpcProxy objects (they can't be
sent natively, right?).

[mattsh, 2011-11-11 22:16:02]

http://codereview.chromium.org/8536006/diff/5001/corelib/src/proxy.dart
File corelib/src/proxy.dart (right):

http://codereview.chromium.org/8536006/diff/5001/corelib/src/proxy.dart#newco...
corelib/src/proxy.dart:25: *      isolate).
On 2011/11/11 21:51:43, Ben Laurie (Google) wrote:
> How does that work? I don't see code to send RpcProxy objects (they can't be
> sent natively, right?).
Right.  That's not in this CL yet, but plan to add that.  The plan is to filter
the arguments for instances of RpcProxy, and for each RpcProxy object, send its
SendPort instead, and on the receiving end map these back to the corresponding
RpcReceiver objects.  Would that make sense?

[Siggi Cherem (dart-lang), 2011-11-11 23:41:51]

http://codereview.chromium.org/8536006/diff/5001/corelib/src/proxy.dart
File corelib/src/proxy.dart (right):

http://codereview.chromium.org/8536006/diff/5001/corelib/src/proxy.dart#newcode6
corelib/src/proxy.dart:6: * Base class for all RpcProxy's
generally use the dart-doc style through-out (e.g. [RpcProxy], [Future]).

http://codereview.chromium.org/8536006/diff/5001/corelib/src/proxy.dart#newco...
corelib/src/proxy.dart:25: *      isolate).
On 2011/11/11 22:16:02, mattsh wrote:
> On 2011/11/11 21:51:43, Ben Laurie (Google) wrote:
> > How does that work? I don't see code to send RpcProxy objects (they can't be
> > sent natively, right?).
> Right.  That's not in this CL yet, but plan to add that.  The plan is to
filter
> the arguments for instances of RpcProxy, and for each RpcProxy object, send
its
> SendPort instead, and on the receiving end map these back to the corresponding
> RpcReceiver objects.  Would that make sense?

is the purpose of this to establish a bi-directional communication?  in that
case it seems that we want to send an RpcReceiver rather than an RpcProxy?
(converting the port and making a proxy on the other side)

http://codereview.chromium.org/8536006/diff/5001/corelib/src/proxy.dart#newco...
corelib/src/proxy.dart:34: port.call({"command" : command, "args" :
args}).receive((value, ignoreReplyTo) {
80

http://codereview.chromium.org/8536006/diff/5001/runtime/bin/directory_exp.dart
File runtime/bin/directory_exp.dart (right):

http://codereview.chromium.org/8536006/diff/5001/runtime/bin/directory_exp.da...
runtime/bin/directory_exp.dart:101: class DirectoryIsolate extends Isolate {
what about have RpcReceiver extend from isolate instead?

[Ben Laurie (Google), 2011-11-11 23:45:37]

http://codereview.chromium.org/8536006/diff/5001/runtime/bin/directory_exp.dart
File runtime/bin/directory_exp.dart (right):

http://codereview.chromium.org/8536006/diff/5001/runtime/bin/directory_exp.da...
runtime/bin/directory_exp.dart:101: class DirectoryIsolate extends Isolate {
On 2011/11/11 23:41:51, sigmund wrote:
> what about have RpcReceiver extend from isolate instead?

That's not a good idea: you control who can send by who has the RpcProxy
corresponding to your RpcReceiver, which means you need more than one per
isolate.

[Siggi Cherem (dart-lang), 2011-11-11 23:54:08]

http://codereview.chromium.org/8536006/diff/5001/runtime/bin/directory_exp.dart
File runtime/bin/directory_exp.dart (right):

http://codereview.chromium.org/8536006/diff/5001/runtime/bin/directory_exp.da...
runtime/bin/directory_exp.dart:101: class DirectoryIsolate extends Isolate {
On 2011/11/11 23:45:38, Ben Laurie (Google) wrote:
> On 2011/11/11 23:41:51, sigmund wrote:
> > what about have RpcReceiver extend from isolate instead?
> 
> That's not a good idea: you control who can send by who has the RpcProxy
> corresponding to your RpcReceiver, which means you need more than one per
> isolate.

makes sense - I guess in this example it was clear that the proxy-user created
an isolate just for this purpose, but more generally there is an isolate running
and you want to connect to a service it provides (rpcreceiver).

[Siggi Cherem (dart-lang), 2011-11-18 01:31:17]

I found the mint maker example very easy to follow.

Ben - is this covering what you initially wanted to illustrate in mint maker? or
something is missing?

http://codereview.chromium.org/8536006/diff/10012/tests/isolate/src/MintMaker...
File tests/isolate/src/MintMakerRpc.dart (right):

http://codereview.chromium.org/8536006/diff/10012/tests/isolate/src/MintMaker...
tests/isolate/src/MintMakerRpc.dart:126: return new PurseReceiver(purse, new
ReceivePort());
interesting that the base RPcReceiver automatically sends a port to this
receiver.

[mattsh, 2011-11-18 01:41:22]

http://codereview.chromium.org/8536006/diff/10012/tests/isolate/src/MintMaker...
File tests/isolate/src/MintMakerRpc.dart (right):

http://codereview.chromium.org/8536006/diff/10012/tests/isolate/src/MintMaker...
tests/isolate/src/MintMakerRpc.dart:126: return new PurseReceiver(purse, new
ReceivePort());
On 2011/11/18 01:31:17, sigmund wrote:
> interesting that the base RPcReceiver automatically sends a port to this
> receiver.
Right.  I was thinking we want to have a one to one relationship between
ReceivePort and RpcReceiver, so RpcReceiver takes a ReceivePort as constructor
parameter.

[Ben Laurie (Google), 2011-11-29 18:59:28]

General comment: I'd like to see the fully isolated version, too.

http://codereview.chromium.org/8536006/diff/10012/corelib/src/proxy.dart
File corelib/src/proxy.dart (right):

http://codereview.chromium.org/8536006/diff/10012/corelib/src/proxy.dart#newc...
corelib/src/proxy.dart:35: port.call({"command" : command, "args" :
args}).receive((value, ignoreReplyTo) {
width

http://codereview.chromium.org/8536006/diff/10012/corelib/src/proxy.dart#newc...
corelib/src/proxy.dart:39: // give derived proxy class a chance to transate
SendPort to RpcProxy
width

http://codereview.chromium.org/8536006/diff/10012/corelib/src/proxy.dart#newc...
corelib/src/proxy.dart:64: filtered.add(proxy.futurePort.value);
How do you know this has completed?

http://codereview.chromium.org/8536006/diff/10012/runtime/bin/directory_exp.cc
File runtime/bin/directory_exp.cc (right):

http://codereview.chromium.org/8536006/diff/10012/runtime/bin/directory_exp.c...
runtime/bin/directory_exp.cc:29: printf("FilePath_List\n");
printf?

http://codereview.chromium.org/8536006/diff/10012/runtime/bin/directory_exp.c...
runtime/bin/directory_exp.cc:72: printf("FilePath_IsDirectory\n");
printf?

http://codereview.chromium.org/8536006/diff/10012/tests/isolate/src/MintMaker...
File tests/isolate/src/MintMakerRpc.dart (right):

http://codereview.chromium.org/8536006/diff/10012/tests/isolate/src/MintMaker...
tests/isolate/src/MintMakerRpc.dart:208: if
(exception.toString().contains("OverdraftException", 0)) {
width

[mattsh, 2011-11-29 21:29:46]

http://codereview.chromium.org/8536006/diff/10012/corelib/src/proxy.dart
File corelib/src/proxy.dart (right):

http://codereview.chromium.org/8536006/diff/10012/corelib/src/proxy.dart#newc...
corelib/src/proxy.dart:35: port.call({"command" : command, "args" :
args}).receive((value, ignoreReplyTo) {
On 2011/11/29 18:59:28, Ben Laurie (Google) wrote:
> width

Done.

http://codereview.chromium.org/8536006/diff/10012/corelib/src/proxy.dart#newc...
corelib/src/proxy.dart:39: // give derived proxy class a chance to transate
SendPort to RpcProxy
On 2011/11/29 18:59:28, Ben Laurie (Google) wrote:
> width

Done.

http://codereview.chromium.org/8536006/diff/10012/corelib/src/proxy.dart#newc...
corelib/src/proxy.dart:64: filtered.add(proxy.futurePort.value);
On 2011/11/29 18:59:28, Ben Laurie (Google) wrote:
> How do you know this has completed?

Because it is called from inside futurePort.then

http://codereview.chromium.org/8536006/diff/10012/runtime/bin/directory_exp.cc
File runtime/bin/directory_exp.cc (right):

http://codereview.chromium.org/8536006/diff/10012/runtime/bin/directory_exp.c...
runtime/bin/directory_exp.cc:29: printf("FilePath_List\n");
On 2011/11/29 18:59:28, Ben Laurie (Google) wrote:
> printf?

Actually I'm going to remove the directory listing code from this CL.

http://codereview.chromium.org/8536006/diff/10012/runtime/bin/directory_exp.c...
runtime/bin/directory_exp.cc:72: printf("FilePath_IsDirectory\n");
On 2011/11/29 18:59:28, Ben Laurie (Google) wrote:
> printf?

Done.

http://codereview.chromium.org/8536006/diff/10012/tests/isolate/src/MintMaker...
File tests/isolate/src/MintMakerRpc.dart (right):

http://codereview.chromium.org/8536006/diff/10012/tests/isolate/src/MintMaker...
tests/isolate/src/MintMakerRpc.dart:208: if
(exception.toString().contains("OverdraftException", 0)) {
On 2011/11/29 18:59:28, Ben Laurie (Google) wrote:
> width

Done.

[Ben Laurie (Google), 2011-11-29 21:36:45]

http://codereview.chromium.org/8536006/diff/10012/corelib/src/proxy.dart
File corelib/src/proxy.dart (right):

http://codereview.chromium.org/8536006/diff/10012/corelib/src/proxy.dart#newc...
corelib/src/proxy.dart:64: filtered.add(proxy.futurePort.value);
On 2011/11/29 21:29:46, mattsh wrote:
> On 2011/11/29 18:59:28, Ben Laurie (Google) wrote:
> > How do you know this has completed?
> 
> Because it is called from inside futurePort.then

So? This is a different proxy's value...

[mattsh, 2011-11-29 22:32:06]

http://codereview.chromium.org/8536006/diff/10012/corelib/src/proxy.dart
File corelib/src/proxy.dart (right):

http://codereview.chromium.org/8536006/diff/10012/corelib/src/proxy.dart#newc...
corelib/src/proxy.dart:64: filtered.add(proxy.futurePort.value);
On 2011/11/29 21:36:45, Ben Laurie (Google) wrote:
> On 2011/11/29 21:29:46, mattsh wrote:
> > On 2011/11/29 18:59:28, Ben Laurie (Google) wrote:
> > > How do you know this has completed?
> > 
> > Because it is called from inside futurePort.then
> 
> So? This is a different proxy's value...
> 

Good point.  Let me add a TODO about this.

[Siggi Cherem (dart-lang), 2011-11-30 01:44:38]

http://codereview.chromium.org/8536006/diff/28001/corelib/src/proxy.dart
File corelib/src/proxy.dart (right):

http://codereview.chromium.org/8536006/diff/28001/corelib/src/proxy.dart#newc...
corelib/src/proxy.dart:56: * Convert RpcProxy objects to SendPorts
style nits:
- use single line /** ... */ for comments this short :)
- end with '.'.

http://codereview.chromium.org/8536006/diff/28001/corelib/src/proxy.dart#newc...
corelib/src/proxy.dart:122: // the port that this receiver will listen on
(nit): to be consistent with all of our libraries, please use /** */ instead.

http://codereview.chromium.org/8536006/diff/28001/corelib/src/proxy.dart#newc...
corelib/src/proxy.dart:189: *       ReceivePort.
nit: is there a 2. ?, remove the bullet list?

http://codereview.chromium.org/8536006/diff/28001/corelib/src/proxy.dart#newc...
corelib/src/proxy.dart:215: // TODO - hack - need better way to serialize
exceptions.  For now
TODO(mattsh)? 
no need to say 'hack'

http://codereview.chromium.org/8536006/diff/28001/tests/isolate/src/MintMaker...
File tests/isolate/src/MintMakerRpc.dart (right):

http://codereview.chromium.org/8536006/diff/28001/tests/isolate/src/MintMaker...
tests/isolate/src/MintMakerRpc.dart:148: PurseReceiver(Purse purse, ReceivePort
receivePort) : super(purse, receivePort) {}
80 col

[Siggi Cherem (dart-lang), 2011-11-30 01:46:45]

hit publish too quickly - lgtm addressing the style comments

http://codereview.chromium.org/8536006/diff/28001/corelib/src/proxy.dart
File corelib/src/proxy.dart (right):

http://codereview.chromium.org/8536006/diff/28001/corelib/src/proxy.dart#newc...
corelib/src/proxy.dart:122: // the port that this receiver will listen on
On 2011/11/30 01:44:38, sigmund wrote:
> (nit): to be consistent with all of our libraries, please use /** */ instead.
(here and elsewhere in this code)

http://codereview.chromium.org/8536006/diff/28001/corelib/src/proxy.dart#newc...
corelib/src/proxy.dart:191: * TODO(mattsh) need to walk deeply
TODO's shouldn't be inside the /** */ docs, so that they don't show up on
generated documentation.

[mattsh, 2011-11-30 05:03:33]

http://codereview.chromium.org/8536006/diff/28001/corelib/src/proxy.dart
File corelib/src/proxy.dart (right):

http://codereview.chromium.org/8536006/diff/28001/corelib/src/proxy.dart#newc...
corelib/src/proxy.dart:56: * Convert RpcProxy objects to SendPorts
On 2011/11/30 01:44:38, sigmund wrote:
> style nits:
> - use single line /** ... */ for comments this short :)
> - end with '.'.

Done.

http://codereview.chromium.org/8536006/diff/28001/corelib/src/proxy.dart#newc...
corelib/src/proxy.dart:122: // the port that this receiver will listen on
On 2011/11/30 01:46:45, sigmund wrote:
> On 2011/11/30 01:44:38, sigmund wrote:
> > (nit): to be consistent with all of our libraries, please use /** */
instead.
> (here and elsewhere in this code)

Done.

http://codereview.chromium.org/8536006/diff/28001/corelib/src/proxy.dart#newc...
corelib/src/proxy.dart:122: // the port that this receiver will listen on
On 2011/11/30 01:44:38, sigmund wrote:
> (nit): to be consistent with all of our libraries, please use /** */ instead.

Done.

http://codereview.chromium.org/8536006/diff/28001/corelib/src/proxy.dart#newc...
corelib/src/proxy.dart:189: *       ReceivePort.
On 2011/11/30 01:44:38, sigmund wrote:
> nit: is there a 2. ?, remove the bullet list?

Done.

http://codereview.chromium.org/8536006/diff/28001/corelib/src/proxy.dart#newc...
corelib/src/proxy.dart:191: * TODO(mattsh) need to walk deeply
On 2011/11/30 01:46:45, sigmund wrote:
> TODO's shouldn't be inside the /** */ docs, so that they don't show up on
> generated documentation.

Done.

http://codereview.chromium.org/8536006/diff/28001/corelib/src/proxy.dart#newc...
corelib/src/proxy.dart:215: // TODO - hack - need better way to serialize
exceptions.  For now
On 2011/11/30 01:44:38, sigmund wrote:
> TODO(mattsh)? 
> no need to say 'hack'

Done.