#3808 Construct media objects that depict an item id correctly - use generator to construct media for DepictedImages

Author: macgills

app/src/main/java/fr/free/nrw/commons/Media.java

@@ -52,27 +52,6 @@ public Media() {
         pageId = UUID.randomUUID().toString();
     }
 
-    /**
-     * Provide Media constructor
-     * @param imageUrl Media image URL
-     * @param filename Media filename
-     * @param fallbackDescription Media description
-     * @param dateUploaded Media date uploaded
-     * @param creator Media creator
-     */
-    public Media(final String imageUrl, final String filename,
-        final String fallbackDescription,
-        final Date dateUploaded,
-        final String creator) {
-        this();
-        thumbUrl = imageUrl;
-        this.imageUrl = imageUrl;
-        this.filename = filename;
-        this.fallbackDescription = fallbackDescription;
-        this.dateUploaded = dateUploaded;
-        this.creator = creator;
-    }
-
     /**
      * Constructor with all parameters
      */
@@ -110,13 +89,20 @@ public Media(Media media) {
         this(media.getThumbUrl(), media.getImageUrl(), media.getFilename(),
             media.getFallbackDescription(), media.getDateUploaded(), media.getLicense(),
             media.getLicenseUrl(), media.getCreator(), media.getPageId(), media.getCategories(),
-            media.getCoordinates(), media.getCaptions(),media.getDescriptions(),media.getDepictionIds());
+            media.getCoordinates(), media.getCaptions(), media.getDescriptions(),
+            media.getDepictionIds());
     }
 
     public Media(final String filename,
         Map<String, String> captions, final String fallbackDescription,
         final String creator, final List<String> categories) {
-        this(null, filename, fallbackDescription, new Date(), creator);
+        this();
+        thumbUrl = null;
+        this.imageUrl = null;
+        this.filename = filename;
+        this.fallbackDescription = fallbackDescription;
+        this.dateUploaded = new Date();
+        this.creator = creator;
         this.categories = categories;
         this.captions=captions;
     }
@@ -132,7 +118,7 @@ protected Media(final Parcel in) {
     }
 
     private static List<String> readList(Parcel in) {
-        final ArrayList<String> list = new ArrayList<>();
+        final List<String> list = new ArrayList<>();
         in.readStringList(list);
         return list;
     }

app/src/main/java/fr/free/nrw/commons/depictions/Media/DepictedImagesPresenter.java

@@ -5,7 +5,6 @@
 
 import android.annotation.SuppressLint;
 import fr.free.nrw.commons.Media;
-import fr.free.nrw.commons.explore.depictions.DepictsClient;
 import fr.free.nrw.commons.kvstore.JsonKvStore;
 import fr.free.nrw.commons.media.MediaClient;
 import io.reactivex.Scheduler;
@@ -27,7 +26,6 @@ public class DepictedImagesPresenter implements DepictedImagesContract.UserActio
                     DepictedImagesContract.View.class.getClassLoader(),
                     new Class[]{DepictedImagesContract.View.class},
                     (proxy, method, methodArgs) -> null);
-    DepictsClient depictsClient;
     MediaClient mediaClient;
     @Named("default_preferences")
     JsonKvStore depictionKvStore;
@@ -40,13 +38,13 @@ public class DepictedImagesPresenter implements DepictedImagesContract.UserActio
      * Ex: Q9394
      */
     private List<Media> queryList = new ArrayList<>();
-    private String entityId;
 
     @Inject
