From 2444970b0b367b335ecca76317418ffc47ba2020 Mon Sep 17 00:00:00 2001
From: firest <blankalupo@163.com>
Date: Fri, 17 Jun 2022 10:25:08 +0800
Subject: [PATCH 1/2] fix(retainer): check does the bucket is exists when
 update the retainer config

---
 .../emqx_limiter/src/emqx_limiter_server.erl  |  4 ---
 apps/emqx_retainer/src/emqx_retainer_api.erl  | 34 +++++++++++++++++--
 .../src/emqx_retainer_dispatcher.erl          |  2 +-
 3 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/apps/emqx/src/emqx_limiter/src/emqx_limiter_server.erl b/apps/emqx/src/emqx_limiter/src/emqx_limiter_server.erl
index d540497fc..519b32eca 100644
--- a/apps/emqx/src/emqx_limiter/src/emqx_limiter_server.erl
+++ b/apps/emqx/src/emqx_limiter/src/emqx_limiter_server.erl
@@ -112,10 +112,6 @@
 %% If no bucket path is set in config, there will be no limit
 connect(_Type, undefined) ->
     {ok, emqx_htb_limiter:make_infinity_limiter()};
-%% Workaround.
-%% After API updated some config, the bucket name maybe become ‘’ (converted from empty binary)
-connect(_Type, '') ->
-    {ok, emqx_htb_limiter:make_infinity_limiter()};
 connect(Type, BucketName) when is_atom(BucketName) ->
     case get_bucket_cfg(Type, BucketName) of
         undefined ->
diff --git a/apps/emqx_retainer/src/emqx_retainer_api.erl b/apps/emqx_retainer/src/emqx_retainer_api.erl
index 2c0bd725c..b8b371cc7 100644
--- a/apps/emqx_retainer/src/emqx_retainer_api.erl
+++ b/apps/emqx_retainer/src/emqx_retainer_api.erl
@@ -151,8 +151,13 @@ config(get, _) ->
     {200, emqx:get_raw_config([retainer])};
 config(put, #{body := Body}) ->
     try
-        {ok, _} = emqx_retainer:update_config(Body),
-        {200, emqx:get_raw_config([retainer])}
+        check_bucket_exists(
+            Body,
+            fun(Conf) ->
+                {ok, _} = emqx_retainer:update_config(Conf),
+                {200, emqx:get_raw_config([retainer])}
+            end
+        )
     catch
         _:Reason:_ ->
             {400, #{
@@ -232,3 +237,28 @@ check_backend(Type, Params, Cont) ->
         _ ->
             {400, 'BAD_REQUEST', <<"This API only support built in database">>}
     end.
+
+check_bucket_exists(
+    #{
+        <<"flow_control">> :=
+            #{<<"batch_deliver_limiter">> := Name} = Flow
+    } = Conf,
+    Cont
+) ->
+    case erlang:binary_to_atom(Name) of
+        '' ->
+            %% workaround, empty string means set the value to undefined,
+            %% but now, we can't store `undefined` in the config file correct,
+            %% but, we can delete this field
+            Cont(Conf#{
+                <<"flow_control">> := maps:remove(<<"batch_deliver_limiter">>, Flow)
+            });
+        Bucket ->
+            Path = emqx_limiter_schema:get_bucket_cfg_path(batch, Bucket),
+            case emqx:get_config(Path, undefined) of
+                undefined ->
+                    {400, 'BAD_REQUEST', <<"The limiter bucket not exists">>};
+                _ ->
+                    Cont(Conf)
+            end
+    end.
diff --git a/apps/emqx_retainer/src/emqx_retainer_dispatcher.erl b/apps/emqx_retainer/src/emqx_retainer_dispatcher.erl
index a0121f721..29818481d 100644
--- a/apps/emqx_retainer/src/emqx_retainer_dispatcher.erl
+++ b/apps/emqx_retainer/src/emqx_retainer_dispatcher.erl
@@ -115,7 +115,7 @@ start_link(Pool, Id) ->
 init([Pool, Id]) ->
     erlang:process_flag(trap_exit, true),
     true = gproc_pool:connect_worker(Pool, {Pool, Id}),
-    BucketName = emqx_conf:get([retainer, flow_control, batch_deliver_limiter], undefined),
+    BucketName = emqx:get_config([retainer, flow_control, batch_deliver_limiter], undefined),
     {ok, Limiter} = emqx_limiter_server:connect(batch, BucketName),
     {ok, #{pool => Pool, id => Id, limiter => Limiter}}.
 

From 7bc9f0af0f1a8cdb3d335d712c09fd081f2a51d4 Mon Sep 17 00:00:00 2001
From: firest <blankalupo@163.com>
Date: Fri, 17 Jun 2022 11:09:06 +0800
Subject: [PATCH 2/2] fix(retainer): fix test case error

---
 apps/emqx_retainer/src/emqx_retainer_api.erl    | 4 +++-
 apps/emqx_retainer/test/emqx_retainer_SUITE.erl | 1 -
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/apps/emqx_retainer/src/emqx_retainer_api.erl b/apps/emqx_retainer/src/emqx_retainer_api.erl
index b8b371cc7..7d085b422 100644
--- a/apps/emqx_retainer/src/emqx_retainer_api.erl
+++ b/apps/emqx_retainer/src/emqx_retainer_api.erl
@@ -261,4 +261,6 @@ check_bucket_exists(
                 _ ->
                     Cont(Conf)
             end
-    end.
+    end;
+check_bucket_exists(Conf, Cont) ->
+    Cont(Conf).
diff --git a/apps/emqx_retainer/test/emqx_retainer_SUITE.erl b/apps/emqx_retainer/test/emqx_retainer_SUITE.erl
index 74944f362..ed49f6f5c 100644
--- a/apps/emqx_retainer/test/emqx_retainer_SUITE.erl
+++ b/apps/emqx_retainer/test/emqx_retainer_SUITE.erl
@@ -55,7 +55,6 @@ common_tests() ->
     "    flow_control {\n"
     "        batch_read_number = 0\n"
     "        batch_deliver_number = 0\n"
-    "        batch_deliver_limiter = retainer\n"
     "     }\n"
     "   backend {\n"
     "        type = built_in_database\n"