Migrate exml attributes from proplists to maps

c_src/exml.cpp

@@ -147,13 +147,6 @@ ERL_NIF_TERM to_subbinary(ParseCtx &ctx, const unsigned char *text,
   return binary;
 }
 
-ERL_NIF_TERM make_attr_tuple(ParseCtx &ctx,
-                             rapidxml::xml_attribute<unsigned char> *attr) {
-  ERL_NIF_TERM name = to_subbinary(ctx, attr->name(), attr->name_size());
-  ERL_NIF_TERM value = to_subbinary(ctx, attr->value(), attr->value_size());
-  return enif_make_tuple2(ctx.env, name, value);
-}
-
 ERL_NIF_TERM get_xmlcdata(ParseCtx &ctx,
                           rapidxml::xml_node<unsigned char> *node) {
   return enif_make_tuple3(ctx.env, atom_xmlcdata,
@@ -248,31 +241,35 @@ ERL_NIF_TERM make_node_name_binary(ParseCtx &ctx,
   return to_subbinary(ctx, start, len);
 }
 
-std::tuple<ERL_NIF_TERM, ERL_NIF_TERM>
-get_open_tag(ParseCtx &ctx, rapidxml::xml_node<unsigned char> *node) {
-  ERL_NIF_TERM name_term = make_node_name_binary(ctx, node);
+ERL_NIF_TERM make_attr_tuple(ParseCtx &ctx,
+                             rapidxml::xml_attribute<unsigned char> *attr) {
+  ERL_NIF_TERM name = to_subbinary(ctx, attr->name(), attr->name_size());
+  ERL_NIF_TERM value = to_subbinary(ctx, attr->value(), attr->value_size());
+  return enif_make_tuple2(ctx.env, name, value);
+}
+
+ERL_NIF_TERM get_attributes(ParseCtx &ctx, rapidxml::xml_node<unsigned char> *node) {
   std::vector<ERL_NIF_TERM> &attrs = Parser::term_buffer;
   std::size_t begin = attrs.size();
 
-  for (rapidxml::xml_attribute<unsigned char> *attr = node->first_attribute();
-       attr; attr = attr->next_attribute())
-    attrs.push_back(make_attr_tuple(ctx, attr));
+  ERL_NIF_TERM attrs_term = enif_make_new_map(ctx.env);
 
-  std::size_t size = attrs.size() - begin;
-  ERL_NIF_TERM attrs_term =
-      size == 0
-          ? enif_make_list(ctx.env, 0)
-          : enif_make_list_from_array(ctx.env, attrs.data() + begin, size);
+  for (rapidxml::xml_attribute<unsigned char> *attr = node->first_attribute();
+       attr; attr = attr->next_attribute()) {
+    ERL_NIF_TERM key = to_subbinary(ctx, attr->name(), attr->name_size());
+    ERL_NIF_TERM value = to_subbinary(ctx, attr->value(), attr->value_size());
+    enif_make_map_put(ctx.env, attrs_term, key, value, &attrs_term);
+  }
 
-  attrs.erase(attrs.end() - size, attrs.end());
-  return std::make_tuple(name_term, attrs_term);
+  return attrs_term;
 }
 
 ERL_NIF_TERM make_stream_start_tuple(ParseCtx &ctx,
                                      rapidxml::xml_node<unsigned char> *node) {
-  auto name_and_attrs = get_open_tag(ctx, node);
-  return enif_make_tuple3(ctx.env, atom_xmlstreamstart, std::get<0>(name_and_attrs),
-                          std::get<1>(name_and_attrs));
+
+  ERL_NIF_TERM name_term = make_node_name_binary(ctx, node);
+  ERL_NIF_TERM attrs_term = get_attributes(ctx, node);
+  return enif_make_tuple3(ctx.env, atom_xmlstreamstart, name_term, attrs_term);
 }
 
 ERL_NIF_TERM make_stream_end_tuple(ParseCtx &ctx) {
@@ -287,10 +284,10 @@ ERL_NIF_TERM make_stream_end_tuple(ParseCtx &ctx) {
 
 ERL_NIF_TERM make_xmlel(ParseCtx &ctx,
                         rapidxml::xml_node<unsigned char> *node) {
-  auto name_and_attrs = get_open_tag(ctx, node);
+  ERL_NIF_TERM name_term = make_node_name_binary(ctx, node);
+  ERL_NIF_TERM attrs_term = get_attributes(ctx, node);
   ERL_NIF_TERM children_term = get_children_tuple(ctx, node);
-  return enif_make_tuple4(ctx.env, atom_xmlel, std::get<0>(name_and_attrs),
-                          std::get<1>(name_and_attrs), children_term);
+  return enif_make_tuple4(ctx.env, atom_xmlel, name_term, attrs_term, children_term);
 }
 
 bool build_children(ErlNifEnv *env, xml_document &doc, ERL_NIF_TERM children,
@@ -319,27 +316,28 @@ bool build_cdata(ErlNifEnv *env, xml_document &doc, const ERL_NIF_TERM elem[],
 bool build_attrs(ErlNifEnv *env, xml_document &doc, ERL_NIF_TERM attrs,
                  rapidxml::xml_node<unsigned char> &node) {
 
-  if (!enif_is_list(env, attrs))
+  if (!enif_is_map(env, attrs))
     return false;
 
-  for (ERL_NIF_TERM head; enif_get_list_cell(env, attrs, &head, &attrs);) {
-    int arity;
-    const ERL_NIF_TERM *tuple;
-    if (!enif_get_tuple(env, head, &arity, &tuple) || arity != 2)
-      return false;
+  ErlNifMapIterator iter;
+  enif_map_iterator_create(env, attrs, &iter, ERL_NIF_MAP_ITERATOR_FIRST);
 
+  ERL_NIF_TERM map_key, map_value;
+  while (enif_map_iterator_get_pair(env, &iter, &map_key, &map_value)) {
     ErlNifBinary key, value;
-    if (!enif_inspect_iolist_as_binary(env, tuple[0], &key))
+    if (!enif_inspect_iolist_as_binary(env, map_key, &key))
       return false;
 
-    if (!enif_inspect_iolist_as_binary(env, tuple[1], &value))
+    if (!enif_inspect_iolist_as_binary(env, map_value, &value))
       return false;
 
     auto attr = doc.impl.allocate_attribute(key.size > 0 ? key.data : EMPTY,
                                             value.size > 0 ? value.data : EMPTY,
                                             key.size, value.size);
     node.append_attribute(attr);
+    enif_map_iterator_next(env, &iter);
   }
+  enif_map_iterator_destroy(env, &iter);
 
   return true;
 }

include/exml.hrl

@@ -10,7 +10,7 @@
                    style = escaped :: escaped | cdata}).
 
 -record(xmlel, {name :: binary(),
-                attrs = [] :: [exml:attr()],
+                attrs = #{} :: exml:attrs(),
                 children =  [] :: [exml:element() | exml:cdata()]}).
 
 %% Implementation of the exmlAssertEqual/2 macro is a modification of

include/exml_stream.hrl

@@ -1,6 +1,6 @@
 -include("exml.hrl").
 
 -record(xmlstreamstart, {name :: binary(),
-                         attrs = [] :: [exml:attr()]}).
+                         attrs = #{} :: exml:attrs()}).
 
 -record(xmlstreamend, {name :: binary()}).

src/exml.erl

@@ -17,15 +17,25 @@
          to_binary/1,
          to_iolist/1,
          xml_size/1,
-         xml_sort/1,
          to_pretty_iolist/1]).
 
+-export([filter_children/2,
+         append_children/2,
+         upsert_attr_value/3,
+         upsert_child/2,
+         insert_new_child/2,
+         remove_cdata/1,
+         remove_attr/2,
+         xml_sort/1]).
+
 -export_type([attr/0,
+              attrs/0,
               cdata/0,
               element/0,
               item/0]).
 
 -type attr() :: {binary(), binary()}.
+-type attrs() :: #{binary() => binary()}.
 -type cdata() :: #xmlcdata{}.
 %% CDATA record. Printing escaping rules defaults to escaping character-wise.
 %%
@@ -49,13 +59,13 @@ xml_size(#xmlcdata{content = Content, style = Style}) ->
     iolist_size(exml_nif:escape_cdata(Content, Style));
 xml_size(#xmlel{ name = Name, attrs = Attrs, children = [] }) ->
     3 % Self-closing: </>
-    + byte_size(Name) + xml_size(Attrs);
+    + byte_size(Name) + xml_size(maps:to_list(Attrs));
 xml_size(#xmlel{ name = Name, attrs = Attrs, children = Children }) ->
     % Opening and closing: <></>
     5 + byte_size(Name)*2
-    + xml_size(Attrs) + xml_size(Children);
+    + xml_size(maps:to_list(Attrs)) + xml_size(Children);
 xml_size(#xmlstreamstart{ name = Name, attrs = Attrs }) ->
-    byte_size(Name) + 2 + xml_size(Attrs);
+    byte_size(Name) + 2 + xml_size(maps:to_list(Attrs));
 xml_size(#xmlstreamend{ name = Name }) ->
     byte_size(Name) + 3;
 xml_size({Key, Value}) when is_binary(Key) ->
@@ -83,20 +93,63 @@ xml_size({Key, Value}) when is_binary(Key) ->
               (exml_stream:stop()) -> exml_stream:stop().
 xml_sort(#xmlcdata{} = Cdata) ->
     Cdata;
-xml_sort(#xmlel{ attrs = Attrs, children = Children } = El) ->
+xml_sort(#xmlel{children = Children} = El) ->
     El#xmlel{
-      attrs = lists:sort(Attrs),
       children = [ xml_sort(C) || C <- Children ]
      };
-xml_sort(#xmlstreamstart{ attrs = Attrs } = StreamStart) ->
-    StreamStart#xmlstreamstart{ attrs = lists:sort(Attrs) };
+xml_sort(#xmlstreamstart{} = StreamStart) ->
+    StreamStart;
 xml_sort(#xmlstreamend{} = StreamEnd) ->
     StreamEnd;
 xml_sort({Key, Value}) ->
     {Key, Value};
 xml_sort(Elements) when is_list(Elements) ->
     lists:sort([ xml_sort(E) || E <- Elements ]).
 
+%% @doc Return the given `t:element/0' with the specified filter passed over its children.
+-spec filter_children(element(), fun((element() | cdata()) -> boolean())) -> element().
+filter_children(#xmlel{children = Children} = El, Pred) ->
+    NoCdata = lists:filter(Pred, Children),
+    El#xmlel{children = NoCdata}.
+
+%% @doc Return the given `t:element/0' without any `t:cdata/0' on its children.
+-spec remove_cdata(element()) -> element().
+remove_cdata(#xmlel{children = Children} = El) ->
+    Pred = fun(Child) -> not is_record(Child, xmlcdata) end,
+    NoCdata = lists:filter(Pred, Children),
+    El#xmlel{children = NoCdata}.
+
+%% @doc Remove a given attribute from a `t:element/0'.
+-spec remove_attr(exml:element(), binary()) -> element().
+remove_attr(#xmlel{attrs = Attrs} = El, Key) ->
+    El#xmlel{attrs = maps:remove(Key, Attrs)}.
+
+%% @doc Append new children elements to a `t:element/0'.
+-spec append_children(element(), [element() | cdata()]) -> element().
+append_children(#xmlel{children = Children} = El, ExtraChildren) ->
+    El#xmlel{children = Children ++ ExtraChildren}.
+
+%% @doc Replace or insert the value of a given attribute.
+-spec upsert_attr_value(element(), binary(), binary()) -> element().
+upsert_attr_value(#xmlel{attrs = Attrs} = El, Key, Value) ->
+    El#xmlel{attrs = Attrs#{Key => Value}}.
+
+%% @doc Replace or insert a child by the given one.
+-spec upsert_child(element(), element()) -> element().
+upsert_child(#xmlel{children = Children} = El, #xmlel{name = Name} = NewChild) ->
+    Children2 = lists:keystore(Name, #xmlel.name, Children, NewChild),
+    El#xmlel{children = Children2}.
+
+%% @doc Insert a child by the given one, if none existed.
+-spec insert_new_child(element(), element()) -> element().
+insert_new_child(#xmlel{children = Children} = El, #xmlel{name = Name} = NewChild) ->
+    case lists:keymember(Name, #xmlel.name, Children) of
+        false ->
+            El#xmlel{children = [NewChild | Children]};
+        true ->
+            El
+    end.
+
 %% @equiv erlang:binary_to_list(to_binary(Element))
 -spec to_list(exml_stream:element() | [exml_stream:element()]) -> string().
 to_list(Element) ->

src/exml_query.erl

@@ -116,7 +116,7 @@ paths(#xmlel{} = Element, [{element_with_attr, AttrName, Value} | Rest]) ->
 paths(#xmlel{} = Element, [cdata]) ->
     [cdata(Element)];
 paths(#xmlel{attrs = Attrs}, [{attr, Name}]) ->
-    lists:sublist([V || {N, V} <- Attrs, N =:= Name], 1);
+    lists:sublist([V || {N, V} <- maps:to_list(Attrs), N =:= Name], 1);
 paths(#xmlel{} = El, Path) when is_list(Path) ->
     erlang:error(invalid_path, [El, Path]).
 
@@ -253,9 +253,9 @@ attr(Element, Name) ->
 %% @equiv path(Element, [{attr, Name}], Default)
 -spec attr(exml:element(), binary(), Default) -> binary() | Default.
 attr(#xmlel{attrs = Attrs}, Name, Default) ->
-    case lists:keyfind(Name, 1, Attrs) of
-        {Name, Value} ->
+    case maps:find(Name, Attrs) of
+        {ok, Value} ->
             Value;
-        false ->
+        error ->
             Default
     end.

test/exml_query_tests.erl

@@ -132,8 +132,8 @@ element_with_name_and_ns_query_test() ->
                                                <<"urn:xmpp:chat-markers:0">>}])).
 
 element_with_name_and_ns_two_names_only_one_ns_query_test() ->
-    Elem1 = #xmlel{name = <<"a">>, attrs = [{<<"xmlns">>, <<"ns1">>}]},
-    Elem2 = #xmlel{name = <<"a">>, attrs = [{<<"xmlns">>, <<"ns2">>}]},
+    Elem1 = #xmlel{name = <<"a">>, attrs = #{<<"xmlns">> => <<"ns1">>}},
+    Elem2 = #xmlel{name = <<"a">>, attrs = #{<<"xmlns">> => <<"ns2">>}},
     Xml = #xmlel{name = <<"element">>, children = [Elem1, Elem2]},
     ?assertEqual(Elem2, exml_query:subelement_with_name_and_ns(Xml, <<"a">>, <<"ns2">>)),
     ?assertEqual(Elem2, exml_query:path(Xml, [{element_with_ns, <<"a">>, <<"ns2">>}])).

test/exml_stream_tests.erl

@@ -14,14 +14,14 @@ basic_parse_test() ->
         exml_stream:parse(Parser1, <<" to='i.am.banana.com' xml:lang='en'><auth">>),
     ?assertEqual(
        [#xmlstreamstart{name = <<"stream:stream">>,
-                        attrs = [{<<"xmlns:stream">>, <<"http://etherx.jabber.org/streams">>},
-                                 {<<"version">>, <<"1.0">>},
-                                 {<<"to">>, <<"i.am.banana.com">>},
-                                 {<<"xml:lang">>, <<"en">>}]}],
+                        attrs = #{<<"xmlns:stream">> => <<"http://etherx.jabber.org/streams">>,
+                                  <<"version">> => <<"1.0">>,
+                                  <<"to">> => <<"i.am.banana.com">>,
+                                  <<"xml:lang">> => <<"en">>}}],
        StreamStart),
     {ok, Parser3, Auth} = exml_stream:parse(Parser2, <<" mechanism='DIGEST-MD5'/>">>),
     ?assertEqual(
-       [#xmlel{name = <<"auth">>, attrs = [{<<"mechanism">>, <<"DIGEST-MD5">>}]}],
+       [#xmlel{name = <<"auth">>, attrs = #{<<"mechanism">> => <<"DIGEST-MD5">>}}],
        Auth),
     {ok, Parser4, Empty1} = exml_stream:parse(Parser3, <<"<stream:features><bind xmlns='some_ns'">>),
     ?assertEqual([], Empty1),
@@ -31,9 +31,9 @@ basic_parse_test() ->
     ?assertMatch(
        [#xmlel{name = <<"stream:features">>,
                     children = [#xmlel{name = <<"bind">>,
-                                            attrs = [{<<"xmlns">>, <<"some_ns">>}]},
+                                            attrs = #{<<"xmlns">> := <<"some_ns">>}},
                                 #xmlel{name = <<"session">>,
-                                            attrs = [{<<"xmlns">>, <<"some_other">>}]},
+                                            attrs = #{<<"xmlns">> := <<"some_other">>}},
                                 _CData]}],
        Features),
     [#xmlel{children=[_, _, CData]}] = Features,
@@ -49,9 +49,9 @@ parser_errors_test() ->
 -define(BANANA_STREAM, <<"<stream:stream xmlns:stream='something'><foo attr='bar'>I am a banana!<baz/></foo></stream:stream>">>).
 -define(assertIsBanana(Elements), (fun() -> % fun instead of begin/end because we bind CData in unhygenic macro
                                            ?assertMatch([#xmlstreamstart{name = <<"stream:stream">>,
-                                                                         attrs = [{<<"xmlns:stream">>, <<"something">>}]},
+                                                                         attrs = #{<<"xmlns:stream">> := <<"something">>}},
                                                          #xmlel{name = <<"foo">>,
-                                                                     attrs = [{<<"attr">>, <<"bar">>}],
+                                                                     attrs = #{<<"attr">> := <<"bar">>},
                                                                      children = [_CData, #xmlel{name = <<"baz">>}]},
                                                          #xmlstreamend{name = <<"stream:stream">>}],
                                                         Elements),
@@ -84,12 +84,12 @@ infinit_framed_stream_test() ->
     {ok, Parser0} = exml_stream:new_parser([{infinite_stream, true},
                                             {autoreset, true}]),
     Els = [#xmlel{name = <<"open">>,
-                  attrs = [{<<"xmlns">>, <<"urn:ietf:params:xml:ns:xmpp-framing">>},
-                           {<<"to">>, <<"example.com">>},
-                           {<<"version">>, <<"1.0">>}]},
+                  attrs = #{<<"xmlns">> => <<"urn:ietf:params:xml:ns:xmpp-framing">>,
+                            <<"to">> => <<"example.com">>,
+                            <<"version">> => <<"1.0">>}},
            #xmlel{name = <<"foo">>},
            #xmlel{name = <<"message">>,
-                  attrs = [{<<"to">>, <<"ala@example.com">>}],
+                  attrs = #{<<"to">> => <<"ala@example.com">>},
                   children = [#xmlel{name = <<"body">>,
                                      children = [#xmlcdata{content = <<"Hi, How Are You?">>}]}]}
     ],
@@ -147,7 +147,7 @@ conv_attr_test() ->
     AssertParses = fun(Input) ->
                            {ok, Parser0} = exml_stream:new_parser(),
                            {ok, _Parser1, Elements} = exml_stream:parse(Parser0, Input),
-                           ?assertMatch([_, #xmlel{attrs = [{<<"attr">>, <<"&<>\"'\n\t\r">>}]} | _],
+                           ?assertMatch([_, #xmlel{attrs = #{<<"attr">> := <<"&<>\"'\n\t\r">>}} | _],
                                                    Elements),
                            Elements
                    end,
@@ -233,18 +233,18 @@ infinite_stream_partial_chunk_test() ->
     {ok, Parser1, Open} = exml_stream:parse(Parser0, <<"<open xmlns='urn:ietf:params:xml:ns:xmpp-framing' to='i.am.banana.com' version='1.0'/>">>),
     ?assertEqual(
        [#xmlel{name = <<"open">>,
-               attrs = [{<<"xmlns">>, <<"urn:ietf:params:xml:ns:xmpp-framing">>},
-                        {<<"to">>, <<"i.am.banana.com">>},
-                        {<<"version">>, <<"1.0">>}]}],
+               attrs = #{<<"xmlns">> => <<"urn:ietf:params:xml:ns:xmpp-framing">>,
+                         <<"to">> => <<"i.am.banana.com">>,
+                         <<"version">> => <<"1.0">>}}],
        Open),
     {ok, Parser2, A} = exml_stream:parse(Parser1, <<"<a></a>">>),
-    ?assertEqual([#xmlel{name = <<"a">>, attrs = []}], A),
+    ?assertEqual([#xmlel{name = <<"a">>, attrs = #{}}], A),
     {ok, Parser3, Empty0} = exml_stream:parse(Parser2, <<" ">>),
     ?assertEqual([], Empty0),
     {ok, Parser4, Empty1} = exml_stream:parse(Parser3, <<"<b></b">>),
     ?assertEqual([], Empty1),
     {ok, _Parser5, B} = exml_stream:parse(Parser4, <<">">>),
-    ?assertEqual([#xmlel{name = <<"b">>, attrs = []}], B).
+    ?assertEqual([#xmlel{name = <<"b">>, attrs = #{}}], B).
 
 null_character_test() ->
     {ok, P1} = exml_stream:new_parser(),