Skip to content

Commit

Permalink
TEST(db): all integration tests pass STRICT MODE
Browse files Browse the repository at this point in the history
This commit ensures that all integration tests pass during SQL strict
mode.  In particular, the following changes have been implemented:

 1. Travis calls SET GLOBAL sql_mode = 'STRICT_ALL_TABLES' to ensure
 that it is running in SQL strict mode.

 2. The following tables now use DATETIME instead of DATE (see #45) to
 store date entities:
   a. `patient` (dob, registration_date)
   b. `employee` (dob, date_embauche)
   c. `exchange_rate` (date)

 3. A dependency on `chai-datetime` has been added to ensure dates are
 properly tested during integration tests.  An example of this can be
 found in the Employees API tests.

 4. Many controller refactors to pre-process dates or fix bugs revealed
 by failing tests.

 5. Error handling has been moved into the `interceptors` file.  Errors
 in the application are now functions, to be called as `new
 req.codes.ERROR()`.  The syntax is liable to change in a future commit.

 6. A series of database errors have been added to ensure that all
 errors are informative and handled uniformly.

 7. Removed unused `errors.js` controller file.

 8. Misc typos and style fixes.
  • Loading branch information
jniles committed Jan 27, 2016
1 parent 996a780 commit 7c88917
Show file tree
Hide file tree
Showing 41 changed files with 603 additions and 556 deletions.
4 changes: 2 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ node_js:
- "iojs"

before_script:
- echo "[mysqld]\nsql_mode = \"STRICT_ALL_TABLES\"" > /etc/my.cnf
- sudo service restart mysql
# ensure mysql is running in "STRICT_ALL_TABLES' mode
- mysql -u root -e "SET GLOBAL sql_mode='STRICT_ALL_TABLES';"
- mysql -u root -e "GRANT ALL PRIVILEGES ON *.* TO 'bhima_travis'@'localhost' IDENTIFIED BY 'TRAVIS_PASSWORD' WITH GRANT OPTION;"
- mysql -u root -e "FLUSH PRIVILEGES;"
- mysql -u root -e "DROP SCHEMA IF EXISTS bhima_travis_db;"
Expand Down
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
"app": "gulp build && cd bin && NODE_ENV=production node server/app.js",
"dev": "gulp build && cd bin && NODE_ENV=development node server/app.js",
"start": "gulp build && cd bin && NODE_ENV=production node server/app.js",
"dev_windows" : "gulp build && cd bin && set NODE_ENV=development && node server/app.js",
"travis" : "gulp build && cd bin && NODE_ENV=travis node server/app.js"
"dev_windows": "gulp build && cd bin && set NODE_ENV=development && node server/app.js",
"travis": "gulp build && cd bin && NODE_ENV=travis node server/app.js"
},
"repository": {
"type": "git",
Expand Down Expand Up @@ -45,6 +45,7 @@
"devDependencies": {
"chai": "^3.4.0",
"chai-as-promised": "^5.1.0",
"chai-datetime": "^1.4.0",
"chai-http": "^1.0.0",
"gulp": "^3.9.0",
"gulp-concat": "^2.6.0",
Expand Down
109 changes: 69 additions & 40 deletions server/config/codes.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,108 +5,137 @@
* within the application. The basic usage should be either:
*
* @example
* next(Error(res.codes.ERR_NOT_FOUND));
* Promise.then(function () { throw new Error(res.codes.ERR_NOT_FOUND); }).catch(next);
* next(new res.codes.ERR_NOT_FOUND());
* Promise.then(function () { throw new res.codes.ERR_NOT_FOUND(); }).catch(next);
*/

/** custom errorFactor function to throw in API endpoints */
function makeError(name, defn) {

// must call 'new' on this function
function AppError() {
this.name = name;

// these should never be blank.
this.message = (defn.reason || '');
this.reason = (defn.reason || '');
this.code = (defn.code || 'ERR_UNKNOWN_ERROR');
this.httpStatus = defn.httpStatus;

// record the callstack where and when it was thrown
this.stack = (new Error()).stack;
}

// ensure that we are inheriting from the global error object
AppError.prototype = Object.create(Error.prototype);
AppError.prototype.constructor = AppError;

return AppError;
}

module.exports = {

/** application error codes */
'ERR_NOT_FOUND' : {
'ERR_NOT_FOUND' : makeError('ApiError', {
code : 'ERR_NOT_FOUND',
httpStatus : 404,
reason : 'We could not find a record by that id.'
},
'ERR_NOT_AUTHENTICATED' : {
}),
'ERR_NOT_AUTHENTICATED' : makeError('ApiError', {
code : 'ERR_NOT_AUTHENTICATED',
httpStatus : 401,
reason : 'You have not yet authenticated with the API to access the endpoint. Send a POST to /login with proper credentials to sign in.'
},
'ERR_NO_PERMISSIONS' : {
}),
'ERR_NO_PERMISSIONS' : makeError('ApiError', {
code : 'ERR_NO_PERMISSIONS',
httpStatus : 301,
reason : 'You do not have permissions to access the endpoint.'
},
'ERR_BAD_CREDENTIALS' : {
}),
'ERR_BAD_CREDENTIALS' : makeError('ApiError', {
code : 'ERR_BAD_CREDENTIALS',
httpStatus : 401,
reason : 'You did not provide the correct credentials to access the endpoint.'
},
'ERR_NO_ENTERPRISE' : {
}),
'ERR_NO_ENTERPRISE' : makeError('ApiError', {
code : 'ERR_NO_ENTERPRISE',
httpStatus : 401,
reason : 'There are no enterprises regiested in the database. Please contact a developer.'
},
'ERR_NO_PROJECT' : {
}),
'ERR_NO_PROJECT' : makeError('ApiError', {
code : 'ERR_NO_PROJECT',
httpStatus : 401,
reason : 'There are no projects registered in the database. The selected action cannot be completed.'
},
'ERR_EMPTY_BODY' : {
}),
'ERR_EMPTY_BODY' : makeError('ApiError', {
code : 'ERR_EMPTY_BODY',
httpStatus : 400,
reason : 'You cannot submit a PUT/POST request with an empty body to the server.'
},
'ERR_PROTECTED_FIELD' : {
}),
'ERR_PROTECTED_FIELD' : makeError('ApiError', {
code: 'ERR_PROTECTED_FIELD',
httpStatus : 400,
reason : 'The update request attempted to change a protected field.'
},
'ERR_PARAMETERS_REQUIRED' : {
}),
'ERR_PARAMETERS_REQUIRED' : makeError('ApiError', {
code : 'ERR_PARAMETERS_REQUIRED',
httpStatus : 400,
reason : 'The request requires at least one parameter.'
},
'ERR_NEGATIVE_VALUES' : {
}),
'ERR_NEGATIVE_VALUES' : makeError('ApiError', {
code : 'ERR_NEGATIVE_VALUES',
httpStatus : 400,
reason : 'Expected some value(s) to be positive, but received a negative value.'
},
'ERR_BAD_VALUE' : {
}),
'ERR_BAD_VALUE' : makeError('ApiError', {
code : 'ERR_BAD_VALUE',
httpStatus : 400,
reason : 'You sent a bad value for some parameters'
},
}),

/** MySQL error codes */
'ER_DISK_FULL' : {
'ER_DISK_FULL' : makeError('DatabaseError', {
code : 'DB.ER_DISK_FULL',
httpStatus : 500,
reason : 'The databsae ran out of space. Please free some memory to continue using the application.'
},
'ER_DUP_KEY' : {
}),
'ER_DUP_KEY' : makeError('DatabaseError', {
code : 'DB.ER_DUP_KEY',
httpStatus: 400,
reason : 'A key collided in a unique database field. Please either retry your action. If the problem persists, contact the developers.'
},
'ER_BAD_FIELD_ERROR' : {
}),
'ER_BAD_FIELD_ERROR' : makeError('DatabaseError', {
code : 'DB.ER_BAD_FIELD_ERROR',
httpStatus : 400,
reason : 'Column does not exist in database.'
},
'ER_ROW_IS_REFERENCED_2' : {
}),
'ER_ROW_IS_REFERENCED_2' : makeError('DatabaseError', {
code : 'DB.ER_ROW_IS_REFERENCED_2',
httpStatus : 400,
reason : 'Cannot delete entity becuase entity is used in another table.'
},
'ER_BAD_NULL_ERROR' : {
}),
'ER_BAD_NULL_ERROR' : makeError('DatabaseError', {
code : 'DB.ER_BAD_NULL_ERROR',
httpStatus: 400,
reason : 'A column was left NULL that cannot be NULL.'
},
'ER_DUP_ENTRY' : {
}),
'ER_DUP_ENTRY' : makeError('DatabaseError', {
code : 'DB.ER_DUP_ENTRY',
httpStatus: 400,
reason : 'You cannot insert a duplicate record. This record already exists.'
},
'ER_PARSE_ERROR' : {
}),
'ER_PARSE_ERROR' : makeError('DatabaseError', {
code : 'DB.ER_PARSE_ERROR',
httpStatus : 400,
reason : 'Your query cannot be translated into valid SQL. Please modify your query and try again.'
},
'ER_EMPTY_QUERY' : {
}),
'ER_EMPTY_QUERY' : makeError('DatabaseError', {
code : 'DB.ER_EMPTY_QUERY',
httpStatus : 400,
reason : 'The query was empty.'
}
}),
'ER_NO_DEFAULT_FOR_FIELD' : makeError('DatabaseError', {
code : 'DB.ER_NO_DEFAULT_FOR_FIELD',
httpStatus : 400,
reason : 'You did not include enough information in your query.'
}),
};
4 changes: 2 additions & 2 deletions server/config/express.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ exports.configure = function configure(app) {
var emptyBody = Object.keys(req.body).length === 0;

if (emptyBody) {
next(Error(req.codes.ERR_EMPTY_BODY));
next(new req.codes.ERR_EMPTY_BODY());
} else {
next();
}
Expand Down Expand Up @@ -93,7 +93,7 @@ exports.configure = function configure(app) {
var publicRoutes = ['/login', '/languages', '/projects', '/logout'];

if (req.session.user === undefined && !within(req.path, publicRoutes)) {
next(Error(req.codes.ERR_NOT_AUTHENTICATED));
next(new req.codes.ERR_NOT_AUTHENTICATED());
} else {
next();
}
Expand Down
21 changes: 16 additions & 5 deletions server/config/interceptors.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,25 @@ exports.databaseErrorHandler = function databaseErrorHandler(error, req, res, ne
// skip native errors - caught in catchAllErrorHandler
if (isNativeError(error)) { return next(error); }


// check to see if this is a database error
if (error && error.sqlState) {

console.log('[SQL ERROR]:', error);

// retrieve the formatted error from
var appError = errors[error.code];
try {
var appError = new errors[error.code]();

//console.log('[MATCHING APP ERROR]:', appError);

// send the formatted error back to the client.
res.status(appError.httpStatus).json(appError);
// send the formatted error back to the client.
res.status(appError.httpStatus).json(appError);

// if no matching error found, pass on to next();
} catch (e) {
next(error);
}

// if not matching a databse error, forward to next interceptor
} else {
Expand All @@ -90,9 +101,9 @@ exports.catchAllErrorHandler = function catchAllErrorHandler(error, req, res, ne
'use strict';

// log errors unless explicitly turned of in the config
if (process.env.LOG_LEVEL !== 'none') {
//if (process.env.LOG_LEVEL !== 'none') {
console.error('[ERROR]', error);
}
//}

// return a 500 error so the client
res.status(500).json(error);
Expand Down
12 changes: 6 additions & 6 deletions server/config/routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,34 +109,34 @@ exports.configure = function (app) {

// API for account routes crud
app.get('/accounts', account.list);
app.get('/accounts/:id', account.getAccount);
app.get('/accounts/:id', account.detail);
app.post('/accounts', account.create);
app.put('/accounts/:id', account.update);

//API for account type routes crud
app.get('/account_types', accountType.list);
app.get('/account_types/:id', accountType.getAccountType);
app.get('/account_types/:id', accountType.detail);
app.post('/account_types', accountType.create);
app.put('/account_types/:id', accountType.update);
app.delete('/account_types/:id', accountType.remove);

//API for cost_center routes crud
app.get('/cost_centers', costCenter.list);
app.get('/cost_centers/:id', costCenter.getCostCenter);
app.get('/cost_centers/:id', costCenter.detail);
app.post('/cost_centers', costCenter.create);
app.put('/cost_centers/:id', costCenter.update);
app.delete('/cost_centers/:id', costCenter.remove);

//API for profit_center routes crud
app.get('/profit_centers', profitCenter.list);
app.get('/profit_centers/:id', profitCenter.getProfitCenter);
app.get('/profit_centers/:id', profitCenter.detail);
app.post('/profit_centers', profitCenter.create);
app.put('/profit_centers/:id', profitCenter.update);
app.delete('/profit_centers/:id', profitCenter.remove);

//API for reference routes crud
app.get('/references', reference.list);
app.get('/references/:id', reference.getReference);
app.get('/references/:id', reference.detail);
app.post('/references', reference.create);
app.put('/references/:id', reference.update);
app.delete('/references/:id', reference.remove);
Expand Down Expand Up @@ -442,7 +442,7 @@ exports.configure = function (app) {

/** employees */
app.get('/employees', employees.list);
app.get('/employees/:id', employees.details);
app.get('/employees/:id', employees.detail);
app.put('/employees/:id', employees.update);
app.post('/employees', employees.create);

Expand Down
8 changes: 4 additions & 4 deletions server/controllers/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ exports.login = function login(req, res, next) {

// if no data found, we return a login error
if (rows.length < 1) {
throw req.codes.ERR_BAD_CREDENTIALS;
throw new req.codes.ERR_BAD_CREDENTIALS();
}

// we assume only one match for the user
Expand All @@ -43,7 +43,7 @@ exports.login = function login(req, res, next) {

// if no permissions, notify the user that way
if (rows.length === 0) {
throw req.codes.ERR_NO_PERMISSIONS;
throw new req.codes.ERR_NO_PERMISSIONS();
}

// update the database for when the user logged in
Expand All @@ -65,7 +65,7 @@ exports.login = function login(req, res, next) {
})
.then(function (rows) {
if (rows.length === 0) {
throw req.codes.ERR_NO_ENTERPRISE;
throw new req.codes.ERR_NO_ENTERPRISE();
}

enterprise = rows[0];
Expand All @@ -77,7 +77,7 @@ exports.login = function login(req, res, next) {
})
.then(function (rows) {
if (rows.length === 0) {
throw req.codes.ERR_NO_PROJECT;
throw new req.codes.ERR_NO_PROJECT();
}

project = rows[0];
Expand Down
Loading

0 comments on commit 7c88917

Please sign in to comment.