mojo.syncbase: Mojo+Syncbase: make list* methods return strings, fix exec string quoting bug
MultiPart: 2/2
Change-Id: I7bf04d424636fd93034f623d43695c0682af536b
diff --git a/lib/src/app.dart b/lib/src/app.dart
index b3f3876..d1052f0 100644
--- a/lib/src/app.dart
+++ b/lib/src/app.dart
@@ -4,7 +4,6 @@
part of syncbase_client;
-// TODO(sadovsky): Add listDatabases method.
class SyncbaseApp extends NamedResource {
// NOTE(sadovsky): For the Mojo Syncbase service, we only store names from app
// down - i.e. there is no service name.
@@ -41,13 +40,10 @@
return v.perms;
}
- // TODO(aghassemi): Maybe add an abstract class for SyncbaseDatabase so we can
- // return either NoSqlDatabase or SqlDatabase (when both exist).
- Future<List<SyncbaseNoSqlDatabase>> listDatabases() async {
+ Future<List<String>> listDatabases() async {
var v = await _ctx.syncbase.appListDatabases(fullName);
if (isError(v.err)) throw v.err;
-
- return v.databases.map((dbName) => this.noSqlDatabase(dbName)).toList();
+ return v.databases;
}
Future setPermissions(mojom.Perms perms, String version) async {
diff --git a/lib/src/client_context.dart b/lib/src/client_context.dart
index 571a9d3..2c61fdb 100644
--- a/lib/src/client_context.dart
+++ b/lib/src/client_context.dart
@@ -4,17 +4,16 @@
part of syncbase_client;
-// ClientContext holds references to objects that other layers need from the
-// SyncbaseClient, e.g. the Mojo proxy and stub manager.
+// ClientContext holds references to objects that other layers need from
+// SyncbaseClient.
class ClientContext {
// Handle to the Syncbase Mojo proxy.
mojom.SyncbaseProxy proxy;
- // Just a convenience getter for the Mojo proxy's Syncbase pointer.
+ // Convenience getter for the Mojo proxy's Syncbase pointer.
mojom.Syncbase get syncbase => proxy.ptr;
- // Handle to the unclosed stubs manager. Used by layers that need to track
- // Mojo stubs that need to be closed when the Syncbase client is closed.
+ // Used to track Mojo stubs to close when the Syncbase client is closed.
UnclosedStubsManager unclosedStubsManager;
ClientContext._internal(this.proxy, this.unclosedStubsManager);
diff --git a/lib/src/nosql/database.dart b/lib/src/nosql/database.dart
index b4e4581..82dc010 100644
--- a/lib/src/nosql/database.dart
+++ b/lib/src/nosql/database.dart
@@ -69,7 +69,8 @@
_ctx.unclosedStubsManager.register(stub);
- // TODO(aghassemi): Implement naming utilities such as Join in Dart and use them instead.
+ // TODO(aghassemi): Implement naming utilities such as Join in Dart and use
+ // them instead.
var pattern = naming.join(tableName, prefix) + '*';
var req = new mojom.GlobRequest();
req.pattern = pattern;
@@ -93,8 +94,7 @@
Future<List<SyncbaseTable>> listTables() async {
var v = await _ctx.syncbase.dbListTables(fullName);
if (isError(v.err)) throw v.err;
-
- return v.tables.map((tableName) => this.table(tableName)).toList();
+ return v.tables;
}
Future<String> beginBatch(mojom.BatchOptions opts) async {
@@ -142,7 +142,7 @@
// NOTE(aghassemi): We need to make newAck optional to match mojo's
// define class, but newAck is always provided by mojo when called.
if (newAck == null) {
- throw new ArgumentError('newAck can not be null');
+ throw new ArgumentError('newAck must not be null');
}
_sc.add(result);
@@ -176,7 +176,7 @@
// NOTE(aghassemi): We need to make newAck optional to match mojo's
// define class, but newAck is always provided by mojo when called.
if (newAck == null) {
- throw new ArgumentError('newAck can not be null');
+ throw new ArgumentError('newAck must not be null');
}
// Testing instrumentation for testing flow control.
if (testing.isTesting) {
diff --git a/lib/src/nosql/table.dart b/lib/src/nosql/table.dart
index 95fe265..2a4cdf5 100644
--- a/lib/src/nosql/table.dart
+++ b/lib/src/nosql/table.dart
@@ -115,7 +115,7 @@
// NOTE(aghassemi): We need to make newAck optional to match mojo's
// define class, but newAck is always provided by mojo when called.
if (newAck == null) {
- throw new ArgumentError('newAck can not be null');
+ throw new ArgumentError('newAck must not be null');
}
_sc.add(keyValue);
diff --git a/lib/src/stream_flow_control.dart b/lib/src/stream_flow_control.dart
index 342c2fb..761b4f0 100644
--- a/lib/src/stream_flow_control.dart
+++ b/lib/src/stream_flow_control.dart
@@ -5,19 +5,20 @@
part of syncbase_client;
// StreamFlowControl is a mixin that exposes methods initFlowControl() and
-// onNextUnlock() to allow mixers add flow control to their stream controllers.
+// onNextUnlock(), enabling mixers to add flow control to their stream
+// controllers.
class StreamFlowControl {
// We are in "locked" state if and only if _mutex is not null.
- // We are locked by default until we gain the first subscriber.
+ // We start out locked, until we gain our first subscriber.
Completer _mutex = new Completer();
- // Setup flow control by adding listeners to the stream controller.
- // Mixin classes can't have constructors, so this is just a method that should
- // be called from mixer's constructor.
+ // Initializes flow control by adding listeners to the stream controller.
+ // The mixer's constructor should call this method. (Mixins can't have
+ // constructors.)
initFlowControl(StreamController sc) {
- // Unlock when gaining the first subscriber.
+ // Unlock when gaining our first subscriber.
sc.onListen = _unlock;
- // Lock when losing the last subscriber.
+ // Lock when losing our last subscriber.
sc.onCancel = _lock;
// Lock when paused.
sc.onPause = _lock;
@@ -25,31 +26,30 @@
sc.onResume = _unlock;
}
- // Returns a future that either completes immediately if we are not locked or
- // if we are locked, it gets completed as soon as we get unlocked.
- // Mixers can use this method to decide when to ack, telling the server to
- // continue sending events.
+ // Returns a future that completes immediately if we are unlocked, or as soon
+ // as we become unlocked otherwise.
+ // Mixers can use this method to decide when to ack, i.e. when to tell the
+ // server to continue sending events.
Future onNextUnlock() {
if (_mutex == null) {
- // We are not locked, return a completed future.
+ // We are not locked; return a completed future.
return new Future.value();
}
return _mutex.future;
}
// Locks the stream controller.
- // When locked, server does not sent us change events anymore until we unlock.
- // This happens because we don't send back an ack to the server when locked.
+ // When locked, the server cannot send us any more change events.
+ // This happens because we don't ack the server's change events when locked.
_lock() {
if (_mutex == null) {
_mutex = new Completer();
}
}
- // Unlcoks the stream controller.
- // When unlocked, server can send up more change events.
- // This happens because we send back an ack to the server when unlocked after
- // every change we receive instructing the server to send more.
+ // Unlocks the stream controller.
+ // When unlocked, the server can send us more change events.
+ // This happens because we ack the server's change events when unlocked.
_unlock() {
if (_mutex != null) {
_mutex.complete();
diff --git a/lib/src/unclosed_stubs_manager.dart b/lib/src/unclosed_stubs_manager.dart
index b154b87..e3ae604 100644
--- a/lib/src/unclosed_stubs_manager.dart
+++ b/lib/src/unclosed_stubs_manager.dart
@@ -4,8 +4,8 @@
part of syncbase_client;
-// UnclosedStubsManager allows different layers to keep track of the Mojo stubs
-// that need to be closed when the Syncbase client is closed.
+// UnclosedStubsManager is used to track Mojo stubs to close when the Syncbase
+// client is closed.
class UnclosedStubsManager {
List _stubs = [];
@@ -19,7 +19,7 @@
bool exists = _stubs.remove(stub);
if (!exists) {
throw new ArgumentError.value(stub, 'stub',
- 'Does not exist. Please ensure it is registered before calling close.');
+ 'Stub has not been registered, or has already been closed.');
}
stub.close();
}
diff --git a/lib/syncbase_client.dart b/lib/syncbase_client.dart
index 5dd4ebb..7875d82 100644
--- a/lib/syncbase_client.dart
+++ b/lib/syncbase_client.dart
@@ -44,7 +44,6 @@
return err != null && err.id != '';
}
-// TODO(sadovsky): Add listApps method.
class SyncbaseClient {
final ClientContext _ctx;
@@ -78,8 +77,7 @@
Future<List<SyncbaseApp>> listApps() async {
var v = await _ctx.syncbase.serviceListApps();
if (isError(v.err)) throw v.err;
-
- return v.apps.map((appName) => this.app(appName)).toList();
+ return v.apps;
}
Future setPermissions(mojom.Perms perms, String version) async {
diff --git a/test/integration/syncbase_app_test.dart b/test/integration/syncbase_app_test.dart
index 6777ec5..b6fc73e 100644
--- a/test/integration/syncbase_app_test.dart
+++ b/test/integration/syncbase_app_test.dart
@@ -35,12 +35,10 @@
var apps = await c.listApps();
- // NOTE(aghassemi): Since the Syncbase instance is shared between all tests,
- // we will get a lot more than just 1 app, so we simply verify that our
- // appName is in the returned list.
- expect(apps.length, greaterThan(0));
- var ourApp = apps.firstWhere((e) => e.name == appName);
- expect(ourApp.name, equals(appName));
+ // Note: The Syncbase instance is shared among all tests, so listApps() will
+ // return lots of apps; here, we simply verify that our appName is in the
+ // returned list.
+ expect(apps.contains(appName), equals(true));
});
test('listing databases', () async {
@@ -48,18 +46,18 @@
var app = c.app(appName);
await app.create(utils.emptyPerms());
- var dbNames = [utils.uniqueName('db1'), utils.uniqueName('db2')];
- dbNames.sort();
+ var want = [utils.uniqueName('db1'), utils.uniqueName('db2')];
+ want.sort();
- for (var dbName in dbNames) {
+ for (var dbName in want) {
await app.noSqlDatabase(dbName).create(utils.emptyPerms());
}
- var dbs = await app.listDatabases();
- dbs.sort((d1, d2) => d1.name.compareTo(d2.name));
- expect(dbs.length, equals(dbNames.length));
- for (var i = 0; i < dbNames.length; i++) {
- expect(dbs[i].name, equals(dbNames[i]));
+ var got = await app.listDatabases();
+ got.sort((d1, d2) => d1.compareTo(d2));
+ expect(got.length, equals(want.length));
+ for (var i = 0; i < got.length; i++) {
+ expect(got[i], equals(want[i]));
}
});
diff --git a/test/integration/syncbase_database_test.dart b/test/integration/syncbase_database_test.dart
index 850766f..ba963b9 100644
--- a/test/integration/syncbase_database_test.dart
+++ b/test/integration/syncbase_database_test.dart
@@ -15,6 +15,10 @@
import './utils.dart' as utils;
+Iterable<String> decodeStrs(List<List<int>> bufList) {
+ return bufList.map((x) => UTF8.decode(x));
+}
+
runDatabaseTests(SyncbaseClient c) {
test('getting a handle to a database', () {
var app = c.app(utils.uniqueName('app'));
@@ -43,18 +47,18 @@
var db = app.noSqlDatabase(utils.uniqueName('db'));
await db.create(utils.emptyPerms());
- var tableNames = [utils.uniqueName('table1'), utils.uniqueName('table2')];
- tableNames.sort();
+ var want = [utils.uniqueName('table1'), utils.uniqueName('table2')];
+ want.sort();
- for (var tableName in tableNames) {
+ for (var tableName in want) {
await db.table(tableName).create(utils.emptyPerms());
}
- var tables = await db.listTables();
- tables.sort((t1, t2) => t1.name.compareTo(t2.name));
- expect(tables.length, equals(tableNames.length));
- for (var i = 0; i < tableNames.length; i++) {
- expect(tables[i].name, equals(tableNames[i]));
+ var got = await db.listTables();
+ got.sort((t1, t2) => t1.compareTo(t2));
+ expect(got.length, equals(want.length));
+ for (var i = 0; i < got.length; i++) {
+ expect(got[i], equals(want[i]));
}
});
@@ -94,13 +98,14 @@
// Ensure we see all the expected changes in order in the watch stream.
var changeNum = 0;
await for (var change in watchStream) {
- // Classes generated by mojom Dart compiler do not override == and hashCode
- // but they do override toString to print all properties. So we use toString
- // to assert equality.
+ // Classes generated by mojom Dart compiler do not override == and
+ // hashCode but they do override toString to print all properties. So we
+ // use toString to assert equality.
expect(change.toString(), equals(expectedChanges[changeNum].toString()));
changeNum++;
- // We need to break out of awaiting for watch stream values when we get everything we expected.
- // because watch stream does not end until canceled by design and we don't have canceling mechanism yet.
+ // We need to break out of awaiting for watch stream values when we get
+ // everything we expected. because watch stream does not end until
+ // canceled by design and we don't have canceling mechanism yet.
if (changeNum == expectedChanges.length) {
break;
}
@@ -180,20 +185,15 @@
var resultStream = db.exec(query);
var results = await resultStream.toList();
-
- // Expect first entry to be column headers.
- var headers = results[0].values;
- expect(headers, equals([UTF8.encode('"code"'), UTF8.encode('"cityname"')]));
-
- // Expect the two entries
- var entry1 = results[1].values;
- expect(entry1, equals([UTF8.encode('"aӲ읔"'), UTF8.encode('ᚸӲ읔+קAل')]));
-
- var entry2 = results[2].values;
- expect(entry2, equals([UTF8.encode('"yyz"'), UTF8.encode('Toronto')]));
-
- // Expect no more entries than two data rows and one column header.
+ // Expect column headers plus two rows.
expect(results.length, 3);
+
+ // Check column headers.
+ expect(decodeStrs(results[0].values), equals(['code', 'cityname']));
+
+ // Check rows.
+ expect(decodeStrs(results[1].values), equals(['aӲ읔', 'ᚸӲ읔+קAل']));
+ expect(decodeStrs(results[2].values), equals(['yyz', 'Toronto']));
});
// TODO(nlacasse): Test database.get/setPermissions.
diff --git a/tool/sync_repos.sh b/tool/sync_repos.sh
index f4244db..b504337 100755
--- a/tool/sync_repos.sh
+++ b/tool/sync_repos.sh
@@ -13,6 +13,9 @@
cd $MOJO_DIR/src
git pull
+# If needed, git checkout at a specific commit as follows.
+# git fetch
+# git checkout -f f871196158df873c7ff8498225bd011e0148174e
gclient sync
if [[ "${DESKTOP}" = "1" ]]; then
./mojo/tools/mojob.py gn
