Merge pull request #25 from jonasrichard/dev

jonasrichard · jonasrichard · commit 04fb8fa8ce2f · 2015-08-01T11:03:07.000+02:00
Refactoring, support "error" in field names
diff --git a/README.md b/README.md
@@ -144,28 +144,7 @@ In the 3rd parameter of the type definition we can write an option list with whi
 
 #### Proplists to objects
 
-The keys in proplist will be camel case converted and those will be the name of the attribute in the JSON object. In order that the decoder can create proplist from an object a `__type` property will be added as an extra.
-
-```erlang
--json({square, {number, side}}).
--json({shapes, {list, data}}).
--json({canvas, {proplist, opts}).
-
-to_json({canvas, [{width, 500}, {height, 300}, {bit_depth: 16}]}).
-```
-
-It is a simple object
-
-```json
-{
-  "__type": "proplist",
-  "width": 500,
-  "height": 300,
-  "bitDepth": 16
- }
-```
-
-Proplist is not a ready feature, since when property values are not numbers it doesn't work.
+In older version there was some proplist support but it was limited and the encoding/decoding wasn't clear. In the new version proplist can be converted by a generic rule, but the conversion functions need to be implemented by the developer (since there is no clear generic algorithm how to convert proplist to json).
 
 #### Transient fields
 
diff --git a/src/ejson_decode.erl b/src/ejson_decode.erl
@@ -23,8 +23,6 @@
 -export([decode/2,
          decode/3]).
 
--include_lib("eunit/include/eunit.hrl").
-
 %% TODO:
 %% exact_value should return with {ok, Value} or {error, Reason}, so
 %% every time when we extract, we also need to case pattern match.
@@ -104,7 +102,7 @@ extract_fields([Field | F], AttrList, Opts) ->
                     case extract_value(Field, Value, Opts) of
                         {error, _} = Error ->
                             Error;
-                        Extracted ->
+                        {ok, Extracted} ->
                             case maybe_post_process(Field, Extracted) of
                                 {ok, NewVal} ->
                                     [NewVal | extract_fields(F, AttrList, Opts)];
@@ -145,89 +143,84 @@ extract_value(Rule, Value, Opts) ->
             extract_list(Value, [], Opts);
         {list, _, FieldOpts} ->
             extract_list(Value, FieldOpts, Opts);
-        {generic, _, _EncFun, DecFun} ->
-            extract_generic(Value, DecFun);
-        {proplist, _} ->
-            %% TODO proper conversion here!
-            undefined;
+        {generic, _Name, _FieldOpts} ->
+            %% Let the post processor function makes the conversion
+            {ok, Value};
         {const, _, _} ->
-            undefined
+            {ok, undefined}
     end.
 
 extract_atom(null) ->
-    undefined;
+    {ok, undefined};
 extract_atom(Value) ->
-    binary_to_atom(Value, utf8).
+    {ok, binary_to_atom(Value, utf8)}.
 
 extract_binary(null) ->
-    undefined;
+    {ok, undefined};
 extract_binary(Value) ->
-    Value.
+    {ok, Value}.
 
 extract_boolean(null) ->
-    undefined;
+    {ok, undefined};
 extract_boolean(Value) ->
-    Value.
+    {ok, Value}.
 
 extract_number(null) ->
-    undefined;
+    {ok, undefined};
 extract_number(Value) ->
-    Value.
+    {ok, Value}.
 
 extract_string(null) ->
-    undefined;
+    {ok, undefined};
 extract_string(Value) ->
-    unicode:characters_to_list(Value, utf8).
+    {ok, unicode:characters_to_list(Value, utf8)}.
 
 extract_record(null, FieldOpts, Opts) ->
     case proplists:get_value(default, FieldOpts) of
         undefined ->
-            undefined;
+            {ok, undefined};
         Default ->
             extract_record(Default, FieldOpts, Opts)
     end;
 extract_record(Value, FieldOpts, Opts) ->
     case proplists:get_value(type, FieldOpts) of
         undefined ->
