]> granicus.if.org Git - ejabberd/commitdiff
Improve error handling/reporting when loading language translations
authorEvgeny Khramtsov <ekhramtsov@process-one.net>
Wed, 10 Jul 2019 07:30:11 +0000 (10:30 +0300)
committerEvgeny Khramtsov <ekhramtsov@process-one.net>
Wed, 10 Jul 2019 07:30:11 +0000 (10:30 +0300)
Also speed up loading on multi-core machines

src/translate.erl

index 0d262ac89ba2313aeed6047f6a628b525a5201d2..562df38d8611e64b43a8b1b93ac73f638393d9cc 100644 (file)
@@ -39,6 +39,9 @@
 
 -define(ZERO_DATETIME, {{0,0,0}, {0,0,0}}).
 
+-type error_reason() :: file:posix() | {integer(), module(), term()} |
+                       badarg | terminated | system_limit | bad_file.
+
 -record(state, {}).
 
 start_link() ->
@@ -46,9 +49,13 @@ start_link() ->
 
 init([]) ->
     process_flag(trap_exit, true),
-    load(),
-    xmpp:set_tr_callback({?MODULE, translate}),
-    {ok, #state{}}.
+    case load() of
+       ok ->
+           xmpp:set_tr_callback({?MODULE, translate}),
+           {ok, #state{}};
+       {error, Reason} ->
+           {stop, Reason}
+    end.
 
 handle_call(_Request, _From, State) ->
     Reply = ok,
@@ -66,23 +73,25 @@ terminate(_Reason, _State) ->
 code_change(_OldVsn, State, _Extra) ->
     {ok, State}.
 
--spec reload() -> ok.
+-spec reload() -> ok | {error, error_reason()}.
 reload() ->
     load(true).
 
--spec load() -> ok.
+-spec load() -> ok | {error, error_reason()}.
 load() ->
     load(false).
 
--spec load(boolean()) -> ok.
+-spec load(boolean()) -> ok | {error, error_reason()}.
 load(ForceCacheRebuild) ->
     {MsgsDirMTime, MsgsDir} = get_msg_dir(),
     {CacheMTime, CacheFile} = get_cache_file(),
     {FilesMTime, MsgFiles} = get_msg_files(MsgsDir),
     LastModified = lists:max([MsgsDirMTime, FilesMTime]),
     if ForceCacheRebuild orelse CacheMTime < LastModified ->
-           load(MsgFiles, MsgsDir),
-           dump_to_file(CacheFile);
+           case load(MsgFiles, MsgsDir) of
+               ok -> dump_to_file(CacheFile);
+               Err -> Err
+           end;
        true ->
            case ets:file2tab(CacheFile) of
                {ok, _} ->
@@ -91,17 +100,18 @@ load(ForceCacheRebuild) ->
                    load(MsgFiles, MsgsDir);
                {error, {read_error, {file_error, _, Reason}}} ->
                    ?WARNING_MSG("Failed to read translation cache from ~s: ~s",
-                                [CacheFile, file:format_error(Reason)]),
+                                [unicode:characters_to_binary(CacheFile),
+                                 format_error(Reason)]),
                    load(MsgFiles, MsgsDir);
                {error, Reason} ->
                    ?WARNING_MSG("Failed to read translation cache from ~s: ~p",
-                                [CacheFile, Reason]),
+                                [unicode:characters_to_binary(CacheFile),
+                                 Reason]),
                    load(MsgFiles, MsgsDir)
            end
     end.
 
--spec load([file:filename()], file:filename()) -> ok.
-
+-spec load([file:filename()], file:filename()) -> ok | {error, error_reason()}.
 load(Files, Dir) ->
     try ets:new(translations, [named_table, public]) of
        _ -> ok
@@ -110,79 +120,43 @@ load(Files, Dir) ->
     case Files of
        [] ->
            ?WARNING_MSG("No translation files found in ~s, "
-                        "check directory access", [Dir]);
+                        "check directory access",
+                        [unicode:characters_to_binary(Dir)]);
        _ ->
