From 48a4e1017035a3211f32189e6cc49e2079056591 Mon Sep 17 00:00:00 2001 From: Sean Mac Gillicuddy Date: Tue, 16 Jun 2020 12:05:01 +0100 Subject: [PATCH] #3808 Construct media objects that depict an item id correctly - use generator to construct media for DepictedImages --- .../main/java/fr/free/nrw/commons/Media.java | 34 +++------ .../Media/DepictedImagesPresenter.java | 15 ++-- .../depictions/models/DepictionResponse.java | 73 ------------------- .../explore/depictions/DepictsClient.kt | 27 ------- .../fr/free/nrw/commons/media/MediaClient.kt | 12 +++ .../nrw/commons/media/MediaInterface.java | 7 +- .../depictions/DepictedImagesPresenterTest.kt | 9 +-- 7 files changed, 35 insertions(+), 142 deletions(-) delete mode 100644 app/src/main/java/fr/free/nrw/commons/depictions/models/DepictionResponse.java diff --git a/app/src/main/java/fr/free/nrw/commons/Media.java b/app/src/main/java/fr/free/nrw/commons/Media.java index 77ddf94b9..032f3fb93 100644 --- a/app/src/main/java/fr/free/nrw/commons/Media.java +++ b/app/src/main/java/fr/free/nrw/commons/Media.java @@ -52,27 +52,6 @@ public class Media implements Parcelable { 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 class Media implements Parcelable { 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 captions, final String fallbackDescription, final String creator, final List 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 @@ public class Media implements Parcelable { } private static List readList(Parcel in) { - final ArrayList list = new ArrayList<>(); + final List list = new ArrayList<>(); in.readStringList(list); return list; } diff --git a/app/src/main/java/fr/free/nrw/commons/depictions/Media/DepictedImagesPresenter.java b/app/src/main/java/fr/free/nrw/commons/depictions/Media/DepictedImagesPresenter.java index df4c74d09..77df508a2 100644 --- a/app/src/main/java/fr/free/nrw/commons/depictions/Media/DepictedImagesPresenter.java +++ b/app/src/main/java/fr/free/nrw/commons/depictions/Media/DepictedImagesPresenter.java @@ -5,7 +5,6 @@ import static fr.free.nrw.commons.di.CommonsApplicationModule.MAIN_THREAD; 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 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 class DepictedImagesPresenter implements DepictedImagesContract.UserActio @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 class DepictedImagesPresenter implements DepictedImagesContract.UserActio @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)); diff --git a/app/src/main/java/fr/free/nrw/commons/depictions/models/DepictionResponse.java b/app/src/main/java/fr/free/nrw/commons/depictions/models/DepictionResponse.java deleted file mode 100644 index 15ea61d50..000000000 --- a/app/src/main/java/fr/free/nrw/commons/depictions/models/DepictionResponse.java +++ /dev/null @@ -1,73 +0,0 @@ -package fr.free.nrw.commons.depictions.models; -import com.google.gson.annotations.Expose; -import com.google.gson.annotations.SerializedName; - -/** - * Model class for list of depicted images obtained by fetching using depiction entity - */ -public class DepictionResponse { - - @SerializedName("batchcomplete") - @Expose - private String batchcomplete; - @SerializedName("continue") - @Expose - private Continue _continue; - @SerializedName("query") - @Expose - private Query query; - - /** - * No args constructor for use in serialization - * - */ - public DepictionResponse() { - } - - /** - * - * @param query - * @param batchcomplete - * @param _continue - */ - public DepictionResponse(String batchcomplete, Continue _continue, Query query) { - super(); - this.batchcomplete = batchcomplete; - this._continue = _continue; - this.query = query; - } - - /** - * returns batchcomplete string from DepictionResponse object - */ - public String getBatchcomplete() { - return batchcomplete; - } - - public void setBatchcomplete(String batchcomplete) { - this.batchcomplete = batchcomplete; - } - - /** - * returns continue object from DepictionResponse object - */ - public Continue getContinue() { - return _continue; - } - - public void setContinue(Continue _continue) { - this._continue = _continue; - } - - /** - * returns query object from DepictionResponse object - */ - public Query getQuery() { - return query; - } - - public void setQuery(Query query) { - this.query = query; - } - -} diff --git a/app/src/main/java/fr/free/nrw/commons/explore/depictions/DepictsClient.kt b/app/src/main/java/fr/free/nrw/commons/explore/depictions/DepictsClient.kt index 3bcd0d4b8..b9a4d15a0 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/depictions/DepictsClient.kt +++ b/app/src/main/java/fr/free/nrw/commons/explore/depictions/DepictsClient.kt @@ -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> { - 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) } diff --git a/app/src/main/java/fr/free/nrw/commons/media/MediaClient.kt b/app/src/main/java/fr/free/nrw/commons/media/MediaClient.kt index df115f810..293a2fc5a 100644 --- a/app/src/main/java/fr/free/nrw/commons/media/MediaClient.kt +++ b/app/src/main/java/fr/free/nrw/commons/media/MediaClient.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> { + return responseToMediaList(mediaInterface.fetchImagesForDepictedItem( + "haswbstatement:" + BuildConfig.DEPICTS_PROPERTY + "=" + query, + sroffset.toString() + )) + } + + private fun responseToMediaList( response: Single, key: String? = null diff --git a/app/src/main/java/fr/free/nrw/commons/media/MediaInterface.java b/app/src/main/java/fr/free/nrw/commons/media/MediaInterface.java index cb5721275..b9a17f132 100644 --- a/app/src/main/java/fr/free/nrw/commons/media/MediaInterface.java +++ b/app/src/main/java/fr/free/nrw/commons/media/MediaInterface.java @@ -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 @@ public interface MediaInterface { * @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 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 fetchImagesForDepictedItem(@Query("gsrsearch") String query, @Query("gsroffset") String sroffset); } diff --git a/app/src/test/kotlin/fr/free/nrw/commons/depictions/DepictedImagesPresenterTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/depictions/DepictedImagesPresenterTest.kt index b1b81b685..f10f814f3 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/depictions/DepictedImagesPresenterTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/depictions/DepictedImagesPresenterTest.kt @@ -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")