v2: Add unique username validation!

Author: aseemk

models/errors.js

@@ -0,0 +1,12 @@
+var util = require('util');
+
+function ValidationError(msg) {
+    Error.call(this);
+    this.name = 'ValidationError';
+    this.message = msg;
+    Error.captureStackTrace(this, this.constructor);
+}
+
+util.inherits(ValidationError, Error);
+
+exports.ValidationError = ValidationError;

models/user.js

@@ -2,6 +2,7 @@
 // User model logic.
 
 var neo4j = require('neo4j');
+var errors = require('./errors');
 
 var db = new neo4j.GraphDatabase({
     // Support specifying database info via environment variables,
@@ -19,6 +20,18 @@ var User = module.exports = function User(_node) {
     this._node = _node;
 }
 
+// Public constants:
+
+User.VALIDATION_INFO = {
+    'username': {
+        required: true,
+        minLength: 2,
+        maxLength: 16,
+        pattern: /^[A-Za-z0-9_]+$/,
+        message: '2-16 characters; letters, numbers, and underscores only.'
+    },
+};
+
 // Public instance properties:
 
 // The user's username, e.g. 'aseemk'.
@@ -28,23 +41,67 @@ Object.defineProperty(User.prototype, 'username', {
 
 // Private helpers:
 
-// Takes the given caller-provided properties (which corresponding to our public
-// instance properties), selects only whitelisted ones for editing, validates
-// them, and translates them to the corresponding internal db properties.
-function translate(props) {
-    // Today, the only property we have is `username`.
-    // TODO: Validate it. E.g. length, acceptable chars, etc.
-    return {
-        username: props.username,
-    };
+// Takes the given caller-provided properties, selects only known ones,
+// validates them, and returns the known subset.
+// By default, only validates properties that are present.
+// (This allows `User.prototype.patch` to not require any.)
+// You can pass `true` for `required` to validate that all required properties
+// are present too. (Useful for `User.create`.)
+function validate(props, required) {
+    var safeProps = {};
+
+    for (var prop in User.VALIDATION_INFO) {
+        var val = props[prop];
+        validateProp(prop, val, required);
+        safeProps[prop] = val;
+    }
+
+    return safeProps;
+}
+
+// Validates the given property based on the validation info above.
+// By default, ignores null/undefined/empty values, but you can pass `true` for
+// the `required` param to enforce that any required properties are present.
+function validateProp(prop, val, required) {
+    var info = User.VALIDATION_INFO[prop];
+    var message = info.message;
+
+    if (!val) {
+        if (info.required && required) {
+            throw new errors.ValidationError(
+                'Missing ' + prop + ' (required).');
+        } else {
+            return;
+        }
+    }
+
+    if (info.minLength && val.length < info.minLength) {
+        throw new errors.ValidationError(
+            'Invalid ' + prop + ' (too short). Requirements: ' + message);
+    }
+
+    if (info.maxLength && val.length > info.maxLength) {
+        throw new errors.ValidationError(
+            'Invalid ' + prop + ' (too long). Requirements: ' + message);
+    }
+
+    if (info.pattern && !info.pattern.test(val)) {
+        throw new errors.ValidationError(
+            'Invalid ' + prop + ' (format). Requirements: ' + message);
+    }
+}
+
+function isConstraintViolation(err) {
+    return err instanceof neo4j.ClientError &&
+        err.neo4j.code === 'Neo.ClientError.Schema.ConstraintViolation';
 }
 
 // Public instance methods:
 
 // Atomically updates this user, both locally and remotely in the db, with the
 // given property updates.
 User.prototype.patch = function (props, callback) {
-    var safeProps = translate(props);
+    var safeProps = validate(props);
 
     var query = [
         'MATCH (user:User {username: {username}})',
@@ -63,6 +120,15 @@ User.prototype.patch = function (props, callback) {
         query: query,
         params: params,
     }, function (err, results) {
+        if (isConstraintViolation(err)) {
+            // TODO: This assumes username is the only relevant constraint.
+            // We could parse the constraint property out of the error message,
+            // but it'd be nicer if Neo4j returned this data semantically.
+            // Alternately, we could tweak our query to explicitly check first
+            // whether the username is taken or not.
+            err = new errors.ValidationError(
+                'The username ‘' + props.username + '’ is taken.');
+        }
         if (err) return callback(err);
 
         if (!results.length) {
@@ -234,13 +300,22 @@ User.create = function (props, callback) {
     ].join('\n');
 
     var params = {
-        props: translate(props)
+        props: validate(props)
     };
 
     db.cypher({
         query: query,
         params: params,
     }, function (err, results) {
+        if (isConstraintViolation(err)) {
+            // TODO: This assumes username is the only relevant constraint.
+            // We could parse the constraint property out of the error message,
+            // but it'd be nicer if Neo4j returned this data semantically.
+            // Alternately, we could tweak our query to explicitly check first
+            // whether the username is taken or not.
+            err = new errors.ValidationError(
+                'The username ‘' + props.username + '’ is taken.');
+        }
         if (err) return callback(err);
         var user = new User(results[0]['user']);
         callback(null, user);

public/stylesheets/style.css

@@ -9,3 +9,6 @@ strong a {
   padding: 0 0.25em;
   background-color: #ffa;
 }
+.error {
+  color: #800;
+}

routes/users.js

@@ -1,29 +1,56 @@
 // users.js
 // Routes to CRUD users.
 
+var URL = require('url');
+
+var errors = require('../models/errors');
 var User = require('../models/user');
 
+function getUserURL(user) {
+    return '/users/' + encodeURIComponent(user.username);
+}
+
 /**
  * GET /users
  */
 exports.list = function (req, res, next) {
     User.getAll(function (err, users) {
         if (err) return next(err);
         res.render('users', {
-            users: users
+            User: User,
+            users: users,
+            username: req.query.username,   // Support pre-filling create form
+            error: req.query.error,     // Errors creating; see create route
         });
     });
 };
 
 /**
- * POST /users
+ * POST /users {username, ...}
  */
 exports.create = function (req, res, next) {
     User.create({
-        username: req.body['username']
+        username: req.body.username
     }, function (err, user) {
-        if (err) return next(err);
-        res.redirect('/users/' + user.username);
+        if (err) {
+            if (err instanceof errors.ValidationError) {
+                // Return to the create form and show the error message.
+                // TODO: Assuming username is the issue; hardcoding for that
+                // being the only input right now.
+                // TODO: It'd be better to use a cookie to "remember" this info,
+                // e.g. using a flash session.
+                return res.redirect(URL.format({
+                    pathname: '/users',
+                    query: {
+                        username: req.body.username,
+                        error: err.message,
+                    },
+                }));
+            } else {
+                return next(err);
+            }
+        }
+        res.redirect(getUserURL(user));
     });
 };
 
@@ -32,28 +59,50 @@ exports.create = function (req, res, next) {
  */
 exports.show = function (req, res, next) {
     User.get(req.params.username, function (err, user) {
+        // TODO: Gracefully "no such user" error. E.g. 404 page.
         if (err) return next(err);
         // TODO: Also fetch and show followers? (Not just follow*ing*.)
         user.getFollowingAndOthers(function (err, following, others) {
             if (err) return next(err);
             res.render('user', {
+                User: User,
                 user: user,
                 following: following,
-                others: others
+                others: others,
+                username: req.query.username,   // Support pre-filling edit form
+                error: req.query.error,     // Errors editing; see edit route
             });
         });
     });
 };
 
 /**
- * POST /users/:username
+ * POST /users/:username {username, ...}
  */
 exports.edit = function (req, res, next) {
     User.get(req.params.username, function (err, user) {
+        // TODO: Gracefully "no such user" error. E.g. 404 page.
         if (err) return next(err);
         user.patch(req.body, function (err) {
-            if (err) return next(err);
-            res.redirect('/users/' + user.username);
+            if (err) {
+                if (err instanceof errors.ValidationError) {
+                    // Return to the edit form and show the error message.
+                    // TODO: Assuming username is the issue; hardcoding for that
+                    // being the only input right now.
+                    // TODO: It'd be better to use a cookie to "remember" this
+                    // info, e.g. using a flash session.
+                    return res.redirect(URL.format({
+                        pathname: getUserURL(user),
+                        query: {
+                            username: req.body.username,
+                            error: err.message,
+                        },
+                    }));
+                } else {
+                    return next(err);
+                }
+            }
+            res.redirect(getUserURL(user));
         });
     });
 };
@@ -63,6 +112,8 @@ exports.edit = function (req, res, next) {
  */
 exports.del = function (req, res, next) {
     User.get(req.params.username, function (err, user) {
+        // TODO: Gracefully handle "no such user" error somehow.
+        // E.g. redirect back to /users with an info message?
         if (err) return next(err);
         user.del(function (err) {
             if (err) return next(err);
@@ -72,32 +123,42 @@ exports.del = function (req, res, next) {
 };
 
 /**
- * POST /users/:username/follow
+ * POST /users/:username/follow {otherUsername}
  */
 exports.follow = function (req, res, next) {
     User.get(req.params.username, function (err, user) {
+        // TODO: Gracefully handle "no such user" error somehow.
+        // This is the source user, so e.g. 404 page?
         if (err) return next(err);
         User.get(req.body.otherUsername, function (err, other) {
+            // TODO: Gracefully handle "no such user" error somehow.
+            // This is the target user, so redirect back to the source user w/
+            // an info message?
             if (err) return next(err);
             user.follow(other, function (err) {
                 if (err) return next(err);
-                res.redirect('/users/' + user.username);
+                res.redirect(getUserURL(user));
             });
         });
     });
 };
 
 /**
- * POST /users/:username/unfollow
+ * POST /users/:username/unfollow {otherUsername}
  */
 exports.unfollow = function (req, res, next) {
     User.get(req.params.username, function (err, user) {
+        // TODO: Gracefully handle "no such user" error somehow.
+        // This is the source user, so e.g. 404 page?
         if (err) return next(err);
         User.get(req.body.otherUsername, function (err, other) {
+            // TODO: Gracefully handle "no such user" error somehow.
+            // This is the target user, so redirect back to the source user w/
+            // an info message?
             if (err) return next(err);
             user.unfollow(other, function (err) {
                 if (err) return next(err);
-                res.redirect('/users/' + user.username);
+                res.redirect(getUserURL(user));
             });
         });
     });