Множественная функция Q.all внутри?

Question

Я хочу отправить пользователю список новых книг. Пока что код ниже работает нормально. Проблема в том, что я не хочу отправлять книгу несколько раз, поэтому я хочу их отфильтровать.

Текущий код работает отлично:

function checkActiveBooks(books) { 
    var queue = _(books).map(function(book) { 
    var deferred = Q.defer(); 

    // Get all alerts on given keywords 
    request('http://localhost:5000/books?l=0&q=' + book.name, function(error, response, body) { 
     if (error) { 
     deferred.reject(error); 
     } 

     var books = JSON.parse(body); 
     if (!_.isEmpty(books)) { 

     // Loop through users of current book. 
     var userBooks = _(book.users).map(function(user) { 

      // Save object for this user with name and deals. 
      return { 
       user: user, 
       book: book.name, 
       books: books 
      } 

     }); 

     if (_.isEmpty(userBooks)) { 
      deferred.resolve(null); 
     } else { 
      deferred.resolve(userBooks); 
     } 
     } else { 
     deferred.resolve(null); 
     } 

    }); 

    return deferred.promise; 

    }); 

    return Q.all(queue); 
} 

Но теперь я хочу, чтобы уже фильтровать послал книги:

function checkActiveBooks(books) { 
    var queue = _(books).map(function(book) { 
     var deferred = Q.defer(); 
     // Get all alerts on given keywords 
     request('http://localhost:5000/books?l=0&q=' + book.name, function(error, response, body) { 
      if (error) { 
       deferred.reject(error); 
      } 
      var books = JSON.parse(body); 
      if (!_.isEmpty(books)) { 
       // Loop through users of current book. 
       var userBooks = _(book.users).map(function(user) { 
        var defer = Q.defer(); 
        var userBook = user.userBook.dataValues; 
        // Check per given UserBook which books are already sent to the user by mail 
        checkSentBooks(userBook).then(function(sentBooks) { 
         // Filter books which are already sent. 
         var leftBooks = _.reject(books, function(obj) { 
          return sentBooks.indexOf(obj.id) > -1; 
         }); 
         // Save object for this user with name and deals. 
         var result = { 
          user: user, 
          book: book.name, 
          books: leftBooks 
         } 
         return deferred.resolve(result); 
        }); 
        return Q.all(userBooks); 
       } else { 
        deferred.resolve(null); 
       } 
      }); 
      return deferred.promise; 
     }); 
     return Q.all(queue); 
    } 

Но приведенный выше код не работает. Он не прекращает цикл. Я думал, что имеет смысл использовать q.all дважды, потому что он содержит две петли. Но я думаю, что я делаю это неправильно ...

Answer 1

Действуя на @ предложение BENJAMINS, вот то, что код будет выглядеть, когда checkSentBooks возвращает обещание:

var request = Q.nfbind(require("request")); // a promised version. 
function checkActiveBooks(books) { 
    return Q.all(_(books).map(function(book) { 
     // a callback with multiple arguments will resolve the promise with 
     // an array, so we use `spread` here 
     return request('http://localhost:5000/books?l=0&q=' + book.name).spread(function(response, body) { 
      var books = JSON.parse(body); 
      if (_.isEmpty(books)) return null; 
      return Q.all(_(book.users).map(function(user) { 
       return checkSentBooks(user.userBook.dataValues).then(function(sentBooks) { 
//    ^^^^^^ return a promise to the array for `Q.all` 
        return { 
         user: user, 
         book: book.name, 
         books: _.reject(books, function(obj) { 
          return sentBooks.indexOf(obj.id) > -1; 
         }) 
        }; 
       }); 
      })); 
     }); 
    })); 
} 

Answer 2

Прежде всего, вы всегда должны обещать на самом низком уровне. Вы усложняете ситуацию здесь и имеете несколько отложенных дней. Как правило, вы должны иметь только отсрочки при конвертации API в обещания. Обещания цепи и создавать так давайте делать это :)

var request = Q.nfbind(require("request")); // a promised version. 

Это может сделать ваш код в верхней части стали:

function checkActiveBooks(books) { 
    return Q.all(books.map(function(book){ 
     return request('http://.../books?l=0&q=' + book.name) 
       .get(1) // body 
       .then(JSON.parse) // parse body as json 
       .then(function(book){ 
        if(_.isEmpty(book.users)) return null; 
        return book.users.map(function(user){ 
         return {user: user, book: book.name, books: books }; 
        }); 
       }); 
    }); 
} 

который намного более изящным, на мой взгляд.

Теперь, если мы хотим, чтобы фильтровать их с помощью предиката мы можем сделать:

function checkActiveBooksThatWereNotSent(books) { 
     return checkActiveBooks(books).then(function(books){ 
      return books.filter(function(book){ 
        return checkSentBooks(book.book); 
       }); 
     }); 
} 

Стоит отметить, что Bluebird библиотека имеет вспомогательные методы для всего этого, как Promise#filter и Promise#map, что бы сделать этот код короче ,

Обратите внимание, что если checkSentBook является асинхронным вам нужно немного изменить код:

function checkActiveBooksThatWereNotSent(books) { 
     return checkActiveBooks(books).then(function(books){ 
      return Q.all(books.map(function(book){ // note the Q.all 
        return Q.all([book, checkSentBooks(book.book)]); 
       })).then(function(results){ 
        return results.filter(function(x){ return x[1]; }) 
            .map(function(x){ return x[0]; }); 
       }); 
     }); 
} 

Как я уже говорил, с различными библиотеками это будет выглядеть намного лучше. Вот как выглядит код в Bluebird, который также на два порядка быстрее и имеет хорошие следы стека и обнаружение необработанных отказов. Для забавы и славы я бросил стрелки ES6 и сокращенные свойства:

var request = Promise.promisify(require("request")); 

var checkActiveBooks = (books) => 
    Promise. 
    map(books, book => request("...&q=" + book.name).get(1)). 
    map(JSON.parse). 
    map(book => book.users.length ? 
     book.users.map(user => {user, books, book: book.name) : null)) 

var checkActiveBooksThatWereNotSent = (books) => 
    checkActiveBooks(books).filter(checkBookSent) 

Что я нахожу намного приятнее.