-            decode1(Value, Opts);
+            {ok, decode1(Value, Opts)};
         Type ->
-            decode1(Value, Opts, Type)
+            {ok, decode1(Value, Opts, Type)}
     end.
 
 extract_list(null, _FieldOpts, _Opts) ->
-    undefined;
+    {ok, undefined};
 extract_list(Value, FieldOpts, Opts) ->
     case proplists:get_value(type, FieldOpts) of
         undefined ->
             %% No target type for list element, it can be an attrlist
             %% or a primitive value
-            lists:map(
+            L = lists:map(
               fun(V) when is_list(V) ->
                       {ok, D} = decode(V, Opts),
                       %% TODO make an error case and gives back error
                       D;
                  (V) ->
                       V
-              end, Value);
+              end, Value),
+            {ok, L};
         Type ->
-            [decode1(V, Opts, Type) || V <- Value]
-    end.
-
-extract_generic(Value, {M, F}) ->
-    try erlang:apply(M, F, [Value]) of
-        Val ->
-            Val
-    catch
-        E:R ->
-            {error, {field_run, {M, F}, {E, R}}}
+            {ok, [decode1(V, Opts, Type) || V <- Value]}
     end.
 
 maybe_post_process({const, _Name, _Const}, Value) ->
     {ok, Value};
-maybe_post_process({_Type, Name, FieldOpts}, Value) ->
+maybe_post_process({Type, Name, FieldOpts}, Value) ->
     case lists:keyfind(post_decode, 1, FieldOpts) of
         false ->
-            {ok, Value};
+            case Type of
+                generic ->
+                    {error, {no_post_decode, Name, Value}};
+                _ ->
+                    {ok, Value}
+            end;
         {post_decode, {M, F}} ->
             try erlang:apply(M, F, [Value]) of
                 NewVal ->
diff --git a/src/ejson_encode.erl b/src/ejson_encode.erl
@@ -99,20 +99,20 @@ convert([], _Tuple, _Opts, Result) ->
 convert([{Name, Value} | T], Tuple, Opts, Result) ->
     case maybe_pre_process(Name, Tuple, Value) of
         {ok, PreProcessed} ->
-            case apply_rule(Name, Tuple, PreProcessed, Opts) of
+            case apply_rule(Name, PreProcessed, Opts) of
                 undefined ->
                     convert(T, Tuple, Opts, Result);
                 {error, _} = Error ->
                     Error;
-                {NewName, NewValue} ->
+                {ok, {NewName, NewValue}} ->
                     convert(T, Tuple, Opts, [{?BIN(NewName), NewValue} | Result])
             end;
         {error, _} = Error2 ->
             Error2
     end.
 
 %% Generate jsx attribute from ejson field
-apply_rule(Name, Tuple, Value, Opts) ->
+apply_rule(Name, Value, Opts) ->
     case Name of
         skip ->
             undefined;
@@ -144,90 +144,77 @@ apply_rule(Name, Tuple, Value, Opts) ->
             list_rule(AttrName, Value, Opts);
         {list, AttrName, _FieldOpts} ->
             list_rule(AttrName, Value, Opts);
-        {generic, AttrName, EncFun, _DecFun} ->
-            generic_rule(AttrName, EncFun, Tuple, Value);
-        {proplist, AttrName} ->
-            proplist_rule(AttrName, Value);
+        {generic, AttrName, _FieldOpts} ->
+            %% Generic encoding is handled in pre_process phase
+            {ok, {AttrName, Value}};
         {const, AttrName, Const} ->
-            {AttrName, encode1(Const, Opts)};
+            {ok, {AttrName, encode1(Const, Opts)}};
         AttrName ->
             {error, {invalid_field_rule, AttrName, Name}}
     end.
 
 boolean_rule(AttrName, undefined) ->
-    {AttrName, null};
+    {ok, {AttrName, null}};
 boolean_rule(AttrName, Value) when is_boolean(Value) ->
-    {AttrName, Value};
+    {ok, {AttrName, Value}};
 boolean_rule(AttrName, Value) ->
     {error, {boolean_value_expected, AttrName, Value}}.
 
 number_rule(AttrName, undefined) ->
