Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

When reigstering a user I request two reads from the database to see if there is a username and email and then I write if the checks pass.. Can anyone tell me if this code will block other users?

I am using node with express.js and mongojs.

I know I am saving passwords in plaintext.. This will be changed.

app.post('/register', function(req, res) {

    var checked = 0;
    var errors = [];

    var finishedCheck = function() {
        checked++;
        register();
    }

    var register = function() {

        if(checked === 2){

            if(errors.length > 0) {
                console.log('errors', errors);
                res.statusCode = 409;
                res.send(errors);
            } else {

                var newuser = {
                    username: req.body.username,
                    uppercase: req.body.username.toUpperCase(),
                    password:req.body.password,
                    email: req.body.email.toLowerCase(),
                    userLevel: 0,
                    createdOn: new Date()
                };

                db.users.save(newuser, function(err, val){
                    if(!err) {
                        req.session.userid = val['_id'];
                        res.end();
                    } else {
                        res.statusCode = 406;
                        res.end(['Something went wrong', err]);
                    }
                });
            }
        }
    }

    var check = function(username, email) {
        console.log('chekc');
        db.users.ensureIndex({email:1},{unique:true});
        db.users.ensureIndex({uppercase:1},{unique:true});
        if(!username.match(/^[A-Za-z0-9_]*$/)) {
            errors.push('Username is invalid');
            finishedCheck();
        } else {
            db.users.findOne({uppercase: username}, function(err, val) {
                if(val) {
                    errors.push('Username Taken');
                }
                finishedCheck();
            });
        }

        //check if the is a valid email address if so then check to see if its already registered
        if(!email.match(/^[-a-z0-9~!$%^&*_=+}{\'?]+(\.[-a-z0-9~!$%^&*_=+}{\'?]+)*@([a-z0-9_][-a-z0-9_]*(\.[-a-z0-9_]+)*\.(aero|arpa|biz|com|coop|edu|gov|info|int|mil|museum|name|net|org|pro|travel|mobi|[a-z][a-z])|([0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}))(:[0-9]{1,5})?$/i)) {
            errors.push('Email is invalid');
            finishedCheck();
        } else {
            db.users.findOne({email: email}, function(err, val) {
                if(val) {
                    errors.push('Email is already in the database');
                } 
                finishedCheck();
            });
        }


    };

    check(req.body.username.toUpperCase(), req.body.email.toLowerCase());

});
share|improve this question
add comment

2 Answers

up vote 2 down vote accepted

No

app.post('/register'...

Has a callback and is non-blocking within Node

db.users.save(...

Has a callback and is non-blocking within Node (but will block on your database level, which is normal)

db.users.findOne(...

Has a callback and is non-blocking within Node (but will block on your database level, which is normal)

Your code looks pretty normal. MongoDB has a global lock across an entire mongoDB server daemon, so this is the only level that your current code will block. It will not block on your Node.js level for your concurrent users.

share|improve this answer
add comment

The answer is NO.

The reason is that the calls to db are asynchronous, that's why you pass a function object as a callback.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.