fix(erlang): post-review cleanups — pread, self-contained tests, poolboy app dep, dead docs

binding/erlang/rebar.config

@@ -1,6 +1,6 @@
 {erl_opts, [
-    debug_info, 
-    export_all, 
+    debug_info,
+    export_all,
     nowarn_export_all
 ]}.
 
@@ -18,7 +18,7 @@
 {ex_doc, [
     {extras, ["README.md"]},
     {main, "README.md"},
-    {source_url, "https://github.com/leihua996/ip2region/tree/master/binding/erlang"}
+    {source_url, "https://github.com/lionsoul2014/ip2region/tree/master/binding/erlang"}
 ]}.
 
 {hex, [{doc, ex_doc}]}.

binding/erlang/src/ip2region.app.src

@@ -5,7 +5,8 @@
   {mod, {ip2region_app, []}},
   {applications,
    [kernel,
-    stdlib
+    stdlib,
+    poolboy
    ]},
   {env,[
     {poolargs, [
@@ -19,5 +20,5 @@
   {modules, []},
 
   {licenses, ["Apache-2.0"]},
-  {links, [{"Github", "https://github.com/leihua996/ip2region/tree/master/binding/erlang"}]}
+  {links, [{"Github", "https://github.com/lionsoul2014/ip2region/tree/master/binding/erlang"}]}
  ]}.

binding/erlang/src/ip2region_sup.erl

@@ -67,12 +67,12 @@ pool_child_specs() ->
                 Acc
         end, [], DbConfig).
 
-make_pool_spec(PoolName, Version, File, PoolArgsCfg) ->
+make_pool_spec(PoolName, _Version, File, PoolArgsCfg) ->
     PoolArgs = [
         {strategy, fifo},
         {name, {local, PoolName}},
         {worker_module, ip2region_worker}
         | PoolArgsCfg
     ],
-    WorkerArgs = [{xdb_file, File}, {version, Version}],
+    WorkerArgs = [{xdb_file, File}],
     poolboy:child_spec(PoolName, PoolArgs, WorkerArgs).

binding/erlang/src/ip2region_worker.erl

@@ -124,7 +124,7 @@ do_call(stop, _From, State) ->
 
 do_call(Request, From, State) ->
     error_logger:error_report(io_lib:format("unknown request: ~p, from:~p", [Request, From])),
-    {noreply, State}.
+    {reply, {error, unknown_request}, State}.
 
 do_cast(Msg, State) ->
     error_logger:error_report(io_lib:format("unknown msg: ~p", [Msg])),
@@ -154,14 +154,15 @@ load_vector_index(IoDevice, Version) ->
     %% so the current file pointer position does not affect correctness.
     Table = ip2region_xdb:vector_index_table(Version),
     case ets:info(Table, size) of
+        ?XDB_VECTOR_INDEX_COUNT ->
+            ok;
         undefined ->
             Opts = [named_table, set, public, {read_concurrency, true}, {keypos, 1}],
             ets:new(Table, Opts),
             load_vector_index_data(IoDevice, Table);
-        0 ->
-            load_vector_index_data(IoDevice, Table);
         _ ->
-            ok
+            %% A previous worker may have crashed midway; reload to be safe.
+            load_vector_index_data(IoDevice, Table)
     end.
 
 load_vector_index_data(IoDevice, Table) ->
@@ -215,8 +216,7 @@ ip_in_range(Ip, SIp, EIp, ipv6) ->
     end.
 
 read_file(IoDevice, Position, DataLength) ->
-    file:position(IoDevice, {bof, Position}),
-    file:read(IoDevice, DataLength).
+    file:pread(IoDevice, Position, DataLength).
 
 read_segment_index(IoDevice, SPtr, SegSize, SegmentTable) ->
     case ets:lookup(SegmentTable, SPtr) of

binding/erlang/src/xdb_benchmark.erl

@@ -14,7 +14,7 @@ main(DataFile) ->
     %% Keep benchmark output clean while still surfacing real errors.
     _ = logger:set_handler_config(default, level, error),
     _ = logger:set_primary_config(level, error),
-    application:ensure_started(ip2region),
+    {ok, _} = application:ensure_all_started(ip2region),
     show_hw_sw_info(),
     IpList = load_test_data(DataFile),
     run(IpList).

binding/erlang/test/ip2region_sup_test.erl

@@ -3,8 +3,8 @@
 -include("ip2region.hrl").
 
 pools_started_test() ->
-    application:stop(ip2region),
-    application:unload(ip2region),
+    try application:stop(ip2region) catch _:_ -> ok end,
+    try application:unload(ip2region) catch _:_ -> ok end,
     ok = application:load(ip2region),
     %% NOTE: rebar3 compiles tests under _build/test/lib/ip2region via a
     %% symlink back to the source tree, so ?FILE resolves to the original
@@ -19,6 +19,6 @@ pools_started_test() ->
         {ipv4, "ip2region.xdb"},
         {ipv6, V6File}
     ]),
