Merge branch 'anders/diameter/decode_format/OTP-14511' into maint

* anders/diameter/decode_format/OTP-14511:
  Map less in traffic suite
  Fix decode_format doc oversights
  Rename decode_format false to none
  Tweak {decode_format, false} semantics
  Fix dialyzer spec

9 files changed, 100 insertions, 107 deletions

diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml
index b7be184058..0169afb619 100644
--- a/lib/diameter/doc/src/diameter.xml
+++ b/lib/diameter/doc/src/diameter.xml
@@ -799,7 +799,7 @@ be matched by corresponding &capability; configuration, of
 
 <tag>
 <marker id="decode_format"/>
-<c>{decode_format, record | list | map | false}</c></tag>
+<c>{decode_format, record | list | map | none}</c></tag>
 <item>
 <p>
 The format of decoded messages and grouped AVPs in the <c>msg</c> field
@@ -808,10 +808,10 @@ records respectively.
 If <c>record</c> then a record whose definition is generated from the
 dictionary file in question.
 If <c>list</c> or <c>map</c> then a <c>[Name | Avps]</c> pair where
-<c>Avps</c> is either a list of AVP name/values pairs or a map keyed on
+<c>Avps</c> is a list of AVP name/values pairs or a map keyed on
 AVP names respectively.
-If <c>false</c> then the representation is omitted and <c>msg</c> and
-<c>value</c> fields are set to <c>false</c>.
+If <c>none</c> then the atom-value message name, or <c>undefined</c>
+for a Grouped AVP.
 See also &codec_message;.</p>
 
 <p>
diff --git a/lib/diameter/doc/src/diameter_codec.xml b/lib/diameter/doc/src/diameter_codec.xml
index 0846334d23..5124b49484 100644
--- a/lib/diameter/doc/src/diameter_codec.xml
+++ b/lib/diameter/doc/src/diameter_codec.xml
@@ -4,7 +4,10 @@
   '<seealso marker="diameter_dict#MESSAGE_RECORDS">diameter_dict(4)</seealso>'>
   <!ENTITY types
   '<seealso marker="diameter_dict#DATA_TYPES">diameter_dict(4)</seealso>'>
-  <!ENTITY % also SYSTEM "seealso.ent" >
+  <!ENTITY decode_format
+  '<seealso marker="diameter#decode_format">decode format</seealso>'>
+
+<!ENTITY % also SYSTEM "seealso.ent" >
   <!ENTITY % here SYSTEM "seehere.ent" >
   %also;
   %here;
@@ -145,7 +148,8 @@ question.</p>
 <p>
 The decoded value of an AVP.
 Will be <c>undefined</c> on decode if the data bytes could
-not be decoded or the AVP is unknown.
+not be decoded, the AVP is unknown, or if the &decode_format; is
+<c>none</c>.
 The type of a decoded value is as document in &types;.</p>
 </item>
 
@@ -243,8 +247,7 @@ Equivalently, a message can also be encoded as a list whose head is
 the atom-valued message name (as specified in the relevant dictionary
 file) and whose tail is either a list of AVP name/values
 pairs or a map with values keyed on AVP names.
-The format at decode is determined by &mod_service_opt;
-<c>decode_format</c>.
+The format at decode is determined by &mod_decode_format;.
 Any of the formats is accepted at encode.</p>
 
 <p>
@@ -288,15 +291,16 @@ value other than <c>undefined</c>.</p>
 <item>
 <p>
 The incoming/outgoing message.
-For an incoming message, a record if the message can be
-decoded in a non-relay application, <c>undefined</c> otherwise.
+For an incoming message, a term corresponding to the configured
+&decode_format; if the message can be decoded in a non-relay
+application, <c>undefined</c> otherwise.
 For an outgoing message, setting a <c>[&header; | &avp;]</c> list is
 equivalent to setting the <c>header</c> and <c>avps</c> fields to the
 corresponding values.</p>
 
 <warning>
 <p>