-    public DepictedImagesPresenter(@Named("default_preferences") JsonKvStore depictionKvStore, DepictsClient depictsClient, MediaClient mediaClient,  @Named(IO_THREAD) Scheduler ioScheduler,
-                                   @Named(MAIN_THREAD) Scheduler mainThreadScheduler) {
+    public DepictedImagesPresenter(@Named("default_preferences") JsonKvStore depictionKvStore,
+        MediaClient mediaClient,
+        @Named(IO_THREAD) Scheduler ioScheduler,
+        @Named(MAIN_THREAD) Scheduler mainThreadScheduler) {
         this.depictionKvStore = depictionKvStore;
-        this.depictsClient = depictsClient;
         this.ioScheduler = ioScheduler;
         this.mainThreadScheduler = mainThreadScheduler;
         this.mediaClient = mediaClient;
@@ -68,11 +66,10 @@ public void onDetachView() {
     @SuppressLint("CheckResult")
     @Override
     public void initList(String entityId) {
-        this.entityId = entityId;
         view.setLoadingStatus(true);
         view.progressBarVisible(true);
         view.setIsLastPage(false);
-        compositeDisposable.add(depictsClient.fetchImagesForDepictedItem(entityId, 0)
+        compositeDisposable.add(mediaClient.fetchImagesForDepictedItem(entityId, 0)
                 .subscribeOn(ioScheduler)
                 .observeOn(mainThreadScheduler)
                 .subscribe(this::handleSuccess, this::handleError));
@@ -86,7 +83,7 @@ public void initList(String entityId) {
     @Override
     public void fetchMoreImages(String entityId) {
         view.progressBarVisible(true);
-        compositeDisposable.add(depictsClient.fetchImagesForDepictedItem(entityId, queryList.size())
+        compositeDisposable.add(mediaClient.fetchImagesForDepictedItem(entityId, queryList.size())
                 .subscribeOn(ioScheduler)
                 .observeOn(mainThreadScheduler)
                 .subscribe(this::handlePaginationSuccess, this::handleError));

app/src/main/java/fr/free/nrw/commons/depictions/models/DepictionResponse.java

@@ -1,8 +1,5 @@
 package fr.free.nrw.commons.explore.depictions
 
-import fr.free.nrw.commons.BuildConfig
-import fr.free.nrw.commons.Media
-import fr.free.nrw.commons.depictions.models.DepictionResponse
 import fr.free.nrw.commons.depictions.subClass.models.SparqlResponse
 import fr.free.nrw.commons.media.MediaInterface
 import fr.free.nrw.commons.upload.depicts.DepictsInterface
@@ -43,30 +40,6 @@ class DepictsClient @Inject constructor(
             .map { it.entities().values.map(::DepictedItem) }
     }
 
-    /**
-     * @return list of images for a particular depict entity
-     */
-    fun fetchImagesForDepictedItem(query: String, sroffset: Int): Single<List<Media>> {
-        return mediaInterface.fetchImagesForDepictedItem(
-            "haswbstatement:" + BuildConfig.DEPICTS_PROPERTY + "=" + query,
-            sroffset.toString()
-        )
-            .map { mwQueryResponse: DepictionResponse ->
-                mwQueryResponse.query
-                    .search
-                    .map {
-                        Media(
-                            getUrl(it.title),
-                            it.title,
-                            "",
-                            safeParseDate(it.timestamp),
-                            ""
-                        )
-                    }
-            }
-    }
-
-
     private fun getUrl(title: String): String {
         return getImageUrl(title, LARGE_IMAGE_SIZE)
     }

app/src/main/java/fr/free/nrw/commons/explore/depictions/DepictsClient.kt

@@ -1,5 +1,6 @@
 package fr.free.nrw.commons.media
 
+import fr.free.nrw.commons.BuildConfig
 import fr.free.nrw.commons.Media
 import fr.free.nrw.commons.depictions.Media.DepictedImagesFragment.PAGE_ID_PREFIX
 import fr.free.nrw.commons.explore.media.MediaConverter
@@ -100,6 +101,17 @@ class MediaClient @Inject constructor(
     fun getMediaListFromSearch(keyword: String?, limit: Int, offset: Int) =
         responseToMediaList(mediaInterface.getMediaListFromSearch(keyword, limit, offset))
 
+    /**
+     * @return list of images for a particular depict entity
+     */
+    fun fetchImagesForDepictedItem(query: String, sroffset: Int): Single<List<Media>> {
+        return responseToMediaList(mediaInterface.fetchImagesForDepictedItem(
+            "haswbstatement:" + BuildConfig.DEPICTS_PROPERTY + "=" + query,
+            sroffset.toString()
+        ))
+    }
+
+
     private fun responseToMediaList(
         response: Single<MwQueryResponse>,
         key: String? = null

app/src/main/java/fr/free/nrw/commons/media/MediaClient.kt

@@ -1,6 +1,5 @@
 package fr.free.nrw.commons.media;
 
-import fr.free.nrw.commons.depictions.models.DepictionResponse;
 import io.reactivex.Single;
 import java.util.Map;
 import org.wikipedia.dataclient.mwapi.MwQueryResponse;
@@ -123,7 +122,9 @@ Single<MwQueryResponse> getMediaListForUser(@Query("gaiuser") String username,
      * @param sroffset number od depictions already fetched, this is useful in implementing pagination
      */
 
-    @GET("w/api.php?action=query&list=search&format=json&srnamespace=6")
-    Single<DepictionResponse> fetchImagesForDepictedItem(@Query("srsearch") String query, @Query("sroffset") String sroffset);
+    @GET("w/api.php?action=query&format=json&formatversion=2" + //Basic parameters
+        "&generator=search&gsrnamespace=6" + //Search parameters
+        MEDIA_PARAMS)
+    Single<MwQueryResponse> fetchImagesForDepictedItem(@Query("gsrsearch") String query, @Query("gsroffset") String sroffset);
 
 }

app/src/main/java/fr/free/nrw/commons/media/MediaInterface.java

@@ -3,7 +3,6 @@ package fr.free.nrw.commons.depictions
 import fr.free.nrw.commons.Media
 import fr.free.nrw.commons.depictions.Media.DepictedImagesFragment
 import fr.free.nrw.commons.depictions.Media.DepictedImagesPresenter
-import fr.free.nrw.commons.explore.depictions.DepictsClient
 import fr.free.nrw.commons.kvstore.JsonKvStore
 import fr.free.nrw.commons.media.MediaClient
 import io.reactivex.Single
@@ -26,9 +25,6 @@ class DepictedImagesPresenterTest {
     @Mock
     lateinit var jsonKvStore: JsonKvStore
 
-    @Mock
-    lateinit var depictsClient: DepictsClient
-
     @Mock
     lateinit var mediaClient: MediaClient
 
@@ -49,14 +45,15 @@ class DepictedImagesPresenterTest {
         testScheduler = TestScheduler()
         mediaList.add(mediaItem)
         testSingle = Single.just(mediaList)
-        depictedImagesPresenter = DepictedImagesPresenter(jsonKvStore, depictsClient, mediaClient, testScheduler, testScheduler)
+        depictedImagesPresenter = DepictedImagesPresenter(jsonKvStore,
+            mediaClient, testScheduler, testScheduler)
         depictedImagesPresenter.onAttachView(view)
     }
 
     @Test
     fun initList() {
         Mockito.`when`(
-            depictsClient.fetchImagesForDepictedItem(ArgumentMatchers.anyString(),
+            mediaClient.fetchImagesForDepictedItem(ArgumentMatchers.anyString(),
                 ArgumentMatchers.anyInt())
         ).thenReturn(testSingle)
         depictedImagesPresenter.initList("rabbit")