Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upsert #69

Merged
merged 4 commits into from Feb 8, 2019
Merged

Upsert #69

merged 4 commits into from Feb 8, 2019

Conversation

critichu
Copy link

@critichu critichu commented Feb 7, 2019

Added a new function called doc_upsert() that updates an existing document or creates it if it doesn't yet exist.

Description

doc_upsert() first retrieves the latest revision number for a given document id (docid) -- using db_revisions()
It then updates that document to the given doc -- using doc_update()

If the docid does not exist, then the first step will fail.
This is handled in doc_upsert(), which reverts to creating the document -- using doc_create()

Example

(x <- Cushion$new())

if ("sofadb" %in% db_list(x)) {
  invisible(db_delete(x, dbname="sofadb"))
}
db_create(x, 'sofadb')

# create a document
doc1 <- '{"name": "drink", "beer": "IPA", "score": 5}'
doc_upsert(x, dbname="sofadb", doc1, docid="abeer")

#update the document
doc2 <- '{"name": "drink", "beer": "lager", "score": 6}'
doc_upsert(x, dbname="sofadb", doc2, docid="abeer")

doc_get(x, dbname = "sofadb", docid = "abeer")

Tests

doc_upsert() is using existing sofa functions.
Nevertheless I included unit tests in the tests/testthat folder.

@codecov-io
Copy link

Codecov Report

Merging #69 into master will increase coverage by 6.63%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #69      +/-   ##
==========================================
+ Coverage   40.61%   47.24%   +6.63%     
==========================================
  Files          32       33       +1     
  Lines         426      436      +10     
==========================================
+ Hits          173      206      +33     
+ Misses        253      230      -23
Impacted Files Coverage Δ
R/doc_upsert.R 100% <100%> (ø)
R/revisions.r 80% <0%> (+80%) ⬆️
R/doc_get.r 100% <0%> (+100%) ⬆️
R/doc_update.r 100% <0%> (+100%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b1f0727...5ad5f86. Read the comment docs.

@sckott
Copy link
Contributor

sckott commented Feb 7, 2019

thanks @critichu i'll have a look

@sckott
Copy link
Contributor

sckott commented Feb 7, 2019

LGTM. The only thing that comes to mind: Currently doc_upsert() doesn't support the data.frame/list inputs to doc_create(). It's made harder in that doc_update() doesn't support data.frame/list inputs anyway. So for now I think we leave as is, and if we add data.frame/list inputs to doc_update() then we can add data.frame/list inputs to doc_upsert() as well. Sound good? Any complaints?

@critichu
Copy link
Author

critichu commented Feb 7, 2019

Sounds good!
Indeed I kept the arguments symmetric for both (create and update) cases, hence the current data.frame/list limitation.
Is an update to doc_update() planned in the near future perhaps?

@sckott
Copy link
Contributor

sckott commented Feb 8, 2019

Is an update to doc_update() planned in the near future perhaps?

it could be, i'll open an issue.

for now I think we move on with this, and we can add support for other classes later

@sckott sckott added this to the v0.4 milestone Feb 8, 2019
@sckott sckott merged commit 6fbe942 into ropensci:master Feb 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants