Skip to content
Browse files

Fix handling of circular imports in CommonJS modules.

Thanks to Caolan McMahon for the patch.

Closes COUCHDB-1075



git-svn-id: https://svn.apache.org/repos/asf/couchdb/trunk@1102006 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information...
1 parent fbb9b3c commit d5cb8c068e82fb181148c004c2c3ea15aeebeed0 @davisp davisp committed
Showing with 132 additions and 15 deletions.
  1. +2 −0 THANKS
  2. +23 −14 share/server/util.js
  3. +107 −1 share/www/script/test/design_docs.js
View
2 THANKS
@@ -82,5 +82,7 @@ suggesting improvements or submitting changes. Some of these people are:
* Dipesh Patel <dipthegeezer.opensource@googlemail.com>
* Sam Bisbee <sam@sbisbee.com>
* Nathan Vander Wilt <natevw@yahoo.com>
+ * Caolan McMahon <caolan.mcmahon@googlemail.com>
+
For a list of authors see the `AUTHORS` file.
View
37 share/server/util.js
@@ -31,16 +31,16 @@ var resolveModule = function(names, mod, root) {
}
return resolveModule(names, {
id : mod.id.slice(0, mod.id.lastIndexOf('/')),
- parent : mod.parent.parent.parent,
- current : mod.parent.parent.current
+ parent : mod.parent.parent,
+ current : mod.parent.current
});
} else if (n == '.') {
if (!mod.parent) {
throw ["error", "invalid_require_path", 'Object has no parent '+JSON.stringify(mod.current)];
}
return resolveModule(names, {
- parent : mod.parent.parent,
- current : mod.parent.current,
+ parent : mod.parent,
+ current : mod.current,
id : mod.id
});
} else if (root) {
@@ -66,19 +66,28 @@ var Couch = {
try {
if (sandbox) {
if (ddoc) {
+ if (!ddoc._module_cache) {
+ ddoc._module_cache = {};
+ }
var require = function(name, module) {
module = module || {};
- var newModule = resolveModule(name.split('/'), module, ddoc);
- var s = "function (module, exports, require) { " + newModule.current + " }";
- try {
- var func = sandbox ? evalcx(s, sandbox) : eval(s);
- func.apply(sandbox, [newModule, newModule.exports, function(name) {
- return require(name, newModule);
- }]);
- } catch(e) {
- throw ["error","compilation_error","Module require('"+name+"') raised error "+e.toSource()];
+ var newModule = resolveModule(name.split('/'), module.parent, ddoc);
+ if (!ddoc._module_cache.hasOwnProperty(newModule.id)) {
+ // create empty exports object before executing the module,
+ // stops circular requires from filling the stack
+ ddoc._module_cache[newModule.id] = {};
+ var s = "function (module, exports, require) { " + newModule.current + " }";
+ try {
+ var func = sandbox ? evalcx(s, sandbox) : eval(s);
+ func.apply(sandbox, [newModule, newModule.exports, function(name) {
+ return require(name, newModule);
+ }]);
+ } catch(e) {
+ throw ["error","compilation_error","Module require('"+name+"') raised error "+e.toSource()];
+ }
+ ddoc._module_cache[newModule.id] = newModule.exports;
}
- return newModule.exports;
+ return ddoc._module_cache[newModule.id];
};
sandbox.require = require;
}
View
108 share/www/script/test/design_docs.js
@@ -49,7 +49,48 @@ couchTests.design_docs = function(debug) {
whynot : "exports.test = require('../stringzone'); " +
"exports.foo = require('whatever/stringzone');",
upper : "exports.testing = require('./whynot').test.string.toUpperCase()+" +
- "module.id+require('./whynot').foo.string"
+ "module.id+require('./whynot').foo.string",
+ circular_one: "require('./circular_two'); exports.name = 'One';",
+ circular_two: "require('./circular_one'); exports.name = 'Two';"
+ },
+ // paths relative to parent
+ idtest1: {
+ a: {
+ b: {d: "module.exports = require('../c/e').id;"},
+ c: {e: "exports.id = module.id;"}
+ }
+ },
+ // multiple paths relative to parent
+ idtest2: {
+ a: {
+ b: {d: "module.exports = require('../../a/c/e').id;"},
+ c: {e: "exports.id = module.id;"}
+ }
+ },
+ // paths relative to module
+ idtest3: {
+ a: {
+ b: "module.exports = require('./c/d').id;",
+ c: {
+ d: "module.exports = require('./e');",
+ e: "exports.id = module.id;"
+ }
+ }
+ },
+ // paths relative to module and parent
+ idtest4: {
+ a: {
+ b: "module.exports = require('../a/./c/d').id;",
+ c: {
+ d: "module.exports = require('./e');",
+ e: "exports.id = module.id;"
+ }
+ }
+ },
+ // paths relative to root
+ idtest5: {
+ a: "module.exports = require('whatever/idtest5/b').id;",
+ b: "exports.id = module.id;"
}
},
views: {
@@ -134,6 +175,25 @@ couchTests.design_docs = function(debug) {
(function() {
var lib = require('whatever/commonjs/upper');
return JSON.stringify(this);
+ }).toString(),
+ circular_require:
+ (function() {
+ return require('whatever/commonjs/circular_one').name;
+ }).toString(),
+ idtest1: (function() {
+ return require('whatever/idtest1/a/b/d');
+ }).toString(),
+ idtest2: (function() {
+ return require('whatever/idtest2/a/b/d');
+ }).toString(),
+ idtest3: (function() {
+ return require('whatever/idtest3/a/b');
+ }).toString(),
+ idtest4: (function() {
+ return require('whatever/idtest4/a/b');
+ }).toString(),
+ idtest5: (function() {
+ return require('whatever/idtest5/a');
}).toString()
}
}; // designDoc
@@ -174,6 +234,52 @@ couchTests.design_docs = function(debug) {
T(xhr.status == 200);
TEquals("javascript", JSON.parse(xhr.responseText).language);
+ // test circular commonjs dependencies
+ xhr = CouchDB.request(
+ "GET",
+ "/test_suite_db/_design/test/_show/circular_require"
+ );
+ TEquals(200, xhr.status);
+ TEquals("One", xhr.responseText);
+
+ // Test that changes to the design doc properly invalidate cached modules:
+
+ // update the designDoc and replace
+ designDoc.whatever.commonjs.circular_one = "exports.name = 'Updated';"
+ T(db.save(designDoc).ok);
+
+ // request circular_require show function again and check the response has
+ // changed
+ xhr = CouchDB.request(
+ "GET",
+ "/test_suite_db/_design/test/_show/circular_require"
+ );
+ TEquals(200, xhr.status);
+ TEquals("Updated", xhr.responseText);
+
+
+ // test module id values are as expected:
+ xhr = CouchDB.request("GET", "/test_suite_db/_design/test/_show/idtest1");
+ TEquals(200, xhr.status);
+ TEquals("whatever/idtest1/a/c/e", xhr.responseText);
+
+ xhr = CouchDB.request("GET", "/test_suite_db/_design/test/_show/idtest2");
+ TEquals(200, xhr.status);
+ TEquals("whatever/idtest2/a/c/e", xhr.responseText);
+
+ xhr = CouchDB.request("GET", "/test_suite_db/_design/test/_show/idtest3");
+ TEquals(200, xhr.status);
+ TEquals("whatever/idtest3/a/c/e", xhr.responseText);
+
+ xhr = CouchDB.request("GET", "/test_suite_db/_design/test/_show/idtest4");
+ TEquals(200, xhr.status);
+ TEquals("whatever/idtest4/a/c/e", xhr.responseText);
+
+ xhr = CouchDB.request("GET", "/test_suite_db/_design/test/_show/idtest5");
+ TEquals(200, xhr.status);
+ TEquals("whatever/idtest5/b", xhr.responseText);
+
+
var prev_view_sig = db.designInfo("_design/test").view_index.signature;
var prev_view_size = db.designInfo("_design/test").view_index.disk_size;

0 comments on commit d5cb8c0

Please sign in to comment.
Something went wrong with that request. Please try again.