Skip to content

Commit

Permalink
Update all usages of attributes as proplists into maps
Browse files Browse the repository at this point in the history
  • Loading branch information
NelsonVides committed Dec 17, 2024
1 parent ac85cc6 commit 0f20d46
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 76 deletions.
2 changes: 1 addition & 1 deletion include/exml.hrl
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
-record(xmlcdata, {content = [] :: iodata()}).

-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
Expand Down
2 changes: 1 addition & 1 deletion include/exml_stream.hrl
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
-include("exml.hrl").

-record(xmlstreamstart, {name :: binary(),
attrs = [] :: [exml:attr()]}).
attrs = #{} :: exml:attrs()}).

-record(xmlstreamend, {name :: binary()}).
22 changes: 10 additions & 12 deletions src/exml.erl
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,13 @@
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{}.
-type element() :: #xmlel{}.
-type item() :: element() | attr() | cdata() | exml_stream:start() | exml_stream:stop().
Expand All @@ -49,13 +51,13 @@ xml_size(#xmlcdata{ content = Content }) ->
iolist_size(exml_nif:escape_cdata(Content));
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) ->
Expand Down Expand Up @@ -83,13 +85,12 @@ 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}) ->
Expand All @@ -113,8 +114,7 @@ remove_cdata(#xmlel{children = Children} = El) ->
%% @doc Remove a given attribute from a `t:element/0'.
-spec remove_attr(exml:element(), binary()) -> element().
remove_attr(#xmlel{attrs = Attrs} = El, Key) ->
NewAttrs = lists:keydelete(Key, 1, Attrs),
El#xmlel{attrs = NewAttrs}.
El#xmlel{attrs = maps:remove(Key, Attrs)}.

%% @doc Append new children elements to a `t:element/0'.
-spec append_children(element(), [element() | cdata()]) -> element().
Expand All @@ -124,9 +124,7 @@ append_children(#xmlel{children = Children} = El, 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) ->
Attrs1 = lists:keydelete(Key, 1, Attrs),
Attrs2 = [{Key, Value} | Attrs1],
El#xmlel{attrs = Attrs2}.
El#xmlel{attrs = Attrs#{Key => Value}}.

%% @doc Replace or insert a child by the given one.
-spec upsert_child(element(), element()) -> element().
Expand Down
8 changes: 4 additions & 4 deletions src/exml_query.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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]).

Expand Down Expand Up @@ -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.
4 changes: 2 additions & 2 deletions test/exml_query_tests.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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">>}])).
Expand Down
38 changes: 19 additions & 19 deletions test/exml_stream_tests.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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,
Expand All @@ -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),
Expand Down Expand Up @@ -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?">>}]}]}
],
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down
62 changes: 25 additions & 37 deletions test/exml_tests.erl
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ size_of_exml_with_cdata_test() ->
sort_xmlel_identity_test() ->
El = #xmlel{
name = <<"foo">>,
attrs = [{<<"attr1">>, <<"bar">>}],
attrs = #{<<"attr1">> => <<"bar">>},
children = [#xmlcdata{ content = <<"some value">> }]
},
?assertEqual(El, exml:xml_sort(El)).
Expand All @@ -46,7 +46,7 @@ sort_xmlel_attributes_test() ->
?assertEqual(Attrs, exml:xml_sort(ToOrder)).

remove_cdata_test() ->
Attrs = [{<<"attr1">>, <<"foo">>}, {<<"attr2">>, <<"bar">>}],
Attrs = #{<<"attr1">> => <<"foo">>, <<"attr2">> => <<"bar">>},
Child1 = #xmlel{name = <<"el1">>, attrs = Attrs},
Child2 = #xmlel{name = <<"el2">>, attrs = Attrs},
CData = #xmlcdata{content = <<"some value">>},
Expand All @@ -55,17 +55,17 @@ remove_cdata_test() ->
?exmlAssertEqual(Expected, exml:remove_cdata(El)).

