From 05ebb17cd64e92c8109575bf066892d8a92efe68 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Mon, 10 Jun 2024 15:54:37 -0300 Subject: [PATCH] refactor: index positions rather than names Addresses https://github.com/emqx/emqx/pull/13199#discussion_r1633096025 --- .../src/emqx_schema_validation_config.erl | 20 ++-- .../src/emqx_schema_validation_registry.erl | 98 +++++++++++++------ 2 files changed, 76 insertions(+), 42 deletions(-) diff --git a/apps/emqx_schema_validation/src/emqx_schema_validation_config.erl b/apps/emqx_schema_validation/src/emqx_schema_validation_config.erl index df882a078..2bfd11c6d 100644 --- a/apps/emqx_schema_validation/src/emqx_schema_validation_config.erl +++ b/apps/emqx_schema_validation/src/emqx_schema_validation_config.erl @@ -66,10 +66,10 @@ load() -> unload() -> Validations = emqx:get_config(?VALIDATIONS_CONF_PATH, []), lists:foreach( - fun(Validation) -> - ok = emqx_schema_validation_registry:delete(Validation) + fun({Pos, Validation}) -> + ok = emqx_schema_validation_registry:delete(Validation, Pos) end, - Validations + lists:enumerate(Validations) ). -spec list() -> [validation()]. @@ -146,11 +146,11 @@ post_config_update(?VALIDATIONS_CONF_PATH, {update, #{<<"name">> := Name}}, New, ok = emqx_schema_validation_registry:update(OldValidation, Pos, NewValidation), ok; post_config_update(?VALIDATIONS_CONF_PATH, {delete, Name}, _New, Old, _AppEnvs) -> - {_Pos, Validation} = fetch_with_index(Old, Name), - ok = emqx_schema_validation_registry:delete(Validation), + {Pos, Validation} = fetch_with_index(Old, Name), + ok = emqx_schema_validation_registry:delete(Validation, Pos), ok; -post_config_update(?VALIDATIONS_CONF_PATH, {reorder, _Order}, New, _Old, _AppEnvs) -> - ok = emqx_schema_validation_registry:reindex_positions(New), +post_config_update(?VALIDATIONS_CONF_PATH, {reorder, _Order}, New, Old, _AppEnvs) -> + ok = emqx_schema_validation_registry:reindex_positions(New, Old), ok; post_config_update([?CONF_ROOT], {merge, _}, ResultingConfig, Old, _AppEnvs) -> #{validations := ResultingValidations} = ResultingConfig, @@ -181,8 +181,8 @@ post_config_update([?CONF_ROOT], {replace, Input}, ResultingConfig, Old, _AppEnv #{validations := OldValidations} = Old, lists:foreach( fun(Name) -> - {_Pos, Validation} = fetch_with_index(OldValidations, Name), - ok = emqx_schema_validation_registry:delete(Validation) + {Pos, Validation} = fetch_with_index(OldValidations, Name), + ok = emqx_schema_validation_registry:delete(Validation, Pos) end, DeletedValidations ), @@ -203,7 +203,7 @@ post_config_update([?CONF_ROOT], {replace, Input}, ResultingConfig, Old, _AppEnv end, ChangedValidations0 ), - ok = emqx_schema_validation_registry:reindex_positions(ResultingValidations), + ok = emqx_schema_validation_registry:reindex_positions(ResultingValidations, OldValidations), {ok, #{changed_validations => ChangedValidations}}. %%------------------------------------------------------------------------------ diff --git a/apps/emqx_schema_validation/src/emqx_schema_validation_registry.erl b/apps/emqx_schema_validation/src/emqx_schema_validation_registry.erl index c25007424..de90fa7cd 100644 --- a/apps/emqx_schema_validation/src/emqx_schema_validation_registry.erl +++ b/apps/emqx_schema_validation/src/emqx_schema_validation_registry.erl @@ -10,8 +10,8 @@ lookup/1, insert/2, update/3, - delete/1, - reindex_positions/1, + delete/2, + reindex_positions/2, matching_validations/1, @@ -51,10 +51,10 @@ -type validation() :: _TODO. -type position_index() :: pos_integer(). --record(reindex_positions, {validations :: [validation()]}). +-record(reindex_positions, {new_validations :: [validation()], old_validations :: [validation()]}). -record(insert, {pos :: position_index(), validation :: validation()}). -record(update, {old :: validation(), pos :: position_index(), new :: validation()}). --record(delete, {validation :: validation()}). +-record(delete, {validation :: validation(), pos :: position_index()}). %%------------------------------------------------------------------------------ %% API @@ -74,9 +74,16 @@ lookup(Name) -> {ok, Validation} end. --spec reindex_positions([validation()]) -> ok. -reindex_positions(Validations) -> - gen_server:call(?MODULE, #reindex_positions{validations = Validations}, infinity). +-spec reindex_positions([validation()], [validation()]) -> ok. +reindex_positions(NewValidations, OldValidations) -> + gen_server:call( + ?MODULE, + #reindex_positions{ + new_validations = NewValidations, + old_validations = OldValidations + }, + infinity + ). -spec insert(position_index(), validation()) -> ok. insert(Pos, Validation) -> @@ -86,23 +93,36 @@ insert(Pos, Validation) -> update(Old, Pos, New) -> gen_server:call(?MODULE, #update{old = Old, pos = Pos, new = New}, infinity). --spec delete(validation()) -> ok. -delete(Validation) -> - gen_server:call(?MODULE, #delete{validation = Validation}, infinity). +-spec delete(validation(), position_index()) -> ok. +delete(Validation, Pos) -> + gen_server:call(?MODULE, #delete{validation = Validation, pos = Pos}, infinity). %% @doc Returns a list of matching validation names, sorted by their configuration order. -spec matching_validations(emqx_types:topic()) -> [validation()]. matching_validations(Topic) -> - Validations0 = [ - {Pos, Validation} - || M <- emqx_topic_index:matches(Topic, ?VALIDATION_TOPIC_INDEX, [unique]), - [Pos] <- [emqx_topic_index:get_record(M, ?VALIDATION_TOPIC_INDEX)], - {ok, Validation} <- [ - lookup(emqx_topic_index:get_id(M)) - ] - ], - Validations1 = lists:sort(fun({Pos1, _V1}, {Pos2, _V2}) -> Pos1 =< Pos2 end, Validations0), - lists:map(fun({_Pos, V}) -> V end, Validations1). + Validations0 = + lists:flatmap( + fun(M) -> + case emqx_topic_index:get_record(M, ?VALIDATION_TOPIC_INDEX) of + [Name] -> + [Name]; + _ -> + [] + end + end, + emqx_topic_index:matches(Topic, ?VALIDATION_TOPIC_INDEX, [unique]) + ), + lists:flatmap( + fun(Name) -> + case lookup(Name) of + {ok, Validation} -> + [Validation]; + _ -> + [] + end + end, + Validations0 + ). -spec metrics_worker_spec() -> supervisor:child_spec(). metrics_worker_spec() -> @@ -133,8 +153,15 @@ init(_) -> State = #{}, {ok, State}. -handle_call(#reindex_positions{validations = Validations}, _From, State) -> - do_reindex_positions(Validations), +handle_call( + #reindex_positions{ + new_validations = NewValidations, + old_validations = OldValidations + }, + _From, + State +) -> + do_reindex_positions(NewValidations, OldValidations), {reply, ok, State}; handle_call(#insert{pos = Pos, validation = Validation}, _From, State) -> do_insert(Pos, Validation), @@ -142,8 +169,8 @@ handle_call(#insert{pos = Pos, validation = Validation}, _From, State) -> handle_call(#update{old = OldValidation, pos = Pos, new = NewValidation}, _From, State) -> ok = do_update(OldValidation, Pos, NewValidation), {reply, ok, State}; -handle_call(#delete{validation = Validation}, _From, State) -> - do_delete(Validation), +handle_call(#delete{validation = Validation, pos = Pos}, _From, State) -> + do_delete(Validation, Pos), {reply, ok, State}; handle_call(_Call, _From, State) -> {reply, ignored, State}. @@ -160,7 +187,14 @@ create_tables() -> _ = emqx_utils_ets:new(?VALIDATION_TAB, [public, ordered_set, {read_concurrency, true}]), ok. -do_reindex_positions(Validations) -> +do_reindex_positions(NewValidations, OldValidations) -> + lists:foreach( + fun({Pos, Validation}) -> + #{topics := Topics} = Validation, + delete_topic_index(Pos, Topics) + end, + lists:enumerate(OldValidations) + ), lists:foreach( fun({Pos, Validation}) -> #{ @@ -170,7 +204,7 @@ do_reindex_positions(Validations) -> do_insert_into_tab(Name, Validation, Pos), update_topic_index(Name, Pos, Topics) end, - lists:enumerate(Validations) + lists:enumerate(NewValidations) ). do_insert(Pos, Validation) -> @@ -193,17 +227,17 @@ do_update(OldValidation, Pos, NewValidation) -> } = NewValidation, maybe_create_metrics(Name), do_insert_into_tab(Name, NewValidation, Pos), - delete_topic_index(Name, OldTopics), + delete_topic_index(Pos, OldTopics), Enabled andalso update_topic_index(Name, Pos, NewTopics), ok. -do_delete(Validation) -> +do_delete(Validation, Pos) -> #{ name := Name, topics := Topics } = Validation, ets:delete(?VALIDATION_TAB, Name), - delete_topic_index(Name, Topics), + delete_topic_index(Pos, Topics), drop_metrics(Name), ok. @@ -226,15 +260,15 @@ drop_metrics(Name) -> update_topic_index(Name, Pos, Topics) -> lists:foreach( fun(Topic) -> - true = emqx_topic_index:insert(Topic, Name, Pos, ?VALIDATION_TOPIC_INDEX) + true = emqx_topic_index:insert(Topic, Pos, Name, ?VALIDATION_TOPIC_INDEX) end, Topics ). -delete_topic_index(Name, Topics) -> +delete_topic_index(Pos, Topics) -> lists:foreach( fun(Topic) -> - true = emqx_topic_index:delete(Topic, Name, ?VALIDATION_TOPIC_INDEX) + true = emqx_topic_index:delete(Topic, Pos, ?VALIDATION_TOPIC_INDEX) end, Topics ).