couchdb-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kxe...@apache.org
Subject [1/2] couch-mrview commit: updated refs/heads/master to 6c9833d
Date Mon, 21 Dec 2015 21:39:26 GMT
Repository: couchdb-couch-mrview
Updated Branches:
  refs/heads/master c3bed460e -> 6c9833d66


Rewrite ddoc validation routines

While it solve the own problem well, it contains few design issues that
makes unable to keep it for further updates:
1. It's type-centric validation. However, our ddocs have quite complex
structure and validation logic, so it eventually have to write special
clause for most of the validated fields turning it into field-centric.
2. It mixes logic of type checking and relation checking.

Here we turn it into rule based validation where each rule is a path of
field-type pairs while all relation checks are moved into own functions.

Moreover, we can now exactly say the path for the field that caused
issues.


Project: http://git-wip-us.apache.org/repos/asf/couchdb-couch-mrview/repo
Commit: http://git-wip-us.apache.org/repos/asf/couchdb-couch-mrview/commit/946d942a
Tree: http://git-wip-us.apache.org/repos/asf/couchdb-couch-mrview/tree/946d942a
Diff: http://git-wip-us.apache.org/repos/asf/couchdb-couch-mrview/diff/946d942a

Branch: refs/heads/master
Commit: 946d942a481c7bae8b1f8045a8909bf921a20e3a
Parents: c3bed46
Author: Alexander Shorin <kxepal@apache.org>
Authored: Tue Nov 10 20:44:25 2015 +0300
Committer: Alexander Shorin <kxepal@apache.org>
Committed: Tue Nov 10 20:44:25 2015 +0300

----------------------------------------------------------------------
 src/couch_mrview.erl                        | 192 ++++++++++++-----------
 test/couch_mrview_ddoc_validation_tests.erl |  54 +++----
 2 files changed, 118 insertions(+), 128 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/couchdb-couch-mrview/blob/946d942a/src/couch_mrview.erl
----------------------------------------------------------------------
diff --git a/src/couch_mrview.erl b/src/couch_mrview.erl
index 40c3fdf..39537cf 100644
--- a/src/couch_mrview.erl
+++ b/src/couch_mrview.erl
@@ -47,111 +47,119 @@
 
 
 
-%% Validate "views" name : value mapping.
-%%
-%% In the most common case name is the view name and
-%% value is an object that must have a "map" function,
-%% and an optional "reduce" function. "map" and "reduce"
-%% values should be strings (function contents), except
-%% in case when langauge is <<query>> then maps must
-%% be objects.
-%%
-%% "lib" is a special case. The value
-%% of "lib" is an object that will contain libary code
-%% so it not validated.
-%%
-validate_view(<<"lib">>, {_Libs}, _Language)->
+validate_ddoc_fields(DDoc) ->
+    MapFuncType = map_function_type(DDoc),
+    lists:foreach(fun(Path) ->
+        validate_ddoc_fields(DDoc, Path)
+    end, [
+        [{<<"filters">>, object}, {any, string}],
+        [{<<"language">>, string}],
+        [{<<"lists">>, object}, {any, string}],
+        [{<<"options">>, object}],
+        [{<<"rewrites">>, array}],
+        [{<<"shows">>, object}, {any, string}],
+        [{<<"updates">>, object}, {any, string}],
+        [{<<"validate_doc_update">>, string}],
+        [{<<"views">>, object}, {<<"lib">>, object}],
+        [{<<"views">>, object}, {any, object}, {<<"map">>, MapFuncType}],
+        [{<<"views">>, object}, {any, object}, {<<"reduce">>, string}]
+    ]),
+    require_map_function_for_views(DDoc),
+    ok.
+
+require_map_function_for_views({Props}) ->
+    case couch_util:get_value(<<"views">>, Props) of
+        undefined -> ok;
+        {Views} ->
+            lists:foreach(fun
+                ({<<"lib">>, _}) -> ok;
+                ({Key, {Value}}) ->
+                    case couch_util:get_value(<<"map">>, Value) of
+                        undefined -> throw({invalid_design_doc,
+                            <<"View `", Key/binary, "` must contain map function">>});
+                        _ -> ok
+                    end
+            end, Views),
+            ok
+    end.
+
+validate_ddoc_fields(DDoc, Path) ->
+    case validate_ddoc_fields(DDoc, Path, []) of
+        ok -> ok;
+        {error, {FailedPath0, Type0}} ->
+            FailedPath = iolist_to_binary(join(FailedPath0, <<".">>)),
+            Type = ?l2b(atom_to_list(Type0)),
+            throw({invalid_design_doc,
+                  <<"`", FailedPath/binary, "` field must have ",
+                     Type/binary, " type">>})
+    end.
+
+validate_ddoc_fields(undefined, _, _) ->
     ok;
