Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove dependencies if not needed #138

Merged
merged 6 commits into from
Sep 3, 2024
Merged

Remove dependencies if not needed #138

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bdcdab1
Remove unneeded dependency, OTP covers since 21.0
NelsonVides Aug 27, 2024
72a1da9
Remain backwards-compatible with older OTP
NelsonVides Aug 28, 2024
9a39bf2
Use json from OTP27 and jsx optionally otherwise
NelsonVides Aug 28, 2024
9e8fa79
Optionally include jsx and rfc3339 in releases
NelsonVides Aug 29, 2024
8b0daa2
Encapsulate json module (elvis error)
NelsonVides Aug 29, 2024
76345bc
Apply review comments
NelsonVides Sep 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions rebar.config
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
%%-*- mode: erlang -*-
{profiles, [{test, [{deps, [{ proper, "1.4.0"}]}]}]}.
{profiles, [{test, [{deps, [{jsx, "3.1.0"}, {proper, "1.4.0"}]}]}]}.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{profiles, [{test, [{deps, [{jsx, "3.1.0"}, {proper, "1.4.0"}]}]}]}.
{profiles, [{test, [{deps, [{jsx, "3.1.0"}, {rfc3339, "0.9.0"}, {proper, "1.4.0"}]}]}]}.

{erl_opts, [ {platform_define, "^R[0-9]+", erlang_deprecated_types}
%% , warn_export_all
, warn_export_vars
Expand All @@ -18,11 +18,6 @@
, undefined_function_calls
, deprecated_function_calls
]}.
{ deps
, [ {jsx, "3.1.0"}
, {rfc3339, "0.9.0"}
]}.

{ project_plugins
, [ {rebar3_lint, "3.2.6"}
, {rebar3_proper, "0.12.1"}
Expand Down
15 changes: 15 additions & 0 deletions rebar.config.script
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{Deps0, Config0} = case lists:keytake(deps, 1, CONFIG) of
false -> {[], CONFIG};
{value, {deps, D}, Cfg} -> {D, Cfg}
end,

Deps = case list_to_integer(erlang:system_info(otp_release)) of
N when N >= 27 ->
[];
N when N >= 21 ->
[{jsx, "3.1.0"}];
_ ->
[{jsx, "3.1.0"}, {rfc3339, "0.9.0"}]
end,

[{deps, Deps ++ Deps0} | Config0].
2 changes: 0 additions & 2 deletions src/jesse.app.src
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
, public_key
, ssl
, inets
, jsx
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: one can actually add .app.src.script, see https://rebar3.org/docs/configuration/config_script

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but we don't actually need jsx here anyways, this list is for apps to be started, jsx is not an app but only code to be loaded, that rebar will put in the right path for the BEAM to load the code automatically

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's also for rebar3 release to find the apps to be copied.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely very good point I don't know how could I forget! Fixed! :)

, rfc3339
]}
, {env, [ {re_options, [unicode, ucp]}
]}
Expand Down
6 changes: 3 additions & 3 deletions src/jesse_cli.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ run(Options, [Schema|_] = Schemata, [JsonInstance|JsonInstances]) ->
undefined ->
io:fwrite("~p\n\n", [Result]);
true ->
io:fwrite("~s\n\n", [jsx:encode(Result)])
io:fwrite("~s\n\n", [?JSON:encode(Result)])
end,
case JesseResult of
{ok, _} ->
Expand All @@ -90,7 +90,7 @@ jesse_run(JsonInstance, Schema, Schemata) ->
{ok, _} = application:ensure_all_started(jesse),
ok = add_schemata(Schemata),
{ok, JsonInstanceBinary} = file:read_file(JsonInstance),
JsonInstanceJsx = jsx:decode(JsonInstanceBinary, [{return_maps, false}]),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be an ok change, and I don't have all the details in my head at this very moment, but why did you decide to remove return_maps=false ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there's the "no macros in dynamic calls" to fix (see the CI run), and I guess the solution will be to create a function for encapsulation, maybe we just keep the return_maps=false

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed it because json does not support it, and I followed the logic and manually verified it is not needed (anymore) 🤔

That CI error, huh, elvis. Will fix 👌🏽

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed a fix :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, made a typo on the fix, pushed again 😅