-A record-valued <c>msg</c> field does <em>not</em> imply an absence of
+A value in the <c>msg</c> field does <em>not</em> imply an absence of
 decode errors.
 The <c>errors</c> field should also be examined.</p>
 </warning>
diff --git a/lib/diameter/doc/src/seealso.ent b/lib/diameter/doc/src/seealso.ent
index ef6af1a3d0..c5a53670d0 100644
--- a/lib/diameter/doc/src/seealso.ent
+++ b/lib/diameter/doc/src/seealso.ent
@@ -72,6 +72,7 @@ significant.
 <!ENTITY watchdog_timer '<seealso marker="#watchdog_timer">watchdog_timer</seealso>'>
 
 <!ENTITY mod_string_decode '<seealso marker="diameter#service_opt">diameter:service_opt()</seealso> <seealso marker="diameter#string_decode">string_decode</seealso>'>
+<!ENTITY mod_decode_format '<seealso marker="diameter#service_opt">diameter:service_opt()</seealso> <seealso marker="diameter#decode_format">decode_format</seealso>'>
 
 <!-- diameter_app -->
 
diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl
index 3b41feac0d..69ef6f4ec0 100644
--- a/lib/diameter/src/base/diameter.erl
+++ b/lib/diameter/src/base/diameter.erl
@@ -340,7 +340,7 @@ call(SvcName, App, Message) ->
    :: record
     | list
     | map
-    | false
+    | none
     | record_from_map.
 
 -type strict_arities()
diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl
index 6fc4277ac8..284f885884 100644
--- a/lib/diameter/src/base/diameter_config.erl
+++ b/lib/diameter/src/base/diameter_config.erl
@@ -588,8 +588,7 @@ opt(service, {K, false})
        K == use_shared_peers;
        K == monitor;
        K == restrict_connections;
-       K == strict_arities;
-       K == decode_format ->
+       K == strict_arities ->
     true;
 
 opt(service, {K, true})
@@ -602,6 +601,7 @@ opt(service, {decode_format, T})
   when T == record;
        T == list;
        T == map;
+       T == none;
        T == record_from_map ->
     true;
 
diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl
index f9172ec59d..0aea982a54 100644
--- a/lib/diameter/src/base/diameter_gen.erl
+++ b/lib/diameter/src/base/diameter_gen.erl
@@ -45,7 +45,7 @@
 -define(THROW(T), throw({?MODULE, T})).
 
 -type parent_name()   :: atom().  %% parent = Message or AVP
--type parent_record() :: tuple(). %%
+-type parent_record() :: tuple() | avp_values() | map().
 -type avp_name()   :: atom().
 -type avp_record() :: tuple().
 -type avp_values() :: [{avp_name(), term()}].
@@ -61,9 +61,7 @@
 %% # encode_avps/3
 %% ---------------------------------------------------------------------------
 
--spec encode_avps(parent_name(),
-                  parent_record() | avp_values() | map(),
-                  map())
+-spec encode_avps(parent_name(), parent_record(), map())
    -> iolist()
     | no_return().
 
@@ -232,7 +230,7 @@ enc(AvpName, Value, Opts, Mod) ->
 %% ---------------------------------------------------------------------------
 
 -spec decode_avps(parent_name(), binary(), map())
