Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

NodeDomain #6193

Merged
merged 5 commits into from
Dec 18, 2013
Merged

NodeDomain #6193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8349821
Add NodeDomain wrapper and refactor StaticServer to use it.
Dec 5, 2013
e320244
Fix a race condition that arose with concurrent requests for the same…
Dec 6, 2013
93b1187
Respond to review comments
Dec 6, 2013
5adeb59
Fix `_requests` JSDoc
iwehrman Dec 18, 2013
434b0db
Use one return instead of many.
iwehrman Dec 18, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/brackets.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ define(function (require, exports, module) {
Resizer = require("utils/Resizer"),
LiveDevelopmentMain = require("LiveDevelopment/main"),
NodeConnection = require("utils/NodeConnection"),
NodeDomain = require("utils/NodeDomain"),
ExtensionUtils = require("utils/ExtensionUtils"),
DragAndDrop = require("utils/DragAndDrop"),
ColorUtils = require("utils/ColorUtils"),
Expand Down
70 changes: 20 additions & 50 deletions src/extensions/default/StaticServer/StaticServer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ define(function (require, exports, module) {
* baseUrl - Optional base URL (populated by the current project)
* pathResolver - Function to covert absolute native paths to project relative paths
* root - Native path to the project root (and base URL)
* nodeConnection - An active NodeConnection
* nodeDomain - An initialized NodeDomain
*/
function StaticServer(config) {
this._nodeConnection = config.nodeConnection;
this._nodeDomain = config.nodeDomain;
this._onRequestFilter = this._onRequestFilter.bind(this);

BaseServer.call(this, config);
Expand All @@ -61,7 +61,7 @@ define(function (require, exports, module) {
* @return {boolean} true for yes, otherwise false.
*/
StaticServer.prototype.canServe = function (localPath) {
if (!this._nodeConnection.connected()) {
if (!this._nodeDomain.ready()) {
return false;
}

Expand All @@ -87,17 +87,9 @@ define(function (require, exports, module) {
* @return {jQuery.Promise} Resolved by the StaticServer domain when the message is acknowledged.
*/
StaticServer.prototype._updateRequestFilterPaths = function () {
if (!this._nodeConnection.connected()) {
return;
}

var paths = [];

Object.keys(this._liveDocuments).forEach(function (path) {
paths.push(path);
});
var paths = Object.keys(this._liveDocuments);

return this._nodeConnection.domains.staticServer.setRequestFilterPaths(this._root, paths);
return this._nodeDomain.exec("setRequestFilterPaths", this._root, paths);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually like the old way better where the command is a method on the domain. Is there a reason we can't do this with NodeDomain?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that I wanted to make a wrapper that doesn't require you to wait on a promise before you start firing off commands. But we have to wait until the domain is loaded before we know what the commands are. If we do it this way, we can just wait for the connection to come up and the domains to be loaded before rejecting the exec promise if the command isn't there. In a previous version, I actually went ahead and bound all the loaded commands to the domain object itself at domain load time, but then if you tried to execute a command before the domain had been loaded you'd get a TypeError. I also tried to find a way to override arbitrary property lookups for an object, but there doesn't seem to be a way to do this in ES5. I also would prefer to not have to pass strings around, but I personally think it's mostly just a style issue. (It's not like a static analysis could ever correctly predict the the properties on these domains anyway for the sake of, say, code hinting.)

I only added the promise and ready methods for backwards compatibility with the basic structure of StaticServer. I don't consider them to really be core features of NodeDomain.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good points. Thanks for the clarification. I think you've made the right trade offs.

};

/**
Expand All @@ -109,37 +101,14 @@ define(function (require, exports, module) {
* the server is ready/failed.
*/
StaticServer.prototype.readyToServe = function () {
var readyToServeDeferred = $.Deferred(),
self = this;

if (this._nodeConnection.connected()) {
this._nodeConnection.domains.staticServer.getServer(self._root).done(function (address) {
var self = this;
return this._nodeDomain.exec("getServer", self._root)
.done(function (address) {
self._baseUrl = "http://" + address.address + ":" + address.port + "/";
readyToServeDeferred.resolve();
}).fail(function () {
})
.fail(function () {
self._baseUrl = "";
readyToServeDeferred.reject();
});
} else {
// nodeConnection has been connected once (because the deferred
// resolved, but is not currently connected).
//
// If we are in this case, then the node process has crashed
// and is in the process of restarting. Once that happens, the
// node connection will automatically reconnect and reload the
// domain. Unfortunately, we don't have any promise to wait on
// to know when that happens. The best we can do is reject this
// readyToServe so that the user gets an error message to try
// again later.
//
// The user will get the error immediately in this state, and
// the new node process should start up in a matter of seconds
// (assuming there isn't a more widespread error). So, asking
// them to retry in a second is reasonable.
readyToServeDeferred.reject();
}

return readyToServeDeferred.promise();
};

/**
Expand Down Expand Up @@ -181,26 +150,27 @@ define(function (require, exports, module) {
* Send HTTP response data back to the StaticServerSomain
*/
StaticServer.prototype._send = function (location, response) {
if (this._nodeConnection.connected()) {
this._nodeConnection.domains.staticServer.writeFilteredResponse(location.root, location.pathname, response);
}
this._nodeDomain.exec("writeFilteredResponse", location.root, location.pathname, response);
};

/**
* @private
* Event handler for StaticServerDomain requestFilter event
* @param {jQuery.Event} event
* @param {{hostname: string, pathname: string, port: number, root: string}} request
* @param {{hostname: string, pathname: string, port: number, root: string, id: number}} request
*/
StaticServer.prototype._onRequestFilter = function (event, request) {
var key = request.location.pathname,
liveDocument = this._liveDocuments[key],
response = null;

response;
// send instrumented response or null to fallback to static file
if (liveDocument && liveDocument.getResponseData) {
response = liveDocument.getResponseData();
} else {
response = {};
}
response.id = request.id;

this._send(request.location, response);
};
Expand All @@ -209,15 +179,15 @@ define(function (require, exports, module) {
* See BaseServer#start. Starts listenting to StaticServerDomain events.
*/
StaticServer.prototype.start = function () {
$(this._nodeConnection).on("staticServer.requestFilter", this._onRequestFilter);
$(this._nodeDomain).on("requestFilter", this._onRequestFilter);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much better. The domainevent notation is a bit confusing when you're used to namespaces.

};

/**
* See BaseServer#stop. Remove event handlers from StaticServerDomain.
*/
StaticServer.prototype.stop = function () {
$(this._nodeConnection).off("staticServer.requestFilter", this._onRequestFilter);
$(this._nodeDomain).off("requestFilter", this._onRequestFilter);
};

exports.StaticServer = StaticServer;
module.exports = StaticServer;
});
76 changes: 12 additions & 64 deletions src/extensions/default/StaticServer/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,31 +34,21 @@ define(function (require, exports, module) {
FileUtils = brackets.getModule("file/FileUtils"),
LiveDevServerManager = brackets.getModule("LiveDevelopment/LiveDevServerManager"),
BaseServer = brackets.getModule("LiveDevelopment/Servers/BaseServer").BaseServer,
NodeConnection = brackets.getModule("utils/NodeConnection"),
NodeDomain = brackets.getModule("utils/NodeDomain"),
ProjectManager = brackets.getModule("project/ProjectManager"),
StaticServer = require("StaticServer").StaticServer;

/**
* @const
* Amount of time to wait before automatically rejecting the connection
* deferred. If we hit this timeout, we'll never have a node connection
* for the static server in this run of Brackets.
*/
var NODE_CONNECTION_TIMEOUT = 5000; // 5 seconds
StaticServer = require("StaticServer");

/**
* @private
* @type{jQuery.Deferred.<NodeConnection>}
* A deferred which is resolved with a NodeConnection or rejected if
* we are unable to connect to Node.
* @type {string} fullPath of the StaticServerDomain implementation
*/
var _nodeConnectionDeferred = new $.Deferred();
var _domainPath = ExtensionUtils.getModulePath(module, "node/StaticServerDomain");

/**
* @private
* @type {NodeConnection}
* @type {NodeDomain}
*/
var _nodeConnection = new NodeConnection();
var _nodeDomain = new NodeDomain("staticServer", _domainPath);

/**
* @private
Expand All @@ -67,61 +57,19 @@ define(function (require, exports, module) {
*/
function _createStaticServer() {
var config = {
nodeConnection : _nodeConnection,
nodeDomain : _nodeDomain,
pathResolver : ProjectManager.makeProjectRelativeIfPossible,
root : ProjectManager.getProjectRoot().fullPath
};

return new StaticServer(config);
}

/**
* Allows access to the deferred that manages the node connection. This
* is *only* for unit tests. Messing with this not in testing will
* potentially break everything.
*
* @private
* @return {jQuery.Deferred} The deferred that manages the node connection
*/
function _getNodeConnectionDeferred() {
return _nodeConnectionDeferred;
}

function initExtension() {
// Start up the node connection, which is held in the
// _nodeConnectionDeferred module variable. (Use
// _nodeConnectionDeferred.done() to access it.
var connectionTimeout = setTimeout(function () {
console.error("[StaticServer] Timed out while trying to connect to node");
_nodeConnectionDeferred.reject();
}, NODE_CONNECTION_TIMEOUT);

_nodeConnection.connect(true).then(function () {
_nodeConnection.loadDomains(
[ExtensionUtils.getModulePath(module, "node/StaticServerDomain")],
true
).then(
function () {
clearTimeout(connectionTimeout);

// Register as a Live Development server provider
LiveDevServerManager.registerServer({ create: _createStaticServer }, 5);

_nodeConnectionDeferred.resolveWith(null, [_nodeConnection]);
},
function () { // Failed to connect
console.error("[StaticServer] Failed to connect to node", arguments);
_nodeConnectionDeferred.reject();
}
);
});

return _nodeConnectionDeferred.promise();
}

exports.initExtension = initExtension;

AppInit.appReady(function () {
LiveDevServerManager.registerServer({ create: _createStaticServer }, 5);
});

// For unit tests only
exports._getStaticServerProvider = _createStaticServer;
exports._getNodeConnectionDeferred = _getNodeConnectionDeferred;
exports._nodeDomain = _nodeDomain;
});
27 changes: 18 additions & 9 deletions src/extensions/default/StaticServer/node/StaticServerDomain.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@
var _domainManager;

var FILTER_REQUEST_TIMEOUT = 5000;

/**
* @private
* @type {number}
* Used to assign unique identifiers to each filter request
*/
var _filterRequestCounter = 0;

/**
* @private
Expand Down Expand Up @@ -68,8 +75,8 @@

/**
* @private
* @type {Object.<string, {Object.<string, http.ServerResponse>}}
* A map from root paths to its request/response mapping.
* @type {Object.<string, {Object.<number, http.ServerResponse>}}
* A map from a request identifier to its request/response mapping.
*/
var _requests = {};

Expand Down Expand Up @@ -139,6 +146,7 @@
function rewrite(req, res, next) {
var location = {pathname: parse(req).pathname},
hasListener = _rewritePaths[pathKey] && _rewritePaths[pathKey][location.pathname],
requestId = _filterRequestCounter++,
timeoutId;

// ignore most HTTP methods and files that we're not watching
Expand All @@ -152,7 +160,7 @@
function resume(doNext) {
// delete the callback after it's used or we hit the timeout.
// if this path is requested again, a new callback is generated.
delete _requests[pathKey][location.pathname];
delete _requests[pathKey][requestId];

// pass request to next middleware
if (doNext) {
Expand All @@ -163,12 +171,12 @@
}

// map request pathname to response callback
_requests[pathKey][location.pathname] = function (resData) {
_requests[pathKey][requestId] = function (resData) {
// clear timeout immediately when this callback is called
clearTimeout(timeoutId);

// response data is optional
if (resData) {
if (resData.body) {
// HTTP headers
var type = mime.lookup(location.pathname),
charset = mime.charsets.lookup(type);
Expand All @@ -194,7 +202,8 @@

var request = {
headers: req.headers,
location: location
location: location,
id: requestId
};

// dispatch request event
Expand Down Expand Up @@ -308,11 +317,11 @@
*
* @param {string} path The absolute path of the server
* @param {string} root The relative path of the file beginning with a forward slash "/"
* @param {Object} resData Response data to use
* @param {!Object} resData Response data to use
*/
function _cmdWriteFilteredResponse(root, path, resData) {
var pathKey = getPathKey(root),
callback = _requests[pathKey][path];
callback = _requests[pathKey][resData.id];

if (callback) {
callback(resData);
Expand Down Expand Up @@ -440,7 +449,7 @@
"requestFilter",
[{
name: "location",
type: "{hostname: string, pathname: string, port: number, root: string}",
type: "{hostname: string, pathname: string, port: number, root: string: id: number}",
description: "request path"
}]
);
Expand Down
Loading