-validate_view(VName, {Views}, Language) ->
-    case {couch_util:get_value(<<"map">>, Views), Language}  of
-        {{[_ | _]}, <<"query">>} ->
-            ok;
-        {_, <<"query">>} ->
-            Err1 = <<"`map` in ", VName/binary, " for queries must be an object">>,
-            throw({invalid_design_doc,Err1});
-        {Mapval, _} when is_binary(Mapval)  ->
-            ok;
-        {undefined, _} ->
-            Err0 = <<"View ",VName/binary, " must have a map function">>,
-            throw ({invalid_design_doc, Err0});
-        {_, _} ->
-            Err2 = <<"`map` in ", VName/binary, " must be a string">>,
-            throw({invalid_design_doc, Err2})
-    end,
-    case couch_util:get_value(<<"reduce">>, Views) of
-        RedVal when is_binary(RedVal) ->
-            ok;
-        undefined ->
-            ok; % note: unlike map,  reduce function is optional
-        _ ->
-            Err3 = <<"`reduce` in ", VName/binary, " must be a string">>,
-            throw({invalid_design_doc, Err3})
-    end,
+validate_ddoc_fields(_, [], _) ->
     ok;
-validate_view(VName, _, _Language) ->
-    throw({invalid_design_doc, <<"View ", VName/binary, " must be an object">>}).
-
-
-%% Validate top level design document objects.
-%%
-%% Most cases (shows,lists,filters,...) will be
-%% function_name : function_contents(string) mappings.
-%% For example, lists can look like:
-%% {
-%%    "lst1" : "function(head, req){...}",
-%%    "lst2" : "function(head, req){...}"
-%% }
-%%
-%% With 2 excpetions:
-%%  - views :  Validated by a special function.
-%%  - options : Allowed to contain non-string values.
-%%
-validate_opt_object(<<"views">>, {Views}, Language)->
-    [validate_view(VName, ViewObj, Language) || {VName, ViewObj} <- Views],
+validate_ddoc_fields({KVS}=Props, [{any, Type} | Rest], Acc) ->
+    lists:foldl(fun
+        ({Key, _}, ok) ->
+            validate_ddoc_fields(Props, [{Key, Type} | Rest], Acc);
+        ({_, _}, {error, _}=Error) ->
+            Error
+    end, ok, KVS);
+validate_ddoc_fields({KVS}=Props, [{Key, Type} | Rest], Acc) ->
+    case validate_ddoc_field(Props, {Key, Type}) of
+        ok ->
+            validate_ddoc_fields(couch_util:get_value(Key, KVS),
+                                 Rest,
+                                 [Key | Acc]);
+        error ->
+            {error, {[Key | Acc], Type}};
+        {error, Key1} ->
+            {error, {[Key1 | Acc], Type}}
+    end.
+
+validate_ddoc_field(undefined, Type) when is_atom(Type) ->
     ok;
-validate_opt_object(<<"options">>, {_}, _Language)->
+validate_ddoc_field(_, any) ->
     ok;
-validate_opt_object(Section, {Fields}, _Language) ->
-    [validate_opt_string(FName, FVal, Section) || {FName, FVal} <- Fields],
+validate_ddoc_field(Value, string) when is_binary(Value) ->
     ok;
-validate_opt_object(_, undefined, _Language) ->
+validate_ddoc_field(Value, array) when is_list(Value) ->
     ok;