-    {AttrName, null};
+    {ok, {AttrName, null}};
 number_rule(AttrName, Value) when is_number(Value) ->
-    {AttrName, Value};
+    {ok, {AttrName, Value}};
 number_rule(AttrName, Value) ->
     {error, {numeric_value_expected, AttrName, Value}}.
 
 atom_rule(AttrName, undefined) ->
-    {AttrName, null};
+    {ok, {AttrName, null}};
 atom_rule(AttrName, Value) when is_atom(Value) ->
-    {AttrName, atom_to_binary(Value, utf8)};
+    {ok, {AttrName, atom_to_binary(Value, utf8)}};
 atom_rule(AttrName, Value) ->
     {error, {atom_value_expected, AttrName, Value}}.
 
 binary_rule(AttrName, undefined) ->
-    {AttrName, null};
+    {ok, {AttrName, null}};
 binary_rule(AttrName, Value) when is_binary(Value) ->
-    {AttrName, Value};
+    {ok, {AttrName, Value}};
 binary_rule(AttrName, Value) ->
     {error, {binary_value_expected, AttrName, Value}}.
 
 string_rule(AttrName, undefined) ->
-    {AttrName, null};
+    {ok, {AttrName, null}};
 string_rule(AttrName, Value) when is_list(Value) ->
-    {AttrName, unicode:characters_to_binary(Value)};
+    {ok, {AttrName, unicode:characters_to_binary(Value)}};
 string_rule(AttrName, Value) ->
     {error, {string_value_expected, AttrName, Value}}.
 
 record_rule(AttrName, undefined, _FieldOpts, _Opts) ->
-    {AttrName, null};
+    {ok, {AttrName, null}};
 record_rule(AttrName, Value, _FieldOpts, Opts) when is_tuple(Value) ->
-    {AttrName, encode1(Value, Opts)};
+    {ok, {AttrName, encode1(Value, Opts)}};
 record_rule(AttrName, Value, _FieldOpts, _Opts) ->
     {error, {record_value_expected, AttrName, Value}}.
 
 list_rule(AttrName, undefined, _Opts) ->
-    {AttrName, null};
+    {ok, {AttrName, null}};
 list_rule(AttrName, Value, Opts) when is_list(Value) ->
     List = [encode1(V, Opts) || V <- Value],
-    {AttrName, List};
+    {ok, {AttrName, List}};
 list_rule(AttrName, Value, _Opts) ->
     {error, {list_value_expected, AttrName, Value}}.
 
-generic_rule(AttrName, {M, F}, Tuple, Value) ->
-    try erlang:apply(M, F, [Tuple, Value]) of
-        Val ->
-            {AttrName, Val}
-    catch
-        E:R ->
-            {error, {generic, {M, F}, {E, R}}}
-    end.
-
-proplist_rule(AttrName, Value) ->
-    Vals = lists:map(
-             fun({Prop, Val}) ->
-                     {ejson_util:atom_to_binary_cc(Prop), Val};
-                (Prop) ->
-                     {ejson_util:atom_to_binary_cc(Prop), true}
-             end, Value),
-    {AttrName, [{<<"__type">>, <<"proplist">>} | Vals]}.
-
 maybe_pre_process({const, _Name, _Const}, _Tuple, Value) ->
     {ok, Value};
-maybe_pre_process({_Type, Name, FieldOpts}, Tuple, Value) ->
+maybe_pre_process({Type, Name, FieldOpts}, Tuple, Value) ->
     case lists:keyfind(pre_encode, 1, FieldOpts) of
         false ->
-            {ok, Value};
+            case Type of
+                generic ->
+                    %% In case of generic pre_encode is mandatory
+                    {error, {no_pre_encode, Name, Value}};
+                _ ->
+                    {ok, Value}
+            end;
         {pre_encode, {M, F}} ->
             try erlang:apply(M, F, [Tuple, Value]) of
                 Val ->