JsonInstanceJsx = ?JSON:decode(JsonInstanceBinary),
jesse:validate( Schema
, JsonInstanceJsx
).
Expand All @@ -99,7 +99,7 @@ add_schemata([]) ->
ok;
add_schemata([SchemaFile|Rest]) ->
{ok, SchemaBin} = file:read_file(SchemaFile),
Schema0 = jsx:decode(SchemaBin, [{return_maps, false}]),
Schema0 = ?JSON:decode(SchemaBin),
Schema = maybe_fill_schema_id(SchemaFile, Schema0),
ok = jesse:add_schema(SchemaFile, Schema),
add_schemata(Rest).
Expand Down
4 changes: 2 additions & 2 deletions src/jesse_database.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ add_file_uri(Key0) ->
"file://" ++ File = Key,
{ok, SchemaBin} = file:read_file(File),
{ok, #file_info{mtime = Mtime}} = file:read_file_info(File),
Schema = jsx:decode(SchemaBin, [{return_maps, false}]),
Schema = ?JSON:decode(SchemaBin),
SchemaInfos = [{Key, Mtime, Schema}],
ValidationFun = fun jesse_lib:is_json_object/1,
store_schemas(SchemaInfos, ValidationFun).
Expand All @@ -321,7 +321,7 @@ add_http_uri(Key0) ->
, HttpOptions
, [{body_format, binary}]),
{{_Line, 200, _}, Headers, SchemaBin} = Response,
Schema = jsx:decode(SchemaBin, [{return_maps, false}]),
Schema = ?JSON:decode(SchemaBin),
SchemaInfos = [{Key, get_http_mtime(Headers), Schema}],
ValidationFun = fun jesse_lib:is_json_object/1,
store_schemas(SchemaInfos, ValidationFun).
Expand Down
10 changes: 2 additions & 8 deletions src/jesse_error.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
, handle_data_invalid/3
, handle_schema_invalid/2
, to_json/1
, to_json/2
, reason_to_jsx/1
]).

Expand Down Expand Up @@ -105,14 +104,9 @@ handle_schema_invalid(Info, State) ->

%% @doc Convert an error to a JSON string using jsx
-spec to_json(Error :: error()) -> binary().
to_json(Error) ->
to_json(Error, [indent]).

%% @doc Convert an error to a JSON string using jsx
-spec to_json(Error :: error(), JsxOptions :: [atom()]) -> binary().
to_json({error, Reasons}, JsxOptions) ->
to_json({error, Reasons}) ->
JsxReasons = lists:map(fun reason_to_jsx/1, Reasons),
jsx:encode([{reasons, JsxReasons}], JsxOptions).
?JSON:encode([{reasons, JsxReasons}]).

%% @doc Convert an error reason to jsx structs
-spec reason_to_jsx(Reason :: error_reason()) -> jesse:json_term().
Expand Down
14 changes: 14 additions & 0 deletions src/jesse_schema_validator.hrl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,20 @@
-define(IF_MAPS(Exp), Exp).
-endif.

%% Use new json library if available
-ifdef(OTP_RELEASE).
-if(?OTP_RELEASE >= 27).
%% OTP 27 or higher
-define(JSON, json).
-else.
%% OTP 26 to 21.
-define(JSON, jsx).
-endif.
-else.
%% OTP 20 or lower.
-define(JSON, jsx).
-endif.

%% Use optimization for sets if available
-ifdef(OTP_RELEASE).
-if(?OTP_RELEASE >= 24).
Expand Down
10 changes: 10 additions & 0 deletions src/jesse_validator_draft4.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1335,13 +1335,23 @@ remove_last_from_path(State) ->
jesse_state:remove_last_from_path(State).

%% @private
-ifdef(OTP_RELEASE).
%% OTP 21 or higher
valid_datetime(DateTimeBin) ->
try calendar:rfc3339_to_system_time(binary_to_list(DateTimeBin)) of
_ -> true
catch error:_Error -> false
end.
-else.
%% OTP 20 or lower.
valid_datetime(DateTimeBin) ->
case rfc3339:parse(DateTimeBin) of
{ok, _} ->
true;
_ ->
false
end.
-endif.

maybe_external_check_value(Value, State) ->
case jesse_state:get_external_validator(State) of
Expand Down
10 changes: 10 additions & 0 deletions src/jesse_validator_draft6.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1285,13 +1285,23 @@ remove_last_from_path(State) ->
jesse_state:remove_last_from_path(State).

%% @private
-ifdef(OTP_RELEASE).
%% OTP 21 or higher
valid_datetime(DateTimeBin) ->
try calendar:rfc3339_to_system_time(binary_to_list(DateTimeBin)) of
_ -> true
catch error:_Error -> false
end.
-else.
%% OTP 20 or lower.
valid_datetime(DateTimeBin) ->
case rfc3339:parse(DateTimeBin) of
{ok, _} ->
true;
_ ->
false
end.
-endif.

maybe_external_check_value(Value, State) ->
case jesse_state:get_external_validator(State) of
Expand Down