-validate_opt_object(FieldName, _, _Language) ->
-    throw({invalid_design_doc, <<"`", FieldName/binary, "` is not an object">>}).
-
-
-%% If this field value is present, it must be a string
-validate_opt_string(FName, FVal)->
-    validate_opt_string(FName, FVal, <<"design doc">>).
-
-validate_opt_string(_, FVal, _) when is_binary(FVal) ->
+validate_ddoc_field({Value}, object) when is_list(Value) ->
     ok;
-validate_opt_string(_, undefined, _) ->
+validate_ddoc_field({Props}, {any, Type}) ->
+    validate_ddoc_field1(Props, Type);
+validate_ddoc_field({Props}, {Key, Type}) ->
+    validate_ddoc_field(couch_util:get_value(Key, Props), Type);
+validate_ddoc_field(_, _) ->
+    error.
+
+validate_ddoc_field1([], _) ->
     ok;
-validate_opt_string(FName, _, Section) ->
-    ErrMsg = <<"`", FName/binary, "` in ", Section/binary, " is not a string">>,
-    throw({invalid_design_doc, ErrMsg}).
+validate_ddoc_field1([{Key, Value} | Rest], Type) ->
+    case validate_ddoc_field(Value, Type) of
+        ok ->
+            validate_ddoc_field1(Rest, Type);
+        error ->
+            {error, Key}
+    end.
 
+map_function_type({Props}) ->
+    case couch_util:get_value(<<"language">>, Props) of
+        <<"query">> -> object;
+        _ -> string
+    end.
 
-%% If this field is present it, must be an array
-validate_opt_array(_, ArrVal) when is_list(ArrVal) ->
-    ok;
-validate_opt_array(_, undefined) ->
-    ok;
-validate_opt_array(FieldName, _) ->
-    throw({invalid_design_doc, <<"`", FieldName/binary, "` is not an array">>}).
+join(L, Sep) ->
+    join(L, Sep, []).
+join([H|[]], _, Acc) ->
+    [H | Acc];
+join([H|T], Sep, Acc) ->
+    join(T, Sep, [Sep, H | Acc]).
 
 
 validate(DbName,  DDoc) ->
-    {Fields} = DDoc#doc.body,
-    ObjFields =  [<<"options">>, <<"filters">>, <<"lists">>,
-                  <<"shows">>, <<"updates">>, <<"views">>],
-    StringFields = [<<"language">>, <<"validate_doc_update">>],
-    ArrayFields = [<<"rewrites">>],
-    [validate_opt_string(F, couch_util:get_value(F, Fields)) || F <- StringFields],
-    [validate_opt_array(F, couch_util:get_value(F, Fields))  || F <- ArrayFields],
-    Language = couch_util:get_value(<<"language">>, Fields),
-    [validate_opt_object(F, couch_util:get_value(F, Fields), Language) || F <- ObjFields],
+    ok = validate_ddoc_fields(DDoc#doc.body),
     GetName = fun
         (#mrview{map_names = [Name | _]}) -> Name;
         (#mrview{reduce_funs = [{Name, _} | _]}) -> Name;

http://git-wip-us.apache.org/repos/asf/couchdb-couch-mrview/blob/946d942a/test/couch_mrview_ddoc_validation_tests.erl
----------------------------------------------------------------------
diff --git a/test/couch_mrview_ddoc_validation_tests.erl b/test/couch_mrview_ddoc_validation_tests.erl
index 279d4e2..459c959 100644
--- a/test/couch_mrview_ddoc_validation_tests.erl
+++ b/test/couch_mrview_ddoc_validation_tests.erl
@@ -117,8 +117,7 @@ should_reject_non_object_options(Db) ->
         {<<"_id">>, <<"_design/should_reject_non_object_options">>},
         {<<"options">>, <<"invalid">>}
     ]}),
-    ?_assertThrow({bad_request,invalid_design_doc,
-                   <<"`options` is not an object">>},
+    ?_assertThrow({bad_request, invalid_design_doc, _},
                   couch_db:update_doc(Db, Doc, [])).
 
 should_reject_non_object_filters(Db) ->
@@ -126,8 +125,7 @@ should_reject_non_object_filters(Db) ->
         {<<"_id">>, <<"_design/should_reject_non_object_filters">>},
         {<<"filters">>, <<"invalid">>}
     ]}),