-           ets:delete_all_objects(translations),
-           ?INFO_MSG("Building translation cache, this may take a while", []),
-           lists:foreach(
-             fun(File) ->
-                     BaseName = filename:basename(File),
-                     Lang = str:to_lower(filename:rootname(BaseName)),
-                     load_file(iolist_to_binary(Lang), File)
-             end, Files)
-    end.
-
-load_file(Lang, File) ->
-    case file:open(File, [read]) of
-        {ok, Fd} ->
-            case io:setopts(Fd, [{encoding,latin1}]) of
-               ok ->
-                   load_file_loop(Fd, 1, File, Lang),
-                   file:close(Fd);
-               {error, Error} ->
-                   ExitText = iolist_to_binary([File, ": ",
-                                         file:format_error(Error)]),
-                   ?ERROR_MSG("Problem loading translation file ~n~s",
-                              [ExitText]),
-                   exit(ExitText)
-           end;
-        {error, Error} ->
-            ExitText = iolist_to_binary([File, ": ",
-                                         file:format_error(Error)]),
-            ?ERROR_MSG("Problem loading translation file ~n~s",
-                       [ExitText]),
-            exit(ExitText)
+           ?INFO_MSG("Building language translation cache", []),
+           Objs = lists:flatten(misc:pmap(fun load_file/1, Files)),
+           case lists:keyfind(error, 1, Objs) of
+               false ->
+                   ets:delete_all_objects(translations),
+                   ets:insert(translations, Objs),
+                   ?DEBUG("Language translation cache built successfully", []);
+               {error, File, Reason} ->
+                   ?ERROR_MSG("Failed to read translation file ~s: ~s",
+                              [unicode:characters_to_binary(File),
+                               format_error(Reason)]),
+                   {error, Reason}
+           end
     end.
 
-load_file_loop(Fd, Line, File, Lang) ->
-    case io:read(Fd, '', Line) of
-        {ok,{Orig, Trans}, NextLine} ->
-            Trans1 = case Trans of
-                         <<"">> -> Orig;
-                         _ -> Trans
-                     end,
-            ets:insert(translations,
-                       {{Lang, iolist_to_binary(Orig)},
-                        iolist_to_binary(Trans1)}),
-
-            load_file_loop(Fd, NextLine, File, Lang);
-        {ok,_, _NextLine} ->
-            ExitText = iolist_to_binary([File,
-                                         " approximately in the line ",
-                                         Line]),
-            ?ERROR_MSG("Problem loading translation file ~n~s",
-                       [ExitText]),
-            exit(ExitText);
-        {error,
-         {_LineNumber, erl_parse, _ParseMessage} = Reason} ->
-            ExitText = iolist_to_binary([File,
-                                         " approximately in the line ",
-                                         file:format_error(Reason)]),
-            ?ERROR_MSG("Problem loading translation file ~n~s",
-                       [ExitText]),
-            exit(ExitText);
-        {error, Reason} ->
-            ExitText = iolist_to_binary([File, ": ",
-                                         file:format_error(Reason)]),
-            ?ERROR_MSG("Problem loading translation file ~n~s",
-                       [ExitText]),
-            exit(ExitText);
-        {eof,_Line} ->
-            ok
+-spec load_file(file:filename()) -> [{{binary(), binary()}, binary()} |
+                                    {error, file:filename(), error_reason()}].
+load_file(File) ->
+    Lang = lang_of_file(File),
+    case file:consult(File) of
+       {ok, Lines} ->
+           lists:map(
+             fun({In, Out}) ->
+                     InB = iolist_to_binary(In),
+                     OutB = iolist_to_binary(Out),
+                     {{Lang, InB}, OutB};
+                (_) ->
+                     {error, File, bad_file}
+             end, Lines);
+       {error, Reason} ->
+           [{error, File, Reason}]
     end.
 
 -spec translate(binary(), binary()) -> binary().
-
 translate(Lang, Msg) ->
     LLang = ascii_tolower(Lang),
     case ets:lookup(translations, {LLang, Msg}) of
@@ -203,6 +177,7 @@ translate(Lang, Msg) ->
          end
     end.
 
