Removing spam from a database using NodeJS

Question

I've written the following in NodeJS as I am trying to learn a bit more about that toolset. It 'works' but is not very robust. I am not concerned with security at this stage and the code will not be accessible to end users. I would like to incorporate some kind of timer which ensures that the call to the Akismet API does not hit a rate limit. Occasionally I see socket hang up and timeout errors.

var mysql      = require('mysql');
var connection = mysql.createConnection({
    host     : 'localhost',
    user     : 'root',
    password : '123',
    database : 'mydb',
    //debug: true,
    multipleStatements: true
});

var akismet = require('akismet').client({ blog: 'http://example.com', apiKey: 'xxxxxxx' });

var selectPosts = "SELECT p.topic_id as id, poster_ip, t.subject as message, p.poster FROM topics  t\
                  LEFT JOIN posts p on t.first_post_id = p.id WHERE poster_ip != ''";

akismet.verifyKey(function(err, verified) {
    if(err) throw err;
    if (verified) {

        console.log('API key successfully verified.');
    }

    else {

        console.log('Unable to verify API key.');
    }

});

connection.connect();

connection.query(selectPosts, function(err, rows, fields) {
    if (err) throw err;

    rows.forEach( function(entry) {

    checkSpam(entry)

    });

});

var afterDelete = function (err, result) {

    if (err) throw err;
    console.log('deleted ' + result.affectedRows + ' rows');


}

var afterCheck = function(err, entry) {

    if (err) throw err;

    var deleteQuery = "DELETE t, p FROM topics AS t LEFT JOIN posts AS p ON t.id = p.topic_id WHERE t.id=";
    var deleteUser = "DELETE FROM users WHERE username='"+entry.poster+"';";
        console.log(deleteQuery + entry.id);
        console.log('entry is ' + entry['id']);
        connection.query(deleteQuery + entry.id, function(err,result)
        {
            afterDelete(err,result);
        });

        connection.query(deleteUser, afterDelete);



}

var checkSpam = function(entry) {
    akismet.checkSpam({
        user_ip: entry['poster_ip'],
        comment_author: entry['poster'],
        comment_content: entry['message']

    }, function(err, spam){
        if (err) throw err;
        if (spam) {
            //console.log('spam');
            afterCheck(err, entry);

        } else {

            console.log('Not spam');

        }

    });


}

//connection.end();

Answer 1

Well, your code can use a little formatting. I usually go with 2-character indents (tabs or spaces) since in JS, code tends to be nested. I usually also limit my newlines to 1 or 2, one being a "breather" and 2 for totally different blocks of code. While everyone's preferences vary, always consider consistency and readability.

var connection = mysql.createConnection({
    host     : 'localhost',
    user     : 'root',
    password : '123',
    database : 'mydb',
    //debug: true,
    multipleStatements: true
});

var akismet = require('akismet').client({ blog: 'http://example.com', apiKey: 'xxxxxxx' });

I suggest you read your credentials from environment variables. You wouldn't want anyone seeing your passwords or API keys, especially when you start to use a version control system. This also makes it easy for multiple deployments (multiple machines, virtual hosts etc) as the credentials can be configured from a provisioning tool. I doubt your host will always be localhost on every machine, or your database be named mydb on every instance, or your API key be just one across your system.

const POSTS_QUERY = `
  SELECT
    p.topic_id as id,
    poster_ip,
    t.subject as message,
    p.poster
  FROM
    topics
  LEFT JOIN
    posts p on t.first_post_id = p.id 
  WHERE
    poster_ip != ''
`;

If you're using the newer Node.js (I believe the 4.x onwards), it should have ES6 support, which includes multi-line (backticked) strings. This allows you to write SQL queries like the one above. Much cleaner than your single-lined strings. And since it's probably a constant string, I suggest using const as well as a conventional all-caps, snake-cased name.

I notice that your async functions are "node functions" (receive an error object first, data second). You can take advantage of Bluebird's promisify function to turn them into Promise-returning functions. That way, you can take advantage of Promises and write fairly linear-looking code.

Here's an excerpt of what I came up with:

let Promise = require("bluebird");

let connection = require('mysql').createConnection({
  host: 'localhost',
  user: 'root',
  password: '123',
  database: 'mydb',
  //debug: true,
  multipleStatements: true
});

let akismet = require('akismet').client({
  blog: 'http://example.com',
  apiKey: 'xxxxxxx'
});

const SELECT_POSTS = `
  SELECT
    p.topic_id as id,
    poster_ip,
    t.subject as message,
    p.poster
  FROM
    topics
  LEFT JOIN
    posts p on t.first_post_id = p.id 
  WHERE
    poster_ip != ''
`;

let verifyKey = Promise.promisify(akismet.verifyKey);
let query = Promise.promisify(connection.query, { multiargs: true });
let checkSpam = Promise.promisify(akismet.checkSpam);

let verifyKeyPromise = verifyKey()
.then(function(verified){
  console.log('API key successfully verified');
  connection.connect()
  return query(SELECT_POSTS);
}, function(err){
  console.log('Unable to verify API key');
})
.then(function(rows, fields){
  console.log('Posts retrieved');
  return Promise.all(rows.map(function(entry){
    return checkSpam({
      user_ip: entry['poster_ip'],
      comment_author: entry['poster'],
      comment_content: entry['message']
    });
  });
}, function(v){
  console.log('Failed to retrieve posts');
})
.then(function(entrySpamStatus){
  console.log('Spam check succeeded');
  // and so on...  
}, function(err){
  console.log('Spam check failed');
  // and so on...  
})

// and so on...