You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2021/04/13 17:03:39 UTC

[GitHub] [couchdb] nickva opened a new pull request #3503: [wip] 3.x applications cleanup

nickva opened a new pull request #3503:
URL: https://github.com/apache/couchdb/pull/3503


   WIP
   
    TODO: need to break it into smaller commits
   
   Summary:
     - about a dozen applications removed
     - all eunit tests are now running, including the ones from couch
     - cleaned up default.ini
     - close local 5986 port
     - return not implemented or not supported for some endpoints
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] bessbd commented on a change in pull request #3503: 3.x applications cleanup

Posted by GitBox <gi...@apache.org>.
bessbd commented on a change in pull request #3503:
URL: https://github.com/apache/couchdb/pull/3503#discussion_r614212380



##########
File path: src/couch_views/src/couch_views_validate.erl
##########
@@ -0,0 +1,460 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+%   http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_views_validate).
+
+
+-export([
+    validate_args/1,
+    validate_args/3,
+    validate_ddoc/2
+]).
+
+
+-define(LOWEST_KEY, null).
+-define(HIGHEST_KEY, {<<255, 255, 255, 255>>}).
+
+
+-include_lib("couch/include/couch_db.hrl").
+-include("couch_views.hrl").
+
+
+% There is another almost identical validate_args in couch_views_util. They
+% should probably be merged at some point in the future.
+%
+validate_args(Args) ->
+    GroupLevel = determine_group_level(Args),
+    Reduce = Args#mrargs.reduce,
+    case Reduce == undefined orelse is_boolean(Reduce) of
+        true -> ok;
+        _ -> mrverror(<<"Invalid `reduce` value.">>)
+    end,
+
+    case {Args#mrargs.view_type, Reduce} of
+        {map, true} -> mrverror(<<"Reduce is invalid for map-only views.">>);
+        _ -> ok
+    end,
+
+    case {Args#mrargs.view_type, GroupLevel, Args#mrargs.keys} of
+        {red, exact, _} -> ok;
+        {red, _, KeyList} when is_list(KeyList) ->
+            Msg = <<"Multi-key fetchs for reduce views must use `group=true`">>,
+            mrverror(Msg);
+        _ -> ok
+    end,
+
+    case Args#mrargs.keys of
+        Keys when is_list(Keys) -> ok;
+        undefined -> ok;
+        _ -> mrverror(<<"`keys` must be an array of strings.">>)
+    end,
+
+    case {Args#mrargs.keys, Args#mrargs.start_key,
+          Args#mrargs.end_key} of
+        {undefined, _, _} -> ok;
+        {[], _, _} -> ok;
+        {[_|_], undefined, undefined} -> ok;
+        _ -> mrverror(<<"`keys` is incompatible with `key`"
+                        ", `start_key` and `end_key`">>)
+    end,
+
+    case Args#mrargs.start_key_docid of
+        undefined -> ok;
+        SKDocId0 when is_binary(SKDocId0) -> ok;
+        _ -> mrverror(<<"`start_key_docid` must be a string.">>)
+    end,
+
+    case Args#mrargs.end_key_docid of
+        undefined -> ok;
+        EKDocId0 when is_binary(EKDocId0) -> ok;
+        _ -> mrverror(<<"`end_key_docid` must be a string.">>)
+    end,
+
+    case Args#mrargs.direction of
+        fwd -> ok;
+        rev -> ok;
+        _ -> mrverror(<<"Invalid direction.">>)
+    end,
+
+    case {Args#mrargs.limit >= 0, Args#mrargs.limit == undefined} of

Review comment:
       I can open a follow-up PR, too if you want me to.
   I personally have no problems with doing some "Boy Scout Rule" changes. (If you agree with the suggestion.)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] bessbd commented on a change in pull request #3503: 3.x applications cleanup

Posted by GitBox <gi...@apache.org>.
bessbd commented on a change in pull request #3503:
URL: https://github.com/apache/couchdb/pull/3503#discussion_r614102478



##########
File path: src/chttpd/src/chttpd_misc.erl
##########
@@ -334,9 +329,33 @@ handle_reload_query_servers_req(#httpd{method='POST'}=Req) ->
 handle_reload_query_servers_req(Req) ->
     send_method_not_allowed(Req, "POST").
 
+handle_uuids_req(#httpd{method='GET'}=Req) ->
+    Max = list_to_integer(config:get("uuids","max_count","1000")),
+    Count = try list_to_integer(couch_httpd:qs_value(Req, "count", "1")) of
+        N when N > Max ->
+            throw({bad_request, <<"count parameter too large">>});
+        N when N < 0 ->
+            throw({bad_request, <<"count must be a positive integer">>});
+        N -> N
+    catch
+        error:badarg ->
+            throw({bad_request, <<"count must be a positive integer">>})
+    end,
+    UUIDs = [couch_uuids:new() || _ <- lists:seq(1, Count)],
+    Etag = couch_httpd:make_etag(UUIDs),
+    couch_httpd:etag_respond(Req, Etag, fun() ->
+        CacheBustingHeaders = [
+            {"Date", couch_util:rfc1123_date()},
+            {"Cache-Control", "no-cache"},
+            % Past date, ON PURPOSE!
+            {"Expires", "Mon, 01 Jan 1990 00:00:00 GMT"},
+            {"Pragma", "no-cache"},
+            {"ETag", Etag}

Review comment:
       Is the `ETag` necessary here?

##########
File path: src/couch_views/src/couch_views_validate.erl
##########
@@ -0,0 +1,460 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+%   http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_views_validate).
+
+
+-export([
+    validate_args/1,
+    validate_args/3,
+    validate_ddoc/2
+]).
+
+
+-define(LOWEST_KEY, null).
+-define(HIGHEST_KEY, {<<255, 255, 255, 255>>}).
+
+
+-include_lib("couch/include/couch_db.hrl").
+-include("couch_views.hrl").
+
+
+% There is another almost identical validate_args in couch_views_util. They
+% should probably be merged at some point in the future.
+%
+validate_args(Args) ->
+    GroupLevel = determine_group_level(Args),
+    Reduce = Args#mrargs.reduce,
+    case Reduce == undefined orelse is_boolean(Reduce) of
+        true -> ok;
+        _ -> mrverror(<<"Invalid `reduce` value.">>)
+    end,
+
+    case {Args#mrargs.view_type, Reduce} of
+        {map, true} -> mrverror(<<"Reduce is invalid for map-only views.">>);
+        _ -> ok
+    end,
+
+    case {Args#mrargs.view_type, GroupLevel, Args#mrargs.keys} of
+        {red, exact, _} -> ok;
+        {red, _, KeyList} when is_list(KeyList) ->
+            Msg = <<"Multi-key fetchs for reduce views must use `group=true`">>,

Review comment:
       Nit/typo:
   ```suggestion
               Msg = <<"Multi-key fetches for reduce views must use `group=true`">>,
   ```

##########
File path: src/couch_views/src/couch_views_validate.erl
##########
@@ -0,0 +1,460 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+%   http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_views_validate).
+
+
+-export([
+    validate_args/1,
+    validate_args/3,
+    validate_ddoc/2
+]).
+
+
+-define(LOWEST_KEY, null).
+-define(HIGHEST_KEY, {<<255, 255, 255, 255>>}).
+
+
+-include_lib("couch/include/couch_db.hrl").
+-include("couch_views.hrl").
+
+
+% There is another almost identical validate_args in couch_views_util. They
+% should probably be merged at some point in the future.
+%
+validate_args(Args) ->
+    GroupLevel = determine_group_level(Args),
+    Reduce = Args#mrargs.reduce,
+    case Reduce == undefined orelse is_boolean(Reduce) of
+        true -> ok;
+        _ -> mrverror(<<"Invalid `reduce` value.">>)
+    end,
+
+    case {Args#mrargs.view_type, Reduce} of
+        {map, true} -> mrverror(<<"Reduce is invalid for map-only views.">>);
+        _ -> ok
+    end,
+
+    case {Args#mrargs.view_type, GroupLevel, Args#mrargs.keys} of
+        {red, exact, _} -> ok;
+        {red, _, KeyList} when is_list(KeyList) ->
+            Msg = <<"Multi-key fetchs for reduce views must use `group=true`">>,
+            mrverror(Msg);
+        _ -> ok
+    end,
+
+    case Args#mrargs.keys of
+        Keys when is_list(Keys) -> ok;
+        undefined -> ok;
+        _ -> mrverror(<<"`keys` must be an array of strings.">>)
+    end,
+
+    case {Args#mrargs.keys, Args#mrargs.start_key,
+          Args#mrargs.end_key} of
+        {undefined, _, _} -> ok;
+        {[], _, _} -> ok;
+        {[_|_], undefined, undefined} -> ok;
+        _ -> mrverror(<<"`keys` is incompatible with `key`"
+                        ", `start_key` and `end_key`">>)
+    end,
+
+    case Args#mrargs.start_key_docid of
+        undefined -> ok;
+        SKDocId0 when is_binary(SKDocId0) -> ok;
+        _ -> mrverror(<<"`start_key_docid` must be a string.">>)
+    end,
+
+    case Args#mrargs.end_key_docid of
+        undefined -> ok;
+        EKDocId0 when is_binary(EKDocId0) -> ok;
+        _ -> mrverror(<<"`end_key_docid` must be a string.">>)
+    end,
+
+    case Args#mrargs.direction of
+        fwd -> ok;
+        rev -> ok;
+        _ -> mrverror(<<"Invalid direction.">>)
+    end,
+
+    case {Args#mrargs.limit >= 0, Args#mrargs.limit == undefined} of
+        {true, _} -> ok;
+        {_, true} -> ok;
+        _ -> mrverror(<<"`limit` must be a positive integer.">>)
+    end,
+
+    case Args#mrargs.skip < 0 of
+        true -> mrverror(<<"`skip` must be >= 0">>);
+        _ -> ok
+    end,
+
+    case {Args#mrargs.view_type, GroupLevel} of
+        {red, exact} -> ok;
+        {_, 0} -> ok;
+        {red, Int} when is_integer(Int), Int >= 0 -> ok;
+        {red, _} -> mrverror(<<"`group_level` must be >= 0">>);
+        {map, _} -> mrverror(<<"Invalid use of grouping on a map view.">>)
+    end,
+
+    case Args#mrargs.stable of
+        true -> ok;
+        false -> ok;
+        _ -> mrverror(<<"Invalid value for `stable`.">>)
+    end,
+
+    case Args#mrargs.update of
+        true -> ok;
+        false -> ok;
+        lazy -> ok;
+        _ -> mrverror(<<"Invalid value for `update`.">>)
+    end,
+
+    case is_boolean(Args#mrargs.inclusive_end) of
+        true -> ok;
+        _ -> mrverror(<<"Invalid value for `inclusive_end`.">>)
+    end,
+
+    case {Args#mrargs.view_type, Args#mrargs.include_docs} of
+        {red, true} -> mrverror(<<"`include_docs` is invalid for reduce">>);
+        {_, ID} when is_boolean(ID) -> ok;
+        _ -> mrverror(<<"Invalid value for `include_docs`">>)
+    end,
+
+    case {Args#mrargs.view_type, Args#mrargs.conflicts} of
+        {_, undefined} -> ok;
+        {map, V} when is_boolean(V) -> ok;
+        {red, undefined} -> ok;
+        {map, _} -> mrverror(<<"Invalid value for `conflicts`.">>);
+        {red, _} -> mrverror(<<"`conflicts` is invalid for reduce views.">>)
+    end,
+
+    SKDocId = case {Args#mrargs.direction, Args#mrargs.start_key_docid} of
+        {fwd, undefined} -> <<>>;
+        {rev, undefined} -> <<255>>;
+        {_, SKDocId1} -> SKDocId1
+    end,
+
+    EKDocId = case {Args#mrargs.direction, Args#mrargs.end_key_docid} of
+        {fwd, undefined} -> <<255>>;
+        {rev, undefined} -> <<>>;
+        {_, EKDocId1} -> EKDocId1
+    end,
+
+    case is_boolean(Args#mrargs.sorted) of
+        true -> ok;
+        _ -> mrverror(<<"Invalid value for `sorted`.">>)
+    end,
+
+    Args#mrargs{
+        start_key_docid=SKDocId,
+        end_key_docid=EKDocId,
+        group_level=GroupLevel
+    }.
+
+
+validate_args(Db, DDoc, Args0) ->
+    {ok, State} = couch_views_util:ddoc_to_mrst(fabric2_db:name(Db), DDoc),
+    Args1 = apply_limit(State#mrst.partitioned, Args0),
+    validate_args(State, Args1).
+
+
+validate_ddoc(#{} = Db, DDoc) ->
+    DbName = fabric2_db:name(Db),
+    IsPartitioned = fabric2_db:is_partitioned(Db),
+    validate_ddoc(DbName, IsPartitioned, DDoc).
+
+
+% Private functionss

Review comment:
       Nit/typo:
   ```suggestion
   % Private functions
   ```

##########
File path: src/couch_views/src/couch_views_validate.erl
##########
@@ -0,0 +1,460 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+%   http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_views_validate).
+
+
+-export([
+    validate_args/1,
+    validate_args/3,
+    validate_ddoc/2
+]).
+
+
+-define(LOWEST_KEY, null).
+-define(HIGHEST_KEY, {<<255, 255, 255, 255>>}).
+
+
+-include_lib("couch/include/couch_db.hrl").
+-include("couch_views.hrl").
+
+
+% There is another almost identical validate_args in couch_views_util. They
+% should probably be merged at some point in the future.
+%
+validate_args(Args) ->
+    GroupLevel = determine_group_level(Args),
+    Reduce = Args#mrargs.reduce,
+    case Reduce == undefined orelse is_boolean(Reduce) of
+        true -> ok;
+        _ -> mrverror(<<"Invalid `reduce` value.">>)
+    end,
+
+    case {Args#mrargs.view_type, Reduce} of
+        {map, true} -> mrverror(<<"Reduce is invalid for map-only views.">>);
+        _ -> ok
+    end,
+
+    case {Args#mrargs.view_type, GroupLevel, Args#mrargs.keys} of
+        {red, exact, _} -> ok;
+        {red, _, KeyList} when is_list(KeyList) ->
+            Msg = <<"Multi-key fetchs for reduce views must use `group=true`">>,
+            mrverror(Msg);
+        _ -> ok
+    end,
+
+    case Args#mrargs.keys of
+        Keys when is_list(Keys) -> ok;
+        undefined -> ok;
+        _ -> mrverror(<<"`keys` must be an array of strings.">>)
+    end,
+
+    case {Args#mrargs.keys, Args#mrargs.start_key,
+          Args#mrargs.end_key} of
+        {undefined, _, _} -> ok;
+        {[], _, _} -> ok;
+        {[_|_], undefined, undefined} -> ok;
+        _ -> mrverror(<<"`keys` is incompatible with `key`"
+                        ", `start_key` and `end_key`">>)
+    end,
+
+    case Args#mrargs.start_key_docid of
+        undefined -> ok;
+        SKDocId0 when is_binary(SKDocId0) -> ok;
+        _ -> mrverror(<<"`start_key_docid` must be a string.">>)
+    end,
+
+    case Args#mrargs.end_key_docid of
+        undefined -> ok;
+        EKDocId0 when is_binary(EKDocId0) -> ok;
+        _ -> mrverror(<<"`end_key_docid` must be a string.">>)
+    end,
+
+    case Args#mrargs.direction of
+        fwd -> ok;
+        rev -> ok;
+        _ -> mrverror(<<"Invalid direction.">>)
+    end,
+
+    case {Args#mrargs.limit >= 0, Args#mrargs.limit == undefined} of

Review comment:
       Nit:
   ```suggestion
       case {Args#mrargs.limit > 0, Args#mrargs.limit == undefined} of
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] nickva commented on a change in pull request #3503: 3.x applications cleanup

Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #3503:
URL: https://github.com/apache/couchdb/pull/3503#discussion_r614213987



##########
File path: src/chttpd/src/chttpd_misc.erl
##########
@@ -334,9 +329,33 @@ handle_reload_query_servers_req(#httpd{method='POST'}=Req) ->
 handle_reload_query_servers_req(Req) ->
     send_method_not_allowed(Req, "POST").
 
+handle_uuids_req(#httpd{method='GET'}=Req) ->
+    Max = list_to_integer(config:get("uuids","max_count","1000")),
+    Count = try list_to_integer(couch_httpd:qs_value(Req, "count", "1")) of
+        N when N > Max ->
+            throw({bad_request, <<"count parameter too large">>});
+        N when N < 0 ->
+            throw({bad_request, <<"count must be a positive integer">>});
+        N -> N
+    catch
+        error:badarg ->
+            throw({bad_request, <<"count must be a positive integer">>})
+    end,
+    UUIDs = [couch_uuids:new() || _ <- lists:seq(1, Count)],
+    Etag = couch_httpd:make_etag(UUIDs),
+    couch_httpd:etag_respond(Req, Etag, fun() ->
+        CacheBustingHeaders = [
+            {"Date", couch_util:rfc1123_date()},
+            {"Cache-Control", "no-cache"},
+            % Past date, ON PURPOSE!
+            {"Expires", "Mon, 01 Jan 1990 00:00:00 GMT"},
+            {"Pragma", "no-cache"},
+            {"ETag", Etag}

Review comment:
       I think it's to ensure the responses are not cached. This was a verbatim copy and paste from https://github.com/apache/couchdb/blob/403d27b16f5209f918766dc62a8b0d039080fd32/src/couch/src/couch_httpd_misc_handlers.erl#L96-L108




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] nickva commented on a change in pull request #3503: 3.x applications cleanup

Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #3503:
URL: https://github.com/apache/couchdb/pull/3503#discussion_r614227011



##########
File path: src/couch_views/src/couch_views_validate.erl
##########
@@ -0,0 +1,460 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+%   http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_views_validate).
+
+
+-export([
+    validate_args/1,
+    validate_args/3,
+    validate_ddoc/2
+]).
+
+
+-define(LOWEST_KEY, null).
+-define(HIGHEST_KEY, {<<255, 255, 255, 255>>}).
+
+
+-include_lib("couch/include/couch_db.hrl").
+-include("couch_views.hrl").
+
+
+% There is another almost identical validate_args in couch_views_util. They
+% should probably be merged at some point in the future.
+%
+validate_args(Args) ->
+    GroupLevel = determine_group_level(Args),
+    Reduce = Args#mrargs.reduce,
+    case Reduce == undefined orelse is_boolean(Reduce) of
+        true -> ok;
+        _ -> mrverror(<<"Invalid `reduce` value.">>)
+    end,
+
+    case {Args#mrargs.view_type, Reduce} of
+        {map, true} -> mrverror(<<"Reduce is invalid for map-only views.">>);
+        _ -> ok
+    end,
+
+    case {Args#mrargs.view_type, GroupLevel, Args#mrargs.keys} of
+        {red, exact, _} -> ok;
+        {red, _, KeyList} when is_list(KeyList) ->
+            Msg = <<"Multi-key fetchs for reduce views must use `group=true`">>,
+            mrverror(Msg);
+        _ -> ok
+    end,
+
+    case Args#mrargs.keys of
+        Keys when is_list(Keys) -> ok;
+        undefined -> ok;
+        _ -> mrverror(<<"`keys` must be an array of strings.">>)
+    end,
+
+    case {Args#mrargs.keys, Args#mrargs.start_key,
+          Args#mrargs.end_key} of
+        {undefined, _, _} -> ok;
+        {[], _, _} -> ok;
+        {[_|_], undefined, undefined} -> ok;
+        _ -> mrverror(<<"`keys` is incompatible with `key`"
+                        ", `start_key` and `end_key`">>)
+    end,
+
+    case Args#mrargs.start_key_docid of
+        undefined -> ok;
+        SKDocId0 when is_binary(SKDocId0) -> ok;
+        _ -> mrverror(<<"`start_key_docid` must be a string.">>)
+    end,
+
+    case Args#mrargs.end_key_docid of
+        undefined -> ok;
+        EKDocId0 when is_binary(EKDocId0) -> ok;
+        _ -> mrverror(<<"`end_key_docid` must be a string.">>)
+    end,
+
+    case Args#mrargs.direction of
+        fwd -> ok;
+        rev -> ok;
+        _ -> mrverror(<<"Invalid direction.">>)
+    end,
+
+    case {Args#mrargs.limit >= 0, Args#mrargs.limit == undefined} of

Review comment:
       We do want a limit=0 option, it returns the `total_rows` in the view without returning any rows. We could update the error to say that we want to limit to be `a non-negative` integer, or just say `limit must be greater or equal to 0`. But it would be a separate PR I'd think




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] nickva commented on a change in pull request #3503: 3.x applications cleanup

Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #3503:
URL: https://github.com/apache/couchdb/pull/3503#discussion_r614238197



##########
File path: src/chttpd/src/chttpd_misc.erl
##########
@@ -334,9 +329,33 @@ handle_reload_query_servers_req(#httpd{method='POST'}=Req) ->
 handle_reload_query_servers_req(Req) ->
     send_method_not_allowed(Req, "POST").
 
+handle_uuids_req(#httpd{method='GET'}=Req) ->
+    Max = list_to_integer(config:get("uuids","max_count","1000")),
+    Count = try list_to_integer(couch_httpd:qs_value(Req, "count", "1")) of
+        N when N > Max ->
+            throw({bad_request, <<"count parameter too large">>});
+        N when N < 0 ->
+            throw({bad_request, <<"count must be a positive integer">>});
+        N -> N
+    catch
+        error:badarg ->
+            throw({bad_request, <<"count must be a positive integer">>})
+    end,
+    UUIDs = [couch_uuids:new() || _ <- lists:seq(1, Count)],
+    Etag = couch_httpd:make_etag(UUIDs),
+    couch_httpd:etag_respond(Req, Etag, fun() ->
+        CacheBustingHeaders = [
+            {"Date", couch_util:rfc1123_date()},
+            {"Cache-Control", "no-cache"},
+            % Past date, ON PURPOSE!
+            {"Expires", "Mon, 01 Jan 1990 00:00:00 GMT"},
+            {"Pragma", "no-cache"},
+            {"ETag", Etag}

Review comment:
       Good idea. Maybe we can just use the first 1st uuid since we already did the work to generate them. Something like?:
   
   ```erlang
   Etag = binary_to_list(hd(UUIDs))
   ``` 
   
   But let's make it a separate PR. As this one is already quite large :-)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] nickva commented on a change in pull request #3503: 3.x applications cleanup

Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #3503:
URL: https://github.com/apache/couchdb/pull/3503#discussion_r614210663



##########
File path: src/couch_views/src/couch_views_validate.erl
##########
@@ -0,0 +1,460 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+%   http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_views_validate).
+
+
+-export([
+    validate_args/1,
+    validate_args/3,
+    validate_ddoc/2
+]).
+
+
+-define(LOWEST_KEY, null).
+-define(HIGHEST_KEY, {<<255, 255, 255, 255>>}).
+
+
+-include_lib("couch/include/couch_db.hrl").
+-include("couch_views.hrl").
+
+
+% There is another almost identical validate_args in couch_views_util. They
+% should probably be merged at some point in the future.
+%
+validate_args(Args) ->
+    GroupLevel = determine_group_level(Args),
+    Reduce = Args#mrargs.reduce,
+    case Reduce == undefined orelse is_boolean(Reduce) of
+        true -> ok;
+        _ -> mrverror(<<"Invalid `reduce` value.">>)
+    end,
+
+    case {Args#mrargs.view_type, Reduce} of
+        {map, true} -> mrverror(<<"Reduce is invalid for map-only views.">>);
+        _ -> ok
+    end,
+
+    case {Args#mrargs.view_type, GroupLevel, Args#mrargs.keys} of
+        {red, exact, _} -> ok;
+        {red, _, KeyList} when is_list(KeyList) ->
+            Msg = <<"Multi-key fetchs for reduce views must use `group=true`">>,
+            mrverror(Msg);
+        _ -> ok
+    end,
+
+    case Args#mrargs.keys of
+        Keys when is_list(Keys) -> ok;
+        undefined -> ok;
+        _ -> mrverror(<<"`keys` must be an array of strings.">>)
+    end,
+
+    case {Args#mrargs.keys, Args#mrargs.start_key,
+          Args#mrargs.end_key} of
+        {undefined, _, _} -> ok;
+        {[], _, _} -> ok;
+        {[_|_], undefined, undefined} -> ok;
+        _ -> mrverror(<<"`keys` is incompatible with `key`"
+                        ", `start_key` and `end_key`">>)
+    end,
+
+    case Args#mrargs.start_key_docid of
+        undefined -> ok;
+        SKDocId0 when is_binary(SKDocId0) -> ok;
+        _ -> mrverror(<<"`start_key_docid` must be a string.">>)
+    end,
+
+    case Args#mrargs.end_key_docid of
+        undefined -> ok;
+        EKDocId0 when is_binary(EKDocId0) -> ok;
+        _ -> mrverror(<<"`end_key_docid` must be a string.">>)
+    end,
+
+    case Args#mrargs.direction of
+        fwd -> ok;
+        rev -> ok;
+        _ -> mrverror(<<"Invalid direction.">>)
+    end,
+
+    case {Args#mrargs.limit >= 0, Args#mrargs.limit == undefined} of

Review comment:
       I copied this one verbatim from `couch_mrviews_util` https://github.com/apache/couchdb/blob/403d27b16f5209f918766dc62a8b0d039080fd32/src/couch_mrview/src/couch_mrview_util.erl#L523-L527
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] nickva merged pull request #3503: 3.x applications cleanup

Posted by GitBox <gi...@apache.org>.
nickva merged pull request #3503:
URL: https://github.com/apache/couchdb/pull/3503


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] bessbd commented on a change in pull request #3503: 3.x applications cleanup

Posted by GitBox <gi...@apache.org>.
bessbd commented on a change in pull request #3503:
URL: https://github.com/apache/couchdb/pull/3503#discussion_r614714903



##########
File path: src/chttpd/src/chttpd_misc.erl
##########
@@ -334,9 +329,33 @@ handle_reload_query_servers_req(#httpd{method='POST'}=Req) ->
 handle_reload_query_servers_req(Req) ->
     send_method_not_allowed(Req, "POST").
 
+handle_uuids_req(#httpd{method='GET'}=Req) ->
+    Max = list_to_integer(config:get("uuids","max_count","1000")),
+    Count = try list_to_integer(couch_httpd:qs_value(Req, "count", "1")) of
+        N when N > Max ->
+            throw({bad_request, <<"count parameter too large">>});
+        N when N < 0 ->
+            throw({bad_request, <<"count must be a positive integer">>});
+        N -> N
+    catch
+        error:badarg ->
+            throw({bad_request, <<"count must be a positive integer">>})
+    end,
+    UUIDs = [couch_uuids:new() || _ <- lists:seq(1, Count)],
+    Etag = couch_httpd:make_etag(UUIDs),
+    couch_httpd:etag_respond(Req, Etag, fun() ->
+        CacheBustingHeaders = [
+            {"Date", couch_util:rfc1123_date()},
+            {"Cache-Control", "no-cache"},
+            % Past date, ON PURPOSE!
+            {"Expires", "Mon, 01 Jan 1990 00:00:00 GMT"},
+            {"Pragma", "no-cache"},
+            {"ETag", Etag}

Review comment:
       Sounds good!




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] nickva commented on a change in pull request #3503: 3.x applications cleanup

Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #3503:
URL: https://github.com/apache/couchdb/pull/3503#discussion_r614210663



##########
File path: src/couch_views/src/couch_views_validate.erl
##########
@@ -0,0 +1,460 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+%   http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_views_validate).
+
+
+-export([
+    validate_args/1,
+    validate_args/3,
+    validate_ddoc/2
+]).
+
+
+-define(LOWEST_KEY, null).
+-define(HIGHEST_KEY, {<<255, 255, 255, 255>>}).
+
+
+-include_lib("couch/include/couch_db.hrl").
+-include("couch_views.hrl").
+
+
+% There is another almost identical validate_args in couch_views_util. They
+% should probably be merged at some point in the future.
+%
+validate_args(Args) ->
+    GroupLevel = determine_group_level(Args),
+    Reduce = Args#mrargs.reduce,
+    case Reduce == undefined orelse is_boolean(Reduce) of
+        true -> ok;
+        _ -> mrverror(<<"Invalid `reduce` value.">>)
+    end,
+
+    case {Args#mrargs.view_type, Reduce} of
+        {map, true} -> mrverror(<<"Reduce is invalid for map-only views.">>);
+        _ -> ok
+    end,
+
+    case {Args#mrargs.view_type, GroupLevel, Args#mrargs.keys} of
+        {red, exact, _} -> ok;
+        {red, _, KeyList} when is_list(KeyList) ->
+            Msg = <<"Multi-key fetchs for reduce views must use `group=true`">>,
+            mrverror(Msg);
+        _ -> ok
+    end,
+
+    case Args#mrargs.keys of
+        Keys when is_list(Keys) -> ok;
+        undefined -> ok;
+        _ -> mrverror(<<"`keys` must be an array of strings.">>)
+    end,
+
+    case {Args#mrargs.keys, Args#mrargs.start_key,
+          Args#mrargs.end_key} of
+        {undefined, _, _} -> ok;
+        {[], _, _} -> ok;
+        {[_|_], undefined, undefined} -> ok;
+        _ -> mrverror(<<"`keys` is incompatible with `key`"
+                        ", `start_key` and `end_key`">>)
+    end,
+
+    case Args#mrargs.start_key_docid of
+        undefined -> ok;
+        SKDocId0 when is_binary(SKDocId0) -> ok;
+        _ -> mrverror(<<"`start_key_docid` must be a string.">>)
+    end,
+
+    case Args#mrargs.end_key_docid of
+        undefined -> ok;
+        EKDocId0 when is_binary(EKDocId0) -> ok;
+        _ -> mrverror(<<"`end_key_docid` must be a string.">>)
+    end,
+
+    case Args#mrargs.direction of
+        fwd -> ok;
+        rev -> ok;
+        _ -> mrverror(<<"Invalid direction.">>)
+    end,
+
+    case {Args#mrargs.limit >= 0, Args#mrargs.limit == undefined} of

Review comment:
       I copied this one verbatim from `couch_mrviews_util` https://github.com/apache/couchdb/blob/403d27b16f5209f918766dc62a8b0d039080fd32/src/couch_mrview/src/couch_mrview_util.erl#L523-L527
   
   I think we do want to consider limit=0 as valid




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] bessbd commented on a change in pull request #3503: 3.x applications cleanup

Posted by GitBox <gi...@apache.org>.
bessbd commented on a change in pull request #3503:
URL: https://github.com/apache/couchdb/pull/3503#discussion_r614215852



##########
File path: src/chttpd/src/chttpd_misc.erl
##########
@@ -334,9 +329,33 @@ handle_reload_query_servers_req(#httpd{method='POST'}=Req) ->
 handle_reload_query_servers_req(Req) ->
     send_method_not_allowed(Req, "POST").
 
+handle_uuids_req(#httpd{method='GET'}=Req) ->
+    Max = list_to_integer(config:get("uuids","max_count","1000")),
+    Count = try list_to_integer(couch_httpd:qs_value(Req, "count", "1")) of
+        N when N > Max ->
+            throw({bad_request, <<"count parameter too large">>});
+        N when N < 0 ->
+            throw({bad_request, <<"count must be a positive integer">>});
+        N -> N
+    catch
+        error:badarg ->
+            throw({bad_request, <<"count must be a positive integer">>})
+    end,
+    UUIDs = [couch_uuids:new() || _ <- lists:seq(1, Count)],
+    Etag = couch_httpd:make_etag(UUIDs),
+    couch_httpd:etag_respond(Req, Etag, fun() ->
+        CacheBustingHeaders = [
+            {"Date", couch_util:rfc1123_date()},
+            {"Cache-Control", "no-cache"},
+            % Past date, ON PURPOSE!
+            {"Expires", "Mon, 01 Jan 1990 00:00:00 GMT"},
+            {"Pragma", "no-cache"},
+            {"ETag", Etag}

Review comment:
       If we think an `ETag` is necessary for some compatibility, should we maybe just generate something random here to avoid the hashing computation overhead?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] bessbd commented on a change in pull request #3503: 3.x applications cleanup

Posted by GitBox <gi...@apache.org>.
bessbd commented on a change in pull request #3503:
URL: https://github.com/apache/couchdb/pull/3503#discussion_r614212380



##########
File path: src/couch_views/src/couch_views_validate.erl
##########
@@ -0,0 +1,460 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+%   http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_views_validate).
+
+
+-export([
+    validate_args/1,
+    validate_args/3,
+    validate_ddoc/2
+]).
+
+
+-define(LOWEST_KEY, null).
+-define(HIGHEST_KEY, {<<255, 255, 255, 255>>}).
+
+
+-include_lib("couch/include/couch_db.hrl").
+-include("couch_views.hrl").
+
+
+% There is another almost identical validate_args in couch_views_util. They
+% should probably be merged at some point in the future.
+%
+validate_args(Args) ->
+    GroupLevel = determine_group_level(Args),
+    Reduce = Args#mrargs.reduce,
+    case Reduce == undefined orelse is_boolean(Reduce) of
+        true -> ok;
+        _ -> mrverror(<<"Invalid `reduce` value.">>)
+    end,
+
+    case {Args#mrargs.view_type, Reduce} of
+        {map, true} -> mrverror(<<"Reduce is invalid for map-only views.">>);
+        _ -> ok
+    end,
+
+    case {Args#mrargs.view_type, GroupLevel, Args#mrargs.keys} of
+        {red, exact, _} -> ok;
+        {red, _, KeyList} when is_list(KeyList) ->
+            Msg = <<"Multi-key fetchs for reduce views must use `group=true`">>,
+            mrverror(Msg);
+        _ -> ok
+    end,
+
+    case Args#mrargs.keys of
+        Keys when is_list(Keys) -> ok;
+        undefined -> ok;
+        _ -> mrverror(<<"`keys` must be an array of strings.">>)
+    end,
+
+    case {Args#mrargs.keys, Args#mrargs.start_key,
+          Args#mrargs.end_key} of
+        {undefined, _, _} -> ok;
+        {[], _, _} -> ok;
+        {[_|_], undefined, undefined} -> ok;
+        _ -> mrverror(<<"`keys` is incompatible with `key`"
+                        ", `start_key` and `end_key`">>)
+    end,
+
+    case Args#mrargs.start_key_docid of
+        undefined -> ok;
+        SKDocId0 when is_binary(SKDocId0) -> ok;
+        _ -> mrverror(<<"`start_key_docid` must be a string.">>)
+    end,
+
+    case Args#mrargs.end_key_docid of
+        undefined -> ok;
+        EKDocId0 when is_binary(EKDocId0) -> ok;
+        _ -> mrverror(<<"`end_key_docid` must be a string.">>)
+    end,
+
+    case Args#mrargs.direction of
+        fwd -> ok;
+        rev -> ok;
+        _ -> mrverror(<<"Invalid direction.">>)
+    end,
+
+    case {Args#mrargs.limit >= 0, Args#mrargs.limit == undefined} of

Review comment:
       I can open a follow-up PR, too if you want me to.
   I personally have to problems with doing some "Boy Scout Rule" changes. (If you agree with the suggestion.)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org