-   -> {parent_record(), [avp()], Failed}
+   -> {parent_record() | parent_name(), [avp()], Failed}
  when Failed :: [{5000..5999, #diameter_avp{}}].
 
 decode_avps(Name, Bin, #{module := Mod, decode_format := Fmt} = Opts) ->
@@ -303,7 +301,7 @@ decode(Bin, Code, Vid, DataLen, Pad, M, P, Name, Mod, Fmt, Strict, Opts0,
                                 type = type(NameT),
                                 index = Idx},
 
-            Dec = decode(Data, Name, NameT, Mod, Opts, Avp),         %% decode
+            Dec = decode1(Data, Name, NameT, Mod, Fmt, Opts, Avp),
             Acc = decode(T, Name, Mod, Fmt, Strict, Opts, Idx+1, AM),%% recurse
             acc(Acc, Dec, I, Name, Field, Arity, Strict, Mod, Opts);
         _ ->
@@ -451,10 +449,10 @@ field({AvpName, _}) ->
 field(_) ->
     'AVP'.
 
-%% decode/6
+%% decode1/7
 
 %% AVP not in dictionary.
-decode(_Data, _Name, 'AVP', _Mod, _Opts, Avp) ->
+decode1(_Data, _Name, 'AVP', _Mod, _Fmt, _Opts, Avp) ->
     Avp;
 
 %% 6733, 4.4:
@@ -504,7 +502,7 @@ decode(_Data, _Name, 'AVP', _Mod, _Opts, Avp) ->
 %% defined the RFC's "unrecognized", which is slightly stronger than
 %% "not defined".)
 
-decode(Data, Name, {AvpName, Type}, Mod, Opts, Avp) ->
+decode1(Data, Name, {AvpName, Type}, Mod, Fmt, Opts, Avp) ->
     #{dictionary := AppMod, failed_avp := Failed}
         = Opts,
 
@@ -518,26 +516,39 @@ decode(Data, Name, {AvpName, Type}, Mod, Opts, Avp) ->
     %% list of component AVPs.
 
     try avp_decode(Data, AvpName, Opts, DecMod, Mod) of
-        {Rec, As} when Type == 'Grouped' ->
-            A = Avp#diameter_avp{value = Rec},
-            [A | As];
-        V when Type /= 'Grouped' ->
-            Avp#diameter_avp{value = V}
+        V ->
+            set(Type, Fmt, Avp, V)
     catch
         throw: {?MODULE, T} ->
-            decode_error(Failed, T, Avp);
+            decode_error(Failed, Fmt, T, Avp);
         error: Reason ->
             decode_error(Failed, Reason, Name, Mod, Opts, Avp)
     end.
 
-%% decode_error/3
+%% set/4
+
+set('Grouped', none, Avp, V) ->
+    {_Rec, As} = V,
+    [Avp | As];
+
+set('Grouped', _, Avp, V) ->
+    {Rec, As} = V,
+    [Avp#diameter_avp{value = Rec} | As];
+
+set(_, _, Avp, V) ->
+    Avp#diameter_avp{value = V}.
+
+%% decode_error/4
 %%
 %% Error when decoding a grouped AVP.
 
-decode_error(true, {Rec, _, _}, Avp) ->
+decode_error(true, none, _, Avp) ->
+    Avp;
+
+decode_error(true, _, {Rec, _, _}, Avp) ->
     Avp#diameter_avp{value = Rec};
 
-decode_error(false, {_, ComponentAvps, [{RC,A} | _]}, Avp) ->
+decode_error(false, _, {_, ComponentAvps, [{RC,A} | _]}, Avp) ->
     {RC, [Avp | ComponentAvps], Avp#diameter_avp{data = [A]}}.
 
 %% decode_error/6
@@ -724,8 +735,9 @@ pack(Arity, F, Avp, Mod, [Failed | Rec]) ->
 
 %% set/5
 
-set(_, _, _, _, false = No) ->
-    No;
+set(_, _, _, _, None)
+  when is_atom(None) ->
+    None;
 
 set(1, F, Value, _, Map)
   when is_map(Map) ->
@@ -819,8 +831,8 @@ empty(Name, #{module := Mod} = Opts) ->
 
 %% newrec/4
 
-newrec(false = No, _, _, _) ->
-    No;
+newrec(none, _, Name, _) ->
+    Name;
 
 newrec(record, Mod, Name, T)
   when T /= decode ->
diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl
index c719ef4739..1a4bb4d0bf 100644
--- a/lib/diameter/src/base/diameter_traffic.erl
+++ b/lib/diameter/src/base/diameter_traffic.erl
@@ -1993,10 +1993,10 @@ avp_decode(Dict, 'Experimental-Result' = N, #diameter_avp{data = Bin}
     {V,_} = Dict:avp(decode, Bin, N, decode_opts(Dict)),
     Avp#diameter_avp{name = N, value = V};
 
-avp_decode(Dict, Name, #diameter_avp{value = X,
+avp_decode(Dict, Name, #diameter_avp{value = undefined,
                                      data = Bin}
                        = Avp)
-  when is_binary(Bin), X == undefined orelse X == false ->
+  when is_binary(Bin) ->
     V = Dict:avp(decode, Bin, Name, decode_opts(Dict)),
     Avp#diameter_avp{name = Name, value = V};
 
diff --git a/lib/diameter/src/base/diameter_watchdog.erl b/lib/diameter/src/base/diameter_watchdog.erl
index bb671e9860..c08e2da672 100644
--- a/lib/diameter/src/base/diameter_watchdog.erl
+++ b/lib/diameter/src/base/diameter_watchdog.erl
@@ -72,7 +72,7 @@
                      restrict := boolean(),
                      suspect := non_neg_integer(), %% OKAY -> SUSPECT
                      okay := non_neg_integer()},   %% REOPEN -> OKAY
-         codec :: #{decode_format := false,
+         codec :: #{decode_format := none,
                     string_decode := false,
                     strict_arities => diameter:strict_arities(),
                     strict_mbit := boolean(),
@@ -157,7 +157,7 @@ i({Ack, T, Pid, {Opts,
                                  string_decode,
                                  rfc,
                                  ordered_encode],
-                                SvcOpts#{decode_format := false,
+                                SvcOpts#{decode_format := none,
                                          string_decode := false,
                                          ordered_encode => false})}.
 
diff --git a/lib/diameter/test/diameter_traffic_SUITE.erl b/lib/diameter/test/diameter_traffic_SUITE.erl
index 31900fc97f..8f2549c8b6 100644
--- a/lib/diameter/test/diameter_traffic_SUITE.erl
+++ b/lib/diameter/test/diameter_traffic_SUITE.erl
@@ -158,7 +158,7 @@
 -define(ENCODINGS, [list, record, map]).
 
 %% How to decode incoming messages.
--define(DECODINGS, [record, false, map, list, record_from_map]).
+-define(DECODINGS, [record, none, map, list, record_from_map]).
 
 %% Which dictionary to use in the clients.
 -define(RFCS, [rfc3588, rfc6733, rfc4005]).
@@ -512,6 +512,7 @@ start_services(Config) ->
                                      | ?SERVICE(SN, Grp)]),
     ok = diameter:start_service(CN, [{traffic_counters, bool()},
                                      {sequence, ?CLIENT_MASK},
+                                     {decode_format, map},
                                      {strict_arities, decode}
                                      | ?SERVICE(CN, Grp)]).
 
@@ -705,9 +706,9 @@ send_proxy_info(Config) ->
     Req = ['ASR', {'Proxy-Info', #{'Proxy-Host'  => H0,
                                    'Proxy-State' => S0}}],
     ['answer-message' | #{'Result-Code' := 3999,
-                          'Proxy-Info' := [Rec]}]
+                          'Proxy-Info' := [#{'Proxy-Host' := H,
+                                             'Proxy-State' := S}]}]
         = call(Config, Req),
