BearND has uploaded a new change for review. https://gerrit.wikimedia.org/r/311182
Change subject: Retrofit LoginTask ...................................................................... Retrofit LoginTask It seems easier to chain request implemented using Retrofit than the traditional java-mwapi requests. In the future we want to chain yet another request, in addition to the two we have already. One to get the login token. The 2nd one for the actual login request. This patch replaces the LoginTask with a Retrofit2 LoginClient. The test changes from LoginTaskTest to LoginClientTest. Change-Id: I6d9818c92bfd834adf966a6c5808b7c0a236235d --- R app/src/androidTest/java/org/wikipedia/test/LoginClientTest.java M app/src/main/java/org/wikipedia/editing/EditSectionActivity.java M app/src/main/java/org/wikipedia/login/LoginActivity.java A app/src/main/java/org/wikipedia/login/LoginClient.java M app/src/main/java/org/wikipedia/util/ThrowableUtil.java 5 files changed, 316 insertions(+), 77 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/apps/android/wikipedia refs/changes/82/311182/1 diff --git a/app/src/androidTest/java/org/wikipedia/test/LoginTaskTest.java b/app/src/androidTest/java/org/wikipedia/test/LoginClientTest.java similarity index 62% rename from app/src/androidTest/java/org/wikipedia/test/LoginTaskTest.java rename to app/src/androidTest/java/org/wikipedia/test/LoginClientTest.java index 32d3197..f130b2b 100644 --- a/app/src/androidTest/java/org/wikipedia/test/LoginTaskTest.java +++ b/app/src/androidTest/java/org/wikipedia/test/LoginClientTest.java @@ -1,47 +1,60 @@ package org.wikipedia.test; +import org.wikipedia.Site; +import org.wikipedia.WikipediaApp; +import org.wikipedia.editing.EditTokenStorage; +import org.wikipedia.login.LoginClient; +import org.wikipedia.login.LoginResult; +import org.wikipedia.login.User; +import org.wikipedia.testlib.TestLatch; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +import android.support.annotation.NonNull; import android.support.annotation.StringRes; import android.support.test.InstrumentationRegistry; import android.support.test.filters.SmallTest; import android.support.test.runner.AndroidJUnit4; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mediawiki.api.json.Api; -import org.wikipedia.Site; -import org.wikipedia.WikipediaApp; -import org.wikipedia.editing.EditTokenStorage; -import org.wikipedia.login.LoginResult; -import org.wikipedia.login.LoginTask; -import org.wikipedia.testlib.TestLatch; - +import static android.support.test.InstrumentationRegistry.getInstrumentation; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; -import static android.support.test.InstrumentationRegistry.getInstrumentation; @SmallTest @RunWith(AndroidJUnit4.class) -public class LoginTaskTest { +public class LoginClientTest { private static final String TEST_WIKI = "test.wikipedia.org"; private static final String USERNAME = getString(R.string.test_username); private static final String PASSWORD = getString(R.string.test_password); private final TestLatch completionLatch = new TestLatch(); + + @Before + public void setUp() throws Exception { + User.disableStorage(); // don't change the app login from this test + } @Test public void testLogin() { InstrumentationRegistry.getInstrumentation().runOnMainSync(new Runnable() { @Override public void run() { - new LoginTask(new Api(TEST_WIKI), USERNAME, PASSWORD) { - @Override - public void onFinish(LoginResult result) { - super.onFinish(result); - completionLatch.countDown(); - assertThat(result.getStatus(), equalTo("PASS")); - WikipediaApp.getInstance().getEditTokenStorage().get(new Site(TEST_WIKI), callback); - } - }.execute(); + new LoginClient().request(new Site(TEST_WIKI), USERNAME, PASSWORD, + new LoginClient.LoginCallback() { + @Override + public void success(@NonNull LoginResult result) { + completionLatch.countDown(); + assertThat(result.getStatus(), equalTo("PASS")); + WikipediaApp.getInstance().getEditTokenStorage().get(new Site(TEST_WIKI), callback); + } + + @Override + public void error(@NonNull Throwable caught) { + assertThat("login failed!", false); + } + }); } }); completionLatch.await(); diff --git a/app/src/main/java/org/wikipedia/editing/EditSectionActivity.java b/app/src/main/java/org/wikipedia/editing/EditSectionActivity.java index c06586b..b341e68 100644 --- a/app/src/main/java/org/wikipedia/editing/EditSectionActivity.java +++ b/app/src/main/java/org/wikipedia/editing/EditSectionActivity.java @@ -1,5 +1,31 @@ package org.wikipedia.editing; +import org.wikipedia.Constants; +import org.wikipedia.R; +import org.wikipedia.ViewAnimations; +import org.wikipedia.WikipediaApp; +import org.wikipedia.activity.ActivityUtil; +import org.wikipedia.activity.ThemedActionBarActivity; +import org.wikipedia.analytics.EditFunnel; +import org.wikipedia.analytics.LoginFunnel; +import org.wikipedia.editing.richtext.SyntaxHighlighter; +import org.wikipedia.editing.summaries.EditSummaryFragment; +import org.wikipedia.login.LoginActivity; +import org.wikipedia.login.LoginClient; +import org.wikipedia.login.LoginResult; +import org.wikipedia.login.User; +import org.wikipedia.page.LinkMovementMethodExt; +import org.wikipedia.page.PageProperties; +import org.wikipedia.page.PageTitle; +import org.wikipedia.util.FeedbackUtil; +import org.wikipedia.util.log.L; + +import org.mediawiki.api.json.Api; +import org.mediawiki.api.json.ApiException; +import org.mediawiki.api.json.RequestBuilder; + +import com.squareup.otto.Bus; + import android.app.ProgressDialog; import android.content.DialogInterface; import android.content.Intent; @@ -24,32 +50,9 @@ import android.widget.LinearLayout; import android.widget.ScrollView; import android.widget.TextView; -import com.squareup.otto.Bus; -import org.mediawiki.api.json.Api; -import org.mediawiki.api.json.ApiException; -import org.mediawiki.api.json.RequestBuilder; -import org.wikipedia.Constants; -import org.wikipedia.activity.ActivityUtil; -import org.wikipedia.login.LoginResult; -import org.wikipedia.login.LoginTask; -import org.wikipedia.page.PageTitle; -import org.wikipedia.R; -import org.wikipedia.activity.ThemedActionBarActivity; -import org.wikipedia.ViewAnimations; -import org.wikipedia.WikipediaApp; -import org.wikipedia.analytics.EditFunnel; -import org.wikipedia.analytics.LoginFunnel; -import org.wikipedia.editing.summaries.EditSummaryFragment; -import org.wikipedia.editing.richtext.SyntaxHighlighter; -import org.wikipedia.login.LoginActivity; -import org.wikipedia.login.User; -import org.wikipedia.page.LinkMovementMethodExt; -import org.wikipedia.page.PageProperties; -import org.wikipedia.util.FeedbackUtil; -import org.wikipedia.util.log.L; -import static org.wikipedia.util.L10nUtil.setConditionalTextDirection; import static org.wikipedia.util.DeviceUtil.hideSoftKeyboard; +import static org.wikipedia.util.L10nUtil.setConditionalTextDirection; import static org.wikipedia.util.UriUtil.handleExternalLink; public class EditSectionActivity extends ThemedActionBarActivity { @@ -433,18 +436,34 @@ } private void doLoginAndSave(final User user) { - new LoginTask(user.getUsername(), user.getPassword()) { - @Override - public void onFinish(LoginResult result) { - if (result.pass()) { - doSave(); - } else { - progressDialog.dismiss(); - ViewAnimations.crossFade(sectionText, sectionError); - sectionError.setVisibility(View.VISIBLE); - } - } - }.execute(); + new LoginClient().request(WikipediaApp.getInstance().getSite(), + user.getUsername(), + user.getPassword(), + new LoginClient.LoginCallback() { + @Override + public void success(@NonNull LoginResult result) { + if (result.pass()) { + doSave(); + } else { + onLoginError(); + } + } + + @Override + public void error(@NonNull Throwable caught) { + onLoginError(); + } + + private void onLoginError() { + if (!progressDialog.isShowing()) { + // no longer attached to activity! + return; + } + progressDialog.dismiss(); + ViewAnimations.crossFade(sectionText, sectionError); + sectionError.setVisibility(View.VISIBLE); + } + }); } private void handleAbuseFilter() { diff --git a/app/src/main/java/org/wikipedia/login/LoginActivity.java b/app/src/main/java/org/wikipedia/login/LoginActivity.java index da2bf7f..5d1bc47 100644 --- a/app/src/main/java/org/wikipedia/login/LoginActivity.java +++ b/app/src/main/java/org/wikipedia/login/LoginActivity.java @@ -40,11 +40,11 @@ private EditText usernameText; private EditText passwordText; private View loginButton; + private ProgressDialog progressDialog; private LoginFunnel funnel; private String loginSource; - - private ProgressDialog progressDialog; + private LoginClient loginClient; private boolean wentStraightToCreateAccount; public static Intent newIntent(@NonNull Context context, @NonNull String source) { @@ -172,25 +172,15 @@ private void doLogin() { final String username = usernameText.getText().toString(); final String password = passwordText.getText().toString(); - new LoginTask(username, password) { - @Override - public void onBeforeExecute() { - progressDialog.show(); - } + if (loginClient == null) { + loginClient = new LoginClient(); + } + progressDialog.show(); + loginClient.request(WikipediaApp.getInstance().getSite(), username, password, + new LoginClient.LoginCallback() { @Override - public void onCatch(Throwable caught) { - if (!progressDialog.isShowing()) { - // no longer attached to activity! - return; - } - progressDialog.dismiss(); - FeedbackUtil.showError(LoginActivity.this, caught); - } - - @Override - public void onFinish(LoginResult result) { - super.onFinish(result); + public void success(@NonNull LoginResult result) { if (!progressDialog.isShowing()) { // no longer attached to activity! return; @@ -214,7 +204,17 @@ handleError(result.getMessage()); } } - }.execute(); + + @Override + public void error(@NonNull Throwable caught) { + if (!progressDialog.isShowing()) { + // no longer attached to activity! + return; + } + progressDialog.dismiss(); + FeedbackUtil.showError(LoginActivity.this, caught); + } + }); } @Override diff --git a/app/src/main/java/org/wikipedia/login/LoginClient.java b/app/src/main/java/org/wikipedia/login/LoginClient.java new file mode 100644 index 0000000..8bd56ca --- /dev/null +++ b/app/src/main/java/org/wikipedia/login/LoginClient.java @@ -0,0 +1,203 @@ +package org.wikipedia.login; + +import org.wikipedia.Constants; +import org.wikipedia.Site; +import org.wikipedia.WikipediaApp; +import org.wikipedia.dataclient.mwapi.MwQueryResponse; +import org.wikipedia.dataclient.retrofit.MwCachedService; +import org.wikipedia.server.mwapi.MwServiceError; +import org.wikipedia.util.log.L; + +import com.google.gson.annotations.SerializedName; + +import retrofit2.Call; +import retrofit2.Callback; +import retrofit2.Response; +import retrofit2.http.Field; +import retrofit2.http.FormUrlEncoded; +import retrofit2.http.POST; + +import android.support.annotation.NonNull; +import android.support.annotation.Nullable; + +import java.io.IOException; + +/** + * Responsible for making login related requests to the server. + */ +public class LoginClient { + public static final String CLIENT_LOGIN = "clientlogin"; + public static final String JSON = "json"; + + @NonNull private final MwCachedService<Service> cachedService + = new MwCachedService<>(Service.class); + + @Nullable private Call<MwQueryResponse<LoginToken>> tokenCall; + @Nullable private Call<LoginResponse> loginCall; + + public interface LoginCallback { + void success(@NonNull LoginResult result); + void error(@NonNull Throwable caught); + } + + public void request(@NonNull final Site site, @NonNull final String userName, + @NonNull final String password, @NonNull final LoginCallback cb) { + cancel(); + + // HACK: T124384 + WikipediaApp.getInstance().getEditTokenStorage().clearAllTokens(); + + tokenCall = cachedService.service(site).requestLoginToken(); + tokenCall.enqueue(new Callback<MwQueryResponse<LoginToken>>() { + @Override + public void onResponse(Call<MwQueryResponse<LoginToken>> call, + Response<MwQueryResponse<LoginToken>> response) { + if (response.isSuccessful()) { + final MwQueryResponse<LoginToken> body = response.body(); + if (body.query() != null && body.query().getLoginToken() != null) { + login(site, userName, password, body.query().getLoginToken(), cb); + } else if (body.getError() != null) { + L.v(body.getError().toString()); + cb.error(new IOException("Failed to retrieve login token. " + + body.getError().toString())); + } else { + cb.error(new IOException("Unexpected error trying to retrieve login token. " + + body.toString())); + } + } else { + cb.error(new IOException(response.message())); + } + } + + @Override + public void onFailure(Call<MwQueryResponse<LoginToken>> call, Throwable caught) { + cb.error(caught); + } + }); + } + + private void login(@NonNull Site site, @NonNull String userName, @NonNull final String password, + @NonNull String loginToken, final LoginCallback cb) { + loginCall = cachedService.service(site).logIn(CLIENT_LOGIN, JSON, userName, password, + loginToken, Constants.WIKIPEDIA_URL); + loginCall.enqueue(new Callback<LoginResponse>() { + @Override + public void onResponse(Call<LoginResponse> call, Response<LoginResponse> response) { + if (response.isSuccessful()) { + LoginResponse loginResponse = response.body(); + LoginResult loginResult = loginResponse.toLoginResult(password); + if (loginResult != null) { + if (loginResult.pass()) { + User.setUser(loginResult.getUser()); + cb.success(loginResult); + } else { + cb.error(new LoginFailedException(loginResult.getMessage())); + } + } else { + cb.error(new IOException("Login failed. Unexpected response.")); + } + } else { + // very unlikely to happen because MW API responds with 200 for failures, too. + cb.error(new IOException("Login failed. Unexpected response.")); + } + } + + @Override + public void onFailure(Call<LoginResponse> call, Throwable t) { + cb.error(t); + } + }); + } + + public void cancel() { + cancelTokenRequest(); + cancelLogin(); + } + + private void cancelTokenRequest() { + if (tokenCall == null) { + return; + } + tokenCall.cancel(); + tokenCall = null; + } + + private void cancelLogin() { + if (loginCall == null) { + return; + } + loginCall.cancel(); + loginCall = null; + } + + private interface Service { + + /** Request a login token to be used later to log in. */ + @NonNull + @POST("w/api.php?format=json&formatversion=2&action=query" + + "&meta=tokens&type=login") + Call<MwQueryResponse<LoginToken>> requestLoginToken(); + + /** Actually log in. Has to be x-www-form-urlencoded */ + @NonNull + @FormUrlEncoded + @POST("w/api.php") + Call<LoginResponse> logIn(@Field("action") String action, @Field("format") String format, + @Field("username") String user, @Field("password") String pass, + @Field("logintoken") String token, + @Field("loginreturnurl") String url); + } + + private static final class LoginToken { + @SerializedName("tokens") private Tokens tokens; + + @Nullable public String getLoginToken() { + return tokens == null ? null : tokens.loginToken; + } + + private class Tokens { + @SerializedName("logintoken") @Nullable + private String loginToken; + } + } + + private static final class LoginResponse { + @SerializedName("error") @Nullable + private MwServiceError error; + + @SerializedName("clientlogin") @Nullable + private ClientLogin clientLogin; + + @Nullable + public MwServiceError getError() { + return error; + } + + public LoginResult toLoginResult(String password) { + return clientLogin != null ? clientLogin.toLoginResult(password) : null; + } + + private static class ClientLogin { + @SerializedName("status") private String status; + @SerializedName("message") @Nullable private String message; + @SerializedName("username") @Nullable private String userName; + + public LoginResult toLoginResult(String password) { + User user = null; + String userMessage = null; + if ("PASS".equals(status)) { + user = new User(userName, password, 0); + } else if ("FAIL".equals(status)) { + userMessage = message; + } + return new LoginResult(status, user, userMessage); + } + } + } + + public class LoginFailedException extends Throwable { + public LoginFailedException(String message) { + super(message); + } + } +} diff --git a/app/src/main/java/org/wikipedia/util/ThrowableUtil.java b/app/src/main/java/org/wikipedia/util/ThrowableUtil.java index e7eac0f..3bc71f5 100644 --- a/app/src/main/java/org/wikipedia/util/ThrowableUtil.java +++ b/app/src/main/java/org/wikipedia/util/ThrowableUtil.java @@ -1,6 +1,8 @@ package org.wikipedia.util; import org.wikipedia.R; +import org.wikipedia.login.LoginClient; + import org.mediawiki.api.json.ApiException; import org.json.JSONException; @@ -53,6 +55,8 @@ result = new AppError(context.getString(R.string.error_network_error), context.getString(R.string.format_error_server_message, inner.getLocalizedMessage())); + } else if (inner instanceof LoginClient.LoginFailedException) { + result = new AppError(inner.getLocalizedMessage(), ""); } else if (ThrowableUtil.throwableContainsException(e, JSONException.class)) { // it's a json exception result = new AppError(context.getString(R.string.error_response_malformed), -- To view, visit https://gerrit.wikimedia.org/r/311182 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6d9818c92bfd834adf966a6c5808b7c0a236235d Gerrit-PatchSet: 1 Gerrit-Project: apps/android/wikipedia Gerrit-Branch: master Gerrit-Owner: BearND <bsitzm...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits