4

I'm pretty new to the node world and trying to migrate our php application to node. To be able to return all article data several different queries have to be done depending on the results of the first query. Currently my data object is empty as it's returned before the two queries run. How can I "chain" these queries using a promised based approach.

I found a library https://github.com/lukeb-uk/node-promise-mysql which I think could help but I have no idea how to implement it with my code.

exports.getArticleData = function(req, done) {
  pool.getConnection(function(error, connection) {
    if (error) throw error;

    var data = {
        article: {},
        listicles: []
    };

    // Inital query
    connection.query(
        `SELECT article_id, title, is_listicle_article, FROM com_magazine_articles AS article WHERE article_id = ${req
            .params.articleId}`,
        function(error, results) {
            data.article = results;
        }
    );

    // This query should only be excuted if is_listicle_article = true
    if (data.article.is_listicle_article) {
        connection.query(
            `SELECT * FROM com_magazine_article_listicles WHERE article_id = ${req.params
                .articleId}`,
            function(error, results) {
                data.listicle = results;
            }
        );
    }

    // More queries depending on the result of the first one
    // ....
    // ....

    // Callback with the data object
    done(data);

    connection.release();
  });
};

What would be the best approach to execute queries based on other queries results? Any help is really appreciated.

Menelik
  • 99
  • 2
  • 8

1 Answers1

2

The functionality you are looking for is Promise chaining, it allows you to construct a sequence of promises, each depending on the result of the previous value. Applying this to your code, you would get something like this:

exports.getArticleData = function(req, done) {
  pool.getConnection(function(error, connection) {
    if (error) throw error;

    // Inital query
    return connection.query(
        `SELECT article_id, title, is_listicle_article, FROM com_magazine_articles AS article WHERE article_id = ${req
            .params.articleId}`
    ).then((rows) => {
    
       return Promise.all(rows.map((article) => {
          if (article.is_listicle_article) {
            return connection.query(
                `SELECT * FROM com_magazine_article_listicles WHERE article_id = ${req.params
                    .articleId}`
             );
          } else {
             return Promise.resolve(null);
          }
       }));
    }).then((res) => {
     connection.release();
     done(res.filter(function(i){ return i != null; }));
    })

    // This query should only be excuted if is_listicle_article = true
    

    // More queries depending on the result of the first one
    // ....
    // ....

    // Callback with the data object
    connection.release();
  });
};

Obviously since I don't have all of your code, I couldn't verify this example, but this should be roughly the functionality you are looking for. That said, I think there were a couple of mistakes you should watch out for in your example code:

  • connection.query() returns a promise (aka doesn't need a callback function). Use this functionality to your advantage- it will make your code prettier.
  • connection.query() returns an array of rows, not a single value. You seemed to ignore this in your example code.
  • Try not to save things into a variable when using promises, it isn't necessary. To remedy this, read more into the Promise API (Promise.resolve(), Promise.reject(), Promise.any(), Promise.catch(), Promise.all()) etc.
  • It seems like these SQL queries could easily be combined into a single query. This will be way more efficient that performing two operations. Not sure if this is the case with the remaining queries you wish to use, but definitely something to look out for.
Derek Brown
  • 4,232
  • 4
  • 27
  • 44
  • Hi Derek, thank you very much for your swift reply. I will try to implement your solution right away and will let you know if it worked. I copied the syntax from https://github.com/mysqljs/mysql and there they used a function within connection.query. I don't know if there is a way to combine those queries as they require values from the first query. The remaining queries will be similar to the one in my code but only one of those queries will run depending on the result of the first one. For example if article type is listicle query 2 will run, if article is sponsored story query 3. – Menelik Oct 23 '17 at 16:39
  • @Menelik 1) if later queries rely on data from the first query, storing a variable is not a terrible idea. 2) You can easily run conditional queries this way too. Just run something like `Promise.then(() => { if(cond){ return connection.query() } else { return connection.query() } }` – Derek Brown Oct 23 '17 at 16:42
  • Just played around with your code. I receive the following error: `TypeError: connection.query(...).then is not a function`. Do I need to import a certain promise library for this to work? – Menelik Oct 23 '17 at 16:45
  • @Menelik node.js (depending on version) has some native promise support, though a promise library like Bluebird can't hurt. – Derek Brown Oct 23 '17 at 16:54
  • @Menelik Though I don't think this is what is causing your error. Make sure you are importing `node-promise-mysql` correctly: `var mysql = require('promise-mysql');` – Derek Brown Oct 23 '17 at 16:55
  • I have updated my initial question with the new code and some other files. Unfortunately I still get the same error message even after requiring promise-mysql. Any idea why this error still occurs? Any help is really appreciated. – Menelik Oct 23 '17 at 17:17
  • @Menelik can you actually roll your question back to the previous version and create a new question? This will help others having the same issue in the future keep track of what's going on. – Derek Brown Oct 23 '17 at 17:27
  • Thank you Derek, promises was the way to go. You pointed me in the right direction. – Menelik Oct 26 '17 at 06:17