Migrated logEvents to retrofit (#3087)

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

* Added UserCLient and UserInterface

* Migrated ContributionsSyncAdapter to UserClient
Fixed sync related bugs

* Removed unused code

* Removed unused code

* Updated wikimedia-android-data-client to new version

Author: ilgazer

app/build.gradle

@@ -31,7 +31,7 @@ dependencies {
     implementation 'com.facebook.fresco:fresco:1.13.0'
     implementation 'com.drewnoakes:metadata-extractor:2.11.0'
     implementation 'org.apache.commons:commons-lang3:3.8.1'
-    implementation 'com.dmitrybrant:wikimedia-android-data-client:0.0.25'
+    implementation 'com.github.ilgazer:wikimedia-android-data-client:f65809ee2c'
 
     // UI
     implementation 'fr.avianey.com.viewpagerindicator:library:2.4.1.1@aar'

app/src/main/java/fr/free/nrw/commons/contributions/Contribution.java

@@ -92,6 +92,15 @@ public Contribution(Uri localUri, String imageUrl, String filename, String descr
         this.dateCreatedSource = "";
     }
 
+    public Contribution(Uri localUri, String imageUrl, String filename, String description, long dataLength,
+                        Date dateCreated, Date dateUploaded, String creator, String editSummary, String decimalCoords, int state) {
+        super(localUri, imageUrl, filename, description, dataLength, dateCreated, dateUploaded, creator);
+        this.decimalCoords = decimalCoords;
+        this.editSummary = editSummary;
+        this.dateCreatedSource = "";
+        this.state=state;
+    }
+
     public Contribution(Parcel in) {
         super(in);
         contentUri = in.readParcelable(Uri.class.getClassLoader());

app/src/main/java/fr/free/nrw/commons/contributions/ContributionsSyncAdapter.java

@@ -1,6 +1,7 @@
 package fr.free.nrw.commons.contributions;
 
 import android.accounts.Account;
+import android.annotation.SuppressLint;
 import android.content.AbstractThreadedSyncAdapter;
 import android.content.ContentProviderClient;
 import android.content.ContentValues;
@@ -11,6 +12,8 @@
 import android.os.RemoteException;
 import android.text.TextUtils;
 
+import org.wikipedia.dataclient.mwapi.MwQueryLogEvent;
+import org.wikipedia.dataclient.mwapi.MwQueryResult;
 import org.wikipedia.util.DateUtil;
 
 import java.io.IOException;
@@ -26,6 +29,7 @@
 import fr.free.nrw.commons.kvstore.JsonKvStore;
 import fr.free.nrw.commons.mwapi.LogEventResult;
 import fr.free.nrw.commons.mwapi.MediaWikiApi;
+import fr.free.nrw.commons.mwapi.UserClient;
 import timber.log.Timber;
 
 import static fr.free.nrw.commons.contributions.Contribution.STATE_COMPLETED;
@@ -44,8 +48,8 @@ public class ContributionsSyncAdapter extends AbstractThreadedSyncAdapter {
     // into the app, rather than the user's setting. Also see Github issue #52.
     public static final int ABSOLUTE_CONTRIBUTIONS_LOAD_LIMIT = 500;
 
-    @SuppressWarnings("WeakerAccess")
-    @Inject MediaWikiApi mwApi;
+    @Inject
+    UserClient userClient;
     @Inject
     @Named("default_preferences")
     JsonKvStore defaultKvStore;
@@ -58,24 +62,19 @@ private boolean fileExists(ContentProviderClient client, String filename) {
         if (filename == null) {
             return false;
         }
-        Cursor cursor = null;
-        try {
-            cursor = client.query(BASE_URI,
-                    existsQuery,
-                    existsSelection,
-                    new String[]{filename},
-                    ""
-            );
+        try (Cursor cursor = client.query(BASE_URI,
+                existsQuery,
+                existsSelection,
+                new String[]{filename},
+                ""
+        )) {
             return cursor != null && cursor.getCount() != 0;
         } catch (RemoteException e) {
             throw new RuntimeException(e);
-        } finally {
-            if (cursor != null) {
-                cursor.close();
-            }
         }
     }
 
+    @SuppressLint("CheckResult")
     @Override
     public void onPerformSync(Account account, Bundle bundle, String authority,
                               ContentProviderClient contentProviderClient, SyncResult syncResult) {
@@ -84,71 +83,20 @@ public void onPerformSync(Account account, Bundle bundle, String authority,
                         .getApplicationContext())
                 .getCommonsApplicationComponent()
                 .inject(this);
-        // This code is fraught with possibilities of race conditions, but lalalalala I can't hear you!
+        // This code is(was?) fraught with possibilities of race conditions, but lalalalala I can't hear you!
         String user = account.name;
-        String lastModified = defaultKvStore.getString("lastSyncTimestamp", "");
-        Date curTime = new Date();
-        LogEventResult result;
-        Boolean done = false;
-        String queryContinue = null;
         ContributionDao contributionDao = new ContributionDao(() -> contentProviderClient);
-        while (!done) {
-
-            try {
-                result = mwApi.logEvents(user, lastModified, queryContinue, ABSOLUTE_CONTRIBUTIONS_LOAD_LIMIT);
-            } catch (IOException e) {
-                // There isn't really much we can do, eh?
-                // FIXME: Perhaps add EventLogging?
-                syncResult.stats.numIoExceptions += 1; // Not sure if this does anything. Shitty docs
-                Timber.d("Syncing failed due to %s", e);
-                return;
-            }
-            Timber.d("Last modified at %s", lastModified);
-
-            List<LogEventResult.LogEvent> logEvents = result.getLogEvents();
-            Timber.d("%d results!", logEvents.size());
-            ArrayList<ContentValues> imageValues = new ArrayList<>();
-            for (LogEventResult.LogEvent image : logEvents) {
-                if (image.isDeleted()) {
-                    // means that this upload was deleted.
-                    continue;
-                }
-                String filename = image.getFilename();
-                if (fileExists(contentProviderClient, filename)) {
-                    Timber.d("Skipping %s", filename);
-                    continue;
-                }
-                Date dateUpdated = image.getDateUpdated();
-                Contribution contrib = new Contribution(null, null, filename,
-                        "", -1, dateUpdated, dateUpdated, user,
-                        "", "");
-                contrib.setState(STATE_COMPLETED);
-                imageValues.add(contributionDao.toContentValues(contrib));
-
-                if (imageValues.size() % COMMIT_THRESHOLD == 0) {
-                    try {
-                        contentProviderClient.bulkInsert(BASE_URI, imageValues.toArray(EMPTY));
-                    } catch (RemoteException e) {
-                        throw new RuntimeException(e);
-                    }
-                    imageValues.clear();
-                }
-            }
-
-            if (imageValues.size() != 0) {
-                try {
-                    contentProviderClient.bulkInsert(BASE_URI, imageValues.toArray(EMPTY));
-                } catch (RemoteException e) {
-                    throw new RuntimeException(e);
-                }
-            }
-
-            queryContinue = result.getQueryContinue();
-            if (TextUtils.isEmpty(queryContinue)) {
-                done = true;
-            }
-        }
-        defaultKvStore.putString("lastSyncTimestamp", DateUtil.iso8601DateFormat(curTime));
+        userClient.logEvents(user)
+                .doOnNext(mwQueryLogEvent->Timber.d("Received image %s", mwQueryLogEvent.title() ))
+                .filter(mwQueryLogEvent -> !mwQueryLogEvent.isDeleted())
+                .filter(mwQueryLogEvent -> !fileExists(contentProviderClient, mwQueryLogEvent.title()))
+                .doOnNext(mwQueryLogEvent->Timber.d("Image %s passed filters", mwQueryLogEvent.title() ))
+                .map(image -> new Contribution(null, null, image.title(),
+                        "", -1, image.date(), image.date(), user,
+                        "", "", STATE_COMPLETED))
+                .map(contributionDao::toContentValues)
+                .buffer(10)
+                .subscribe(imageValues->contentProviderClient.bulkInsert(BASE_URI, imageValues.toArray(EMPTY)));
         Timber.d("Oh hai, everyone! Look, a kitty!");
     }
 }

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

@@ -30,6 +30,7 @@
 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.mwapi.UserInterface;
 import fr.free.nrw.commons.review.ReviewInterface;
 import fr.free.nrw.commons.upload.UploadInterface;
 import okhttp3.Cache;
@@ -229,4 +230,9 @@ public MediaInterface provideMediaInterface(@Named(NAMED_COMMONS_WIKI_SITE) Wiki
     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) {
+        return ServiceFactory.get(commonsWikiSite, BuildConfig.COMMONS_URL, UserInterface.class);
+    }
 }

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

@@ -83,54 +83,6 @@ public Single<MediaResult> fetchMediaByFilename(String filename) {
         });
     }
 
-    @Override
-    @NonNull
-    public LogEventResult logEvents(String user, String lastModified, String queryContinue, int limit) throws IOException {
-        CustomMwApi.RequestBuilder builder = api.action("query")
-                .param("list", "logevents")
-                .param("letype", "upload")
-                .param("leprop", "title|timestamp|ids")
-                .param("leuser", user)
-                .param("lelimit", limit);
-        if (!TextUtils.isEmpty(lastModified)) {
-            builder.param("leend", lastModified);
-        }
-        if (!TextUtils.isEmpty(queryContinue)) {
-            builder.param("lestart", queryContinue);
-        }
-        CustomApiResult result = builder.get();
-
-        return new LogEventResult(
-                getLogEventsFromResult(result),
-                result.getString("/api/query-continue/logevents/@lestart"));
-    }
-
-    @NonNull
-    private ArrayList<LogEventResult.LogEvent> getLogEventsFromResult(CustomApiResult result) {
-        ArrayList<CustomApiResult> uploads = result.getNodes("/api/query/logevents/item");
-        Timber.d("%d results!", uploads.size());
-        ArrayList<LogEventResult.LogEvent> logEvents = new ArrayList<>();
-        for (CustomApiResult image : uploads) {
-            logEvents.add(new LogEventResult.LogEvent(
-                    image.getString("@pageid"),
-                    image.getString("@title"),
-                    parseMWDate(image.getString("@timestamp")))
-            );
-        }
-        return logEvents;
-    }
-
-    @Override
-    @Nullable
-    public String revisionsByFilename(String filename) throws IOException {
-        return api.action("query")
-                .param("prop", "revisions")
-                .param("rvprop", "timestamp|content")
-                .param("titles", filename)
-                .get()
-                .getString("/api/query/pages/page/revisions/rev");
-    }
-
     /**
      * Checks to see if a user is currently blocked from Commons
      *

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

@@ -14,12 +14,6 @@ public interface MediaWikiApi {
     @NonNull
     Single<MediaResult> fetchMediaByFilename(String filename);
 
-    @Nullable
-    String revisionsByFilename(String filename) throws IOException;
-
-    @NonNull
-    LogEventResult logEvents(String user, String lastModified, String queryContinue, int limit) throws IOException;
-
     boolean isUserBlockedFromCommons();
 
     void logout();

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

@@ -0,0 +1,32 @@
+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;
+
+public class UserClient {
+    private final UserInterface userInterface;
+
+    @Inject
+    public UserClient(UserInterface userInterface) {
+        this.userInterface = userInterface;
+    }
+
+    public Observable<MwQueryLogEvent> logEvents(String user) {
+        try {
+            return userInterface.getUserLogEvents(user, Collections.emptyMap())
+                    .map(MwQueryResponse::query)
+                    .map(MwQueryResult::logevents)
+                    .flatMap(Observable::fromIterable);
+        } catch (Throwable throwable) {
+            return Observable.empty();
+        }
+
+    }
+}

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

@@ -0,0 +1,17 @@
+package fr.free.nrw.commons.mwapi;
+
+import org.wikipedia.dataclient.mwapi.MwQueryResponse;
+
+import java.util.Map;
+
+import io.reactivex.Observable;
+import retrofit2.http.GET;
+import retrofit2.http.Query;
+import retrofit2.http.QueryMap;
+
+import static org.wikipedia.dataclient.Service.MW_API_PREFIX;
+
+public interface UserInterface {
+    @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);
+}