-    {H, S, []} = proxy_info(Rec, Config),
     [H0, S0] = [?B(X) || X <- [H,S]].
 
 %% Send an unknown AVP (to some client) and check that it comes back.
@@ -733,12 +734,12 @@ send_unknown_short(Config, M, RC) ->
                                          data = <<17>>}]}],
     ['ASA' | #{'Session-Id' := _,
                'Result-Code' := RC,
-               'Failed-AVP' := Avps}]
+               'Failed-AVP' := [#{'AVP' := [Avp]}]}]
         = call(Config, Req),
-    [[#diameter_avp{code = 999,
-                    is_mandatory = M,
-                    data = <<17, _/binary>>}]] %% extra bits from padding
-        = failed_avps(Avps, Config).
+    #diameter_avp{code = 999,
+                  is_mandatory = M,
+                  data = <<17, _/binary>>} %% extra bits from padding
+        = Avp.
 
 %% Ditto but set the M flag.
 send_unknown_mandatory(Config) ->
@@ -747,12 +748,12 @@ send_unknown_mandatory(Config) ->
                                          data = <<17>>}]}],
     ['ASA' | #{'Session-Id' := _,
                'Result-Code' := ?AVP_UNSUPPORTED,
-               'Failed-AVP' := Avps}]
+               'Failed-AVP' := [#{'AVP' := [Avp]}]}]
         = call(Config, Req),
-    [[#diameter_avp{code = 999,
-                    is_mandatory = true,
-                    data = <<17>>}]]
-        = failed_avps(Avps, Config).
+    #diameter_avp{code = 999,
+                  is_mandatory = true,
+                  data = <<17>>}
+        = Avp.
 
 %% Ditto, and point the AVP length past the end of the message. Expect
 %% 5014 instead of 5001.
