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.

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) ->