From 922d95fd927fafadcfaba35ebde76ae836638b51 Mon Sep 17 00:00:00 2001
From: Nicolas Berbiche <n.berbiche@samsung.com>
Date: Sun, 24 Sep 2023 17:32:02 -0400
Subject: [PATCH] Use single error type for ac string parsing failure

---
 src/additional_consent_string.erl | 23 ++++++++++-------------
 test/ac_string_tests.erl          |  7 ++++---
 2 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/src/additional_consent_string.erl b/src/additional_consent_string.erl
index 780f30c..70c1ffa 100644
--- a/src/additional_consent_string.erl
+++ b/src/additional_consent_string.erl
@@ -8,16 +8,11 @@
 -export([parse_version/1, parse_atp_ids/1]).
 -endif.
 
--type addtl_parse_error() ::
-    invalid_atp_id |
-    invalid_version.
--export_type([addtl_parse_error/0]).
-
 -define(KNOWN_SEPARATOR, "~").
 
 
 -spec parse(binary()) ->
-    {ok, addtl_consent()} | {error, addtl_parse_error()}.
+    {ok, addtl_consent()} | {error, invalid_ac_string}.
 
 % @doc Parses an additional consent string, returning the version
 % number and the list of numerical ATP ids.
@@ -25,7 +20,7 @@
 % @returns `{ok, #addtl_consent{}}' if the string is valid,
 % otherwise an appropriate error is returned.
 %
-parse(Bin) ->
+parse(Bin) when is_binary(Bin) ->
     case parse_version(Bin) of
         {ok, Version, Bin1} ->
             case parse_atp_ids(Bin1) of
@@ -37,7 +32,9 @@ parse(Bin) ->
                 Error -> Error
             end;
         Error -> Error
-    end.
+    end;
+parse(_) ->
+    {error, invalid_ac_string}.
 
 parse_version(Bin) ->
     parse_version(Bin, <<"">>).
@@ -48,12 +45,12 @@ parse_version(<<Bin, Rest/binary>>, Acc) when Bin >= $0, Bin =< $9 ->
 parse_version(<<?KNOWN_SEPARATOR, Rest/binary>>, Acc) ->
     case safe_binary_to_integer(Acc) of
         undefined ->
-            {error, invalid_version};
+            {error, invalid_ac_string};
         Version ->
             {ok, Version, Rest}
     end;
 parse_version(_, _) ->
-    {error, invalid_version}.
+    {error, invalid_ac_string}.
 
 parse_atp_ids(Bin) ->
     parse_atp_ids(Bin, <<"">>, []).
@@ -63,18 +60,18 @@ parse_atp_ids(<<Bin, Rest/binary>>, Acc, Ids) when Bin >= $0, Bin =< $9 ->
 parse_atp_ids(<<".", Rest/binary>>, Acc, Ids) when Rest =/= <<"">> ->
     case safe_binary_to_integer(Acc) of
         undefined ->
-            {error, invalid_atp_id};
+            {error, invalid_ac_string};
         Id ->
             parse_atp_ids(Rest, <<"">>, [Id | Ids])
     end;
 parse_atp_ids(<<_Invalid, _Rest/binary>>, _Acc, _Ids) ->
-    {error, invalid_atp_id};
+    {error, invalid_ac_string};
 parse_atp_ids(<<"">>, <<"">>, Ids) ->
     {ok, Ids};
 parse_atp_ids(<<"">>, Acc, Ids) ->
     case safe_binary_to_integer(Acc) of
         undefined ->
-            {error, invalid_atp_id};
+            {error, invalid_ac_string};
         Id ->
             % Preserve the order of ids
             {ok, lists:reverse([Id | Ids])}
diff --git a/test/ac_string_tests.erl b/test/ac_string_tests.erl
index 0a1efef..1128dd6 100644
--- a/test/ac_string_tests.erl
+++ b/test/ac_string_tests.erl
@@ -7,9 +7,9 @@
 -define(_assertMatchVersion(A, B),
        ?_assertMatch({ok, (A), _}, parse_version((B)))).
 -define(_assertErrorVersion(B),
-        ?_assertMatch({error, invalid_version}, parse_version((B)))).
+        ?_assertMatch({error, invalid_ac_string}, parse_version((B)))).
 -define(_assertErrorAtp(B),
-        ?_assertMatch({error, invalid_atp_id}, parse_atp_ids((B)))).
+        ?_assertMatch({error, invalid_ac_string}, parse_atp_ids((B)))).
 
 parse_version_only_test_() ->
     [
@@ -52,7 +52,8 @@ parse_test_() ->
         ?_assertMatch({ok, #addtl_consent{version = 1, atp_ids = [1, 3, 2]}}, parse(<<"1~1.3.2">>)),
 
         % Test errors
-        ?_assertMatch({error, invalid_atp_id}, parse(<<"1~a">>))
+        ?_assertMatch({error, invalid_ac_string}, parse(<<"1~a">>)),
+        ?_assertMatch({error, invalid_ac_string}, parse(<<"1a1~">>))
     ].
 
 %% Helpers