#112 Adding feature to insert document when overwrite and putIfAbsent… #155
Conversation
|
Thanks for submitting the PR. Do you have an Apache ICLA on file? https://github.com/apache/incubator-openwhisk-package-cloudant/blob/master/CONTRIBUTING.md |
|
It would be nice to add a unit test similar to the one here https://github.com/apache/incubator-openwhisk-package-cloudant/blob/master/tests/src/test/scala/system/packages/CloudantDatabaseActionsTests.scala#L290. |
|
I can add a test and update the documentation. I will open a separate PR |
There was a problem hiding this comment.
Please do not add a new variable.
It's a bug if overwrite=true not working.
If overwrite=true then it should check if exists if exists then will get the rev and do an update, if it doesn't exist it should move forward and insert/write a doc.
/**
* If id defined and overwrite is true, checks if doc exists to retrieve version
* before insert. Else inserts.
*/
It should read to be more explicit and clear
/**
* If id defined and overwrite is true, checks if doc exists to retrieve version
* before insert. Else inserts a new doc.
*/
If the user action wants an error on trying to update existing doc, then don't pass the overwrite or pass overwrite=false
|
Added a new variable to provide backward compatibility. I'll update the code as per @csantanapr review comments, do @jasonpet and @rabbah agree? |
|
@csantanapr I generally agree the intended behavior is to just update. The concern was about breaking existing code. There are create and update actions which do the more conservative behavior. So we can treat this as a bug, remove the parameter and just do the update on 404. I'm OK with that too. |
|
I guess only a test is needed now since we are not adding a new parameter. No annotation updates in the install script or new documentation would be necessary. |
|
Removed putIfAbsent flag, document is inserted if overwrite=true and it does not already exists in the database |
| @@ -46,6 +46,7 @@ function main(message) { | |||
| /** | |||
| * If id defined and overwrite is true, checks if doc exists to retrieve version | |||
| * before insert. Else inserts. | |||
| * If | |||
| reject(error); | ||
| if(error.statusCode === 404) { | ||
| // If document not found, insert it | ||
| return insert(cloudantDb, doc); |
There was a problem hiding this comment.
the promise that contains the cloudant "get" is never resolved for this path
There was a problem hiding this comment.
good catch, looks like insert creates it own promise.
There was a problem hiding this comment.
let me update that
|
@csantanapr changes updated, can you update your review? |
… true