Use data client for peer review calls (#2937)

* Use data client for peer review calls

* With java docs

* Optimise API calls and fix tests
This commit is contained in:
Vivek Maskara 2019-06-07 12:29:08 +05:30 committed by neslihanturan
parent 51e2febc53
commit 6f9d69e63c
7 changed files with 205 additions and 168 deletions

View file

@ -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;

View file

@ -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);
}
}

View file

@ -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<String, String> getContinueValues(String keyword) {
return defaultKvStore.getJson("query_continue_" + keyword, mapType);
}
/**
* Returns recent changes on commons
*
* @return list of recent changes made
*/
@Nullable
public Single<List<RecentChange>> 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<MwQueryPage.Revision> 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;
});
}
}

View file

@ -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;
}

View file

@ -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<Media> 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<RecentChange> 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<Media> 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<String> getRandomFileChange() {
return okHttpJsonApiClient.getRecentFileChanges()
.map(this::findImageInRecentChanges)
.flatMap(title -> mediaWikiApi.pageExists("Commons:Deletion_requests/" + title)
.map(pageExists -> new Pair<>(title, pageExists)))
.map((Pair<String, Boolean> pair) -> {
if (!pair.second) {
return pair.first;
private Single<Media> 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<MwQueryPage.Revision> getFirstRevisionOfFile(String fileName) {
return okHttpJsonApiClient.getFirstRevisionOfFile(fileName);
/**
* Gets the first revision of the file from filename
* @param filename
* @return
*/
Observable<MwQueryPage.Revision> getFirstRevisionOfFile(String filename) {
return reviewInterface.getFirstRevisionOfFile(filename)
.map(response -> response.query().firstPage().revisions().get(0));
}
@Nullable
private String findImageInRecentChanges(List<RecentChange> 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;
}
// 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);
/**
* 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)) {
// For log entries, we only want ones where old_revid is zero, indicating a new file
continue;
return false;
}
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;
}
}

View file

@ -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<MwQueryResponse> 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<MwQueryResponse> getFirstRevisionOfFile(@Query("titles") String titles);
}

View file

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