From c09d5e1578adb43212f1b1bb6a774961f7d2009e Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Fri, 14 Feb 2014 18:44:56 -0800 Subject: [PATCH 01/38] Remove stray %s in update message. --- tools/updater.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/updater.js b/tools/updater.js index 21000dd45f..baf79fdd22 100644 --- a/tools/updater.js +++ b/tools/updater.js @@ -105,6 +105,6 @@ var check = function (showBanner) { ! release.forced) { runLog.log( "=> Meteor " + localLatestRelease + - " %s is available. Update this project with 'meteor update'."); + " is available. Update this project with 'meteor update'."); } }; From 3eb4a1339d72c0f74d5a441e51da9b8395ec86ed Mon Sep 17 00:00:00 2001 From: David Glasser Date: Fri, 14 Feb 2014 17:38:44 -0800 Subject: [PATCH 02/38] Update back-compat comment --- packages/oauth/oauth_client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/oauth/oauth_client.js b/packages/oauth/oauth_client.js index c038332de6..dad37006a3 100644 --- a/packages/oauth/oauth_client.js +++ b/packages/oauth/oauth_client.js @@ -62,7 +62,7 @@ var openCenteredPopup = function(url, width, height) { return newwindow; }; -// XXX COMPAT WITH 0.6.6.3 +// XXX COMPAT WITH 0.7.0.1 // Private interface but probably used by many oauth clients in atmosphere. Oauth.initiateLogin = function (credentialToken, url, callback, dimensions) { Oauth.showPopup( From 764b41adf4a667894b26459cedfd6cc01a084605 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 17 Feb 2014 15:17:07 -0800 Subject: [PATCH 03/38] Clone DDP data as it goes into the merge box Fixes #1750. --- History.md | 2 ++ packages/livedata/livedata_server.js | 4 ++++ packages/livedata/livedata_tests.js | 25 +++++++++++++++++++++++++ 3 files changed, 31 insertions(+) diff --git a/History.md b/History.md index e9f4358d6e..53167f13c0 100644 --- a/History.md +++ b/History.md @@ -152,6 +152,8 @@ * User-supplied connect handlers now see the URL's full path, even if ROOT_URL contains a non-empty path. +* Don't cache direct references to the fields arguments to the subscription + `added` and `changed` methods. #1750 ## v0.7.0.1 diff --git a/packages/livedata/livedata_server.js b/packages/livedata/livedata_server.js index 77e7a1b0b4..95ddab9e6f 100644 --- a/packages/livedata/livedata_server.js +++ b/packages/livedata/livedata_server.js @@ -68,6 +68,10 @@ _.extend(SessionDocumentView.prototype, { // Publish API ignores _id if present in fields if (key === "_id") return; + + // Don't share state with the data passed in by the user. + value = EJSON.clone(value); + if (!_.has(self.dataByKey, key)) { self.dataByKey[key] = [{subscriptionHandle: subscriptionHandle, value: value}]; diff --git a/packages/livedata/livedata_tests.js b/packages/livedata/livedata_tests.js index 1fe7ce9702..65736d76f8 100644 --- a/packages/livedata/livedata_tests.js +++ b/packages/livedata/livedata_tests.js @@ -691,6 +691,31 @@ if (Meteor.isServer) { ]); })(); +if (Meteor.isServer) { + Meteor.publish("publisherCloning", function () { + var self = this; + var fields = {x: {y: 42}}; + self.added("publisherCloning", "a", fields); + fields.x.y = 43; + self.changed("publisherCloning", "a", fields); + self.ready(); + }); +} else { + var PublisherCloningCollection = new Meteor.Collection("publisherCloning"); + testAsyncMulti("livedata - publish callbacks clone", [ + function (test, expect) { + Meteor.subscribe("publisherCloning", {normal: 1}, { + onReady: expect(function () { + test.equal(PublisherCloningCollection.findOne(), { + _id: "a", + x: {y: 43}}); + }), + onError: failure() + }); + } + ]); +} + // XXX some things to test in greater detail: // staying in simulation mode From d8617769482feec87aa22cae3c2dd6cb5cb44d1c Mon Sep 17 00:00:00 2001 From: Robert Dickert Date: Mon, 17 Feb 2014 15:35:50 -0800 Subject: [PATCH 04/38] Put `/node_modules` in package root on build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1761. Package code pulled from a git repo does not contain the directory `/node_modules`, as these modules are downloaded during the build process. However, this can confuse NPM prior to build completion. If the package directory doesn’t have a '/node_modules' subdirectory, NPM will assume we are not at the package root and will search the directory's parents for one. It will then set the root to any ancestor that does have a '/node_modules' subdir. This can cause the build to fail for downloaded user packages and also Meteor itself if run from checkout. See also #927. --- History.md | 3 +++ tools/meteor-npm.js | 13 +++++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/History.md b/History.md index 53167f13c0..9c4cb26bd3 100644 --- a/History.md +++ b/History.md @@ -111,6 +111,9 @@ * Do a better job of handling shrinkwrap files when a npm module depends on something that isn't a semver. #1684 +* Fix failures updating npm dependencies when a node_modules directory exists + above the project directory. #1761 + * In email package, print a message in dev mode when email is not sent. #1196 * Meteor accounts logins (or anything else using the `localstorage` package) no diff --git a/tools/meteor-npm.js b/tools/meteor-npm.js index cfd0b1062f..a137888836 100644 --- a/tools/meteor-npm.js +++ b/tools/meteor-npm.js @@ -195,7 +195,8 @@ var updateExistingNpmDirectory = function (packageName, newPackageNpmDir, // We need to rebuild all node modules when the Node version // changes, in case there are some binary ones. Technically this is // racey, but it shouldn't fail very often. - if (fs.existsSync(path.join(packageNpmDir, 'node_modules'))) { + var nodeModulesDir = path.join(packageNpmDir, 'node_modules'); + if (fs.existsSync(nodeModulesDir)) { var oldNodeVersion; try { oldNodeVersion = fs.readFileSync( @@ -209,9 +210,17 @@ var updateExistingNpmDirectory = function (packageName, newPackageNpmDir, } if (oldNodeVersion !== process.version) - files.rm_recursive(path.join(packageNpmDir, 'node_modules')); + files.rm_recursive(nodeModulesDir); } + // Make sure node_modules is present (fix for #1761). Prevents npm install + // from installing to an existing node_modules dir higher up in the + // filesystem. node_modules may be absent due to a change in Node version or + // when `meteor add`ing a cloned package for the first time (node_modules is + // excluded by .gitignore) + if (! fs.existsSync(nodeModulesDir)) + fs.mkdirSync(nodeModulesDir); + var installedDependencies = getInstalledDependencies(packageNpmDir); // If we already have the right things installed, life is good. From 6b122ab51b2a26d44a6409615e99a5fcf2fb4e3f Mon Sep 17 00:00:00 2001 From: Tim Haines Date: Mon, 17 Feb 2014 15:48:29 -0800 Subject: [PATCH 05/38] Allow _sockjsOptions to be provided on DDP.connect This is an unsupported API that may later be removed. Fixes #1762. --- packages/livedata/livedata_connection.js | 4 +++- packages/livedata/stream_client_common.js | 4 ++++ packages/livedata/stream_client_sockjs.js | 9 +++++---- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/livedata/livedata_connection.js b/packages/livedata/livedata_connection.js index b6756ecae7..30ff1ef377 100644 --- a/packages/livedata/livedata_connection.js +++ b/packages/livedata/livedata_connection.js @@ -10,6 +10,7 @@ if (Meteor.isServer) { // reloadWithOutstanding: is it OK to reload if there are outstanding methods? // headers: extra headers to send on the websockets connection, for // server-to-server DDP only +// _sockjsOptions: Specifies options to pass through to the sockjs client // onDDPNegotiationVersionFailure: callback when version negotiation fails. // // XXX There should be a way to destroy a DDP connection, causing all @@ -47,7 +48,8 @@ var Connection = function (url, options) { } else { self._stream = new LivedataTest.ClientStream(url, { retry: options.retry, - headers: options.headers + headers: options.headers, + _sockjsOptions: options._sockjsOptions }); } diff --git a/packages/livedata/stream_client_common.js b/packages/livedata/stream_client_common.js index e4fde2f1ec..6f551ca424 100644 --- a/packages/livedata/stream_client_common.js +++ b/packages/livedata/stream_client_common.js @@ -137,6 +137,10 @@ _.extend(LivedataTest.ClientStream.prototype, { self._changeUrl(options.url); } + if (options._sockjsOptions) { + self.options._sockjsOptions = options._sockjsOptions; + } + if (self.currentStatus.connected) { if (options._force || options.url) { // force reconnect. diff --git a/packages/livedata/stream_client_sockjs.js b/packages/livedata/stream_client_sockjs.js index d99889f339..3f42a76371 100644 --- a/packages/livedata/stream_client_sockjs.js +++ b/packages/livedata/stream_client_sockjs.js @@ -147,13 +147,14 @@ _.extend(LivedataTest.ClientStream.prototype, { var self = this; self._cleanup(); // cleanup the old socket, if there was one. + var options = _.extend({ + protocols_whitelist:self._sockjsProtocolsWhitelist() + }, self.options._sockjsOptions); + // Convert raw URL to SockJS URL each time we open a connection, so that we // can connect to random hostnames and get around browser per-host // connection limits. - self.socket = new SockJS( - toSockjsUrl(self.rawUrl), undefined, { - debug: false, protocols_whitelist: self._sockjsProtocolsWhitelist() - }); + self.socket = new SockJS(toSockjsUrl(self.rawUrl), undefined, options); self.socket.onopen = function (data) { self._connected(); }; From fb20ee822a6104edacac4916f42ec448eb61d3f0 Mon Sep 17 00:00:00 2001 From: Slava Kim Date: Tue, 4 Feb 2014 13:31:55 -0800 Subject: [PATCH 06/38] Revert "Do less deep copies" This reverts commit cb2f2adb1bd1a88a9fd651fe797592e2cd725941. --- packages/mongo-livedata/oplog_observe_driver.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/mongo-livedata/oplog_observe_driver.js b/packages/mongo-livedata/oplog_observe_driver.js index face62ccbb..d3d9796eda 100644 --- a/packages/mongo-livedata/oplog_observe_driver.js +++ b/packages/mongo-livedata/oplog_observe_driver.js @@ -144,7 +144,7 @@ _.extend(OplogObserveDriver.prototype, { + EJSON.stringify(self._cursorDescription)); } - var inCollection = !!self._collection.find(id).count(); + var inCollection = !!self._collection.findOne(id); if (matchesNow && !inCollection) { // It matches the selector and it isn't in our collection, so add it. @@ -250,7 +250,7 @@ _.extend(OplogObserveDriver.prototype, { if (op.op === 'd') { self._remove(id); } else if (op.op === 'i') { - if (self._collection.find(id).count()) + if (self._collection.findOne(id)) throw new Error("insert found for already-existing ID"); // XXX what if selector yields? for now it can't but later it could have @@ -279,9 +279,7 @@ _.extend(OplogObserveDriver.prototype, { // this directly. // XXX just send the modifier to _collection.update? but then // we don't necessarily get to GC - - // We can avoid another deep clone here since the findOne above would - // return a copy anyways + newDoc = EJSON.clone(newDoc); LocalCollection._modify(newDoc, op.o); self._handleDoc(id, newDoc); } else if (!canDirectlyModifyDoc || From 4541f72279838dd7178898b6efbc29250e413838 Mon Sep 17 00:00:00 2001 From: Slava Kim Date: Tue, 4 Feb 2014 13:32:03 -0800 Subject: [PATCH 07/38] Revert "Merge branch 'oplog-localcollection' into devel" This reverts commit 297b97659b0ca71dd2065242e6d455c7581f4667, reversing changes made to 204959b5092265455abc2482d17622a8aa3d7b9c. --- packages/meteor/fiber_stubs_client.js | 68 ++++++++ packages/meteor/package.js | 1 - packages/meteor/unyielding_queue.js | 72 --------- packages/minimongo/minimongo.js | 31 +--- packages/minimongo/sort.js | 18 +-- .../mongo-livedata/local_collection_driver.js | 2 +- .../mongo-livedata/oplog_observe_driver.js | 151 +++++++++--------- 7 files changed, 158 insertions(+), 185 deletions(-) delete mode 100644 packages/meteor/unyielding_queue.js diff --git a/packages/meteor/fiber_stubs_client.js b/packages/meteor/fiber_stubs_client.js index 46163a26e1..3babd0e08f 100644 --- a/packages/meteor/fiber_stubs_client.js +++ b/packages/meteor/fiber_stubs_client.js @@ -6,3 +6,71 @@ Meteor._noYieldsAllowed = function (f) { return f(); }; + +// An even simpler queue of tasks than the fiber-enabled one. This one just +// runs all the tasks when you call runTask or flush, synchronously. +// +Meteor._SynchronousQueue = function () { + var self = this; + self._tasks = []; + self._running = false; +}; + +_.extend(Meteor._SynchronousQueue.prototype, { + runTask: function (task) { + var self = this; + if (!self.safeToRunTask()) + throw new Error("Could not synchronously run a task from a running task"); + self._tasks.push(task); + var tasks = self._tasks; + self._tasks = []; + self._running = true; + try { + while (!_.isEmpty(tasks)) { + var t = tasks.shift(); + try { + t(); + } catch (e) { + if (_.isEmpty(tasks)) { + // this was the last task, that is, the one we're calling runTask + // for. + throw e; + } else { + Meteor._debug("Exception in queued task: " + e.stack); + } + } + } + } finally { + self._running = false; + } + }, + + queueTask: function (task) { + var self = this; + var wasEmpty = _.isEmpty(self._tasks); + self._tasks.push(task); + // Intentionally not using Meteor.setTimeout, because it doesn't like runing + // in stubs for now. + if (wasEmpty) + setTimeout(_.bind(self.flush, self), 0); + }, + + flush: function () { + var self = this; + self.runTask(function () {}); + }, + + drain: function () { + var self = this; + if (!self.safeToRunTask()) + return; + while (!_.isEmpty(self._tasks)) { + self.flush(); + } + }, + + safeToRunTask: function () { + var self = this; + return !self._running; + } +}); diff --git a/packages/meteor/package.js b/packages/meteor/package.js index 9e2db6e68e..439d80e6f2 100644 --- a/packages/meteor/package.js +++ b/packages/meteor/package.js @@ -23,7 +23,6 @@ Package.on_use(function (api) { api.add_files('errors.js', ['client', 'server']); api.add_files('fiber_helpers.js', 'server'); api.add_files('fiber_stubs_client.js', 'client'); - api.add_files('unyielding_queue.js'); api.add_files('startup_client.js', ['client']); api.add_files('startup_server.js', ['server']); api.add_files('debug.js', ['client', 'server']); diff --git a/packages/meteor/unyielding_queue.js b/packages/meteor/unyielding_queue.js deleted file mode 100644 index 95c62b23a4..0000000000 --- a/packages/meteor/unyielding_queue.js +++ /dev/null @@ -1,72 +0,0 @@ -// A simpler version of Meteor._SynchronousQueue with the same external -// interface. It runs on both client and server, unlike _SynchronousQueue which -// only runs on the server. When used on the server, tasks may not yield. This -// one just runs all the tasks when you call runTask or flush, synchronously. -// It itself also does not yield. -// -Meteor._UnyieldingQueue = function () { - var self = this; - self._tasks = []; - self._running = false; -}; - -_.extend(Meteor._UnyieldingQueue.prototype, { - runTask: function (task) { - var self = this; - if (!self.safeToRunTask()) - throw new Error("Could not synchronously run a task from a running task"); - self._tasks.push(task); - var tasks = self._tasks; - self._tasks = []; - self._running = true; - try { - while (!_.isEmpty(tasks)) { - var t = tasks.shift(); - try { - Meteor._noYieldsAllowed(function () { - t(); - }); - } catch (e) { - if (_.isEmpty(tasks)) { - // this was the last task, that is, the one we're calling runTask - // for. - throw e; - } else { - Meteor._debug("Exception in queued task: " + e.stack); - } - } - } - } finally { - self._running = false; - } - }, - - queueTask: function (task) { - var self = this; - var wasEmpty = _.isEmpty(self._tasks); - self._tasks.push(task); - // Intentionally not using Meteor.setTimeout, because it doesn't like runing - // in stubs for now. - if (wasEmpty) - setTimeout(_.bind(self.flush, self), 0); - }, - - flush: function () { - var self = this; - self.runTask(function () {}); - }, - - drain: function () { - var self = this; - if (!self.safeToRunTask()) - return; - while (!_.isEmpty(self._tasks)) { - self.flush(); - } - }, - - safeToRunTask: function () { - var self = this; - return !self._running; - } -}); diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js index aa99c091da..de7134fedb 100644 --- a/packages/minimongo/minimongo.js +++ b/packages/minimongo/minimongo.js @@ -7,34 +7,13 @@ // ObserveHandle: the return value of a live query. -LocalCollection = function (options) { +LocalCollection = function (name) { var self = this; - options = options || {}; - - self.name = options.name; + self.name = name; // _id -> document (also containing id) self._docs = new LocalCollection._IdMap; - // When writing to this collection, we batch all observeChanges callbacks - // until the end of the write, and run them at this point. On the server, we - // use a single SynchronousQueue to do so, so that we never deliver callbacks - // out of order even if other writes occur during a yield. On the client, or - // on the server if we promise that our callbacks will never yield via an - // undocumented option, we use the simpler UnyieldingQueue. - // - // (What is the _observeCallbacksWillNeverYield option for? In some cases, it - // can be nice (on the server) to be able to write to a LocalCollection - // without yielding (eg, in a _noYieldsAllowed block). It's necessary to - // provide non-yielding allow callbacks in that case, but just doing that - // wouldn't be good enough if we always used SynchronousQueue on the server, - // since it tends to yield in order to run even non-yielding callbacks.) - var queueClass; - if (Meteor._SynchronousQueue && !options._observeCallbacksWillNeverYield) { - queueClass = Meteor._SynchronousQueue; - } else { - queueClass = Meteor._UnyieldingQueue; - } - self._observeQueue = new queueClass(); + self._observeQueue = new Meteor._SynchronousQueue(); self.next_qid = 1; // live query id generator @@ -47,8 +26,8 @@ LocalCollection = function (options) { // selector, sorter, (callbacks): functions self.queries = {}; - // null if not saving originals; an IdMap from id to original document value - // if saving originals. See comments before saveOriginals(). + // null if not saving originals; an IdMap from id to original document value if + // saving originals. See comments before saveOriginals(). self._savedOriginals = null; // True when observers are paused and we should not send callbacks. diff --git a/packages/minimongo/sort.js b/packages/minimongo/sort.js index 02218593ec..718ec1537e 100644 --- a/packages/minimongo/sort.js +++ b/packages/minimongo/sort.js @@ -48,19 +48,11 @@ Sorter = function (spec) { // min/max.) // // XXX This is actually wrong! In fact, the whole attempt to compile sort - // functions independently of selectors is wrong. In MongoDB, if you have - // documents {_id: 'x', a: [1, 10]} and {_id: 'y', a: [5, 15]}, then - // C.find({}, {sort: {a: 1}}) puts x before y (1 comes before 5). But - // C.find({a: {$gt: 3}}, {sort: {a: 1}}) puts y before x (1 does not match - // the selector, and 5 comes before 10). - // - // The way this works is pretty subtle! For example, if the documents are - // instead {_id: 'x', a: [{x: 1}, {x: 10}]}) and - // {_id: 'y', a: [{x: 5}, {x: 15}]}), - // then C.find({'a.x': {$gt: 3}}, {sort: {'a.x': 1}}) and - // C.find({a: {$elemMatch: {x: {$gt: 3}}}}, {sort: {'a.x': 1}}) - // both follow this rule (y before x). ie, you do have to apply this - // through $elemMatch. + // functions independently of selectors is wrong. In MongoDB, if you have + // documents {_id: 'x', a: [1, 10]} and {_id: 'y', a: [5, 15]}, + // then C.find({}, {sort: {a: 1}}) puts x before y (1 comes before 5). + // But C.find({a: {$gt: 3}}, {sort: {a: 1}}) puts y before x (1 does not match + // the selector, and 5 comes before 10). var reduceValue = function (branchValues, findMin) { // Expand any leaf arrays that we find, and ignore those arrays themselves. branchValues = expandArraysInBranches(branchValues, true); diff --git a/packages/mongo-livedata/local_collection_driver.js b/packages/mongo-livedata/local_collection_driver.js index aa325979e5..ec78544154 100644 --- a/packages/mongo-livedata/local_collection_driver.js +++ b/packages/mongo-livedata/local_collection_driver.js @@ -5,7 +5,7 @@ LocalCollectionDriver = function () { var ensureCollection = function (name, collections) { if (!(name in collections)) - collections[name] = new LocalCollection({name: name}); + collections[name] = new LocalCollection(name); return collections[name]; }; diff --git a/packages/mongo-livedata/oplog_observe_driver.js b/packages/mongo-livedata/oplog_observe_driver.js index d3d9796eda..2cfa2a7775 100644 --- a/packages/mongo-livedata/oplog_observe_driver.js +++ b/packages/mongo-livedata/oplog_observe_driver.js @@ -30,24 +30,13 @@ OplogObserveDriver = function (options) { self._registerPhaseChange(PHASE.QUERYING); - // A minimongo LocalCollection containing the docs that match the selector, - // and maybe more. It is guaranteed to contain all the fields needed for the - // selector and the projection, and may have other fields too. (In the future - // we may try to make this collection be shared between multiple - // OplogObserveDrivers, but not currently.) - self._collection = - new LocalCollection({_observeCallbacksWillNeverYield: true}); - // XXX think about what all the options are - var minimongoCursor = self._collection.find( - self._cursorDescription.selector, self._cursorDescription.options); - self._stopHandles.push(minimongoCursor.observeChanges(self._multiplexer)); - + self._published = new LocalCollection._IdMap; var selector = self._cursorDescription.selector; self._matcher = options.matcher; - + var projection = self._cursorDescription.options.fields || {}; + self._projectionFn = LocalCollection._compileProjection(projection); // Projection function, result of combining important fields for selector and // existing fields projection - var projection = self._cursorDescription.options.fields || {}; self._sharedProjection = self._matcher.combineIntoProjection(projection); self._sharedProjectionFn = LocalCollection._compileProjection( self._sharedProjection); @@ -120,51 +109,47 @@ OplogObserveDriver = function (options) { _.extend(OplogObserveDriver.prototype, { _add: function (doc) { var self = this; - doc = self._sharedProjectionFn(doc); - // XXX does _sharedProjection always preserve id? - if (!_.has(doc, '_id')) - throw Error("Can't add doc without _id"); - self._collection.insert(doc); + var id = doc._id; + var fields = _.clone(doc); + delete fields._id; + if (self._published.has(id)) + throw Error("tried to add something already published " + id); + self._published.set(id, self._sharedProjectionFn(fields)); + self._multiplexer.added(id, self._projectionFn(fields)); }, - _remove: function (id, options) { + _remove: function (id) { var self = this; - options = options || {}; - var removed = self._collection.remove({_id: id}); - if (options.mustExist && removed !== 1) + if (!self._published.has(id)) throw Error("tried to remove something unpublished " + id); + self._published.remove(id); + self._multiplexer.removed(id); }, _handleDoc: function (id, newDoc, mustMatchNow) { var self = this; - newDoc = _.clone(newDoc); // *shallow* clone + newDoc = _.clone(newDoc); - // XXX this is just about "matching selector", not about skip/limit var matchesNow = newDoc && self._matcher.documentMatches(newDoc).result; if (mustMatchNow && !matchesNow) { throw Error("expected " + EJSON.stringify(newDoc) + " to match " + EJSON.stringify(self._cursorDescription)); } - var inCollection = !!self._collection.findOne(id); + var matchedBefore = self._published.has(id); - if (matchesNow && !inCollection) { - // It matches the selector and it isn't in our collection, so add it. - // XXX once we add skip/limit, this may not always send an added, and - // we may need to do some GC + if (matchesNow && !matchedBefore) { self._add(newDoc); - } else if (inCollection && !matchesNow) { - // We remove this from the collection to achieve two goals: (a) causing - // the observeChanges to fire removed() and (b) saving memory. That said, - // it would be legitimate (if !!newDoc) to update the collection instead - // of removing, if we thought we might need this doc again soon. - self._remove(id, {mustExist: true}); + } else if (matchedBefore && !matchesNow) { + self._remove(id); } else if (matchesNow) { - // Replace the doc inside our collection, which may trigger a changed - // callback. - newDoc = self._sharedProjectionFn(newDoc); - // XXX does _sharedProjection always preserve id? - if (!_.has(newDoc, '_id')) - throw Error("Can't add newDoc without _id"); - self._collection.update(id, newDoc); + var oldDoc = self._published.get(id); + if (!oldDoc) + throw Error("thought that " + id + " was there!"); + delete newDoc._id; + self._published.set(id, self._sharedProjectionFn(newDoc)); + var changed = LocalCollection._makeChangedFields(_.clone(newDoc), oldDoc); + changed = self._projectionFn(changed); + if (!_.isEmpty(changed)) + self._multiplexer.changed(id, changed); } }, _fetchModifiedDocuments: function () { @@ -248,9 +233,10 @@ _.extend(OplogObserveDriver.prototype, { } if (op.op === 'd') { - self._remove(id); + if (self._published.has(id)) + self._remove(id); } else if (op.op === 'i') { - if (self._collection.findOne(id)) + if (self._published.has(id)) throw new Error("insert found for already-existing ID"); // XXX what if selector yields? for now it can't but later it could have @@ -272,22 +258,18 @@ _.extend(OplogObserveDriver.prototype, { if (isReplace) { self._handleDoc(id, _.extend({_id: id}, op.o)); - } else { - var newDoc = self._collection.findOne(id); - if (newDoc && canDirectlyModifyDoc) { - // Oh great, we actually know what the document is, so we can apply - // this directly. - // XXX just send the modifier to _collection.update? but then - // we don't necessarily get to GC - newDoc = EJSON.clone(newDoc); - LocalCollection._modify(newDoc, op.o); - self._handleDoc(id, newDoc); - } else if (!canDirectlyModifyDoc || - self._matcher.canBecomeTrueByModifier(op.o)) { - self._needToFetch.set(id, op.ts.toString()); - if (self._phase === PHASE.STEADY) - self._fetchModifiedDocuments(); - } + } else if (self._published.has(id) && canDirectlyModifyDoc) { + // Oh great, we actually know what the document is, so we can apply + // this directly. + var newDoc = EJSON.clone(self._published.get(id)); + newDoc._id = id; + LocalCollection._modify(newDoc, op.o); + self._handleDoc(id, self._sharedProjectionFn(newDoc)); + } else if (!canDirectlyModifyDoc || + self._matcher.canBecomeTrueByModifier(op.o)) { + self._needToFetch.set(id, op.ts.toString()); + if (self._phase === PHASE.STEADY) + self._fetchModifiedDocuments(); } } else { throw Error("XXX SURPRISING OPERATION: " + op); @@ -336,19 +318,18 @@ _.extend(OplogObserveDriver.prototype, { self._currentlyFetching = null; ++self._fetchGeneration; // ignore any in-flight fetches self._registerPhaseChange(PHASE.QUERYING); - self._collection.pauseObservers(); - // XXX this won't be quite correct for skip/limit - self._collection.remove({}); // Defer so that we don't block. Meteor.defer(function () { - // Insert all the documents currently found by the query. - self._cursorForQuery().forEach(function (doc) { - self._collection.insert(doc); + // subtle note: _published does not contain _id fields, but newResults + // does + var newResults = new LocalCollection._IdMap; + var cursor = self._cursorForQuery(); + cursor.forEach(function (doc) { + newResults.set(doc._id, doc); }); - // Allow observe callbacks (ie multiplexer invocations) to fire. - self._collection.resumeObservers(); + self._publishNewResults(newResults); self._doneQuerying(); }); @@ -418,6 +399,34 @@ _.extend(OplogObserveDriver.prototype, { }, + // Replace self._published with newResults (both are IdMaps), invoking observe + // callbacks on the multiplexer. + // + // XXX This is very similar to LocalCollection._diffQueryUnorderedChanges. We + // should really: (a) Unify IdMap and OrderedDict into Unordered/OrderedDict (b) + // Rewrite diff.js to use these classes instead of arrays and objects. + _publishNewResults: function (newResults) { + var self = this; + + // First remove anything that's gone. Be careful not to modify + // self._published while iterating over it. + var idsToRemove = []; + self._published.forEach(function (doc, id) { + if (!newResults.has(id)) + idsToRemove.push(id); + }); + _.each(idsToRemove, function (id) { + self._remove(id); + }); + + // Now do adds and changes. + newResults.forEach(function (doc, id) { + // "true" here means to throw if we think this doc doesn't match the + // selector. + self._handleDoc(id, doc, true); + }); + }, + // This stop function is invoked from the onStop of the ObserveMultiplexer, so // it shouldn't actually be possible to call it until the multiplexer is // ready. @@ -441,6 +450,7 @@ _.extend(OplogObserveDriver.prototype, { self._writesToCommitWhenWeReachSteady = null; // Proactively drop references to potentially big things. + self._published = null; self._needToFetch = null; self._currentlyFetching = null; self._oplogEntryHandle = null; @@ -454,9 +464,6 @@ _.extend(OplogObserveDriver.prototype, { var self = this; var now = new Date; - if (phase === self._phase) - return; - if (self._phase) { var timeDiff = now - self._phaseStartTime; Package.facts && Package.facts.Facts.incrementServerFact( From 10cdd65f2cc1d1067ce7bc685a8857bd0812b038 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 27 Jan 2014 21:55:36 -0800 Subject: [PATCH 08/38] More comments about our sort inconsistency (overzealously reverted) --- packages/minimongo/sort.js | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/minimongo/sort.js b/packages/minimongo/sort.js index 718ec1537e..02218593ec 100644 --- a/packages/minimongo/sort.js +++ b/packages/minimongo/sort.js @@ -48,11 +48,19 @@ Sorter = function (spec) { // min/max.) // // XXX This is actually wrong! In fact, the whole attempt to compile sort - // functions independently of selectors is wrong. In MongoDB, if you have - // documents {_id: 'x', a: [1, 10]} and {_id: 'y', a: [5, 15]}, - // then C.find({}, {sort: {a: 1}}) puts x before y (1 comes before 5). - // But C.find({a: {$gt: 3}}, {sort: {a: 1}}) puts y before x (1 does not match - // the selector, and 5 comes before 10). + // functions independently of selectors is wrong. In MongoDB, if you have + // documents {_id: 'x', a: [1, 10]} and {_id: 'y', a: [5, 15]}, then + // C.find({}, {sort: {a: 1}}) puts x before y (1 comes before 5). But + // C.find({a: {$gt: 3}}, {sort: {a: 1}}) puts y before x (1 does not match + // the selector, and 5 comes before 10). + // + // The way this works is pretty subtle! For example, if the documents are + // instead {_id: 'x', a: [{x: 1}, {x: 10}]}) and + // {_id: 'y', a: [{x: 5}, {x: 15}]}), + // then C.find({'a.x': {$gt: 3}}, {sort: {'a.x': 1}}) and + // C.find({a: {$elemMatch: {x: {$gt: 3}}}}, {sort: {'a.x': 1}}) + // both follow this rule (y before x). ie, you do have to apply this + // through $elemMatch. var reduceValue = function (branchValues, findMin) { // Expand any leaf arrays that we find, and ignore those arrays themselves. branchValues = expandArraysInBranches(branchValues, true); From 54694a6c2a6aaf6485d2a8962a638aae3532e9c3 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 17 Feb 2014 19:32:49 -0800 Subject: [PATCH 09/38] Revert "Remove run-command (and "raw" commands)" This reverts commit 47cf0f476d8a7affa06297f8c20049cd236523ec. --- tools/commands.js | 67 +++++++++++++++++++++++++++++++++++++++++++++++ tools/help.txt | 13 +++++++++ tools/main.js | 40 +++++++++++++++++++++------- 3 files changed, 110 insertions(+), 10 deletions(-) diff --git a/tools/commands.js b/tools/commands.js index 6b9a788193..c8f37e6af3 100644 --- a/tools/commands.js +++ b/tools/commands.js @@ -6,6 +6,7 @@ var files = require('./files.js'); var deploy = require('./deploy.js'); var library = require('./library.js'); var buildmessage = require('./buildmessage.js'); +var unipackage = require('./unipackage.js'); var project = require('./project.js'); var warehouse = require('./warehouse.js'); var auth = require('./auth.js'); @@ -1133,6 +1134,72 @@ main.registerCommand({ }); +/////////////////////////////////////////////////////////////////////////////// +// run-command +/////////////////////////////////////////////////////////////////////////////// + +main.registerCommand({ + name: 'run-command', + hidden: true, + raw: true +}, function (options) { + // This is marked as raw, so we have to do all of our argument + // parsing ourselves. This lets us make sure that the arguments to + // the command being run don't get accidentally intrepreted. + + var library = release.current.library; + var argv = process.argv.slice(3); + if (! argv.length || argv[0] === "--help") + throw new main.ShowUsage; + + if (! fs.existsSync(argv[0]) || + ! fs.statSync(argv[0]).isDirectory()) { + process.stderr.write(argv[0] + ": not a directory\n"); + return 1; + } + + // Build and load the package + var world, packageName; + var messages = buildmessage.capture( + { title: "building the program" }, function () { + // Make the directory visible as a package. Derive the last + // package name from the last component of the directory, and + // bail out if that creates a conflict. + var packageDir = path.resolve(argv[0]); + packageName = path.basename(packageDir) + "-tool"; + if (library.get(packageName, false)) { + buildmessage.error("'" + packageName + + "' conflicts with the name " + + "of a package in the library"); + } + library.override(packageName, packageDir); + + world = unipackage.load({ + library: library, + packages: [ packageName ], + release: release.current.name + }); + }); + if (messages.hasMessages()) { + process.stderr.write(messages.formatMessages()); + return 1; + } + + if (! ('main' in world[packageName])) { + process.stderr.write("Package does not define a main() function.\n"); + return 1; + } + + var ret = world[packageName].main(argv.slice(1)); + // let exceptions propagate and get printed by node + if (ret === undefined) + ret = 0; + if (typeof ret !== "number") + ret = 1; + ret = +ret; // cast to integer + return ret; +}); + /////////////////////////////////////////////////////////////////////////////// // login /////////////////////////////////////////////////////////////////////////////// diff --git a/tools/help.txt b/tools/help.txt index 4e9b57b8d4..207a00d8cc 100644 --- a/tools/help.txt +++ b/tools/help.txt @@ -264,6 +264,19 @@ You should never need to use this command. It is intended for use while debugging the Meteor packaging tools themselves. +>>> run-command +Build and run a command-line tool +Usage: meteor run-command [arguments..] + +Builds the provided directory as a package, then loads the package and +calls the main() function inside the package. The function will receive +any remaining arguments. The exit status will be the return value of +main() (which is called inside a fiber). + +This command is for temporary, internal use, until we have a more mature +system for building standalone command-line programs with Meteor. + + >>> login Log in to your Meteor account Usage: meteor login [--email] [--galaxy ] diff --git a/tools/main.js b/tools/main.js index f22f707da7..66031dea67 100644 --- a/tools/main.js +++ b/tools/main.js @@ -32,6 +32,7 @@ var Command = function (options) { options: {}, requiresApp: false, requiresRelease: true, + raw: false, hidden: false }, options); @@ -134,6 +135,10 @@ main.SpringboardToLatestRelease = function () {}; // just like we always do; it's just that in that one case, instead // of bailing out with an error we will run your command with // release.current === null. +// - raw: if true, option parsing is completely skipped (including +// --release and --help). To be activated the command will need to be +// literally the first argument, and it will need to do its own option +// parsing from process.argv. // - hidden: do not show in command list in help // // An error will be printed if an unrecognized option is passed on the @@ -180,6 +185,9 @@ main.registerCommand = function (options, func) { nameParts.unshift('--'); } + if (nameParts.length !== 1 && options.raw) + throw new Error("raw mode can't be used with subcommands or --commands"); + var target = commands; while (nameParts.length > 1) { var part = nameParts.shift(); @@ -296,10 +304,11 @@ var springboard = function (toolsVersion, releaseOverride) { var cmd = path.join(warehouse.getToolsDir(toolsVersion), 'bin', 'meteor'); if (releaseOverride !== undefined) - // We used to just append --release= to the arguments, and - // though that's probably safe in practice, it makes us worry about things - // like other --release options. So now we use an environment - // variable. #SpringboardEnvironmentVar + // We used to just append --release= to the + // arguments, and though that's probably safe in practice, there's + // a lot to worry about: conflicts with other --release options, + // or 'raw' commands that do their own argument parsing. So now we + // use environment variable. #SpringboardEnvironmentVar process.env['METEOR_SPRINGBOARD_RELEASE'] = releaseOverride; // Now exec; we're not coming back. @@ -337,6 +346,21 @@ Fiber(function () { process.exit(1); } + var commandName = ''; + var command = null; + var isRawCommand = false; + var showHelp = false; + + // Check to see if it is a "raw" command that handles its own + // argument parsing. + if (process.argv.length > 2 && + _.has(commands, process.argv[2]) && + commands[process.argv[2]].raw) { + command = commands[process.argv[2]]; + commandName = command.name; + isRawCommand = true; + } + // Parse the arguments. // // We must first identify which options are boolean and which take @@ -657,22 +681,18 @@ Fiber(function () { } // Check for the '--help' option. - var showHelp = false; if (_.has(rawOptions, '--help')) { showHelp = true; delete rawOptions['--help']; } - var commandName = ''; - var command = null; - // Check for a command like '--arch' or '--version'. Make sure // it stands alone. (And this is ignored if you've passed --help.) - if (! showHelp) { + if (! command) { _.each(commands['--'] || {}, function (value, key) { var fullName = "--" + key; - if (rawOptions[fullName]) { + if (rawOptions[fullName] && ! showHelp) { if (rawOptions[fullName].length > 1) { process.stderr.write("It doesn't make sense to pass " + fullName + " more than once.\n"); From fde0717d5d0e8e05a0edd97a2a35268f4225a019 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 17 Feb 2014 19:33:35 -0800 Subject: [PATCH 10/38] Update help --- tools/help.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tools/help.txt b/tools/help.txt index 207a00d8cc..068e489cba 100644 --- a/tools/help.txt +++ b/tools/help.txt @@ -273,6 +273,12 @@ calls the main() function inside the package. The function will receive any remaining arguments. The exit status will be the return value of main() (which is called inside a fiber). +The arguments will be parsed by Meteor's option parser, which means that +--release will be effective (but not passed to the command), and that it will be +an error to pass any unknown options. If you want to pass options to your tool, +place them after a '--' argument (which turns off option parsing for the rest of +the arguments). + This command is for temporary, internal use, until we have a more mature system for building standalone command-line programs with Meteor. From 5eef5fcfd3c1141b81adf33c22f80f801a41537c Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 17 Feb 2014 19:34:02 -0800 Subject: [PATCH 11/38] keep raw commands removed --- tools/main.js | 40 ++++++++++------------------------------ 1 file changed, 10 insertions(+), 30 deletions(-) diff --git a/tools/main.js b/tools/main.js index 66031dea67..f22f707da7 100644 --- a/tools/main.js +++ b/tools/main.js @@ -32,7 +32,6 @@ var Command = function (options) { options: {}, requiresApp: false, requiresRelease: true, - raw: false, hidden: false }, options); @@ -135,10 +134,6 @@ main.SpringboardToLatestRelease = function () {}; // just like we always do; it's just that in that one case, instead // of bailing out with an error we will run your command with // release.current === null. -// - raw: if true, option parsing is completely skipped (including -// --release and --help). To be activated the command will need to be -// literally the first argument, and it will need to do its own option -// parsing from process.argv. // - hidden: do not show in command list in help // // An error will be printed if an unrecognized option is passed on the @@ -185,9 +180,6 @@ main.registerCommand = function (options, func) { nameParts.unshift('--'); } - if (nameParts.length !== 1 && options.raw) - throw new Error("raw mode can't be used with subcommands or --commands"); - var target = commands; while (nameParts.length > 1) { var part = nameParts.shift(); @@ -304,11 +296,10 @@ var springboard = function (toolsVersion, releaseOverride) { var cmd = path.join(warehouse.getToolsDir(toolsVersion), 'bin', 'meteor'); if (releaseOverride !== undefined) - // We used to just append --release= to the - // arguments, and though that's probably safe in practice, there's - // a lot to worry about: conflicts with other --release options, - // or 'raw' commands that do their own argument parsing. So now we - // use environment variable. #SpringboardEnvironmentVar + // We used to just append --release= to the arguments, and + // though that's probably safe in practice, it makes us worry about things + // like other --release options. So now we use an environment + // variable. #SpringboardEnvironmentVar process.env['METEOR_SPRINGBOARD_RELEASE'] = releaseOverride; // Now exec; we're not coming back. @@ -346,21 +337,6 @@ Fiber(function () { process.exit(1); } - var commandName = ''; - var command = null; - var isRawCommand = false; - var showHelp = false; - - // Check to see if it is a "raw" command that handles its own - // argument parsing. - if (process.argv.length > 2 && - _.has(commands, process.argv[2]) && - commands[process.argv[2]].raw) { - command = commands[process.argv[2]]; - commandName = command.name; - isRawCommand = true; - } - // Parse the arguments. // // We must first identify which options are boolean and which take @@ -681,18 +657,22 @@ Fiber(function () { } // Check for the '--help' option. + var showHelp = false; if (_.has(rawOptions, '--help')) { showHelp = true; delete rawOptions['--help']; } + var commandName = ''; + var command = null; + // Check for a command like '--arch' or '--version'. Make sure // it stands alone. (And this is ignored if you've passed --help.) - if (! command) { + if (! showHelp) { _.each(commands['--'] || {}, function (value, key) { var fullName = "--" + key; - if (rawOptions[fullName] && ! showHelp) { + if (rawOptions[fullName]) { if (rawOptions[fullName].length > 1) { process.stderr.write("It doesn't make sense to pass " + fullName + " more than once.\n"); From 91832f8b1a8833cdb3f280af512e307ba2147316 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 17 Feb 2014 19:36:05 -0800 Subject: [PATCH 12/38] Update run-command to not be a raw command (raw commands don't exist any more) --- tools/commands.js | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/tools/commands.js b/tools/commands.js index c8f37e6af3..cafcfa7890 100644 --- a/tools/commands.js +++ b/tools/commands.js @@ -1141,20 +1141,14 @@ main.registerCommand({ main.registerCommand({ name: 'run-command', hidden: true, - raw: true + minArgs: 1, + maxArgs: Infinity }, function (options) { - // This is marked as raw, so we have to do all of our argument - // parsing ourselves. This lets us make sure that the arguments to - // the command being run don't get accidentally intrepreted. - var library = release.current.library; - var argv = process.argv.slice(3); - if (! argv.length || argv[0] === "--help") - throw new main.ShowUsage; - if (! fs.existsSync(argv[0]) || - ! fs.statSync(argv[0]).isDirectory()) { - process.stderr.write(argv[0] + ": not a directory\n"); + if (! fs.existsSync(options.args[0]) || + ! fs.statSync(options.args[0]).isDirectory()) { + process.stderr.write(options.args[0] + ": not a directory\n"); return 1; } @@ -1165,7 +1159,7 @@ main.registerCommand({ // Make the directory visible as a package. Derive the last // package name from the last component of the directory, and // bail out if that creates a conflict. - var packageDir = path.resolve(argv[0]); + var packageDir = path.resolve(options.args[0]); packageName = path.basename(packageDir) + "-tool"; if (library.get(packageName, false)) { buildmessage.error("'" + packageName + @@ -1190,7 +1184,7 @@ main.registerCommand({ return 1; } - var ret = world[packageName].main(argv.slice(1)); + var ret = world[packageName].main(options.args.slice(1)); // let exceptions propagate and get printed by node if (ret === undefined) ret = 0; From 9f93b95f5389e80d6a044f751634c88409f6f358 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 17 Feb 2014 19:39:38 -0800 Subject: [PATCH 13/38] Better main detection --- tools/commands.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/commands.js b/tools/commands.js index cafcfa7890..5992657887 100644 --- a/tools/commands.js +++ b/tools/commands.js @@ -1179,7 +1179,7 @@ main.registerCommand({ return 1; } - if (! ('main' in world[packageName])) { + if (typeof world[packageName].main !== "function") { process.stderr.write("Package does not define a main() function.\n"); return 1; } From 993fbc90cbdf3326219b061ab11c01e302699fe5 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 17 Feb 2014 19:46:29 -0800 Subject: [PATCH 14/38] run-command tests --- .../tests/apps/with-command/.meteor/.gitignore | 1 + tools/tests/apps/with-command/.meteor/packages | 4 ++++ tools/tests/apps/with-command/.meteor/release | 1 + .../apps/with-command/packages/command/foo.js | 5 +++++ .../with-command/packages/command/package.js | 4 ++++ tools/tests/run-command.js | 18 ++++++++++++++++++ 6 files changed, 33 insertions(+) create mode 100644 tools/tests/apps/with-command/.meteor/.gitignore create mode 100644 tools/tests/apps/with-command/.meteor/packages create mode 100644 tools/tests/apps/with-command/.meteor/release create mode 100644 tools/tests/apps/with-command/packages/command/foo.js create mode 100644 tools/tests/apps/with-command/packages/command/package.js create mode 100644 tools/tests/run-command.js diff --git a/tools/tests/apps/with-command/.meteor/.gitignore b/tools/tests/apps/with-command/.meteor/.gitignore new file mode 100644 index 0000000000..4083037423 --- /dev/null +++ b/tools/tests/apps/with-command/.meteor/.gitignore @@ -0,0 +1 @@ +local diff --git a/tools/tests/apps/with-command/.meteor/packages b/tools/tests/apps/with-command/.meteor/packages new file mode 100644 index 0000000000..301b942bde --- /dev/null +++ b/tools/tests/apps/with-command/.meteor/packages @@ -0,0 +1,4 @@ +# Meteor packages used by this project, one per line. +# +# 'meteor add' and 'meteor remove' will edit this file for you, +# but you can also edit it by hand. diff --git a/tools/tests/apps/with-command/.meteor/release b/tools/tests/apps/with-command/.meteor/release new file mode 100644 index 0000000000..621e94f0ec --- /dev/null +++ b/tools/tests/apps/with-command/.meteor/release @@ -0,0 +1 @@ +none diff --git a/tools/tests/apps/with-command/packages/command/foo.js b/tools/tests/apps/with-command/packages/command/foo.js new file mode 100644 index 0000000000..0d1fd9f1b1 --- /dev/null +++ b/tools/tests/apps/with-command/packages/command/foo.js @@ -0,0 +1,5 @@ +// For testing meteor run-command. +main = function (argv) { + console.log("argv", argv); + return 17; +}; diff --git a/tools/tests/apps/with-command/packages/command/package.js b/tools/tests/apps/with-command/packages/command/package.js new file mode 100644 index 0000000000..1471b66aa2 --- /dev/null +++ b/tools/tests/apps/with-command/packages/command/package.js @@ -0,0 +1,4 @@ +Package.on_use(function (api) { + api.add_files('foo.js'); + api.export('main', 'server'); +}); diff --git a/tools/tests/run-command.js b/tools/tests/run-command.js new file mode 100644 index 0000000000..80cb8c2abd --- /dev/null +++ b/tools/tests/run-command.js @@ -0,0 +1,18 @@ +var selftest = require('../selftest.js'); +var Sandbox = selftest.Sandbox; + +selftest.define("run-command", function () { + var s = new Sandbox; + var run; + + s.createApp("myapp", "with-command"); + run = s.run("run-command", "myapp/packages/command"); + run.read("argv []\n"); + run.expectEnd(); + run.expectExit(17); + + run = s.run("run-command", "myapp/packages/command", "x", "--", "-f", "--bla"); + run.read("argv [ 'x', '-f', '--bla' ]\n"); + run.expectEnd(); + run.expectExit(17); +}); From fb9592a93dc8aaaf181038d1f34ebc1f1daf2f53 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 17 Feb 2014 20:01:27 -0800 Subject: [PATCH 15/38] Allow setting nested field named _id Disallow any mod on (top-level) _id, not just $sets that change the _id. Fixes #1794. --- packages/minimongo/minimongo_tests.js | 5 +++++ packages/minimongo/modify.js | 6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js index 4bb58d2b94..ef2fed60b1 100644 --- a/packages/minimongo/minimongo_tests.js +++ b/packages/minimongo/minimongo_tests.js @@ -1949,6 +1949,7 @@ Tinytest.add("minimongo - modify", function (test) { modify({a: [1, 2]}, {$inc: {'a.3': 10}}, {a: [1, 2, null, 10]}); modify({a: {b: 2}}, {$inc: {'a.b': 10}}, {a: {b: 12}}); modify({a: {b: 2}}, {$inc: {'a.c': 10}}, {a: {b: 2, c: 10}}); + exception({}, {$inc: {_id: 1}}); // $set modify({a: 1, b: 2}, {$set: {a: 10}}, {a: 10, b: 2}); @@ -1960,6 +1961,9 @@ Tinytest.add("minimongo - modify", function (test) { modify({a: [1], b: 2}, {$set: {'a.1': 9}}, {a: [1, 9], b: 2}); modify({a: [1], b: 2}, {$set: {'a.2': 9}}, {a: [1, null, 9], b: 2}); modify({a: {b: 1}}, {$set: {'a.c': 9}}, {a: {b: 1, c: 9}}); + modify({}, {$set: {'x._id': 4}}, {x: {_id: 4}}); + exception({}, {$set: {_id: 4}}); + exception({_id: 4}, {$set: {_id: 4}}); // even not-changing _id is bad // $unset modify({}, {$unset: {a: 1}}, {}); @@ -1977,6 +1981,7 @@ Tinytest.add("minimongo - modify", function (test) { modify({a: {b: 1}}, {$unset: {'a.b.c.d': 1}}, {a: {b: 1}}); modify({a: {b: 1}}, {$unset: {'a.x.c.d': 1}}, {a: {b: 1}}); modify({a: {b: {c: 1}}}, {$unset: {'a.b.c': 1}}, {a: {b: {}}}); + exception({}, {$unset: {_id: 1}}); // $push modify({}, {$push: {a: 1}}, {a: [1]}); diff --git a/packages/minimongo/modify.js b/packages/minimongo/modify.js index ec768fd11b..3b95e5363f 100644 --- a/packages/minimongo/modify.js +++ b/packages/minimongo/modify.js @@ -47,6 +47,9 @@ LocalCollection._modify = function (doc, mod, options) { throw MinimongoError( "Invalid mod field name, may not end in a period"); + if (keypath === '_id') + throw MinimongoError("Mod on _id not allowed"); + var keyparts = keypath.split('.'); var noCreate = _.has(NO_CREATE_MODIFIERS, op); var forbidArray = (op === "$rename"); @@ -194,9 +197,6 @@ var MODIFIERS = { e.setPropertyError = true; throw e; } - if (field === '_id' && !EJSON.equals(arg, target._id)) - throw MinimongoError("Cannot change the _id of a document"); - target[field] = EJSON.clone(arg); }, $setOnInsert: function (target, field, arg) { From c0d5fed96c8e976aa7565ccdae3246f07148bc4b Mon Sep 17 00:00:00 2001 From: Geoff Schmidt Date: Tue, 18 Feb 2014 14:03:06 -0800 Subject: [PATCH 16/38] help text tweaks --- tools/help.txt | 52 +++++++++++++++++++++++++------------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/tools/help.txt b/tools/help.txt index 068e489cba..c8fd32c737 100644 --- a/tools/help.txt +++ b/tools/help.txt @@ -218,6 +218,32 @@ site into your account. If you had set a password on the site you will be prompted for it one last time. +>>> login +Log in to your Meteor developer account +Usage: meteor login [--email] + +Prompts for your username and password and logs you in to your Meteor +developer account. Pass --email to log in by email address rather than +by username. + + +>>> logout +Log out of your Meteor developer account +Usage: meteor logout + +Log out of your Meteor developer account. + + +>>> whoami +Prints the username of your Meteor developer account +Usage: meteor whoami + +Prints the username of the currently logged-in Meteor developer. + +See 'meteor login' to log into or 'meteor logout' to log out of your +Meteor developer account. + + >>> test-packages Test one or more packages Usage: meteor test-packages [--release ] [options] [package...] @@ -283,32 +309,6 @@ This command is for temporary, internal use, until we have a more mature system for building standalone command-line programs with Meteor. ->>> login -Log in to your Meteor account -Usage: meteor login [--email] [--galaxy ] - -Prompts for your username and password and logs you in to your -Meteor account. Pass --email to log in by email address instead of -username. Pass --galaxy to specify a galaxy to log in to. - - ->>> logout -Log out of your Meteor account -Usage: meteor logout - -Log out of your Meteor account. - - ->>> whoami -Prints the username of your Meteor developer account -Usage: meteor whoami - -Prints the username of the currently logged-in Meteor developer. - -See 'meteor login' to log into or 'meteor logout' to log out of of your -Meteor account. - - >>> self-test Run tests of the 'meteor' tool. Usage: meteor self-test [--changed] [--slow] [--force-online] [--history n] From dd2b5ce7299b6f0296122f66ed988955e5e942c1 Mon Sep 17 00:00:00 2001 From: Geoff Schmidt Date: Tue, 18 Feb 2014 14:17:41 -0800 Subject: [PATCH 17/38] 'login' without --galaxy always prompts for username/password --- tools/commands.js | 6 +++++- tools/tests/login.js | 14 ++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/tools/commands.js b/tools/commands.js index 5992657887..6eff8b91aa 100644 --- a/tools/commands.js +++ b/tools/commands.js @@ -1202,10 +1202,14 @@ main.registerCommand({ name: 'login', options: { email: { type: String }, + // Undocumented: get credentials on a specific Galaxy. Do we still + // need this? galaxy: { type: String } } }, function (options) { - return auth.loginCommand(options); + return auth.loginCommand(_.extend({ + overwriteExistingToken: true + }, options)); }); diff --git a/tools/tests/login.js b/tools/tests/login.js index 6862048982..b0ae42c57a 100644 --- a/tools/tests/login.js +++ b/tools/tests/login.js @@ -11,6 +11,20 @@ selftest.define("login", ['net'], function () { // Username and password prompts happen on stderr so that scripts can // run commands that do login interactively and still save the command // output with the login prompts appearing in it. + // + // Do this twice to confirm that the login command prints a prompt + // even if you are already logged in. + for (var i = 0; i < 2; i++) { + run = s.run("login"); + run.matchErr("Username:"); + run.write("test\n"); + run.matchErr("Password:"); + run.write("testtest\n"); + run.waitSecs(5); + run.matchErr("Logged in as test."); + run.expectExit(0); + } + run = s.run("login"); run.matchErr("Username:"); run.write("test\n"); From 653045ffec060855ae9d01ddcedbe13216fddf3e Mon Sep 17 00:00:00 2001 From: Geoff Schmidt Date: Tue, 18 Feb 2014 14:18:07 -0800 Subject: [PATCH 18/38] In self-test, make '--tests' into an argument and document it. --- tools/commands.js | 9 +++++---- tools/help.txt | 6 +++++- tools/selftest.js | 4 ++-- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/tools/commands.js b/tools/commands.js index 6eff8b91aa..d787019208 100644 --- a/tools/commands.js +++ b/tools/commands.js @@ -1256,12 +1256,13 @@ main.registerCommand({ main.registerCommand({ name: 'self-test', + minArgs: 0, + maxArgs: 1, options: { changed: { type: Boolean }, 'force-online': { type: Boolean }, slow: { type: Boolean }, history: { type: Number }, - tests: { type: String } }, hidden: true }, function (options) { @@ -1279,13 +1280,13 @@ main.registerCommand({ } var testRegexp = undefined; - if (options.tests) { + if (options.args.length) { try { - testRegexp = new RegExp(options.tests); + testRegexp = new RegExp(options.args[0]); } catch (e) { if (!(e instanceof SyntaxError)) throw e; - process.stderr.write("Bad regular expression: " + options.tests + "\n"); + process.stderr.write("Bad regular expression: " + options.args[0] + "\n"); return 1; } } diff --git a/tools/help.txt b/tools/help.txt index c8fd32c737..0c9a492294 100644 --- a/tools/help.txt +++ b/tools/help.txt @@ -311,10 +311,14 @@ system for building standalone command-line programs with Meteor. >>> self-test Run tests of the 'meteor' tool. -Usage: meteor self-test [--changed] [--slow] [--force-online] [--history n] +Usage: meteor self-test [pattern] [--changed] [--slow] + [--force-online] [--history n] Runs internal tests. Exits with status 0 on success. +If 'pattern' is provided, it should be a regular expression. Only +tests that match the regular expression will be run. + Pass --changed to run only tests that have changed since they last passed. This uses a really rough heuristic: A test has changed iff there has been any change to the file in the 'selftests' subdirectory diff --git a/tools/selftest.js b/tools/selftest.js index bfabb4db87..9c91fee132 100644 --- a/tools/selftest.js +++ b/tools/selftest.js @@ -976,7 +976,7 @@ var tagDescriptions = { // these last two are not actually test tags; they reflect the use of // --changed and --tests unchanged: 'unchanged since last pass', - misnamed: "don't match --tests argument" + 'non-matching': "don't match specified pattern" }; // options: onlyChanged, offline, includeSlowTests, historyLines, testRegexp @@ -1015,7 +1015,7 @@ var runTests = function (options) { tests = _.filter(tests, function (test) { return options.testRegexp.test(test.name); }); - skipCounts.misnamed = lengthBeforeTestRegexp - tests.length; + skipCounts['non-matching'] = lengthBeforeTestRegexp - tests.length; } if (options.onlyChanged) { From bee4fbbd247c0fcd5a5bb1d11ee19d24c5ab9419 Mon Sep 17 00:00:00 2001 From: Geoff Schmidt Date: Tue, 18 Feb 2014 14:48:29 -0800 Subject: [PATCH 19/38] Reprompt if they enter a blank username to logs or mongo. With test. --- tools/auth.js | 15 ++++++++++----- tools/tests/login.js | 17 ++++++++++++++--- tools/tests/logs-mongo-auth.js | 18 +++++++++++++++++- 3 files changed, 41 insertions(+), 9 deletions(-) diff --git a/tools/auth.js b/tools/auth.js index b679086441..f2268f364b 100644 --- a/tools/auth.js +++ b/tools/auth.js @@ -584,13 +584,18 @@ var doInteractivePasswordLogin = function (options) { return true; }; -// options are the same as for doInteractivePasswordLogin, exept without +// options are the same as for doInteractivePasswordLogin, except without // username and email. exports.doUsernamePasswordLogin = function (options) { - var username = utils.readLine({ - prompt: "Username: ", - stream: process.stderr - }); + var username; + + do { + username = utils.readLine({ + prompt: "Username: ", + stream: process.stderr + }).trim(); + } while (username.length === 0); + return doInteractivePasswordLogin(_.extend({}, options, { username: username })); diff --git a/tools/tests/login.js b/tools/tests/login.js index b0ae42c57a..c4e04b5853 100644 --- a/tools/tests/login.js +++ b/tools/tests/login.js @@ -25,14 +25,25 @@ selftest.define("login", ['net'], function () { run.expectExit(0); } + // Leaving username blank, or getting the password wrong, doesn't + // reprompt. It also doesn't log you out. + run = s.run("login"); + run.matchErr("Username:"); + run.write("\n"); + run.matchErr("Password:"); + run.write("whatever\n"); + run.waitSecs(5); + run.matchErr("failed"); + run.expectExit(1); + run = s.run("login"); run.matchErr("Username:"); run.write("test\n"); run.matchErr("Password:"); - run.write("testtest\n"); + run.write("whatever\n"); run.waitSecs(5); - run.matchErr("Logged in as test."); - run.expectExit(0); + run.matchErr("failed"); + run.expectExit(1); // XXX test login by email diff --git a/tools/tests/logs-mongo-auth.js b/tools/tests/logs-mongo-auth.js index 47f5abf0ab..778306ab55 100644 --- a/tools/tests/logs-mongo-auth.js +++ b/tools/tests/logs-mongo-auth.js @@ -68,8 +68,23 @@ var logsOrMongoForApp = function (sandbox, command, appName, options) { } else { // If we are not logged in and this is not a legacy app, then we // expect a login prompt. + // + // (If testReprompt is true, try getting reprompted as a result + // of entering no username or a bad password.) + if (options.testReprompt) { + run.matchErr('Username: '); + run.write("\n"); + run.matchErr("Username:"); + run.write(" \n"); + } run.matchErr('Username: '); run.write((options.username || 'test') + '\n'); + if (options.testReprompt) { + run.matchErr("Password:"); + run.write("wrongpassword\n"); + run.waitSecs(15); + run.matchErr("failed"); + } run.matchErr('Password: '); run.write((options.password || 'testtest') + '\n'); run.waitSecs(15); @@ -119,7 +134,8 @@ _.each([false, true], function (loggedIn) { logsOrMongoForApp(s, command, 'app-for-selftest-not-test-owned', { loggedIn: loggedIn, - authorized: false + authorized: false, + testReprompt: true }); if (! loggedIn) { From 97d2264a3aebd0bf5aeea48cbb7ea11bd7f45fec Mon Sep 17 00:00:00 2001 From: Geoff Schmidt Date: Tue, 18 Feb 2014 15:38:26 -0800 Subject: [PATCH 20/38] Fix a "Future resolved more than once" error --- tools/auth.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tools/auth.js b/tools/auth.js index f2268f364b..395129324f 100644 --- a/tools/auth.js +++ b/tools/auth.js @@ -229,8 +229,13 @@ var currentUsername = function (data) { // username, then ask the server if we have one. var username = null; var fut = new Future(); + var resolved = false; var connection = loggedInAccountsConnection(sessionData.token); connection.call('getUsername', function (err, result) { + if (resolved) + return; + resolved = true; + if (err) { // If anything went wrong, return null just as we would have if // we hadn't bothered to ask the server. @@ -240,12 +245,16 @@ var currentUsername = function (data) { fut['return'](result); }); - setTimeout(inFiber(function () { - fut['return'](null); + var timer = setTimeout(inFiber(function () { + if (! resolved) { + fut['return'](null); + resolved = true; + } }), 5000); username = fut.wait(); connection.close(); + clearTimeout(timer); if (username) { writeMeteorAccountsUsername(username); } From 96e3273f284d175142c786a024afbbceef6e83b1 Mon Sep 17 00:00:00 2001 From: Geoff Schmidt Date: Tue, 18 Feb 2014 15:43:55 -0800 Subject: [PATCH 21/38] make account setup link even more handy --- tools/auth.js | 24 ++++++++++++++++++++++++ tools/commands.js | 36 ++++++++++++++---------------------- tools/deploy.js | 5 +++-- 3 files changed, 41 insertions(+), 24 deletions(-) diff --git a/tools/auth.js b/tools/auth.js index 395129324f..6c1ab6a3a6 100644 --- a/tools/auth.js +++ b/tools/auth.js @@ -835,6 +835,30 @@ exports.registerOrLogIn = withAccountsConnection(function (connection) { } }); +// options: firstTime, leadingNewline +exports.maybePrintRegistrationLink = function (options) { + options = options || {}; + + var registrationUrl = auth.registrationUrl(); + if (registrationUrl && ! auth.currentUsername()) { + if (options.leadingNewline) + process.stderr.write("\n"); + if (! options.firstTime) { + // If they've already been prompted to set a password then this + // is more of a friendly reminder, so we word it slightly + // differently than the first time they're being shown a + // registration url. + process.stderr.write( +"You should set a password on your Meteor developer account. It takes\n" + +"about a minute at: " + registrationUrl + "\n\n"); + } else { + process.stderr.write( +"You can set a password on your account or change your email address at:\n" + +registrationUrl + "\n\n"); + } + } +}; + exports.tryRevokeOldTokens = tryRevokeOldTokens; exports.getSessionId = function (domain) { diff --git a/tools/commands.js b/tools/commands.js index d787019208..39c26a6cf9 100644 --- a/tools/commands.js +++ b/tools/commands.js @@ -678,6 +678,7 @@ main.registerCommand({ } }, function (options) { var mongoUrl; + var usedMeteorAccount = false; if (options.args.length === 0) { // localhost mode @@ -708,6 +709,7 @@ main.registerCommand({ mongoUrl = deployGalaxy.temporaryMongoUrl(site); } else { mongoUrl = deploy.temporaryMongoUrl(site); + usedMeteorAccount = true; } if (! mongoUrl) @@ -717,6 +719,8 @@ main.registerCommand({ if (options.url) { console.log(mongoUrl); } else { + if (usedMeteorAccount) + auth.maybePrintRegistrationLink(); process.stdin.pause(); var runMongo = require('./run-mongo.js'); runMongo.runMongoShell(mongoUrl); @@ -862,26 +866,16 @@ main.registerCommand({ }); } - var registrationUrl = auth.registrationUrl(); - if (registrationUrl && - deployResult === 0 && - ! auth.currentUsername()) { - process.stderr.write("\n"); - if (loggedIn) { + if (deployResult === 0) { + auth.maybePrintRegistrationLink({ + leadingNewline: true, // If the user was already logged in at the beginning of the // deploy, then they've already been prompted to set a password - // and this is more of a friendly reminder to set their password, - // so we word it slightly differently than the first time they're - // being shown a registration url. - process.stderr.write( -"You should set a password on your Meteor developer account. It takes\n" + -"about a minute at: " + registrationUrl + "\n\n"); - } else { - process.stderr.write( -"You can set a password on your account or change your email address at:\n" + -registrationUrl + "\n\n"); - } + // at least once before, so we use a slightly different message. + firstTime: ! loggedIn + }); } + return deployResult; }); @@ -979,12 +973,10 @@ main.registerCommand({ var site = qualifySitename(options.args[0]); if (! auth.isLoggedIn()) { - // XXX meteor.com/create-account or something should have a nice - // registration form process.stderr.write( -"\nYou must be logged in to claim sites. Use 'meteor login' to log in.\n" + -"If you don't have a Meteor developer account yet, you can quickly\n" + -"create one at www.meteor.com.\n\n"); +"You must be logged in to claim sites. Use 'meteor login' to log in.\n" + +"If you don't have a Meteor developer account yet, create one by clicking\n" + +"'Sign in' and then 'Create account' at www.meteor.com.\n\n"); return 1; } diff --git a/tools/deploy.js b/tools/deploy.js index 4524014edb..236d2b3b02 100644 --- a/tools/deploy.js +++ b/tools/deploy.js @@ -539,6 +539,7 @@ var logs = function (site) { return 1; } else { process.stdout.write(result.message); + auth.maybePrintRegistrationLink({ leadingNewline: true }); return 0; } }; @@ -662,8 +663,8 @@ var claim = function (site) { if (! auth.currentUsername() && auth.registrationUrl()) { process.stderr.write( -"\nBefore you can claim existing sites, you need to set a password on\n" + -"your Meteor developer account. You can do that here in under a minute:\n\n" + +"You need to set a password on your Meteor developer account before\n" + +"you can claim sites. You can do that here in under a minute:\n\n" + auth.registrationUrl() + "\n\n"); } else { process.stderr.write("Couldn't claim site: " + From d3291f723431aec31adec8ce6a00399559de7458 Mon Sep 17 00:00:00 2001 From: Geoff Schmidt Date: Tue, 18 Feb 2014 15:44:09 -0800 Subject: [PATCH 22/38] one space after period --- tools/deploy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/deploy.js b/tools/deploy.js index 236d2b3b02..64251f55eb 100644 --- a/tools/deploy.js +++ b/tools/deploy.js @@ -341,7 +341,7 @@ var bundleAndDeploy = function (options) { var buildDir = path.join(options.appDir, '.meteor', 'local', 'build_tar'); var bundlePath = path.join(buildDir, 'bundle'); - process.stdout.write('Deploying to ' + site + '. Bundling...\n'); + process.stdout.write('Deploying to ' + site + '. Bundling...\n'); var settings = null; var messages = buildmessage.capture({ From d1d714ec66aeb5ecae1130da1d9e5a39268318d0 Mon Sep 17 00:00:00 2001 From: Geoff Schmidt Date: Tue, 18 Feb 2014 17:25:19 -0800 Subject: [PATCH 23/38] 'whoami' gracefully handles revocation of a credential on a passwordless account. with test --- scripts/admin/publish-release/packages/awssum | 2 +- tools/auth.js | 198 +++++++++++------- tools/commands.js | 2 + tools/deploy.js | 2 +- tools/selftest.js | 14 ++ tools/tests/registration.js | 46 ++++ 6 files changed, 185 insertions(+), 79 deletions(-) diff --git a/scripts/admin/publish-release/packages/awssum b/scripts/admin/publish-release/packages/awssum index 69dc3c7afb..0221218663 160000 --- a/scripts/admin/publish-release/packages/awssum +++ b/scripts/admin/publish-release/packages/awssum @@ -1 +1 @@ -Subproject commit 69dc3c7afb455cb487b6fbe551478384dea0cb0b +Subproject commit 0221218663b12b47f61cd9745da9c2be3f247bdb diff --git a/tools/auth.js b/tools/auth.js index 6c1ab6a3a6..632d9400b0 100644 --- a/tools/auth.js +++ b/tools/auth.js @@ -50,25 +50,39 @@ var withAccountsConnection = function (f) { }; }; -// Open a DDP connection to the accounts server, log in using the -// provided token, and ensure that the connection stays logged in across -// reconnects. +// Open a DDP connection to the accounts server and log in using the +// provided token. Returns the connection, or null if login fails. +// +// XXX if we reconnect we won't reauthenticate. Fix that before using +// this for long-lived connections. var loggedInAccountsConnection = function (token) { var connection = getLoadedPackages().livedata.DDP.connect( config.getAuthDDPUrl() ); - var onReconnect = function () { - connection.apply( - 'login', - [{ resume: token }], - { wait: true }, - function (err, result) { - if (err) - throw err; - } - ); - }; - onReconnect(); + + var fut = new Future; + connection.apply( + 'login', + [{ resume: token }], + { wait: true }, + function (err, result) { + fut['return']({ err: err, result: result }); + } + ); + var outcome = fut.wait(); + + if (outcome.err) { + if (outcome.err.error === 403) { + // This is not an ideal value for the error code, but it means + // "server rejected our access token". For example, it expired + // or we revoked it from the web. + return null; + } + + // Something else went wrong + throw outcome.err; + } + return connection; }; @@ -189,26 +203,33 @@ var writeMeteorAccountsUsername = function (username) { // Given an object 'data' in the format returned by readSessionData, // modify it to make the user logged out. var logOutAllSessions = function (data) { + _.each(data.sessions, function (session, domain) { + logOutSession(session); + }); +}; + +// As logOutAllSessions, but for a session on a particular domain +// rather than all sessions. +var logOutSession = function (session) { var crypto = require('crypto'); - _.each(data.sessions, function (session, domain) { - delete session.username; - delete session.userId; + delete session.username; + delete session.userId; + delete session.registrationUrl; - if (_.has(session, 'token')) { - if (! (session.pendingRevoke instanceof Array)) - session.pendingRevoke = []; + if (_.has(session, 'token')) { + if (! (session.pendingRevoke instanceof Array)) + session.pendingRevoke = []; - // Delete the auth token itself, but save the tokenId, which - // is useless for authentication. The next time we're online, - // we'll send the tokenId to the server to revoke the token on - // the server side too. - if (typeof session.tokenId === "string") - session.pendingRevoke.push(session.tokenId); - delete session.token; - delete session.tokenId; - } - }); + // Delete the auth token itself, but save the tokenId, which is + // useless for authentication. The next time we're online, we'll + // send the tokenId to the server to revoke the token on the + // server side too. + if (typeof session.tokenId === "string") + session.pendingRevoke.push(session.tokenId); + delete session.token; + delete session.tokenId; + } }; // Given an object 'data' in the format returned by readSessionData, @@ -222,46 +243,7 @@ var loggedIn = function (data) { // the logged in user doesn't have a username. var currentUsername = function (data) { var sessionData = getSession(data, config.getAccountsDomain()); - if (sessionData.username) { - return sessionData.username; - } else if (loggedIn(data) && sessionData.token) { - // If it looks like we are logged in but we don't yet have a - // username, then ask the server if we have one. - var username = null; - var fut = new Future(); - var resolved = false; - var connection = loggedInAccountsConnection(sessionData.token); - connection.call('getUsername', function (err, result) { - if (resolved) - return; - resolved = true; - - if (err) { - // If anything went wrong, return null just as we would have if - // we hadn't bothered to ask the server. - fut['return'](null); - return; - } - fut['return'](result); - }); - - var timer = setTimeout(inFiber(function () { - if (! resolved) { - fut['return'](null); - resolved = true; - } - }), 5000); - - username = fut.wait(); - connection.close(); - clearTimeout(timer); - if (username) { - writeMeteorAccountsUsername(username); - } - return username; - } else { - return null; - } + return sessionData.username || null; }; var removePendingRevoke = function (domain, tokenIds) { @@ -699,9 +681,66 @@ exports.logoutCommand = function (options) { process.stderr.write("Not logged in.\n"); }; -exports.currentUsername = function () { +// If this is fully set up account (with a username and password), or +// if not logged in, do nothing. If it is an account without a +// username, contact the server and see if the user finished setting +// it up, and if so grab and save the username. But contact the server +// only once per run of the program. +var alreadyPolledForRegistration = false; +exports.pollForRegistrationCompletion = function () { + if (alreadyPolledForRegistration) + return; + alreadyPolledForRegistration = true; + var data = readSessionData(); - return currentUsername(data); + var session = getSession(data, config.getAccountsDomain()); + if (session.username || ! session.token) + return; + + // We are logged in but we don't yet have a username. Ask the server + // if a username was chosen since we last checked. + var username = null; + var fut = new Future(); + var resolved = false; + var connection = loggedInAccountsConnection(session.token); + + if (! connection) { + // Server says our credential isn't any good anymore! Get rid of + // it. Note that, out of an abundance of caution, this also will + // enqueue the credential for invalidation (on a future run, we + // will try to explicitly revoke the credential ourselves). + logOutSession(session); + writeSessionData(data); + return; + } + + connection.call('getUsername', function (err, result) { + if (resolved) + return; + resolved = true; + + if (err) { + // If anything went wrong, return null just as we would have if + // we hadn't bothered to ask the server. + fut['return'](null); + return; + } + fut['return'](result); + }); + + var timer = setTimeout(inFiber(function () { + if (! resolved) { + fut['return'](null); + resolved = true; + } + }), 5000); + + username = fut.wait(); + connection.close(); + clearTimeout(timer); + if (username) { + writeMeteorAccountsUsername(username); + } }; exports.registrationUrl = function () { @@ -712,6 +751,7 @@ exports.registrationUrl = function () { exports.whoAmICommand = function (options) { config.printUniverseBanner(); + auth.pollForRegistrationCompletion(); var data = readSessionData(); if (! loggedIn(data)) { @@ -839,8 +879,12 @@ exports.registerOrLogIn = withAccountsConnection(function (connection) { exports.maybePrintRegistrationLink = function (options) { options = options || {}; - var registrationUrl = auth.registrationUrl(); - if (registrationUrl && ! auth.currentUsername()) { + auth.pollForRegistrationCompletion(); + + var data = readSessionData(); + var session = getSession(data, config.getAccountsDomain()); + + if (session.userId && ! session.username && session.registrationUrl) { if (options.leadingNewline) process.stderr.write("\n"); if (! options.firstTime) { @@ -850,11 +894,11 @@ exports.maybePrintRegistrationLink = function (options) { // registration url. process.stderr.write( "You should set a password on your Meteor developer account. It takes\n" + -"about a minute at: " + registrationUrl + "\n\n"); +"about a minute at: " + session.registrationUrl + "\n\n"); } else { process.stderr.write( "You can set a password on your account or change your email address at:\n" + -registrationUrl + "\n\n"); +session.registrationUrl + "\n\n"); } } }; diff --git a/tools/commands.js b/tools/commands.js index 39c26a6cf9..d25771be44 100644 --- a/tools/commands.js +++ b/tools/commands.js @@ -936,6 +936,7 @@ main.registerCommand({ } config.printUniverseBanner(); + auth.pollForRegistrationCompletion(); var site = qualifySitename(options.args[0]); if (hostedWithGalaxy(site)) { @@ -970,6 +971,7 @@ main.registerCommand({ maxArgs: 1 }, function (options) { config.printUniverseBanner(); + auth.pollForRegistrationCompletion(); var site = qualifySitename(options.args[0]); if (! auth.isLoggedIn()) { diff --git a/tools/deploy.js b/tools/deploy.js index 64251f55eb..ea226e79b4 100644 --- a/tools/deploy.js +++ b/tools/deploy.js @@ -262,7 +262,7 @@ var printLegacyPasswordMessage = function (site) { // authorized for, instruct them to get added via 'meteor authorized // --add' or switch accounts. var printUnauthorizedMessage = function () { - var username = auth.currentUsername(); + var username = auth.loggedInUsername(); process.stderr.write( "\nSorry, that site belongs to a different user.\n" + (username ? "You are currently logged in as " + username + ".\n" : "") + diff --git a/tools/selftest.js b/tools/selftest.js index 9c91fee132..dc1bcf8445 100644 --- a/tools/selftest.js +++ b/tools/selftest.js @@ -477,6 +477,20 @@ _.extend(Sandbox.prototype, { unlink: function (filename) { var self = this; fs.unlinkSync(path.join(self.cwd, filename)); + }, + + // Return the current contents of .meteorsession in the sandbox. + readSessionFile: function () { + var self = this; + return fs.readFileSync(path.join(self.root, '.meteorsession'), 'utf8'); + }, + + // Overwrite .meteorsession in the sandbox with 'contents'. You + // could use this in conjunction with readSessionFile to save and + // restore authentication states. + writeSessionFile: function (contents) { + var self = this; + return fs.readFileSync(path.join(self.root, '.meteorsession'), 'utf8'); } }); diff --git a/tools/tests/registration.js b/tools/tests/registration.js index d740400608..43ce9adbfa 100644 --- a/tools/tests/registration.js +++ b/tools/tests/registration.js @@ -159,3 +159,49 @@ selftest.define('deferred registration', ['net'], function () { // return (e.g. deploy and login with an existing username that // doesn't have a password set yet) }); + +selftest.define('deferred registration revocation', ['net'], function () { + // Test that if we are logged in as a passwordless user, and our + // credential gets revoked, and we do something like 'meteor whoami' + // that polls to see if registration is complete, then we handle it + // gracefully. + + var s = new Sandbox; + s.createApp('deployapp', 'empty'); + s.cd('deployapp'); + + // Create a new deferred registration account. (Don't bother to wait + // for the deploy to go through.) + var email = testUtils.randomUserEmail(); + var username = testUtils.randomString(10); + var appName = testUtils.randomAppName(); + var run = s.run('deploy', appName); + run.waitSecs(5); + run.matchErr('Email:'); + run.write(email + '\n'); + run.waitSecs(90); + run.match('Deploying'); + run.stop(); + + // 'whoami' says that we don't have a password + run = s.run('whoami'); + run.waitSecs(15); + run.matchErr('/setPassword?'); + run.expectExit(1); + + // Revoke the credential without updating .meteorsession. + var sessionState = s.readSessionFile(); + run = s.run('logout'); + run.waitSecs(15); + run.readErr("Logged out.\n"); + run.expectEnd(); + run.expectExit(0); + s.writeSessionFile(sessionState); + + // 'whoami' now says that we're not logged in. No errors are printed. + run = s.run('whoami'); + run.waitSecs(15); + run.readErr("Not logged in. 'meteor login' to log in.\n"); + run.expectEnd(); + run.expectExit(1); +}); \ No newline at end of file From 31b387b775f0fecd16d1f8f8ea0438afea902b1f Mon Sep 17 00:00:00 2001 From: Geoff Schmidt Date: Wed, 19 Feb 2014 12:34:08 -0800 Subject: [PATCH 24/38] fixed for 'meteor logs' on a galaxy --- tools/deploy-galaxy.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/deploy-galaxy.js b/tools/deploy-galaxy.js index 021c570051..452ec7d033 100644 --- a/tools/deploy-galaxy.js +++ b/tools/deploy-galaxy.js @@ -32,14 +32,14 @@ var handleError = function (error, galaxyName, messages) { var Package = getPackage(); messages = messages || {}; - if (e instanceof Package.meteor.Meteor.Error) { + if (error instanceof Package.meteor.Meteor.Error) { var msg = messages[error.error]; if (msg) process.stderr.write(msg + "\n"); else if (error.message) process.stderr.write("Denied: " + error.message + "\n"); return 1; - } else if (e instanceof ConnectionTimeoutError) { + } else if (error instanceof ConnectionTimeoutError) { // If we have an http/https URL for a galaxyName instead of a // proper galaxyName (which is what the code in this file // currently passes), strip off the scheme and trailing slash. @@ -150,7 +150,7 @@ _.extend(ServiceConnection.prototype, { } }); - self.subscribe.apply(self, args); + self.connection.subscribe.apply(self.connection, args); return fut.wait(); }, From 92abbf2f5c4ad2ee0c5ba0f8ea9a968a62ef43cf Mon Sep 17 00:00:00 2001 From: Geoff Schmidt Date: Wed, 19 Feb 2014 12:34:33 -0800 Subject: [PATCH 25/38] deploy server now returns 401 for invalid credentials --- tools/deploy.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/deploy.js b/tools/deploy.js index ea226e79b4..a05db5d40f 100644 --- a/tools/deploy.js +++ b/tools/deploy.js @@ -147,7 +147,7 @@ var authedRpc = function (options) { expectPayload: [] }); - if (infoResult.statusCode === 403 && rpcOptions.promptIfAuthFails) { + if (infoResult.statusCode === 401 && rpcOptions.promptIfAuthFails) { // Our authentication didn't validate, so prompt the user to log in // again, and resend the RPC if the login succeeds. var username = utils.readLine({ prompt: "Username: " }); @@ -264,7 +264,7 @@ var printLegacyPasswordMessage = function (site) { var printUnauthorizedMessage = function () { var username = auth.loggedInUsername(); process.stderr.write( -"\nSorry, that site belongs to a different user.\n" + +"Sorry, that site belongs to a different user.\n" + (username ? "You are currently logged in as " + username + ".\n" : "") + "\nEither have the site owner use 'meteor authorized --add' to add you\n" + "as an authorized developer for the site, or switch to an authorized\n" + @@ -487,6 +487,7 @@ var checkAuthThenSendRpc = function (site, operation, what) { return null; } } else { // User is logged in but not authorized for this app + process.stderr.write("\n"); printUnauthorizedMessage(); return null; } From c8e75d9afc9b5c45c0ef914e0c28644ec55843c5 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 19 Feb 2014 14:46:28 -0800 Subject: [PATCH 26/38] Use fut.isResolved() instead of a separate 'resolved' variable. Add test for 'meteor whoami' after setting username but before logging in again. --- tools/auth.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tools/auth.js b/tools/auth.js index 632d9400b0..cf3601cceb 100644 --- a/tools/auth.js +++ b/tools/auth.js @@ -701,7 +701,6 @@ exports.pollForRegistrationCompletion = function () { // if a username was chosen since we last checked. var username = null; var fut = new Future(); - var resolved = false; var connection = loggedInAccountsConnection(session.token); if (! connection) { @@ -715,9 +714,8 @@ exports.pollForRegistrationCompletion = function () { } connection.call('getUsername', function (err, result) { - if (resolved) + if (fut.isResolved()) return; - resolved = true; if (err) { // If anything went wrong, return null just as we would have if @@ -729,9 +727,8 @@ exports.pollForRegistrationCompletion = function () { }); var timer = setTimeout(inFiber(function () { - if (! resolved) { + if (! fut.isResolved()) { fut['return'](null); - resolved = true; } }), 5000); From 423ae65d93c184c677d4653a4ae90fa1df3daa6f Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 19 Feb 2014 14:47:54 -0800 Subject: [PATCH 27/38] This time, actually add the test for 'meteor whoami'. --- tools/tests/registration.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tools/tests/registration.js b/tools/tests/registration.js index 43ce9adbfa..c56723ccd6 100644 --- a/tools/tests/registration.js +++ b/tools/tests/registration.js @@ -144,8 +144,13 @@ selftest.define('deferred registration', ['net'], function () { }); accountsConn.close(); - // Success! We should be able to log out and log back in with our new - // password. + // Success! 'meteor whoami' should now know who we are. + run = s.run('whoami'); + run.waitSecs(testUtils.accountsCommandTimeoutSecs); + run.read(username + '\n'); + run.expectExit(0); + + // We should be able to log out and log back in with our new password. testUtils.logout(s); testUtils.login(s, username, 'testtest'); @@ -204,4 +209,4 @@ selftest.define('deferred registration revocation', ['net'], function () { run.readErr("Not logged in. 'meteor login' to log in.\n"); run.expectEnd(); run.expectExit(1); -}); \ No newline at end of file +}); From 2f6bb62170fe8f491180804fc4d38f04de448dc4 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 19 Feb 2014 15:49:06 -0800 Subject: [PATCH 28/38] Use the same timeouts in login tests as everywhere else --- tools/tests/login.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/tests/login.js b/tools/tests/login.js index 7a14824220..25b3dc0ecb 100644 --- a/tools/tests/login.js +++ b/tools/tests/login.js @@ -23,7 +23,7 @@ selftest.define("login", ['net'], function () { run.write("test\n"); run.matchErr("Password:"); run.write("testtest\n"); - run.waitSecs(5); + run.waitSecs(commandTimeoutSecs); run.matchErr("Logged in as test."); run.expectExit(0); } @@ -35,7 +35,7 @@ selftest.define("login", ['net'], function () { run.write("\n"); run.matchErr("Password:"); run.write("whatever\n"); - run.waitSecs(5); + run.waitSecs(commandTimeoutSecs); run.matchErr("failed"); run.expectExit(1); From 2ff0fa38ed9fe75f053ae461ef41899c1eb9ca71 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 19 Feb 2014 15:49:46 -0800 Subject: [PATCH 29/38] Correctly check for current username when deploying --- tools/deploy.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/deploy.js b/tools/deploy.js index a05db5d40f..201576e253 100644 --- a/tools/deploy.js +++ b/tools/deploy.js @@ -661,7 +661,8 @@ var claim = function (site) { }); if (result.errorMessage) { - if (! auth.currentUsername() && + auth.pollForRegistrationCompletion(); + if (! auth.loggedInUsername() && auth.registrationUrl()) { process.stderr.write( "You need to set a password on your Meteor developer account before\n" + From 1f8760acfcc87022aa77913fec1e7901ac0481a5 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 19 Feb 2014 15:52:31 -0800 Subject: [PATCH 30/38] Update 'meteor claim' test for new prose --- tools/tests/claim.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/tests/claim.js b/tools/tests/claim.js index 9d2d3a5680..cde0984170 100644 --- a/tools/tests/claim.js +++ b/tools/tests/claim.js @@ -141,7 +141,7 @@ selftest.define('claim - no username', ['net', 'slow'], function () { run.matchErr('Password:'); run.write('test\n'); run.waitSecs(commandTimeoutSecs); - run.matchErr('you need to set a password'); + run.matchErr('You need to set a password'); run.matchErr(testUtils.registrationUrlRegexp); run.expectExit(1); // After we set a username, we should be able to claim sites. From 023276cd50e9f0be817870b34da711b2b79ddf21 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 19 Feb 2014 15:52:57 -0800 Subject: [PATCH 31/38] Only show username prompt on deploy if we know the user has a username. We don't want a user to see a username/password prompt if they haven't set a username yet. --- tools/deploy.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tools/deploy.js b/tools/deploy.js index 201576e253..fc2117792d 100644 --- a/tools/deploy.js +++ b/tools/deploy.js @@ -313,12 +313,25 @@ var bundleAndDeploy = function (options) { if (! site) return 1; + // We should give a username/password prompt if the user was logged in + // but the credentials are expired, unless the user is logged in but + // doesn't have a username (in which case they should hit the email + // prompt -- a user without a username shouldn't be given a username + // prompt). There's an edge case where things happen in the following + // order: user creates account, user sets username, credential expires + // or is revoked, user comes back to deploy again. In that case, + // they'll get an email prompt instead of a username prompt because + // the command-line tool didn't have time to learn about their + // username before the credential was expired. + auth.pollForRegistrationCompletion(); + var promptIfAuthFails = (auth.loggedInUsername() !== null); + // Check auth up front, rather than after the (potentially lengthy) // bundling process. var preflight = authedRpc({ site: site, preflight: true, - promptIfAuthFails: true + promptIfAuthFails: promptIfAuthFails }); if (preflight.errorMessage) { From 32ad71e50e5406fa3553e6185cb3deb77fde8e3d Mon Sep 17 00:00:00 2001 From: Geoff Schmidt Date: Wed, 19 Feb 2014 15:15:30 -0800 Subject: [PATCH 32/38] Improve email-in-use flow --- tools/auth.js | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/tools/auth.js b/tools/auth.js index cf3601cceb..2773829721 100644 --- a/tools/auth.js +++ b/tools/auth.js @@ -72,6 +72,8 @@ var loggedInAccountsConnection = function (token) { var outcome = fut.wait(); if (outcome.err) { + connection.close(); + if (outcome.err.error === 403) { // This is not an ideal value for the error code, but it means // "server rejected our access token". For example, it expired @@ -826,11 +828,23 @@ exports.registerOrLogIn = withAccountsConnection(function (connection) { } else if (result.alreadyExisted && result.sentRegistrationEmail) { process.stderr.write( "\n" + -"That email address is already in use. We need to confirm that it belongs\n" + -"to you. Luckily this will only take a moment.\n" + -"\n" + -"Check your mail! We've sent you a link. Click it, pick a password,\n" + -"and then come back here to deploy your app.\n"); +"You need to pick a password for your account so that you can log in.\n" + +"An email has been sent to you with the link.\n\n"); + + var animationFrame = 0; + var lastLinePrinted = ""; + var timer = setInterval(function () { + var spinner = ['-', '\\', '|', '/']; + lastLinePrinted = "Waiting for you to register on the web... " + + spinner[animationFrame]; + process.stderr.write(lastLinePrinted + "\r"); + animationFrame = (animationFrame + 1) % spinner.length; + }, 200); + var stopSpinner = function () { + process.stderr.write(new Array(lastLinePrinted.length + 1).join(' ') + + "\r"); + clearInterval(timer); + }; try { var waitForRegistrationResult = connection.call( @@ -838,17 +852,17 @@ exports.registerOrLogIn = withAccountsConnection(function (connection) { email ); } catch (e) { + stopSpinner(); if (! (e instanceof getLoadedPackages().meteor.Meteor.Error)) throw e; process.stderr.write( - "\nWhen you've picked your password, run 'meteor login' and then you'll\n" + - "be good to go.\n"); + "When you've picked your password, run 'meteor login' to log in.\n") return false; } - process.stderr.write("\nGreat! Nice to meet you, " + - waitForRegistrationResult.username + - "! Now log in with your new password.\n"); + stopSpinner(); + process.stderr.write("Username: " + + waitForRegistrationResult.username + "\n"); loginResult = doInteractivePasswordLogin({ username: waitForRegistrationResult.username, retry: true, From 0663aaa435b4c3446260b685bc0dddced97e2d0c Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 19 Feb 2014 17:04:22 -0800 Subject: [PATCH 33/38] Make Sandbox.writeSessionFile actually write the file --- tools/selftest.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/selftest.js b/tools/selftest.js index dc1bcf8445..8be27aef85 100644 --- a/tools/selftest.js +++ b/tools/selftest.js @@ -490,7 +490,8 @@ _.extend(Sandbox.prototype, { // restore authentication states. writeSessionFile: function (contents) { var self = this; - return fs.readFileSync(path.join(self.root, '.meteorsession'), 'utf8'); + return fs.writeFileSync(path.join(self.root, '.meteorsession'), + contents, 'utf8'); } }); From 829dfdac681fe242668f4f7e8af5d6152e9fa5c5 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 19 Feb 2014 19:21:30 -0800 Subject: [PATCH 34/38] Don't log out an invalid session immediately before deploy. When we deploy: * If we are logged in with a username, then we go straight to doing the actual deploy. * If we are logged in without a username, we check if we have a username yet. If we have an invalid credential at this point, we do NOT want `pollForRegistrationComplete` to wipe our session file, because we already passed the point at which we handle the case of the user being logged out. Instead, we just want to continue with the deploy and let the deploy server handle the expired credential. (This case, where a user's credential has been expired before they set a username, shouldn't happen too often in real life.) * If we deploy with an invalid credential, we get an "Expired credential" message. --- tools/auth.js | 14 ++++++++++---- tools/deploy.js | 4 +++- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/tools/auth.js b/tools/auth.js index cf3601cceb..3d6e91a41a 100644 --- a/tools/auth.js +++ b/tools/auth.js @@ -685,9 +685,13 @@ exports.logoutCommand = function (options) { // if not logged in, do nothing. If it is an account without a // username, contact the server and see if the user finished setting // it up, and if so grab and save the username. But contact the server -// only once per run of the program. +// only once per run of the program. Options: +// - noLogout: boolean. Set to true if you don't want this function to +// log out the session if wehave an invalid credential (for example, +// if a caller wants to do its own error handling for invalid +// credentials). Defaults to false. var alreadyPolledForRegistration = false; -exports.pollForRegistrationCompletion = function () { +exports.pollForRegistrationCompletion = function (options) { if (alreadyPolledForRegistration) return; alreadyPolledForRegistration = true; @@ -708,8 +712,10 @@ exports.pollForRegistrationCompletion = function () { // it. Note that, out of an abundance of caution, this also will // enqueue the credential for invalidation (on a future run, we // will try to explicitly revoke the credential ourselves). - logOutSession(session); - writeSessionData(data); + if (! options.noLogout) { + logOutSession(session); + writeSessionData(data); + } return; } diff --git a/tools/deploy.js b/tools/deploy.js index fc2117792d..37ffd4e39f 100644 --- a/tools/deploy.js +++ b/tools/deploy.js @@ -323,7 +323,9 @@ var bundleAndDeploy = function (options) { // they'll get an email prompt instead of a username prompt because // the command-line tool didn't have time to learn about their // username before the credential was expired. - auth.pollForRegistrationCompletion(); + auth.pollForRegistrationCompletion({ + noLogout: true + }); var promptIfAuthFails = (auth.loggedInUsername() !== null); // Check auth up front, rather than after the (potentially lengthy) From eee4c0c9e783ad1a618a01931ffe8570603db5c6 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 19 Feb 2014 19:36:54 -0800 Subject: [PATCH 35/38] Add test for deploying with expired credentials --- tools/tests/deploy-auth.js | 54 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/tools/tests/deploy-auth.js b/tools/tests/deploy-auth.js index 9fae26a238..9597cd4d22 100644 --- a/tools/tests/deploy-auth.js +++ b/tools/tests/deploy-auth.js @@ -3,9 +3,63 @@ var selftest = require('../selftest.js'); var testUtils = require('../test-utils.js'); var files = require('../files.js'); var Sandbox = selftest.Sandbox; +var httpHelpers = require('../http-helpers.js'); var commandTimeoutSecs = testUtils.accountsCommandTimeoutSecs; +selftest.define('deploy - expired credentials', ['net', 'slow'], function () { + var s = new Sandbox; + // Create an account and then expire the login token before setting a + // username. On the next deploy, we should get an email prompt + // followed by a registration email, not a username prompt. + var email = testUtils.randomUserEmail(); + var appName = testUtils.randomAppName(); + var token = testUtils.deployWithNewEmail(s, email, appName); + var sessionFile = s.readSessionFile(); + testUtils.logout(s); + s.writeSessionFile(sessionFile); + var run = s.run('deploy', appName); + run.waitSecs(commandTimeoutSecs); + run.matchErr('Expired credential'); + run.expectExit(1); + + // Complete registration so that we can clean up our app. + var username = testUtils.randomString(10); + testUtils.registerWithToken(token, username, + 'testtest', email); + testUtils.login(s, username, 'testtest'); + testUtils.cleanUpApp(s, appName); + testUtils.logout(s); + + // Create an account, set a username, expire the login token, and + // deploy again. We should get a username/password prompt. + email = testUtils.randomUserEmail(); + appName = testUtils.randomAppName(); + username = testUtils.randomString(10); + token = testUtils.deployWithNewEmail(s, email, appName); + testUtils.registerWithToken(token, username, + 'testtest', email); + run = s.run('whoami'); + run.waitSecs(commandTimeoutSecs); + run.read(username + '\n'); + run.expectExit(0); + + sessionFile = s.readSessionFile(); + testUtils.logout(s); + s.writeSessionFile(sessionFile); + + run = s.run('deploy', appName); + run.waitSecs(commandTimeoutSecs); + run.matchErr('Username:'); + run.write(username + '\n'); + run.matchErr('Password:'); + run.write('testtest' + '\n'); + run.waitSecs(90); + run.expectExit(0); + + testUtils.cleanUpApp(s, appName); +}); + selftest.define('deploy - bad arguments', [], function () { var s = new Sandbox; From 89b1a730025bf8494e851d0cf36a4c218149fcef Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 19 Feb 2014 23:43:18 -0800 Subject: [PATCH 36/38] Make options optional on pollForRegistrationCompletion --- tools/auth.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/auth.js b/tools/auth.js index 0da69fb0b9..3378fd7c5c 100644 --- a/tools/auth.js +++ b/tools/auth.js @@ -698,6 +698,8 @@ exports.pollForRegistrationCompletion = function (options) { return; alreadyPolledForRegistration = true; + options = options || {}; + var data = readSessionData(); var session = getSession(data, config.getAccountsDomain()); if (session.username || ! session.token) From cebffa7848add77893cc433acf4ab7c243f635db Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 19 Feb 2014 23:43:35 -0800 Subject: [PATCH 37/38] Use stderr for login prompt on deploy with expired token --- tools/deploy.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/deploy.js b/tools/deploy.js index 37ffd4e39f..84e82db94d 100644 --- a/tools/deploy.js +++ b/tools/deploy.js @@ -150,7 +150,10 @@ var authedRpc = function (options) { if (infoResult.statusCode === 401 && rpcOptions.promptIfAuthFails) { // Our authentication didn't validate, so prompt the user to log in // again, and resend the RPC if the login succeeds. - var username = utils.readLine({ prompt: "Username: " }); + var username = utils.readLine({ + prompt: "Username: ", + stream: process.stderr + }); var loginOptions = { username: username, suppressErrorMessage: true From ab3577d3d54f0b840248b43c956363ece5b60d08 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 19 Feb 2014 23:44:04 -0800 Subject: [PATCH 38/38] Update tests for changed accounts UX --- tools/tests/deploy-auth.js | 2 +- tools/tests/registration.js | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/tests/deploy-auth.js b/tools/tests/deploy-auth.js index 9597cd4d22..f1c193aa60 100644 --- a/tools/tests/deploy-auth.js +++ b/tools/tests/deploy-auth.js @@ -256,7 +256,7 @@ selftest.define('deploy - logged out', ['net', 'slow'], function () { run.matchErr('Email:'); run.write(email + '\n'); run.waitSecs(commandTimeoutSecs); - run.matchErr('already in use'); run.matchErr('pick a password'); + run.matchErr('An email has been sent to you with the link'); run.stop(); }); diff --git a/tools/tests/registration.js b/tools/tests/registration.js index 21d5da234b..ee2eac71ab 100644 --- a/tools/tests/registration.js +++ b/tools/tests/registration.js @@ -236,8 +236,8 @@ selftest.define( run.matchErr('Email:'); run.write(email + '\n'); run.waitSecs(testUtils.accountsCommandTimeoutSecs); - run.matchErr('already in use'); - run.matchErr('come back here to deploy your app'); + run.matchErr('pick a password'); + run.matchErr('Waiting for you to register on the web...'); var registrationEmail = waitForEmail( email, @@ -254,8 +254,8 @@ selftest.define( testUtils.registerWithToken(token[1], username, 'testtest', email); run.waitSecs(testUtils.accountsCommandTimeoutSecs); - run.matchErr('log in with your new password'); - run.matchErr('Password:'); + run.matchErr('Username: ' + username + '\n'); + run.matchErr('Password: '); run.write('testtest\n'); run.waitSecs(90); run.match('Now serving at');