@@ -767,13 +768,13 @@ send_unexpected_mandatory_decode(Config) ->
                                          data = <<12:32>>}]}],
     ['ASA' | #{'Session-Id' := _,
                'Result-Code' := ?AVP_UNSUPPORTED,
-               'Failed-AVP' := Avps}]
+               'Failed-AVP' := [#{'AVP' := [Avp]}]}]
         = call(Config, Req),
-    [[#diameter_avp{code = 27,
-                    is_mandatory = true,
-                    value = 12,
-                    data = <<12:32>>}]]
-        = failed_avps(Avps, Config).
+    #diameter_avp{code = 27,
+                  is_mandatory = true,
+                  value = 12,
+                  data = <<12:32>>}
+        = Avp.
 
 %% Try to two Auth-Application-Id in ASR expect 5009.
 send_too_many(Config) ->
@@ -781,11 +782,11 @@ send_too_many(Config) ->
 
     ['ASA' | #{'Session-Id' := _,
                'Result-Code' := ?TOO_MANY,
-               'Failed-AVP' := Avps}]
+               'Failed-AVP' := [#{'AVP' := [Avp]}]}]
         = call(Config, Req),
-    [[#diameter_avp{name = 'Auth-Application-Id',
-                    value = 44}]]
-        = failed_avps(Avps, Config).
+    #diameter_avp{name = 'Auth-Application-Id',
+                  value = 44}
+        = Avp.
 
 %% Send an containing a faulty Grouped AVP (empty Proxy-Host in
 %% Proxy-Info) and expect that only the faulty AVP is sent in