+-spec translate(binary()) -> binary().
 translate(Msg) ->
     case ejabberd_option:language() of
       <<"en">> -> Msg;
@@ -227,13 +202,15 @@ translate(Msg) ->
          end
     end.
 
-ascii_tolower(B) ->
-    iolist_to_binary(ascii_tolower_s(binary_to_list(B))).
-
-ascii_tolower_s([C | Cs]) when C >= $A, C =< $Z ->
-    [C + ($a - $A) | ascii_tolower_s(Cs)];
-ascii_tolower_s([C | Cs]) -> [C | ascii_tolower_s(Cs)];
-ascii_tolower_s([]) -> [].
+-spec ascii_tolower(list() | binary()) -> binary().
+ascii_tolower(B) when is_binary(B) ->
+    << <<(if X >= $A, X =< $Z ->
+                  X + 32;
+             true ->
+                  X
+          end)>> || <<X>> <= B >>;
+ascii_tolower(S) ->
+    ascii_tolower(unicode:characters_to_binary(S)).
 
 -spec get_msg_dir() -> {calendar:datetime(), file:filename()}.
 get_msg_dir() ->
@@ -243,7 +220,8 @@ get_msg_dir() ->
            {MTime, Dir};
        {error, Reason} ->
            ?ERROR_MSG("Failed to read directory ~s: ~s",
-                      [Dir, file:format_error(Reason)]),
+                      [unicode:characters_to_binary(Dir),
+                       format_error(Reason)]),
            {?ZERO_DATETIME, Dir}
     end.
 
@@ -252,12 +230,21 @@ get_msg_files(MsgsDir) ->
     Res = filelib:fold_files(
            MsgsDir, ".+\\.msg", false,
            fun(File, {MTime, Files} = Acc) ->
-                   case file:read_file_info(File) of
-                       {ok, #file_info{mtime = Time}} ->
-                           {lists:max([MTime, Time]), [File|Files]};
-                       {error, Reason} ->
-                           ?ERROR_MSG("Failed to read translation file ~s: ~s",
-                                      [File, file:format_error(Reason)]),
+                   case xmpp_lang:is_valid(lang_of_file(File)) of
+                       true ->
+                           case file:read_file_info(File) of
+                               {ok, #file_info{mtime = Time}} ->
+                                   {lists:max([MTime, Time]), [File|Files]};
+                               {error, Reason} ->
+                                   ?ERROR_MSG("Failed to read translation file ~s: ~s",
+                                              [unicode:characters_to_binary(File),
+                                               format_error(Reason)]),
+                                   Acc
+                           end;
+                       false ->
+                           ?WARNING_MSG("Ignoring translation file ~s: file name "
+                                        "must be a valid language tag",
+                                        [unicode:characters_to_binary(File)]),
                            Acc
                    end
            end, {?ZERO_DATETIME, []}),
@@ -267,7 +254,8 @@ get_msg_files(MsgsDir) ->
                {ok, _} -> ok;
                {error, Reason} ->
                    ?ERROR_MSG("Failed to read directory ~s: ~s",
-                              [MsgsDir, file:format_error(Reason)])
+                              [unicode:characters_to_binary(MsgsDir),
+                               format_error(Reason)])
            end;
        _ ->
            ok
@@ -290,5 +278,18 @@ dump_to_file(CacheFile) ->
        ok -> ok;
        {error, Reason} ->
            ?WARNING_MSG("Failed to create translation cache in ~s: ~p",
-                        [CacheFile, Reason])
+                        [unicode:characters_to_binary(CacheFile), Reason])
     end.
+
+-spec lang_of_file(file:filename()) -> binary().
+lang_of_file(FileName) ->
+    BaseName = filename:basename(FileName),
+    ascii_tolower(filename:rootname(BaseName)).
+
+-spec format_error(error_reason()) -> string().
+format_error(bad_file) ->
+    "corrupted or invalid translation file";
+format_error({_, _, _} = Reason) ->
+    "at line " ++ file:format_error(Reason);
+format_error(Reason) ->
+    file:format_error(Reason).