-    ?_assertThrow({bad_request,invalid_design_doc,
-                   <<"`filters` is not an object">>},
+    ?_assertThrow({bad_request, invalid_design_doc, _},
                   couch_db:update_doc(Db, Doc, [])).
 
 should_reject_non_object_lists(Db) ->
@@ -135,8 +133,7 @@ should_reject_non_object_lists(Db) ->
         {<<"_id">>, <<"_design/should_reject_non_object_lists">>},
         {<<"lists">>, <<"invalid">>}
     ]}),
-    ?_assertThrow({bad_request,invalid_design_doc,
-                   <<"`lists` is not an object">>},
+    ?_assertThrow({bad_request, invalid_design_doc, _},
                   couch_db:update_doc(Db, Doc, [])).
 
 should_reject_non_object_shows(Db) ->
@@ -144,8 +141,7 @@ should_reject_non_object_shows(Db) ->
         {<<"_id">>, <<"_design/should_reject_non_object_shows">>},
         {<<"shows">>, <<"invalid">>}
     ]}),
-    ?_assertThrow({bad_request,invalid_design_doc,
-                   <<"`shows` is not an object">>},
+    ?_assertThrow({bad_request, invalid_design_doc, _},
                   couch_db:update_doc(Db, Doc, [])).
 
 should_reject_non_object_updates(Db) ->
@@ -153,8 +149,7 @@ should_reject_non_object_updates(Db) ->
         {<<"_id">>, <<"_design/should_reject_non_object_updates">>},
         {<<"updates">>, <<"invalid">>}
     ]}),
-    ?_assertThrow({bad_request,invalid_design_doc,
-                   <<"`updates` is not an object">>},
+    ?_assertThrow({bad_request, invalid_design_doc, _},
                   couch_db:update_doc(Db, Doc, [])).
 
 should_reject_non_object_views(Db) ->
@@ -162,8 +157,7 @@ should_reject_non_object_views(Db) ->
         {<<"_id">>, <<"_design/should_reject_non_object_views">>},
         {<<"views">>, <<"invalid">>}
     ]}),
-    ?_assertThrow({bad_request,invalid_design_doc,
-                   <<"`views` is not an object">>},
+    ?_assertThrow({bad_request, invalid_design_doc, _},
                   couch_db:update_doc(Db, Doc, [])).
 
 should_reject_non_string_language(Db) ->
@@ -171,8 +165,7 @@ should_reject_non_string_language(Db) ->
         {<<"_id">>, <<"_design/should_reject_non_string_language">>},
         {<<"language">>, 1}
     ]}),
-    ?_assertThrow({bad_request,invalid_design_doc,
-                   <<"`language` in design doc is not a string">>},
+    ?_assertThrow({bad_request, invalid_design_doc, _},
                   couch_db:update_doc(Db, Doc, [])).
 
 should_reject_non_string_validate_doc_update(Db) ->
@@ -180,8 +173,7 @@ should_reject_non_string_validate_doc_update(Db) ->
         {<<"_id">>, <<"_design/should_reject_non_string_vdu">>},
         {<<"validate_doc_update">>, 1}
     ]}),
-    ?_assertThrow({bad_request,invalid_design_doc,
-                   <<"`validate_doc_update` in design doc is not a string">>},
+    ?_assertThrow({bad_request, invalid_design_doc, _},
                   couch_db:update_doc(Db, Doc, [])).
 
 should_reject_non_array_rewrites(Db) ->
@@ -189,8 +181,7 @@ should_reject_non_array_rewrites(Db) ->
         {<<"_id">>, <<"_design/should_reject_non_array_rewrites">>},
         {<<"rewrites">>, <<"invalid">>}
     ]}),
-    ?_assertThrow({bad_request,invalid_design_doc,
-                   <<"`rewrites` is not an array">>},
+    ?_assertThrow({bad_request, invalid_design_doc, _},
                   couch_db:update_doc(Db, Doc, [])).
 
 should_accept_option(Db) ->
@@ -219,8 +210,7 @@ should_reject_non_string_filter_function(Db) ->
         {<<"_id">>, <<"_design/should_reject_non_string_filter_function">>},
         {<<"filters">>, {[ {<<"filter1">>, 1} ]}}
     ]}),