diff --git a/src/ejson_util.erl b/src/ejson_util.erl
@@ -78,11 +78,7 @@ get_field_name({Type, Field, _FieldOpts}) when Type =:= atom orelse
                                                Type =:= record orelse
                                                Type =:= string ->
     Field;
-get_field_name({field_fun, Field, _EncFun, _DecFun}) ->
-    Field;
-get_field_name({field_fun, Field, _EncFun, _DecFun, _FieldOpts}) ->
-    Field;
-get_field_name({rec_fun, Field, _EncFun}) ->
+get_field_name({generic, Field, _FieldOpts}) ->
     Field;
 get_field_name(Field) ->
     {error, {invalid_field_rule, Field}}.
diff --git a/test/ejson_basic_test.erl b/test/ejson_basic_test.erl
@@ -35,6 +35,23 @@ pre_post_callback_test_() ->
     {ok, D} = ejson_decode:decode(E, Opts),
     ?_assertEqual(Record, D).
 
+to_jsx(_Tuple, {High, Low}) ->
+    [{<<"high">>, High}, {<<"low">>, Low}].
+
+from_jsx(Attrs) ->
+    {_, High} = lists:keyfind(<<"high">>, 1, Attrs),
+    {_, Low} = lists:keyfind(<<"low">>, 1, Attrs),
+    {High, Low}.
+
+generic_test_() ->
+    Opts = [{item, {generic, count, [{pre_encode, {?MODULE, to_jsx}},
+                                     {post_decode, {?MODULE, from_jsx}}]}}],
+    Record = {item, {15, 2}},
+    {ok, E} = ejson_encode:encode(Record, Opts),
+    ?debugVal(E),
+    {ok, D} = ejson_decode:decode(E, Opts),
+    ?_assertEqual(Record, D).
+
 -define(TYPE(Val, Opts, Err),
         ?_assertEqual(Exp(Val, Err), Enc(Val, Opts))).
 
diff --git a/test/ejson_error.erl b/test/ejson_error.erl
@@ -9,14 +9,6 @@ no_record_test_() ->
     ?_assertEqual({error, {no_such_record, person}},
                   ejson:to_json_modules({person, "Joe"}, [?MODULE])).
 
-proplist_test_() ->
-    Rec = {rec, [{p1, 10}, {p2, false}, {p3, 4.5}]},
-    {ok, Enc} = ejson_encode:encode(Rec, [{rec, {proplist, "test"}}]),
-    {_, Props} = lists:keyfind(<<"test">>, 1, Enc),
-    [?_assert(lists:member({<<"p1">>, 10}, Props)),
-     ?_assert(lists:member({<<"p2">>, false}, Props)),
-     ?_assert(lists:member({<<"p3">>, 4.5}, Props))].
-
 duplicate_record_test_() ->
     ?_assertEqual({error, {duplicate_records, [a]}},
                   ejson_encode:encode(1, [{a, b}, {a, {atom, c}}, {a, d}])).
@@ -29,7 +21,14 @@ duplicate_field_test_() ->
         end,
     [
      ?_assert(F([{rec, {string, a}, {binary, a}}])),
-     ?_assert(F([{rec, {number, a}, {rec_fun, a, f}}])),
-     ?_assert(F([{rec, {boolean, a}, {field_fun, a, ff, ff2}}])),
+     ?_assert(F([{rec, {number, a}, {list, a}}])),
+     ?_assert(F([{rec, {boolean, a}, {generic, a, []}}])),
      ?_assert(F([{rec, {const, a, 1}, {list, a}}]))
     ].
+
+error_conflict_test_() ->
+    Opts = [{message, {string, error, [{default, "No errors"}]}}],
+    Record = {message, "Syntax error"},
+    {ok, E} = ejson_encode:encode(Record, Opts),
+    {ok, D} = ejson_decode:decode(E, Opts),
+    ?_assertEqual(Record, D).
diff --git a/test/ejson_list_test.erl b/test/ejson_list_test.erl
diff --git a/test/ejson_prop.erl b/test/ejson_prop.erl