-    application:ensure_started(ip2region),
+    {ok, _} = application:ensure_all_started(ip2region),
     ?assert(is_pid(whereis(?IP2REGION_POOL_V4))),
     ?assert(is_pid(whereis(?IP2REGION_POOL_V6))).

binding/erlang/test/xdb_test.erl

@@ -2,8 +2,10 @@
 
 -include_lib("eunit/include/eunit.hrl").
 
+-define(IPV6_RESULT, "United States|Florida|Miami|Google LLC|US").
+
 search_test_() ->
-    application:ensure_started(ip2region),
+    {ok, _} = application:ensure_all_started(ip2region),
     A = "中国|广东省|广州市|中国电信|CN",
     Region0 = xdb:search("1.0.8.0"),
     Region1 = xdb:search(<<"1.0.8.0">>),
@@ -17,17 +19,35 @@ search_test_() ->
     ].
 
 ipv6_search_test_() ->
-    application:ensure_started(ip2region),
+    setup_ipv6(),
     [
-        ?_assert(is_list(xdb:search("2001:4860:4860::8888"))),
-        ?_assert(is_list(xdb:search(<<"2001:4860:4860::8888">>))),
-        ?_assert(is_list(xdb:search({8193, 10304, 10304, 0, 0, 0, 0, 34952})))
+        ?_assert(?IPV6_RESULT =:= xdb:search("2001:4860:4860::8888")),
+        ?_assert(?IPV6_RESULT =:= xdb:search(<<"2001:4860:4860::8888">>)),
+        ?_assert(?IPV6_RESULT =:= xdb:search({8193, 18528, 18528, 0, 0, 0, 0, 34952}))
     ].
 
 invalid_search_test_() ->
-    application:ensure_started(ip2region),
+    {ok, _} = application:ensure_all_started(ip2region),
     [
         ?_assertEqual({error, bad_ip_format}, xdb:search("xxx.0.8.0")),
         ?_assertEqual({error, bad_ip_format}, xdb:search("::ggg")),
         ?_assertEqual({error, bad_ip_format}, xdb:search({1,2,3}))
-    ].
+    ].
+
+setup_ipv6() ->
+    try application:stop(ip2region) catch _:_ -> ok end,
+    try application:unload(ip2region) catch _:_ -> ok end,
+    ok = application:load(ip2region),
+    RepoRoot = repo_root(),
+    V6File = filename:join([RepoRoot, "data", "ip2region_v6.xdb"]),
+    ok = application:set_env(ip2region, db, [
+        {ipv4, "ip2region.xdb"},
+        {ipv6, V6File}
+    ]),
+    {ok, _} = application:ensure_all_started(ip2region).
+
+repo_root() ->
+    TestDir = filename:dirname(?FILE),
+    ErlangDir = filename:dirname(TestDir),
+    BindingDir = filename:dirname(ErlangDir),
+    filename:dirname(BindingDir).