-    ?_assertThrow({bad_request,invalid_design_doc,
-                  <<"`filter1` in filters is not a string">> },
+    ?_assertThrow({bad_request, invalid_design_doc, _},
                   couch_db:update_doc(Db, Doc, [])).
 
 should_accept_list(Db) ->
@@ -235,8 +225,7 @@ should_reject_non_string_list_function(Db) ->
         {<<"_id">>, <<"_design/should_reject_non_string_list_function">>},
         {<<"lists">>, {[ {<<"list1">>, 1} ]}}
     ]}),
-    ?_assertThrow({bad_request,invalid_design_doc,
-                  <<"`list1` in lists is not a string">> },
+    ?_assertThrow({bad_request, invalid_design_doc, _},
                   couch_db:update_doc(Db, Doc, [])).
 
 should_accept_show(Db) ->
@@ -251,8 +240,7 @@ should_reject_non_string_show_function(Db) ->
         {<<"_id">>, <<"_design/should_reject_non_string_show_function">>},
         {<<"shows">>, {[ {<<"show1">>, 1} ]}}
     ]}),
-    ?_assertThrow({bad_request,invalid_design_doc,
-                  <<"`show1` in shows is not a string">> },
+    ?_assertThrow({bad_request, invalid_design_doc, _},
                   couch_db:update_doc(Db, Doc, [])).
 
 should_accept_update(Db) ->
@@ -267,8 +255,7 @@ should_reject_non_string_update_function(Db) ->
         {<<"_id">>, <<"_design/should_reject_non_string_update_function">>},
         {<<"updates">>, {[ {<<"update1">>, 1} ]}}
     ]}),
-    ?_assertThrow({bad_request,invalid_design_doc,
-                  <<"`update1` in updates is not a string">> },
+    ?_assertThrow({bad_request, invalid_design_doc, _},
                   couch_db:update_doc(Db, Doc, [])).
 
 should_accept_view(Db) ->
@@ -311,8 +298,7 @@ should_reject_view_that_is_not_an_object(Db) ->
         {<<"_id">>, <<"_design/should_reject_non_object_view">>},
         {<<"views">>, {[{<<"view1">>, <<"thisisbad">>}]}}
     ]}),
-    ?_assertThrow({bad_request,invalid_design_doc,
-                  <<"View view1 must be an object">>},
+    ?_assertThrow({bad_request, invalid_design_doc, _},
                   couch_db:update_doc(Db, Doc, [])).
 
 should_reject_view_without_map_function(Db) ->
@@ -322,8 +308,7 @@ should_reject_view_without_map_function(Db) ->
                          {<<"view1">>, {[]}}
                        ]}}
     ]}),
-    ?_assertThrow({bad_request,invalid_design_doc,
-                  <<"View view1 must have a map function">>},
+    ?_assertThrow({bad_request, invalid_design_doc, _},
                   couch_db:update_doc(Db, Doc, [])).
 
 
@@ -336,8 +321,7 @@ should_reject_view_with_non_string_map_function(Db) ->
                                         ]}}
                        ]}}
     ]}),
-    ?_assertThrow({bad_request,invalid_design_doc,
-                  <<"`map` in view1 must be a string">>},
+    ?_assertThrow({bad_request, invalid_design_doc, _},
                   couch_db:update_doc(Db, Doc, [])).
 
 should_reject_view_with_non_string_reduce_function(Db) ->
@@ -350,8 +334,7 @@ should_reject_view_with_non_string_reduce_function(Db) ->
                                         ]}}
                        ]}}
     ]}),
-    ?_assertThrow({bad_request,invalid_design_doc,
-                  <<"`reduce` in view1 must be a string">>},
+    ?_assertThrow({bad_request, invalid_design_doc, _},
                   couch_db:update_doc(Db, Doc, [])).
 
 should_accept_any_in_lib(Db) ->
@@ -392,6 +375,5 @@ should_reject_map_non_objects_for_queries(Db) ->
             ]}}
         ]}}
     ]}),
-    ?_assertThrow({bad_request,invalid_design_doc,
-                   <<"`map` in view1 for queries must be an object">>},
+    ?_assertThrow({bad_request, invalid_design_doc, _},
                   couch_db:update_doc(Db, Doc, [])).


Mime
View raw message