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_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..15b3238 100644 --- a/apps/dmt/src/dmt_author.erl +++ b/apps/dmt/src/dmt_author.erl @@ -1,6 +1,5 @@ -module(dmt_author). - -%% Public API +-typing([eqwalizer]). -include_lib("damsel/include/dmsl_domain_conf_v2_thrift.hrl"). @@ -20,12 +19,6 @@ -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) -> @@ -42,16 +35,3 @@ get_by_email(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 a4e64af..a0a2c17 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,7 +14,7 @@ search/3 ]). --type worker() :: dmt_database:worker(). +-type worker() :: atom(). -type author_id() :: dmt_author:author_id(). -type name() :: dmt_author:name(). -type email() :: dmt_author:email(). @@ -181,7 +182,9 @@ is_uuid(_) -> -spec try_string_to_uuid(uuid:uuid_string()) -> boolean(). try_string_to_uuid(UUID) -> try uuid:string_to_uuid(UUID) of - _ -> true + _ -> + 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..3fddb20 100644 --- a/apps/dmt/src/dmt_repository.erl +++ b/apps/dmt/src/dmt_repository.erl @@ -1,7 +1,67 @@ -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()}. + +-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]). @@ -19,47 +79,16 @@ -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()}. +-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} @@ -74,18 +103,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_ref(), object_ref()) -> - {ok, dmsl_domain_conf_v2_thrift:'VersionedObjectWithReferences'()} | {error, get_error()}. +-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} -> @@ -104,7 +133,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) @@ -116,8 +145,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_ref(), [object_ref()]) -> - {ok, [dmsl_domain_conf_v2_thrift:'VersionedObject'()]} | {error, version_not_found | term()}. +-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 -> @@ -133,7 +162,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 ], @@ -153,8 +182,8 @@ 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()}. +-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} -> @@ -172,9 +201,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 }}; @@ -186,8 +216,7 @@ get_snapshot(Worker, {version, Version}) -> 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()}. + {ok, dmsl_domain_conf_v2_thrift:'RelatedGraph'()} | {error, term()}. get_related_graph(Request) -> #domain_conf_v2_RelatedGraphRequest{ ref = ObjectRef, @@ -248,28 +277,17 @@ 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()]. +-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 - [Obj || ID <- IDs, {ok, Obj} <- [maps:find(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, not_found | term()}. + {ok, dmsl_domain_conf_v2_thrift:'ObjectVersionsResponse'()} | {error, term()}. get_object_history(ObjectRef, RequestParams) -> #domain_conf_v2_RequestParams{ limit = Limit, @@ -338,47 +356,55 @@ get_all_objects_history(Request) -> {error, Reason} end. --spec maybe_to_string(term(), Default) -> binary() | Default. +-spec maybe_to_string(term() | undefined, undefined) -> binary() | undefined. 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(). +-spec maybe_from_string(binary() | undefined, non_neg_integer()) -> non_neg_integer(). 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}) +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(object_map()) -> dmsl_domain_conf_v2_thrift:'VersionedObjectInfo'(). +-spec marshall_to_object_info(enriched_object()) -> + 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 = thrift_version(maps:get(version, Object)), + changed_at = thrift_timestamp(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 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 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 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 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 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, object_type_not_found | term()}. + {ok, dmsl_domain_conf_v2_thrift:'SearchResponse'()} | {error, read_error() | term()}. search_objects(Request) -> #domain_conf_v2_SearchRequestParams{ query = Query, @@ -405,7 +431,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) } @@ -423,8 +449,7 @@ search_objects(Request) -> end. -spec search_full_objects(dmsl_domain_conf_v2_thrift:'SearchRequestParams'()) -> - {ok, dmsl_domain_conf_v2_thrift:'SearchFullResponse'()} - | {error, object_type_not_found | term()}. + {ok, dmsl_domain_conf_v2_thrift:'SearchFullResponse'()} | {error, read_error() | term()}. search_full_objects(Request) -> #domain_conf_v2_SearchRequestParams{ query = Query, @@ -451,7 +476,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 ], @@ -466,7 +491,7 @@ search_full_objects(Request) -> {error, Reason} end. --spec filter_search_results([object_map()]) -> [object_map()]. +-spec filter_search_results([db_object()]) -> [db_object()]. filter_search_results(Objects) -> lists:filter( fun(Object) -> @@ -474,8 +499,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} -> @@ -489,11 +514,11 @@ filter_search_results(Objects) -> Objects ). --spec maybe_check_entity_type_exists(atom() | binary() | undefined) -> - ok | {error, object_type_not_found | term()}. +-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, @@ -502,6 +527,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} @@ -562,6 +588,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), @@ -579,6 +606,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) -> @@ -610,8 +638,12 @@ commit_relations_changes(Worker, NewVersion, RelationsChanges) -> RelationsChanges ). --spec commit(version(), [operation()], author_id()) -> - {ok, version(), [term()]} | {error, commit_error()}. +-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, @@ -640,7 +672,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,13 +683,10 @@ 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. +-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 @@ -669,6 +698,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]), @@ -680,6 +710,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 @@ -693,6 +724,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, _} -> @@ -701,6 +733,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} -> @@ -709,6 +742,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), @@ -725,6 +759,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( @@ -738,26 +773,23 @@ 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}}. +-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), @@ -780,6 +812,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 @@ -808,6 +841,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} -> @@ -827,10 +862,9 @@ 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) -> - %% `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 @@ -842,6 +876,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 @@ -856,29 +892,62 @@ 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)] - }, - EnrichedObjects = [ - Object#{ - created_by => maps:get(Version, AuthorsOfVersions, undefined) - } - || #{version := Version} = Object <- Objects - ], - {ok, EnrichedObjects}. + 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 + ], + {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), @@ -889,15 +958,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 @@ -937,6 +1009,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}; @@ -948,6 +1021,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 @@ -956,8 +1030,7 @@ validate_object_exists(Worker, ObjectRef, Version) -> 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()}. + {ok, dmsl_domain_conf_v2_thrift:'RelatedGraph'()} | {error, term()}. get_multiple_related_graph(Request) -> #domain_conf_v2_MultipleRelatedGraphRequest{ refs = ObjectRefs, @@ -1016,8 +1089,7 @@ get_multiple_related_graph(Request) -> 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()}. + {ok, dmsl_domain_conf_v2_thrift:'RelatedGraph'()} | {error, term()}. search_related_graph(Request) -> #domain_conf_v2_SearchRelatedGraphRequest{ query = Query, @@ -1089,6 +1161,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/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_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/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..29c1936 100644 --- a/apps/dmt/test/dmt_repository_filter_test.erl +++ b/apps/dmt/test/dmt_repository_filter_test.erl @@ -4,9 +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]}). --eqwalizer({nowarn_function, test_all_invalid_objects/0}). --eqwalizer({nowarn_function, 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_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.erl b/apps/dmt_object/src/dmt_object.erl index 049a89f..bc49ae0 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"). @@ -10,50 +9,49 @@ -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_id/0]). --export_type([timestamp/0]). --export_type([domain_object/0]). --export_type([refless_domain_object/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 %% 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 object_ref() :: {object_type(), term()}. -type refless_domain_object() :: dmsl_domain_thrift:'ReflessDomainObject'(). +-type domain_object() :: dmsl_domain_thrift:'DomainObject'(). -type insertable_object() :: #{ - tmp_id := binary(), type := object_type(), + tmp_id := binary(), forced_id := term() | undefined, data := refless_domain_object() }. -type object_changes() :: #{ - id := {object_type(), object_id()}, + id := object_ref(), 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_ref()], + referenced_by => [object_ref()], + data => domain_object(), is_active => boolean() }. -type object() :: #{ - id := term(), + id := object_ref(), type := object_type(), version := number() | binary(), data := term(), created_at := binary() | list(), is_active := boolean(), - atom() => term() + references => [object_ref()], + referenced_by => [object_ref()], + created_by => binary() }. -spec new_object(dmsl_domain_conf_v2_thrift:'InsertOp'()) -> @@ -74,9 +72,9 @@ new_object(#domain_conf_v2_InsertOp{ {error, Error} end. --spec update_object(domain_object() | term(), object_changes()) -> +-spec update_object(domain_object(), object_changes()) -> {ok, object_changes()} | {error, {is_not_domain_object, term()}}. -update_object({Type, {_Record, ID, _Data}} = Object, ExistingUpdate) when is_atom(Type) -> +update_object({Type, {_Object, ID, _Data}} = Object, ExistingUpdate) -> {ok, ExistingUpdate#{ id => {Type, ID}, type => Type, @@ -92,7 +90,14 @@ remove_object(OG) -> is_active => false }. --spec just_object(term(), term(), term(), term(), term(), term()) -> object(). +-spec just_object( + object_ref(), + object_type(), + number() | binary(), + term(), + binary() | list(), + boolean() +) -> object(). just_object( ID, Type, diff --git a/apps/dmt_object/src/dmt_object_id.erl b/apps/dmt_object/src/dmt_object_id.erl index 8b59636..65a8926 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"). 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{}) -> 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..bcdb50c --- /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 +dmt_object_id +dmt_object +dmt_author +dmt_author_database + +# Phase 2a–2b +dmt_repository diff --git a/rebar.config b/rebar.config index 47b27e9..a5aaa37 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