From d4a116525bf998d76b5cde859d2b665ad17b2777 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Tue, 23 Oct 2012 17:02:31 +0200 Subject: [PATCH 1/4] some documentation and error message updates --- arangod/Documentation/user-manual.dox | 10 +- js/server/modules/org/arangodb/actions.js | 140 +++++++++++++--------- 2 files changed, 87 insertions(+), 63 deletions(-) diff --git a/arangod/Documentation/user-manual.dox b/arangod/Documentation/user-manual.dox index ada75ad697..7e78efe559 100644 --- a/arangod/Documentation/user-manual.dox +++ b/arangod/Documentation/user-manual.dox @@ -398,7 +398,7 @@ //////////////////////////////////////////////////////////////// /// /// In some ways the communication layer of the ArangoDB server behaves like a -/// Web server. Unlike a Web server, it normally responses to HTTP requests by +/// Web server. Unlike a Web server, it normally responds to HTTP requests by /// delivering JSON objects. Remember, documents in the database are just JSON /// objects. So, most of the time the HTTP response will contain a JSON document /// from the database as body. You can extract the documents stored in the @@ -436,7 +436,7 @@ /// The following sections will explain actions within ArangoDB and show how to /// define them. The examples start with delivering static HTML pages - even if /// this is not the primary use-case for actions. The later sections will then -/// show you, how to code some pieces of your business logic and return JSON +/// show you how to code some pieces of your business logic and return JSON /// objects. /// /// The interface is loosely modelled after the JavaScript classes for HTTP @@ -451,14 +451,14 @@ ////////////////////////////////////////////////////////////// /// /// The client API or browser sends a HTTP request to the ArangoDB server and -/// the server returns a HTTP response to the client. A HTTP requests consists +/// the server returns a HTTP response to the client. A HTTP requests consist /// of a method, normally @LIT{GET} or @LIT{POST} when using a browser, and a /// request path like @LIT{/hello/world}. For a real Web server there are a zillion /// of other thing to consider, we will ignore this for the moment. The HTTP /// response contains a content type, describing how to interpret the returned /// data, and the data itself. /// -/// In the following example, we want to define action in ArangoDB, so that the +/// In the following example, we want to define an action in ArangoDB, so that the /// server returns the HTML document /// /// @code @@ -576,7 +576,7 @@ /// @subsection UserManualActionsMatchesConstraint Constraint Match /// /// A constraint match is similar to a parameterized match, but the parameters -/// carries a constraint. +/// can carry constraints. /// /// If the definition is /// diff --git a/js/server/modules/org/arangodb/actions.js b/js/server/modules/org/arangodb/actions.js index 8f46d1cbbe..a3b87bef51 100644 --- a/js/server/modules/org/arangodb/actions.js +++ b/js/server/modules/org/arangodb/actions.js @@ -65,6 +65,38 @@ var RoutingCache; /// @{ //////////////////////////////////////////////////////////////////////////////// +//////////////////////////////////////////////////////////////////////////////// +/// @brief function that's returned for non-implemented actions +//////////////////////////////////////////////////////////////////////////////// + +function notImplementedFunction (triggerRoute, message) { + message += "\nThis error is triggered by the following route " + JSON.stringify(triggerRoute); + + console.error(message); + + return function (req, res, options, next) { + res.responseCode = exports.HTTP_NOT_IMPLEMENTED; + res.contentType = "text/plain"; + res.body = message; + }; +} + +//////////////////////////////////////////////////////////////////////////////// +/// @brief function that's returned for actions that produce an error +//////////////////////////////////////////////////////////////////////////////// + +function errorFunction (triggerRoute, message) { + message += "\nThis error is triggered by the following route " + JSON.stringify(triggerRoute); + + console.error(message); + + return function (req, res, options, next) { + res.responseCode = exports.HTTP_SERVER_ERROR; + res.contentType = "text/plain"; + res.body = message; + }; +} + //////////////////////////////////////////////////////////////////////////////// /// @brief splits an URL into parts //////////////////////////////////////////////////////////////////////////////// @@ -181,14 +213,22 @@ function lookupCallbackStatic (content) { /// @brief looks up a callback for an action //////////////////////////////////////////////////////////////////////////////// -function lookupCallbackAction (action) { +function lookupCallbackAction (route, action) { var path; var name; var func; var module; + var httpMethods = { + 'get': exports.GET, + 'head': exports.HEAD, + 'put': exports.PUT, + 'post': exports.POST, + 'delete': exports.DELETE, + 'patch': exports.PATCH + }; if (typeof action === 'string') { - return lookupCallbackAction({ prefixController: action }); + return lookupCallbackAction(route, { prefixController: action }); } if (action.hasOwnProperty('do')) { @@ -203,17 +243,15 @@ function lookupCallbackAction (action) { func = module[name]; } else { - console.error("cannot find action named '%s' in module '%s'", name, path.join("/")); + func = notImplementedFunction(route, "Could not find action named '" + name + "' in module '" + path.join("/") + "'"); } } catch (err) { - console.error("cannot find action named '%s' in module '%s': %s", - name, path.join("/"), String(err)); - return null; + func = errorFunction(route, "An error occurred while loading action named '" + name + "' in module '" + path.join("/") + "': " + String(err)); } if (func === null || typeof func !== 'function') { - return null; + func = errorFunction(route, "Invalid definition for the action named '" + name + "' in module '" + path.join("/") + "'"); } return { @@ -229,32 +267,25 @@ function lookupCallbackAction (action) { return { controller: function (req, res, options, next) { - if (req.requestType === exports.GET && module.hasOwnProperty('get')) { - return module['get'](req, res, options, next); - } + // enum all HTTP methods + for (var m in httpMethods) { + if (! httpMethods.hasOwnProperty(m)) { + continue; + } - if (req.requestType === exports.HEAD && module.hasOwnProperty('head')) { - return module['head'](req, res, options, next); - } + if (req.requestType == httpMethods[m] && module.hasOwnProperty(m)) { + func = module[m] || + errorFunction(route, "Invalid definition for " + m + " action in action controller module '" + action.controller + "'"); - if (req.requestType === exports.PUT && module.hasOwnProperty('put')) { - return module['put'](req, res, options, next); - } - - if (req.requestType === exports.POST && module.hasOwnProperty('post')) { - return module['post'](req, res, options, next); - } - - if (req.requestType === exports.DELETE && module.hasOwnProperty('delete')) { - return module['delete'](req, res, options, next); - } - - if (req.requestType === exports.PATCH && module.hasOwnProperty('patch')) { - return module['patch'](req, res, options, next); + return func(req, res, options, next); + } } if (module.hasOwnProperty('do')) { - return module['do'](req, res, options, next); + func = module['do'] || + errorFunction(route, "Invalid definition for do action in action controller module '" + action.controller + "'"); + + return func(req, res, options, next); } next(); @@ -264,9 +295,7 @@ function lookupCallbackAction (action) { }; } catch (err) { - console.error("cannot load action controller module '%s': %s", - action.controller, String(err)); - return null; + return errorFunction(route, "cannot load/execute action controller module '" + action.controller + ": " + String(err))(req, res, options, next); } } @@ -286,37 +315,32 @@ function lookupCallbackAction (action) { } } catch (err) { - console.error("cannot load prefix controller: %s", String(err)); - next(); - return; + return errorFunction(route, "cannot load prefix controller: " + String(err))(req, res, options, next); } - if (req.requestType === exports.GET && module.hasOwnProperty('get')) { - return module['get'](req, res, options, next); - } + try { + // enum all HTTP methods + for (var m in httpMethods) { + if (! httpMethods.hasOwnProperty(m)) { + continue; + } - if (req.requestType === exports.HEAD && module.hasOwnProperty('head')) { - return module['head'](req, res, options, next); - } + if (req.requestType == httpMethods[m] && module.hasOwnProperty(m)) { + func = module[m] || + errorFunction(route, "Invalid definition for " + m + " action in prefix controller '" + action.prefixController + "'"); - if (req.requestType === exports.PUT && module.hasOwnProperty('put')) { - return module['put'](req, res, options, next); + return func(req, res, options, next); + } + } + + if (module.hasOwnProperty('do')) { + func = module['do'] || + errorFunction(route, "Invalid definition for do action in prefix controller '" + action.prefixController + "'"); + return func(req, res, options, next); + } } - - if (req.requestType === exports.POST && module.hasOwnProperty('post')) { - return module['post'](req, res, options, next); - } - - if (req.requestType === exports.DELETE && module.hasOwnProperty('delete')) { - return module['delete'](req, res, options, next); - } - - if (req.requestType === exports.PATCH && module.hasOwnProperty('patch')) { - return module['patch'](req, res, options, next); - } - - if (module.hasOwnProperty('do')) { - return module['do'](req, res, options, next); + catch (err) { + return errorFunction(route, "Cannot load/execute prefix controller '" + action.prefixController + "': " + String(err))(req, res, options, next); } next(); @@ -340,7 +364,7 @@ function lookupCallback (route) { result = lookupCallbackStatic(route.content); } else if (route.hasOwnProperty('action')) { - result = lookupCallbackAction(route.action); + result = lookupCallbackAction(route, route.action); } return result; From c3f84cb2a680daba1470e3379608d27e16d6e074 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Tue, 23 Oct 2012 18:50:34 +0200 Subject: [PATCH 2/4] initialise variable --- js/server/modules/org/arangodb/actions.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js/server/modules/org/arangodb/actions.js b/js/server/modules/org/arangodb/actions.js index a3b87bef51..dc5bd3db56 100644 --- a/js/server/modules/org/arangodb/actions.js +++ b/js/server/modules/org/arangodb/actions.js @@ -843,7 +843,7 @@ function reloadRouting () { // ............................................................................. RoutingCache = {}; - + RoutingCache.flat = {}; RoutingCache.routes = {}; RoutingCache.middleware = {}; @@ -1009,7 +1009,7 @@ function firstRouting (type, parts) { url = "/" + parts.join("/"); } - if (! RoutingCache.flat.hasOwnProperty(type)) { + if (! RoutingCache.flat || ! RoutingCache.flat.hasOwnProperty(type)) { return { parts: parts, position: -1, From 95c9c8560827d1815603a491ee59132db3dc8780 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Wed, 24 Oct 2012 09:38:46 +0200 Subject: [PATCH 3/4] documentation update --- arangod/Documentation/user-manual.dox | 37 ++++++------------- .../org/arangodb/actions/echoController.js | 2 +- 2 files changed, 12 insertions(+), 27 deletions(-) diff --git a/arangod/Documentation/user-manual.dox b/arangod/Documentation/user-manual.dox index 7e78efe559..100a45a72f 100644 --- a/arangod/Documentation/user-manual.dox +++ b/arangod/Documentation/user-manual.dox @@ -451,7 +451,7 @@ ////////////////////////////////////////////////////////////// /// /// The client API or browser sends a HTTP request to the ArangoDB server and -/// the server returns a HTTP response to the client. A HTTP requests consist +/// the server returns a HTTP response to the client. A HTTP requests consists /// of a method, normally @LIT{GET} or @LIT{POST} when using a browser, and a /// request path like @LIT{/hello/world}. For a real Web server there are a zillion /// of other thing to consider, we will ignore this for the moment. The HTTP @@ -753,7 +753,7 @@ /// @code /// arangosh> db._routing.save({ /// ........> url: "/hello/echo", -/// ........> action: "org/arangodb/actions/echoRequest" }); +/// ........> action: { controller: "org/arangodb/actions", do: "echoRequest" } }); /// @endcode /// /// Reload the routing and check @@ -874,7 +874,7 @@ /// @code /// arangosh> db._routing.save({ /// ........> url: "/echo", -/// ........> action: "org/arangodb/actions/echoRequest" }); +/// ........> action: { controller: "org/arangodb/actions", do: "echoRequest" } }); /// @endcode /// /// Reload the routing and check @@ -907,29 +907,14 @@ /// } /// @endcode /// -/// Note that -/// -/// @code -/// arangosh> db._routing.save({ -/// ........> url: "/echo", -/// ........> action: "org/arangodb/actions/echoRequest" }); -/// @endcode -/// -/// is a short-cut for -/// -/// @code -/// arangosh> db._routing.save({ -/// ........> url: "/echo", -/// ........> action: { do: "org/arangodb/actions/echoRequest" } }); -/// @endcode -/// -/// The latter form allows you to pass options to the called function: +/// You may also pass options to the called function: /// /// @code /// arangosh> db._routing.save({ /// ........> url: "/echo", /// ........> action: { -/// ........> do: "org/arangodb/actions/echoRequest", +/// ........> controller: "org/arangodb/actions", +/// ........> do: "echoRequest", /// ........> options: { "Hallo": "World" } } }); /// @endcode /// @@ -938,11 +923,10 @@ /// @code /// { /// "request": { -/// "prefix": "/hello/echo-long", /// ... /// }, /// "options": { -/// "option": "my option1" +/// "Hallo": "world" /// } /// } /// @endcode @@ -960,7 +944,8 @@ /// arangosh> db._routing.save({ /// ........> url: "/", /// ........> action: { -/// ........> do: "org/arangodb/actions/redirectRequest", +/// ........> controller: "org/arangodb/actions", +/// ........> do: "redirectRequest", /// ........> options: { /// ........> permanently: true, /// ........> destination: "http://somewhere.else/" } } }); @@ -1015,14 +1000,14 @@ /// }; /// @endcode /// -/// This functions is available as @LIT{org/arangodb/actions/logRequest}. +/// This function is available as @LIT{org/arangodb/actions/logRequest}. /// You need to tell ArangoDB that it is should use a prefix match and /// that the shortest match should win in this case: /// /// @code /// arangosh> db._routing.save({ /// ........> middleware: [ -/// ........> { url: { match: "/*" }, action: "org/arangodb/actions/logRequest" } +/// ........> { url: { match: "/*" }, action: { controller: "org/arangodb/actions", do: "logRequest" } } /// ........> ] /// ........> }); /// @endcode diff --git a/js/server/modules/org/arangodb/actions/echoController.js b/js/server/modules/org/arangodb/actions/echoController.js index 5a59d34b86..5628f80ea5 100644 --- a/js/server/modules/org/arangodb/actions/echoController.js +++ b/js/server/modules/org/arangodb/actions/echoController.js @@ -36,7 +36,7 @@ exports.head = function (req, res, next, options) { exports.do = function (req, res, next, options) { res.responseCode = actions.HTTP_OK; res.contentType = "application/json; charset=utf-8"; - res.body = JSON.stringify(req); + res.body = JSON.stringify( { "request" : req, "options" : options }); } // ----------------------------------------------------------------------------- From b95831c906a81387d73abc552d5bd7cda7f2aae4 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Wed, 24 Oct 2012 10:10:41 +0200 Subject: [PATCH 4/4] another bugfix --- js/server/modules/org/arangodb/actions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/server/modules/org/arangodb/actions.js b/js/server/modules/org/arangodb/actions.js index dc5bd3db56..5f3b265e63 100644 --- a/js/server/modules/org/arangodb/actions.js +++ b/js/server/modules/org/arangodb/actions.js @@ -295,7 +295,7 @@ function lookupCallbackAction (route, action) { }; } catch (err) { - return errorFunction(route, "cannot load/execute action controller module '" + action.controller + ": " + String(err))(req, res, options, next); + return errorFunction(route, "cannot load/execute action controller module '" + action.controller + ": " + String(err)); } }