Update with saveAll

[nfantone]

I wasn't able to get this to work. I was able, however, to gather some bits from:

• Random unanswered questions on the internet
• Some issues in this repository
• ...and from comments like this one here

It seems like a common issue. Basically, what I'm trying to do is just mimic the merge example on the docs, while also saving all the joined documents in the process.

Ideally, it would look something like this:

var data = req.body; // Data posted by the user
Post.get(data.id).run().then(function(post) {
    post.merge(data).saveAll().then(function(result) {
        // post was updated with `data`
    });
});

Just retrieve what the client sent in the request, merge docs and update everything. This is a very typical scenario in PUT requests. How would I go about doing that without ending in a Duplicate primary key 'id' error?

[nfantone]

Following the advice of the comment I have referenced previously, I've managed to update some fields on the doc by manually setting them, like so:

var data = req.body; // Data posted by the user
Post.get(data.id).run().then(function(post) {
    post.creationTime = req.body.creationTime;
    post.saveAll().then(function(result) {
        // post was updated with `data`
    });
});

But this seems like it doesn't scale very well. It's almost as if I had to roll out my own merge function by replacing every updateable field with its new value, mutating the original object. Weirdly enough, lodash _.extend or _.assign don't work in this scenario.

---

EDIT: lodash doesn't work because it's extending the array properties, and thus just providing a new array of elements, instead of mutating contents.

[nfantone]

This is becoming a pain. Trying to keep original references in arrays of joined documents while merging them is no easy task. Also, saveAll doesn't seem to handle adding new elements (creates document in table, but ends up in a Document failed validation error somewhere along the way) or removing them all correctly.

Let me bring this down to Earth a little bit. Say I have tables called dashboard and metrics linked by a Dashboard.hasMany(Metric, 'metrics', 'id', 'reportId'); association. I now attempt to update the following dashboard via PUT requests:

{
  "id": "dca0401d-6329-41e0-bd37-ef0446113ef4",
  "createdAt": "2015-09-27T05:56:55.114Z",
  "dashboard": true,
  "metrics": [
    {
      "id": "7efa3f1e-0cbd-40e2-8607-d53eaa46b916",
      "index": 0,
      "main": false,
      "type": {
        "codes": {
          "name": "label.totalRevenue",
          "short": "label.totalRevenueShort"
        },
        "id": "total_revenue",
        "name": "Total revenue"
      },
      "typeId": "total_revenue"
     }
   ]
}

I want all the following cases to work:

1. Update properties on dashboard. For instance, update "dashboard" boolean value to false. Every other property is ignored and is left as is.

PUT /dashboard
{
  "id": "dca0401d-6329-41e0-bd37-ef0446113ef4",
  "dashboard": false
}

2. Update specific metric inside the array. Here, it should change "index" and "main" values on metric "7efa3f1e-0cbd-40e2-8607-d53eaa46b916"

PUT /dashboard

{
  "id": "dca0401d-6329-41e0-bd37-ef0446113ef4",
  "metrics": [
    {
      "id": "7efa3f1e-0cbd-40e2-8607-d53eaa46b916",
      "index": 1,
      "main": true
    }
   ]
}

3. Remove a metric from the array. In the example below, it removes every metric on the relationship.

PUT /dashboard

{
  "id": "dca0401d-6329-41e0-bd37-ef0446113ef4",
  "metrics": [  ]
}

4. Add a new metric to the array. Note the lack of an "id" property on the element.

PUT /dashboard
{
  "id": "dca0401d-6329-41e0-bd37-ef0446113ef4",
  "metrics": [
 {
      "id": "7efa3f1e-0cbd-40e2-8607-d53eaa46b916",
      "index": 0,
      "main": false,
      "type": {
        "codes": {
          "name": "label.totalRevenue",
          "short": "label.totalRevenueShort"
        },
        "id": "total_revenue",
        "name": "Total revenue"
      },
      "typeId": "total_revenue"
     },
   {
      "index": 1,
      "main": true,
      "typeId": "total_revenue"
    }
 ]
}

5. Any combination of the above. For example, this request removes a metric, creates a new one, and updates the "dashboard" boolean.

