3
\$\begingroup\$

I just finished a simple script for Node that will delete all the folders and files inside a specified folder ID through Box's API. It works by getting a list of items inside a folder, then generating a list of File IDs and Folder IDs, then making parallel calls to Box's delete file and delete folder endpoints.

This script is currently working, it'll delete everything inside a folder I specified. However, I'm not confident about my understandings in regards to Promises, as well as dealing with the response returned from an API call.

Can you please give me some feedback on the way I'm using async/await, promises, and the way I'm mapping certain arrays? perhaps make it more elegant and follow best practices?

Things to do:

  1. Refactor Axios auth header and baseUrl into config to reduce repeated code

Few Questions I have:

  1. On line 78, why don't I need to use .then() after .all()? A: found the answer, await replaces .then()
  2. Is there a way to combine line 78 & line 79 into one statement?
  3. Am I using async/await properly and following best practices?
  4. My javascript is kind of rusty, my background is mostly C#, are there any structural changes I should make to the whole script?

const axios = require('axios');

const BOX_TOKEN = '1111';
const DOC_FOLDERID = '1111';

(async function() {
    let allItems = await getAllFolderItems();
    let filteredItems = await filterItems(allItems);
    deleteFiles(filteredItems[0]);    
    deleteFolders(filteredItems[1]);
})();

async function getFolderItems(folderId, nextMarker) {
    let limit = 1;
    let responseData = await axios({
        method: 'get',
        url: `https://api.box.com/2.0/folders/${folderId}/items?fields=id,name,content_created_at,type&limit=${limit}&usemarker=true&marker=${nextMarker}`,
        headers: {'Authorization': `Bearer ${BOX_TOKEN}`}   
    })
    .then(response => { return response.data })
    .catch(error => { return error });
    return responseData;
}

async function getAllFolderItems() {
    let allItems = [], keepGoing = true, nextMarker = '';

    // Recursive call to get all the items in a large folder
    while(keepGoing) {
        let folderItems = await getFolderItems(DOC_FOLDERID, nextMarker);
        if (folderItems.entries)
            allItems.push(...folderItems.entries);
        if (folderItems.next_marker != '' && folderItems.next_marker != undefined) {
            nextMarker = folderItems.next_marker;
            keepGoing = true;
        } else 
            keepGoing = false;
    }

    console.log(allItems);
    return allItems;
}

async function filterItems(allItems) {
    let folderIDsToDelete = {}, fileIDsToDelete = {};

    // Compile a list of folders and list of files if they are older than 1 day
    // Goal is to only delete items that are more than 1 day old
    for (let i = 0; i < allItems.length; i++) {
        let oneDayAgo = new Date().getTime() - (1 * 24 * 60 * 60 * 1000);
        let itemDate = new Date(allItems[i].content_created_at);
        if (oneDayAgo > itemDate) {
            if (allItems[i].type === 'file')
                fileIDsToDelete[allItems[i].id] = allItems[i].content_created_at;
            else
                folderIDsToDelete[allItems[i].id] = allItems[i].content_created_at;
        }
    }
    console.log('Files to Delete ', fileIDsToDelete);
    console.log('Folders to Delete ', folderIDsToDelete);

    return [fileIDsToDelete, folderIDsToDelete];
}

async function deleteFiles(fileIDsToDelete) {
    let baseUrl = 'https://api.box.com/2.0/files/';
    let requestUrls = Object.keys(fileIDsToDelete).map(key => baseUrl + key);
    console.log(requestUrls);

    var promises = [];
    for (let request of requestUrls) {
        promises.push(axios.delete(request, {
            headers: {'Authorization': `Bearer ${BOX_TOKEN}`}
        }));
    }

    try {
        var responses = await axios.all(promises);
        var responseStatuses = responses.map(responses => responses.status);
        console.log(responseStatuses);
    } catch (error) {
        console.log(error);
    }
}

async function deleteFolders(folderIDsToDelete) {
    let baseUrl = 'https://api.box.com/2.0/folders/';
    let queryParam = '?recursive=true';
    let requestUrls = Object.keys(folderIDsToDelete).map(key => baseUrl + key + queryParam);
    console.log(requestUrls);

    var promises = [];
    for (let request of requestUrls) {
        promises.push(axios.delete(request, {
            headers: {'Authorization': `Bearer ${BOX_TOKEN}`}
        }));
    }

    try {
        var responses = await axios.all(promises);
        var responseStatuses = responses.map(responses => responses.status);
        console.log(responseStatuses);
    } catch (error) {
        console.log(error);
    }
}

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$
  1. Am I using async/await properly and following best practices?

Given how you responded to your own question #1 (i.e. "A: found the answer, await replaces .then()") I feel like you could keep going with this. For instance, the function getFolderItems could be updated similarly:

async function getFolderItems(folderId, nextMarker) {
    let limit = 1;
    let responseData = await axios({
        method: 'get',
        url: `https://api.box.com/2.0/folders/${folderId}/items?fields=id,name,content_created_at,type&limit=${limit}&usemarker=true&marker=${nextMarker}`,
        headers: {'Authorization': `Bearer ${BOX_TOKEN}`}   
    })
    .then(response => { return response.data })
    .catch(error => { return error });
    return responseData;
}

The .then() callback feels awkward here, given await is used. You could use a try/catch block instead of the promise-oriented approach. Something like this:

async function getFolderItems(folderId, nextMarker) {
    const limit = 1;
    try {
        const response = await axios({
            method: 'get',
            url: `https://api.box.com/2.0/folders/${folderId}/items?fields=id,name,content_created_at,type&limit=${limit}&usemarker=true&marker=${nextMarker}`,
            headers: {'Authorization': `Bearer ${BOX_TOKEN}`}   
        });
        return response.data; 
    }
    catch (error) {
        return error;
    }
}

  1. My javascript is kind of rusty, my background is mostly C#, are there any structural changes I should make to the whole script?

Well, after a cursory glance I don't see any major structural changes that I would recommend, however I would recommend using const instead of let for any variable that doesn't get re-assigned (as you may have noticed in the re-written sample above) - like limit, responseData, as well as arrays like allItems in getAllFolderItems(), folderIDsToDelete and fileIDsToDelete in filterItems(), since pushing elements into an array does not reassign the value. This will avoid accidental re-assignment.

Why is promises declared with the var keyword in deleteFolders()? While it works, it could be declared with const to avoid accidental re-assignment...

Also, the baseUrl should be a constant, perhaps in all capitals since it doesn't change, and move it to the top with the other pre-defined constants like BOX_TOKEN and DOC_FOLDERID. That way, if you need to update that value, you don't have to hunt for it in the code. Optionally, those constants could be stored in a separate config/environment file.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Thank you so much, these are exactly what I was looking for. I'll take these into consideration for my future code. \$\endgroup\$ Commented Mar 28, 2019 at 22:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.