From 100f2e9a131bacec7c232790fff6a85ca8ce16fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20K=C3=B6hler?= Date: Fri, 5 Nov 2010 18:32:25 +0100 Subject: [PATCH] Refactor mod_last to use the same core get_last/2 functionality, but keep api stable The local function get_last/4 has been renamed to get_last_iq/4, since it converts the result of get_last/2 (typically {ok, TimeStamp, Status}) to an iq packet. --- src/mod_last.erl | 35 +++++++++++++++---------- src/mod_last_odbc.erl | 60 ++++++++++++++++++++++--------------------- 2 files changed, 52 insertions(+), 43 deletions(-) diff --git a/src/mod_last.erl b/src/mod_last.erl index d343f18e4..b5bca2cd4 100644 --- a/src/mod_last.erl +++ b/src/mod_last.erl @@ -129,7 +129,7 @@ process_sm_iq(From, To, #iq{type = Type, sub_el = SubEl} = IQ) -> {xmlelement, "presence", [], []}}, out]) of allow -> - get_last(IQ, SubEl, User, Server); + get_last_iq(IQ, SubEl, User, Server); deny -> IQ#iq{type = error, sub_el = [SubEl, ?ERR_FORBIDDEN]} @@ -140,14 +140,25 @@ process_sm_iq(From, To, #iq{type = Type, sub_el = SubEl} = IQ) -> end end. -%% TODO: This function could use get_last_info/2 -get_last(IQ, SubEl, LUser, LServer) -> +%% @spec (LUser::string(), LServer::string()) -> +%% {ok, TimeStamp::integer(), Status::string()} | not_found | {error, Reason} +get_last(LUser, LServer) -> case catch mnesia:dirty_read(last_activity, {LUser, LServer}) of - {'EXIT', _Reason} -> - IQ#iq{type = error, sub_el = [SubEl, ?ERR_INTERNAL_SERVER_ERROR]}; + {'EXIT', Reason} -> + {error, Reason}; [] -> - IQ#iq{type = error, sub_el = [SubEl, ?ERR_SERVICE_UNAVAILABLE]}; + not_found; [#last_activity{timestamp = TimeStamp, status = Status}] -> + {ok, TimeStamp, Status} + end. + +get_last_iq(IQ, SubEl, LUser, LServer) -> + case get_last(LUser, LServer) of + {error, _Reason} -> + IQ#iq{type = error, sub_el = [SubEl, ?ERR_INTERNAL_SERVER_ERROR]}; + not_found -> + IQ#iq{type = error, sub_el = [SubEl, ?ERR_SERVICE_UNAVAILABLE]}; + {ok, TimeStamp, Status} -> TimeStamp2 = now_to_seconds(now()), Sec = TimeStamp2 - TimeStamp, IQ#iq{type = result, @@ -157,8 +168,6 @@ get_last(IQ, SubEl, LUser, LServer) -> [{xmlcdata, Status}]}]} end. - - on_presence_update(User, Server, _Resource, Status) -> TimeStamp = now_to_seconds(now()), store_last_info(User, Server, TimeStamp, Status). @@ -177,13 +186,11 @@ store_last_info(User, Server, TimeStamp, Status) -> %% @spec (LUser::string(), LServer::string()) -> %% {ok, TimeStamp::integer(), Status::string()} | not_found get_last_info(LUser, LServer) -> - case catch mnesia:dirty_read(last_activity, {LUser, LServer}) of - {'EXIT', _Reason} -> + case get_last(LUser, LServer) of + {error, _Reason} -> not_found; - [] -> - not_found; - [#last_activity{timestamp = TimeStamp, status = Status}] -> - {ok, TimeStamp, Status} + Res -> + Res end. remove_user(User, Server) -> diff --git a/src/mod_last_odbc.erl b/src/mod_last_odbc.erl index 202508bb3..7a806e28f 100644 --- a/src/mod_last_odbc.erl +++ b/src/mod_last_odbc.erl @@ -121,7 +121,7 @@ process_sm_iq(From, To, #iq{type = Type, sub_el = SubEl} = IQ) -> {xmlelement, "presence", [], []}}, out]) of allow -> - get_last(IQ, SubEl, User, Server); + get_last_iq(IQ, SubEl, User, Server); deny -> IQ#iq{type = error, sub_el = [SubEl, ?ERR_FORBIDDEN]} @@ -132,28 +132,38 @@ process_sm_iq(From, To, #iq{type = Type, sub_el = SubEl} = IQ) -> end end. -%% TODO: This function could use get_last_info/2 -get_last(IQ, SubEl, LUser, LServer) -> +%% @spec (LUser::string(), LServer::string()) -> +%% {ok, TimeStamp::integer(), Status::string()} | not_found | {error, Reason} +get_last(LUser, LServer) -> Username = ejabberd_odbc:escape(LUser), case catch odbc_queries:get_last(LServer, Username) of {selected, ["seconds","state"], []} -> - IQ#iq{type = error, sub_el = [SubEl, ?ERR_SERVICE_UNAVAILABLE]}; + not_found; {selected, ["seconds","state"], [{STimeStamp, Status}]} -> case catch list_to_integer(STimeStamp) of TimeStamp when is_integer(TimeStamp) -> - TimeStamp2 = now_to_seconds(now()), - Sec = TimeStamp2 - TimeStamp, - IQ#iq{type = result, - sub_el = [{xmlelement, "query", - [{"xmlns", ?NS_LAST}, - {"seconds", integer_to_list(Sec)}], - [{xmlcdata, Status}]}]}; - _ -> - IQ#iq{type = error, - sub_el = [SubEl, ?ERR_INTERNAL_SERVER_ERROR]} + {ok, TimeStamp, Status}; + Reason -> + {error, {invalid_timestamp, Reason}} end; - _ -> - IQ#iq{type = error, sub_el = [SubEl, ?ERR_INTERNAL_SERVER_ERROR]} + Reason -> + {error, {invalid_result, Reason}} + end. + +get_last_iq(IQ, SubEl, LUser, LServer) -> + case get_last(LUser, LServer) of + {error, _Reason} -> + IQ#iq{type = error, sub_el = [SubEl, ?ERR_INTERNAL_SERVER_ERROR]}; + not_found -> + IQ#iq{type = error, sub_el = [SubEl, ?ERR_SERVICE_UNAVAILABLE]}; + {ok, TimeStamp, Status} -> + TimeStamp2 = now_to_seconds(now()), + Sec = TimeStamp2 - TimeStamp, + IQ#iq{type = result, + sub_el = [{xmlelement, "query", + [{"xmlns", ?NS_LAST}, + {"seconds", integer_to_list(Sec)}], + [{xmlcdata, Status}]}]} end. on_presence_update(User, Server, _Resource, Status) -> @@ -169,21 +179,13 @@ store_last_info(User, Server, TimeStamp, Status) -> odbc_queries:set_last_t(LServer, Username, Seconds, State). %% @spec (LUser::string(), LServer::string()) -> -%% {ok, Timestamp::integer(), Status::string()} | not_found +%% {ok, TimeStamp::integer(), Status::string()} | not_found get_last_info(LUser, LServer) -> - Username = ejabberd_odbc:escape(LUser), - case catch odbc_queries:get_last(LServer, Username) of - {selected, ["seconds","state"], []} -> + case get_last(LUser, LServer) of + {error, _Reason} -> not_found; - {selected, ["seconds","state"], [{STimeStamp, Status}]} -> - case catch list_to_integer(STimeStamp) of - TimeStamp when is_integer(TimeStamp) -> - {ok, TimeStamp, Status}; - _ -> - not_found - end; - _ -> - not_found + Res -> + Res end. remove_user(User, Server) ->