From 6c2185bed7f5fc0523f4700335320f90b6623c16 Mon Sep 17 00:00:00 2001
From: Kjell Winblad <kjellwinblad@gmail.com>
Date: Mon, 3 Jul 2023 12:19:13 +0200
Subject: [PATCH] fix: problems found by @thalesmg in review

---
 .../priv/{LICENCE => LICENSE}                 |  0
 .../src/emqx_ee_schema_registry.erl           | 19 ++-----------------
 .../src/emqx_ee_schema_registry_schema.erl    | 12 +++++++++++-
 .../test/emqx_ee_schema_registry_SUITE.erl    | 17 +++++------------
 4 files changed, 18 insertions(+), 30 deletions(-)
 rename lib-ee/emqx_ee_schema_registry/priv/{LICENCE => LICENSE} (100%)

diff --git a/lib-ee/emqx_ee_schema_registry/priv/LICENCE b/lib-ee/emqx_ee_schema_registry/priv/LICENSE
similarity index 100%
rename from lib-ee/emqx_ee_schema_registry/priv/LICENCE
rename to lib-ee/emqx_ee_schema_registry/priv/LICENSE
diff --git a/lib-ee/emqx_ee_schema_registry/src/emqx_ee_schema_registry.erl b/lib-ee/emqx_ee_schema_registry/src/emqx_ee_schema_registry.erl
index df294bcbb..31153805c 100644
--- a/lib-ee/emqx_ee_schema_registry/src/emqx_ee_schema_registry.erl
+++ b/lib-ee/emqx_ee_schema_registry/src/emqx_ee_schema_registry.erl
@@ -38,11 +38,6 @@
     import_config/1
 ]).
 
-%% For testing
--export([
-    ensure_serde_absent/1
-]).
-
 -type schema() :: #{
     type := serde_type(),
     source := binary(),
@@ -248,7 +243,7 @@ do_build_serdes(Schemas) ->
 maybe_build_sparkplug_b_serde() ->
     case get_schema(?EMQX_SCHEMA_REGISTRY_SPARKPLUGB_SCHEMA_NAME) of
         {error, not_found} ->
-            do_build_serde_no_check(
+            do_build_serde(
                 ?EMQX_SCHEMA_REGISTRY_SPARKPLUGB_SCHEMA_NAME,
                 #{
                     type => protobuf,
@@ -293,15 +288,7 @@ build_serdes([], _Acc) ->
 
 do_build_serde(Name, Serde) when not is_binary(Name) ->
     do_build_serde(to_bin(Name), Serde);
-do_build_serde(?EMQX_SCHEMA_REGISTRY_SPARKPLUGB_SCHEMA_NAME, _Serde) ->
-    {error,
-        erlang:iolist_to_binary(
-            io_lib:format("Illigal schema name ~s", [?EMQX_SCHEMA_REGISTRY_SPARKPLUGB_SCHEMA_NAME])
-        )};
-do_build_serde(Name, Serde) ->
-    do_build_serde_no_check(Name, Serde).
-
-do_build_serde_no_check(Name, #{type := Type, source := Source}) ->
+do_build_serde(Name, #{type := Type, source := Source}) ->
     try
         {Serializer, Deserializer, Destructor} =
             emqx_ee_schema_registry_serde:make_serde(Type, Name, Source),
@@ -331,8 +318,6 @@ do_build_serde_no_check(Name, #{type := Type, source := Source}) ->
 
 ensure_serde_absent(Name) when not is_binary(Name) ->
     ensure_serde_absent(to_bin(Name));
-% ensure_serde_absent(?EMQX_SCHEMA_REGISTRY_SPARKPLUGB_SCHEMA_NAME) ->
-%     {error, <<"Cannot delete serde for Sparkplug B schema">>};
 ensure_serde_absent(Name) ->
     case get_serde(Name) of
         {ok, #{destructor := Destructor}} ->
diff --git a/lib-ee/emqx_ee_schema_registry/src/emqx_ee_schema_registry_schema.erl b/lib-ee/emqx_ee_schema_registry/src/emqx_ee_schema_registry_schema.erl
index 237ec706f..d641cf593 100644
--- a/lib-ee/emqx_ee_schema_registry/src/emqx_ee_schema_registry_schema.erl
+++ b/lib-ee/emqx_ee_schema_registry/src/emqx_ee_schema_registry_schema.erl
@@ -42,7 +42,8 @@ fields(?CONF_KEY_ROOT) ->
                 ),
                 #{
                     default => #{},
-                    desc => ?DESC("schema_registry_schemas")
+                    desc => ?DESC("schema_registry_schemas"),
+                    validator => fun validate_name/1
                 }
             )}
     ];
@@ -89,6 +90,15 @@ union_member_selector_get_api(all_union_members) ->
 union_member_selector_get_api({value, V}) ->
     refs_get_api(V).
 
+validate_name(NameSchemaMap) ->
+    case maps:is_key(?EMQX_SCHEMA_REGISTRY_SPARKPLUGB_SCHEMA_NAME, NameSchemaMap) of
+        true ->
+            {error,
+                <<"Illegal schema name ", ?EMQX_SCHEMA_REGISTRY_SPARKPLUGB_SCHEMA_NAME/binary>>};
+        false ->
+            ok
+    end.
+
 %%------------------------------------------------------------------------------
 %% `minirest_trails' "APIs"
 %%------------------------------------------------------------------------------
diff --git a/lib-ee/emqx_ee_schema_registry/test/emqx_ee_schema_registry_SUITE.erl b/lib-ee/emqx_ee_schema_registry/test/emqx_ee_schema_registry_SUITE.erl
index 6d724a4e4..aadc6d280 100644
--- a/lib-ee/emqx_ee_schema_registry/test/emqx_ee_schema_registry_SUITE.erl
+++ b/lib-ee/emqx_ee_schema_registry/test/emqx_ee_schema_registry_SUITE.erl
@@ -790,18 +790,11 @@ sparkplug_example_data() ->
     }.
 
 wait_for_sparkplug_schema_registered() ->
-    wait_for_sparkplug_schema_registered(100).
-
-wait_for_sparkplug_schema_registered(0) ->
-    ct:fail("Timed out waiting for sparkplug schema to be registered");
-wait_for_sparkplug_schema_registered(AttemptsLeft) ->
-    case ets:info(?SERDE_TAB, size) of
-        0 ->
-            timer:sleep(100),
-            wait_for_sparkplug_schema_registered(AttemptsLeft - 1);
-        _ ->
-            ok
-    end.
+    ?retry(
+        100,
+        100,
+        [_] = ets:lookup(?SERDE_TAB, ?EMQX_SCHEMA_REGISTRY_SPARKPLUGB_SCHEMA_NAME)
+    ).
 
 t_sparkplug_decode(_Config) ->
     SQL =