@@ -797,12 +798,11 @@ send_grouped_error(Config) ->
                                    {'Proxy-State', ""}]]}],
     ['ASA' | #{'Session-Id' := _,
                'Result-Code' := ?INVALID_AVP_LENGTH,
-               'Failed-AVP' := Avps}]
+               'Failed-AVP' := [#{'AVP' := [Avp]}]}]
         = call(Config, Req),
-    [[#diameter_avp{name = 'Proxy-Info', value = V}]]
-        = failed_avps(Avps, Config),
-    {Empty, undefined, []} = proxy_info(V, Config),
-    <<0>> = ?B(Empty).
+    #diameter_avp{name = 'Proxy-Info', value = #{'Proxy-Host' := H}}
+        = Avp,
+    <<0>> = ?B(H).
 
 %% Send an STR that the server ignores.
 send_noreply(Config) ->
@@ -855,9 +855,8 @@ send_invalid_avp_length(Config) ->
                'Result-Code' := ?INVALID_AVP_LENGTH,
                'Origin-Host' := _,
                'Origin-Realm' := _,
-               'Failed-AVP' := Avps}]
-        = call(Config, Req),
-    [[_]] = failed_avps(Avps, Config).
+               'Failed-AVP' := [#{'AVP' := [_]}]}]
+        = call(Config, Req).
 
 %% Send a request containing 5xxx errors that the server rejects with
 %% 3xxx.
@@ -1068,29 +1067,6 @@ send_anything(Config) ->
 
 %% ===========================================================================
 
-failed_avps(Avps, Config) ->
-    #group{client_dict = D} = proplists:get_value(group, Config),
-    [failed_avp(D, T) || T <- Avps].
-
-failed_avp(nas4005, {'nas_Failed-AVP', As}) ->
-    As;
-failed_avp(_, #'diameter_base_Failed-AVP'{'AVP' = As}) ->
-    As.
-
-proxy_info(Rec, Config) ->
-    #group{client_dict = D} = proplists:get_value(group, Config),
-    if D == nas4005 ->
-            {'nas_Proxy-Info', H, S, As}
-                = Rec,
-            {H,S,As};
-       true ->
-            #'diameter_base_Proxy-Info'{'Proxy-Host' = H,
-                                        'Proxy-State' = S,
-                                        'AVP' = As}
-                = Rec,
-            {H,S,As}
-    end.
-
 group(Config) ->
     #group{} = proplists:get_value(group, Config).
 
@@ -1131,12 +1107,12 @@ origin(N) ->
 decode(record) -> 0;
 decode(list)   -> 1;
 decode(map)    -> 2;
-decode(false)  -> 3;
+decode(none)   -> 3;
 decode(record_from_map) -> 4;
 decode(0) -> record;
 decode(1) -> list;
 decode(2) -> map;
-decode(3) -> false;
+decode(3) -> none;
 decode(4) -> record_from_map.
 
 encode(record) -> 0;
@@ -1183,16 +1159,17 @@ to_map(#diameter_packet{header = H, msg = Rec},
 
 %% No record decode: do it ourselves.
 to_map(#diameter_packet{header = H,
-                        msg = false,
+                        msg = Name,
                         bin = Bin},
-      #group{server_decoding = false,
+      #group{server_decoding = none,
              strings = B}) ->
     Opts = #{decode_format => map,
              string_decode => B,
              strict_mbit => true,
              rfc => 6733},
-    #diameter_packet{msg = [_MsgName | _Map] = Msg}
+    #diameter_packet{msg = [MsgName | _Map] = Msg}
         = diameter_codec:decode(dict(H), Opts, Bin),
+    {MsgName, _} = {Name, Msg},  %% assert
     Msg.
 
 dict(#diameter_header{application_id = Id,
@@ -1560,24 +1537,23 @@ answer(Pkt, Req, _Peer, Name, #group{client_dict = Dict0}) ->
     #diameter_packet{header = H, msg = Ans, errors = Es} = Pkt,
     ApplId = app(Req, Name, Dict0),
     #diameter_header{application_id = ApplId} = H,  %% assert
-    Dict = dict(Ans, Dict0),
-    rec_to_map(answer(Ans, Es, Name), Dict).
+    answer(Ans, Es, Name).
 
 %% Missing Result-Code and inappropriate Experimental-Result-Code.
-answer(Rec, Es, send_experimental_result) ->
+answer(Ans, Es, send_experimental_result) ->
     [{5004, #diameter_avp{name = 'Experimental-Result'}},
      {5005, #diameter_avp{name = 'Result-Code'}}]
         =  Es,
-    Rec;
+    Ans;
 
 %% An inappropriate E-bit results in a decode error ...
-answer(Rec, Es, send_bad_answer) ->
+answer(Ans, Es, send_bad_answer) ->
     [{5004, #diameter_avp{name = 'Result-Code'}} | _] = Es,
-    Rec;
+    Ans;
 
 %% ... while other errors are reflected in Failed-AVP.
-answer(Rec, [], _) ->
-    Rec.
+answer(Ans, [], _) ->
+    Ans.
 
 app(_, send_unsupported_app, _) ->
     ?BAD_APP;