Migrated isUserBlockedFromCommons to retrofit (#3085)

* Switched wikimedia-android-data-client for new features

* Added UserCLient and UserInterface

* Migrated isUserBlockedFromCommons to retrofit

* Added tests and removed dead code

* Implemented ban checking functionality in UploadActivity

* Removed unused class AuthenticatedActivity

* Fixed tests

* Fixed NullPointerException when a user accessed image details without logging in.

app/src/main/java/fr/free/nrw/commons/auth/AuthenticatedActivity.java

@@ -1,40 +0,0 @@
-package fr.free.nrw.commons.auth;
-
-import android.os.Bundle;
-
-import javax.inject.Inject;
-
-import fr.free.nrw.commons.R;
-import fr.free.nrw.commons.mwapi.MediaWikiApi;
-import fr.free.nrw.commons.theme.NavigationBaseActivity;
-import fr.free.nrw.commons.utils.ViewUtil;
-import io.reactivex.Observable;
-import io.reactivex.android.schedulers.AndroidSchedulers;
-import io.reactivex.schedulers.Schedulers;
-
-public abstract class AuthenticatedActivity extends NavigationBaseActivity {
-
-    @Inject
-    protected SessionManager sessionManager;
-    @Inject
-    MediaWikiApi mediaWikiApi;
-
-    @Override
-    protected void onCreate(Bundle savedInstanceState) {
-        super.onCreate(savedInstanceState);
-        showBlockStatus();
-    }
-
-    /**
-     * Makes API call to check if user is blocked from Commons. If the user is blocked, a snackbar
-     * is created to notify the user
-     */
-    protected void showBlockStatus() {
-        compositeDisposable.add(Observable.fromCallable(() -> mediaWikiApi.isUserBlockedFromCommons())
-                .subscribeOn(Schedulers.io())
-                .observeOn(AndroidSchedulers.mainThread())
-                .filter(result -> result)
-                .subscribe(result -> ViewUtil.showShortSnackbar(findViewById(android.R.id.content), R.string.block_notification)
-                ));
-    }
-}

app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.java

@@ -15,6 +15,7 @@ import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import javax.inject.Named;
 import javax.inject.Singleton;
@@ -194,6 +195,6 @@ public class CommonsApplicationModule {
     @Named("username")
     @Provides
     public String provideLoggedInUsername() {
-        return AppAdapter.get().getUserName();
+        return Objects.toString(AppAdapter.get().getUserName(), "");
     }
 }

app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java

@@ -230,6 +230,7 @@ public class NetworkingModule {
     public CategoryInterface provideCategoryInterface(@Named(NAMED_COMMONS_WIKI_SITE) WikiSite commonsWikiSite) {
         return ServiceFactory.get(commonsWikiSite, BuildConfig.COMMONS_URL, CategoryInterface.class);
     }
+
     @Provides
     @Singleton
     public UserInterface provideUserInterface(@Named(NAMED_COMMONS_WIKI_SITE) WikiSite commonsWikiSite) {

app/src/main/java/fr/free/nrw/commons/mwapi/ApacheHttpClientMediaWikiApi.java

@@ -49,47 +49,6 @@ public class ApacheHttpClientMediaWikiApi implements MediaWikiApi {
         api = new CustomMwApi(apiURL, httpClient);
     }
 
-    /**
-     * Checks to see if a user is currently blocked from Commons
-     *
-     * @return whether or not the user is blocked from Commons
-     */
-    @Override
-    public boolean isUserBlockedFromCommons() {
-        boolean userBlocked = false;
-        try {
-            CustomApiResult result = api.action("query")
-                    .param("action", "query")
-                    .param("format", "xml")
-                    .param("meta", "userinfo")
-                    .param("uiprop", "blockinfo")
-                    .get();
-            if (result != null) {
-                String blockEnd = result.getString("/api/query/userinfo/@blockexpiry");
-                if (blockEnd.equals("infinite")) {
-                    userBlocked = true;
-                } else if (!blockEnd.isEmpty()) {
-                    Date endDate = parseMWDate(blockEnd);
-                    Date current = new Date();
-                    userBlocked = endDate.after(current);
-                }
-
-            }
-        } catch (Exception e) {
-            e.printStackTrace();
-        }
-
-        return userBlocked;
-    }
-
-    private Date parseMWDate(String mwDate) {
-        try {
-            return DateUtil.iso8601DateParse(mwDate);
-        } catch (ParseException e) {
-            throw new RuntimeException(e);
-        }
-    }
-
     /**
      * Calls media wiki's logout API
      */

app/src/main/java/fr/free/nrw/commons/mwapi/MediaWikiApi.java

@@ -9,8 +9,6 @@ import io.reactivex.Single;
 
 public interface MediaWikiApi {
 
-    boolean isUserBlockedFromCommons();
-
     void logout();
 
 //    Single<CampaignResponseDTO> getCampaigns();

app/src/main/java/fr/free/nrw/commons/mwapi/UserClient.java

@@ -1,15 +1,23 @@
 package fr.free.nrw.commons.mwapi;
 
-import org.wikipedia.dataclient.mwapi.MwQueryLogEvent;
-import org.wikipedia.dataclient.mwapi.MwQueryResponse;
-import org.wikipedia.dataclient.mwapi.MwQueryResult;
-
 import java.util.Collections;
 
 import javax.inject.Inject;
 
 import io.reactivex.Observable;
 
+import org.wikipedia.dataclient.mwapi.MwQueryLogEvent;
+import org.wikipedia.dataclient.mwapi.MwQueryResponse;
+import org.wikipedia.dataclient.mwapi.MwQueryResult;
+import org.wikipedia.dataclient.mwapi.UserInfo;
+import org.wikipedia.util.DateUtil;
+
+import java.util.Date;
+
+import javax.inject.Inject;
+
+import io.reactivex.Single;
+
 public class UserClient {
     private final UserInterface userInterface;
 
@@ -18,6 +26,29 @@ public class UserClient {
         this.userInterface = userInterface;
     }
 
+    /**
+     * Checks to see if a user is currently blocked from Commons
+     *
+     * @return whether or not the user is blocked from Commons
+     */
+    public Single<Boolean> isUserBlockedFromCommons() {
+        return userInterface.getUserBlockInfo()
+                .map(MwQueryResponse::query)
+                .map(MwQueryResult::userInfo)
+                .map(UserInfo::blockexpiry)
+                .map(blockExpiry -> {
+                    if (blockExpiry.isEmpty())
+                        return false;
+                    else if ("infinite".equals(blockExpiry))
+                        return true;
+                    else {
+                        Date endDate = DateUtil.iso8601DateParse(blockExpiry);
+                        Date current = new Date();
+                        return endDate.after(current);
+                    }
+                }).single(false);
+    }
+
     public Observable<MwQueryLogEvent> logEvents(String user) {
         try {
             return userInterface.getUserLogEvents(user, Collections.emptyMap())

app/src/main/java/fr/free/nrw/commons/mwapi/UserInterface.java

@@ -12,6 +12,20 @@ import retrofit2.http.QueryMap;
 import static org.wikipedia.dataclient.Service.MW_API_PREFIX;
 
 public interface UserInterface {
+
+    /**
+     * Gets the log events of user
+     * @param user name of user without prefix
+     * @param continuation continuation params returned in previous query
+     * @return query response
+     */
+
     @GET(MW_API_PREFIX+"action=query&list=logevents&letype=upload&leprop=title|timestamp|ids&lelimit=500")
     Observable<MwQueryResponse> getUserLogEvents(@Query("leuser") String user, @QueryMap Map<String, String> continuation);
+
+    /**
+     * Checks to see if a user is currently blocked from Commons
+     */
+    @GET(MW_API_PREFIX + "action=query&meta=userinfo&uiprop=blockinfo")
+    Observable<MwQueryResponse> getUserBlockInfo();
 }

app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java

@@ -9,6 +9,7 @@ import android.annotation.SuppressLint;
 import android.app.ProgressDialog;
 import android.content.Intent;
 import android.os.Bundle;
+
 import androidx.appcompat.app.AlertDialog;
 import androidx.cardview.widget.CardView;
 import androidx.fragment.app.Fragment;
@@ -25,6 +26,7 @@ import android.widget.ImageButton;
 import android.widget.LinearLayout;
 import android.widget.RelativeLayout;
 import android.widget.TextView;
+
 import java.util.ArrayList;
 import java.util.List;
 
@@ -43,6 +45,7 @@ import fr.free.nrw.commons.contributions.Contribution;
 import fr.free.nrw.commons.contributions.ContributionController;
 import fr.free.nrw.commons.filepicker.UploadableFile;
 import fr.free.nrw.commons.kvstore.JsonKvStore;
+import fr.free.nrw.commons.mwapi.UserClient;
 import fr.free.nrw.commons.nearby.Place;
 import fr.free.nrw.commons.theme.BaseActivity;
 import fr.free.nrw.commons.upload.categories.UploadCategoriesFragment;
@@ -51,17 +54,29 @@ import fr.free.nrw.commons.upload.mediaDetails.UploadMediaDetailFragment;
 import fr.free.nrw.commons.upload.mediaDetails.UploadMediaDetailFragment.UploadMediaDetailFragmentCallback;
 import fr.free.nrw.commons.utils.PermissionUtils;
 import fr.free.nrw.commons.utils.ViewUtil;
+import io.reactivex.android.schedulers.AndroidSchedulers;
 import io.reactivex.disposables.CompositeDisposable;
+
 import java.util.Collections;
+
+import io.reactivex.schedulers.Schedulers;
 import timber.log.Timber;
 
 public class UploadActivity extends BaseActivity implements UploadContract.View, UploadBaseFragment.Callback {
     @Inject
     ContributionController contributionController;
-    @Inject @Named("default_preferences") JsonKvStore directKvStore;
+    @Inject
-    @Inject UploadContract.UserActionListener presenter;
+    @Named("default_preferences")
-    @Inject CategoriesModel categoriesModel;
+    JsonKvStore directKvStore;
-    @Inject SessionManager sessionManager;
+    @Inject
+    UploadContract.UserActionListener presenter;
+    @Inject
+    CategoriesModel categoriesModel;
+    @Inject
+    SessionManager sessionManager;
+    @Inject
+    UserClient userClient;
+
 
     @BindView(R.id.cv_container_top_card)
     CardView cvContainerTopCard;
@@ -179,9 +194,24 @@ public class UploadActivity extends BaseActivity implements UploadContract.View
         if (!isLoggedIn()) {
             askUserToLogIn();
         }
+        checkBlockStatus();
         checkStoragePermissions();
     }
 
+    /**
+     * Makes API call to check if user is blocked from Commons. If the user is blocked, a snackbar
+     * is created to notify the user
+     */
+    protected void checkBlockStatus() {
+        compositeDisposable.add(userClient.isUserBlockedFromCommons()
+                .subscribeOn(Schedulers.io())
+                .observeOn(AndroidSchedulers.mainThread())
+                .filter(result -> result)
+                .subscribe(result -> showInfoAlert(R.string.block_notification_title,
+                        R.string.block_notification, UploadActivity.this::finish)
+                ));
+    }
+
     private void checkStoragePermissions() {
         PermissionUtils.checkPermissionsAndPerformAction(this,
                 Manifest.permission.WRITE_EXTERNAL_STORAGE,
@@ -383,12 +413,15 @@ public class UploadActivity extends BaseActivity implements UploadContract.View
         finish();
     }
 
-    private void showInfoAlert(int titleStringID, int messageStringId, String... formatArgs) {
+    private void showInfoAlert(int titleStringID, int messageStringId, Runnable positive, String... formatArgs) {
         new AlertDialog.Builder(this)
                 .setTitle(titleStringID)
                 .setMessage(getString(messageStringId, (Object[]) formatArgs))
                 .setCancelable(true)
-                .setPositiveButton(android.R.string.ok, (dialog, id) -> dialog.cancel())
+                .setPositiveButton(android.R.string.ok, (dialog, id) -> {
+                    positive.run();
+                    dialog.cancel();
+                })
                 .create()
                 .show();
     }
@@ -426,11 +459,13 @@ public class UploadActivity extends BaseActivity implements UploadContract.View
             notifyDataSetChanged();
         }
 
-        @Override public Fragment getItem(int position) {
+        @Override
+        public Fragment getItem(int position) {
             return fragments.get(position);
         }
 
-        @Override public int getCount() {
+        @Override
+        public int getCount() {
             return fragments.size();
         }
 

app/src/main/res/values/strings.xml

@@ -316,6 +316,7 @@
   <string name="error_loading_images">Error occurred while loading images.</string>
   <string name="image_uploaded_by">Uploaded by: %1$s</string>
 
+  <string name="block_notification_title">Blocked</string>
   <string name="block_notification">You are blocked from editing Commons</string>
   <string name="appwidget_img">Pic of the Day</string>
   <string name="app_widget_heading">Pic of the Day</string>

app/src/test/kotlin/fr/free/nrw/commons/mwapi/ApacheHttpClientMediaWikiApiTest.kt

@@ -1,147 +0,0 @@
-package fr.free.nrw.commons.mwapi
-
-import android.os.Build
-import androidx.test.core.app.ApplicationProvider
-import fr.free.nrw.commons.TestCommonsApplication
-import fr.free.nrw.commons.kvstore.JsonKvStore
-import fr.free.nrw.commons.utils.ConfigUtils
-import okhttp3.OkHttpClient
-import okhttp3.mockwebserver.MockResponse
-import okhttp3.mockwebserver.MockWebServer
-import okhttp3.mockwebserver.RecordedRequest
-import org.junit.After
-import org.junit.Assert.*
-import org.junit.Before
-import org.junit.Test
-import org.junit.runner.RunWith
-import org.mockito.Mockito.mock
-import org.robolectric.RobolectricTestRunner
-import org.robolectric.annotation.Config
-import org.wikipedia.util.DateUtil
-import java.net.URLDecoder
-import java.util.*
-
-@RunWith(RobolectricTestRunner::class)
-@Config(sdk = [21], application = TestCommonsApplication::class)
-class ApacheHttpClientMediaWikiApiTest {
-
-    private lateinit var testObject: ApacheHttpClientMediaWikiApi
-    private lateinit var server: MockWebServer
-    private lateinit var wikidataServer: MockWebServer
-    private lateinit var sharedPreferences: JsonKvStore
-    private lateinit var okHttpClient: OkHttpClient
-
-    @Before
-    fun setUp() {
-        server = MockWebServer()
-        wikidataServer = MockWebServer()
-        okHttpClient = OkHttpClient()
-        sharedPreferences = mock(JsonKvStore::class.java)
-        testObject = ApacheHttpClientMediaWikiApi("http://" + server.hostName + ":" + server.port + "/")
-    }
-
-    @After
-    fun teardown() {
-        server.shutdown()
-    }
-
-    @Test
-    fun isUserBlockedFromCommonsForInfinitelyBlockedUser() {
-        server.enqueue(MockResponse().setBody("<?xml version=\"1.0\"?><api><query><userinfo id=\"1000\" name=\"testusername\" blockid=\"3000\" blockedby=\"blockerusername\" blockedbyid=\"1001\" blockreason=\"testing\" blockedtimestamp=\"2018-05-24T15:32:09Z\" blockexpiry=\"infinite\"></userinfo></query></api>"))
-
-        val result = testObject.isUserBlockedFromCommons()
-
-        assertBasicRequestParameters(server, "GET").let { userBlockedRequest ->
-            parseQueryParams(userBlockedRequest).let { body ->
-                assertEquals("xml", body["format"])
-                assertEquals("query", body["action"])
-                assertEquals("userinfo", body["meta"])
-                assertEquals("blockinfo", body["uiprop"])
-            }
-        }
-
-        assertTrue(result)
-    }
-
-    @Test
-    fun isUserBlockedFromCommonsForTimeBlockedUser() {
-        val currentDate = Date()
-        val expiredDate = Date(currentDate.time + 10000)
-        server.enqueue(MockResponse().setBody("<?xml version=\"1.0\"?><api><query><userinfo id=\"1000\" name=\"testusername\" blockid=\"3000\" blockedby=\"blockerusername\" blockedbyid=\"1001\" blockreason=\"testing\" blockedtimestamp=\"2018-05-24T15:32:09Z\" blockexpiry=\"" + DateUtil.iso8601DateFormat(expiredDate) + "\"></userinfo></query></api>"))
-
-        val result = testObject.isUserBlockedFromCommons()
-
-        assertBasicRequestParameters(server, "GET").let { userBlockedRequest ->
-            parseQueryParams(userBlockedRequest).let { body ->
-                assertEquals("xml", body["format"])
-                assertEquals("query", body["action"])
-                assertEquals("userinfo", body["meta"])
-                assertEquals("blockinfo", body["uiprop"])
-            }
-        }
-
-        assertTrue(result)
-    }
-
-    @Test
-    fun isUserBlockedFromCommonsForExpiredBlockedUser() {
-        val currentDate = Date()
-        val expiredDate = Date(currentDate.time - 10000)
-        server.enqueue(MockResponse().setBody("<?xml version=\"1.0\"?><api><query><userinfo id=\"1000\" name=\"testusername\" blockid=\"3000\" blockedby=\"blockerusername\" blockedbyid=\"1001\" blockreason=\"testing\" blockedtimestamp=\"2018-05-24T15:32:09Z\" blockexpiry=\"" + DateUtil.iso8601DateFormat(expiredDate) + "\"></userinfo></query></api>"))
-
-        val result = testObject.isUserBlockedFromCommons()
-
-        assertBasicRequestParameters(server, "GET").let { userBlockedRequest ->
-            parseQueryParams(userBlockedRequest).let { body ->
-                assertEquals("xml", body["format"])
-                assertEquals("query", body["action"])
-                assertEquals("userinfo", body["meta"])
-                assertEquals("blockinfo", body["uiprop"])
-            }
-        }
-
-        assertFalse(result)
-    }
-
-    @Test
-    fun isUserBlockedFromCommonsForNotBlockedUser() {
-        server.enqueue(MockResponse().setBody("<?xml version=\"1.0\"?><api><query><userinfo id=\"1000\" name=\"testusername\"></userinfo></query></api>"))
-
-        val result = testObject.isUserBlockedFromCommons()
-
-        assertBasicRequestParameters(server, "GET").let { userBlockedRequest ->
-            parseQueryParams(userBlockedRequest).let { body ->
-                assertEquals("xml", body["format"])
-                assertEquals("query", body["action"])
-                assertEquals("userinfo", body["meta"])
-                assertEquals("blockinfo", body["uiprop"])
-            }
-        }
-
-        assertFalse(result)
-    }
-
-    private fun assertBasicRequestParameters(server: MockWebServer, method: String): RecordedRequest = server.takeRequest().let {
-        assertEquals("/", it.requestUrl.encodedPath())
-        assertEquals(method, it.method)
-        assertEquals("Commons/${ConfigUtils.getVersionNameWithSha(ApplicationProvider.getApplicationContext())} (https://mediawiki.org/wiki/Apps/Commons) Android/${Build.VERSION.RELEASE}",
-                it.getHeader("User-Agent"))
-        if ("POST" == method) {
-            assertEquals("application/x-www-form-urlencoded", it.getHeader("Content-Type"))
-        }
-        return it
-    }
-
-    private fun parseQueryParams(request: RecordedRequest) = HashMap<String, String?>().apply {
-        request.requestUrl.let {
-            it.queryParameterNames().forEach { name -> put(name, it.queryParameter(name)) }
-        }
-    }
-
-    private fun parseBody(body: String): Map<String, String> = HashMap<String, String>().apply {
-        body.split("&".toRegex()).dropLastWhile { it.isEmpty() }.toTypedArray().forEach { prop ->
-            val pair = prop.split("=".toRegex()).dropLastWhile { it.isEmpty() }.toTypedArray()
-            put(pair[0], URLDecoder.decode(pair[1], "utf-8"))
-        }
-    }
-}

app/src/test/kotlin/fr/free/nrw/commons/mwapi/UserClientTest.kt

@@ -0,0 +1,77 @@
+package fr.free.nrw.commons.mwapi
+
+import io.reactivex.Observable
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+import org.mockito.*
+import org.wikipedia.dataclient.mwapi.MwQueryPage
+import org.wikipedia.dataclient.mwapi.MwQueryResponse
+import org.wikipedia.dataclient.mwapi.MwQueryResult
+import org.wikipedia.dataclient.mwapi.UserInfo
+import org.wikipedia.util.DateUtil
+import java.util.*
+
+class UserClientTest{
+    @Mock
+    internal var userInterface: UserInterface? = null
+
+    @InjectMocks
+    var userClient: UserClient? = null
+
+    @Before
+    @Throws(Exception::class)
+    fun setUp() {
+        MockitoAnnotations.initMocks(this)
+    }
+
+    @Test
+    fun isUserBlockedFromCommonsForInfinitelyBlockedUser() {
+        val userInfo = Mockito.mock(UserInfo::class.java)
+        Mockito.`when`(userInfo.blockexpiry()).thenReturn("infinite")
+        val mwQueryResult = Mockito.mock(MwQueryResult::class.java)
+        Mockito.`when`(mwQueryResult.userInfo()).thenReturn(userInfo)
+        val mockResponse = Mockito.mock(MwQueryResponse::class.java)
+        Mockito.`when`(mockResponse.query()).thenReturn(mwQueryResult)
+        Mockito.`when`(userInterface!!.userBlockInfo)
+                .thenReturn(Observable.just(mockResponse))
+
+        val isBanned = userClient!!.isUserBlockedFromCommons.blockingGet()
+        assertTrue(isBanned)
+    }
+
+    @Test
+    fun isUserBlockedFromCommonsForTimeBlockedUser() {
+        val currentDate = Date()
+        val expiredDate = Date(currentDate.time + 10000)
+
+        val userInfo = Mockito.mock(UserInfo::class.java)
+        Mockito.`when`(userInfo.blockexpiry()).thenReturn(DateUtil.iso8601DateFormat(expiredDate))
+        val mwQueryResult = Mockito.mock(MwQueryResult::class.java)
+        Mockito.`when`(mwQueryResult.userInfo()).thenReturn(userInfo)
+        val mockResponse = Mockito.mock(MwQueryResponse::class.java)
+        Mockito.`when`(mockResponse.query()).thenReturn(mwQueryResult)
+        Mockito.`when`(userInterface!!.userBlockInfo)
+                .thenReturn(Observable.just(mockResponse))
+
+        val isBanned = userClient!!.isUserBlockedFromCommons.blockingGet()
+        assertTrue(isBanned)
+    }
+
+    @Test
+    fun isUserBlockedFromCommonsForNeverBlockedUser() {
+        val userInfo = Mockito.mock(UserInfo::class.java)
+        Mockito.`when`(userInfo.blockexpiry()).thenReturn("")
+        val mwQueryResult = Mockito.mock(MwQueryResult::class.java)
+        Mockito.`when`(mwQueryResult.userInfo()).thenReturn(userInfo)
+        val mockResponse = Mockito.mock(MwQueryResponse::class.java)
+        Mockito.`when`(mockResponse.query()).thenReturn(mwQueryResult)
+        Mockito.`when`(userInterface!!.userBlockInfo)
+                .thenReturn(Observable.just(mockResponse))
+
+        val isBanned = userClient!!.isUserBlockedFromCommons.blockingGet()
+        assertFalse(isBanned)
+    }
+
+}