Search code examples
javascriptnode.jsasynchronousrecursiones6-promise

How to use promises in recursive function


I'm doing a task in which I have to implement analyze function. The analyze function should gather information about a folder and its content. The function should return an object that will have the following format.

{
    totalFiles: number; // Contains total count of files in the folder and all its sub-folders.
    totalSubFolders: number; // Contains total count of subfolders in the folder and all its sub-folders.
    fileTypesInformation: FileTypeInformation[]; // Contains an array of FileTypeInformation objects that are described below.
}

The FileTypeInformation object type has the following format.

{
    fileExtension: string; // Contains file extension. This should be unique in the whole fileTypesInformation array.
    fileCount: number; // Contains total count of files with such extension in the folder and all its sub-folders.
}

Create a file analyze.js that will export a function that implements expected behavior. The callback function passed to the analyze function should be called with the result object that has the format described in the beginning of the task. The first parameter of the callback will be an error object in case any occurred, the second parameter will be result. Keep in mind the node.js callback pattern.

This is code that checks this task:

const path = require('path');
const analyze = require('../src/analyze');
const { expect } = require('chai');
const sinon = require('sinon');

const testAssetsPath = path.resolve(__dirname, 'assets')

describe('analyze(folderPath, callback)', () => {
  it('should return an error in case a path to invalid folder was provided', async () => {
    const callbackStub = sinon.stub();
    const promise = new Promise(resolve => {
      callbackStub.callsFake(() => resolve());
    });

    analyze('/unexistedfolderblab-blab', callbackStub);

    await promise;

    expect(callbackStub.calledOnce).to.be.true;

    const callArguments = callbackStub.getCall(0).args;
    expect(callArguments[0]).to.be.instanceOf(Error);
    expect(callArguments[1]).to.be.equal(undefined);
  });

  let result;
  const callbackStub = sinon.stub();

  before(async () => {
    result = await new Promise((resolve) => {
      callbackStub.callsFake(() => {
        resolve();
      })

      analyze(testAssetsPath, callbackStub);
    });
  })

  it('should call callback function once', () => {
    expect(callbackStub.calledOnce).to.be.true;
  })

  it('should pass null as an error in case of a success', () => {
    expect(callbackStub.getCall(0).args[0]).to.be.equal(null);
  });

  it('should pass an object that has totalFiles number property', () => {
    expect(callbackStub.getCall(0).args[1]).to.has.property('totalFiles');
    expect(typeof callbackStub.getCall(0).args[1].totalFiles).to.be.equal('number');
  });

  it('should pass an object that has totalSubFolders number property', () => {
    expect(callbackStub.getCall(0).args[1]).to.has.property('totalSubFolders');
    expect(callbackStub.getCall(0).args[1].totalSubFolders).to.be.an('number');
  });

  it('should pass an object that has fileTypesInformation array property', () => {
    expect(callbackStub.getCall(0).args[1]).to.has.property('fileTypesInformation');
    expect(callbackStub.getCall(0).args[1].fileTypesInformation).to.be.instanceOf(Array);
  });

  it('should pass correct format of objects for fileTypesInformation array', () => {
    for (const fileTypeInfo of callbackStub.getCall(0).args[1].fileTypesInformation) {
      expect(fileTypeInfo).to.has.all.keys('fileExtension', 'fileCount');
      expect(fileTypeInfo.fileExtension).to.be.an('string');
      expect(fileTypeInfo.fileCount).to.be.an('number');
    }
  });

  it('should pass correct totalFiles value', () => {
    expect(callbackStub.getCall(0).args[1].totalFiles).to.be.equal(10);
  });

  it('should pass correct totalSubFolders value', () => {
    expect(callbackStub.getCall(0).args[1].totalSubFolders).to.be.equal(4);
  });

  it('should pass fileTypesInformation array that has correct information about extension', () => {
    expect(callbackStub.getCall(0).args[1].fileTypesInformation).to.have.deep.members([
      {
        fileExtension: '.logs',
        fileCount: 1,
      },
      {
        fileExtension: '.errors',
        fileCount: 1,
      },
      {
        fileExtension: '.html',
        fileCount: 3,
      },
      {
        fileExtension: '.json',
        fileCount: 1,
      },
      {
        fileExtension: '.jpeg',
        fileCount: 2,
      },
      {
        fileExtension: '.css',
        fileCount: 2,
      }
    ],
      'It should has the following information about files: .logs - 1, .errors - 1, .html - 3, .json - 1, .jpeg - 2, .css - 2'
    );
  })
});

This was my implementation:

/**
 * @param folderPath: {String}
 * @param callback: {Function}
 */
const fs = require("fs");

class FileTypeInformation {
  constructor(fileExtension, fileCount) {
    this.fileExtension = fileExtension;
    this.fileCount = fileCount;
  }
}

function analyze(folderPath, callback) {
  let result = {
    totalFiles: 0,
    totalSubFolders: 0,
    fileTypesInformation: []
  };

  function readDirectory(path) {
    fs.readdir(path, (err, files) => {
      if (err) {
        throw err;
      }

      let fileCount = 0;
      let subFolderCount = 0;
      let fileTypeInformation = {};

      files.forEach((name) => {
        const filePath = `${path}/${name}`;
        const stats = fs.statSync(filePath);

        if (stats.isDirectory()) {
          subFolderCount++;
          readDirectory(filePath);
        } else {
          fileCount++;
          const fileExtension = `.${name.split(".").pop().toLowerCase()}`;
          fileTypeInformation[fileExtension] =
            (fileTypeInformation[fileExtension] || 0) + 1;
        }
      });

      result.totalFiles += fileCount;
      result.totalSubFolders += subFolderCount;

      Object.entries(fileTypeInformation).forEach(([fileExtension, fileCount]) => {
        const fileType = new FileTypeInformation(fileExtension, fileCount);
        result.fileTypesInformation.push(fileType);
      });

      
    });

  }

  readDirectory(folderPath);
  
  setTimeout(() => {
    for (let i = 0; i < result.fileTypesInformation.length; i++){
        for (let j = 0; j < result.fileTypesInformation.length; j++) {
            if(result.fileTypesInformation[i].fileExtension === result.fileTypesInformation[j].fileExtension && i !== j){
                result.fileTypesInformation[i].fileCount += result.fileTypesInformation[j].fileCount;
                result.fileTypesInformation.splice(j,1);
            }
        }
      }
    callback(null,result);
  }, 4000);
}

module.exports = analyze;

Failure message:

Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/tmp/builds/autocode_11661/test/analyze.js)

I think It should be solved with promises but I am not sure how to implement promises in recursive functions, I can show my implementation with promises also but it does not work as intended.


Solution

  • Since the tests must complete in 2 seconds and you have implemented a delay 4 seconds before calling the callback, it should come as no surprise that you get a timeout error.

    Instead you should call the callback as soon as you have the result ready. It is probably easiest to use a promise-enabled library.

    The fs library which use use has promise enabled equivalents:

    import * as fs from 'node:fs/promises';
    

    Then readdir returns a promise. Your function can then change to an async function, using await and Promise.all

      async function readDirectory(path) {
        const files = await fs.readdir(path);
        let fileCount = 0;
        let subFolderCount = 0;
        let fileTypeInformation = {};
        await Promise.all(files.map(async (name) => {
          const filePath = `${path}/${name}`;
          const stats = fs.statSync(filePath);
          if (stats.isDirectory()) {
            subFolderCount++;
            await readDirectory(filePath);
          } else {
            fileCount++;
            const fileExtension = `.${name.split(".").pop().toLowerCase()}`;
            fileTypeInformation[fileExtension] =
              (fileTypeInformation[fileExtension] || 0) + 1;
          }
        }));
    
        result.totalFiles += fileCount;
        result.totalSubFolders += subFolderCount;
    
        Object.entries(fileTypeInformation).forEach(([fileExtension, fileCount]) => {
          const fileType = new FileTypeInformation(fileExtension, fileCount);
          result.fileTypesInformation.push(fileType);
        });
      }
    
      readDirectory(folderPath).then(() => callback(null,result));
    

    That entire setTimeout call can be removed. Those for loops are not needed. They assume that the collected file types could have duplicate file extensions, but that is not possible, because they were keys of an object (fileTypeInformation). Furthermore, doing a splice on an array you are iterating is asking for trouble.