Code from the "Dependency Injection Basics" episode of Fun Fun Function

episode.js

const assert = require('assert')

function getAnimals(fetch, id) {
  return fetch('http://api.animalfarmgame.com/animals/' + id)
    .then(response => response.json())
    .then(data => data.results[0])
}

describe('getAnimals', () => {
  it('calls fetch with the correct url', () => {
    const fakeFetch = url => {
      assert(
        url ===
        'http://api.animalfarmgame.com/animals/123'
      )
      return new Promise(function(resolve) {

      })
    }
    getAnimals(fakeFetch, 123)
  })

  it('parses the response of fetch correctly', (done) => {
    const fakeFetch = () => {
      return Promise.resolve({
        json: () => Promise.resolve({
          results: [
            { name: 'fluffykins' }
          ]
        })
      })
    }
    getAnimals(fakeFetch, 12345)
      .then(result => {
        assert(result.name === 'fluffykins')
        done()
      })
  })
})

Thanks mustache uncle.

I rewrote the second test to be a little shorter leaving the original style and formatting.

• Replaced the done() with the return in the end of the it() test.
• Removed the redundant (second) Promise.resolve().
• Replaced the const fakeFetch with move novice friendly function fakeFetch syntax.
• Replaced the json: () => { with the shorter syntax version json() {

  it('parses the response of fetch correctly', () => {
    function fakeFetch() {
      return Promise.resolve({
        json() { return {
          results: [
            { name: 'fluffykins' }
          ]
        }}
      })
    }
    return getAnimals(fakeFetch, 12345)
      .then(result => assert(result.name === 'fluffykins'))
  })

Question to @mpj

I have this function which I want to unit test:

import boom from 'boom';
import {userDB, folderDB} from './db';
import {userAccess, fileStorage} from './somewhere';

function storeDocument(userID, folderLocation, fileStream) {
  return Promise.all([
    userDB.findOne(userID),
    folderDB.findOne(folderLocation)
  ])
  .then(([user, folder]) => userAccess.canWrite(user, folder))
  .then(allowed => {
    if (!allowed) return Promise.reject(boom.forbidden());
    return fileStorage.save(fileStream, {user, folder});
  });
}

The function just uploads the file to the folder. There are 4 dependencies: userDB, folderDB, userAccess, fileStorage.

According to your video the storeDocument() should accept 4 more arguments. Which would total 7 arguments to the same function.
In the application code I'd need to pass all 7 arguments every time to the storeDocument() function. Which is kind of a lot.

What would be your recommendations? How to deal with that?

@koresar You're not the first to suggest to remove the second promise.resolve. The code and test will still work in it's current form, but your mock is now behaving differently than the real thing would, which returns a promise.

@mpj and @koresar:

If i understand this code correctly:

import boom from 'boom';
import {userDB, folderDB} from './db';
import {userAccess, fileStorage} from './somewhere';

function storeDocument(userID, folderLocation, fileStream) {
  return Promise.all([
    userDB.findOne(userID),
    folderDB.findOne(folderLocation)
  ])
  .then(([user, folder]) => userAccess.canWrite(user, folder))
  .then(allowed => {
    if (!allowed) return Promise.reject(boom.forbidden());
    return fileStorage.save(fileStream, {user, folder});
  });
}

then boom.forbidden() is more or less a constant which we don't need to replace with a stub, we are actually very interested in exactly this result, right?

Other observations about the code is that we can use composition of the Promise Monad and write alot of it without needing Promises at all:

const throwForbiddenOnFalse = value => {
  if (value)
  {
    throw boom.forbidden();
  }
};

This part would be super simple to test...

Other things... that the original code does in the same function is fetching data from databases, looking up Access policies, and writing the data to a stream.

checking the access:

const checkAccess = userAccess => (user, folder) => userAccess.canWrite(user, folder)
    .then(throwForbiddenOnFalse)

I have used currying here to help with configuration, so that the user of the function at runtime don't need to know which userAccess is being used.

saving the actual file:

const saveFileStream = fileStorage => (fileStream, user, folder)  => fileStorage.save(fileStream, {user, folder});

combining them:

const checkAccsessAndSaveStream = (checkAccess, saveFileStream) => (user, folder, fileStream) =>
    checkAccess(user, folder).then(() => saveFileStream(fileStream, user, folder));

and the final product:

const fetchUserAndFolderThenStoreDocument = (userDB, folderDB, checkAccessAndSaveStream) =>
  (userID, folderLocation, fileStream) =>
    Promise.all([userDB.findOne(userID), folderDB.findOne(folderLocation)])
      .then(([user, folder]) => checkAccessAndSaveStream(user, folder, fileStream));

so the factory would look something like this:

const storeDocument = fetchUserAndFolderThenStoreDocument(
  userDB,
  folderDB,
  checkAccsessAndSaveStream(checkAccess, saveFileStream(fileStorage))
);

some of the interfaces feels klunky, and they could be a written abit better with reader monads and cofunctors, depending on how the two db objects are related.

Also note that the real factory would be simpler since it would be using other factories also that the tests would be simpler since they would only care about the dependencies one level down.

So the real benefit here is that the checkAccsessAndSaveStream function don't need to know about the databases or the fileStorage. You could say that these dependensies juped over the parents of the user and made them simpler.

I also have the feeling that there would be even more code that could have been written without needing to know that it was async.

what do you think?

I did some more thingking and this fileStorage.save(fileStream, {user, folder}) strikes me as a bit odd. why do the filesStorage need the user and folder. Is it somehow part of the location that the fileStreamwill bes saved at?

Also there is some asymmetri here. storeDocument gets raw data as arguments, except for the stream object. but the access method userAccess.canWrite(user, folder) takes full-blown object. there might be some answers hidden in this odd relationship.