You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by nickva <gi...@git.apache.org> on 2016/04/15 15:51:35 UTC
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
GitHub user nickva opened a pull request:
https://github.com/apache/couchdb-couch/pull/162
Implement Mango selectors for change feeds
API is modeled after _doc_ids filter for change feeds.
User POSTs to {db}/_changes with `filter=_selector`.
Document body should have a "selector" field, with
a Mango selector object as value.
For example:
```
http://.../d1/_changes?filter=_selector
{
"selector": {"z" : {"$gte" : 1} }
}
```
In case "selector" is missing or its value is not a
JSON object, a 400 error is returned with a
corresponding error message. For example:
```
{
"error": "bad_request",
"reason": "Selector error: 42, expected an object"
}
```
In case "selector" object cannot be validated as a
Mango selector, a 400 error is returned with a validation
error as "reason".
Jira: COUCHDB-2988
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cloudant/couchdb-couch couchdb-2988
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-couch/pull/162.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #162
----
commit a703360b140ec610f65187818016522033709188
Author: Nick Vatamaniuc <va...@gmail.com>
Date: 2016-04-14T21:29:00Z
Implement Mango selectors for change feeds
API is modeled after _doc_ids filter for change feeds.
User POSTs to {db}/_changes with `filter=_selector`.
Document body should have a "selector" field, with
a Mango selector object as value.
For example:
```
http http://.../d1/_changes?filter=_selector
{
"selector": {"z" : {"$gte" : 1} }
}
In case "selector" is missing or its value is not a
JSON object, a 400 error is returned with a
corresponding error message. For example:
```
{
"error": "bad_request",
"reason": "Selector error: 42, expected an object"
}
```
In case "selector" object cannot be validated as a
Mango selector, a 400 error is returned with a validation
error as "reason".
Jira: COUCHDB-2988
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-couch/pull/162#issuecomment-210511943
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/162#discussion_r59884759
--- Diff: src/couch_changes.erl ---
@@ -336,6 +343,16 @@ get_doc_ids(_) ->
throw({bad_request, no_doc_ids_provided}).
+get_selector({json_req, {Props}}) ->
+ check_selector(couch_util:get_value(<<"selector">>, Props));
+get_selector(#httpd{method='POST'}=Req) ->
+ couch_httpd:validate_ctype(Req, "application/json"),
+ {Props} = couch_httpd:json_body_obj(Req),
+ check_selector(couch_util:get_value(<<"selector">>, Props));
--- End diff --
The pattern that @kxepal is highlighting is to recurse to unwrap objects. So your first clause has the base case, and the second clause would take out the thing to be selected and then recurse to the same check. That way your extraction logic is only written once.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by nickva <gi...@git.apache.org>.
Github user nickva commented on the pull request:
https://github.com/apache/couchdb-couch/pull/162#issuecomment-210510951
@kxepal @davisp @rnewson Updated error message for bad methods to be explicit, fixed get_selector duplicate code, use mango_error:info to format selector error messages.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by colinskow <gi...@git.apache.org>.
Github user colinskow commented on the pull request:
https://github.com/apache/couchdb-couch/pull/162#issuecomment-222065965
@nickva thank you very much for your response. A "live query" would be a very useful feature for front-end app developers.
I have [successfully implemented this](https://github.com/colinskow/pouchdb-live-find), but it requires listening to the entire changes feed. So my solution would not be entirely practical with a noisy database.
Would it be practical to maintain state of the query in a `_design`, `_replication`, or `_live_query` doc so the front end can maintain a live list of docs that match that query without having to listen to other noise?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-couch/pull/162#issuecomment-210493613
@davisp Ok, thanks. Indeed decoupling these things would be quite problematic task now without any significant profit.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:
https://github.com/apache/couchdb-couch/pull/162#issuecomment-210497574
impressive brevity for such a powerful new feature.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by tonysun83 <gi...@git.apache.org>.
Github user tonysun83 commented on the pull request:
https://github.com/apache/couchdb-couch/pull/162#issuecomment-210548778
simpler than I thought, +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by nickva <gi...@git.apache.org>.
Github user nickva commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/162#discussion_r59881398
--- Diff: src/couch_changes.erl ---
@@ -336,6 +343,16 @@ get_doc_ids(_) ->
throw({bad_request, no_doc_ids_provided}).
+get_selector({json_req, {Props}}) ->
+ check_selector(couch_util:get_value(<<"selector">>, Props));
+get_selector(#httpd{method='POST'}=Req) ->
+ couch_httpd:validate_ctype(Req, "application/json"),
+ {Props} = couch_httpd:json_body_obj(Req),
+ check_selector(couch_util:get_value(<<"selector">>, Props));
+get_selector(_) ->
+ throw({bad_request, invalid_selector}).
--- End diff --
It is an invalid_selector because this check happens after noticing there is a _selector as filter. Which prompted the the call to `configure_filter("_selector",...`. At this point if it is not GET request, or object not is an array it is an "invalid selector".
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/162#discussion_r59886640
--- Diff: src/couch_changes.erl ---
@@ -336,6 +343,16 @@ get_doc_ids(_) ->
throw({bad_request, no_doc_ids_provided}).
+get_selector({json_req, {Props}}) ->
+ check_selector(couch_util:get_value(<<"selector">>, Props));
+get_selector(#httpd{method='POST'}=Req) ->
+ couch_httpd:validate_ctype(Req, "application/json"),
+ {Props} = couch_httpd:json_body_obj(Req),
+ check_selector(couch_util:get_value(<<"selector">>, Props));
+get_selector(_) ->
+ throw({bad_request, invalid_selector}).
--- End diff --
The spec (obviously) is not so clear. We could send 405 Method Not Allowed but that's a resource-level thing and seems inappropriate. I'd send a 400 with a clear message if ?selector param exists, as both kxepal and davisp suggest.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-couch/pull/162#issuecomment-210476013
Nice feature, but I feel quite strange about creating dependency from mango for couch app. Since mango is a cluster thing, shouldn't it be on chttpd side?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/162#discussion_r59878224
--- Diff: src/couch_changes.erl ---
@@ -349,6 +366,19 @@ check_docids(_) ->
throw({bad_request, Msg}).
+check_selector(Selector={_}) ->
+ try
+ mango_selector:normalize(Selector)
+ catch
+ {mango_error, _Mod, Reason} -> % see MANGO_ERROR(R) in mango.hrl
+ Msg = io_lib:format("Selector error: ~p", [Reason]),
+ throw({bad_request, Msg})
+ end;
+check_selector(Selector) ->
+ Msg = io_lib:format("Selector error: ~p, expected an object", [Selector]),
--- End diff --
May be not send back the Selector data? The case: send ~1GiB of data like `{"selector": [....]}` and let server try to serialize that 1GiB of data and send it back to you.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/162#discussion_r59882439
--- Diff: src/couch_changes.erl ---
@@ -336,6 +343,16 @@ get_doc_ids(_) ->
throw({bad_request, no_doc_ids_provided}).
+get_selector({json_req, {Props}}) ->
+ check_selector(couch_util:get_value(<<"selector">>, Props));
+get_selector(#httpd{method='POST'}=Req) ->
+ couch_httpd:validate_ctype(Req, "application/json"),
+ {Props} = couch_httpd:json_body_obj(Req),
+ check_selector(couch_util:get_value(<<"selector">>, Props));
+get_selector(_) ->
+ throw({bad_request, invalid_selector}).
--- End diff --
In case of bad selector type - yes, but in case of GET request it's just missed. May be add one more clause about:
```
get_selector({json_req, _}) -> throw({bad_request, invalid_selector});
```
And the this clause turns into something more friendly:
```
get_selector(_) -> throw({bad_request, "Selector must be specified in POST payload"}).
```
What do you think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/couchdb-couch/pull/162
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/162#issuecomment-210489852
Except for the various tweaks to error messaging, +1 to the feature. Sure did come out a lot simpler than I was expecting.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by nickva <gi...@git.apache.org>.
Github user nickva commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/162#discussion_r59887227
--- Diff: src/couch_changes.erl ---
@@ -336,6 +343,16 @@ get_doc_ids(_) ->
throw({bad_request, no_doc_ids_provided}).
+get_selector({json_req, {Props}}) ->
+ check_selector(couch_util:get_value(<<"selector">>, Props));
+get_selector(#httpd{method='POST'}=Req) ->
+ couch_httpd:validate_ctype(Req, "application/json"),
+ {Props} = couch_httpd:json_body_obj(Req),
+ check_selector(couch_util:get_value(<<"selector">>, Props));
+get_selector(_) ->
+ throw({bad_request, invalid_selector}).
--- End diff --
Ok, good point. I can make that change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by nickva <gi...@git.apache.org>.
Github user nickva commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/162#discussion_r59880422
--- Diff: src/couch_changes.erl ---
@@ -336,6 +343,16 @@ get_doc_ids(_) ->
throw({bad_request, no_doc_ids_provided}).
+get_selector({json_req, {Props}}) ->
+ check_selector(couch_util:get_value(<<"selector">>, Props));
+get_selector(#httpd{method='POST'}=Req) ->
+ couch_httpd:validate_ctype(Req, "application/json"),
+ {Props} = couch_httpd:json_body_obj(Req),
+ check_selector(couch_util:get_value(<<"selector">>, Props));
--- End diff --
Tried that change but now a simple selector: {} which worked before fails with:
```
- {Props} = couch_httpd:json_body_obj(Req),
- check_selector(couch_util:get_value(<<"selector">>, Props));
+ check_selector({json_req, couch_httpd:json_body_obj(Req)});
```
{ ... "reason": "Selector error: {json_req,{[{<<\"selector\">>,{[]}}]}}, expected an object"...}
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/162#discussion_r59877477
--- Diff: src/couch_changes.erl ---
@@ -336,6 +343,16 @@ get_doc_ids(_) ->
throw({bad_request, no_doc_ids_provided}).
+get_selector({json_req, {Props}}) ->
+ check_selector(couch_util:get_value(<<"selector">>, Props));
+get_selector(#httpd{method='POST'}=Req) ->
+ couch_httpd:validate_ctype(Req, "application/json"),
+ {Props} = couch_httpd:json_body_obj(Req),
+ check_selector(couch_util:get_value(<<"selector">>, Props));
--- End diff --
Seems like you repeat yourself here. How about to change that branch into:
```
get_selector(#httpd{method='POST'}=Req) ->
couch_httpd:validate_ctype(Req, "application/json"),
get_selector({json_req, couch_httpd:json_body_obj(Req)});
```
You'll also produce bad request instead of server internal error with function clause in case of invalid request json data, like array.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/162#discussion_r59877824
--- Diff: src/couch_changes.erl ---
@@ -336,6 +343,16 @@ get_doc_ids(_) ->
throw({bad_request, no_doc_ids_provided}).
+get_selector({json_req, {Props}}) ->
+ check_selector(couch_util:get_value(<<"selector">>, Props));
+get_selector(#httpd{method='POST'}=Req) ->
+ couch_httpd:validate_ctype(Req, "application/json"),
+ {Props} = couch_httpd:json_body_obj(Req),
+ check_selector(couch_util:get_value(<<"selector">>, Props));
+get_selector(_) ->
+ throw({bad_request, invalid_selector}).
--- End diff --
Probably, `invalid_selection` is misleading as it actually about invalid request data. All about invalid selector should throw check_selector fun.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/162#discussion_r59884495
--- Diff: src/couch_changes.erl ---
@@ -349,6 +366,18 @@ check_docids(_) ->
throw({bad_request, Msg}).
+check_selector(Selector={_}) ->
+ try
+ mango_selector:normalize(Selector)
+ catch
+ {mango_error, _Mod, Reason} -> % see MANGO_ERROR(R) in mango.hrl
+ Msg = io_lib:format("Selector error: ~p", [Reason]),
--- End diff --
You want `mango_error:info(Mod, Reason) which gives you back a {StatusCode, Error, Reason} triple where Reason is formatted correctly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by nickva <gi...@git.apache.org>.
Github user nickva commented on the pull request:
https://github.com/apache/couchdb-couch/pull/162#issuecomment-221969885
@colinskow there is no easy way to add that I can think of. The query is in the context of the changes request. As long as there are results filtering back from that query, they'll contain documents which match the selector. There is nothing returning from the query which knows which documents matched before (using a previous changes request).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by nickva <gi...@git.apache.org>.
Github user nickva commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/162#discussion_r59881499
--- Diff: src/couch_changes.erl ---
@@ -349,6 +366,19 @@ check_docids(_) ->
throw({bad_request, Msg}).
+check_selector(Selector={_}) ->
+ try
+ mango_selector:normalize(Selector)
+ catch
+ {mango_error, _Mod, Reason} -> % see MANGO_ERROR(R) in mango.hrl
+ Msg = io_lib:format("Selector error: ~p", [Reason]),
+ throw({bad_request, Msg})
+ end;
+check_selector(Selector) ->
+ Msg = io_lib:format("Selector error: ~p, expected an object", [Selector]),
--- End diff --
Agreed. Good point!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by nickva <gi...@git.apache.org>.
Github user nickva commented on the pull request:
https://github.com/apache/couchdb-couch/pull/162#issuecomment-222195609
@colinskow that implies some kind of persistence. The selector passed in a change feed request is active only in the context of the POST request. There could be multiple such concurrent requests. After request is finished, there is no history preserved of previous request.
Or were you thinking about a long / continuous request, and while it is returning data back to the user, a document which passed the filter and was returned before, was updated to no longer pass the selector. But there is nothing to tell the user about it. So perhaps have something in the change feed saying "this document passed the selector before but no longer does".
That is an interesting case then. If document is deleted we return it with `"_deleted":true`, is it reasonable to have something like `"_selector_failed":true`? I am not sure about this. You might want to take this up on the couchdb mailing list. This looks like an API change then and needs a better discussion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by nickva <gi...@git.apache.org>.
Github user nickva commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/162#discussion_r59887144
--- Diff: src/couch_changes.erl ---
@@ -336,6 +343,16 @@ get_doc_ids(_) ->
throw({bad_request, no_doc_ids_provided}).
+get_selector({json_req, {Props}}) ->
+ check_selector(couch_util:get_value(<<"selector">>, Props));
+get_selector(#httpd{method='POST'}=Req) ->
+ couch_httpd:validate_ctype(Req, "application/json"),
+ {Props} = couch_httpd:json_body_obj(Req),
+ check_selector(couch_util:get_value(<<"selector">>, Props));
--- End diff --
Ah sorry. Missed that. Will fix it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by colinskow <gi...@git.apache.org>.
Github user colinskow commented on the pull request:
https://github.com/apache/couchdb-couch/pull/162#issuecomment-218081585
That's a great feature! In order to maintain a live list of docs that match a query, I will need changes from docs that used to match the query but no longer do so I can remove them from the list. Is that possible with this feature?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/162#issuecomment-210532037
Good point @rnewson. I'd go with the 400 approach then.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/162#issuecomment-210489692
@kxepal for the clustered vs. not dependency, it is slightly odd. However the changes code is sadly tied into the main couch repo, there's nothing we can really do to undo that without some pretty awkward contortions just for purity's sake.
Also, in this case, the mango logic being used is purely functional, there's no cluster or anything. Its strictly just a "does this json blob match this other json blob using the mango selector DSL rules" function.
So while I do agree it would be best to have this specific change in a different app, I'd argue that it's more of that all of this particular changes code should be in a different app that understands clustered vs not rather than the hodge podge of things we have now. Until then I wouldn't worry about this too much since it will work for both clustered and single node.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/162#discussion_r59880805
--- Diff: src/couch_changes.erl ---
@@ -336,6 +343,16 @@ get_doc_ids(_) ->
throw({bad_request, no_doc_ids_provided}).
+get_selector({json_req, {Props}}) ->
+ check_selector(couch_util:get_value(<<"selector">>, Props));
+get_selector(#httpd{method='POST'}=Req) ->
+ couch_httpd:validate_ctype(Req, "application/json"),
+ {Props} = couch_httpd:json_body_obj(Req),
+ check_selector(couch_util:get_value(<<"selector">>, Props));
--- End diff --
Last line: `get_selector`, not `check_selector`. Also, point about function_clause is false: I miss the moment that json_body_obj already handles all the routines to ensure that we got JSON object, nothing else.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Implement Mango selectors for change f...
Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/162#discussion_r59885221
--- Diff: src/couch_changes.erl ---
@@ -336,6 +343,16 @@ get_doc_ids(_) ->
throw({bad_request, no_doc_ids_provided}).
+get_selector({json_req, {Props}}) ->
+ check_selector(couch_util:get_value(<<"selector">>, Props));
+get_selector(#httpd{method='POST'}=Req) ->
+ couch_httpd:validate_ctype(Req, "application/json"),
+ {Props} = couch_httpd:json_body_obj(Req),
+ check_selector(couch_util:get_value(<<"selector">>, Props));
+get_selector(_) ->
+ throw({bad_request, invalid_selector}).
--- End diff --
I agree with @kxepal here, if a user does a GET with a filter=_selector, then it'll be super opaque as to what wen't wrong. Though rather than a 400 you might throw a whatever it is for invalid method. Though i'd want to hear from @rnewson or someone if restricting methods based on query string paramters is kosher. If not kosher, than at least the 400 with the "you must specify the selector in a POST body" message would be fine so that users have a bread crumb when they get the error.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---