Async db repository

Question

This is a simple repository where I want to save all the invoices of some queried users. I wanted this module to only expose a simple saveInvoices method and return a Promise.

As a DB, I use Firebase whose API is async but does not return standard JS promises so I promisify the db queries.

I haven't yet taken every reject and fail scenarios into account. Looking forward to read your feedback.

const _ = require('lodash');
const Firebase = require('firebase');
const Config = require('./../../shared/config');
const Repository = (function() {
 
 let _conn;
 const _connect = function() {
 return new Promise((resolve, reject) => {
 _conn = new Firebase(Config.FIREBASESERVER);
 _conn.authWithCustomToken(Config.FIREBASESECRET, () => {
 resolve();
 });
 });
 };
 const _getQueuedUsers = function() {
 return new Promise((resolve, reject) => { 
 _conn.child('users')
 .orderByChild('nextInvoiceDate')
 .startAt(1)
 .endAt(Date.now())
 .once('value', (usersSnap) => {
 resolve(_.values(usersSnap.val()));
 });
 });
 };
 const _saveInvoice = function(user) {
 return new Promise((resolve, reject) => {
 // ...
 resolve();
 });
 };
 const saveInvoices = function() {
 return new Promise((resolve, reject) => {
 _connect()
 .then(() => _getQueuedUsers())
 .then((users) => {
 resolve(Promise.all(users.map((u) => _saveInvoice(u))));
 }, (rejection) => {
 console.log(rejection);
 });
 });
 };
 return { 
 saveInvoices 
 }
})();
Repository.saveInvoices
 .then(() => { console.log('done'); })
 .catch((err) => { console.error(err); });

Answer 1

I see you're using modules. Consider moving Repository to its own module. That way, you can take advantage of encapsulation inside a module file and remove the need for an IIFE. You can then export only the APIs you want exposed. I even don't use _ anymore to mark private functions as export is a good enough indicator that it is exposed outside the module.

// repository.js
const _ = require('lodash');
const Firebase = require('firebase');
const Config = require('./../../shared/config');
let conn = null;
const connect = function() {
 return new Promise((resolve, reject) => {
 conn = new Firebase(Config.FIREBASESERVER);
 conn.authWithCustomToken(Config.FIREBASESECRET, () => resolve());
 });
};
const getQueuedUsers = function() {
 return new Promise((resolve, reject) => { 
 conn.child('users')
 .orderByChild('nextInvoiceDate')
 .startAt(1)
 .endAt(Date.now())
 .once('value', usersSnap => resolve(_.values(usersSnap.val())));
 });
};
const saveInvoice = function(user) {
 return new Promise((resolve, reject) => {
 // ...
 resolve();
 });
};
export function saveInvoices() {
 return new Promise((resolve, reject) => {
 connect()
 .then(() => getQueuedUsers())
 .then(
 users => resolve(Promise.all(users.map(u => saveInvoice(u)))),
 rejection => console.log(rejection)
 );
 });
};
// your-code.js
import * as Repository from 'repository.js';
Repository.saveInvoices
 .then(() => console.log('done'))
 .catch(err => console.error(err));

A few other things include the optional () around arguments of arrow functions only when the arguments is just one (() required for none or multiple). {} is also optional if you just do one-liner arrow function bodies.

Also, your configs are in a file? I suggest you use environment variables instead, especially when you're dealing with API keys. Config files are easily accidentally checked-in into the repo and we all know Git doesn't forget anything that's checked in. You wouldn't want anyone using an API under your name for malicious purposes.

Other than that, the code looks ok!

• teebot: Thank you Joseph. Really insightful! Avoiding the IIFE makes it more readable. By the way, isn't there a better way of returning a Promise in saveInvoices than resolve(Promise.all(users.map(u => saveInvoice(u)))) ?
• Joseph: @teebot I'm not really sure what saveInvoice does nor am I familiar with the Firebase API if they have a bulk operation. If they do, you could just create one promise that resolves when all entries are saved instead of a promise each entry that gets listened by all.