filter_children_test() ->
Attrs = [{<<"attr1">>, <<"foo">>}, {<<"attr2">>, <<"bar">>}],
Child1 = #xmlel{name = <<"el1">>, attrs = [{<<"xmlns">>, <<"foo">>}]},
Child2 = #xmlel{name = <<"el2">>, attrs = [{<<"xmlns">>, <<"bar">>} | Attrs]},
Child3 = #xmlel{name = <<"el3">>, attrs = [{<<"xmlns">>, <<"baz">>} | Attrs]},
Attrs = #{<<"attr1">> => <<"foo">>, <<"attr2">> => <<"bar">>},
Child1 = #xmlel{name = <<"el1">>, attrs = #{<<"xmlns">> => <<"foo">>}},
Child2 = #xmlel{name = <<"el2">>, attrs = maps:merge(#{<<"xmlns">> => <<"bar">>}, Attrs)},
Child3 = #xmlel{name = <<"el3">>, attrs = maps:merge(#{<<"xmlns">> => <<"baz">>}, Attrs)},
El = #xmlel{name = <<"foo">>, children = [Child1, Child2, Child3]},
Expected = #xmlel{name = <<"foo">>, children = [Child1, Child3]},
Pred = fun(Child) -> <<"bar">> =/= exml_query:attr(Child, <<"xmlns">>) end,
?exmlAssertEqual(Expected, exml:filter_children(El, Pred)).

append_children_test() ->
Attrs = [{<<"attr1">>, <<"foo">>}, {<<"attr2">>, <<"bar">>}],
Attrs = #{<<"attr1">> => <<"foo">>, <<"attr2">> => <<"bar">>},
Child1 = #xmlel{name = <<"el1">>, attrs = Attrs},
Child2 = #xmlel{name = <<"el2">>, attrs = Attrs},
CData = #xmlcdata{content = <<"some value">>},
Expand All @@ -74,21 +74,21 @@ append_children_test() ->
?exmlAssertEqual(Expected, exml:append_children(El, [Child2, CData])).

replace_attribute_value_test() ->
Attrs1 = [{<<"attr1">>, <<"foo">>}, {<<"attr2">>, <<"bar">>}],
Attrs2 = [{<<"attr1">>, <<"foo">>}, {<<"attr2">>, <<"baz">>}],
Attrs1 = #{<<"attr1">> => <<"foo">>, <<"attr2">> => <<"bar">>},
Attrs2 = #{<<"attr1">> => <<"foo">>, <<"attr2">> => <<"baz">>},
El = #xmlel{name = <<"foo">>, attrs = Attrs1},
Expected = #xmlel{name = <<"foo">>, attrs = Attrs2},
?exmlAssertEqual(Expected, exml:upsert_attr_value(El, <<"attr2">>, <<"baz">>)).

remove_attribute_test() ->
Attrs1 = [{<<"attr1">>, <<"foo">>}, {<<"attr2">>, <<"bar">>}],
Attrs2 = [{<<"attr2">>, <<"bar">>}],
Attrs1 = #{<<"attr1">> => <<"foo">>, <<"attr2">> => <<"bar">>},
Attrs2 = #{<<"attr2">> => <<"bar">>},
El = #xmlel{name = <<"foo">>, attrs = Attrs1},
Expected = #xmlel{name = <<"foo">>, attrs = Attrs2},
?exmlAssertEqual(Expected, exml:remove_attr(El, <<"attr1">>)).

replace_child_test() ->
Attrs = [{<<"attr1">>, <<"foo">>}, {<<"attr2">>, <<"bar">>}],
Attrs = #{<<"attr1">> => <<"foo">>, <<"attr2">> => <<"bar">>},
Child1 = #xmlel{name = <<"el">>},
Child2 = #xmlel{name = <<"el">>, attrs = Attrs},
Child3 = #xmlel{name = <<"last">>, attrs = Attrs, children = [Child1]},
Expand All @@ -97,27 +97,16 @@ replace_child_test() ->
?exmlAssertEqual(Expected, exml:upsert_child(El, Child2)).

insert_new_child_test() ->
Attrs = [{<<"attr1">>, <<"foo">>}, {<<"attr2">>, <<"bar">>}],
Attrs = #{<<"attr1">> => <<"foo">>, <<"attr2">> => <<"bar">>},
Child1 = #xmlel{name = <<"el">>},
Child2 = #xmlel{name = <<"el">>, attrs = Attrs},
Child3 = #xmlel{name = <<"last">>, attrs = Attrs, children = [Child1]},
El = #xmlel{name = <<"foo">>, children = [Child1, Child3]},
Expected = #xmlel{name = <<"foo">>, children = [Child1, Child3]},
?exmlAssertEqual(Expected, exml:insert_new_child(El, Child2)).

