Node + Sequelize: How to check if item exists before adding? (async confusion)

Question

I am unfortunately new to node and running into some confusion regarding the asynchronous/synchronous execution of node.

I am using node, sequelize with sqlite and async.js.

I have a series of Articles, each of which has a number of Authors.

For each Authors in each Article, I'd like to check if the Author exists. If not, create it.

The problem is, on the initial run, duplicate authors are being created, I assume due to asynchronous functionality causing an issue with checking for existence.

For example, with the array: authors = ['A. Test', 'B. Test', 'C. Test', 'A. Test']

and the code:

async.each(authors, function(item, callback){
    Author.sync().then(function(){
      Author.count({ where: {name: item.trim()} }).then(function(count){
        if (count != 0) {
          console.log('Author already exists')
        } else {
          console.log('Creating author...')
          Author.create({
            name: item.trim()
          })
        }
      })
    })
  })

On the first run, will create a table:

ID | name
------------
0  | A. Test
1  | B. Test
2  | C. Test
3  | A. Test

What am I doing wrong? I seem to be missing a fundamental concept of asynchronous vs synchronous execution in Node.

(I've also tried async.eachSeries which is supposed to execute in series rather than in parallel?)

Edit: Slightly refactored, but still creating duplicates

async.eachSeries(authors, function(authorName, callback){
    Author.findOne({ where: {name: authorName.trim()} }).
    then(function(author){
      if (author) {
        // Author exists...
        callback()
      } else {
        // Author does not exist...
        Author.create({
          name: authorName.trim()
        }).then(function(author){
          callback()
        })
      }
    })
  })

Historied

Answer 1

Author.count isn't really needed, unless you need the count. See findOrCreate().

With findOrCreate() you could have the following. (Edited trex005's snippet for this)

async.eachSeries(authors, function(item, callback) {
  Author.sync().then(function() {
    Author.findOrCreate({
      where: {
        name: item.trim()
      },
      defaults: { // set the default properties if it doesn't exist
        name: item.trim()
      }
    }).then(function(result) {
      var author = result[0], // the instance of the author
        created = result[1]; // boolean stating if it was created or not

      if (!created) { // false if author already exists and was not created.
        console.log('Author already exists');
      }

      console.log('Created author...');
      callback();
    });
  })
})

Wallis

Hi thanks for the reply, actually I tried findOrCreate (and refactored to a somewhat cleaner snippet added to my answer above) but findOrCreate was causing all sorts of issues with sqlite blocking/database locking - apparently this was happening in postgres for some users as well. Unfortunately, my new snippet still doesn't actually work, and is still creating duplicates. – Historied

@joshua-f btw, you can use spread method instead of then method for seperating promise arguments. Example: .spread(function(author, isCreated) { ... }); – Wan

@AliBARIN Oh sweet, did not know that. – Wallis

Is there a reason for calling sync() inside the async function? – Luann

@RonenTeva No, that should probably be outside of the async function. – Wallis

Answer 2

Change your each to eachSeries and actually call the callback and you should be golden.

async.eachSeries(authors, function(item, callback){
    Author.sync().then(function(){
      Author.count({ where: {name: item.trim()} }).then(function(count){
        if (count != 0) {
          console.log('Author already exists')
          callback(); //assuming you want it to keep looping, if not use callback(new Error("Author already exists"))
        } else {
          console.log('Creating author...')
          Author.create({
            name: item.trim()
          }).then(function(author){
            callback();
          })
        }
      })
    })
  })

Brunabrunch

Hm, this doesn't appear to be working, it's only adding the first author. – Historied

Does your Author.create support a callback like I mentioned in the comments? – Brunabrunch

Sorry, how do you mean? The create method is implemented by sequelize which results in a promise – Historied

Ok, it works, thankyou! Just to clear my understanding of the flow, what is the callback() function that is called? I find the wording of the async.js documentation confusing. – Historied

It tells the loop to continue – Brunabrunch

Sorry, unfortunately after further testing, the function it is still creating duplicate results :( – Historied