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

Apparent contradiction between Emo's never-null doc get api and conditional application #89

Open
2 tasks
vvcephei opened this issue Feb 8, 2017 · 3 comments

Comments

@vvcephei
Copy link
Contributor

vvcephei commented Feb 8, 2017

What is the Issue?

There is an apparent contradiction between Emo's never-null doc get api and conditional application.

How to Test and Verify

  1. Start Emo and run through this scenario:

  2. I start out by deleting the doc so we know it doesn't exist...

$ http DELETE :8080/sor/1/review:testcustomer/testdoc000 audit=="comment:'blah'" X-BV-API-Key:local_admin Content-Type:application/x.json-delta
HTTP/1.1 200 OK
Content-Type: application/json
Date: Wed, 08 Feb 2017 15:07:46 GMT
Transfer-Encoding: chunked

{
    "success": true
}
  1. I get the doc so you can see the state of it. Notice that there is no "nofield" attribute.
$ http GET :8080/sor/1/review:testcustomer/testdoc000 X-BV-API-Key:local_admin
HTTP/1.1 200 OK
Content-Type: application/json
Date: Wed, 08 Feb 2017 15:07:50 GMT
Transfer-Encoding: chunked

{
    "client": "TestCustomer", 
    "type": "review", 
    "~deleted": true, 
    "~firstUpdateAt": "2017-02-08T15:06:10.121Z", 
    "~id": "testdoc000", 
    "~lastUpdateAt": "2017-02-08T15:07:46.685Z", 
    "~signature": "233b2c5fcc29921f9728a317d5398b07", 
    "~table": "review:testcustomer", 
    "~version": 2
}
  1. Since the "nofield" attribute is missing, I'd expect '{.., "nofield": ~}' to evaluate to true...
$ http POST :8080/sor/1/review:testcustomer/testdoc000 audit=="comment:'blah'" X-BV-API-Key:local_admin Content-Type:application/x.json-delta <<< 'if {..,"nofield":~} then {"author":"Fred","title":"Best Ever!","rating":5} end'
HTTP/1.1 200 OK
Content-Type: application/json
Date: Wed, 08 Feb 2017 15:07:59 GMT
Transfer-Encoding: chunked

{
    "success": true
}
  1. But it didn't!!!
$ http GET :8080/sor/1/review:testcustomer/testdoc000 X-BV-API-Key:local_admin
HTTP/1.1 200 OK
Content-Type: application/json
Date: Wed, 08 Feb 2017 15:08:04 GMT
Transfer-Encoding: chunked

{
    "client": "TestCustomer", 
    "type": "review", 
    "~deleted": true, 
    "~firstUpdateAt": "2017-02-08T15:06:10.121Z", 
    "~id": "testdoc000", 
    "~lastUpdateAt": "2017-02-08T15:07:59.769Z", 
    "~signature": "618e69f4f5592b4b9a39df057d3dbf51", 
    "~table": "review:testcustomer", 
    "~version": 3
}
  1. But if I add a condition that the document itself may be missing...
$ http POST :8080/sor/1/review:testcustomer/testdoc000 audit=="comment:'blah'" X-BV-API-Key:local_admin Content-Type:application/x.json-delta <<< 'if or(~, {..,"nofield":~}) then {"author":"Fred","title":"Best Ever!","rating":5} end'
HTTP/1.1 200 OK
Content-Type: application/json
Date: Wed, 08 Feb 2017 15:08:31 GMT
Transfer-Encoding: chunked

{
    "success": true
}
  1. Then it works
$ http GET :8080/sor/1/review:testcustomer/testdoc000 X-BV-API-Key:local_admin
HTTP/1.1 200 OK
Content-Type: application/json
Date: Wed, 08 Feb 2017 15:08:33 GMT
Transfer-Encoding: chunked

{
    "author": "Fred", 
    "client": "TestCustomer", 
    "rating": 5, 
    "title": "Best Ever!", 
    "type": "review", 
    "~deleted": false, 
    "~firstUpdateAt": "2017-02-08T15:06:10.121Z", 
    "~id": "testdoc000", 
    "~lastUpdateAt": "2017-02-08T15:08:31.106Z", 
    "~signature": "582736be3895778a4cc2e1286ba6ffb6", 
    "~table": "review:testcustomer", 
    "~version": 4
}
  1. Now that the document exists, my original conditional works as expected
$ http POST :8080/sor/1/review:testcustomer/testdoc000 audit=="comment:'blah'" X-BV-API-Key:local_admin Content-Type:application/x.json-delta <<< 'if {..,"nofield":~} then {"author":"Kenny","title":"Best Ever!","rating":5} end'
HTTP/1.1 200 OK
Content-Type: application/json
Date: Wed, 08 Feb 2017 15:14:14 GMT
Transfer-Encoding: chunked

{
    "success": true
}
  1. See?
$ http GET :8080/sor/1/review:testcustomer/testdoc000 X-BV-API-Key:local_admin
HTTP/1.1 200 OK
Content-Type: application/json
Date: Wed, 08 Feb 2017 15:14:16 GMT
Transfer-Encoding: chunked

{
    "author": "Kenny", 
    "client": "TestCustomer", 
    "rating": 5, 
    "title": "Best Ever!", 
    "type": "review", 
    "~deleted": false, 
    "~firstUpdateAt": "2017-02-08T15:06:10.121Z", 
    "~id": "testdoc000", 
    "~lastUpdateAt": "2017-02-08T15:14:14.226Z", 
    "~signature": "01c5de13fdf7fe5dc27ca6852226ee68", 
    "~table": "review:testcustomer", 
    "~version": 5
}

Risk

Level

Low: I can just always use the redundant conditional or(~, {..,"nofield":~}). New users will find this behavior confusing, though... 'cause it is ;)

Issue Checklist

  • Make sure to label the issue.

  • Well documented description of use-cases and bugs.

@billkalter
Copy link
Contributor

Interesting edge condition. Not sure if the correct fix is to update the behavior or to updated the documentation. A root level document can only be in one of two states: an object or UNDEFINED. Undefined content is not a map, it's nothing. So the statement {..,"nofield":~} evaluates to false for UNDEFINED content just as it would if the content were any other non-object value (false, "a string", and so on), because the expected behavior when a conditional is for a non-matching data type is to return false.

Consider the following non-deleted content:

{
  "x": "a string"
}

and the conditional:

{..,"x":{..,"nofield":~}}

In this case it's more clear that the conditional should evaluate to false since the data type for the actual value of "x" is incapable of matching the condition which requires an object value.

@vvcephei
Copy link
Contributor Author

Thanks for the reply, @billkalter ,

I agree that in your example, the semantics clearly indicate the condition will evaluate to false.

The difference is that when I do a get on your document, I'll clearly see that x is a string, and therefore will not match an object delta of any kind.

However, in my example, when you do a get on the document, it does come back as an object, so it's not clear at all that the object delta will not match.

This is an inconistency introduced by showing me an "empty" object but applying UNDEFINED during delta application. My recommendation would be to replace UNDEFINED with {} during delta application.

This may of course have some potential edge cases to consider, but it's valuable to present an internally consistent interface.

@vvcephei
Copy link
Contributor Author

I guess it bears mention that {} is only valid as a replacement for UNDEFINED if the value in question is known to be an object, which is the case for Emo documents.

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

No branches or pull requests

2 participants