From fa275bf0a2404145b7d11212253016185fe15a71 Mon Sep 17 00:00:00 2001
From: "Zaiming (Stone) Shi" <zmstone@gmail.com>
Date: Fri, 1 Mar 2024 07:20:49 +0100
Subject: [PATCH] refactor(schema_registry): use one func for both encode and
 decode check

---
 .../src/emqx_schema_registry_serde.erl        | 16 +++++--------
 .../test/emqx_schema_registry_serde_SUITE.erl | 24 ++++++++++---------
 2 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/apps/emqx_schema_registry/src/emqx_schema_registry_serde.erl b/apps/emqx_schema_registry/src/emqx_schema_registry_serde.erl
index e3a365386..169c686ba 100644
--- a/apps/emqx_schema_registry/src/emqx_schema_registry_serde.erl
+++ b/apps/emqx_schema_registry/src/emqx_schema_registry_serde.erl
@@ -74,24 +74,20 @@ handle_rule_function(schema_encode, [SchemaId, Term | MoreArgs]) ->
     iolist_to_binary(IOList);
 handle_rule_function(schema_encode, Args) ->
     error({args_count_error, {schema_encode, Args}});
-handle_rule_function(schema_check_decode, [SchemaId, Data | MoreArgs]) ->
-    check_decode(SchemaId, Data, MoreArgs);
-handle_rule_function(schema_check_encode, [SchemaId, Term | MoreArgs]) ->
-    check_encode(SchemaId, Term, MoreArgs);
+handle_rule_function(schema_check, [SchemaId, Data | MoreArgs]) ->
+    schema_check(SchemaId, Data, MoreArgs);
 handle_rule_function(_, _) ->
     {error, no_match_for_function}.
 
--spec check_decode(schema_name(), encoded_data(), [term()]) -> decoded_data().
-check_decode(SerdeName, Data, VarArgs) ->
+-spec schema_check(schema_name(), decoded_data() | encoded_data(), [term()]) -> decoded_data().
+schema_check(SerdeName, Data, VarArgs) when is_list(VarArgs), is_binary(Data) ->
     with_serde(
         SerdeName,
         fun(Serde) ->
             ?BOOL(SerdeName, eval_decode(Serde, [Data | VarArgs]))
         end
-    ).
-
--spec check_encode(schema_name(), decoded_data(), [term()]) -> encoded_data().
-check_encode(SerdeName, Data, VarArgs) when is_list(VarArgs) ->
+    );
+schema_check(SerdeName, Data, VarArgs) when is_list(VarArgs), is_map(Data) ->
     with_serde(
         SerdeName,
         fun(Serde) ->
diff --git a/apps/emqx_schema_registry/test/emqx_schema_registry_serde_SUITE.erl b/apps/emqx_schema_registry/test/emqx_schema_registry_serde_SUITE.erl
index 3c6ecd14e..0fad015f0 100644
--- a/apps/emqx_schema_registry/test/emqx_schema_registry_serde_SUITE.erl
+++ b/apps/emqx_schema_registry/test/emqx_schema_registry_serde_SUITE.erl
@@ -143,24 +143,26 @@ t_avro_invalid_schema(_Config) ->
 t_serde_not_found(_Config) ->
     %% for coverage
     NonexistentSerde = <<"nonexistent">>,
+    EncodeData = #{},
+    DecodeData = <<"data">>,
     ?assertError(
         {serde_not_found, NonexistentSerde},
-        emqx_schema_registry_serde:encode(NonexistentSerde, data)
+        emqx_schema_registry_serde:encode(NonexistentSerde, EncodeData)
     ),
     ?assertError(
         {serde_not_found, NonexistentSerde},
-        emqx_schema_registry_serde:decode(NonexistentSerde, data)
+        emqx_schema_registry_serde:decode(NonexistentSerde, DecodeData)
     ),
     ?assertError(
         {serde_not_found, NonexistentSerde},
-        emqx_schema_registry_serde:handle_rule_function(schema_check_decode, [
-            NonexistentSerde, data
+        emqx_schema_registry_serde:handle_rule_function(schema_check, [
+            NonexistentSerde, EncodeData
         ])
     ),
     ?assertError(
         {serde_not_found, NonexistentSerde},
-        emqx_schema_registry_serde:handle_rule_function(schema_check_encode, [
-            NonexistentSerde, data
+        emqx_schema_registry_serde:handle_rule_function(schema_check, [
+            NonexistentSerde, DecodeData
         ])
     ),
     ok.
@@ -232,9 +234,9 @@ t_json_validation(_Config) ->
     end,
     OK = #{<<"foo">> => 1, <<"bar">> => 2},
     NotOk = #{<<"bar">> => 2},
-    ?assert(F(schema_check_encode, OK)),
-    ?assert(F(schema_check_decode, <<"{\"foo\": 1, \"bar\": 2}">>)),
-    ?assertNot(F(schema_check_encode, NotOk)),
-    ?assertNot(F(schema_check_decode, <<"{\"bar\": 2}">>)),
-    ?assertNot(F(schema_check_decode, <<"{\"foo\": \"notinteger\", \"bar\": 2}">>)),
+    ?assert(F(schema_check, OK)),
+    ?assert(F(schema_check, <<"{\"foo\": 1, \"bar\": 2}">>)),
+    ?assertNot(F(schema_check, NotOk)),
+    ?assertNot(F(schema_check, <<"{\"bar\": 2}">>)),
+    ?assertNot(F(schema_check, <<"{\"foo\": \"notinteger\", \"bar\": 2}">>)),
     ok.