diff --git a/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.java b/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.java index 903c35ab0..7f0ee4048 100644 --- a/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.java +++ b/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.java @@ -3,11 +3,12 @@ package fr.free.nrw.commons.di; import android.app.Activity; import android.content.ContentProviderClient; import android.content.Context; -import androidx.collection.LruCache; import android.view.inputmethod.InputMethodManager; import com.google.gson.Gson; +import org.wikipedia.dataclient.WikiSite; + import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -16,6 +17,7 @@ import java.util.Map; import javax.inject.Named; import javax.inject.Singleton; +import androidx.collection.LruCache; import dagger.Module; import dagger.Provides; import fr.free.nrw.commons.BuildConfig; @@ -24,7 +26,6 @@ import fr.free.nrw.commons.auth.AccountUtil; import fr.free.nrw.commons.auth.SessionManager; import fr.free.nrw.commons.data.DBOpenHelper; import fr.free.nrw.commons.kvstore.JsonKvStore; -import fr.free.nrw.commons.kvstore.JsonKvStore; import fr.free.nrw.commons.location.LocationServiceManager; import fr.free.nrw.commons.settings.Prefs; import fr.free.nrw.commons.upload.UploadController; diff --git a/app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java b/app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java index 23bf7e3fc..c8664cd9f 100644 --- a/app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java +++ b/app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java @@ -4,6 +4,8 @@ import android.content.Context; import com.google.gson.Gson; +import org.wikipedia.dataclient.ServiceFactory; +import org.wikipedia.dataclient.WikiSite; import org.wikipedia.json.GsonUtil; import java.io.File; @@ -20,6 +22,7 @@ import fr.free.nrw.commons.kvstore.JsonKvStore; import fr.free.nrw.commons.mwapi.ApacheHttpClientMediaWikiApi; import fr.free.nrw.commons.mwapi.MediaWikiApi; import fr.free.nrw.commons.mwapi.OkHttpJsonApiClient; +import fr.free.nrw.commons.review.ReviewInterface; import okhttp3.Cache; import okhttp3.HttpUrl; import okhttp3.OkHttpClient; @@ -108,4 +111,16 @@ public class NetworkingModule { return GsonUtil.getDefaultGson(); } + @Provides + @Singleton + @Named("commons-wikisite") + public WikiSite provideCommonsWikiSite() { + return new WikiSite(BuildConfig.COMMONS_URL); + } + + @Provides + @Singleton + public ReviewInterface provideReviewInterface(@Named("commons-wikisite") WikiSite commonsWikiSite) { + return ServiceFactory.get(commonsWikiSite, BuildConfig.COMMONS_URL, ReviewInterface.class); + } } diff --git a/app/src/main/java/fr/free/nrw/commons/mwapi/OkHttpJsonApiClient.java b/app/src/main/java/fr/free/nrw/commons/mwapi/OkHttpJsonApiClient.java index 36189edc0..e829e946f 100644 --- a/app/src/main/java/fr/free/nrw/commons/mwapi/OkHttpJsonApiClient.java +++ b/app/src/main/java/fr/free/nrw/commons/mwapi/OkHttpJsonApiClient.java @@ -1,10 +1,28 @@ package fr.free.nrw.commons.mwapi; import android.text.TextUtils; + import androidx.annotation.NonNull; import androidx.annotation.Nullable; + import com.google.gson.Gson; import com.google.gson.reflect.TypeToken; + +import org.apache.commons.lang3.StringUtils; +import org.wikipedia.dataclient.mwapi.MwQueryPage; +import org.wikipedia.dataclient.mwapi.MwQueryResponse; + +import java.io.IOException; +import java.lang.reflect.Type; +import java.util.ArrayList; +import java.util.Date; +import java.util.List; +import java.util.Locale; +import java.util.Map; + +import javax.inject.Inject; +import javax.inject.Singleton; + import fr.free.nrw.commons.Media; import fr.free.nrw.commons.achievements.FeaturedImages; import fr.free.nrw.commons.achievements.FeedbackResponse; @@ -20,25 +38,10 @@ import fr.free.nrw.commons.utils.ConfigUtils; import fr.free.nrw.commons.wikidata.model.GetWikidataEditCountResponse; import io.reactivex.Observable; import io.reactivex.Single; -import java.io.IOException; -import java.lang.reflect.Type; -import java.util.ArrayList; -import java.util.Date; -import java.util.List; -import java.util.Locale; -import java.util.Map; -import java.util.Random; -import javax.inject.Inject; -import javax.inject.Singleton; import okhttp3.HttpUrl; import okhttp3.OkHttpClient; import okhttp3.Request; import okhttp3.Response; -import org.apache.commons.lang3.StringUtils; -import org.wikipedia.dataclient.mwapi.MwQueryPage; -import org.wikipedia.dataclient.mwapi.MwQueryResponse; -import org.wikipedia.dataclient.mwapi.RecentChange; -import org.wikipedia.util.DateUtil; import timber.log.Timber; /** @@ -418,80 +421,4 @@ public class OkHttpJsonApiClient { private Map getContinueValues(String keyword) { return defaultKvStore.getJson("query_continue_" + keyword, mapType); } - - /** - * Returns recent changes on commons - * - * @return list of recent changes made - */ - @Nullable - public Single> getRecentFileChanges() { - final int RANDOM_SECONDS = 60 * 60 * 24 * 30; - final String FILE_NAMESPACE = "6"; - Random r = new Random(); - Date now = new Date(); - Date startDate = new Date(now.getTime() - r.nextInt(RANDOM_SECONDS) * 1000L); - - String rcStart = DateUtil.iso8601DateFormat(startDate); - HttpUrl.Builder urlBuilder = HttpUrl - .parse(commonsBaseUrl) - .newBuilder() - .addQueryParameter("action", "query") - .addQueryParameter("format", "json") - .addQueryParameter("formatversion", "2") - .addQueryParameter("list", "recentchanges") - .addQueryParameter("rcstart", rcStart) - .addQueryParameter("rcnamespace", FILE_NAMESPACE) - .addQueryParameter("rcprop", "title|ids") - .addQueryParameter("rctype", "new|log") - .addQueryParameter("rctoponly", "1"); - - Request request = new Request.Builder() - .url(urlBuilder.build()) - .build(); - - return Single.fromCallable(() -> { - Response response = okHttpClient.newCall(request).execute(); - if (response.body() != null && response.isSuccessful()) { - String json = response.body().string(); - MwQueryResponse mwQueryPage = gson.fromJson(json, MwQueryResponse.class); - return mwQueryPage.query().getRecentChanges(); - } - return new ArrayList<>(); - }); - } - - /** - * Returns the first revision of the file - * - * @return Revision object - */ - @Nullable - public Single getFirstRevisionOfFile(String filename) { - HttpUrl.Builder urlBuilder = HttpUrl - .parse(commonsBaseUrl) - .newBuilder() - .addQueryParameter("action", "query") - .addQueryParameter("format", "json") - .addQueryParameter("formatversion", "2") - .addQueryParameter("prop", "revisions") - .addQueryParameter("rvprop", "timestamp|ids|user") - .addQueryParameter("titles", filename) - .addQueryParameter("rvdir", "newer") - .addQueryParameter("rvlimit", "1"); - - Request request = new Request.Builder() - .url(urlBuilder.build()) - .build(); - - return Single.fromCallable(() -> { - Response response = okHttpClient.newCall(request).execute(); - if (response.body() != null && response.isSuccessful()) { - String json = response.body().string(); - MwQueryResponse mwQueryPage = gson.fromJson(json, MwQueryResponse.class); - return mwQueryPage.query().firstPage().revisions().get(0); - } - return null; - }); - } } diff --git a/app/src/main/java/fr/free/nrw/commons/review/ReviewActivity.java b/app/src/main/java/fr/free/nrw/commons/review/ReviewActivity.java index 7f029bc6f..cbb9848ca 100644 --- a/app/src/main/java/fr/free/nrw/commons/review/ReviewActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/review/ReviewActivity.java @@ -12,13 +12,20 @@ import android.view.View; import android.widget.Button; import android.widget.ProgressBar; import android.widget.TextView; + import androidx.appcompat.widget.Toolbar; import androidx.drawerlayout.widget.DrawerLayout; -import butterknife.BindView; -import butterknife.ButterKnife; + import com.facebook.drawee.view.SimpleDraweeView; import com.google.android.material.navigation.NavigationView; import com.viewpagerindicator.CirclePageIndicator; + +import java.util.ArrayList; + +import javax.inject.Inject; + +import butterknife.BindView; +import butterknife.ButterKnife; import fr.free.nrw.commons.Media; import fr.free.nrw.commons.R; import fr.free.nrw.commons.auth.AuthenticatedActivity; @@ -29,8 +36,6 @@ import fr.free.nrw.commons.utils.ViewUtil; import io.reactivex.android.schedulers.AndroidSchedulers; import io.reactivex.disposables.CompositeDisposable; import io.reactivex.schedulers.Schedulers; -import java.util.ArrayList; -import javax.inject.Inject; public class ReviewActivity extends AuthenticatedActivity { @@ -139,7 +144,11 @@ public class ReviewActivity extends AuthenticatedActivity { compositeDisposable.add(reviewHelper.getRandomMedia() .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) - .subscribe(this::updateImage)); + .subscribe(media -> { + if (media != null) { + updateImage(media); + } + })); return true; } diff --git a/app/src/main/java/fr/free/nrw/commons/review/ReviewHelper.java b/app/src/main/java/fr/free/nrw/commons/review/ReviewHelper.java index 7468cc7f6..30903a89a 100644 --- a/app/src/main/java/fr/free/nrw/commons/review/ReviewHelper.java +++ b/app/src/main/java/fr/free/nrw/commons/review/ReviewHelper.java @@ -1,39 +1,63 @@ package fr.free.nrw.commons.review; + +import org.apache.commons.lang3.StringUtils; import org.wikipedia.dataclient.mwapi.MwQueryPage; import org.wikipedia.dataclient.mwapi.RecentChange; +import org.wikipedia.util.DateUtil; -import java.util.List; +import java.util.Date; import java.util.Random; import javax.inject.Inject; import javax.inject.Singleton; -import androidx.annotation.Nullable; -import androidx.core.util.Pair; import fr.free.nrw.commons.Media; import fr.free.nrw.commons.mwapi.MediaWikiApi; import fr.free.nrw.commons.mwapi.OkHttpJsonApiClient; +import io.reactivex.Observable; import io.reactivex.Single; @Singleton public class ReviewHelper { - private static final int MAX_RANDOM_TRIES = 5; private static final String[] imageExtensions = new String[]{".jpg", ".jpeg", ".png"}; private final OkHttpJsonApiClient okHttpJsonApiClient; private final MediaWikiApi mediaWikiApi; + private final ReviewInterface reviewInterface; @Inject - public ReviewHelper(OkHttpJsonApiClient okHttpJsonApiClient, MediaWikiApi mediaWikiApi) { + public ReviewHelper(OkHttpJsonApiClient okHttpJsonApiClient, + MediaWikiApi mediaWikiApi, + ReviewInterface reviewInterface) { this.okHttpJsonApiClient = okHttpJsonApiClient; this.mediaWikiApi = mediaWikiApi; + this.reviewInterface = reviewInterface; } - Single getRandomMedia() { - return getRandomFileChange() - .flatMap(fileName -> okHttpJsonApiClient.getMedia(fileName, false)); + /** + * Fetches recent changes from MediaWiki API + * Calls the API to get 10 changes in the last 1 hour + * Earlier we were getting changes for the last 30 days but as the API returns just 10 results + * its best to fetch for just last 1 hour. + * @return + */ + private Observable getRecentChanges() { + final int RANDOM_SECONDS = 60 * 60; + Random r = new Random(); + Date now = new Date(); + Date startDate = new Date(now.getTime() - r.nextInt(RANDOM_SECONDS) * 1000L); + + String rcStart = DateUtil.iso8601DateFormat(startDate); + return reviewInterface.getRecentChanges(rcStart) + .map(mwQueryResponse -> mwQueryResponse.query().getRecentChanges()) + .map(recentChanges -> { + //Collections.shuffle(recentChanges); + return recentChanges; + }) + .flatMapIterable(changes -> changes) + .filter(recentChange -> isChangeReviewable(recentChange)); } /** @@ -43,57 +67,62 @@ public class ReviewHelper { * - Checks if the file is nominated for deletion * - Retries upto 5 times for getting a file which is not nominated for deletion * + * @return Random file change + */ + public Single getRandomMedia() { + return getRecentChanges() + .flatMapSingle(change -> getRandomMediaFromRecentChange(change)) + .onExceptionResumeNext(Observable.just(new Media(""))) + .filter(media -> !StringUtils.isBlank(media.getFilename())) + .firstOrError(); + } + + /** + * Returns a proper Media object if the file is not already nominated for deletion + * Else it returns an empty Media object + * @param recentChange * @return */ - private Single getRandomFileChange() { - return okHttpJsonApiClient.getRecentFileChanges() - .map(this::findImageInRecentChanges) - .flatMap(title -> mediaWikiApi.pageExists("Commons:Deletion_requests/" + title) - .map(pageExists -> new Pair<>(title, pageExists))) - .map((Pair pair) -> { - if (!pair.second) { - return pair.first; + private Single getRandomMediaFromRecentChange(RecentChange recentChange) { + return Single.just(recentChange) + .flatMap(change -> mediaWikiApi.pageExists("Commons:Deletion_requests/" + change.getTitle())) + .flatMap(isDeleted -> { + if (isDeleted) { + return Single.just(new Media("")); } - throw new Exception("Already nominated for deletion"); - }).retry(MAX_RANDOM_TRIES); + return okHttpJsonApiClient.getMedia(recentChange.getTitle(), false); + }); + } - Single getFirstRevisionOfFile(String fileName) { - return okHttpJsonApiClient.getFirstRevisionOfFile(fileName); + /** + * Gets the first revision of the file from filename + * @param filename + * @return + */ + Observable getFirstRevisionOfFile(String filename) { + return reviewInterface.getFirstRevisionOfFile(filename) + .map(response -> response.query().firstPage().revisions().get(0)); } - @Nullable - private String findImageInRecentChanges(List recentChanges) { - String imageTitle; - Random r = new Random(); - int count = recentChanges.size(); - // Build a range array - int[] randomIndexes = new int[count]; - for (int i = 0; i < count; i++) { - randomIndexes[i] = i; + /** + * Checks if the change is reviewable or not. + * - checks the type and revisionId of the change + * - checks supported image extensions + * @param recentChange + * @return + */ + private boolean isChangeReviewable(RecentChange recentChange) { + if (recentChange.getType().equals("log") && !(recentChange.getOldRevisionId() == 0)) { + return false; } - // Then shuffle it - for (int i = 0; i < count; i++) { - int swapIndex = r.nextInt(count); - int temp = randomIndexes[i]; - randomIndexes[i] = randomIndexes[swapIndex]; - randomIndexes[swapIndex] = temp; - } - for (int i = 0; i < count; i++) { - int randomIndex = randomIndexes[i]; - RecentChange recentChange = recentChanges.get(randomIndex); - if (recentChange.getType().equals("log") && !(recentChange.getOldRevisionId() == 0)) { - // For log entries, we only want ones where old_revid is zero, indicating a new file - continue; - } - imageTitle = recentChange.getTitle(); - for (String imageExtension : imageExtensions) { - if (imageTitle.toLowerCase().endsWith(imageExtension)) { - return imageTitle; - } + for (String extension : imageExtensions) { + if (recentChange.getTitle().endsWith(extension)) { + return true; } } - return null; + + return false; } } diff --git a/app/src/main/java/fr/free/nrw/commons/review/ReviewInterface.java b/app/src/main/java/fr/free/nrw/commons/review/ReviewInterface.java new file mode 100644 index 000000000..7dee125fc --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/review/ReviewInterface.java @@ -0,0 +1,18 @@ +package fr.free.nrw.commons.review; + +import org.wikipedia.dataclient.mwapi.MwQueryResponse; + +import io.reactivex.Observable; +import retrofit2.http.GET; +import retrofit2.http.Query; + +/** + * Interface class for peer review calls + */ +public interface ReviewInterface { + @GET("w/api.php?action=query&format=json&formatversion=2&list=recentchanges&rcprop=title|ids&rctype=new|log&rctoponly=1&rcnamespace=6") + Observable getRecentChanges(@Query("rcstart") String rcStart); + + @GET("w/api.php?action=query&format=json&formatversion=2&prop=revisions&rvprop=timestamp|ids|user&rvdir=newer&rvlimit=1") + Observable getFirstRevisionOfFile(@Query("titles") String titles); +} diff --git a/app/src/test/kotlin/fr/free/nrw/commons/review/ReviewHelperTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/review/ReviewHelperTest.kt index a878fb5ae..d9c808f77 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/review/ReviewHelperTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/review/ReviewHelperTest.kt @@ -3,7 +3,10 @@ package fr.free.nrw.commons.review import fr.free.nrw.commons.Media import fr.free.nrw.commons.mwapi.MediaWikiApi import fr.free.nrw.commons.mwapi.OkHttpJsonApiClient +import io.reactivex.Observable import io.reactivex.Single +import junit.framework.Assert.assertNotNull +import junit.framework.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test @@ -13,6 +16,8 @@ import org.mockito.Mock import org.mockito.Mockito.* import org.mockito.MockitoAnnotations import org.wikipedia.dataclient.mwapi.MwQueryPage +import org.wikipedia.dataclient.mwapi.MwQueryResponse +import org.wikipedia.dataclient.mwapi.MwQueryResult import org.wikipedia.dataclient.mwapi.RecentChange /** @@ -20,6 +25,8 @@ import org.wikipedia.dataclient.mwapi.RecentChange */ class ReviewHelperTest { + @Mock + internal var reviewInterface: ReviewInterface? = null @Mock internal var okHttpJsonApiClient: OkHttpJsonApiClient? = null @Mock @@ -35,6 +42,31 @@ class ReviewHelperTest { @Throws(Exception::class) fun setUp() { MockitoAnnotations.initMocks(this) + + val mwQueryPage = mock(MwQueryPage::class.java) + val mockRevision = mock(MwQueryPage.Revision::class.java) + `when`(mockRevision.user).thenReturn("TestUser") + `when`(mwQueryPage.revisions()).thenReturn(listOf(mockRevision)) + + val recentChange = getMockRecentChange("test", "File:Test1.jpeg", 0) + val recentChange1 = getMockRecentChange("test", "File:Test2.png", 0) + val recentChange2 = getMockRecentChange("test", "File:Test3.jpg", 0) + val mwQueryResult = mock(MwQueryResult::class.java) + `when`(mwQueryResult.recentChanges).thenReturn(listOf(recentChange, recentChange1, recentChange2)) + `when`(mwQueryResult.firstPage()).thenReturn(mwQueryPage) + `when`(mwQueryResult.pages()).thenReturn(listOf(mwQueryPage)) + val mockResponse = mock(MwQueryResponse::class.java) + `when`(mockResponse.query()).thenReturn(mwQueryResult) + `when`(reviewInterface?.getRecentChanges(ArgumentMatchers.anyString())) + .thenReturn(Observable.just(mockResponse)) + + `when`(reviewInterface?.getFirstRevisionOfFile(ArgumentMatchers.anyString())) + .thenReturn(Observable.just(mockResponse)) + + val media = mock(Media::class.java) + `when`(media.filename).thenReturn("File:Test.jpg") + `when`(okHttpJsonApiClient?.getMedia(ArgumentMatchers.anyString(), ArgumentMatchers.anyBoolean())) + .thenReturn(Single.just(media)) } /** @@ -42,40 +74,48 @@ class ReviewHelperTest { */ @Test fun getRandomMedia() { - val recentChange = getMockRecentChange("test", "File:Test1.jpeg", 0) - val recentChange1 = getMockRecentChange("test", "File:Test2.png", 0) - val recentChange2 = getMockRecentChange("test", "File:Test3.jpg", 0) - `when`(okHttpJsonApiClient?.recentFileChanges) - .thenReturn(Single.just(listOf(recentChange, recentChange1, recentChange2))) - `when`(mediaWikiApi?.pageExists(ArgumentMatchers.anyString())) .thenReturn(Single.just(false)) - `when`(okHttpJsonApiClient?.getMedia(ArgumentMatchers.anyString(), ArgumentMatchers.anyBoolean())) - .thenReturn(Single.just(mock(Media::class.java))) - val randomMedia = reviewHelper?.randomMedia?.blockingGet() + assertNotNull(randomMedia) assertTrue(randomMedia is Media) + verify(reviewInterface, times(1))!!.getRecentChanges(ArgumentMatchers.anyString()) } /** * Test scenario when all media is already nominated for deletion */ - @Test(expected = Exception::class) + @Test(expected = RuntimeException::class) fun getRandomMediaWithWithAllMediaNominatedForDeletion() { - val recentChange = getMockRecentChange("test", "File:Test1.jpeg", 0) - val recentChange1 = getMockRecentChange("test", "File:Test2.png", 0) - val recentChange2 = getMockRecentChange("test", "File:Test3.jpg", 0) - `when`(okHttpJsonApiClient?.recentFileChanges) - .thenReturn(Single.just(listOf(recentChange, recentChange1, recentChange2))) - `when`(mediaWikiApi?.pageExists(ArgumentMatchers.anyString())) .thenReturn(Single.just(true)) - reviewHelper?.randomMedia?.blockingGet() + val media = reviewHelper?.randomMedia?.blockingGet() + assertNull(media) + verify(reviewInterface, times(1))!!.getRecentChanges(ArgumentMatchers.anyString()) } - fun getMockRecentChange(type: String, title: String, oldRevisionId: Long): RecentChange { + /** + * Test scenario when first media is already nominated for deletion + */ + @Test + fun getRandomMediaWithWithOneMediaNominatedForDeletion() { + `when`(mediaWikiApi?.pageExists("Commons:Deletion_requests/File:Test1.jpeg")) + .thenReturn(Single.just(true)) + `when`(mediaWikiApi?.pageExists("Commons:Deletion_requests/File:Test2.png")) + .thenReturn(Single.just(false)) + `when`(mediaWikiApi?.pageExists("Commons:Deletion_requests/File:Test3.jpg")) + .thenReturn(Single.just(true)) + + val media = reviewHelper?.randomMedia?.blockingGet() + + assertNotNull(media) + assertTrue(media is Media) + verify(reviewInterface, times(1))!!.getRecentChanges(ArgumentMatchers.anyString()) + } + + private fun getMockRecentChange(type: String, title: String, oldRevisionId: Long): RecentChange { val recentChange = mock(RecentChange::class.java) `when`(recentChange!!.type).thenReturn(type) `when`(recentChange.title).thenReturn(title) @@ -88,9 +128,7 @@ class ReviewHelperTest { */ @Test fun getFirstRevisionOfFile() { - `when`(okHttpJsonApiClient?.getFirstRevisionOfFile(ArgumentMatchers.anyString())) - .thenReturn(Single.just(mock(MwQueryPage.Revision::class.java))) - val firstRevisionOfFile = reviewHelper?.getFirstRevisionOfFile("Test.jpg")?.blockingGet() + val firstRevisionOfFile = reviewHelper?.getFirstRevisionOfFile("Test.jpg")?.blockingFirst() assertTrue(firstRevisionOfFile is MwQueryPage.Revision) }