PUT /dashboard
{
  "id": "dca0401d-6329-41e0-bd37-ef0446113ef4",
  "dashboard": false,
  "metrics": [
   {
      "index": 1,
      "main": true,
      "typeId": "total_revenue"
    }
 ]
}

@neumino If you could provide a working example for this, it would be swell. Or at least shed some light on this?

[neumino]

Quick answers first: 1.

Dashboard.get('dca0401d-6329-41e0-bd37-ef0446113ef4').update({dashboad: false}).run()

Metric.get('7efa3f1e-0cbd-40e2-8607-d53eaa46b916').update({index: 1, main: true}).run()

Dashboard.get('dca0401d-6329-41e0-bd37-ef0446113ef4').removeRelation('metrics')

Remove the metrics already saved, and just call saveAll({metrics: true})

1. Work out with all the previous cases.

Here is why things don't work well in your case:

• Thinky doesn't make the assumption that if you provide the id, the document is saved. Some people explicitly set the id of their document.
• merge on a document wasn't mean to update relations. That being said, it's doable-ish. What I'm not sure is how you merge an array with an another. RethinkDB's merge just blows the whole thing. Do we just add new documents (based on the primary key?
• You are technically not building a REST API. REST uses PUT/POST to differentiate between creating and updating a document. All the pieces you should need are
  • save to save a new document
  • get(...).update(...) to update a document
  • get(...).addRelation/removeRelation(...) to add/remove relations

Tagging as feature to have merge handle joined documents.

[nfantone]

@neumino Thank you very much for your response.

Let me flesh out some things:

1. While your quick answers work (except for number 4: more on that below), what I was a looking for is a command or small set commands that will accomplish merging and updating, much like the proposed doc.merge(anotherDoc).saveAll() in a "tidy" way. Your suggestions imply that I need to analyze the payload and execute different queries on different inputs (ie.: check if metrics array is empty, before calling removeRelations; or query db for each element on the array). That quickly becomes a nested mess of combinations and possibilities if I wanted to contemplate each case on arbitrary payloads (think of array of arrays of arrays).

Thinky doesn't make the assumption that if you provide the id, the document is saved. Some people explicitly set the id of their document.

Got that. But if I provide a primary key and it is already on the table, it should update it. And if I don't, it should create a new one. This is what I expect to happen in my scenario, at least.

merge on a document wasn't mean to update relations. That being said, it's doable-ish. What I'm not sure is how you merge an array with an another.

What I ended up doing, given my original array and the incoming payload, is mutating the array in the original document in such a way that:

• For each element on it, check if it also exists on the payload with the same pk. If it does, merge it.
• If it doesn't, remove it.
• Any element on the payload that is not in the original, push it. Those will become new documents.

You are technically not building a REST API. REST uses PUT/POST to differentiate between creating and updating a document.

Well, this is rather debatable. In our design, Metric does not exist beyond a Dashboard, so it doesn't have its own endpoints. Removing or creating metrics is really updating a dashboard. Conceptually, at least. I might be willing to separate those concerns if it makes my life any easier.

5. get(...).addRelation/removeRelation(...)

How does that work for new relations? That is, adding a new element to the joined array.

Remove the metrics already saved, and just call saveAll({metrics: true})

Doing that throws the aforementioned Document failed validation error. I can see it inserts new documents on the table and I can get them back with a get(...).getJoin(), but the saveAll({metrics:true}) promise is rejected. It complains about a missing reportId property, which serves as the fk on the relationship. Adding it manually, doesn't make a difference.

[grantcarthew]

Hi Michel @neumino ,

So I came across this issue today and have had to make this frankencode to get around it. Either I don't know what I am doing, or this is the best it gets.

To understand the code below, the user is saving an activity and can add or remove pictures from it.

I suspected that saveAll() would update as stated here. Instead I was getting the duplicate id errors.

I will comment in the code so you can see what I am doing.

  return Promise.each(req.body.Pictures, reqPicture => {
    // All pictures already exist in the database, the user
    // selected pictures need a relationship.
    return modelActivity
      .get(req.body.id)
      .addRelation('Pictures', { id: reqPicture.id })
  }).then(() => {
    // Get all pictures with a relationship
    return modelActivity.get(req.body.id).getJoin().run()
  }).then(activity => {
    // Filter only the pictures which have been removed
    return activity.Pictures.filter(dbPicture => {
      return req.body.Pictures.every(reqPicture => {
        return dbPicture.id !== reqPicture.id
      })
    })
  }).then(picturesToRemove => {
    // Remove the old relationship
    return Promise.each(picturesToRemove, removePicture => {
      return modelActivity
        .get(req.body.id)
        .removeRelation('Pictures', { id: removePicture.id })
    })
  }).then(() => {
    // Finally save any other fields changed by the user
    return modelActivity.save(req.body, {conflict: 'update'})
  }).then(result => {
    return sendData(res, services.Api.Success, result, module)
  }).catch(err => {
    return next(err)
  })

If I am correct and I have the same issue that @nfantone has, then an option like saveAll('clobber') or similar would be helpful.

Am I way off the mark?

[neumino]

Does that work?

picture_ids = [];
for(var i=0; i<req.body.Pictures.length; i++) {
  picture_ids.push(req.body.Pictures.id);
}
modelActivity.get(req.body.id).getJoin().run().then(function(activity) {
  this.activity = activity;
  return modelPictures.getAll([picture_ids]).run()
}).then(function(pictures) {
  this.activity.pictures = pictures;
  this.activity.saveAll({pictures: true})
})

[grantcarthew]

Thanks for the reply. I see what you are getting at, I need to get the picture documents, not just JSON data. This might work, although the getAll(array) is reporting a primary key too long. Does not seem to like an array for ids. I have removed the extra array brackets.

[grantcarthew]

I used getAll.apply(model, array), doesn't save the relationships though.

[grantcarthew]

Ah, found it. Case issue on the saveAll({ Pictures: true }) This is great Michel. Every time I use Thinky I learn more. You have helped me often. I don't know how you do it.

Thank you very much.

[grantcarthew]

OK, another issue with saveAll() Michel. @neumino

Your above solution worked until I added in the second relationship. The Activity model has the following relationships;

modelActivity.hasMany(modelFile, 'Pictures', 'id', 'ActivityId-Pictures')
modelActivity.hasMany(modelFile, 'Attachments', 'id', 'ActivityId-Attachments')

So if I get the current Activity from the database joined with options { Pictures: true, Attachments: true }, then get the Pictures and Attachments as Models from the database. Finally add the Pictures and Attachments to the Activity model, then saveAll({ Pictures: true, Attachments: true }), it saves the pictures and attachments, but will not delete removed items.

I have a workaround by getting the Activity with getJoin({ Pictures: true }), then getting the pictures, adding them, then saveAll({ Pictures: true }). This works for the pictures, but I have to repeat the process for the attachments. So I am getting and saving twice, once for pictures, once for attachments.

Here is some code to show you what I could get working. Is this expected behavior?

const Promise = require('bluebird')
const models = require('../../models')
const modelActivity = models.Activity
const modelFile = models.File
const services = require('../../services')
const sendData = services.Api.SendData

const getActivityFiles = function (files) {
  return Promise.map(files, file => {
    return file.id
  }).then(fileIds => {
    if (fileIds.length > 0) {
      return modelFile.getAll.apply(modelFile, fileIds).run()
    }
    return []
  })
}

module.exports = function (req, res, next) {
  req.body.DateModified = new Date().toString()

  modelActivity.get(req.body.id).getJoin({ Pictures: true }).run()
  .then(activityWithPictures => {
    return activityWithPictures.merge(req.body)
  }).then(updatedActivity => {
    return getActivityFiles(req.body.Pictures).then(newPictures => {
      updatedActivity.Pictures = newPictures
      return updatedActivity.saveAll({ Pictures: true })
    })
  }).then(() => {
    return modelActivity.get(req.body.id).getJoin({ Attachments: true }).run()
  }).then(activityWithAttachments => {
    return getActivityFiles(req.body.Attachments).then(newAttachments => {
      activityWithAttachments.Attachments = newAttachments
      activityWithAttachments.saveAll({ Attachments: true })
    })
  }).then(result => {
    return sendData(res, services.Api.Success, result, module)
  }).catch(err => {
    next(err)
    return null
  })
}

Again, great work and thanks for all your help.

[neumino]

The steps should be:

1. Get the whole activity with pictures and attachement
2. Fetch all the pictures you want to keep
3. Fetch all the attachements you want to keep
4. Merge what you fetched in 2 and 3 in the activity
5. Save the activity

[grantcarthew]

Yes that's what I did. It didn't work. It would save the additions of the child relationships, but not the deletions.

[djeeg]

To handle my use-case of multi-level model nesting, I did this note: this does not yet handle all the operations I would like, just the ones I need currently (ie not deleting items from the lists)

Model relationships

COUNTRY
--COUNTRY.id
--COUNTRY.name
--COUNTRY.states
----STATE.id
----STATE.name
----STATE.cities
------CITY.id
------CITY.name
----STATE.rivers
------RIVER.id
------RIVER.name

Helper save method

function saveCountry(postdata) {
    COUNTRY.get(data.id).getJoin(COUNTRY_objecttree).run().then(function(country) {
        mergeAndCombineNestedObjectsWithMatchingIds(country, postdata);
        return country.saveAll(COUNTRY_objecttree).then(function(result) {
            console.log("[saveCountry]updated: " + data.id)
            return Promise.resolve(0);
        });

Deep merge, combining items with the same IDs

function mergeAndCombineNestedObjectsWithMatchingIds(primary, delta) {
    for(var i in primary) { //for all properties on the primary
        if(!primary.hasOwnProperty(i)){
            //skip non-data property
        }
        if(i == "id") { //skip id property, it has special relationship handling
            continue;
        }
        if(i.indexOf("_id") != -1) { //skip _id reference properties, these should be mapped via the object relationship
            continue;
        }
        if(delta[i]) { //does this property exist in the change object
            if (primary[i] instanceof Array) { //is the object type a list
                //loop over the PRIMARY list (left)
                primary[i].map(function (nestedprimary) {
                    var nesteddelta = delta[i].filter(function (nesteddelta) {
                        return nesteddelta.id == nestedprimary.id;
                    })
                    if(nesteddelta[0]) { //found a nested object in DELTA list with the same id -> recurse merge
                        mergeAndCombineNestedObjectsWithMatchingIds(nestedprimary, nesteddelta[0]);
                    } else {
                        //no match found in DELTA list -> no-op
                    }
                })
                //loop over the DELTA list (right)
                delta[i].map(function (nesteddelta) {
                    var nestedprimary = primary[i].filter(function (nestedprimary) {
                        return nesteddelta.id == nestedprimary.id;
                    })
                    if(nestedprimary[0]) {
                        //found a nested object in PRIMARY -> no-op, already merged in (left)
                    } else { //no match found in PRIMARY list -> new record
                        primary[i].push(nesteddelta); //todo: should this be merge()
                    }
                })

                //todo: handle delete items

            } else if (typeof(primary[i]) == "boolean" || typeof(primary[i]) == "string" || typeof(primary[i]) == "number") { //primative
                //console.log("primative:" + i);
                primary[i] = delta[i]; //simple update
            } else if (typeof(primary[i]) == "function") { //skip functions

            } else if (primary[i] instanceof Date) { //known single object
                primary[i] = delta[i]; //update object
            } else if (primary[i] instanceof Object) { //single object
                console.log("unknown:" + i + " --> " + primary[i].constructor);
                //todo: what about back links? this could recurse forever
                //primary[i] = mergeAndCombineNestedObjectsWithMatchingIds(primary[i], delta[i]);
            } else {
                console.log("unknown:" + i + " --> " + primary[i].constructor);
            }
        } else {
            //todo: handle delete properties??
        }
    }
    //note: dont return anything, so object references on the primary dont change
}

[grantcarthew]

That's extreme @djeeg. What is the COUNTRY_objecttree? I'm sticking with the double get/save at the moment because it works and I have more to worry about.

[puradox]

What is the status of merge handling joined documents? Is this feature currently in the works?