0

The problem:

I want to keep track of my uploaded files by writing the fileinformation each uploaded file for a multi upload into my database. However when I upload 2 files it usually creates 3 entries in the database and when I upload 6 files it will create a lot more than 6 entries.

My db function:

function saveAssetInDatabase(project, fileInformation) {
    return new Promise((reject, resolve) => {
        let uploaded_file = {}
        uploaded_file = fileInformation
        uploaded_file.file_type = 'asset'
        uploaded_file.display_name = fileInformation.originalname
        project.uploaded_files.push(uploaded_file)
        project.save()
    })
}

The simplified code which calls the function:

for(var i=0; i<req.files["sourceStrings"].length; i++) {
    // Unknown file format, let's save it as asset
    saveAssetInDatabase(project, fileInformation).then(result => {
        return res.status(200).send()
    }).catch(err => {
        logger.error(err)
        return res.status(500).send()
    })
}

I guess that there is something wrong with my db function as it leads to duplicate file entries. What am I doing wrong here? One file should get one entry.

3
  • 1
    What's the point of using req.files.sourceStrings as a loop condition but not use it within the loop itself? You're always saving the same asset right now Commented Jul 1, 2017 at 20:23
  • Be sure your req.files['sourseStrings'] has no dublicates? Commented Jul 1, 2017 at 20:27
  • @nem035 it is used but this is just a simplified version. I cut it down to the relevant part. req.files.sourceStrings is used for creating fileInformation Commented Jul 1, 2017 at 20:47

2 Answers 2

1

If I read the specs on model.save correctly on the mongoose website, the problem with your save is rather that you are always reusing the original project, and not the newly saved project that should contain the latest state.

So what you are essentially are doing:

project.files.push(file1);
// file1 is marked as new
project.save();
project.files.push(file2);
// file1 & file2 are marked as new 
// (the project doesn't know file1 has been saved already)
// ...

Now, that actually brings quite some advantages, since you are doing currently a save per file, while you could save all files at once ;)

I guess the easiest way would be to put the project.save method outside of your for loop and change your first method like

function saveAssetInDatabase(project, fileInformation) {
    let uploaded_file = {};
    uploaded_file = fileInformation;
    uploaded_file.file_type = 'asset';
    uploaded_file.display_name = fileInformation.originalname;
    project.uploaded_files.push(uploaded_file);
}

with the for loop changed to

function saveSourceString(project, req) {
    for(var i=0; i<req.files["sourceStrings"].length; i++) {
        // Unknown file format, let's save it as asset
        saveAssetInDatabase(project, fileInformation);
    }
    // save after all files were added
    return project.save().then(result => {
        return res.status(200).send()
      }).catch(err => {
        logger.error(err)
        return res.status(500).send()
      });
}

Note that project.save() will return a promise, with an argument containing the newly saved project. If you wish to manipulate this object at a later time, make sure you take the saved file, and not, as you have done till now, the non-saved model

Sign up to request clarification or add additional context in comments.

Comments

1

Problem

Each time in your for loop create a promise then send your that time project object. It's not correct way. Each promise resolved you have send project object to DB then store it.

For example you have 3 asset details. While first time loop running first asset data would store in project object and the promise resolved you have send that time project in your DB store it. This time project object has first asset details.

While second time loop running second asset data would store in project object with first asset data and the promise resolved you have send that time project in your DB store it. This time project object has first and second asset details.

While third time loop running third asset data would store in project object with first and second asset data and the promise resolved you have send that time project in your DB store it. This time project object has first, second and third asset details.

So you have store same datas in your DB.

Solution

You have use Promise.all. Resolve all assest's promise after you store your project data in your DB.

    // your DB function
    function saveAssetInDatabase(project, fileInformation) {
        return new Promise((resolve, reject) => {
            let uploaded_file = {}
            uploaded_file = fileInformation
            uploaded_file.file_type = 'asset'
            uploaded_file.display_name = fileInformation.originalname
            project.uploaded_files.push(uploaded_file)
            project.save();
            resolve();
        })
    }

    // calls function

    let promiseArray = [];
    for(var i=0; i<req.files["sourceStrings"].length; i++) {
        promiseArray.push(saveAssetInDatabase(project, fileInformation));
    }

    Promise.all(promiseArray).then(result => {
        return res.status(200).send();
    }).catch(err => {
        logger.error(err)
        return res.status(500).send()
    })
}

7 Comments

promiseArray.(saveAssetInDatabase(project, fileInformation)); is that a typo or is this some kind of ES6 syntax for .push?
It executes fine, but the problem is still existent - just like before. Thanks for the Promise.all hint though, you are right that I should use this
Also I don't think that I need to do resolve() in my saveAssetInDatabase as it's a mongoose promise already. It would reject automatically in case it succeeds and rejects in case it fails, no?!
How mongoose promise rejects your saveAssetInDatabase promise?
I think it would reject if it throws an exception during project.save()
|

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.