From 8cb0ec00d21af1a81d4db6c1f74fc4f4ad3427b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D0=B5=D0=BC?= Date: Mon, 25 May 2026 13:12:32 +0300 Subject: [PATCH 1/5] Refactor DMT modules by removing unused types and specifications This commit cleans up the codebase by removing several unused type definitions and specifications across various modules, including `dmt_api_woody_utils`, `dmt_author_database`, `dmt_author_handler`, and others. The changes streamline the code and improve readability by eliminating unnecessary complexity. Additionally, some function specifications have been updated to reflect the current implementation accurately. No functional changes were made; this is purely a cleanup effort. --- apps/dmt/src/dmt_api_woody_utils.erl | 57 +----- apps/dmt/src/dmt_app.erl | 8 +- apps/dmt/src/dmt_author.erl | 14 -- apps/dmt/src/dmt_author_database.erl | 35 +--- apps/dmt/src/dmt_author_handler.erl | 17 +- apps/dmt/src/dmt_database.erl | 192 ++---------------- apps/dmt/src/dmt_db_migration.erl | 85 +++----- apps/dmt/src/dmt_json.erl | 8 +- apps/dmt/src/dmt_mapper.erl | 54 +---- apps/dmt/src/dmt_repository.erl | 146 ++----------- .../dmt/src/dmt_repository_client_handler.erl | 17 +- apps/dmt/src/dmt_sup.erl | 99 ++------- apps/dmt/src/dmt_thrift.erl | 19 +- apps/dmt/src/dmt_thrift_validator.erl | 11 +- apps/dmt/test/dmt_client_api.erl | 10 +- apps/dmt/test/dmt_repository_filter_test.erl | 2 - apps/dmt_core/src/dmt_domain.erl | 74 +------ apps/dmt_core/src/dmt_domain_pt.erl | 8 +- apps/dmt_object/src/dmt_object_id.erl | 8 - apps/dmt_object/src/dmt_object_reference.erl | 58 +----- apps/dmt_object/src/dmt_object_type.erl | 2 - 21 files changed, 119 insertions(+), 805 deletions(-) diff --git a/apps/dmt/src/dmt_api_woody_utils.erl b/apps/dmt/src/dmt_api_woody_utils.erl index c73af2d..ce3fbd2 100644 --- a/apps/dmt/src/dmt_api_woody_utils.erl +++ b/apps/dmt/src/dmt_api_woody_utils.erl @@ -5,51 +5,12 @@ %% API --type service_opts() :: #{ - url := binary() | list(), - transport_opts => term(), - resolver_opts => term() -}. - -%% Options passed to every thrift handler via woody's `handlers` config. --type handler_options() :: #{default_handling_timeout := timeout(), atom() => term()}. - --export_type([service_opts/0, handler_options/0]). - -spec get_woody_client(atom()) -> woody_client:options(). get_woody_client(Service) -> - Services = get_services(genlib_app:env(dmt_api, services, #{})), + Services = genlib_app:env(dmt_api, services, #{}), make_woody_client(maps:get(Service, Services)). --spec get_services(term()) -> #{atom() => service_opts()}. -get_services(Map) when is_map(Map) -> - maps:fold(fun fold_service/3, #{}, Map); -get_services(_) -> - #{}. - --spec fold_service(term(), term(), #{atom() => service_opts()}) -> - #{atom() => service_opts()}. -fold_service(K, #{url := Url} = V, Acc) when is_atom(K), is_binary(Url) -> - Acc#{K => build_service_opts(V, Url)}; -fold_service(K, #{url := Url} = V, Acc) when is_atom(K), is_list(Url) -> - Acc#{K => build_service_opts(V, Url)}; -fold_service(_, _, Acc) -> - Acc. - --spec build_service_opts(map(), binary() | list()) -> service_opts(). -build_service_opts(V, Url) -> - Base = #{url => Url}, - Base1 = - case maps:find(transport_opts, V) of - {ok, T} -> Base#{transport_opts => T}; - error -> Base - end, - case maps:find(resolver_opts, V) of - {ok, R} -> Base1#{resolver_opts => R}; - error -> Base1 - end. - --spec make_woody_client(service_opts()) -> woody_client:options(). +-spec make_woody_client(#{atom() => _}) -> woody_client:options(). make_woody_client(#{url := Url} = Service) -> lists:foldl( fun(Opt, Acc) -> @@ -70,19 +31,7 @@ make_woody_client(#{url := Url} = Service) -> -spec get_woody_event_handlers() -> woody:ev_handlers(). get_woody_event_handlers() -> - Default = [scoper_woody_event_handler], - case genlib_app:env(dmt_api, woody_event_handlers, Default) of - Handler when is_atom(Handler) -> Handler; - {Mod, Opts} when is_atom(Mod) -> {Mod, Opts}; - [_ | _] = List -> [ensure_ev_handler(H) || H <- List]; - [] -> Default; - _ -> Default - end. - --spec ensure_ev_handler(term()) -> woody:ev_handler() | no_return(). -ensure_ev_handler(Mod) when is_atom(Mod) -> Mod; -ensure_ev_handler({Mod, Opts}) when is_atom(Mod) -> {Mod, Opts}; -ensure_ev_handler(Other) -> erlang:error({bad_ev_handler, Other}). + genlib_app:env(dmt_api, woody_event_handlers, [scoper_woody_event_handler]). -spec ensure_woody_deadline_set(woody_context:ctx(), woody_deadline:deadline()) -> woody_context:ctx(). ensure_woody_deadline_set(WoodyContext, Default) -> diff --git a/apps/dmt/src/dmt_app.erl b/apps/dmt/src/dmt_app.erl index 7f5a766..57b060c 100644 --- a/apps/dmt/src/dmt_app.erl +++ b/apps/dmt/src/dmt_app.erl @@ -9,15 +9,9 @@ -export([start/2, stop/1]). --spec start(application:start_type(), term()) -> - {ok, pid()} | {ok, pid(), term()} | {error, term()}. start(_StartType, _StartArgs) -> - case dmt_sup:start_link() of - ignore -> {error, ignore}; - Other -> Other - end. + dmt_sup:start_link(). --spec stop(term()) -> ok. stop(_State) -> ok. diff --git a/apps/dmt/src/dmt_author.erl b/apps/dmt/src/dmt_author.erl index 08c8736..68e0643 100644 --- a/apps/dmt/src/dmt_author.erl +++ b/apps/dmt/src/dmt_author.erl @@ -2,8 +2,6 @@ %% Public API --include_lib("damsel/include/dmsl_domain_conf_v2_thrift.hrl"). - -define(POOL_NAME, author_pool). -export([ @@ -13,33 +11,21 @@ delete/1 ]). --type author_id() :: binary(). --type name() :: binary(). --type email() :: binary(). --type author() :: dmsl_domain_conf_v2_thrift:'Author'(). - --export_type([author_id/0, name/0, email/0, author/0]). - %% Optional: Extended API (can be uncommented if these functions should be exposed) %% -export([ %% list/2, %% search/2 %% ]). --spec insert(name(), email()) -> - {ok, author_id()} | {ok, {already_exists, author_id()}} | {error, unknown}. insert(Name, Email) -> dmt_author_database:insert(?POOL_NAME, Name, Email). --spec get(author_id()) -> {ok, author()} | {error, author_not_found | term()}. get(AuthorID) -> dmt_author_database:get(?POOL_NAME, AuthorID). --spec get_by_email(email()) -> {ok, author()} | {error, author_not_found | term()}. get_by_email(Email) -> dmt_author_database:get_by_email(?POOL_NAME, Email). --spec delete(author_id()) -> ok | {error, author_not_found | term()}. delete(AuthorID) -> dmt_author_database:delete(?POOL_NAME, AuthorID). diff --git a/apps/dmt/src/dmt_author_database.erl b/apps/dmt/src/dmt_author_database.erl index a4e64af..081e4a3 100644 --- a/apps/dmt/src/dmt_author_database.erl +++ b/apps/dmt/src/dmt_author_database.erl @@ -13,16 +13,6 @@ search/3 ]). --type worker() :: dmt_database:worker(). --type author_id() :: dmt_author:author_id(). --type name() :: dmt_author:name(). --type email() :: dmt_author:email(). --type author() :: dmt_author:author(). - --export_type([worker/0, author_id/0, name/0, email/0, author/0]). - --spec insert(worker(), name(), email()) -> - {ok, author_id()} | {ok, {already_exists, author_id()}} | {error, unknown}. insert(Worker, Name, Email) -> Sql = """ INSERT INTO author (name, email) @@ -42,7 +32,6 @@ insert(Worker, Name, Email) -> {error, unknown} end. --spec get(worker(), author_id()) -> {ok, author()} | {error, author_not_found | term()}. get(Worker, AuthorID) -> case is_uuid(AuthorID) of true -> @@ -51,7 +40,6 @@ get(Worker, AuthorID) -> {error, author_not_found} end. --spec get_(worker(), author_id()) -> {ok, author()} | {error, author_not_found | term()}. get_(Worker, AuthorID) -> Sql = """ SELECT id, name, email @@ -72,7 +60,6 @@ get_(Worker, AuthorID) -> {error, Reason} end. --spec get_by_email(worker(), email()) -> {ok, author()} | {error, author_not_found | term()}. get_by_email(Worker, Email) -> Sql = """ SELECT id, name, email @@ -93,7 +80,6 @@ get_by_email(Worker, Email) -> {error, Reason} end. --spec delete(worker(), author_id()) -> ok | {error, author_not_found | term()}. delete(Worker, AuthorID) -> case is_uuid(AuthorID) of true -> @@ -102,7 +88,6 @@ delete(Worker, AuthorID) -> {error, author_not_found} end. --spec delete_(worker(), author_id()) -> ok | {error, author_not_found | term()}. delete_(Worker, AuthorID) -> Sql = """ DELETE FROM author @@ -118,8 +103,6 @@ delete_(Worker, AuthorID) -> {error, Reason} end. --spec list(worker(), pos_integer(), non_neg_integer()) -> - {ok, [author()]} | {error, term()}. list(Worker, Limit, Offset) -> Sql = """ SELECT id, name, email @@ -143,7 +126,6 @@ list(Worker, Limit, Offset) -> {error, Reason} end. --spec search(worker(), binary(), pos_integer()) -> {ok, [author()]} | {error, term()}. search(Worker, SearchTerm, Limit) -> Sql = """ SELECT id, name, email FROM author @@ -170,18 +152,11 @@ search(Worker, SearchTerm, Limit) -> %% Internal functions --spec is_uuid(term()) -> boolean(). -is_uuid(<>) -> - try_string_to_uuid(UUID); -is_uuid(<>) -> - try_string_to_uuid(UUID); -is_uuid(_) -> - false. - --spec try_string_to_uuid(uuid:uuid_string()) -> boolean(). -try_string_to_uuid(UUID) -> +is_uuid(UUID) -> try uuid:string_to_uuid(UUID) of - _ -> true + _UUID -> + true catch - exit:badarg -> false + exit:badarg -> + false end. diff --git a/apps/dmt/src/dmt_author_handler.erl b/apps/dmt/src/dmt_author_handler.erl index 4ec02a1..fd2c549 100644 --- a/apps/dmt/src/dmt_author_handler.erl +++ b/apps/dmt/src/dmt_author_handler.erl @@ -2,32 +2,17 @@ -include_lib("damsel/include/dmsl_domain_conf_v2_thrift.hrl"). --behaviour(woody_server_thrift_handler). - %% API -export([handle_function/4]). --type options() :: dmt_api_woody_utils:handler_options(). - --export_type([options/0]). - --spec handle_function(woody:func(), woody:args(), woody_context:ctx(), options()) -> - {ok, woody:result()} | no_return(). handle_function(Function, Args, WoodyContext0, Options) -> DefaultDeadline = woody_deadline:from_timeout(default_handling_timeout(Options)), WoodyContext = dmt_api_woody_utils:ensure_woody_deadline_set(WoodyContext0, DefaultDeadline), - %% Cast: `woody:args()` is `tuple() | any()`; each `do_handle_function/4` - %% clause pattern-matches a specific arg tuple guaranteed by the thrift - %% schema. The shape is enforced by woody at deserialisation, not by - %% the type system, so we cross the trust boundary explicitly here. - do_handle_function(Function, eqwalizer:dynamic_cast(Args), WoodyContext, Options). + do_handle_function(Function, Args, WoodyContext, Options). --spec default_handling_timeout(options()) -> timeout(). default_handling_timeout(#{default_handling_timeout := Timeout}) -> Timeout. --spec do_handle_function(woody:func(), eqwalizer:dynamic(tuple()), woody_context:ctx(), options()) -> - {ok, woody:result()} | no_return(). %% Implement the Create function do_handle_function('Create', {Params}, _Context, _Options) -> #domain_conf_v2_AuthorParams{email = Email, name = Name} = Params, diff --git a/apps/dmt/src/dmt_database.erl b/apps/dmt/src/dmt_database.erl index b814f70..e2b341a 100644 --- a/apps/dmt/src/dmt_database.erl +++ b/apps/dmt/src/dmt_database.erl @@ -26,30 +26,6 @@ -export([search_related_graph/8]). -export([parse_entity_validation_error/1]). --type worker() :: atom() | epgsql:connection(). --type version() :: integer(). --type entity_id() :: binary(). --type entity_type() :: atom() | binary(). --type author_id() :: dmt_author:author_id(). --type object_map() :: dmt_mapper:object_map(). --type edge_map() :: #{ - source_ref := dmsl_domain_thrift:'Reference'(), - target_ref := dmsl_domain_thrift:'Reference'() -}. --type sql_error() :: term(). - --export_type([ - worker/0, - version/0, - entity_id/0, - entity_type/0, - author_id/0, - object_map/0, - sql_error/0, - edge_map/0 -]). - --spec get_latest_version(worker()) -> {ok, version()} | {error, sql_error()}. get_latest_version(Worker) -> Query1 = """ @@ -64,8 +40,6 @@ get_latest_version(Worker) -> {error, Reason} end. --spec get_object_latest_version(worker(), entity_id()) -> - {ok, version()} | {error, not_found | sql_error()}. get_object_latest_version(Worker, ChangedObjectId) -> Query0 = """ @@ -86,7 +60,6 @@ get_object_latest_version(Worker, ChangedObjectId) -> {error, Reason} end. --spec get_new_version(worker(), author_id()) -> {ok, version()} | {error, sql_error()}. get_new_version(Worker, AuthorID) -> {ok, #{ git_ref := GitRef @@ -103,13 +76,12 @@ get_new_version(Worker, AuthorID) -> {error, Reason} end. --spec clean_utf8_string(binary() | string()) -> binary(). clean_utf8_string(String) -> % Convert to binary if it's not already Binary = case is_binary(String) of true -> String; - false -> to_utf8_binary(unicode:characters_to_binary(String)) + false -> unicode:characters_to_binary(String) end, % Remove any invalid UTF-8 sequences case unicode:characters_to_binary(Binary, utf8, utf8) of @@ -118,22 +90,11 @@ clean_utf8_string(String) -> % and convert it to UTF-8. % binary_to_list(Binary) gives a list of bytes (0-255), % which are treated as Latin-1 codepoints. - to_utf8_binary(unicode:characters_to_binary(binary_to_list(Binary), utf8)); - CleanBinary when is_binary(CleanBinary) -> - CleanBinary; - {incomplete, Bin, _Rest} when is_binary(Bin) -> - Bin + unicode:characters_to_binary(binary_to_list(Binary), utf8); + CleanBinary -> + CleanBinary end. --spec to_utf8_binary( - binary() | {error, binary(), term()} | {incomplete, binary(), binary()} -) -> binary(). -to_utf8_binary(Bin) when is_binary(Bin) -> Bin; -to_utf8_binary({error, Bin, _Rest}) when is_binary(Bin) -> Bin; -to_utf8_binary({incomplete, Bin, _Rest}) when is_binary(Bin) -> Bin. - --spec insert_object(worker(), entity_id(), entity_type(), version(), binary(), binary() | string()) -> - ok | {error, sql_error()}. insert_object(Worker, ID1, Type, Version, Data1, SearchVector) -> Query = """ INSERT INTO entity @@ -152,10 +113,6 @@ insert_object(Worker, ID1, Type, Version, Data1, SearchVector) -> {error, Reason} end. --spec update_object( - worker(), entity_id(), entity_type(), version(), binary(), binary() | string(), boolean() -) -> - ok | {error, sql_error()}. update_object( Worker, ID1, @@ -181,13 +138,6 @@ update_object( {error, Reason} end. --spec insert_relations(worker(), entity_id(), entity_id(), version(), boolean()) -> - ok - | {error, - {duplicate_relation, binary()} - | {source_entity_not_found, dmsl_domain_thrift:'Reference'()} - | {target_entity_not_found, dmsl_domain_thrift:'Reference'()} - | sql_error()}. insert_relations(Worker, SourceID, TargetID, Version, IsActive) -> Query = """ INSERT INTO entity_relation @@ -223,8 +173,8 @@ insert_relations(Worker, SourceID, TargetID, Version, IsActive) -> %% @doc Parse validation error messages from the validate_entity_exists trigger %% Expected format: "ENTITY_NOT_EXISTS|SOURCE|entity_id" or "ENTITY_NOT_EXISTS|TARGET|entity_id" -spec parse_entity_validation_error(binary()) -> - {source_entity_not_found, dmsl_domain_thrift:'Reference'()} - | {target_entity_not_found, dmsl_domain_thrift:'Reference'()} + {source_entity_not_found, binary()} + | {target_entity_not_found, binary()} | unknown. parse_entity_validation_error(Message) -> case binary:split(Message, <<"|">>, [global]) of @@ -236,8 +186,6 @@ parse_entity_validation_error(Message) -> unknown end. --spec get_references_to(worker(), entity_id(), version()) -> - [dmsl_domain_thrift:'Reference'()] | no_return(). get_references_to(Worker, ID, Version) -> Query = """ WITH LatestEdge AS ( @@ -263,13 +211,11 @@ get_references_to(Worker, ID, Version) -> Params = [Version, ID], case epg_pool:query(Worker, Query, Params) of {ok, _Columns, Refs} -> - [dmt_mapper:string_to_ref(Res) || {Res} <- Refs, is_binary(Res)]; + lists:map(fun({Res}) -> dmt_mapper:string_to_ref(Res) end, Refs); {error, Reason} -> - erlang:error({sql_error, Reason}) + {error, Reason} end. --spec get_referenced_by(worker(), entity_id(), version()) -> - [dmsl_domain_thrift:'Reference'()] | no_return(). get_referenced_by(Worker, ID, Version) -> Query = """ WITH LatestEdge AS ( @@ -295,13 +241,11 @@ get_referenced_by(Worker, ID, Version) -> Params = [Version, ID], case epg_pool:query(Worker, Query, Params) of {ok, _Columns, Refs} -> - [dmt_mapper:string_to_ref(Res) || {Res} <- Refs, is_binary(Res)]; + lists:map(fun({Res}) -> dmt_mapper:string_to_ref(Res) end, Refs); {error, Reason} -> - erlang:error({sql_error, Reason}) + {error, Reason} end. --spec get_next_sequence(worker(), entity_type()) -> - {ok, integer()} | {error, sequence_not_enabled | sql_error()}. get_next_sequence(Worker, Type) -> Query = """ UPDATE entity_type @@ -320,7 +264,6 @@ get_next_sequence(Worker, Type) -> {error, Reason} end. --spec check_if_object_id_active(worker(), entity_id()) -> boolean() | {error, sql_error()}. check_if_object_id_active(Worker, ID0) -> Query = """ SELECT is_active @@ -338,7 +281,6 @@ check_if_object_id_active(Worker, ID0) -> {error, Reason} end. --spec check_version_exists(worker(), version()) -> boolean() | {error, sql_error()}. check_version_exists(Worker, Version) -> Query = """ SELECT 1 @@ -355,8 +297,6 @@ check_version_exists(Worker, Version) -> {error, Reason} end. --spec get_object(worker(), entity_id(), version()) -> - {ok, object_map()} | {error, not_found | sql_error()}. get_object(Worker, ID0, Version) -> Request = """ WITH LatestVersionAtRequestedTime AS ( @@ -398,8 +338,6 @@ get_object(Worker, ID0, Version) -> end end. --spec get_objects(worker(), [entity_id()], version()) -> - {ok, [object_map()]} | {error, sql_error()}. get_objects(Worker, IDs, Version) -> Request = """ WITH LatestVersionAtRequestedTime AS ( @@ -436,8 +374,6 @@ get_objects(Worker, IDs, Version) -> {error, Reason} end. --spec get_latest_object(worker(), entity_id()) -> - {ok, object_map()} | {error, not_found | sql_error()}. get_latest_object(Worker, ID0) -> Request = """ WITH LatestVersion AS ( @@ -473,8 +409,6 @@ get_latest_object(Worker, ID0) -> end end. --spec get_version_creator(worker(), version()) -> - {ok, author_id()} | {error, not_found | sql_error()}. get_version_creator(Worker, Version) -> Request = """ SELECT created_by @@ -490,13 +424,6 @@ get_version_creator(Worker, Version) -> {ok, CreatedBy} end. --spec get_version(worker(), version()) -> - {ok, #{ - version := version(), - created_at := dmt_mapper:pg_datetime(), - created_by := author_id() - }} - | {error, not_found | sql_error()}. get_version(Worker, Version) -> Request = """ SELECT version, created_at, created_by @@ -508,34 +435,14 @@ get_version(Worker, Version) -> case epg_pool:query(Worker, Request, [Version]) of {ok, _Columns, []} -> {error, not_found}; - {ok, _Columns, [{Version, CreatedAt, CreatedBy}]} when is_binary(CreatedBy) -> + {ok, _Columns, [{Version, CreatedAt, CreatedBy}]} -> {ok, #{ version => Version, - created_at => ensure_pg_datetime(CreatedAt), + created_at => CreatedAt, created_by => CreatedBy - }}; - {error, Reason} -> - {error, Reason} + }} end. -%% @doc Refine an opaque epgsql column value into `pg_datetime()`. Does no -%% conversion — just asserts the runtime shape and lets the type system see it. --spec ensure_pg_datetime(term()) -> dmt_mapper:pg_datetime() | no_return(). -ensure_pg_datetime({{Y, Mo, D}, {H, Mi, S}} = T) when - is_integer(Y), - is_integer(Mo), - is_integer(D), - is_integer(H), - is_integer(Mi), - is_integer(S) orelse is_float(S) --> - T; -ensure_pg_datetime(Other) -> - erlang:error({bad_pg_datetime, Other}). - --spec get_object_history(worker(), entity_id(), pos_integer(), non_neg_integer()) -> - {ok, [object_map()], non_neg_integer() | undefined} - | {error, not_found | sql_error()}. get_object_history(Worker, Ref, Limit, Offset) -> Query = """ SELECT e.id, @@ -568,7 +475,6 @@ get_object_history(Worker, Ref, Limit, Offset) -> {error, Reason} end. --spec has_more_object_history(worker(), entity_id(), non_neg_integer()) -> boolean(). has_more_object_history(Worker, Ref, Offset) -> Query = """ SELECT 1 @@ -589,9 +495,6 @@ has_more_object_history(Worker, Ref, Offset) -> false end. --spec get_all_objects_history(worker(), pos_integer(), non_neg_integer()) -> - {ok, [object_map()], non_neg_integer() | undefined} - | {error, sql_error()}. get_all_objects_history(Worker, Limit, Offset) -> Query = """ SELECT e.id, @@ -621,7 +524,6 @@ get_all_objects_history(Worker, Limit, Offset) -> {error, Reason} end. --spec has_more_all_objects_history(worker(), non_neg_integer()) -> boolean(). has_more_all_objects_history(Worker, Offset) -> % Сначала проверим, есть ли еще записи после текущего смещения + лимит Query = """ @@ -643,10 +545,6 @@ has_more_all_objects_history(Worker, Offset) -> false end. --spec search_objects( - worker(), binary(), version(), entity_type() | undefined, pos_integer(), non_neg_integer() -) -> - {ok, {[object_map()], non_neg_integer() | undefined}}. search_objects(Worker, <<"*">>, Version, Type, Limit, Offset) -> % Use a pattern where the condition is always true when Type is NULL TypeValue = @@ -766,10 +664,6 @@ search_objects(Worker, Query, Version, Type, Limit, Offset) -> end. % Helper function to check if there are more search results --spec has_more_search_results( - worker(), binary(), version(), entity_type() | undefined, non_neg_integer() -) -> - boolean(). has_more_search_results(Worker, <<"*">>, Version, TypeValue, Offset) -> CheckMoreQuery = """ WITH LatestVersionAtRequestedTime AS ( @@ -840,8 +734,6 @@ has_more_search_results(Worker, Query, Version, TypeValue, Offset) -> false end. --spec check_entity_type_exists(worker(), entity_type()) -> - ok | {error, object_type_not_found}. check_entity_type_exists(Worker, Type) -> CheckMoreQuery = """ SELECT 1 FROM entity_type @@ -861,8 +753,6 @@ check_entity_type_exists(Worker, Type) -> {error, object_type_not_found} end. --spec get_all_objects(worker(), version()) -> - {ok, [object_map()]} | {error, sql_error()}. get_all_objects(Worker, Version) -> Query = """ WITH LatestVersions AS ( @@ -897,16 +787,6 @@ get_all_objects(Worker, Version) -> {error, Reason} end. --spec get_related_graph( - worker(), - entity_id(), - version(), - non_neg_integer() | undefined, - boolean() | undefined, - boolean() | undefined, - entity_type() | undefined -) -> - {ok, {[object_map()], [edge_map()]}} | {error, sql_error()}. get_related_graph( Worker, ObjectRef, Version, Depth, IncludeInbound, IncludeOutbound, TypeFilter ) -> @@ -920,10 +800,6 @@ get_related_graph( end. %% Helper function to get objects and apply type filter --spec get_objects_and_filter( - worker(), [entity_id()], version(), entity_type() | undefined, [edge_map()], term() -) -> - {ok, {[object_map()], [edge_map()]}} | {error, sql_error()}. get_objects_and_filter(Worker, EntityIds, Version, TypeFilter, Edges, ObjectRef) -> case get_objects(Worker, EntityIds, Version) of {ok, AllNodes} -> @@ -937,7 +813,6 @@ get_objects_and_filter(Worker, EntityIds, Version, TypeFilter, Edges, ObjectRef) end. %% Filter nodes by type if type filter is specified --spec filter_nodes_by_type([object_map()], entity_type() | undefined) -> [object_map()]. filter_nodes_by_type(Nodes, undefined) -> Nodes; filter_nodes_by_type(Nodes, FilterType) -> @@ -948,7 +823,6 @@ filter_nodes_by_type(Nodes, FilterType) -> end, [Node || Node <- Nodes, maps:get(type, Node) =:= FilterTypeBinary]. --spec filter_edges_by_nodes([edge_map()], [object_map()]) -> [edge_map()]. filter_edges_by_nodes(Edges, Nodes) -> logger:error("filter_edges_by_nodes Edges: ~p, Nodes: ~p", [Edges, Nodes]), lists:filter( @@ -970,15 +844,6 @@ filter_edges_by_nodes(Edges, Nodes) -> Edges ). --spec get_related_graph_edges( - worker(), - entity_id(), - version(), - non_neg_integer() | undefined, - boolean() | undefined, - boolean() | undefined -) -> - {ok, {[binary()], [edge_map()]}} | {error, sql_error()}. get_related_graph_edges(Worker, ObjectRef, Version, Depth, IncludeInbound, IncludeOutbound) -> Query = """ WITH RECURSIVE @@ -1074,7 +939,6 @@ get_related_graph_edges(Worker, ObjectRef, Version, Depth, IncludeInbound, Inclu {error, Reason} end. --spec parse_graph_edges_result([{binary(), binary()}]) -> {[binary()], [edge_map()]}. parse_graph_edges_result(Rows) -> Edges = [ #{ @@ -1091,16 +955,6 @@ parse_graph_edges_result(Rows) -> {EntityIds1, Edges}. --spec get_multiple_related_graph( - worker(), - [dmsl_domain_thrift:'Reference'()], - version(), - non_neg_integer() | undefined, - boolean() | undefined, - boolean() | undefined, - entity_type() | undefined -) -> - {ok, {[object_map()], [edge_map()]}} | {error, sql_error()}. get_multiple_related_graph( Worker, ObjectRefs, Version, Depth, IncludeInbound, IncludeOutbound, TypeFilter ) -> @@ -1118,15 +972,6 @@ get_multiple_related_graph( {error, Reason} end. --spec get_multiple_related_graph_edges( - worker(), - [binary()], - version(), - non_neg_integer() | undefined, - boolean() | undefined, - boolean() | undefined -) -> - {ok, {[binary()], [edge_map()]}} | {error, sql_error()}. get_multiple_related_graph_edges(Worker, ObjectRefStrings, Version, Depth, IncludeInbound, IncludeOutbound) -> Query = """ WITH RECURSIVE @@ -1224,17 +1069,6 @@ get_multiple_related_graph_edges(Worker, ObjectRefStrings, Version, Depth, Inclu {error, Reason} end. --spec search_related_graph( - worker(), - binary(), - binary() | undefined, - version(), - non_neg_integer() | undefined, - boolean() | undefined, - boolean() | undefined, - binary() | undefined -) -> - {ok, {[object_map()], [edge_map()]}} | {error, sql_error()}. search_related_graph( Worker, Query, SearchedType, Version, Depth, IncludeInbound, IncludeOutbound, ReturnedType ) -> diff --git a/apps/dmt/src/dmt_db_migration.erl b/apps/dmt/src/dmt_db_migration.erl index f7b8dd9..0d2fe3a 100644 --- a/apps/dmt/src/dmt_db_migration.erl +++ b/apps/dmt/src/dmt_db_migration.erl @@ -157,9 +157,7 @@ with_connection(Args, Fun) -> {ok, Conn} -> Fun(Conn); {error, Error} -> - {error, lists:flatten(io_lib:format("Failed to connect to database: ~p~n", [Args])), [ - Error - ]} + {error, io_lib:format("Failed to connect to database: ~p~n", [Args]), [Error]} end. connection_opts(Args) -> @@ -175,71 +173,40 @@ connection_opts(_Args, {url, DatabaseUrl}) -> case uri_string:parse(string:trim(DatabaseUrl)) of {error, Error, Term} -> {error, {Error, Term}}; - #{userinfo := UserPass, host := Host, path := Path} = Map -> - UserPassStr = to_string(UserPass), - PathStr = to_string(Path), - {User, Pass} = split_user_pass(UserPassStr), + Map = #{userinfo := UserPass, host := Host, path := Path} -> + {User, Pass} = + case string:split(UserPass, ":") of + [[]] -> {"postgres", ""}; + [U] -> {U, ""}; + [[], []] -> {"postgres", ""}; + [U, P] -> {U, P} + end, + ConnectionOpts = #{ port => maps:get(port, Map, 5432), username => User, password => Pass, host => Host, - database => string:slice(PathStr, 1) + database => string:slice(Path, 1) }, - apply_query_opts(maps:get(query, Map, []), ConnectionOpts) - end. -%% @doc Normalise chardata into a string, dropping any error/incomplete tails. --spec to_string(unicode:chardata()) -> string(). -to_string(C) -> - case unicode:characters_to_list(C) of - L when is_list(L) -> L; - {error, L, _} when is_list(L) -> L; - {incomplete, L, _} when is_list(L) -> L - end. - --spec split_user_pass(string()) -> {string(), string()}. -split_user_pass(UserPass) -> - case string:split(UserPass, ":") of - [[]] -> {"postgres", ""}; - [U] when is_list(U) -> {U, ""}; - [[], []] -> {"postgres", ""}; - [U, P] when is_list(U), is_list(P) -> {U, P}; - _ -> {"postgres", ""} + case maps:get(query, Map, []) of + [] -> + {ok, ConnectionOpts}; + "?" ++ QueryString -> + case uri_string:dissect_query(QueryString) of + [] -> + {ok, ConnectionOpts}; + QueryList -> + case proplists:get_value("ssl", QueryList) of + undefined -> {ok, ConnectionOpts}; + [] -> {ok, maps:put(ssl, true, ConnectionOpts)}; + Value -> {ok, maps:put(ssl, list_to_atom(Value), ConnectionOpts)} + end + end + end end. --spec apply_query_opts(term(), map()) -> {ok, map()}. -apply_query_opts([], ConnectionOpts) -> - {ok, ConnectionOpts}; -apply_query_opts([$? | QueryString], ConnectionOpts) when is_list(QueryString) -> - %% Cast: `uri_string:dissect_query/1` wants `uri_string:uri_string()` - %% (a specific `[char() | byte() | ...]` chardata variant). - %% `is_list/1` only narrows `term()` to `[term()]`; eqwalizer doesn't - %% refine list element types, but at runtime this is the tail of the - %% printable URL string we just parsed. - case uri_string:dissect_query(eqwalizer:dynamic_cast(QueryString)) of - [_ | _] = QueryList -> - apply_ssl_opt(proplists:get_value("ssl", QueryList), ConnectionOpts); - _ -> - {ok, ConnectionOpts} - end; -apply_query_opts(_, ConnectionOpts) -> - {ok, ConnectionOpts}. - --spec apply_ssl_opt(term(), map()) -> {ok, map()}. -apply_ssl_opt(undefined, ConnectionOpts) -> - {ok, ConnectionOpts}; -apply_ssl_opt(true, ConnectionOpts) -> - {ok, maps:put(ssl, true, ConnectionOpts)}; -apply_ssl_opt(Value, ConnectionOpts) when is_list(Value) -> - %% Cast: `to_string/1` (and the underlying `unicode:characters_to_list/1`) - %% wants `unicode:chardata()`. `is_list/1` only proves `[term()]`; the - %% value comes from `uri_string:dissect_query/1` which produces chardata - %% at runtime. - {ok, maps:put(ssl, list_to_atom(to_string(eqwalizer:dynamic_cast(Value))), ConnectionOpts)}; -apply_ssl_opt(_, ConnectionOpts) -> - {ok, ConnectionOpts}. - -spec open_connection(list() | map()) -> {ok, epgsql:connection()} | {error, term()}. open_connection(Args) when is_list(Args) -> {ok, Opts} = connection_opts(Args), diff --git a/apps/dmt/src/dmt_json.erl b/apps/dmt/src/dmt_json.erl index 1f337ec..b650930 100644 --- a/apps/dmt/src/dmt_json.erl +++ b/apps/dmt/src/dmt_json.erl @@ -12,11 +12,7 @@ decode(S) when is_binary(S) -> jsone:decode(S, [{keys, binary}, {object_format, proplist}]); decode(S) when is_list(S) -> - case unicode:characters_to_binary(S) of - Bin when is_binary(Bin) -> decode(Bin); - {error, Bin, _} when is_binary(Bin) -> decode(Bin); - {incomplete, Bin, _} when is_binary(Bin) -> decode(Bin) - end. + decode(unicode:characters_to_binary(S)). -spec encode(jsone:json_value()) -> binary(). encode(J) -> @@ -28,7 +24,7 @@ encode(J) -> -define(is_number(T), (?is_integer(T) orelse T == double)). -define(is_scalar(T), (?is_number(T) orelse T == string orelse element(1, T) == enum)). --spec json_to_term(jsone:json_value(), dmt_thrift:thrift_type()) -> eqwalizer:dynamic(). +-spec json_to_term(jsone:json_value(), dmt_thrift:thrift_type()) -> term(). json_to_term(Json, Type) -> json_to_term(Json, Type, []). diff --git a/apps/dmt/src/dmt_mapper.erl b/apps/dmt/src/dmt_mapper.erl index 0284c41..5bb2cde 100644 --- a/apps/dmt/src/dmt_mapper.erl +++ b/apps/dmt/src/dmt_mapper.erl @@ -14,20 +14,9 @@ -export([from_string/1]). -export([extract_searchable_text_from_term/1]). -%% Object map keyed by either atom (when produced from typed sources) or binary -%% (when produced from epgsql row column names). --type object_map() :: #{atom() | binary() => term()}. --type pg_datetime() :: calendar:datetime() | {calendar:date(), {0..23, 0..59, float()}}. - --export_type([object_map/0, pg_datetime/0]). - --spec to_marshalled_maps([epgsql:column()], [epgsql:equery_row()]) -> [object_map()]. to_marshalled_maps(Columns, Rows) -> to_marshalled_maps(Columns, Rows, fun marshall_object/1). --spec to_marshalled_maps( - [epgsql:column()], [epgsql:equery_row()], fun((object_map()) -> Out) -) -> [Out]. to_marshalled_maps(Columns, Rows, TransformRowFun) -> ColNumbers = erlang:length(Columns), Seq = lists:seq(1, ColNumbers), @@ -47,35 +36,19 @@ to_marshalled_maps(Columns, Rows, TransformRowFun) -> ). %% for reference https://github.com/epgsql/epgsql#data-representation --spec convert(epgsql:epgsql_type(), term()) -> term(). convert(timestamp, Value) -> - convert_datetime(Value); + datetime_to_binary(Value); convert(timestamptz, Value) -> - convert_datetime(Value); + datetime_to_binary(Value); convert(_Type, Value) -> Value. --spec convert_datetime(term()) -> binary() | term(). -convert_datetime({{Y, Mo, D}, {H, Mi, S}} = Value) when - is_integer(Y), - is_integer(Mo), - is_integer(D), - is_integer(H), - is_integer(Mi), - is_integer(S) orelse is_float(S) --> - datetime_to_binary(Value); -convert_datetime(Other) -> - Other. - --spec datetime_to_binary(pg_datetime()) -> binary(). datetime_to_binary({Date, {Hour, Minute, Second}}) when is_float(Second) -> datetime_to_binary({Date, {Hour, Minute, trunc(Second)}}); -datetime_to_binary({_Date, {_H, _M, S}} = DateTime) when is_integer(S) -> +datetime_to_binary(DateTime) -> UnixTime = genlib_time:daytime_to_unixtime(DateTime), genlib_rfc3339:format(UnixTime, second). --spec marshall_object(object_map()) -> dmt_object:object(). marshall_object(#{ <<"id">> := ID, <<"entity_type">> := Type, @@ -83,11 +56,7 @@ marshall_object(#{ <<"data">> := Data, <<"created_at">> := CreatedAt, <<"is_active">> := IsActive -}) when - is_binary(ID), - is_binary(Data), - is_boolean(IsActive) --> +}) -> dmt_object:just_object( string_to_ref(ID), Type, @@ -100,43 +69,28 @@ marshall_object(#{ -define(REF_TYPE, {struct, union, {dmsl_domain_thrift, 'Reference'}}). -define(OBJECT_TYPE, {struct, union, {dmsl_domain_thrift, 'DomainObject'}}). --spec ref_to_string(dmsl_domain_thrift:'Reference'()) -> binary(). ref_to_string({_Type, _} = Ref) -> thrift_term_to_string_(Ref, ?REF_TYPE). -%% Returns the decoded thrift Reference. The string is assumed to be the -%% serialised form produced by `ref_to_string/1` — its shape is enforced by the -%% JSON-to-thrift round-trip rather than the type system, hence the dynamic -%% return type. --spec string_to_ref(binary() | string()) -> eqwalizer:dynamic(dmsl_domain_thrift:'Reference'()). string_to_ref(Str) -> string_to_thrift_term_(Str, ?REF_TYPE). --spec object_to_string(dmsl_domain_thrift:'DomainObject'()) -> binary(). object_to_string({_Type, _} = Data) -> thrift_term_to_string_(Data, ?OBJECT_TYPE). --spec string_to_object(binary() | string()) -> - eqwalizer:dynamic(dmsl_domain_thrift:'DomainObject'()). string_to_object(Str) -> string_to_thrift_term_(Str, ?OBJECT_TYPE). --spec thrift_term_to_string_(term(), dmt_thrift:thrift_type()) -> binary(). thrift_term_to_string_(Term, ThriftType) -> dmt_json:encode(dmt_json:term_to_json(Term, ThriftType)). --spec string_to_thrift_term_(binary() | string(), dmt_thrift:thrift_type()) -> eqwalizer:dynamic(). string_to_thrift_term_(Str, ThriftType) -> dmt_json:json_to_term(dmt_json:decode(Str), ThriftType). --spec to_string(term()) -> binary(). to_string(A0) -> A1 = term_to_binary(A0), base64:encode(A1). -%% Returns the decoded term — its concrete shape depends on what was passed to -%% `to_string/1` originally. Callers must guard the value before use. --spec from_string(binary()) -> term(). from_string(B0) -> B1 = base64:decode(B0), binary_to_term(B1). diff --git a/apps/dmt/src/dmt_repository.erl b/apps/dmt/src/dmt_repository.erl index a79edc7..4fa58ce 100644 --- a/apps/dmt/src/dmt_repository.erl +++ b/apps/dmt/src/dmt_repository.erl @@ -19,41 +19,8 @@ -export([get_multiple_related_graph/1]). -export([search_related_graph/1]). --type worker() :: dmt_database:worker(). --type version() :: dmt_database:version(). --type version_ref() :: {version, version()} | {head, dmsl_domain_conf_v2_thrift:'Head'()}. --type object_ref() :: dmsl_domain_thrift:'Reference'(). --type author_id() :: dmt_author:author_id(). --type operation() :: dmsl_domain_conf_v2_thrift:'Operation'(). --type object_map() :: dmt_mapper:object_map(). - --type get_error() :: version_not_found | {object_not_found, object_ref()} | term(). - --type commit_error() :: - {operation_error, {conflict, term()} | {invalid, term()}} - | version_not_found - | author_not_found - | migration_in_progress - | {object_update_too_old, {object_ref(), version()}} - | {conflict, binary()} - | eqwalizer:dynamic(). - --export_type([ - worker/0, - version/0, - version_ref/0, - object_ref/0, - author_id/0, - operation/0, - object_map/0, - get_error/0, - commit_error/0 -]). - %% --spec get_object(worker(), version_ref(), object_ref()) -> - {ok, dmsl_domain_conf_v2_thrift:'VersionedObject'()} | {error, get_error()}. get_object(Worker, {version, V}, ObjectRef) -> case get_target_object(Worker, ObjectRef, V) of {ok, #{data := Data} = Object} -> @@ -84,8 +51,6 @@ get_object(Worker, {head, #domain_conf_v2_Head{}}, ObjectRef) -> {error, Reason} end. --spec get_object_with_references(worker(), version_ref(), object_ref()) -> - {ok, dmsl_domain_conf_v2_thrift:'VersionedObjectWithReferences'()} | {error, get_error()}. get_object_with_references(Worker, {version, V}, ObjectRef) -> case get_target_object(Worker, ObjectRef, V) of {ok, #{data := Data} = Object} -> @@ -116,8 +81,6 @@ get_object_with_references(Worker, {head, #domain_conf_v2_Head{}}, ObjectRef) -> {ok, Version} = dmt_database:get_latest_version(Worker), get_object_with_references(Worker, {version, Version}, ObjectRef). --spec get_objects(worker(), version_ref(), [object_ref()]) -> - {ok, [dmsl_domain_conf_v2_thrift:'VersionedObject'()]} | {error, version_not_found | term()}. get_objects(Worker, {version, V}, ObjectRefs) -> case dmt_database:check_version_exists(Worker, V) of true -> @@ -153,8 +116,6 @@ get_objects(Worker, {head, #domain_conf_v2_Head{}}, ObjectRefs) -> {error, Reason} end. --spec get_snapshot(worker(), version_ref()) -> - {ok, dmsl_domain_conf_v2_thrift:'Snapshot'()} | {error, version_not_found | term()}. get_snapshot(Worker, {head, #domain_conf_v2_Head{}}) -> case dmt_database:get_latest_version(Worker) of {ok, LatestVersion} -> @@ -172,9 +133,10 @@ get_snapshot(Worker, {version, Version}) -> created_by := AuthorID }} = dmt_database:get_version(Worker, Version), {ok, Author} = dmt_author:get(AuthorID), + Domain = #{K => V || #{id := K, data := V} <- Objects}, {ok, #domain_conf_v2_Snapshot{ version = Version, - domain = to_domain(Objects), + domain = Domain, created_at = dmt_mapper:datetime_to_binary(CreatedAt), changed_by = Author }}; @@ -185,9 +147,6 @@ get_snapshot(Worker, {version, Version}) -> {error, version_not_found} end. --spec get_related_graph(dmsl_domain_conf_v2_thrift:'RelatedGraphRequest'()) -> - {ok, dmsl_domain_conf_v2_thrift:'RelatedGraph'()} - | {error, object_not_found | version_not_found | term()}. get_related_graph(Request) -> #domain_conf_v2_RelatedGraphRequest{ ref = ObjectRef, @@ -248,28 +207,18 @@ get_related_graph(Request) -> {error, Reason} end. -%% @doc Build a thrift `Domain` map from a list of stored objects. Each row's -%% `id` becomes the map key and `data` becomes the value; rows whose shape -%% isn't a proper `{tag, _}` tagged tuple are skipped. The result is presented -%% as `dynamic()` because eqwalizer can't refine `{atom(), _}` to the precise -%% `Reference()` / `DomainObject()` tagged-union shapes. --spec to_domain([object_map()]) -> eqwalizer:dynamic(). -to_domain(Objects) -> - maps:from_list( - [{K, V} || #{id := {KT, _} = K, data := {VT, _} = V} <- Objects, is_atom(KT), is_atom(VT)] - ). - --spec sort_objects_by_ids([object_map()], [term()]) -> [object_map()]. sort_objects_by_ids(Objects, IDs) -> % Create a map of ID -> Object for easier lookup ObjectsMap = maps:from_list([{maps:get(id, Obj), Obj} || Obj <- Objects]), % Use list comprehension to order objects according to input IDs % Skip IDs that don't have corresponding objects - [Obj || ID <- IDs, {ok, Obj} <- [maps:find(ID, ObjectsMap)]]. + [ + maps:get(ID, ObjectsMap, undefined) + || ID <- IDs, + maps:is_key(ID, ObjectsMap) + ]. --spec get_object_history(object_ref(), dmsl_domain_conf_v2_thrift:'RequestParams'()) -> - {ok, dmsl_domain_conf_v2_thrift:'ObjectVersionsResponse'()} | {error, not_found | term()}. get_object_history(ObjectRef, RequestParams) -> #domain_conf_v2_RequestParams{ limit = Limit, @@ -303,12 +252,9 @@ get_object_history(ObjectRef, RequestParams) -> end. % Done this way to keep hierarchy of calls --spec get_latest_version() -> {ok, version()} | {error, term()}. get_latest_version() -> dmt_database:get_latest_version(default_pool). --spec get_all_objects_history(dmsl_domain_conf_v2_thrift:'RequestParams'()) -> - {ok, dmsl_domain_conf_v2_thrift:'ObjectVersionsResponse'()} | {error, term()}. get_all_objects_history(Request) -> #domain_conf_v2_RequestParams{ limit = Limit, @@ -338,47 +284,19 @@ get_all_objects_history(Request) -> {error, Reason} end. --spec maybe_to_string(term(), Default) -> binary() | Default. maybe_to_string(undefined, Default) -> Default; maybe_to_string(Value, _) -> dmt_mapper:to_string(Value). --spec maybe_from_string(binary() | undefined, Default) -> integer() | Default | no_return(). -maybe_from_string(undefined, Default) -> - Default; -maybe_from_string(Value, _) -> - case dmt_mapper:from_string(Value) of - N when is_integer(N) -> N; - Other -> erlang:error({bad_continuation_token, Other}) - end. +maybe_from_string(undefined, Default) -> Default; +maybe_from_string(Value, _) -> dmt_mapper:from_string(Value). --spec marshall_to_object_info(object_map()) -> dmsl_domain_conf_v2_thrift:'VersionedObjectInfo'(). marshall_to_object_info(Object) -> #domain_conf_v2_VersionedObjectInfo{ - version = get_version(Object), - changed_at = get_timestamp(created_at, Object), - changed_by = get_author(created_by, Object) + version = maps:get(version, Object), + changed_at = maps:get(created_at, Object), + changed_by = maps:get(created_by, Object) }. --spec get_version(object_map()) -> dmsl_domain_conf_v2_thrift:'Version'() | no_return(). -get_version(#{version := V}) when is_integer(V) -> V; -get_version(Object) -> erlang:error({bad_version, Object}). - --spec get_timestamp(atom(), object_map()) -> dmsl_base_thrift:'Timestamp'() | no_return(). -get_timestamp(Key, Object) -> - case maps:get(Key, Object, undefined) of - B when is_binary(B) -> B; - Other -> erlang:error({bad_timestamp, Key, Other}) - end. - --spec get_author(atom(), object_map()) -> dmsl_domain_conf_v2_thrift:'Author'() | no_return(). -get_author(Key, Object) -> - case maps:get(Key, Object, undefined) of - #domain_conf_v2_Author{} = A -> A; - Other -> erlang:error({bad_author, Key, Other}) - end. - --spec search_objects(dmsl_domain_conf_v2_thrift:'SearchRequestParams'()) -> - {ok, dmsl_domain_conf_v2_thrift:'SearchResponse'()} | {error, object_type_not_found | term()}. search_objects(Request) -> #domain_conf_v2_SearchRequestParams{ query = Query, @@ -422,9 +340,6 @@ search_objects(Request) -> {error, Reason} end. --spec search_full_objects(dmsl_domain_conf_v2_thrift:'SearchRequestParams'()) -> - {ok, dmsl_domain_conf_v2_thrift:'SearchFullResponse'()} - | {error, object_type_not_found | term()}. search_full_objects(Request) -> #domain_conf_v2_SearchRequestParams{ query = Query, @@ -466,7 +381,6 @@ search_full_objects(Request) -> {error, Reason} end. --spec filter_search_results([object_map()]) -> [object_map()]. filter_search_results(Objects) -> lists:filter( fun(Object) -> @@ -489,8 +403,6 @@ filter_search_results(Objects) -> Objects ). --spec maybe_check_entity_type_exists(atom() | binary() | undefined) -> - ok | {error, object_type_not_found | term()}. maybe_check_entity_type_exists(undefined) -> ok; maybe_check_entity_type_exists(Type) -> dmt_database:check_entity_type_exists(default_pool, Type). @@ -610,8 +522,6 @@ commit_relations_changes(Worker, NewVersion, RelationsChanges) -> RelationsChanges ). --spec commit(version(), [operation()], author_id()) -> - {ok, version(), [term()]} | {error, commit_error()}. commit(Version, Operations, AuthorID) -> Result = epg_pool:transaction( default_pool, @@ -640,7 +550,7 @@ commit(Version, Operations, AuthorID) -> end ), case Result of - {ok, ResVersion, NewObjects, AuthorID} when is_integer(ResVersion), is_list(NewObjects) -> + {ok, ResVersion, NewObjects, AuthorID} -> {ok, ResVersion, NewObjects}; {error, {error, error, _, conflict_detected, Msg, _}} -> {error, {conflict, Msg}}; @@ -651,11 +561,7 @@ commit(Version, Operations, AuthorID) -> {error, {invalid, _} = Error} -> {error, {operation_error, Error}}; {error, Error} -> - %% Cast: catch-all for unrecognised errors from `epg_pool:transaction/2`. - %% `Error` is `term()` here; `commit_error()` enumerates the structured - %% alternatives we map. The handler maps the known atoms / tuples and - %% anything else propagates as-is for a generic internal-error response. - {error, eqwalizer:dynamic_cast(Error)} + {error, Error} end. validate_no_references_to_entities(Worker, RemovedObjectsReferences, Version) -> @@ -738,17 +644,11 @@ give_data_id({Tag, Data}, Ref) -> end, DomainObjects ), - finish_give_data_id(ObjectName, Tag, Data, Ref). - --spec finish_give_data_id(term(), atom(), term(), term()) -> {atom(), tuple()}. -finish_give_data_id(ObjectName0, Tag, Data, Ref) when is_atom(ObjectName0) -> - %% Cast: `dmsl_domain_thrift:struct_info/1` and `:record_name/1` accept - %% a specific atom union (`struct_name() | exception_name()`). `ObjectName0` - %% was pulled out of a runtime schema tuple, so the type system can only - %% see `atom()` — wider than the union the callees declare. - ObjectName = eqwalizer:dynamic_cast(ObjectName0), RecordName = dmsl_domain_thrift:record_name(ObjectName), - {_, _, [FirstField, SecondField]} = dmsl_domain_thrift:struct_info(ObjectName), + {_, _, [ + FirstField, + SecondField + ]} = dmsl_domain_thrift:struct_info(ObjectName), First = get_object_field(FirstField, Data, Ref), Second = get_object_field(SecondField, Data, Ref), {Tag, {RecordName, First, Second}}. @@ -828,9 +728,7 @@ get_unique_numerical_id(Worker, Type) -> end. get_unique_uuid(Worker, Type) -> - %% `uuid:uuid_to_string/2` with `binary_standard` always returns a binary - %% at runtime; assert and narrow with a binary pattern match. - <<_/binary>> = NewUUID = uuid:uuid_to_string(uuid:get_v4_urandom(), binary_standard), + NewUUID = uuid:uuid_to_string(uuid:get_v4_urandom(), binary_standard), NewID = dmt_object_id:get_uuid_object_id(Type, NewUUID), NewRefString = dmt_mapper:ref_to_string({Type, NewID}), case dmt_database:check_if_object_id_active(Worker, NewRefString) of @@ -955,9 +853,6 @@ validate_object_exists(Worker, ObjectRef, Version) -> {error, not_found} -> {error, object_not_found} end. --spec get_multiple_related_graph(dmsl_domain_conf_v2_thrift:'MultipleRelatedGraphRequest'()) -> - {ok, dmsl_domain_conf_v2_thrift:'RelatedGraph'()} - | {error, object_not_found | version_not_found | term()}. get_multiple_related_graph(Request) -> #domain_conf_v2_MultipleRelatedGraphRequest{ refs = ObjectRefs, @@ -1015,9 +910,6 @@ get_multiple_related_graph(Request) -> {error, Reason} end. --spec search_related_graph(dmsl_domain_conf_v2_thrift:'SearchRelatedGraphRequest'()) -> - {ok, dmsl_domain_conf_v2_thrift:'RelatedGraph'()} - | {error, object_type_not_found | version_not_found | term()}. search_related_graph(Request) -> #domain_conf_v2_SearchRelatedGraphRequest{ query = Query, diff --git a/apps/dmt/src/dmt_repository_client_handler.erl b/apps/dmt/src/dmt_repository_client_handler.erl index ba51795..e5f4ece 100644 --- a/apps/dmt/src/dmt_repository_client_handler.erl +++ b/apps/dmt/src/dmt_repository_client_handler.erl @@ -2,33 +2,18 @@ -include_lib("damsel/include/dmsl_domain_conf_v2_thrift.hrl"). --behaviour(woody_server_thrift_handler). - -define(EPGPOOL, default_pool). -export([handle_function/4]). --type options() :: dmt_api_woody_utils:handler_options(). - --export_type([options/0]). - --spec handle_function(woody:func(), woody:args(), woody_context:ctx(), options()) -> - {ok, woody:result()} | no_return(). handle_function(Function, Args, WoodyContext0, Options) -> DefaultDeadline = woody_deadline:from_timeout(default_handling_timeout(Options)), WoodyContext = dmt_api_woody_utils:ensure_woody_deadline_set(WoodyContext0, DefaultDeadline), - %% Cast: `woody:args()` is `tuple() | any()`; each `do_handle_function/4` - %% clause pattern-matches a specific arg tuple guaranteed by the thrift - %% schema. The shape is enforced by woody at deserialisation, not by - %% the type system, so we cross the trust boundary explicitly here. - do_handle_function(Function, eqwalizer:dynamic_cast(Args), WoodyContext, Options). + do_handle_function(Function, Args, WoodyContext, Options). --spec default_handling_timeout(options()) -> timeout(). default_handling_timeout(#{default_handling_timeout := Timeout}) -> Timeout. --spec do_handle_function(woody:func(), eqwalizer:dynamic(tuple()), woody_context:ctx(), options()) -> - {ok, woody:result()} | no_return(). do_handle_function('CheckoutObject', {VersionRef, ObjectRef}, _Context, _Options) -> %% Fetch the object based on VersionReference and Reference case dmt_repository:get_object(?EPGPOOL, VersionRef, ObjectRef) of diff --git a/apps/dmt/src/dmt_sup.erl b/apps/dmt/src/dmt_sup.erl index ee4bfee..1d1b6e3 100644 --- a/apps/dmt/src/dmt_sup.erl +++ b/apps/dmt/src/dmt_sup.erl @@ -17,7 +17,6 @@ -define(APP, dmt). -define(DEFAULT_DB, default_db). --spec start_link() -> supervisor:startlink_ret(). start_link() -> supervisor:start_link({local, ?APP}, ?MODULE, []). @@ -30,7 +29,6 @@ start_link() -> %% shutdown => shutdown(), % optional %% type => worker(), % optional %% modules => modules()} % optional --spec init(term()) -> {ok, {supervisor:sup_flags(), [supervisor:child_spec()]}}. init(_) -> ok = dbinit(), ok = setup_kafka(dmt_kafka_publisher:is_kafka_enabled()), @@ -62,7 +60,6 @@ init(_) -> {ok, {SupFlags, ChildSpecs}}. --spec dbinit() -> ok | no_return(). dbinit() -> WorkDir = get_env_var("WORK_DIR"), _ = set_database_url(), @@ -78,28 +75,17 @@ dbinit() -> throw({migrations_error, Reason}) end. --type db_conn_opts() :: #{ - host := list(), - port := non_neg_integer(), - username := list(), - password := list(), - database := list() -}. - --spec set_database_url() -> true. set_database_url() -> EpgDbName = application_get_env(?APP, epg_db_name, ?DEFAULT_DB), - Databases = application_get_env(epg_connector, databases), - set_database_url_(get_db_opts(EpgDbName, Databases)). - --spec set_database_url_(db_conn_opts()) -> true. -set_database_url_(#{ - host := PgHost, - port := PgPort, - username := PgUser, - password := PgPassword, - database := DbName -}) -> + #{ + EpgDbName := #{ + host := PgHost, + port := PgPort, + username := PgUser, + password := PgPassword, + database := DbName + } + } = application_get_env(epg_connector, databases), %% DATABASE_URL=postgresql://postgres:postgres@db/dmtv2 PgPortStr = erlang:integer_to_list(PgPort), Value = @@ -107,45 +93,14 @@ set_database_url_(#{ DbName, true = os:putenv("DATABASE_URL", Value). --spec get_db_opts(term(), term()) -> db_conn_opts(). -get_db_opts(EpgDbName, Databases) when is_map(Databases) -> - case maps:get(EpgDbName, Databases, undefined) of - #{ - host := Host, - port := Port, - username := User, - password := Pass, - database := DbName - } when - is_list(Host), - is_integer(Port), - is_list(User), - is_list(Pass), - is_list(DbName) - -> - #{ - host => Host, - port => Port, - username => User, - password => Pass, - database => DbName - }; - Other -> - erlang:error({bad_db_config, EpgDbName, Other}) - end; -get_db_opts(_EpgDbName, Other) -> - erlang:error({bad_epg_connector_databases, Other}). - %% internal functions --spec get_env_var(string()) -> string() | no_return(). get_env_var(Name) -> case os:getenv(Name) of false -> throw({os_env_required, Name}); V -> V end. --spec get_repository_handlers() -> [woody:http_handler(woody:th_handler())]. get_repository_handlers() -> DefaultTimeout = application_get_env(?APP, default_woody_handling_timeout, timer:seconds(30)), [ @@ -181,7 +136,6 @@ get_handler(author, Options) -> {dmt_author_handler, Options} }}. --spec get_service(repository | repository_client | author) -> woody:service(). get_service(repository) -> {dmsl_domain_conf_v2_thrift, 'Repository'}; get_service(repository_client) -> @@ -189,7 +143,7 @@ get_service(repository_client) -> get_service(author) -> {dmsl_domain_conf_v2_thrift, 'AuthorManagement'}. --spec enable_health_logging(map()) -> map(). +-spec enable_health_logging(erl_health:check()) -> erl_health:check(). enable_health_logging(Check) -> EvHandler = {erl_health_event_handler, []}, maps:map(fun(_, {_, _, _} = V) -> #{runner => V, event_handler => EvHandler} end, Check). @@ -199,7 +153,6 @@ get_prometheus_route() -> {"/metrics/[:registry]", prometheus_cowboy2_handler, []}. %% @doc Setup damsel version information from multiple sources --spec setup_damsel_version() -> ok. setup_damsel_version() -> DamselVersionInfo = get_damsel_version(), logger:warning("Damsel version info: ~p", [DamselVersionInfo]), @@ -257,48 +210,29 @@ get_damsel_git_ref_from_lock() -> extract_damsel_ref_from_lock_data({_LockVersion, Deps}) when is_list(Deps) -> case lists:keyfind(<<"damsel">>, 1, Deps) of {<<"damsel">>, {git, _Url, {ref, Ref}}, _Level} -> - ensure_string(Ref); + {ok, Ref}; _ -> error end; extract_damsel_ref_from_lock_data(_) -> error. --spec ensure_string(term()) -> {ok, string()} | error. -ensure_string(S) when is_list(S) -> - case io_lib:printable_list(S) of - %% Cast: `is_list/1` narrows `term()` to `[term()]`, and - %% `io_lib:printable_list/1` is a runtime predicate eqwalizer can't use - %% to refine the element type further. We've just confirmed it's a - %% printable string at runtime so cast to `string() = [char()]`. - true -> {ok, eqwalizer:dynamic_cast(S)}; - false -> error - end; -ensure_string(_) -> - error. - -%% @doc Read an `application:get_env/2` value. The runtime contract is dynamic -%% because sys.config is opaque to the type system, so callers narrow with -%% guards before use. --spec application_get_env(atom(), atom()) -> eqwalizer:dynamic(). application_get_env(App, Key) -> application_get_env(App, Key, undefined). --spec application_get_env(atom(), atom(), Default) -> eqwalizer:dynamic() | Default. application_get_env(App, Key, Default) -> case application:get_env(App, Key) of {ok, Value} -> Value; undefined -> Default end. --spec setup_kafka(boolean()) -> ok | no_return(). setup_kafka(false) -> ok; setup_kafka(_) -> ClientName = dmt_kafka_client, - Clients = ensure_list(application_get_env(brod, clients, [])), - Client = ensure_list(proplists:get_value(ClientName, Clients)), - Endpoints = ensure_list(proplists:get_value(endpoints, Client)), + Clients = application_get_env(brod, clients, []), + Client = proplists:get_value(ClientName, Clients), + Endpoints = proplists:get_value(endpoints, Client), ClientConfig = proplists:delete(endpoints, Client), _ = logger:info("Starting Kafka client ~p with endpoints ~p and config ~p", [ @@ -314,8 +248,3 @@ setup_kafka(_) -> logger:error("Failed to start Kafka client ~p: ~p", [ClientName, Reason]), throw({kafka_client_start_error, Reason}) end. - -%% @doc Narrow a `term()` to a list (defaulting to []). --spec ensure_list(term()) -> list(). -ensure_list(L) when is_list(L) -> L; -ensure_list(_) -> []. diff --git a/apps/dmt/src/dmt_thrift.erl b/apps/dmt/src/dmt_thrift.erl index 7e04bec..68eb2ca 100644 --- a/apps/dmt/src/dmt_thrift.erl +++ b/apps/dmt/src/dmt_thrift.erl @@ -6,25 +6,17 @@ -export_type([thrift_type/0]). -export_type([function_schema/0]). -export_type([thrift_value/0]). --export_type([struct_info/0]). --export_type([field_info/0]). --export_type([struct_flavour/0]). --export_type([type_ref/0]). -type thrift_value() :: term(). -%% A thrift field type. Recursive: structs may carry their own inline schema -%% or a reference to a struct by name. -type thrift_type() :: base_type() | collection_type() | enum_type() - | struct_type() - | struct_info(). + | struct_type(). -type base_type() :: bool - | byte | double | i8 | i16 @@ -41,17 +33,12 @@ {enum, type_ref()}. -type struct_type() :: - {struct, struct_flavour(), type_ref()}. + {struct, struct_flavor(), type_ref()}. --type struct_flavour() :: struct | union | exception. +-type struct_flavor() :: struct | union | exception. -type type_ref() :: {module(), Name :: atom()}. -%% Inline struct schema as produced by `:struct_info/1`. --type struct_info() :: {struct, struct_flavour(), [field_info()]}. - --type field_info() :: {pos_integer(), atom(), thrift_type(), atom(), term()}. - -type function_schema() :: tuple(). -spec encode(binary, thrift_type(), thrift_value()) -> binary(). diff --git a/apps/dmt/src/dmt_thrift_validator.erl b/apps/dmt/src/dmt_thrift_validator.erl index ad157cd..2025368 100644 --- a/apps/dmt/src/dmt_thrift_validator.erl +++ b/apps/dmt/src/dmt_thrift_validator.erl @@ -8,15 +8,16 @@ -include_lib("damsel/include/dmsl_domain_thrift.hrl"). -%% @doc Validate a domain object using thrift strict validation. Takes any term -%% (validation is the whole point) and either confirms shape or reports errors. --spec validate_domain_object(term()) -> ok | {error, {invalid, [atom()], term()}}. +%% @doc Validate a domain object using thrift strict validation +-spec validate_domain_object(dmsl_domain_thrift:'DomainObject'()) -> + ok | {error, {invalid, [atom()], term()}}. validate_domain_object(DomainObject) -> Type = dmsl_domain_thrift:struct_info('DomainObject'), thrift_strict_binary_codec:validate({Type, DomainObject}). -%% @doc Validate a reference. Takes any term — validation is the whole point. --spec validate_reference(term()) -> ok | {error, {invalid, [atom()], term()}}. +%% @doc Validate a reference +-spec validate_reference(dmsl_domain_thrift:'Reference'()) -> + ok | {error, {invalid, [atom()], term()}}. validate_reference(PayoutMethodRef) -> Type = dmsl_domain_thrift:struct_info('Reference'), thrift_strict_binary_codec:validate({Type, PayoutMethodRef}). diff --git a/apps/dmt/test/dmt_client_api.erl b/apps/dmt/test/dmt_client_api.erl index b02437f..be03413 100644 --- a/apps/dmt/test/dmt_client_api.erl +++ b/apps/dmt/test/dmt_client_api.erl @@ -15,10 +15,7 @@ new(Context) -> -spec call(Name :: atom(), woody:func(), [any()], t()) -> {ok, _Response} | {exception, _} | {error, _}. call(ServiceName, Function, Args, Context) -> - %% Cast: `dmt_sup:get_service/1` accepts a specific atom union - %% (`repository | repository_client | author`); test callers pass a - %% generic `atom()` and the runtime contract holds. - Service = dmt_sup:get_service(eqwalizer:dynamic_cast(ServiceName)), + Service = dmt_sup:get_service(ServiceName), Request = {Service, Function, list_to_tuple(Args)}, Opts = get_opts(ServiceName), try @@ -33,10 +30,7 @@ get_opts(ServiceName) -> Opts0 = #{ event_handler => {scoper_woody_event_handler, EventHandlerOpts} }, - %% Cast: `genlib_app:env/3` returns `term()` because sys.config is opaque - %% to the type system; `maps:get/3` expects a map argument so we assert - %% the runtime shape here. - case maps:get(ServiceName, eqwalizer:dynamic_cast(genlib_app:env(dmt, services, #{})), undefined) of + case maps:get(ServiceName, genlib_app:env(dmt, services), undefined) of #{} = Opts -> maps:merge(Opts, Opts0); _ -> diff --git a/apps/dmt/test/dmt_repository_filter_test.erl b/apps/dmt/test/dmt_repository_filter_test.erl index 23bd024..b2c84de 100644 --- a/apps/dmt/test/dmt_repository_filter_test.erl +++ b/apps/dmt/test/dmt_repository_filter_test.erl @@ -5,8 +5,6 @@ % We modify records in improper way in tests, so we need to suppress dialyzer warnings -dialyzer({nowarn_function, [test_all_invalid_objects/0, test_mixed_valid_invalid_objects/0]}). --eqwalizer({nowarn_function, test_all_invalid_objects/0}). --eqwalizer({nowarn_function, test_mixed_valid_invalid_objects/0}). %% Test the filter_search_results/1 function from dmt_repository module diff --git a/apps/dmt_core/src/dmt_domain.erl b/apps/dmt_core/src/dmt_domain.erl index 6912775..98ea6de 100644 --- a/apps/dmt_core/src/dmt_domain.erl +++ b/apps/dmt_core/src/dmt_domain.erl @@ -14,8 +14,6 @@ -export_type([operation_error/0]). -export_type([domain_object/0]). --export_type([thrift_type/0]). --export_type([object_reference/0]). %% @@ -36,13 +34,6 @@ {conflict, operation_conflict()} | {invalid, operation_invalid()}. --type thrift_type() :: dmt_thrift:thrift_type(). --type struct_info() :: dmt_thrift:struct_info(). --type field_info() :: dmt_thrift:field_info(). - --type object_reference() :: {atom(), term()}. - --spec references(domain_object()) -> [object_reference()]. references(DomainObject) -> case get_data(DomainObject) of {error, _} -> @@ -51,12 +42,9 @@ references(DomainObject) -> references(Data, DataType) end. --spec references(eqwalizer:dynamic(), thrift_type()) -> [object_reference()]. references(Object, DataType) -> references(Object, DataType, []). --spec references(eqwalizer:dynamic(), thrift_type(), [object_reference()]) -> - [object_reference()]. references(undefined, _StructInfo, Refs) -> Refs; references({Tag, Object}, {struct, union, FieldsInfo} = StructInfo, Refs) when @@ -114,14 +102,11 @@ references(Object, {map, KeyType, ValueType}, Refs) -> references(_DomainObject, _Primitive, Refs) -> Refs. --spec indexfold(fun((pos_integer(), Elem, Acc) -> Acc), Acc, pos_integer(), [Elem]) -> Acc. indexfold(Fun, Acc, I, [E | Rest]) -> indexfold(Fun, Fun(I, E, Acc), I + 1, Rest); indexfold(_Fun, Acc, _I, []) -> Acc. --spec check_reference_type(eqwalizer:dynamic(), thrift_type(), [object_reference()]) -> - [object_reference()]. check_reference_type(Object, Type, Refs) -> case is_reference_type(Type) of {true, Tag} -> @@ -130,13 +115,10 @@ check_reference_type(Object, Type, Refs) -> references(Object, Type, Refs) end. --spec get_data(domain_object()) -> - {thrift_type(), eqwalizer:dynamic()} | {error, {unknown_domain_object_tag, atom()}}. +-spec get_data(domain_object()) -> any(). get_data(DomainObject) -> get_domain_object_field(data, DomainObject). --spec get_domain_object_field(atom(), domain_object()) -> - {thrift_type(), eqwalizer:dynamic()} | {error, {unknown_domain_object_tag, atom()}}. get_domain_object_field(Field, {Tag, Struct}) -> case get_domain_object_schema(Tag) of {error, _} = Error -> @@ -145,10 +127,8 @@ get_domain_object_field(Field, {Tag, Struct}) -> get_field(Field, Struct, Schema) end. --spec maybe_get_domain_object_data_field(atom(), domain_object()) -> - eqwalizer:dynamic() | undefined. -maybe_get_domain_object_data_field(Field, {Tag, _Struct} = DomainObject) -> - try get_data(DomainObject) of +maybe_get_domain_object_data_field(Field, {Tag, Struct}) -> + try get_data({Tag, Struct}) of {error, _} -> undefined; {_, Data} -> @@ -156,13 +136,11 @@ maybe_get_domain_object_data_field(Field, {Tag, _Struct} = DomainObject) -> catch Error:Reason:Stacktrace -> logger:warning("Error getting data field ~p for ~p: ~p", [ - Field, DomainObject, {Error, Reason, Stacktrace} + Field, {Tag, Struct}, {Error, Reason, Stacktrace} ]), undefined end. --spec maybe_extract_field_from_data(atom(), atom(), eqwalizer:dynamic()) -> - eqwalizer:dynamic() | undefined. maybe_extract_field_from_data(Field, Tag, Data) -> SchemaInfo = get_struct_info('ReflessDomainObject'), case get_field_info(Tag, SchemaInfo) of @@ -172,7 +150,6 @@ maybe_extract_field_from_data(Field, Tag, Data) -> undefined end. --spec maybe_get_field_by_index(atom(), atom(), tuple()) -> eqwalizer:dynamic() | undefined. maybe_get_field_by_index(Field, ObjectStructName, Data) -> DomainObjectSchema = get_struct_info(ObjectStructName), case get_field_index(Field, DomainObjectSchema) of @@ -183,55 +160,32 @@ maybe_get_field_by_index(Field, ObjectStructName, Data) -> end. % limit_config is an exception, it's not in domain.thrift --spec get_domain_object_schema(atom()) -> - struct_info() | {error, {unknown_domain_object_tag, atom()}}. get_domain_object_schema(limit_config) -> - %% Cast: `dmsl_limiter_config_thrift` exports its own nominal `struct_info()` - %% type. Our local `struct_info()` is the structurally identical alias — - %% eqwalizer treats the two as distinct nominal types so we need to cross - %% the cross-thrift-module boundary explicitly here. - eqwalizer:dynamic_cast(dmsl_limiter_config_thrift:struct_info('LimitConfig')); + dmsl_limiter_config_thrift:struct_info('LimitConfig'); get_domain_object_schema(Tag) -> SchemaInfo = get_struct_info('DomainObject'), case get_field_info(Tag, SchemaInfo) of - {_, _, {struct, _, {_, ObjectStructName}}, _, _} when is_atom(ObjectStructName) -> + {_, _, {struct, _, {_, ObjectStructName}}, _, _} -> get_struct_info(ObjectStructName); - _ -> + false -> {error, {unknown_domain_object_tag, Tag}} end. --spec get_field(atom(), tuple(), struct_info()) -> {thrift_type(), eqwalizer:dynamic()}. get_field(Field, Struct, StructInfo) when is_atom(Field) -> {FieldIndex, {_, _, Type, _, _}} = get_field_index(Field, StructInfo), {Type, element(FieldIndex, Struct)}. --spec get_struct_info(atom()) -> struct_info(). get_struct_info(StructName) -> - %% Outer cast: `dmsl_domain_thrift:struct_info/1` returns its nominal - %% `struct_info()` type which is structurally identical to but nominally - %% distinct from our local `struct_info()` alias. - %% Inner cast on `StructName`: the callee expects a specific atom union - %% (`struct_name() | exception_name()`); the value comes from a runtime - %% schema field so we only know it's an `atom()`. - eqwalizer:dynamic_cast( - dmsl_domain_thrift:struct_info(eqwalizer:dynamic_cast(StructName)) - ). + dmsl_domain_thrift:struct_info(StructName). --spec get_field_info(atom(), struct_info()) -> field_info() | false. get_field_info(Field, {struct, _StructType, FieldsInfo}) -> - case lists:keyfind(Field, 4, FieldsInfo) of - false -> false; - T -> T - end. + lists:keyfind(Field, 4, FieldsInfo). --spec get_field_index(atom(), struct_info()) -> {pos_integer(), field_info()} | false. get_field_index(Field, {struct, _StructType, FieldsInfo}) -> % NOTE % This `2` gives index of the first significant field in a record tuple. get_field_index(Field, 2, FieldsInfo). --spec get_field_index(atom(), pos_integer(), [field_info()]) -> - {pos_integer(), field_info()} | false. get_field_index(_Field, _, []) -> false; get_field_index(Field, I, [F | Rest]) -> @@ -242,16 +196,10 @@ get_field_index(Field, I, [F | Rest]) -> get_field_index(Field, I + 1, Rest) end. --spec is_reference_type(thrift_type()) -> {true, atom()} | false. is_reference_type(Type) -> - case get_struct_info('Reference') of - {struct, union, StructInfo} when is_list(StructInfo) -> - is_reference_type(Type, StructInfo); - _ -> - false - end. + {struct, union, StructInfo} = get_struct_info('Reference'), + is_reference_type(Type, StructInfo). -%% NOTE: dmt_domain_pt parse_transform removes is_reference_type/2 — no spec. is_reference_type(_Type, []) -> false; is_reference_type(Type, [{_, _, Type, Tag, _} | _Rest]) -> diff --git a/apps/dmt_core/src/dmt_domain_pt.erl b/apps/dmt_core/src/dmt_domain_pt.erl index b07d761..321da22 100644 --- a/apps/dmt_core/src/dmt_domain_pt.erl +++ b/apps/dmt_core/src/dmt_domain_pt.erl @@ -5,16 +5,12 @@ -spec parse_transform(Forms, term()) -> Forms when Forms :: [erl_parse:abstract_form() | erl_parse:form_info()]. parse_transform(Forms, _Options) -> - %% Cast: `erl_syntax:revert/1` returns `erl_syntax:syntaxTree()` but the - %% `parse_transform/2` callback contract requires - %% `[erl_parse:abstract_form() | erl_parse:form_info()]`. The two are the - %% same underlying AST representation but have distinct nominal types. - eqwalizer:dynamic_cast([ + [ erl_syntax:revert(FormNext) || Form <- Forms, FormNext <- [erl_syntax_lib:map(fun transform/1, Form)], FormNext /= delete - ]). + ]. transform(Form) -> case erl_syntax:type(Form) of diff --git a/apps/dmt_object/src/dmt_object_id.erl b/apps/dmt_object/src/dmt_object_id.erl index 8b59636..c446df8 100644 --- a/apps/dmt_object/src/dmt_object_id.erl +++ b/apps/dmt_object/src/dmt_object_id.erl @@ -6,13 +6,6 @@ -export([get_numerical_object_id/2]). -export([get_uuid_object_id/2]). --type numerical_id() :: integer(). --type uuid_id() :: binary(). --type type_tag() :: atom(). - --export_type([numerical_id/0, uuid_id/0, type_tag/0]). - --spec get_numerical_object_id(type_tag(), numerical_id()) -> tuple() | no_return(). get_numerical_object_id(category, ID) -> #domain_CategoryRef{id = ID}; get_numerical_object_id(business_schedule, ID) -> @@ -50,7 +43,6 @@ get_numerical_object_id(document_type, ID) -> get_numerical_object_id(Type, _ID) -> throw({not_supported, Type}). --spec get_uuid_object_id(type_tag(), uuid_id()) -> tuple() | no_return(). get_uuid_object_id(party_config, ID) -> #domain_PartyConfigRef{id = ID}; get_uuid_object_id(shop_config, ID) -> diff --git a/apps/dmt_object/src/dmt_object_reference.erl b/apps/dmt_object/src/dmt_object_reference.erl index fa05a80..439b94f 100644 --- a/apps/dmt_object/src/dmt_object_reference.erl +++ b/apps/dmt_object/src/dmt_object_reference.erl @@ -8,15 +8,6 @@ -define(DOMAIN, dmsl_domain_thrift). --type object_type() :: dmt_object:object_type(). --type object_reference() :: {object_type(), term()}. --type thrift_type() :: dmt_thrift:thrift_type(). --type struct_info() :: dmt_thrift:struct_info(). --type field_info() :: dmt_thrift:field_info(). - --export_type([object_reference/0]). - --spec get_domain_object_ref(tuple()) -> object_reference(). get_domain_object_ref({Tag, _Struct} = DomainObject) -> {_Type, Ref} = get_domain_object_field(ref, DomainObject), {Tag, Ref}. @@ -24,12 +15,10 @@ get_domain_object_ref({Tag, _Struct} = DomainObject) -> %% RefflessObject ZONE %% FIXME doesn't work --spec refless_object_references(tuple()) -> [object_reference()]. refless_object_references(DomainObject) -> {Data, DataType} = get_refless_data(DomainObject), references(Data, DataType). --spec get_refless_data(tuple()) -> {eqwalizer:dynamic(), thrift_type()}. get_refless_data({Tag, Struct}) -> SchemaInfo = get_struct_info('ReflessDomainObject'), case get_field_info(Tag, SchemaInfo) of @@ -41,38 +30,30 @@ get_refless_data({Tag, Struct}) -> %% DomainObject ZONE --spec domain_object_references(tuple()) -> [object_reference()]. domain_object_references(DomainObject) -> {Data, DataType} = get_domain_object_data(DomainObject), references(Data, DataType). --spec get_domain_object_data(tuple()) -> {eqwalizer:dynamic(), thrift_type()}. get_domain_object_data(DomainObject) -> get_domain_object_field(data, DomainObject). --spec get_domain_object_field(atom(), tuple()) -> {eqwalizer:dynamic(), thrift_type()}. get_domain_object_field(Field, {Tag, Struct}) -> get_field(Field, Struct, get_domain_object_schema(Tag)). --spec get_domain_object_schema(atom()) -> struct_info(). get_domain_object_schema(Tag) -> SchemaInfo = get_struct_info('DomainObject'), {_, _, {struct, _, {_, ObjectStructName}}, _, _} = get_field_info(Tag, SchemaInfo), get_struct_info(ObjectStructName). --spec get_field(atom(), tuple(), struct_info()) -> {eqwalizer:dynamic(), thrift_type()}. get_field(Field, Struct, StructInfo) when is_atom(Field) -> {FieldIndex, {_, _, Type, _, _}} = get_field_index(Field, StructInfo), {element(FieldIndex, Struct), Type}. --spec get_field_index(atom(), struct_info()) -> {pos_integer(), field_info()} | false. get_field_index(Field, {struct, _StructType, FieldsInfo}) -> % NOTE % This `2` gives index of the first significant field in a record tuple. get_field_index(Field, 2, FieldsInfo). --spec get_field_index(atom(), pos_integer(), [field_info()]) -> - {pos_integer(), field_info()} | false. get_field_index(_Field, _, []) -> false; get_field_index(Field, I, [F | Rest]) -> @@ -85,12 +66,9 @@ get_field_index(Field, I, [F | Rest]) -> %% References Gathering ZONE --spec references(eqwalizer:dynamic(), thrift_type()) -> [object_reference()]. references(Object, DataType) -> references(Object, DataType, []). --spec references(eqwalizer:dynamic(), thrift_type(), [object_reference()]) -> - [object_reference()]. references(undefined, _StructInfo, Refs) -> Refs; references({Tag, Object}, {struct, union, FieldsInfo} = StructInfo, Refs) when @@ -148,8 +126,6 @@ references(Object, {map, KeyType, ValueType}, Refs) -> references(_DomainObject, _Primitive, Refs) -> Refs. --spec check_reference_type(eqwalizer:dynamic(), thrift_type(), [object_reference()]) -> - [object_reference()]. check_reference_type(Object, Type, Refs) -> case is_reference_type(Type) of {true, Tag} -> @@ -158,16 +134,10 @@ check_reference_type(Object, Type, Refs) -> references(Object, Type, Refs) end. --spec is_reference_type(thrift_type()) -> {true, atom()} | false. is_reference_type(Type) -> - case get_struct_info('Reference') of - {struct, union, StructInfo} when is_list(StructInfo) -> - is_reference_type(Type, StructInfo); - _ -> - false - end. + {struct, union, StructInfo} = get_struct_info('Reference'), + is_reference_type(Type, StructInfo). --spec is_reference_type(thrift_type(), [field_info()]) -> {true, atom()} | false. is_reference_type(_Type, []) -> false; is_reference_type(Type, [{_, _, Type, Tag, _} | _Rest]) -> @@ -175,7 +145,6 @@ is_reference_type(Type, [{_, _, Type, Tag, _} | _Rest]) -> is_reference_type(Type, [_ | Rest]) -> is_reference_type(Type, Rest). --spec indexfold(fun((pos_integer(), Elem, Acc) -> Acc), Acc, pos_integer(), [Elem]) -> Acc. indexfold(Fun, Acc, I, [E | Rest]) -> indexfold(Fun, Fun(I, E, Acc), I + 1, Rest); indexfold(_Fun, Acc, _I, []) -> @@ -183,28 +152,13 @@ indexfold(_Fun, Acc, _I, []) -> %% Common --spec get_struct_info(atom()) -> struct_info(). get_struct_info('LimitConfig') -> - %% Cast: each `dmsl_*_thrift` module exports its own nominal `struct_info()` - %% type. Ours is the structurally identical local alias — eqwalizer treats - %% the two as distinct nominal types so the cross-module boundary needs an - %% explicit cast. - eqwalizer:dynamic_cast(dmsl_limiter_config_thrift:struct_info('LimitConfig')); + dmsl_limiter_config_thrift:struct_info('LimitConfig'); get_struct_info(StructName) -> - %% Outer cast: same nominal-vs-structural reason as the `LimitConfig` clause. - %% Inner cast on `StructName`: `dmsl_domain_thrift:struct_info/1` expects a - %% specific atom union (`struct_name() | exception_name()`); the value here - %% comes from runtime schema lookups so we only know it's an `atom()`. - eqwalizer:dynamic_cast( - dmsl_domain_thrift:struct_info(eqwalizer:dynamic_cast(StructName)) - ). - --spec get_field_info(atom(), struct_info()) -> field_info() | false. + dmsl_domain_thrift:struct_info(StructName). + get_field_info(Field, {struct, _StructType, FieldsInfo}) -> - case lists:keyfind(Field, 4, FieldsInfo) of - false -> false; - T -> T - end. + lists:keyfind(Field, 4, FieldsInfo). -ifdef(TEST). diff --git a/apps/dmt_object/src/dmt_object_type.erl b/apps/dmt_object/src/dmt_object_type.erl index afd3428..e2373e8 100644 --- a/apps/dmt_object/src/dmt_object_type.erl +++ b/apps/dmt_object/src/dmt_object_type.erl @@ -6,7 +6,6 @@ -export([get_refless_object_type/1]). -export([get_ref_type/1]). --spec get_refless_object_type(tuple()) -> atom() | no_return(). get_refless_object_type(#domain_Category{}) -> category; get_refless_object_type(#domain_Currency{}) -> @@ -14,7 +13,6 @@ get_refless_object_type(#domain_Currency{}) -> get_refless_object_type(_) -> error(not_impl). --spec get_ref_type(tuple() | undefined) -> atom() | undefined | no_return(). get_ref_type(#domain_CurrencyRef{}) -> currency; get_ref_type(#domain_CategoryRef{}) -> From e1979ab774dc83e92c894f927fcb6c92675735a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D0=B5=D0=BC?= Date: Mon, 25 May 2026 13:12:41 +0300 Subject: [PATCH 2/5] Update dependencies and enhance static analysis workflow This commit updates the Erlang version from 28.0 to 28.5 in the .tool-versions file. It also modifies the Makefile to improve the eqwalizer task by iterating over modules defined in eqwalizer.modules. Additionally, the rebar.config file now references a specific commit for eqwalizer_support instead of the main branch. The static analysis GitHub Actions workflow has been enhanced by adding a new job for running Dialyzer, which includes steps for checking out the repository, loading environment variables, building the development image, and executing the Dialyzer analysis. No functional changes were made to the application code itself. --- .elp.toml | 3 + .github/workflows/static-analysis.yml | 27 +- .tool-versions | 2 +- Makefile | 8 +- apps/dmt/src/dmt_repository_handler.erl | 30 +- apps/dmt_object/src/dmt_object.erl | 81 +++--- docs/eqwalizer-adoption-plan.md | 347 ++++++++++++++++++++++++ eqwalizer.modules | 11 + rebar.config | 3 +- 9 files changed, 429 insertions(+), 83 deletions(-) create mode 100644 .elp.toml create mode 100644 docs/eqwalizer-adoption-plan.md create mode 100644 eqwalizer.modules diff --git a/.elp.toml b/.elp.toml new file mode 100644 index 0000000..2eb81df --- /dev/null +++ b/.elp.toml @@ -0,0 +1,3 @@ +[eqwalizer] +enable_all = false +max_tasks = 4 diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index 5a1457f..f7e50eb 100644 --- a/.github/workflows/static-analysis.yml +++ b/.github/workflows/static-analysis.yml @@ -12,6 +12,24 @@ permissions: contents: read jobs: + dialyzer: + name: Dialyzer + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + submodules: recursive + + - name: Load .env + run: grep -v '^#' .env >> "$GITHUB_ENV" + + - name: Build dev image + run: make dev-image + + - name: Run dialyzer + run: make wc-dialyze + eqwalizer: name: Eqwalizer runs-on: ubuntu-latest @@ -28,11 +46,4 @@ jobs: run: make dev-image - name: Run eqwalizer - run: | - make wc-eqwalizer 2>&1 | tee /tmp/eqwalizer.log - # `elp eqwalize-all` exits 0 even when it reports errors; fail the - # job ourselves if any module produced errors. - if grep -qE "^[0-9]+ ERRORS" /tmp/eqwalizer.log; then - echo "::error::Eqwalizer reported errors" - exit 1 - fi + run: make wc-eqwalizer diff --git a/.tool-versions b/.tool-versions index 37b1884..539eb88 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,2 +1,2 @@ -erlang 28.0 +erlang 28.5 rebar 3.25.0 diff --git a/Makefile b/Makefile index 358b3ee..acc6c44 100644 --- a/Makefile +++ b/Makefile @@ -92,11 +92,13 @@ check-format: dialyze: $(REBAR) as test dialyzer +EQW_MODULES ?= $(shell grep -v '^\#' eqwalizer.modules | tr '\n' ' ') + eqwalizer: $(REBAR) compile - # ERL_LIBS lets elp's erlang_service load compiled parse_transforms - # (e.g. dmt_domain_pt) when analysing modules that use them. - ERL_LIBS=$(CURDIR)/_build/default/lib elp eqwalize-all + @for m in $(EQW_MODULES); do \ + ERL_LIBS=$(CURDIR)/_build/default/lib elp eqwalize $$m || exit 1; \ + done release: $(REBAR) as prod release diff --git a/apps/dmt/src/dmt_repository_handler.erl b/apps/dmt/src/dmt_repository_handler.erl index 9935a2f..7722e5b 100644 --- a/apps/dmt/src/dmt_repository_handler.erl +++ b/apps/dmt/src/dmt_repository_handler.erl @@ -2,28 +2,14 @@ -include_lib("damsel/include/dmsl_domain_conf_v2_thrift.hrl"). --behaviour(woody_server_thrift_handler). - %% API -export([handle_function/4]). --type options() :: dmt_api_woody_utils:handler_options(). - --export_type([options/0]). - --spec handle_function(woody:func(), woody:args(), woody_context:ctx(), options()) -> - {ok, woody:result()} | no_return(). handle_function(Function, Args, WoodyContext0, Options) -> DefaultDeadline = woody_deadline:from_timeout(default_handling_timeout(Options)), WoodyContext = dmt_api_woody_utils:ensure_woody_deadline_set(WoodyContext0, DefaultDeadline), - %% Cast: `woody:args()` is `tuple() | any()`; each `do_handle_function/4` - %% clause pattern-matches a specific arg tuple guaranteed by the thrift - %% schema. The shape is enforced by woody at deserialisation, not by - %% the type system, so we cross the trust boundary explicitly here. - do_handle_function(Function, eqwalizer:dynamic_cast(Args), WoodyContext, Options). + do_handle_function(Function, Args, WoodyContext, Options). --spec do_handle_function(woody:func(), eqwalizer:dynamic(tuple()), woody_context:ctx(), options()) -> - {ok, woody:result()} | no_return(). do_handle_function('Commit', {Version, Operations, AuthorID}, _Context, _Options) -> case dmt_repository:commit(Version, Operations, AuthorID) of {ok, NextVersion, NewObjects} -> @@ -123,13 +109,9 @@ do_handle_function('SearchRelatedGraph', {SearchRelatedGraphRequest}, _Context, woody_error:raise(system, {internal, Reason}) end. --spec default_handling_timeout(options()) -> timeout(). default_handling_timeout(#{default_handling_timeout := Timeout}) -> Timeout. --spec handle_operation_error({conflict, term()} | {invalid, term()}) -> - dmsl_domain_conf_v2_thrift:'OperationConflict'() - | dmsl_domain_conf_v2_thrift:'OperationInvalid'(). handle_operation_error({conflict, Conflict}) -> #domain_conf_v2_OperationConflict{ conflict = handle_operation_conflict(Conflict) @@ -139,7 +121,6 @@ handle_operation_error({invalid, Invalid}) -> errors = handle_operation_invalid(Invalid) }. --spec handle_operation_conflict(term()) -> dmsl_domain_conf_v2_thrift:'Conflict'() | no_return(). handle_operation_conflict({object_already_exists, Ref}) -> {object_already_exists, #domain_conf_v2_ObjectAlreadyExistsConflict{object_ref = to_ref(Ref)}}; handle_operation_conflict({forced_id_exists, Ref}) -> @@ -155,7 +136,6 @@ handle_operation_conflict({object_needs_reference, Object}) -> object = to_refless_object(Object) }}. --spec handle_operation_invalid(term()) -> [dmsl_domain_conf_v2_thrift:'OperationError'()] | no_return(). handle_operation_invalid({objects_not_exist, Refs}) when is_list(Refs) -> [ {object_not_exists, #domain_conf_v2_NonexistantObject{ @@ -170,22 +150,14 @@ handle_operation_invalid({object_reference_cycles, Cycles}) when is_list(Cycles) || Cycle <- Cycles ]. -%% @doc Narrow a `term()` to a `Reference` or crash with a useful diagnostic. -%% Eqwalizer can't refine `{atom(), _}` to a thrift tagged-union, hence the -%% `dynamic()` return type — the guard enforces the shape at runtime. --spec to_ref(term()) -> eqwalizer:dynamic() | no_return(). to_ref({Tag, _} = Ref) when is_atom(Tag) -> Ref; to_ref(Other) -> erlang:error({bad_reference, Other}). --spec to_ref_list(term()) -> [eqwalizer:dynamic()] | no_return(). to_ref_list(L) when is_list(L) -> [to_ref(R) || R <- L]; to_ref_list(Other) -> erlang:error({bad_reference_list, Other}). --spec to_refless_object(term()) -> eqwalizer:dynamic() | no_return(). to_refless_object({Tag, _} = Obj) when is_atom(Tag) -> Obj; to_refless_object(Other) -> erlang:error({bad_refless_object, Other}). -%% @doc Restrict a list of new objects to tagged-tuple shapes. --spec filter_domain_objects([term()]) -> [eqwalizer:dynamic()]. filter_domain_objects(L) -> [Obj || {Tag, _} = Obj <- L, is_atom(Tag)]. diff --git a/apps/dmt_object/src/dmt_object.erl b/apps/dmt_object/src/dmt_object.erl index 049a89f..1db68fe 100644 --- a/apps/dmt_object/src/dmt_object.erl +++ b/apps/dmt_object/src/dmt_object.erl @@ -13,51 +13,42 @@ -export_type([insertable_object/0]). -export_type([object_changes/0]). -export_type([object/0]). --export_type([object_type/0]). --export_type([object_id/0]). --export_type([timestamp/0]). --export_type([domain_object/0]). --export_type([refless_domain_object/0]). %% Object type tag. Constructed as an atom in code (e.g. `category`, %% `provider`) but stored as text in the DB, so values read back from a row %% surface as a binary. Both shapes flow through the same APIs. -type object_type() :: atom() | binary(). --type object_id() :: term(). --type timestamp() :: binary() | string(). - --type domain_object() :: dmsl_domain_thrift:'DomainObject'(). --type refless_domain_object() :: dmsl_domain_thrift:'ReflessDomainObject'(). +-type object_id() :: string(). +-type timestamp() :: string(). -type insertable_object() :: #{ - tmp_id := binary(), type := object_type(), - forced_id := term() | undefined, - data := refless_domain_object() + tmp_id := object_id(), + forced_id := string() | undefined, + references := [{object_type(), object_id()}], + data := binary() }. -type object_changes() :: #{ - id := {object_type(), object_id()}, + id := object_id(), type := object_type(), - %% Carries a domain object value; the shape is enforced by the producer - %% (`update_object/2` / `commit_operation/2`) and not by this type. - data := term(), - referenced_by => [term()], + references => [{object_type(), object_id()}], + referenced_by => [{object_type(), object_id()}], + data => binary(), is_active => boolean() }. -type object() :: #{ - id := term(), + id := object_id(), type := object_type(), - version := number() | binary(), - data := term(), - created_at := binary() | list(), - is_active := boolean(), - atom() => term() + version := string(), + references := [{object_type(), object_id()}], + referenced_by := [{object_type(), object_id()}], + data := binary(), + created_at := timestamp(), + created_by := string() }. --spec new_object(dmsl_domain_conf_v2_thrift:'InsertOp'()) -> - {ok, insertable_object()} | {error, {type_mismatch, term(), term()}}. new_object(#domain_conf_v2_InsertOp{ object = NewObject, force_ref = ForcedRef @@ -74,25 +65,26 @@ new_object(#domain_conf_v2_InsertOp{ {error, Error} end. --spec update_object(domain_object() | term(), object_changes()) -> - {ok, object_changes()} | {error, {is_not_domain_object, term()}}. -update_object({Type, {_Record, ID, _Data}} = Object, ExistingUpdate) when is_atom(Type) -> - {ok, ExistingUpdate#{ - id => {Type, ID}, - type => Type, - data => Object - }}; -update_object(Obj, _ExistingUpdate) -> - {error, {is_not_domain_object, Obj}}. +update_object( + Object, + ExistingUpdate +) -> + maybe + {ok, Type} ?= get_object_type(Object), + {ok, ID} ?= get_object_ref(Object), + {ok, ExistingUpdate#{ + id => ID, + type => Type, + data => Object + }} + end. --spec remove_object(object_changes()) -> object_changes(). remove_object(OG) -> OG#{ referenced_by => [], is_active => false }. --spec just_object(term(), term(), term(), term(), term(), term()) -> object(). just_object( ID, Type, @@ -115,8 +107,6 @@ just_object( is_active => IsActive }. --spec get_checked_type(term() | undefined, refless_domain_object()) -> - {ok, object_type()} | {error, {type_mismatch, term(), term()}}. get_checked_type(undefined, {Type, _}) -> {ok, Type}; get_checked_type({Type, _}, {Type, _}) -> @@ -124,7 +114,16 @@ get_checked_type({Type, _}, {Type, _}) -> get_checked_type(Ref, Object) -> {error, {type_mismatch, Ref, Object}}. --spec filter_out_inactive_objects([object()]) -> [object()]. +get_object_type({Type, {_Object, _Ref, _Data}}) -> + {ok, Type}; +get_object_type(Obj) -> + {error, {is_not_domain_object, Obj}}. + +get_object_ref({Type, {_Object, ID, _Data}}) -> + {ok, {Type, ID}}; +get_object_ref(Obj) -> + {error, {is_not_domain_object, Obj}}. + filter_out_inactive_objects(Objects) -> lists:filter( fun(Obj) -> diff --git a/docs/eqwalizer-adoption-plan.md b/docs/eqwalizer-adoption-plan.md new file mode 100644 index 0000000..c5c8517 --- /dev/null +++ b/docs/eqwalizer-adoption-plan.md @@ -0,0 +1,347 @@ +# План внедрения eqWAlizer в DMT + +Документ описывает поэтапный rollout eqWAlizer с минимальным шумом в diff и максимальной +пользой от статической типизации. Dialyzer остаётся глобальным safety net; eqWAlizer — +точечным type checker'ом на выбранных модулях. + +## Цель + +Получить реальный signal от типизации (ошибки в domain-логике, некорректные return paths, +несовпадение типов на границах модулей) без типизации всего проекта «для галочки». + +## Принципы + +1. **Opt-in, не opt-out** — по умолчанию eqWAlizer выключен (`enable_all = false`). +2. **Specs только там, где eqWAlizer проверяет** — не типизируем весь проект ради CI. +3. **Один `dynamic_cast` на trust boundary** — woody/config/thrift reflection, не на каждую функцию. +4. **Reflection-модули — за dialyzer + CT**, не за eqWAlizer. +5. **Каждая фаза — отдельный mergeable PR** с зелёным CI. + +## Текущее состояние ветки `add_specs` + +| Что есть | Оценка | +|----------|--------| +| ELP в Docker, Makefile, CI workflow | Хорошо | +| `eqwalizer_support` в deps | Хорошо, но нужен pin на sha, не `main` | +| Specs в ~20 модулях (~800 строк) | Шум, мало пользы | +| `eqwalize-all` в CI | Заставляет типизировать всё | +| Нет `.elp.toml` | Default = все non-test модули | +| Фикс `object_type() :: atom() \| binary()` | Оставить — runtime bugfix | + +## Разделение ответственности + +``` +Dialyzer → весь проект, global analysis, specs не обязательны +EqWAlizer → opted-in модули, local analysis, specs обязательны в проверяемом модуле +CT / eunit → integration и regression +``` + +### Tier 1 — eqWAlizer opt-in (Phase 1) + +- `dmt_object_id` (~63 строк) +- `dmt_object` (~134 строк) +- `dmt_author` (~57 строк) +- `dmt_author_database` (~187 строк) + +### Tier 2 — eqWAlizer opt-in позже (Phase 2–3) + +- `dmt_repository` (~1102 строк) — ядро бизнес-логики +- `dmt_database` (~1317 строк) — CRUD/search, без graph-функций + +### Tier ∞ — `-eqwalizer(ignore).` (dialyzer + CT) + +| Модуль | Причина | +|--------|---------| +| `dmt_domain` | Thrift schema reflection | +| `dmt_object_reference` | Дубликат reflection | +| `dmt_domain_pt` | Parse transform | +| `dmt_json` | Generic JSON↔term bridge | +| `dmt_mapper` (string↔thrift) | Return `dynamic()` неизбежен | +| `dmt_sup` | Opaque application env | +| `dmt_db_migration` | CLI / URI parsing | +| `dmt_author_handler`, `dmt_repository_handler`, `dmt_repository_client_handler` | Woody boundary | +| `dmt_kafka_publisher` | Brod config opaque | +| `dmt_api_woody_utils` | Config opaque | + +--- + +## Фаза 0: Инфраструктура и откат шума + +**PR:** `chore/eqwalizer-infra` +**Оценка:** 0.5–1 день + +### Задачи + +1. Добавить `.elp.toml`: + + ```toml + [eqwalizer] + enable_all = false + max_tasks = 4 + ``` + +2. Pin `eqwalizer_support` на конкретный sha/tag в `rebar.config`, не `{branch, "main"}`. + +3. Синхронизировать версии: `.env` OTP 28.5 ↔ `.tool-versions` erlang 28.5. + +4. Добавить `eqwalizer.modules` — список модулей для CI (version controlled). + +5. Переписать Makefile: + + ```makefile + EQW_MODULES ?= $(shell grep -v '^\#' eqwalizer.modules | tr '\n' ' ') + + eqwalizer: + $(REBAR) compile + @for m in $(EQW_MODULES); do \ + ERL_LIBS=$(CURDIR)/_build/default/lib elp eqwalize $$m || exit 1; \ + done + ``` + +6. Обновить `.github/workflows/static-analysis.yml`: + - проверять модули из `eqwalizer.modules`, не `eqwalize-all`; + - использовать exit code `elp eqwalize` вместо `grep "^[0-9]+ ERRORS"`; + - добавить `make wc-dialyze` в тот же workflow или в `erlang-checks`. + +7. Откатить из ветки `add_specs`: + - specs в `dmt_sup`, `dmt_app`, handlers, `dmt_domain*`, `dmt_object_reference`, `dmt_json`, `dmt_db_migration`, `dmt_api_woody_utils`; + - `eqwalizer:dynamic_cast` в reflection-коде; + - размытые типы (`atom() => term()`, `commit_error() | dynamic()`). + +8. Оставить без отката: + - фикс `object_type() :: atom() | binary()` + guard в `just_object/6`; + - runtime guards в `dmt_repository_handler` (`to_ref/1`, `filter_domain_objects/1`); + - partner migration — отдельным PR, если ещё не в master. + +### Критерий готовности + +- `make wc-eqwalizer` с пустым `eqwalizer.modules` — no-op; +- dialyzer зелёный; +- CT зелёный. + +--- + +## Фаза 1: Tier 1 — быстрая победа + +**PR:** `feat/eqwalizer-tier1-object-author` +**Оценка:** 1–2 дня + +### Модули + +| Модуль | `-typing([eqwalizer]).` | Комментарий | +|--------|--------------------------|-------------| +| `dmt_object_id` | да | Pattern match по type tag | +| `dmt_object` | да | Domain types, insert/update/remove | +| `dmt_author` | да | Thin facade | +| `dmt_author_database` | да | CRUD + SQL | + +### Задачи + +1. Добавить `-typing([eqwalizer]).` в каждый модуль. +2. Написать минимальные specs на exported функции (и private, если вызываются из public API). +3. Shared types — только здесь (`-export_type` в `dmt_object`, `dmt_author`). +4. Добавить 4 модуля в `eqwalizer.modules`. +5. CI: eqwalizer job проверяет только эти модули. + +### Не делать + +- Не трогать `dmt_object_reference`. +- `dmt_object_type` — `-eqwalizer(ignore).` если попадёт в scope. + +### Критерий готовности + +- `elp eqwalize dmt_object` — 0 errors; +- diff specs < 200 строк (vs ~800 в текущей ветке). + +--- + +## Фаза 2: Tier 2a — dmt_repository + +**PR:** `feat/eqwalizer-tier2-repository` +**Оценка:** 3–5 дней + +### Слайсы (порядок включения) + +| # | Область | Ценность | Сложность | +|---|---------|----------|-----------| +| 2a | `get_object*`, `get_snapshot`, `get_version` | Высокая | Средняя | +| 2b | `search_objects*`, `filter_search_results` | Высокая | Низкая | +| 2c | `commit*`, `validate_*`, `commit_operation*` | Критическая | Высокая | +| 2d | `get_related_graph*`, `search_related_graph*` | Средняя | Высокая | + +### Стратегия + +Specs на весь модуль, но `-eqwalizer({nowarn_function, ...})` на функции ещё не готовых слайсов. +Постепенно снимать `nowarn` по мере типизации. + +### Типы — конкретные, без escape hatches + +```erlang +%% Хорошо +-type commit_error() :: + {operation_error, {conflict, term()} | {invalid, term()}} + | version_not_found + | author_not_found + | migration_in_progress + | {object_update_too_old, {object_ref(), version()}} + | {conflict, binary()}. + +%% Плохо +-type commit_error() :: ... | eqwalizer:dynamic(). +``` + +### Исправить регрессии из текущей ветки + +| Место | Проблема | Fix | +|-------|----------|-----| +| `to_domain/1` | Silent skip объектов | Crash `{bad_domain_object, Obj}` или warning + metric | +| `maybe_from_string/1` | Crash на non-integer | Явный `{error, bad_token}` или soft default | +| `marshall_to_object_info/1` | Crash если `created_by = undefined` | Fail-fast в `add_created_by_to_objects` | + +### Критерий готовности + +- Слайсы 2a + 2b в CI; +- `commit/3` типизирован (2c) — можно отдельным sub-PR; +- `dmt_integration_tests_SUITE` зелёный. + +--- + +## Фаза 3: Tier 2b — dmt_database (CRUD, без graph) + +**PR:** `feat/eqwalizer-tier2-database` +**Оценка:** 3–5 дней + +### Включить + +- `get_*` / `insert_*` / `update_*` / `search_objects` / `check_*` +- Exported types: `worker/0`, `object_map/0`, `entity_type/0`, `version/0` + +### Исключить (`-eqwalizer({nowarn_function, ...})`) + +- `get_related_graph*` / `filter_edges_by_nodes` +- `parse_graph_edges_result` + +### Исправить до типизации + +| Bug | Fix | +|-----|-----| +| `get_version/2` — `case_clause` если `CreatedBy` не binary | Catch-all → `{error, malformed_row}` | +| `filter_nodes_by_type/2` — atom vs binary | Normalize both sides | + +--- + +## Фаза 4: Boundary modules (опционально, low ROI) + +**PR:** `chore/eqwalizer-handlers-boundary` +**Оценка:** 1 день + +Handlers не типизировать полностью: + +```erlang +-module(dmt_repository_handler). +-eqwalizer(ignore). + +-spec handle_function(woody:func(), woody:args(), woody_context:ctx(), options()) -> + {ok, woody:result()} | no_return(). +handle_function(F, Args, Ctx, Opts) -> + do_handle_function(F, eqwalizer:dynamic_cast(Args), Ctx, Opts). +``` + +Runtime guards (`to_ref/1`, `filter_domain_objects/1`) оставить — они для production safety, +не для eqWAlizer. + +--- + +## CI/CD: финальная схема + +```yaml +# .github/workflows/static-analysis.yml +jobs: + dialyzer: + run: make wc-dialyze + + eqwalizer: + run: make wc-eqwalizer # читает eqwalizer.modules +``` + +### Файл `eqwalizer.modules` + +``` +# Phase 1 +dmt_object_id +dmt_object +dmt_author +dmt_author_database + +# Phase 2a (uncomment when ready) +# dmt_repository +``` + +### Gate policy + +- PR, затрагивающий модуль из списка → eqwalizer обязан быть зелёным. +- PR, добавляющий модуль в список → review specs + green eqwalizer. +- PR в ignore-модули → только dialyzer. + +--- + +## Метрики прогресса + +| Метрика | Сейчас (ветка) | Цель Phase 1 | Цель Phase 2 | +|---------|----------------|--------------|--------------| +| Модулей под eqWAlizer | ~20 | 4 | 5–6 | +| Строк specs | ~800 | ~150 | ~400 | +| `dynamic_cast` | ~56 | <5 | <15 | +| `-eqwalizer(ignore)` | 0 | ~8 | ~10 | +| CI time (eqwalizer) | all modules | ~4 modules, <30s | ~5 modules, <60s | + +Tracking: + +```bash +elp eqwalize-stats +``` + +--- + +## Roadmap PR + +``` +PR0 chore/eqwalizer-infra ← .elp.toml, Makefile, CI, revert noise, pin deps +PR1 fix/object-type-binary ← bugfix (можно до PR0) +PR2 feat/eqwalizer-tier1 ← 4 модуля +PR3 fix/repository-regressions ← to_domain, get_version, filter_nodes +PR4 feat/eqwalizer-tier2a-read ← dmt_repository read path +PR5 feat/eqwalizer-tier2a-commit ← dmt_repository commit path +PR6 feat/eqwalizer-tier2b-db ← dmt_database CRUD +PR7 migration/partner ← если ещё не в master, отдельно +``` + +--- + +## Definition of Done + +- [ ] `.elp.toml` с `enable_all = false` +- [ ] `eqwalizer.modules` — единственный source of truth для CI +- [ ] Dialyzer в CI на всём проекте +- [ ] EqWAlizer в CI только на opted-in модулях +- [ ] Tier 1 (4 модуля) — 0 errors +- [ ] `dmt_repository` read + search — 0 errors +- [ ] `dmt_repository:commit/3` — 0 errors +- [ ] Reflection-модули явно помечены `-eqwalizer(ignore).` +- [ ] Нет `eqwalizer:dynamic()` в union types «для зелёного CI» +- [ ] `eqwalizer_support` pinned на sha + +--- + +## Справка: escape hatches eqWAlizer + +| Механизм | Когда использовать | +|----------|-------------------| +| `-typing([eqwalizer]).` | Opt-in модуль | +| `-eqwalizer(ignore).` | Opt-out модуля целиком | +| `-eqwalizer({nowarn_function, F/N}).` | Функция ещё не готова / тест с намеренно broken types | +| `eqwalizer:dynamic_cast/1` | Осознанная trust boundary (woody args, application env) | +| `eqwalizer:fix_me/1` | Tech debt — надо починить позже | +| `eqwalizer:dynamic(T)` | Return type bridge (лучше минимизировать) | + +Документация: [ELP eqwalizer config](https://whatsapp.github.io/erlang-language-platform/docs/get-started/configure-project/elp-toml/), [eqWAlizer FAQ](https://github.com/WhatsApp/eqwalizer/blob/main/FAQ.md). diff --git a/eqwalizer.modules b/eqwalizer.modules new file mode 100644 index 0000000..3730921 --- /dev/null +++ b/eqwalizer.modules @@ -0,0 +1,11 @@ +# Modules checked by `make eqwalizer` / CI eqwalizer job. +# One module name per line; lines starting with # are ignored. +# +# Phase 1 (uncomment when ready): +# dmt_object_id +# dmt_object +# dmt_author +# dmt_author_database +# +# Phase 2a (uncomment when ready): +# dmt_repository diff --git a/rebar.config b/rebar.config index 47b27e9..ca06b96 100644 --- a/rebar.config +++ b/rebar.config @@ -67,7 +67,8 @@ {jsx, "3.1.0"}, {eqwalizer_support, - {git_subdir, "https://github.com/whatsapp/eqwalizer.git", {branch, "main"}, "eqwalizer_support"}} + {git_subdir, "https://github.com/whatsapp/eqwalizer.git", + {ref, "c4d1098174cec06bd124855f3a28dfd6eda0a581"}, "eqwalizer_support"}} ]}. %% XRef checks From 5fc7b5b5ef7a5b8d9e5470ff77929f6487e6218f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D0=B5=D0=BC?= Date: Mon, 25 May 2026 13:31:02 +0300 Subject: [PATCH 3/5] Enhance DMT modules with type specifications and enable eqwalizer checks This commit adds type specifications to the `dmt_author`, `dmt_author_database`, `dmt_object`, and `dmt_object_id` modules, improving static analysis capabilities. The `eqwalizer.modules` file is updated to include the DMT modules for phase 1 checks. These changes aim to enhance code clarity and maintainability without altering existing functionality. --- apps/dmt/src/dmt_author.erl | 32 ++++------ apps/dmt/src/dmt_author_database.erl | 30 +++++++++- apps/dmt_object/src/dmt_object.erl | 86 ++++++++++++++------------- apps/dmt_object/src/dmt_object_id.erl | 7 +++ eqwalizer.modules | 14 ++--- 5 files changed, 99 insertions(+), 70 deletions(-) diff --git a/apps/dmt/src/dmt_author.erl b/apps/dmt/src/dmt_author.erl index 68e0643..15b3238 100644 --- a/apps/dmt/src/dmt_author.erl +++ b/apps/dmt/src/dmt_author.erl @@ -1,6 +1,7 @@ -module(dmt_author). +-typing([eqwalizer]). -%% Public API +-include_lib("damsel/include/dmsl_domain_conf_v2_thrift.hrl"). -define(POOL_NAME, author_pool). @@ -11,33 +12,26 @@ delete/1 ]). -%% Optional: Extended API (can be uncommented if these functions should be exposed) -%% -export([ -%% list/2, -%% search/2 -%% ]). +-type author_id() :: binary(). +-type name() :: binary(). +-type email() :: binary(). +-type author() :: dmsl_domain_conf_v2_thrift:'Author'(). +-export_type([author_id/0, name/0, email/0, author/0]). + +-spec insert(name(), email()) -> + {ok, author_id()} | {ok, {already_exists, author_id()}} | {error, unknown}. insert(Name, Email) -> dmt_author_database:insert(?POOL_NAME, Name, Email). +-spec get(author_id()) -> {ok, author()} | {error, author_not_found | term()}. get(AuthorID) -> dmt_author_database:get(?POOL_NAME, AuthorID). +-spec get_by_email(email()) -> {ok, author()} | {error, author_not_found | term()}. get_by_email(Email) -> dmt_author_database:get_by_email(?POOL_NAME, Email). +-spec delete(author_id()) -> ok | {error, author_not_found | term()}. delete(AuthorID) -> dmt_author_database:delete(?POOL_NAME, AuthorID). - -%% Optional: Extended functionality that could be exposed if needed -%% @doc Lists authors with pagination -%% -spec list(Limit :: pos_integer(), Offset :: non_neg_integer()) -> -%% {ok, [#domain_conf_v2_Author{}]} | {error, term()}. -%% list(Limit, Offset) -> -%% dmt_author_database:list(?POOL_NAME, Limit, Offset). - -%% @doc Searches for authors by name or email -%% -spec search(SearchTerm :: binary(), Limit :: pos_integer()) -> -%% {ok, [#domain_conf_v2_Author{}]} | {error, term()}. -%% search(SearchTerm, Limit) -> -%% dmt_author_database:search(?POOL_NAME, SearchTerm, Limit). diff --git a/apps/dmt/src/dmt_author_database.erl b/apps/dmt/src/dmt_author_database.erl index 081e4a3..5c61c92 100644 --- a/apps/dmt/src/dmt_author_database.erl +++ b/apps/dmt/src/dmt_author_database.erl @@ -1,4 +1,5 @@ -module(dmt_author_database). +-typing([eqwalizer]). -include_lib("damsel/include/dmsl_domain_conf_v2_thrift.hrl"). -include_lib("epgsql/include/epgsql.hrl"). @@ -13,6 +14,14 @@ search/3 ]). +-type worker() :: atom(). +-type author_id() :: dmt_author:author_id(). +-type name() :: dmt_author:name(). +-type email() :: dmt_author:email(). +-type author() :: dmt_author:author(). + +-spec insert(worker(), name(), email()) -> + {ok, author_id()} | {ok, {already_exists, author_id()}} | {error, unknown}. insert(Worker, Name, Email) -> Sql = """ INSERT INTO author (name, email) @@ -32,6 +41,7 @@ insert(Worker, Name, Email) -> {error, unknown} end. +-spec get(worker(), author_id()) -> {ok, author()} | {error, author_not_found | term()}. get(Worker, AuthorID) -> case is_uuid(AuthorID) of true -> @@ -40,6 +50,7 @@ get(Worker, AuthorID) -> {error, author_not_found} end. +-spec get_(worker(), author_id()) -> {ok, author()} | {error, author_not_found | term()}. get_(Worker, AuthorID) -> Sql = """ SELECT id, name, email @@ -60,6 +71,7 @@ get_(Worker, AuthorID) -> {error, Reason} end. +-spec get_by_email(worker(), email()) -> {ok, author()} | {error, author_not_found | term()}. get_by_email(Worker, Email) -> Sql = """ SELECT id, name, email @@ -80,6 +92,7 @@ get_by_email(Worker, Email) -> {error, Reason} end. +-spec delete(worker(), author_id()) -> ok | {error, author_not_found | term()}. delete(Worker, AuthorID) -> case is_uuid(AuthorID) of true -> @@ -88,6 +101,7 @@ delete(Worker, AuthorID) -> {error, author_not_found} end. +-spec delete_(worker(), author_id()) -> ok | {error, author_not_found | term()}. delete_(Worker, AuthorID) -> Sql = """ DELETE FROM author @@ -103,6 +117,8 @@ delete_(Worker, AuthorID) -> {error, Reason} end. +-spec list(worker(), pos_integer(), non_neg_integer()) -> + {ok, [author()]} | {error, term()}. list(Worker, Limit, Offset) -> Sql = """ SELECT id, name, email @@ -126,6 +142,7 @@ list(Worker, Limit, Offset) -> {error, Reason} end. +-spec search(worker(), binary(), pos_integer()) -> {ok, [author()]} | {error, term()}. search(Worker, SearchTerm, Limit) -> Sql = """ SELECT id, name, email FROM author @@ -152,9 +169,18 @@ search(Worker, SearchTerm, Limit) -> %% Internal functions -is_uuid(UUID) -> +-spec is_uuid(term()) -> boolean(). +is_uuid(<>) -> + try_string_to_uuid(UUID); +is_uuid(<>) -> + try_string_to_uuid(UUID); +is_uuid(_) -> + false. + +-spec try_string_to_uuid(uuid:uuid_string()) -> boolean(). +try_string_to_uuid(UUID) -> try uuid:string_to_uuid(UUID) of - _UUID -> + _ -> true catch exit:badarg -> diff --git a/apps/dmt_object/src/dmt_object.erl b/apps/dmt_object/src/dmt_object.erl index 1db68fe..99deda0 100644 --- a/apps/dmt_object/src/dmt_object.erl +++ b/apps/dmt_object/src/dmt_object.erl @@ -1,6 +1,5 @@ -module(dmt_object). - --feature(maybe_expr, enable). +-typing([eqwalizer]). -include_lib("damsel/include/dmsl_domain_conf_v2_thrift.hrl"). @@ -13,42 +12,46 @@ -export_type([insertable_object/0]). -export_type([object_changes/0]). -export_type([object/0]). +-export_type([object_type/0]). %% Object type tag. Constructed as an atom in code (e.g. `category`, %% `provider`) but stored as text in the DB, so values read back from a row %% surface as a binary. Both shapes flow through the same APIs. -type object_type() :: atom() | binary(). --type object_id() :: string(). --type timestamp() :: string(). +-type object_ref() :: {object_type(), term()}. +-type refless_domain_object() :: dmsl_domain_thrift:'ReflessDomainObject'(). +-type domain_object() :: dmsl_domain_thrift:'DomainObject'(). -type insertable_object() :: #{ type := object_type(), - tmp_id := object_id(), - forced_id := string() | undefined, - references := [{object_type(), object_id()}], - data := binary() + tmp_id := binary(), + forced_id := term() | undefined, + data := refless_domain_object() }. -type object_changes() :: #{ - id := object_id(), + id := object_ref(), type := object_type(), - references => [{object_type(), object_id()}], - referenced_by => [{object_type(), object_id()}], - data => binary(), + references => [object_ref()], + referenced_by => [object_ref()], + data => domain_object(), is_active => boolean() }. -type object() :: #{ - id := object_id(), + id := term(), type := object_type(), - version := string(), - references := [{object_type(), object_id()}], - referenced_by := [{object_type(), object_id()}], - data := binary(), - created_at := timestamp(), - created_by := string() + version := number() | binary(), + data := term(), + created_at := binary() | list(), + is_active := boolean(), + references => [object_ref()], + referenced_by => [object_ref()], + created_by => binary() }. +-spec new_object(dmsl_domain_conf_v2_thrift:'InsertOp'()) -> + {ok, insertable_object()} | {error, {type_mismatch, term(), term()}}. new_object(#domain_conf_v2_InsertOp{ object = NewObject, force_ref = ForcedRef @@ -65,26 +68,32 @@ new_object(#domain_conf_v2_InsertOp{ {error, Error} end. -update_object( - Object, - ExistingUpdate -) -> - maybe - {ok, Type} ?= get_object_type(Object), - {ok, ID} ?= get_object_ref(Object), - {ok, ExistingUpdate#{ - id => ID, - type => Type, - data => Object - }} - end. +-spec update_object(domain_object(), object_changes()) -> + {ok, object_changes()} | {error, {is_not_domain_object, term()}}. +update_object({Type, {_Object, ID, _Data}} = Object, ExistingUpdate) -> + {ok, ExistingUpdate#{ + id => {Type, ID}, + type => Type, + data => Object + }}; +update_object(Obj, _ExistingUpdate) -> + {error, {is_not_domain_object, Obj}}. +-spec remove_object(object_changes()) -> object_changes(). remove_object(OG) -> OG#{ referenced_by => [], is_active => false }. +-spec just_object( + term(), + object_type(), + number() | binary(), + term(), + binary() | list(), + boolean() +) -> object(). just_object( ID, Type, @@ -107,6 +116,8 @@ just_object( is_active => IsActive }. +-spec get_checked_type(term() | undefined, refless_domain_object()) -> + {ok, object_type()} | {error, {type_mismatch, term(), term()}}. get_checked_type(undefined, {Type, _}) -> {ok, Type}; get_checked_type({Type, _}, {Type, _}) -> @@ -114,16 +125,7 @@ get_checked_type({Type, _}, {Type, _}) -> get_checked_type(Ref, Object) -> {error, {type_mismatch, Ref, Object}}. -get_object_type({Type, {_Object, _Ref, _Data}}) -> - {ok, Type}; -get_object_type(Obj) -> - {error, {is_not_domain_object, Obj}}. - -get_object_ref({Type, {_Object, ID, _Data}}) -> - {ok, {Type, ID}}; -get_object_ref(Obj) -> - {error, {is_not_domain_object, Obj}}. - +-spec filter_out_inactive_objects([object()]) -> [object()]. filter_out_inactive_objects(Objects) -> lists:filter( fun(Obj) -> diff --git a/apps/dmt_object/src/dmt_object_id.erl b/apps/dmt_object/src/dmt_object_id.erl index c446df8..66d1bfe 100644 --- a/apps/dmt_object/src/dmt_object_id.erl +++ b/apps/dmt_object/src/dmt_object_id.erl @@ -1,4 +1,5 @@ -module(dmt_object_id). +-typing([eqwalizer]). -include_lib("damsel/include/dmsl_domain_thrift.hrl"). @@ -6,6 +7,11 @@ -export([get_numerical_object_id/2]). -export([get_uuid_object_id/2]). +-type numerical_id() :: integer(). +-type uuid_id() :: binary(). +-type type_tag() :: atom(). + +-spec get_numerical_object_id(type_tag(), numerical_id()) -> tuple() | no_return(). get_numerical_object_id(category, ID) -> #domain_CategoryRef{id = ID}; get_numerical_object_id(business_schedule, ID) -> @@ -43,6 +49,7 @@ get_numerical_object_id(document_type, ID) -> get_numerical_object_id(Type, _ID) -> throw({not_supported, Type}). +-spec get_uuid_object_id(type_tag(), uuid_id()) -> tuple() | no_return(). get_uuid_object_id(party_config, ID) -> #domain_PartyConfigRef{id = ID}; get_uuid_object_id(shop_config, ID) -> diff --git a/eqwalizer.modules b/eqwalizer.modules index 3730921..da4694f 100644 --- a/eqwalizer.modules +++ b/eqwalizer.modules @@ -1,11 +1,11 @@ # Modules checked by `make eqwalizer` / CI eqwalizer job. # One module name per line; lines starting with # are ignored. -# -# Phase 1 (uncomment when ready): -# dmt_object_id -# dmt_object -# dmt_author -# dmt_author_database -# + +# Phase 1 +dmt_object_id +dmt_object +dmt_author +dmt_author_database + # Phase 2a (uncomment when ready): # dmt_repository From c95ade59c0d92b289492ce4c335333b05b7f8417 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D0=B5=D0=BC?= Date: Mon, 25 May 2026 14:54:39 +0300 Subject: [PATCH 4/5] =?UTF-8?q?Implement=20phase=202a=E2=80=932b=20for=20D?= =?UTF-8?q?MT=20repository=20and=20enhance=20type=20specifications?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit updates the `eqwalizer.modules` file to include the `dmt_repository` module for phase 2a–2b checks. In the `dmt_repository.erl` file, several type specifications are added to improve static analysis, including types for worker, version, object references, and various error handling. Additionally, function specifications are defined for new API functions, enhancing clarity and maintainability. The `dmt_object.erl` file is also updated to export the `object_ref` type, ensuring consistency across modules. These changes aim to strengthen the codebase without altering existing functionality. --- apps/dmt/src/dmt_repository.erl | 223 +++++++++++++++++++++++++---- apps/dmt_object/src/dmt_object.erl | 5 +- eqwalizer.modules | 4 +- 3 files changed, 198 insertions(+), 34 deletions(-) diff --git a/apps/dmt/src/dmt_repository.erl b/apps/dmt/src/dmt_repository.erl index 4fa58ce..97c2768 100644 --- a/apps/dmt/src/dmt_repository.erl +++ b/apps/dmt/src/dmt_repository.erl @@ -1,7 +1,57 @@ -module(dmt_repository). +-typing([eqwalizer]). -include_lib("damsel/include/dmsl_domain_conf_v2_thrift.hrl"). +%% Phase 2c (commit), 2d (graph), history — eqwalizer deferred. +-eqwalizer({nowarn_function, commit/3}). +-eqwalizer({nowarn_function, get_object_history/2}). +-eqwalizer({nowarn_function, get_all_objects_history/1}). +-eqwalizer({nowarn_function, get_related_graph/1}). +-eqwalizer({nowarn_function, get_multiple_related_graph/1}). +-eqwalizer({nowarn_function, search_related_graph/1}). +-eqwalizer({nowarn_function, commit_operations/4}). +-eqwalizer({nowarn_function, commit_operation/2}). +-eqwalizer({nowarn_function, give_data_id/2}). +-eqwalizer({nowarn_function, get_object_ref/1}). +-eqwalizer({nowarn_function, publish_commit_event/3}). +-eqwalizer({nowarn_function, validate_objects_exist/3}). +-eqwalizer({nowarn_function, get_insert_object_id/4}). +-eqwalizer({nowarn_function, get_unique_numerical_id/2}). +-eqwalizer({nowarn_function, get_unique_uuid/2}). + +-type worker() :: atom(). +-type version() :: non_neg_integer(). +-type version_reference() :: + {version, version()} | {head, dmsl_domain_conf_v2_thrift:'Head'()}. +-type object_ref() :: dmt_object:object_ref(). +-type db_object() :: dmt_object:object(). +-type enriched_object() :: #{ + id := object_ref(), + type := dmt_object:object_type(), + version := number() | binary(), + data := term(), + created_at := binary() | list(), + is_active := boolean(), + created_by := dmt_author:author(), + references => [object_ref()], + referenced_by => [object_ref()] +}. +-type read_error() :: + version_not_found + | {object_not_found, object_ref()} + | not_found + | object_type_not_found + | version_author_not_found + | term(). +-type commit_error() :: + {operation_error, {conflict, term()} | {invalid, term()}} + | version_not_found + | author_not_found + | migration_in_progress + | {object_update_too_old, {object_ref(), version()}} + | {conflict, binary()}. + %% API -export([commit/3]). @@ -21,12 +71,14 @@ %% +-spec get_object(worker(), version_reference(), object_ref()) -> + {ok, dmsl_domain_conf_v2_thrift:'VersionedObject'()} | {error, read_error()}. get_object(Worker, {version, V}, ObjectRef) -> case get_target_object(Worker, ObjectRef, V) of {ok, #{data := Data} = Object} -> {ok, #domain_conf_v2_VersionedObject{ info = marshall_to_object_info(Object), - object = Data + object = to_domain_object(Data) }}; {error, Reason} -> {error, Reason} @@ -41,16 +93,18 @@ get_object(Worker, {head, #domain_conf_v2_Head{}}, ObjectRef) -> }} -> {ok, #domain_conf_v2_VersionedObject{ info = #domain_conf_v2_VersionedObjectInfo{ - version = Version, - changed_at = CreatedAt, + version = thrift_version(Version), + changed_at = thrift_timestamp(CreatedAt), changed_by = CreatedBy }, - object = Data + object = to_domain_object(Data) }}; {error, Reason} -> {error, Reason} end. +-spec get_object_with_references(worker(), version_reference(), object_ref()) -> + {ok, dmsl_domain_conf_v2_thrift:'VersionedObjectWithReferences'()} | {error, read_error()}. get_object_with_references(Worker, {version, V}, ObjectRef) -> case get_target_object(Worker, ObjectRef, V) of {ok, #{data := Data} = Object} -> @@ -69,7 +123,7 @@ get_object_with_references(Worker, {version, V}, ObjectRef) -> object = #domain_conf_v2_VersionedObject{ info = marshall_to_object_info(Object), - object = Data + object = to_domain_object(Data) }, referenced_by = ordsets:from_list(ReferencedBy), references_to = ordsets:from_list(ReferencesTo) @@ -81,6 +135,8 @@ get_object_with_references(Worker, {head, #domain_conf_v2_Head{}}, ObjectRef) -> {ok, Version} = dmt_database:get_latest_version(Worker), get_object_with_references(Worker, {version, Version}, ObjectRef). +-spec get_objects(worker(), version_reference(), [object_ref()]) -> + {ok, [dmsl_domain_conf_v2_thrift:'VersionedObject'()]} | {error, read_error() | term()}. get_objects(Worker, {version, V}, ObjectRefs) -> case dmt_database:check_version_exists(Worker, V) of true -> @@ -96,7 +152,7 @@ get_objects(Worker, {version, V}, ObjectRefs) -> VersionedObjects = [ #domain_conf_v2_VersionedObject{ info = marshall_to_object_info(Object), - object = maps:get(data, Object) + object = to_domain_object(maps:get(data, Object)) } || Object <- EnrichedObjects ], @@ -116,6 +172,8 @@ get_objects(Worker, {head, #domain_conf_v2_Head{}}, ObjectRefs) -> {error, Reason} end. +-spec get_snapshot(worker(), version_reference()) -> + {ok, dmsl_domain_conf_v2_thrift:'Snapshot'()} | {error, read_error() | term()}. get_snapshot(Worker, {head, #domain_conf_v2_Head{}}) -> case dmt_database:get_latest_version(Worker) of {ok, LatestVersion} -> @@ -147,6 +205,8 @@ get_snapshot(Worker, {version, Version}) -> {error, version_not_found} end. +-spec get_related_graph(dmsl_domain_conf_v2_thrift:'RelatedGraphRequest'()) -> + {ok, dmsl_domain_conf_v2_thrift:'RelatedGraph'()} | {error, term()}. get_related_graph(Request) -> #domain_conf_v2_RelatedGraphRequest{ ref = ObjectRef, @@ -207,18 +267,17 @@ get_related_graph(Request) -> {error, Reason} end. +-spec sort_objects_by_ids([db_object()], [term()]) -> [db_object()]. sort_objects_by_ids(Objects, IDs) -> % Create a map of ID -> Object for easier lookup ObjectsMap = maps:from_list([{maps:get(id, Obj), Obj} || Obj <- Objects]), % Use list comprehension to order objects according to input IDs % Skip IDs that don't have corresponding objects - [ - maps:get(ID, ObjectsMap, undefined) - || ID <- IDs, - maps:is_key(ID, ObjectsMap) - ]. + [maps:get(ID, ObjectsMap) || ID <- IDs, maps:is_key(ID, ObjectsMap)]. +-spec get_object_history(object_ref(), dmsl_domain_conf_v2_thrift:'RequestParams'()) -> + {ok, dmsl_domain_conf_v2_thrift:'ObjectVersionsResponse'()} | {error, term()}. get_object_history(ObjectRef, RequestParams) -> #domain_conf_v2_RequestParams{ limit = Limit, @@ -252,9 +311,12 @@ get_object_history(ObjectRef, RequestParams) -> end. % Done this way to keep hierarchy of calls +-spec get_latest_version() -> {ok, version()} | {error, term()}. get_latest_version() -> dmt_database:get_latest_version(default_pool). +-spec get_all_objects_history(dmsl_domain_conf_v2_thrift:'RequestParams'()) -> + {ok, dmsl_domain_conf_v2_thrift:'ObjectVersionsResponse'()} | {error, term()}. get_all_objects_history(Request) -> #domain_conf_v2_RequestParams{ limit = Limit, @@ -284,19 +346,55 @@ get_all_objects_history(Request) -> {error, Reason} end. +-spec maybe_to_string(term() | undefined, undefined) -> binary() | undefined. maybe_to_string(undefined, Default) -> Default; maybe_to_string(Value, _) -> dmt_mapper:to_string(Value). -maybe_from_string(undefined, Default) -> Default; -maybe_from_string(Value, _) -> dmt_mapper:from_string(Value). +-spec maybe_from_string(binary() | undefined, non_neg_integer()) -> non_neg_integer(). +maybe_from_string(undefined, Default) -> + Default; +maybe_from_string(Value, Default) -> + try dmt_mapper:from_string(Value) of + Offset when is_integer(Offset), Offset >= 0 -> + Offset; + _ -> + Default + catch + _:_ -> + Default + end. +-spec marshall_to_object_info(enriched_object()) -> + dmsl_domain_conf_v2_thrift:'VersionedObjectInfo'(). marshall_to_object_info(Object) -> #domain_conf_v2_VersionedObjectInfo{ - version = maps:get(version, Object), - changed_at = maps:get(created_at, Object), + version = thrift_version(maps:get(version, Object)), + changed_at = thrift_timestamp(maps:get(created_at, Object)), changed_by = maps:get(created_by, Object) }. +-spec thrift_version(number() | binary()) -> dmsl_domain_conf_v2_thrift:'Version'(). +thrift_version(V) when is_integer(V) -> + V; +thrift_version(V) when is_binary(V) -> + binary_to_integer(V). + +-spec thrift_timestamp(binary() | list()) -> dmsl_base_thrift:'Timestamp'(). +thrift_timestamp(Ts) when is_binary(Ts) -> + Ts; +thrift_timestamp(Ts) when is_list(Ts) -> + list_to_binary(Ts). + +-spec to_thrift_reference(object_ref()) -> dmsl_domain_thrift:'Reference'(). +to_thrift_reference(Ref) -> + eqwalizer:dynamic_cast(Ref). + +-spec to_domain_object(term()) -> dmsl_domain_thrift:'DomainObject'(). +to_domain_object(Data) -> + eqwalizer:dynamic_cast(Data). + +-spec search_objects(dmsl_domain_conf_v2_thrift:'SearchRequestParams'()) -> + {ok, dmsl_domain_conf_v2_thrift:'SearchResponse'()} | {error, read_error() | term()}. search_objects(Request) -> #domain_conf_v2_SearchRequestParams{ query = Query, @@ -323,7 +421,7 @@ search_objects(Request) -> result = [ #domain_conf_v2_LimitedVersionedObject{ info = marshall_to_object_info(Object), - ref = maps:get(id, Object), + ref = to_thrift_reference(maps:get(id, Object)), name = dmt_domain:maybe_get_domain_object_data_field(name, Data), description = dmt_domain:maybe_get_domain_object_data_field(description, Data) } @@ -340,6 +438,8 @@ search_objects(Request) -> {error, Reason} end. +-spec search_full_objects(dmsl_domain_conf_v2_thrift:'SearchRequestParams'()) -> + {ok, dmsl_domain_conf_v2_thrift:'SearchFullResponse'()} | {error, read_error() | term()}. search_full_objects(Request) -> #domain_conf_v2_SearchRequestParams{ query = Query, @@ -366,7 +466,7 @@ search_full_objects(Request) -> result = [ #domain_conf_v2_VersionedObject{ info = marshall_to_object_info(Object), - object = Data + object = to_domain_object(Data) } || #{data := Data} = Object <- Objects2 ], @@ -381,6 +481,7 @@ search_full_objects(Request) -> {error, Reason} end. +-spec filter_search_results([db_object()]) -> [db_object()]. filter_search_results(Objects) -> lists:filter( fun(Object) -> @@ -388,8 +489,8 @@ filter_search_results(Objects) -> Data = maps:get(data, Object), case { - dmt_thrift_validator:validate_reference(ID), - dmt_thrift_validator:validate_domain_object(Data) + dmt_thrift_validator:validate_reference(to_thrift_reference(ID)), + dmt_thrift_validator:validate_domain_object(to_domain_object(Data)) } of {ok, ok} -> @@ -403,9 +504,11 @@ filter_search_results(Objects) -> Objects ). +-spec maybe_check_entity_type_exists(dmt_object:object_type() | undefined) -> ok | {error, object_type_not_found}. maybe_check_entity_type_exists(undefined) -> ok; maybe_check_entity_type_exists(Type) -> dmt_database:check_entity_type_exists(default_pool, Type). +-spec commit_operations(worker(), term(), version(), version()) -> term(). commit_operations(Worker, Operations, TargetVersion, NewVersion) -> {_, _, _, RelationsChanges, FinalOperations, NewObjects, RemovedObjectsReferences} = lists:foldl( fun commit_operation/2, @@ -414,6 +517,7 @@ commit_operations(Worker, Operations, TargetVersion, NewVersion) -> ), {RelationsChanges, FinalOperations, NewObjects, RemovedObjectsReferences}. +-spec commit_operation(term(), term()) -> term(). commit_operation( {insert, #domain_conf_v2_InsertOp{object = Object, force_ref = ForceRef}}, {Worker, TargetVersion, NewVersion, RelationsChanges, FinalOperations, NewObjects, RemovedObjectsReferences} @@ -474,6 +578,7 @@ commit_operation( [Ref | RemovedObjectsReferences] }. +-spec insert_relation(worker(), term(), term(), version(), boolean()) -> ok | no_return(). insert_relation(Worker, OriginRef, Reference, NewVersion, IsActive) -> OriginRef1 = dmt_mapper:ref_to_string(OriginRef), Reference1 = dmt_mapper:ref_to_string(Reference), @@ -491,6 +596,7 @@ insert_relation(Worker, OriginRef, Reference, NewVersion, IsActive) -> throw({error, Reason}) end. +-spec commit_relations_changes(worker(), version(), map()) -> ok. commit_relations_changes(Worker, NewVersion, RelationsChanges) -> maps:foreach( fun(OriginRef, References) -> @@ -522,6 +628,12 @@ commit_relations_changes(Worker, NewVersion, RelationsChanges) -> RelationsChanges ). +-spec commit( + version(), + [dmsl_domain_conf_v2_thrift:'Operation'()], + dmt_author:author_id() +) -> + {ok, version(), [term()]} | {error, commit_error() | term()}. commit(Version, Operations, AuthorID) -> Result = epg_pool:transaction( default_pool, @@ -564,6 +676,7 @@ commit(Version, Operations, AuthorID) -> {error, Error} end. +-spec validate_no_references_to_entities(worker(), [object_ref()], version()) -> ok. validate_no_references_to_entities(Worker, RemovedObjectsReferences, Version) -> %% Ensure there are no inbound references to any removed objects at Version %% If any exist, validate_no_references_to_entity/3 will throw @@ -575,6 +688,7 @@ validate_no_references_to_entities(Worker, RemovedObjectsReferences, Version) -> ), ok. +-spec validate_no_references_to_entity(worker(), object_ref(), version()) -> ok | no_return(). validate_no_references_to_entity(Worker, Ref, Version) -> Ref1 = dmt_mapper:ref_to_string(Ref), _ = logger:warning("Validating no references to entity ~p at version ~p", [Ref, Version]), @@ -586,6 +700,7 @@ validate_no_references_to_entity(Worker, Ref, Version) -> throw({error, {invalid, {objects_not_exist, [{Ref, ReferencedBy}]}}}) end. +-spec validate_latest_version(worker(), version(), object_ref()) -> ok | no_return(). validate_latest_version(Worker, TargetVersion, Ref) -> Ref0 = dmt_mapper:ref_to_string(Ref), case dmt_database:get_object_latest_version(Worker, Ref0) of @@ -599,6 +714,7 @@ validate_latest_version(Worker, TargetVersion, Ref) -> throw(Error) end. +-spec validate_author_exists(worker(), dmt_author:author_id()) -> ok | no_return(). validate_author_exists(Worker, AuthorID) -> case dmt_author_database:get(Worker, AuthorID) of {ok, _} -> @@ -607,6 +723,7 @@ validate_author_exists(Worker, AuthorID) -> throw({error, author_not_found}) end. +-spec get_new_version(worker(), dmt_author:author_id()) -> version() | no_return(). get_new_version(Worker, AuthorID) -> case dmt_database:get_new_version(Worker, AuthorID) of {ok, NewVersion} -> @@ -615,6 +732,7 @@ get_new_version(Worker, AuthorID) -> throw({error, Reason}) end. +-spec insert_object(worker(), dmt_object:object_type(), term(), version(), term()) -> term() | no_return(). insert_object(Worker, Type, ID0, Version, Data0) -> ID1 = dmt_mapper:ref_to_string(ID0), Data1 = dmt_mapper:object_to_string(Data0), @@ -631,6 +749,7 @@ insert_object(Worker, Type, ID0, Version, Data0) -> throw({error, Reason}) end. +-spec give_data_id(term(), object_ref()) -> term(). give_data_id({Tag, Data}, Ref) -> {struct, union, DomainObjects} = dmsl_domain_thrift:struct_info('DomainObject'), {value, {_, _, {_, _, {_, ObjectName}}, Tag, _}} = lists:search( @@ -653,11 +772,14 @@ give_data_id({Tag, Data}, Ref) -> Second = get_object_field(SecondField, Data, Ref), {Tag, {RecordName, First, Second}}. +-spec get_object_field(term(), term(), object_ref()) -> term(). get_object_field({_, _, _, ref, _}, _Data, {_Type, Ref}) -> Ref; get_object_field({_, _, _, data, _}, Data, _Ref) -> Data. +-spec update_object(worker(), dmt_object:object_type(), term(), boolean(), term(), version()) -> + ok | no_return(). update_object(Worker, Type, ID0, IsActive, Data0, Version) -> Data1 = dmt_mapper:object_to_string(Data0), ID1 = dmt_mapper:ref_to_string(ID0), @@ -680,6 +802,7 @@ update_object(Worker, Type, ID0, IsActive, Data0, Version) -> throw({error, Reason}) end. +-spec get_insert_object_id(worker(), term(), dmt_object:object_type(), term()) -> object_ref() | no_return(). get_insert_object_id(Worker, undefined, Type, Object) -> %% Check if sequence column exists in table %% -- if it doesn't, then raise exception @@ -708,6 +831,8 @@ get_insert_object_id(Worker, Ref, _Type, _Object) -> throw({error, Reason}) end. +-spec get_unique_numerical_id(worker(), dmt_object:object_type()) -> + {ok, term()} | {error, sequence_not_enabled} | no_return(). get_unique_numerical_id(Worker, Type) -> case dmt_database:get_next_sequence(Worker, Type) of {ok, NewID} -> @@ -727,6 +852,7 @@ get_unique_numerical_id(Worker, Type) -> throw({error, Reason}) end. +-spec get_unique_uuid(worker(), dmt_object:object_type()) -> {ok, term()} | no_return(). get_unique_uuid(Worker, Type) -> NewUUID = uuid:uuid_to_string(uuid:get_v4_urandom(), binary_standard), NewID = dmt_object_id:get_uuid_object_id(Type, NewUUID), @@ -740,6 +866,8 @@ get_unique_uuid(Worker, Type) -> throw({error, Reason}) end. +-spec get_target_object(worker(), object_ref(), version()) -> + {ok, enriched_object()} | {error, read_error() | term()}. get_target_object(Worker, Ref, Version) -> Ref0 = dmt_mapper:ref_to_string(Ref), case dmt_database:check_version_exists(Worker, Version) of @@ -754,29 +882,54 @@ get_target_object(Worker, Ref, Version) -> {error, version_not_found} end. +-spec add_created_by_to_objects(worker(), [db_object()]) -> + {ok, [enriched_object()]} | {error, version_author_not_found | term()}. add_created_by_to_objects(Worker, Objects) -> - Versions = lists:uniq([Version || #{version := Version} <- Objects]), - AuthorsOfVersions = - #{ - Version => Author - || Version <- Versions, - {ok, AuthorID} <- [dmt_database:get_version_creator(Worker, Version)], - {ok, Author} <- [dmt_author:get(AuthorID)] - }, + Versions = [V || #{version := V} <- Objects], + case load_authors_for_versions(Worker, Versions) of + {ok, AuthorsOfVersions} -> EnrichedObjects = [ Object#{ - created_by => maps:get(Version, AuthorsOfVersions, undefined) + created_by => maps:get(thrift_version(maps:get(version, Object)), AuthorsOfVersions) } - || #{version := Version} = Object <- Objects + || Object <- Objects ], - {ok, EnrichedObjects}. + {ok, EnrichedObjects}; + {error, Reason} -> + {error, Reason} + end. + +-spec load_authors_for_versions(worker(), [number() | binary()]) -> + {ok, #{version() => dmt_author:author()}} | {error, version_author_not_found | term()}. +load_authors_for_versions(Worker, Versions) -> + Normalized = lists:uniq([thrift_version(V) || V <- Versions]), + Results = [{Version, resolve_version_author(Worker, Version)} || Version <- Normalized], + case lists:all(fun({_, {ok, _}}) -> true; (_) -> false end, Results) of + true -> + {ok, maps:from_list([{V, A} || {V, {ok, A}} <- Results])}; + false -> + {error, version_author_not_found} + end. + +-spec resolve_version_author(worker(), version()) -> {ok, dmt_author:author()} | {error, term()}. +resolve_version_author(Worker, Version) -> + case dmt_database:get_version_creator(Worker, Version) of + {ok, AuthorID} -> + dmt_author:get(AuthorID); + {error, Reason} -> + {error, Reason} + end. +-spec add_created_by_to_object(worker(), db_object()) -> + {ok, enriched_object()} | {error, term()}. add_created_by_to_object(Worker, Object) -> #{version := Version} = Object, {ok, AuthorID} = dmt_database:get_version_creator(Worker, Version), {ok, Author} = dmt_author:get(AuthorID), {ok, Object#{created_by => Author}}. +-spec get_latest_target_object(worker(), object_ref()) -> + {ok, enriched_object()} | {error, {object_not_found, object_ref()} | term()}. get_latest_target_object(Worker, Ref) -> Ref0 = dmt_mapper:ref_to_string(Ref), @@ -787,15 +940,18 @@ get_latest_target_object(Worker, Ref) -> {error, {object_not_found, Ref}} end. +-spec get_version(worker(), version() | undefined) -> version(). get_version(Worker, undefined) -> {ok, LatestVersion} = dmt_database:get_latest_version(Worker), LatestVersion; get_version(_Worker, Version) -> Version. +-spec get_object_ref(term()) -> {ok, object_ref()}. get_object_ref({Type, {_Object, ID, _Data}}) -> {ok, {Type, ID}}. +-spec publish_commit_event(version(), term(), dmt_author:author_id()) -> ok | {error, term()}. publish_commit_event(Version, FinalOperations, AuthorID) -> try %% Get author information @@ -835,6 +991,7 @@ publish_commit_event(Version, FinalOperations, AuthorID) -> %% Helper functions for get_related_graph +-spec resolve_version_reference(worker(), version() | undefined) -> {ok, version()} | {error, term()}. resolve_version_reference(Worker, undefined) -> case dmt_database:get_latest_version(Worker) of {ok, LatestVersion} -> {ok, LatestVersion}; @@ -846,6 +1003,7 @@ resolve_version_reference(Worker, Version) -> false -> {error, version_not_found} end. +-spec validate_object_exists(worker(), object_ref(), version()) -> ok | {error, object_not_found}. validate_object_exists(Worker, ObjectRef, Version) -> ObjectRefString = dmt_mapper:ref_to_string(ObjectRef), case dmt_database:get_object(Worker, ObjectRefString, Version) of @@ -853,6 +1011,8 @@ validate_object_exists(Worker, ObjectRef, Version) -> {error, not_found} -> {error, object_not_found} end. +-spec get_multiple_related_graph(dmsl_domain_conf_v2_thrift:'MultipleRelatedGraphRequest'()) -> + {ok, dmsl_domain_conf_v2_thrift:'RelatedGraph'()} | {error, term()}. get_multiple_related_graph(Request) -> #domain_conf_v2_MultipleRelatedGraphRequest{ refs = ObjectRefs, @@ -910,6 +1070,8 @@ get_multiple_related_graph(Request) -> {error, Reason} end. +-spec search_related_graph(dmsl_domain_conf_v2_thrift:'SearchRelatedGraphRequest'()) -> + {ok, dmsl_domain_conf_v2_thrift:'RelatedGraph'()} | {error, term()}. search_related_graph(Request) -> #domain_conf_v2_SearchRelatedGraphRequest{ query = Query, @@ -981,6 +1143,7 @@ search_related_graph(Request) -> {error, Reason} end. +-spec validate_objects_exist(worker(), [object_ref()], version()) -> ok | {error, object_not_found}. validate_objects_exist(Worker, ObjectRefs, Version) -> genlib_list:foldl_while( fun(ObjectRef, _Acc) -> diff --git a/apps/dmt_object/src/dmt_object.erl b/apps/dmt_object/src/dmt_object.erl index 99deda0..c5ba39d 100644 --- a/apps/dmt_object/src/dmt_object.erl +++ b/apps/dmt_object/src/dmt_object.erl @@ -13,6 +13,7 @@ -export_type([object_changes/0]). -export_type([object/0]). -export_type([object_type/0]). +-export_type([object_ref/0]). %% Object type tag. Constructed as an atom in code (e.g. `category`, %% `provider`) but stored as text in the DB, so values read back from a row @@ -39,7 +40,7 @@ }. -type object() :: #{ - id := term(), + id := object_ref(), type := object_type(), version := number() | binary(), data := term(), @@ -87,7 +88,7 @@ remove_object(OG) -> }. -spec just_object( - term(), + object_ref(), object_type(), number() | binary(), term(), diff --git a/eqwalizer.modules b/eqwalizer.modules index da4694f..bcdb50c 100644 --- a/eqwalizer.modules +++ b/eqwalizer.modules @@ -7,5 +7,5 @@ dmt_object dmt_author dmt_author_database -# Phase 2a (uncomment when ready): -# dmt_repository +# Phase 2a–2b +dmt_repository From 1f6816e387633612e4961535146269949ec5f7d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D0=B5=D0=BC?= Date: Mon, 25 May 2026 15:58:14 +0300 Subject: [PATCH 5/5] Enhance DMT modules with additional type exports and improve test configurations This commit adds new type exports to the `dmt_author_database`, `dmt_repository`, `dmt_object_id`, and `dmt_object` modules, improving the clarity and usability of type specifications across the codebase. Additionally, the `dmt_repository_filter_test` file is updated to suppress dialyzer warnings for several test functions, ensuring smoother test execution. These changes aim to enhance static analysis capabilities and maintainability without altering existing functionality. --- apps/dmt/src/dmt_author_database.erl | 2 ++ apps/dmt/src/dmt_repository.erl | 32 +++++++++++++++----- apps/dmt/test/dmt_repository_filter_test.erl | 11 ++++++- apps/dmt_object/src/dmt_object.erl | 13 +++++--- apps/dmt_object/src/dmt_object_id.erl | 2 ++ rebar.config | 4 +-- 6 files changed, 49 insertions(+), 15 deletions(-) diff --git a/apps/dmt/src/dmt_author_database.erl b/apps/dmt/src/dmt_author_database.erl index 5c61c92..a0a2c17 100644 --- a/apps/dmt/src/dmt_author_database.erl +++ b/apps/dmt/src/dmt_author_database.erl @@ -20,6 +20,8 @@ -type email() :: dmt_author:email(). -type author() :: dmt_author:author(). +-export_type([worker/0, author_id/0, name/0, email/0, author/0]). + -spec insert(worker(), name(), email()) -> {ok, author_id()} | {ok, {already_exists, author_id()}} | {error, unknown}. insert(Worker, Name, Email) -> diff --git a/apps/dmt/src/dmt_repository.erl b/apps/dmt/src/dmt_repository.erl index 97c2768..3fddb20 100644 --- a/apps/dmt/src/dmt_repository.erl +++ b/apps/dmt/src/dmt_repository.erl @@ -52,6 +52,16 @@ | {object_update_too_old, {object_ref(), version()}} | {conflict, binary()}. +-export_type([ + worker/0, + version/0, + version_reference/0, + object_ref/0, + db_object/0, + read_error/0, + commit_error/0 +]). + %% API -export([commit/3]). @@ -888,12 +898,12 @@ add_created_by_to_objects(Worker, Objects) -> Versions = [V || #{version := V} <- Objects], case load_authors_for_versions(Worker, Versions) of {ok, AuthorsOfVersions} -> - EnrichedObjects = [ - Object#{ - created_by => maps:get(thrift_version(maps:get(version, Object)), AuthorsOfVersions) - } - || Object <- Objects - ], + EnrichedObjects = [ + Object#{ + created_by => maps:get(thrift_version(maps:get(version, Object)), AuthorsOfVersions) + } + || Object <- Objects + ], {ok, EnrichedObjects}; {error, Reason} -> {error, Reason} @@ -904,7 +914,15 @@ add_created_by_to_objects(Worker, Objects) -> load_authors_for_versions(Worker, Versions) -> Normalized = lists:uniq([thrift_version(V) || V <- Versions]), Results = [{Version, resolve_version_author(Worker, Version)} || Version <- Normalized], - case lists:all(fun({_, {ok, _}}) -> true; (_) -> false end, Results) of + case + lists:all( + fun + ({_, {ok, _}}) -> true; + (_) -> false + end, + Results + ) + of true -> {ok, maps:from_list([{V, A} || {V, {ok, A}} <- Results])}; false -> diff --git a/apps/dmt/test/dmt_repository_filter_test.erl b/apps/dmt/test/dmt_repository_filter_test.erl index b2c84de..29c1936 100644 --- a/apps/dmt/test/dmt_repository_filter_test.erl +++ b/apps/dmt/test/dmt_repository_filter_test.erl @@ -4,7 +4,16 @@ -include_lib("damsel/include/dmsl_domain_thrift.hrl"). % We modify records in improper way in tests, so we need to suppress dialyzer warnings --dialyzer({nowarn_function, [test_all_invalid_objects/0, test_mixed_valid_invalid_objects/0]}). +-dialyzer( + {nowarn_function, [ + test_all_valid_objects/0, + test_all_invalid_objects/0, + test_mixed_valid_invalid_objects/0, + test_single_valid_object/0, + test_single_invalid_object/0, + test_missing_data_field/0 + ]} +). %% Test the filter_search_results/1 function from dmt_repository module diff --git a/apps/dmt_object/src/dmt_object.erl b/apps/dmt_object/src/dmt_object.erl index c5ba39d..bc49ae0 100644 --- a/apps/dmt_object/src/dmt_object.erl +++ b/apps/dmt_object/src/dmt_object.erl @@ -9,11 +9,14 @@ -export([just_object/6]). -export([filter_out_inactive_objects/1]). --export_type([insertable_object/0]). --export_type([object_changes/0]). --export_type([object/0]). --export_type([object_type/0]). --export_type([object_ref/0]). +-export_type([ + insertable_object/0, + object_changes/0, + object/0, + object_type/0, + object_ref/0, + domain_object/0 +]). %% Object type tag. Constructed as an atom in code (e.g. `category`, %% `provider`) but stored as text in the DB, so values read back from a row diff --git a/apps/dmt_object/src/dmt_object_id.erl b/apps/dmt_object/src/dmt_object_id.erl index 66d1bfe..65a8926 100644 --- a/apps/dmt_object/src/dmt_object_id.erl +++ b/apps/dmt_object/src/dmt_object_id.erl @@ -11,6 +11,8 @@ -type uuid_id() :: binary(). -type type_tag() :: atom(). +-export_type([numerical_id/0, uuid_id/0, type_tag/0]). + -spec get_numerical_object_id(type_tag(), numerical_id()) -> tuple() | no_return(). get_numerical_object_id(category, ID) -> #domain_CategoryRef{id = ID}; diff --git a/rebar.config b/rebar.config index ca06b96..a5aaa37 100644 --- a/rebar.config +++ b/rebar.config @@ -67,8 +67,8 @@ {jsx, "3.1.0"}, {eqwalizer_support, - {git_subdir, "https://github.com/whatsapp/eqwalizer.git", - {ref, "c4d1098174cec06bd124855f3a28dfd6eda0a581"}, "eqwalizer_support"}} + {git_subdir, "https://github.com/whatsapp/eqwalizer.git", {ref, "c4d1098174cec06bd124855f3a28dfd6eda0a581"}, + "eqwalizer_support"}} ]}. %% XRef checks