sort_xmlel_test() ->
Attrs = [{<<"attr1">>, <<"bar">>}, {<<"attr2">>, <<"baz">>}],
El1 = #xmlel{
name = <<"foo">>,
attrs = Attrs,
children = [#xmlcdata{ content = <<"some value">> }]
},
El2 = El1#xmlel{ attrs = lists:reverse(Attrs) },
?assertNotEqual(El1, El2),
?assertEqual(exml:xml_sort(El1), exml:xml_sort(El2)).

sort_xmlel_nested_test() ->
Attrs = [{<<"attr1">>, <<"bar">>}, {<<"attr2">>, <<"baz">>}],
Attrs = #{<<"attr1">> => <<"bar">>, <<"attr2">> => <<"baz">>},
CData = [#xmlcdata{ content = <<"some value">> }],
Nested1 = #xmlel{
name = <<"n1">>,
Expand All @@ -126,7 +115,7 @@ sort_xmlel_nested_test() ->
},
Nested2 = #xmlel{
name = <<"n2">>,
attrs = lists:reverse(Attrs),
attrs = Attrs,
children = CData
},
Children = [Nested1, Nested2],
Expand All @@ -140,10 +129,9 @@ sort_xmlel_nested_test() ->
?assertNotEqual(exml:xml_sort(El1), exml:xml_sort(El2)).

sort_xmlstreamstart_test() ->
Attrs = [{<<"attr1">>, <<"bar">>}, {<<"attr2">>, <<"baz">>}],
Attrs = #{<<"attr1">> => <<"bar">>, <<"attr2">> => <<"baz">>},
SS1 = #xmlstreamstart{name = <<"n1">>, attrs = Attrs},
SS2 = SS1#xmlstreamstart{attrs = lists:reverse(Attrs)},
?assertNotEqual(SS1, SS2),
SS2 = SS1#xmlstreamstart{attrs = Attrs},
?assertEqual(exml:xml_sort(SS1), exml:xml_sort(SS2)).

sort_xmlstreamend_test() ->
Expand All @@ -155,7 +143,7 @@ sort_xmlstreamend_test() ->
sort_xmlel_list_test() ->
El1 = #xmlel{
name = <<"foo">>,
attrs = [{<<"attr1">>, <<"bar">>}],
attrs = #{<<"attr1">> => <<"bar">>},
children = [#xmlcdata{ content = <<"some value">> }]
},
El2 = El1#xmlel{ name = <<"baz">> },
Expand All @@ -165,22 +153,22 @@ sort_xmlel_list_test() ->
?assertEqual(exml:xml_sort(L1), exml:xml_sort(L2)).

assert_xmlel_equal_macro_positive_test() ->
Attrs = [{<<"attr1">>, <<"bar">>}, {<<"attr2">>, <<"baz">>}],
Attrs = #{<<"attr1">> => <<"bar">>, <<"attr2">> => <<"baz">>},
El1 = #xmlel{
name = <<"foo">>,
attrs = Attrs,
children = [#xmlcdata{ content = <<"some value">> }]
children = [#xmlcdata{content = <<"some value">>}]
},
El2 = El1#xmlel{ attrs = lists:reverse(Attrs) },
El2 = El1#xmlel{attrs = Attrs},
?exmlAssertEqual(El1, El2).

assert_xmlel_equal_macro_negative_test() ->
El1 = #xmlel{
name = <<"foo">>,
attrs = [{<<"attr1">>, <<"bar">>}, {<<"attr2">>, <<"baz">>}],
children = [#xmlcdata{ content = <<"some value">> }]
attrs = #{<<"attr1">> => <<"bar">>, <<"attr2">> => <<"baz">>},
children = [#xmlcdata{content = <<"some value">>}]
},
El2 = El1#xmlel{ attrs = [] },
El2 = El1#xmlel{attrs = #{}},
?assertError({exmlAssertEqual, [_, _, _, {expected, El1}, {value, El2}]},
?exmlAssertEqual(El1, El2)).

Expand All @@ -191,7 +179,7 @@ throws_error_when_record_is_invalid_test() ->
to_binary_arbitrary_stream_elements_test() ->
Elements = [#xmlcdata{content = <<"content">>},
#xmlstreamend{name = <<"endname">>},
#xmlstreamstart{name = <<"name">>, attrs = [{<<"a">>, <<"b">>}]}],
#xmlstreamstart{name = <<"name">>, attrs = #{<<"a">> => <<"b">>}}],
?assertEqual(<<"content</endname><name a='b'>">>, exml:to_binary(Elements)).

parse(Doc) ->
Expand Down

0 comments on commit 0f20d46

Please sign in to comment.