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 96df2a4ac..a82928718 100644 --- a/app/src/main/java/fr/free/nrw/commons/Media.java +++ b/app/src/main/java/fr/free/nrw/commons/Media.java @@ -1,6 +1,5 @@ package fr.free.nrw.commons; -import android.net.Uri; import android.os.Parcel; import android.os.Parcelable; import androidx.annotation.NonNull; @@ -8,44 +7,25 @@ import androidx.annotation.Nullable; import androidx.room.Entity; import androidx.room.PrimaryKey; import fr.free.nrw.commons.location.LatLng; -import fr.free.nrw.commons.media.Depictions; -import fr.free.nrw.commons.utils.CommonsDateUtil; -import fr.free.nrw.commons.utils.MediaDataExtractorUtil; -import java.text.ParseException; +import java.io.Serializable; import java.util.ArrayList; +import java.util.Collections; import java.util.Date; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Objects; import java.util.UUID; -import org.apache.commons.lang3.StringUtils; -import org.wikipedia.dataclient.mwapi.MwQueryPage; -import org.wikipedia.gallery.ExtMetadata; -import org.wikipedia.gallery.ImageInfo; +import org.jetbrains.annotations.NotNull; import org.wikipedia.page.PageTitle; @Entity public class Media implements Parcelable { - public static final Media EMPTY = new Media(""); - - // Primary metadata fields - @Nullable - private Uri localUri; private String thumbUrl; private String imageUrl; private String filename; - private String thumbnailTitle; - /* - * Captions are a feature part of Structured data. They are meant to store short, multilingual descriptions about files - * This is a replacement of the previously used titles for images (titles were not multilingual) - * Also now captions replace the previous convention of using title for filename - */ - private String caption; - private String description; // monolingual description on input... - private String discussion; - private long dataLength; - private Date dateCreated; + private String fallbackDescription; // monolingual description on input... @Nullable private Date dateUploaded; private String license; private String licenseUrl; @@ -57,13 +37,14 @@ public class Media implements Parcelable { @NonNull private String pageId; private List categories; // as loaded at runtime? - /** - * Depicts is a feature part of Structured data. Multiple Depictions can be added for an image just like categories. - * However unlike categories depictions is multi-lingual - */ - private Depictions depictions; - private boolean requestedDeletion; @Nullable private LatLng coordinates; + @NotNull + private Map captions = Collections.emptyMap(); + @NotNull + private Map descriptions = Collections.emptyMap(); + + @NotNull + private List depictionIds = Collections.emptyList(); /** * Provides local constructor @@ -72,6 +53,82 @@ public class Media implements Parcelable { pageId = UUID.randomUUID().toString(); } + /** + * Constructor with all parameters + */ + public Media(final String thumbUrl, + final String imageUrl, + final String filename, + final String fallbackDescription, + @Nullable final Date dateUploaded, + final String license, + final String licenseUrl, + final String creator, + @NonNull final String pageId, + final List categories, + @Nullable final LatLng coordinates, + @NotNull final Map captions, + @NotNull final Map descriptions, + @NotNull final List depictionIds) { + this.thumbUrl = thumbUrl; + this.imageUrl = imageUrl; + this.filename = filename; + this.fallbackDescription = fallbackDescription; + this.dateUploaded = dateUploaded; + this.license = license; + this.licenseUrl = licenseUrl; + this.creator = creator; + this.pageId = pageId; + this.categories = categories; + this.coordinates = coordinates; + this.captions = captions; + this.descriptions = descriptions; + this.depictionIds = depictionIds; + } + + 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()); + } + + public Media(final String filename, + Map captions, final String fallbackDescription, + final String creator, final List categories) { + 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; + } + + protected Media(final Parcel in) { + this(in.readString(), in.readString(), in.readString(), + in.readString(), readDateUploaded(in), in.readString(), + in.readString(), in.readString(), in.readString(), readList(in), + in.readParcelable(LatLng.class.getClassLoader()), + ((Map) in.readSerializable()), + ((Map) in.readSerializable()), + readList(in)); + } + + private static List readList(Parcel in) { + final List list = new ArrayList<>(); + in.readStringList(list); + return list; + } + + private static Date readDateUploaded(Parcel in) { + final long tmpDateUploaded = in.readLong(); + return tmpDateUploaded == -1 ? null : new Date(tmpDateUploaded); + } + public static final Creator CREATOR = new Creator() { @Override public Media createFromParcel(final Parcel source) { @@ -84,192 +141,6 @@ public class Media implements Parcelable { } }; - /** - * Provides a minimal constructor - * - * @param filename Media filename - */ - public Media(final String filename) { - this(); - this.filename = filename; - } - - /** - * Provide Media constructor - * @param localUri Media URI - * @param imageUrl Media image URL - * @param filename Media filename - * @param description Media description - * @param dataLength Media date length - * @param dateCreated Media creation date - * @param dateUploaded Media date uploaded - * @param creator Media creator - */ - public Media(final Uri localUri, final String imageUrl, final String filename, - final String description, - final long dataLength, final Date dateCreated, final Date dateUploaded, - final String creator) { - this(); - this.localUri = localUri; - thumbUrl = imageUrl; - this.imageUrl = imageUrl; - this.filename = filename; - this.description = description; - this.dataLength = dataLength; - this.dateCreated = dateCreated; - this.dateUploaded = dateUploaded; - this.creator = creator; - } - - /** - * Constructor with all parameters - */ - public Media(final String pageId, - final Uri localUri, - final String thumbUrl, - final String imageUrl, - final String filename, - final String description, - final String discussion, - final long dataLength, - final Date dateCreated, - final Date dateUploaded, - final String license, - final String licenseUrl, - final String creator, - final List categories, - final boolean requestedDeletion, - final LatLng coordinates) { - this.pageId = pageId; - this.localUri = localUri; - this.thumbUrl = thumbUrl; - this.imageUrl = imageUrl; - this.filename = filename; - this.description = description; - this.discussion = discussion; - this.dataLength = dataLength; - this.dateCreated = dateCreated; - this.dateUploaded = dateUploaded; - this.license = license; - this.licenseUrl = licenseUrl; - this.creator = creator; - this.categories = categories; - this.requestedDeletion = requestedDeletion; - this.coordinates = coordinates; - } - - public Media(final Uri localUri, final String filename, - final String description, final String creator, final List categories) { - this(localUri,null, filename, - description, -1, null, new Date(), creator); - this.categories = categories; - } - - public Media(final String title, final Date date, final String user) { - this(null, null, title, "", -1, date, date, user); - } - - protected Media(final Parcel in) { - localUri = in.readParcelable(Uri.class.getClassLoader()); - thumbUrl = in.readString(); - imageUrl = in.readString(); - filename = in.readString(); - thumbnailTitle = in.readString(); - caption = in.readString(); - description = in.readString(); - discussion = in.readString(); - dataLength = in.readLong(); - final long tmpDateCreated = in.readLong(); - dateCreated = tmpDateCreated == -1 ? null : new Date(tmpDateCreated); - final long tmpDateUploaded = in.readLong(); - dateUploaded = tmpDateUploaded == -1 ? null : new Date(tmpDateUploaded); - license = in.readString(); - licenseUrl = in.readString(); - creator = in.readString(); - pageId = in.readString(); - final ArrayList list = new ArrayList<>(); - in.readStringList(list); - categories = list; - in.readParcelable(Depictions.class.getClassLoader()); - requestedDeletion = in.readByte() != 0; - coordinates = in.readParcelable(LatLng.class.getClassLoader()); - } - - /** - * Creating Media object from MWQueryPage. - * Earlier only basic details were set for the media object but going forward, - * a full media object(with categories, descriptions, coordinates etc) can be constructed using this method - * - * @param page response from the API - * @return Media object - */ - @NonNull - public static Media from(final MwQueryPage page) { - final ImageInfo imageInfo = page.imageInfo(); - if (imageInfo == null) { - return new Media(); // null is not allowed - } - final ExtMetadata metadata = imageInfo.getMetadata(); - if (metadata == null) { - final Media media = new Media(null, imageInfo.getOriginalUrl(), - page.title(), "", 0, null, null, null); - if (!StringUtils.isBlank(imageInfo.getThumbUrl())) { - media.setThumbUrl(imageInfo.getThumbUrl()); - } - return media; - } - - final Media media = new Media(null, - imageInfo.getOriginalUrl(), - page.title(), - "", - 0, - safeParseDate(metadata.dateTime()), - safeParseDate(metadata.dateTime()), - getArtist(metadata) - ); - - if (!StringUtils.isBlank(imageInfo.getThumbUrl())) { - media.setThumbUrl(imageInfo.getThumbUrl()); - } - - media.setPageId(String.valueOf(page.pageId())); - - String language = Locale.getDefault().getLanguage(); - if (StringUtils.isBlank(language)) { - language = "default"; - } - - media.setDescription(metadata.imageDescription()); - media.setCategories(MediaDataExtractorUtil.extractCategoriesFromList(metadata.getCategories())); - final String latitude = metadata.getGpsLatitude(); - final String longitude = metadata.getGpsLongitude(); - - if (!StringUtils.isBlank(latitude) && !StringUtils.isBlank(longitude)) { - final LatLng latLng = new LatLng(Double.parseDouble(latitude), - Double.parseDouble(longitude), 0); - media.setCoordinates(latLng); - } - - media.setLicenseInformation(metadata.licenseShortName(), metadata.licenseUrl()); - return media; - } - - /** - * This method extracts the Commons Username from the artist HTML information - * @param metadata - * @return - */ - private static String getArtist(final ExtMetadata metadata) { - try { - final String artistHtml = metadata.artist(); - return artistHtml.substring(artistHtml.indexOf("title=\""), artistHtml.indexOf("\">")) - .replace("title=\"User:", ""); - } catch (final Exception ex) { - return ""; - } - } - @Nullable public String getThumbUrl() { return thumbUrl; @@ -283,23 +154,6 @@ public class Media implements Parcelable { return filename != null ? getPageTitle().getDisplayTextWithoutNamespace().replaceFirst("[.][^.]+$", "") : ""; } - @Nullable - private static Date safeParseDate(final String dateStr) { - try { - return CommonsDateUtil.getMediaSimpleDateFormat().parse(dateStr); - } catch (final ParseException e) { - return null; - } - } - - /** - * @return title to be shown on image thumbnail - * If caption is available for the image then it returns caption else filename - */ - public String getThumbnailTitle() { - return thumbnailTitle != null? thumbnailTitle : getDisplayTitle(); - } - /** * Gets file page title * @return New media page title @@ -308,13 +162,6 @@ public class Media implements Parcelable { return Utils.getPageTitle(getFilename()); } - /** - * Gets local URI - * @return Media local URI - */ - public Uri getLocalUri() { - return localUri; - } /** * Gets image URL @@ -348,38 +195,12 @@ public class Media implements Parcelable { this.pageId = pageId; } - /** - * Gets the file discussion as a string. - * @return file discussion as a string - */ - public String getDiscussion() { - return discussion; - } - /** * Gets the file description. * @return file description as a string */ - public String getDescription() { - return description; - } - - /** - * Captions are a feature part of Structured data. They are meant to store short, multilingual descriptions about files - * This is a replacement of the previously used titles for images (titles were not multilingual) - * Also now captions replace the previous convention of using title for filename - * - * @return caption - */ - public String getCaption() { - return caption; - } - - /** - * @return depictions associated with the current media - */ - public Depictions getDepiction() { - return depictions; + public String getFallbackDescription() { + return fallbackDescription; } /** @@ -390,36 +211,12 @@ public class Media implements Parcelable { this.filename = filename; } - /** - * Gets the dataLength of the file. - * @return file dataLength as a long - */ - public long getDataLength() { - return dataLength; - } - - /** - * Sets the discussion of the file. - * @param discussion - */ - public void setDiscussion(final String discussion) { - this.discussion = discussion; - } - - /** - * Gets the creation date of the file. - * @return creation date as a Date - */ - public Date getDateCreated() { - return dateCreated; - } - /** * Sets the file description. - * @param description the new description of the file + * @param fallbackDescription the new description of the file */ - public void setDescription(final String description) { - this.description = description; + public void setFallbackDescription(final String fallbackDescription) { + this.fallbackDescription = fallbackDescription; } /** @@ -440,14 +237,6 @@ public class Media implements Parcelable { return creator; } - /** - * Sets the dataLength of the file. - * @param dataLength as a long - */ - public void setDataLength(final long dataLength) { - this.dataLength = dataLength; - } - /** * Gets the license name of the file. * @return license as a String @@ -456,13 +245,6 @@ public class Media implements Parcelable { return license; } - /** - * Set Caption(if available) as the thumbnail title of the image - */ - public void setThumbnailTitle(final String title) { - thumbnailTitle = title; - } - public String getLicenseUrl() { return licenseUrl; } @@ -492,24 +274,10 @@ public class Media implements Parcelable { * Gets the categories the file falls under. * @return file categories as an ArrayList of Strings */ - @SuppressWarnings("unchecked") public List getCategories() { return categories; } - /** - * Sets the license name of the file. - * @param license license name as a String - */ - public void setLicenseInformation(final String license, String licenseUrl) { - this.license = license; - - if (!licenseUrl.startsWith("http://") && !licenseUrl.startsWith("https://")) { - licenseUrl = "https://" + licenseUrl; - } - this.licenseUrl = licenseUrl; - } - /** * Sets the coordinates of where the file was created. * @param coordinates file coordinates as a LatLng @@ -523,7 +291,18 @@ public class Media implements Parcelable { * @return */ public String getWikiCode() { - return String.format("[[%s|thumb|%s]]", filename, thumbnailTitle); + return String.format("[[%s|thumb|%s]]", filename, getMostRelevantCaption()); + } + + public String getMostRelevantCaption() { + final String languageAppropriateCaption = captions.get(Locale.getDefault().getLanguage()); + if (languageAppropriateCaption != null) { + return languageAppropriateCaption; + } + for (String firstCaption : captions.values()) { + return firstCaption; + } + return getDisplayTitle(); } /** @@ -537,21 +316,6 @@ public class Media implements Parcelable { this.categories = categories; } - /** - * Get the value of requested deletion - * @return boolean requestedDeletion - */ - public boolean isRequestedDeletion(){ - return requestedDeletion; - } - - /** - * Set requested deletion to true - * @param requestedDeletion - */ - public void setRequestedDeletion(final boolean requestedDeletion) { - this.requestedDeletion = requestedDeletion; - } /** * Sets the license name of the file. @@ -562,22 +326,6 @@ public class Media implements Parcelable { this.license = license; } - /** - * Captions are a feature part of Structured data. They are meant to store short, multilingual descriptions about files - * This is a replacement of the previously used titles for images (titles were not multilingual) - * Also now captions replace the previous convention of using title for filename - * - * This function sets captions - * @param caption - */ - public void setCaption(final String caption) { - this.caption = caption; - } - - public void setLocalUri(@Nullable final Uri localUri) { - this.localUri = localUri; - } - public void setImageUrl(final String imageUrl) { this.imageUrl = imageUrl; } @@ -590,28 +338,11 @@ public class Media implements Parcelable { this.licenseUrl = licenseUrl; } - public Depictions getDepictions() { - return depictions; - } - @Override public int describeContents() { return 0; } - /* Sets depictions for the current media obtained fro Wikibase API*/ - public void setDepictions(final Depictions depictions) { - this.depictions = depictions; - } - - /** - * Sets the creation date of the file. - * @param date creation date as a Date - */ - public void setDateCreated(final Date date) { - dateCreated = date; - } - /** * Creates a way to transfer information between two or more * activities. @@ -620,69 +351,76 @@ public class Media implements Parcelable { */ @Override public void writeToParcel(final Parcel dest, final int flags) { - dest.writeParcelable(localUri, flags); dest.writeString(thumbUrl); dest.writeString(imageUrl); dest.writeString(filename); - dest.writeString(thumbnailTitle); - dest.writeString(caption); - dest.writeString(description); - dest.writeString(discussion); - dest.writeLong(dataLength); - dest.writeLong(dateCreated != null ? dateCreated.getTime() : -1); + dest.writeString(fallbackDescription); dest.writeLong(dateUploaded != null ? dateUploaded.getTime() : -1); dest.writeString(license); dest.writeString(licenseUrl); dest.writeString(creator); dest.writeString(pageId); dest.writeStringList(categories); - dest.writeParcelable(depictions, flags); - dest.writeByte(requestedDeletion ? (byte) 1 : (byte) 0); dest.writeParcelable(coordinates, flags); + dest.writeSerializable((Serializable) captions); + dest.writeSerializable((Serializable) descriptions); + dest.writeList(depictionIds); + } + + public Map getCaptions() { + return captions; + } + + public void setCaptions(Map captions) { + this.captions = captions; + } + + public Map getDescriptions() { + return descriptions; + } + + public void setDescriptions(Map descriptions) { + this.descriptions = descriptions; + } + + public List getDepictionIds() { + return depictionIds; + } + + public void setDepictionIds(final List depictionIds) { + this.depictionIds = depictionIds; } - /** - * Equals implementation that matches all parameters for equality check - */ @Override public boolean equals(final Object o) { if (this == o) { return true; } - if (!(o instanceof Media)) { + if (o == null || getClass() != o.getClass()) { return false; } final Media media = (Media) o; - return getDataLength() == media.getDataLength() && - isRequestedDeletion() == media.isRequestedDeletion() && - Objects.equals(getLocalUri(), media.getLocalUri()) && - Objects.equals(getThumbUrl(), media.getThumbUrl()) && - Objects.equals(getImageUrl(), media.getImageUrl()) && - Objects.equals(getFilename(), media.getFilename()) && - Objects.equals(getThumbnailTitle(), media.getThumbnailTitle()) && - Objects.equals(getCaption(), media.getCaption()) && - Objects.equals(getDescription(), media.getDescription()) && - Objects.equals(getDiscussion(), media.getDiscussion()) && - Objects.equals(getDateCreated(), media.getDateCreated()) && - Objects.equals(getDateUploaded(), media.getDateUploaded()) && - Objects.equals(getLicense(), media.getLicense()) && - Objects.equals(getLicenseUrl(), media.getLicenseUrl()) && - Objects.equals(getCreator(), media.getCreator()) && - getPageId().equals(media.getPageId()) && - Objects.equals(getCategories(), media.getCategories()) && - Objects.equals(getDepictions(), media.getDepictions()) && - Objects.equals(getCoordinates(), media.getCoordinates()); + return Objects.equals(thumbUrl, media.thumbUrl) && + Objects.equals(imageUrl, media.imageUrl) && + Objects.equals(filename, media.filename) && + Objects.equals(fallbackDescription, media.fallbackDescription) && + Objects.equals(dateUploaded, media.dateUploaded) && + Objects.equals(license, media.license) && + Objects.equals(licenseUrl, media.licenseUrl) && + Objects.equals(creator, media.creator) && + pageId.equals(media.pageId) && + Objects.equals(categories, media.categories) && + Objects.equals(coordinates, media.coordinates) && + captions.equals(media.captions) && + descriptions.equals(media.descriptions) && + depictionIds.equals(media.depictionIds); } - /** - * Hashcode implementation that uses all parameters for calculating hash - */ @Override public int hashCode() { return Objects - .hash(getLocalUri(), getThumbUrl(), getImageUrl(), getFilename(), getThumbnailTitle(), - getCaption(), getDescription(), getDiscussion(), getDataLength(), getDateCreated(), - getDateUploaded(), getLicense(), getLicenseUrl(), getCreator(), getPageId(), - getCategories(), getDepictions(), isRequestedDeletion(), getCoordinates()); + .hash(thumbUrl, imageUrl, filename, fallbackDescription, dateUploaded, license, + licenseUrl, + creator, pageId, categories, coordinates, captions, descriptions, depictionIds); } } diff --git a/app/src/main/java/fr/free/nrw/commons/MediaDataExtractor.kt b/app/src/main/java/fr/free/nrw/commons/MediaDataExtractor.kt index 041507f76..d5adb84ed 100644 --- a/app/src/main/java/fr/free/nrw/commons/MediaDataExtractor.kt +++ b/app/src/main/java/fr/free/nrw/commons/MediaDataExtractor.kt @@ -1,11 +1,10 @@ package fr.free.nrw.commons import androidx.core.text.HtmlCompat -import fr.free.nrw.commons.depictions.Media.DepictedImagesFragment -import fr.free.nrw.commons.media.Depictions +import fr.free.nrw.commons.depictions.Media.DepictedImagesFragment.PAGE_ID_PREFIX +import fr.free.nrw.commons.media.IdAndCaptions import fr.free.nrw.commons.media.MediaClient import io.reactivex.Single -import io.reactivex.functions.Function5 import timber.log.Timber import javax.inject.Inject import javax.inject.Singleton @@ -13,107 +12,40 @@ import javax.inject.Singleton /** * Fetch additional media data from the network that we don't store locally. * - * This includes things like category lists and multilingual descriptions, - * which are not intrinsic to the media and may change due to editing. + * + * This includes things like category lists and multilingual descriptions, which are not intrinsic + * to the media and may change due to editing. */ @Singleton class MediaDataExtractor @Inject constructor(private val mediaClient: MediaClient) { - /** - * Simplified method to extract all details required to show media details. - * It fetches media object, deletion status, talk page and captions for the filename - * @param filename for which the details are to be fetched - * @return full Media object with all details including deletion status and talk page - */ - fun fetchMediaDetails(filename: String, pageId: String?): Single { - return Single.zip( - getMediaFromFileName(filename), - mediaClient.checkPageExistsUsingTitle("Commons:Deletion_requests/$filename"), - getDiscussion(filename), - if (pageId != null) - getCaption(DepictedImagesFragment.PAGE_ID_PREFIX + pageId) - else Single.just(MediaClient.NO_CAPTION), - getDepictions(filename), - Function5 { media: Media, deletionStatus: Boolean, discussion: String, caption: String, depictions: Depictions -> - combineToMedia( - media, - deletionStatus, - discussion, - caption, - depictions - ) + fun fetchDepictionIdsAndLabels(media: Media) = + mediaClient.getEntities(media.depictionIds) + .map { + it.entities() + .mapValues { entry -> entry.value.labels().mapValues { it.value.value() } } } - ) - } + .map { it.map { (key, value) -> IdAndCaptions(key, value) } } + .onErrorReturn { emptyList() } - private fun combineToMedia( - media: Media, - deletionStatus: Boolean, - discussion: String, - caption: String, - depictions: Depictions - ): Media { - media.discussion = discussion - media.caption = caption - media.depictions = depictions - if (deletionStatus) { - media.isRequestedDeletion = true - } - return media - } + fun checkDeletionRequestExists(media: Media) = + mediaClient.checkPageExistsUsingTitle("Commons:Deletion_requests/" + media.filename) - /** - * Obtains captions using filename - * @param wikibaseIdentifier - * - * @return caption for the image in user's locale - * Ex: "a nice painting" (english locale) and "No Caption" in case the caption is not available for the image - */ - private fun getCaption(wikibaseIdentifier: String): Single { - return mediaClient.getCaptionByWikibaseIdentifier(wikibaseIdentifier) - } - - /** - * Fetch depictions from the MediaWiki API - * @param filename the filename we will return the caption for - * @return Depictions - */ - private fun getDepictions(filename: String): Single { - return mediaClient.getDepictions(filename) - .doOnError { throwable: Throwable? -> - Timber.e( - throwable, - "error while fetching depictions" - ) - } - } - - /** - * Method can be used to fetch media for a given filename - * @param filename Eg. File:Test.jpg - * @return return data rich Media object - */ - fun getMediaFromFileName(filename: String?): Single { - return mediaClient.getMedia(filename) - } - - /** - * Fetch talk page from the MediaWiki API - * @param filename - * @return - */ - private fun getDiscussion(filename: String): Single { - return mediaClient.getPageHtml(filename.replace("File", "File talk")) - .map { discussion: String? -> - HtmlCompat.fromHtml( - discussion!!, - HtmlCompat.FROM_HTML_MODE_LEGACY - ).toString() - } - .onErrorReturn { throwable: Throwable? -> - Timber.e(throwable, "Error occurred while fetching discussion") + fun fetchDiscussion(media: Media) = + mediaClient.getPageHtml(media.filename.replace("File", "File talk")) + .map { HtmlCompat.fromHtml(it, HtmlCompat.FROM_HTML_MODE_LEGACY).toString() } + .onErrorReturn { + Timber.d("Error occurred while fetching discussion") "" } - } + fun refresh(media: Media): Single { + return Single.ambArray( + mediaClient.getMediaById(PAGE_ID_PREFIX + media.pageId) + .onErrorResumeNext { Single.never() }, + mediaClient.getMedia(media.filename) + .onErrorResumeNext { Single.never() } + ) + + } } diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/pictures/BookmarkPicturesController.java b/app/src/main/java/fr/free/nrw/commons/bookmarks/pictures/BookmarkPicturesController.java index 3274809e9..abab7a1c3 100644 --- a/app/src/main/java/fr/free/nrw/commons/bookmarks/pictures/BookmarkPicturesController.java +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/pictures/BookmarkPicturesController.java @@ -1,13 +1,5 @@ package fr.free.nrw.commons.bookmarks.pictures; -import org.apache.commons.lang3.StringUtils; - -import java.util.ArrayList; -import java.util.List; - -import javax.inject.Inject; -import javax.inject.Singleton; - import fr.free.nrw.commons.Media; import fr.free.nrw.commons.bookmarks.Bookmark; import fr.free.nrw.commons.media.MediaClient; @@ -15,6 +7,10 @@ import io.reactivex.Observable; import io.reactivex.ObservableSource; import io.reactivex.Single; import io.reactivex.functions.Function; +import java.util.ArrayList; +import java.util.List; +import javax.inject.Inject; +import javax.inject.Singleton; @Singleton public class BookmarkPicturesController { @@ -40,16 +36,13 @@ public class BookmarkPicturesController { currentBookmarks = bookmarks; return Observable.fromIterable(bookmarks) .flatMap((Function>) this::getMediaFromBookmark) - .filter(media -> media != null && !StringUtils.isBlank(media.getFilename())) .toList(); } private Observable getMediaFromBookmark(Bookmark bookmark) { - Media dummyMedia = new Media(""); return mediaClient.getMedia(bookmark.getMediaName()) - .map(media -> media == null ? dummyMedia : media) - .onErrorReturn(throwable -> dummyMedia) - .toObservable(); + .toObservable() + .onErrorResumeNext(Observable.empty()); } /** diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategoryImagesListFragment.java b/app/src/main/java/fr/free/nrw/commons/category/CategoryImagesListFragment.java index f0c78ed24..a7faab005 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategoryImagesListFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategoryImagesListFragment.java @@ -2,7 +2,6 @@ package fr.free.nrw.commons.category; import static android.view.View.GONE; import static android.view.View.VISIBLE; -import static fr.free.nrw.commons.depictions.Media.DepictedImagesFragment.PAGE_ID_PREFIX; import android.annotation.SuppressLint; import android.os.Bundle; @@ -253,37 +252,6 @@ public class CategoryImagesListFragment extends DaggerFragment { progressBar.setVisibility(GONE); isLoading = false; statusTextView.setVisibility(GONE); - for (Media m : collection) { - final String pageId = m.getPageId(); - if (pageId != null) { - replaceTitlesWithCaptions(PAGE_ID_PREFIX + pageId, mediaSize++); - } - } - } - - /** - * fetch captions for the image using filename and replace title of on the image thumbnail(if captions are available) - * else show filename - */ - public void replaceTitlesWithCaptions(String wikibaseIdentifier, int i) { - compositeDisposable.add(mediaClient.getCaptionByWikibaseIdentifier(wikibaseIdentifier) - .subscribeOn(Schedulers.io()) - .observeOn(AndroidSchedulers.mainThread()) - .subscribe(subscriber -> { - handleLabelforImage(subscriber, i); - })); - - } - - /** - * If caption is available for the image, then modify grid adapter - * to show captions - */ - private void handleLabelforImage(String s, int position) { - if (!s.trim().equals(getString(R.string.detail_caption_empty))) { - gridAdapter.getItem(position).setThumbnailTitle(s); - gridAdapter.notifyDataSetChanged(); - } } /** diff --git a/app/src/main/java/fr/free/nrw/commons/category/GridViewAdapter.java b/app/src/main/java/fr/free/nrw/commons/category/GridViewAdapter.java index c983af03a..e8aab51bc 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/GridViewAdapter.java +++ b/app/src/main/java/fr/free/nrw/commons/category/GridViewAdapter.java @@ -88,7 +88,7 @@ public class GridViewAdapter extends ArrayAdapter { SimpleDraweeView imageView = convertView.findViewById(R.id.categoryImageView); TextView fileName = convertView.findViewById(R.id.categoryImageTitle); TextView author = convertView.findViewById(R.id.categoryImageAuthor); - fileName.setText(item.getThumbnailTitle()); + fileName.setText(item.getMostRelevantCaption()); setAuthorView(item, author); imageView.setImageURI(item.getThumbUrl()); return convertView; diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/Contribution.java b/app/src/main/java/fr/free/nrw/commons/contributions/Contribution.java index 4b969be03..7bfc8debf 100644 --- a/app/src/main/java/fr/free/nrw/commons/contributions/Contribution.java +++ b/app/src/main/java/fr/free/nrw/commons/contributions/Contribution.java @@ -1,17 +1,18 @@ package fr.free.nrw.commons.contributions; +import android.net.Uri; import android.os.Parcel; +import androidx.annotation.Nullable; import androidx.room.Entity; import fr.free.nrw.commons.Media; import fr.free.nrw.commons.auth.SessionManager; -import fr.free.nrw.commons.upload.UploadMediaDetail; import fr.free.nrw.commons.upload.UploadItem; +import fr.free.nrw.commons.upload.UploadMediaDetail; import fr.free.nrw.commons.upload.WikidataPlace; import fr.free.nrw.commons.upload.structure.depictions.DepictedItem; import java.util.ArrayList; -import java.util.HashMap; +import java.util.Date; import java.util.List; -import java.util.Map; import java.util.Objects; @Entity(tableName = "contribution") @@ -34,24 +35,23 @@ public class Contribution extends Media { */ private List depictedItems = new ArrayList<>(); private String mimeType; - /** - * This hasmap stores the list of multilingual captions, where key of the HashMap is the language - * and value is the caption in the corresponding language Ex: key = "en", value: "" key = "de" , value: "" - */ - private HashMap captions = new HashMap<>(); + @Nullable + private Uri localUri; + private long dataLength; + private Date dateCreated; public Contribution() { } public Contribution(final UploadItem item, final SessionManager sessionManager, final List depictedItems, final List categories) { - super(item.getMediaUri(), + super( item.getFileName(), - UploadMediaDetail.formatList(item.getUploadMediaDetails()), + UploadMediaDetail.formatCaptions(item.getUploadMediaDetails()), + UploadMediaDetail.formatDescriptions(item.getUploadMediaDetails()), sessionManager.getAuthorName(), categories); - captions = new HashMap<>(UploadMediaDetail.formatCaptions(item.getUploadMediaDetails())); + localUri = item.getMediaUri(); decimalCoords = item.getGpsCoords().getDecimalCoords(); dateCreatedSource = ""; this.depictedItems = depictedItems; @@ -117,24 +117,6 @@ public class Contribution extends Media { this.mimeType = mimeType; } - /** - * Captions are a feature part of Structured data. They are meant to store short, multilingual - * descriptions about files This is a replacement of the previously used titles for images (titles - * were not multilingual) Also now captions replace the previous convention of using title for - * filename - *

- * key of the HashMap is the language and value is the caption in the corresponding language - *

- * returns list of captions stored in hashmap - */ - public HashMap getCaptions() { - return captions; - } - - public void setCaptions(HashMap captions) { - this.captions = captions; - } - @Override public int describeContents() { return 0; @@ -147,7 +129,6 @@ public class Contribution extends Media { dest.writeLong(transferred); dest.writeString(decimalCoords); dest.writeString(dateCreatedSource); - dest.writeSerializable(captions); } /** @@ -156,13 +137,7 @@ public class Contribution extends Media { * @param state */ public Contribution(Media media, int state) { - super(media.getPageId(), - media.getLocalUri(), media.getThumbUrl(), media.getImageUrl(), media.getFilename(), - media.getDescription(), - media.getDiscussion(), - media.getDataLength(), media.getDateCreated(), media.getDateUploaded(), - media.getLicense(), media.getLicenseUrl(), media.getCreator(), media.getCategories(), - media.isRequestedDeletion(), media.getCoordinates()); + super(media); this.state = state; } @@ -172,7 +147,6 @@ public class Contribution extends Media { transferred = in.readLong(); decimalCoords = in.readString(); dateCreatedSource = in.readString(); - captions = (HashMap) in.readSerializable(); } public static final Creator CREATOR = new Creator() { @@ -187,34 +161,60 @@ public class Contribution extends Media { } }; - /** - * Equals implementation of Contributions that compares all parameters for checking equality - */ + @Nullable + public Uri getLocalUri() { + return localUri; + } + + public void setLocalUri(@Nullable Uri localUri) { + this.localUri = localUri; + } + + public long getDataLength() { + return dataLength; + } + + public void setDataLength(long dataLength) { + this.dataLength = dataLength; + } + + public Date getDateCreated() { + return dateCreated; + } + + public void setDateCreated(Date dateCreated) { + this.dateCreated = dateCreated; + } + @Override public boolean equals(final Object o) { if (this == o) { return true; } - if (!(o instanceof Contribution)) { + if (o == null || getClass() != o.getClass()) { + return false; + } + if (!super.equals(o)) { return false; } final Contribution that = (Contribution) o; - return getState() == that.getState() && getTransferred() == that.getTransferred() && Objects - .equals(getDecimalCoords(), that.getDecimalCoords()) && Objects - .equals(getDateCreatedSource(), that.getDateCreatedSource()) && Objects - .equals(getWikidataPlace(), that.getWikidataPlace()) && Objects - .equals(getDepictedItems(), that.getDepictedItems()) && Objects - .equals(getMimeType(), that.getMimeType()) && Objects - .equals(getCaptions(), that.getCaptions()); + return state == that.state && + transferred == that.transferred && + dataLength == that.dataLength && + Objects.equals(decimalCoords, that.decimalCoords) && + Objects.equals(dateCreatedSource, that.dateCreatedSource) && + Objects.equals(wikidataPlace, that.wikidataPlace) && + Objects.equals(depictedItems, that.depictedItems) && + Objects.equals(mimeType, that.mimeType) && + Objects.equals(localUri, that.localUri) && + Objects.equals(dateCreated, that.dateCreated); } - /** - * Hash code implementation of contributions that considers all parameters for calculating hash. - */ @Override public int hashCode() { return Objects - .hash(getState(), getTransferred(), getDecimalCoords(), getDateCreatedSource(), - getWikidataPlace(), getDepictedItems(), getMimeType(), getCaptions()); + .hash(super.hashCode(), state, transferred, decimalCoords, dateCreatedSource, + wikidataPlace, + depictedItems, mimeType, localUri, dataLength, dateCreated); } } diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionBoundaryCallback.kt b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionBoundaryCallback.kt index 598b88c7f..a001da3ca 100644 --- a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionBoundaryCallback.kt +++ b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionBoundaryCallback.kt @@ -62,9 +62,7 @@ class ContributionBoundaryCallback @Inject constructor( } } .subscribeOn(ioThreadScheduler) - .subscribe( - ::saveContributionsToDB - ) { error: Throwable -> + .subscribe(::saveContributionsToDB) { error: Throwable -> Timber.e( "Failed to fetch contributions: %s", error.message diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionViewHolder.java b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionViewHolder.java index 1064d86ca..9d6823603 100644 --- a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionViewHolder.java +++ b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionViewHolder.java @@ -1,12 +1,9 @@ package fr.free.nrw.commons.contributions; -import static fr.free.nrw.commons.depictions.Media.DepictedImagesFragment.PAGE_ID_PREFIX; - import android.net.Uri; import android.text.TextUtils; import android.view.View; import android.widget.ImageButton; -import android.widget.LinearLayout; import android.widget.ProgressBar; import android.widget.RelativeLayout; import android.widget.TextView; @@ -24,8 +21,6 @@ import fr.free.nrw.commons.media.MediaClient; import io.reactivex.android.schedulers.AndroidSchedulers; import io.reactivex.disposables.CompositeDisposable; import io.reactivex.schedulers.Schedulers; -import org.wikipedia.dataclient.WikiSite; -import timber.log.Timber; public class ContributionViewHolder extends RecyclerView.ViewHolder { @@ -65,8 +60,8 @@ public class ContributionViewHolder extends RecyclerView.ViewHolder { public void init(final int position, final Contribution contribution) { this.contribution = contribution; - fetchAndDisplayCaption(contribution); this.position = position; + titleView.setText(contribution.getMostRelevantCaption()); final String imageSource = chooseImageSource(contribution.getThumbUrl(), contribution.getLocalUri()); if (!TextUtils.isEmpty(imageSource)) { @@ -116,37 +111,6 @@ public class ContributionViewHolder extends RecyclerView.ViewHolder { } } - /** - * In contributions first we show the title for the image stored in cache, then we fetch captions - * associated with the image and replace title on the thumbnail with caption - * - * @param contribution - */ - private void fetchAndDisplayCaption(final Contribution contribution) { - if ((contribution.getState() != Contribution.STATE_COMPLETED)) { - titleView.setText(contribution.getDisplayTitle()); - } else { - final String pageId = contribution.getPageId(); - if (pageId != null) { - Timber.d("Fetching caption for %s", contribution.getFilename()); - final String wikibaseMediaId = PAGE_ID_PREFIX - + pageId; // Create Wikibase media id from the page id. Example media id: M80618155 for https://commons.wikimedia.org/wiki/File:Tantanmen.jpeg with has the pageid 80618155 - compositeDisposable.add(mediaClient.getCaptionByWikibaseIdentifier(wikibaseMediaId) - .subscribeOn(Schedulers.io()) - .observeOn(AndroidSchedulers.mainThread()) - .subscribe(subscriber -> { - if (!subscriber.trim().equals(MediaClient.NO_CAPTION)) { - titleView.setText(subscriber); - } else { - titleView.setText(contribution.getDisplayTitle()); - } - })); - } else { - titleView.setText(contribution.getDisplayTitle()); - } - } - } - /** * Checks if a media exists on the corresponding Wikipedia article Currently the check is made for * the device's current language Wikipedia diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsContract.java b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsContract.java index d578d1185..f73ce5501 100644 --- a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsContract.java +++ b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsContract.java @@ -1,9 +1,6 @@ package fr.free.nrw.commons.contributions; -import java.util.List; - import fr.free.nrw.commons.BasePresenter; -import fr.free.nrw.commons.Media; /** * The contract for Contributions View & Presenter @@ -21,8 +18,5 @@ public class ContributionsContract { void deleteUpload(Contribution contribution); - void updateContribution(Contribution contribution); - - void fetchMediaDetails(Contribution contribution); } } diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsListAdapter.java b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsListAdapter.java index 0b2556f0e..0b777043b 100644 --- a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsListAdapter.java +++ b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsListAdapter.java @@ -46,9 +46,8 @@ public class ContributionsListAdapter extends * Initializes the view holder with contribution data */ @Override - public void onBindViewHolder(@NonNull final ContributionViewHolder holder, final int position) { - final Contribution contribution = getItem(position); - holder.init(position, contribution); + public void onBindViewHolder(@NonNull ContributionViewHolder holder, int position) { + holder.init(position, getItem(position)); } Contribution getContributionForPosition(final int position) { diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsPresenter.java b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsPresenter.java index b4725fed8..2cae4f04c 100644 --- a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsPresenter.java +++ b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsPresenter.java @@ -1,30 +1,10 @@ package fr.free.nrw.commons.contributions; -import android.content.Context; -import androidx.annotation.NonNull; -import androidx.annotation.Nullable; -import androidx.lifecycle.LifecycleOwner; -import androidx.lifecycle.LiveData; -import fr.free.nrw.commons.CommonsApplication; -import fr.free.nrw.commons.Media; -import fr.free.nrw.commons.MediaDataExtractor; -import fr.free.nrw.commons.auth.SessionManager; import fr.free.nrw.commons.MediaDataExtractor; import fr.free.nrw.commons.contributions.ContributionsContract.UserActionListener; import fr.free.nrw.commons.di.CommonsApplicationModule; -import fr.free.nrw.commons.mwapi.UserClient; -import fr.free.nrw.commons.utils.NetworkUtils; import io.reactivex.Scheduler; -import io.reactivex.android.schedulers.AndroidSchedulers; import io.reactivex.disposables.CompositeDisposable; -import io.reactivex.schedulers.Schedulers; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import javax.inject.Inject; -import javax.inject.Named; -import timber.log.Timber; - import javax.inject.Inject; import javax.inject.Named; @@ -77,24 +57,4 @@ public class ContributionsPresenter implements UserActionListener { .subscribeOn(ioThreadScheduler) .subscribe()); } - - @Override - public void updateContribution(Contribution contribution) { - compositeDisposable.add(repository - .updateContribution(contribution) - .subscribeOn(ioThreadScheduler) - .subscribe()); - } - - @Override - public void fetchMediaDetails(Contribution contribution) { - compositeDisposable.add(mediaDataExtractor - .getMediaFromFileName(contribution.getFilename()) - .subscribeOn(Schedulers.io()) - .observeOn(AndroidSchedulers.mainThread()) - .subscribe(media -> { - contribution.setThumbUrl(media.getThumbUrl()); - updateContribution(contribution); - })); - } } diff --git a/app/src/main/java/fr/free/nrw/commons/db/AppDatabase.kt b/app/src/main/java/fr/free/nrw/commons/db/AppDatabase.kt index a45fd0c6e..35b4c551d 100644 --- a/app/src/main/java/fr/free/nrw/commons/db/AppDatabase.kt +++ b/app/src/main/java/fr/free/nrw/commons/db/AppDatabase.kt @@ -10,7 +10,7 @@ import fr.free.nrw.commons.contributions.ContributionDao * The database for accessing the respective DAOs * */ -@Database(entities = [Contribution::class], version = 2, exportSchema = false) +@Database(entities = [Contribution::class], version = 3, exportSchema = false) @TypeConverters(Converters::class) abstract class AppDatabase : RoomDatabase() { abstract fun contributionDao(): ContributionDao diff --git a/app/src/main/java/fr/free/nrw/commons/db/Converters.java b/app/src/main/java/fr/free/nrw/commons/db/Converters.java index b5326f777..6eecebaec 100644 --- a/app/src/main/java/fr/free/nrw/commons/db/Converters.java +++ b/app/src/main/java/fr/free/nrw/commons/db/Converters.java @@ -7,11 +7,9 @@ import com.google.gson.reflect.TypeToken; import fr.free.nrw.commons.CommonsApplication; import fr.free.nrw.commons.di.ApplicationlessInjection; import fr.free.nrw.commons.location.LatLng; -import fr.free.nrw.commons.media.Depictions; import fr.free.nrw.commons.upload.WikidataPlace; import fr.free.nrw.commons.upload.structure.depictions.DepictedItem; import java.util.Date; -import java.util.HashMap; import java.util.List; import java.util.Map; @@ -55,13 +53,13 @@ public class Converters { } @TypeConverter - public static String mapObjectToString(HashMap objectList) { + public static String mapObjectToString(Map objectList) { return writeObjectToString(objectList); } @TypeConverter - public static HashMap stringToMap(String objectList) { - return readObjectWithTypeToken(objectList, new TypeToken>(){}); + public static Map stringToMap(String objectList) { + return readObjectWithTypeToken(objectList, new TypeToken>(){}); } @TypeConverter @@ -94,16 +92,6 @@ public class Converters { return readObjectWithTypeToken(depictedItems, new TypeToken>() {}); } - @TypeConverter - public static String depictionsToString(Depictions depictedItems) { - return writeObjectToString(depictedItems); - } - - @TypeConverter - public static Depictions stringToDepictions(String depictedItems) { - return readObjectFromString(depictedItems, Depictions.class); - } - private static String writeObjectToString(Object object) { return object == null ? null : getGson().toJson(object); } diff --git a/app/src/main/java/fr/free/nrw/commons/depictions/GridViewAdapter.java b/app/src/main/java/fr/free/nrw/commons/depictions/GridViewAdapter.java index 56cb73728..aeaa708cf 100644 --- a/app/src/main/java/fr/free/nrw/commons/depictions/GridViewAdapter.java +++ b/app/src/main/java/fr/free/nrw/commons/depictions/GridViewAdapter.java @@ -88,7 +88,7 @@ public class GridViewAdapter extends ArrayAdapter { SimpleDraweeView imageView = convertView.findViewById(R.id.depict_image_view); TextView fileName = convertView.findViewById(R.id.depict_image_title); TextView author = convertView.findViewById(R.id.depict_image_author); - fileName.setText(item.getThumbnailTitle()); + fileName.setText(item.getDisplayTitle()); setAuthorView(item, author); imageView.setImageURI(item.getThumbUrl()); return convertView; diff --git a/app/src/main/java/fr/free/nrw/commons/depictions/Media/DepictedImagesContract.java b/app/src/main/java/fr/free/nrw/commons/depictions/Media/DepictedImagesContract.java index d79757ecd..e69d5cc3b 100644 --- a/app/src/main/java/fr/free/nrw/commons/depictions/Media/DepictedImagesContract.java +++ b/app/src/main/java/fr/free/nrw/commons/depictions/Media/DepictedImagesContract.java @@ -31,10 +31,6 @@ public interface DepictedImagesContract { */ void setAdapter(List mediaList); - /** - * Seat caption to the image at the given position - */ - void handleLabelforImage(String caption, int position); /** * Display snackbar @@ -94,12 +90,6 @@ public interface DepictedImagesContract { */ void fetchMoreImages(String entityId); - /** - * fetch captions for the image using filename and replace title of on the image thumbnail(if captions are available) - * else show filename - */ - void replaceTitlesWithCaptions(String title, int position); - /** * add items to query list */ diff --git a/app/src/main/java/fr/free/nrw/commons/depictions/Media/DepictedImagesFragment.java b/app/src/main/java/fr/free/nrw/commons/depictions/Media/DepictedImagesFragment.java index 6e3c128d3..1efe63121 100644 --- a/app/src/main/java/fr/free/nrw/commons/depictions/Media/DepictedImagesFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/depictions/Media/DepictedImagesFragment.java @@ -50,7 +50,6 @@ public class DepictedImagesFragment extends DaggerFragment implements DepictedIm private String entityId = null; private boolean isLastPage; private boolean isLoading = true; - private int mediaSize = 0; @Nullable @Override @@ -146,17 +145,6 @@ public class DepictedImagesFragment extends DaggerFragment implements DepictedIm }); } - /** - * Seat caption to the image at the given position - */ - @Override - public void handleLabelforImage(String caption, int position) { - if (!caption.trim().equals(getString(R.string.detail_caption_empty))) { - gridAdapter.getItem(position).setThumbnailTitle(caption); - gridAdapter.notifyDataSetChanged(); - } - } - /** * Display snackbar */ @@ -257,11 +245,5 @@ public class DepictedImagesFragment extends DaggerFragment implements DepictedIm progressBar.setVisibility(GONE); isLoading = false; statusTextView.setVisibility(GONE); - for (Media media : collection) { - final String pageId = media.getPageId(); - if (pageId != null) { - presenter.replaceTitlesWithCaptions(PAGE_ID_PREFIX + pageId, mediaSize++); - } - } } } 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 02707b652..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)); @@ -136,20 +133,6 @@ public class DepictedImagesPresenter implements DepictedImagesContract.UserActio } } - /** - * fetch captions for the image using filename and replace title of on the image thumbnail(if captions are available) - * else show filename - */ - @Override - public void replaceTitlesWithCaptions(String wikibaseIdentifier, int position) { - compositeDisposable.add(mediaClient.getCaptionByWikibaseIdentifier(wikibaseIdentifier) - .subscribeOn(ioScheduler) - .observeOn(mainThreadScheduler) - .subscribe(caption -> { - view.handleLabelforImage(caption, position); - })); - - } /** * add items to query list 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/categories/ExploreActivity.java b/app/src/main/java/fr/free/nrw/commons/explore/categories/ExploreActivity.java index 17b3d2c4a..de4884b7f 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/categories/ExploreActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/explore/categories/ExploreActivity.java @@ -9,18 +9,12 @@ import android.view.MenuItem; import android.view.View; import android.widget.AdapterView; import android.widget.FrameLayout; - import androidx.fragment.app.Fragment; import androidx.fragment.app.FragmentManager; import androidx.viewpager.widget.ViewPager; - -import com.google.android.material.tabs.TabLayout; - -import java.util.ArrayList; -import java.util.List; - import butterknife.BindView; import butterknife.ButterKnife; +import com.google.android.material.tabs.TabLayout; import fr.free.nrw.commons.Media; import fr.free.nrw.commons.R; import fr.free.nrw.commons.category.CategoryImagesCallback; @@ -29,6 +23,8 @@ import fr.free.nrw.commons.explore.SearchActivity; import fr.free.nrw.commons.explore.ViewPagerAdapter; import fr.free.nrw.commons.media.MediaDetailPagerFragment; import fr.free.nrw.commons.theme.NavigationBaseActivity; +import java.util.ArrayList; +import java.util.List; /** * This activity displays featured images and images uploaded via mobile 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 183282d33..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,33 +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): Observable> { - return mediaInterface.fetchImagesForDepictedItem( - "haswbstatement:" + BuildConfig.DEPICTS_PROPERTY + "=" + query, - sroffset.toString() - ) - .map { mwQueryResponse: DepictionResponse -> - mwQueryResponse.query - .search - .map { - Media( - null, - getUrl(it.title), - it.title, - "", - 0, - safeParseDate(it.timestamp), - 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/explore/depictions/SearchDepictionsFragment.kt b/app/src/main/java/fr/free/nrw/commons/explore/depictions/SearchDepictionsFragment.kt index 579281da2..11932dfc3 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/depictions/SearchDepictionsFragment.kt +++ b/app/src/main/java/fr/free/nrw/commons/explore/depictions/SearchDepictionsFragment.kt @@ -3,7 +3,6 @@ package fr.free.nrw.commons.explore.depictions import fr.free.nrw.commons.R import fr.free.nrw.commons.depictions.WikidataItemDetailsActivity import fr.free.nrw.commons.explore.BaseSearchFragment -import fr.free.nrw.commons.explore.SearchFragmentContract import fr.free.nrw.commons.upload.structure.depictions.DepictedItem import javax.inject.Inject diff --git a/app/src/main/java/fr/free/nrw/commons/explore/media/MediaConverter.kt b/app/src/main/java/fr/free/nrw/commons/explore/media/MediaConverter.kt index 41483dd8e..db9096415 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/media/MediaConverter.kt +++ b/app/src/main/java/fr/free/nrw/commons/explore/media/MediaConverter.kt @@ -1,9 +1,93 @@ package fr.free.nrw.commons.explore.media import fr.free.nrw.commons.Media +import fr.free.nrw.commons.location.LatLng +import fr.free.nrw.commons.upload.structure.depictions.get +import fr.free.nrw.commons.utils.CommonsDateUtil +import fr.free.nrw.commons.utils.MediaDataExtractorUtil +import fr.free.nrw.commons.wikidata.WikidataProperties +import org.apache.commons.lang3.StringUtils import org.wikipedia.dataclient.mwapi.MwQueryPage +import org.wikipedia.gallery.ExtMetadata +import org.wikipedia.wikidata.DataValue +import org.wikipedia.wikidata.Entities +import java.text.ParseException +import java.util.* import javax.inject.Inject class MediaConverter @Inject constructor() { - fun convert(mwQueryPage: MwQueryPage): Media = Media.from(mwQueryPage) + fun convert(page: MwQueryPage, entity: Entities.Entity): Media { + val imageInfo = page.imageInfo() + requireNotNull(imageInfo) { "No image info" } + val metadata = imageInfo.metadata + requireNotNull(metadata) { "No metadata" } + return Media( + imageInfo.thumbUrl.takeIf { it.isNotBlank() } ?: imageInfo.originalUrl, + imageInfo.originalUrl, + page.title(), + metadata.imageDescription(), + safeParseDate(metadata.dateTime()), + metadata.licenseShortName(), + metadata.prefixedLicenseUrl, + getArtist(metadata), + page.pageId().toString(), + MediaDataExtractorUtil.extractCategoriesFromList(metadata.categories), + metadata.latLng, + entity.labels().mapValues { it.value.value() }, + entity.descriptions().mapValues { it.value.value() }, + entity.depictionIds() + ) + + } + + /** + * Creating Media object from MWQueryPage. + * Earlier only basic details were set for the media object but going forward, + * a full media object(with categories, descriptions, coordinates etc) can be constructed using this method + * + * @param page response from the API + * @return Media object + */ + + private fun safeParseDate(dateStr: String): Date? { + return try { + CommonsDateUtil.getMediaSimpleDateFormat().parse(dateStr) + } catch (e: ParseException) { + null + } + } + + + /** + * This method extracts the Commons Username from the artist HTML information + * @param metadata + * @return + */ + private fun getArtist(metadata: ExtMetadata): String? { + return try { + val artistHtml = metadata.artist() + artistHtml.substring(artistHtml.indexOf("title=\""), artistHtml.indexOf("\">")) + .replace("title=\"User:", "") + } catch (ex: java.lang.Exception) { + "" + } + } } + +private fun Entities.Entity.depictionIds() = + this[WikidataProperties.DEPICTS]?.mapNotNull { (it.mainSnak.dataValue as? DataValue.EntityId)?.value?.id } + ?: emptyList() + +private val ExtMetadata.prefixedLicenseUrl: String + get() = licenseUrl().let { + if (!it.startsWith("http://") && !it.startsWith("https://")) + "https://$it" + else + it + } + +private val ExtMetadata.latLng: LatLng? + get() = if (!StringUtils.isBlank(gpsLatitude) && !StringUtils.isBlank(gpsLongitude)) + LatLng(gpsLatitude.toDouble(), gpsLongitude.toDouble(), 0.0f) + else + null diff --git a/app/src/main/java/fr/free/nrw/commons/explore/media/PageableMediaDataSource.kt b/app/src/main/java/fr/free/nrw/commons/explore/media/PageableMediaDataSource.kt index 8572b42bb..c39992c62 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/media/PageableMediaDataSource.kt +++ b/app/src/main/java/fr/free/nrw/commons/explore/media/PageableMediaDataSource.kt @@ -1,34 +1,17 @@ package fr.free.nrw.commons.explore.media import fr.free.nrw.commons.Media -import fr.free.nrw.commons.depictions.Media.DepictedImagesFragment.PAGE_ID_PREFIX import fr.free.nrw.commons.explore.LiveDataConverter import fr.free.nrw.commons.explore.PageableDataSource import fr.free.nrw.commons.explore.depictions.LoadFunction import fr.free.nrw.commons.media.MediaClient -import fr.free.nrw.commons.media.MediaClient.Companion.NO_CAPTION import javax.inject.Inject class PageableMediaDataSource @Inject constructor( liveDataConverter: LiveDataConverter, - private val mediaConverter: MediaConverter, private val mediaClient: MediaClient ) : PageableDataSource(liveDataConverter) { override val loadFunction: LoadFunction = { loadSize: Int, startPosition: Int -> - mediaClient.getMediaListFromSearch(query, loadSize, startPosition) - .map { it.query()?.pages()?.map(mediaConverter::convert) ?: emptyList() } - .map { it.zip(getCaptions(it)) } - .map { it.map { (media, caption) -> media.also { it.caption = caption } } } - .blockingGet() + mediaClient.getMediaListFromSearch(query, loadSize, startPosition).blockingGet() } - - private fun getCaptions(it: List) = - mediaClient.getEntities(it.joinToString("|") { PAGE_ID_PREFIX + it.pageId }) - .map { - it.entities().values.map { entity -> - entity.labels().values.firstOrNull()?.value() ?: NO_CAPTION - } - } - .blockingGet() - } diff --git a/app/src/main/java/fr/free/nrw/commons/explore/media/SearchMediaAdapter.kt b/app/src/main/java/fr/free/nrw/commons/explore/media/SearchMediaAdapter.kt index 71a25d6c9..422e64438 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/media/SearchMediaAdapter.kt +++ b/app/src/main/java/fr/free/nrw/commons/explore/media/SearchMediaAdapter.kt @@ -35,7 +35,7 @@ class SearchImagesViewHolder(containerView: View, val onImageClicked: (Int) -> U override fun bind(item: Pair) { val media = item.first categoryImageView.setOnClickListener { onImageClicked(item.second) } - categoryImageTitle.text = media.thumbnailTitle + categoryImageTitle.text = media.mostRelevantCaption categoryImageView.setImageURI(media.thumbUrl) if (media.creator?.isNotEmpty() == true) { categoryImageAuthor.visibility = View.VISIBLE diff --git a/app/src/main/java/fr/free/nrw/commons/media/Depictions.kt b/app/src/main/java/fr/free/nrw/commons/media/Depictions.kt deleted file mode 100644 index f9667ae1a..000000000 --- a/app/src/main/java/fr/free/nrw/commons/media/Depictions.kt +++ /dev/null @@ -1,30 +0,0 @@ -package fr.free.nrw.commons.media - -import android.os.Parcelable -import androidx.annotation.WorkerThread -import fr.free.nrw.commons.wikidata.WikidataProperties.DEPICTS -import kotlinx.android.parcel.Parcelize -import org.wikipedia.wikidata.DataValue.EntityId -import org.wikipedia.wikidata.Entities -import java.util.* - -@Parcelize -data class Depictions(val depictions: List) : Parcelable { - companion object { - @JvmStatic - @WorkerThread - fun from(entities: Entities, mediaClient: MediaClient) = - Depictions( - entities.first?.statements - ?.getOrElse(DEPICTS.propertyName, { emptyList() }) - ?.map { statement -> - (statement.mainSnak.dataValue as EntityId).value.id - } - ?.map { id -> IdAndLabel(id, fetchLabel(mediaClient, id)) } - ?: emptyList() - ) - - private fun fetchLabel(mediaClient: MediaClient, id: String) = - mediaClient.getLabelForDepiction(id, Locale.getDefault().language).blockingGet() - } -} diff --git a/app/src/main/java/fr/free/nrw/commons/media/IdAndCaptions.kt b/app/src/main/java/fr/free/nrw/commons/media/IdAndCaptions.kt new file mode 100644 index 000000000..f9e6b4aee --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/media/IdAndCaptions.kt @@ -0,0 +1,4 @@ +package fr.free.nrw.commons.media + +data class IdAndCaptions(val id: String, val captions: Map) + diff --git a/app/src/main/java/fr/free/nrw/commons/media/IdAndLabel.kt b/app/src/main/java/fr/free/nrw/commons/media/IdAndLabel.kt deleted file mode 100644 index 00ca69f08..000000000 --- a/app/src/main/java/fr/free/nrw/commons/media/IdAndLabel.kt +++ /dev/null @@ -1,14 +0,0 @@ -package fr.free.nrw.commons.media - -import android.os.Parcelable -import kotlinx.android.parcel.Parcelize -import org.wikipedia.wikidata.Entities - -@Parcelize -data class IdAndLabel(val entityId: String, val entityLabel: String) : Parcelable { - constructor(entityId: String, entities: MutableMap) : this( - entityId, - entities.values.first().labels().values.first().value() - ) -} - 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 c7c177364..f0fe5f3bd 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,18 +1,17 @@ package fr.free.nrw.commons.media +import fr.free.nrw.commons.BuildConfig import fr.free.nrw.commons.Media -import fr.free.nrw.commons.media.Depictions.Companion.from +import fr.free.nrw.commons.depictions.Media.DepictedImagesFragment.PAGE_ID_PREFIX +import fr.free.nrw.commons.explore.media.MediaConverter import fr.free.nrw.commons.utils.CommonsDateUtil -import io.reactivex.Observable import io.reactivex.Single import org.wikipedia.dataclient.mwapi.MwQueryPage import org.wikipedia.dataclient.mwapi.MwQueryResponse import org.wikipedia.wikidata.Entities -import timber.log.Timber import java.util.* import javax.inject.Inject import javax.inject.Singleton -import kotlin.collections.ArrayList /** * Media Client to handle custom calls to Commons MediaWiki APIs @@ -21,12 +20,16 @@ import kotlin.collections.ArrayList class MediaClient @Inject constructor( private val mediaInterface: MediaInterface, private val pageMediaInterface: PageMediaInterface, - private val mediaDetailInterface: MediaDetailInterface + private val mediaDetailInterface: MediaDetailInterface, + private val mediaConverter: MediaConverter ) { + fun getMediaById(id: String) = + responseToMediaList(mediaInterface.getMediaById(id)).map { it.first() } + //OkHttpJsonApiClient used JsonKvStore for this. I don't know why. - private val continuationStore: MutableMap?> - private val continuationExists: MutableMap + private val continuationStore: MutableMap?> = mutableMapOf() + private val continuationExists: MutableMap = mutableMapOf() /** * Checks if a page exists on Commons @@ -36,11 +39,7 @@ class MediaClient @Inject constructor( */ fun checkPageExistsUsingTitle(title: String?): Single { return mediaInterface.checkPageExistsUsingTitle(title) - .map { mwQueryResponse: MwQueryResponse -> - mwQueryResponse - .query()!!.firstPage()!!.pageId() > 0 - } - .singleOrError() + .map { it.query()!!.firstPage()!!.pageId() > 0 } } /** @@ -50,11 +49,7 @@ class MediaClient @Inject constructor( */ fun checkFileExistsUsingSha(fileSha: String?): Single { return mediaInterface.checkFileExistsUsingSha(fileSha) - .map { mwQueryResponse: MwQueryResponse -> - mwQueryResponse - .query()!!.allImages().size > 0 - } - .singleOrError() + .map { it.query()!!.allImages().size > 0 } } /** @@ -66,18 +61,13 @@ class MediaClient @Inject constructor( */ fun getMediaListFromCategory(category: String): Single> { return responseToMediaList( - if (continuationStore.containsKey("category_$category")) mediaInterface.getMediaListFromCategory( + mediaInterface.getMediaListFromCategory( category, 10, - continuationStore["category_$category"] - ) else //if true - mediaInterface.getMediaListFromCategory( - category, - 10, - emptyMap() - ), + continuationStore["category_$category"] ?: emptyMap() + ), "category_$category" - ) //if false + ) } /** @@ -89,25 +79,16 @@ class MediaClient @Inject constructor( * @return */ fun getMediaListForUser(userName: String): Single> { - val continuation = - if (continuationStore.containsKey("user_$userName")) continuationStore["user_$userName"] else emptyMap() return responseToMediaList( - mediaInterface - .getMediaListForUser(userName, 10, continuation), "user_$userName" + mediaInterface.getMediaListForUser( + userName, + 10, + continuationStore["user_$userName"] ?: Collections.emptyMap() + ), + "user_$userName" ) } - /** - * Check if media for user has reached the end of the list. - * @param userName - * @return - */ - fun doesMediaListForUserHaveMorePages(userName: String): Boolean { - val key = "user_$userName" - return if (continuationExists.containsKey(key)) { - continuationExists[key]!! - } else true - } /** * This method takes a keyword as input and returns a list of Media objects filtered using image generator query @@ -118,40 +99,45 @@ class MediaClient @Inject constructor( * @param offset * @return */ - fun getMediaListFromSearch( - keyword: String?, - limit: Int, - offset: Int - ): Single { - return mediaInterface.getMediaListFromSearch(keyword, limit, offset) + 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: Observable, - key: String + response: Single, + key: String? = null ): Single> { - return response.flatMap { mwQueryResponse: MwQueryResponse? -> - if (null == mwQueryResponse || null == mwQueryResponse.query() || null == mwQueryResponse.query()!! - .pages() - ) { - return@flatMap Observable.empty() + return response.map { + if (key != null) { + continuationExists[key] = + it.continuation()?.let { continuation -> + continuationStore[key] = continuation + true + } ?: false } - if (mwQueryResponse.continuation() != null) { - continuationStore[key] = mwQueryResponse.continuation() - continuationExists[key] = true - } else { - continuationExists[key] = false + it.query()?.pages() ?: emptyList() + }.flatMap(::mediaFromPageAndEntity) + + } + + private fun mediaFromPageAndEntity(pages: List): Single> { + return getEntities(pages.map { "$PAGE_ID_PREFIX${it.pageId()}" }) + .map { + pages.zip(it.entities().values) + .map { (page, entity) -> mediaConverter.convert(page, entity) } } - Observable.fromIterable(mwQueryResponse.query()!!.pages()) - } - .map { page: MwQueryPage? -> Media.from(page) } - .collect( - { ArrayList() } - ) { obj: MutableList, e: Media -> - obj.add( - e - ) - }.map { it.toList() } } /** @@ -161,17 +147,8 @@ class MediaClient @Inject constructor( * @return */ fun getMedia(titles: String?): Single { - return mediaInterface.getMedia(titles) - .flatMap { mwQueryResponse: MwQueryResponse? -> - if (null == mwQueryResponse || null == mwQueryResponse.query() || null == mwQueryResponse.query()!! - .firstPage() - ) { - return@flatMap Observable.empty() - } - Observable.just(mwQueryResponse.query()!!.firstPage()) - } - .map { page: MwQueryPage? -> Media.from(page) } - .single(Media.EMPTY) + return responseToMediaList(mediaInterface.getMedia(titles)) + .map { it.first() } } /** @@ -179,122 +156,37 @@ class MediaClient @Inject constructor( * * @return Media object corresponding to the picture of the day */ - val pictureOfTheDay: Single - get() { - val date = - CommonsDateUtil.getIso8601DateFormatShort().format(Date()) - Timber.d("Current date is %s", date) - val template = "Template:Potd/$date" - return mediaInterface.getMediaWithGenerator(template) - .flatMap { mwQueryResponse: MwQueryResponse? -> - if (null == mwQueryResponse || null == mwQueryResponse.query() || null == mwQueryResponse.query()!! - .firstPage() - ) { - return@flatMap Observable.empty() - } - Observable.just(mwQueryResponse.query()!!.firstPage()) - } - .map { page: MwQueryPage? -> Media.from(page) } - .single(Media.EMPTY) - } + fun getPictureOfTheDay(): Single { + val date = CommonsDateUtil.getIso8601DateFormatShort().format(Date()) + return responseToMediaList(mediaInterface.getMediaWithGenerator("Template:Potd/$date")).map { it.first() } + + } fun getPageHtml(title: String?): Single { return mediaInterface.getPageHtml(title) - .filter { obj: MwParseResponse -> obj.success() } - .map { obj: MwParseResponse -> obj.parse() } - .map { obj: MwParseResult? -> obj!!.text() } - .first("") + .map { obj: MwParseResponse -> obj.parse()?.text() ?: "" } } + fun getEntities(entityIds: List): Single { + return if (entityIds.isEmpty()) + Single.error(Exception("empty list passed for ids")) + else + mediaDetailInterface.getEntity(entityIds.joinToString("|")) + } + + /** - * @return caption for image using wikibaseIdentifier + * Check if media for user has reached the end of the list. + * @param userName + * @return */ - fun getCaptionByWikibaseIdentifier(wikibaseIdentifier: String?): Single { - return mediaDetailInterface.getEntityForImage( - Locale.getDefault().language, - wikibaseIdentifier - ) - .map { mediaDetailResponse: Entities -> - if (isSuccess(mediaDetailResponse)) { - for (wikibaseItem in mediaDetailResponse.entities().values) { - for (label in wikibaseItem.labels().values) { - return@map label.value() - } - } - } - NO_CAPTION - } - .singleOrError() + fun doesMediaListForUserHaveMorePages(userName: String): Boolean { + val key = "user_$userName" + return if (continuationExists.containsKey(key)) continuationExists[key]!! else true } fun doesPageContainMedia(title: String?): Single { return pageMediaInterface.getMediaList(title) .map { it.items.isNotEmpty() } } - - private fun isSuccess(response: Entities?): Boolean { - return response != null && response.success == 1 && response.entities() != null - } - - /** - * Fetches Structured data from API - * - * @param filename - * @return a map containing caption and depictions (empty string in the map if no caption/depictions) - */ - fun getDepictions(filename: String?): Single { - return mediaDetailInterface.fetchEntitiesByFileName( - Locale.getDefault().language, filename - ) - .map { entities: Entities? -> - from( - entities!!, - this - ) - } - .singleOrError() - } - - /** - * Gets labels for Depictions using Entity Id from MediaWikiAPI - * - * @param entityId EntityId (Ex: Q81566) of the depict entity - * @return label - */ - fun getLabelForDepiction( - entityId: String?, - language: String - ): Single { - return mediaDetailInterface.getEntity(entityId) - .map { entities: Entities -> - if (isSuccess(entities)) { - for (entity in entities.entities().values) { - val languageToLabelMap = - entity.labels() - if (languageToLabelMap.containsKey(language)) { - return@map languageToLabelMap[language]!!.value() - } - for (label in languageToLabelMap.values) { - return@map label.value() - } - } - } - throw RuntimeException("failed getEntities") - } - } - - fun getEntities(entityId: String?): Single { - return mediaDetailInterface.getEntity(entityId) - } - - companion object { - const val NO_CAPTION = "No caption" - private const val NO_DEPICTION = "No depiction" - } - - init { - continuationStore = - HashMap() - continuationExists = HashMap() - } } diff --git a/app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java b/app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java index d78c0026b..0bd70b9ea 100644 --- a/app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java @@ -54,11 +54,12 @@ import fr.free.nrw.commons.ui.widget.HtmlTextView; import fr.free.nrw.commons.utils.ViewUtilWrapper; import io.reactivex.Single; import io.reactivex.android.schedulers.AndroidSchedulers; -import io.reactivex.disposables.Disposable; import io.reactivex.schedulers.Schedulers; 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 org.apache.commons.lang3.StringUtils; import org.wikipedia.util.DateUtil; @@ -70,7 +71,6 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { private boolean isCategoryImage; private MediaDetailPagerFragment.MediaDetailProvider detailProvider; private int index; - private Locale locale; private boolean isDeleted = false; @@ -141,7 +141,6 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { @BindView(R.id.mediaDetailScrollView) ScrollView scrollView; - private ArrayList categoryNames; /** * Depicts is a feature part of Structured data. Multiple Depictions can be added for an image just like categories. * However unlike categories depictions is multi-lingual @@ -150,10 +149,6 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { private ImageInfo imageInfoCache; private int oldWidthOfImageView; private int newWidthOfImageView; - private Depictions depictions; - private boolean categoriesLoaded = false; - private boolean categoriesPresent = false; - private boolean depictionLoaded = false; private boolean heightVerifyingBoolean = true; // helps in maintaining aspect ratio private ViewTreeObserver.OnGlobalLayoutListener layoutListener; // for layout stuff, only used once! @@ -203,9 +198,6 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { reasonList.add(getString(R.string.deletion_reason_no_longer_want_public)); reasonList.add(getString(R.string.deletion_reason_bad_for_my_privacy)); - categoryNames = new ArrayList<>(); - categoryNames.add(getString(R.string.detail_panel_cats_loading)); - final View view = inflater.inflate(R.layout.fragment_media_detail, container, false); ButterKnife.bind(this,view); @@ -217,7 +209,6 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { authorLayout.setVisibility(GONE); } - locale = getResources().getConfiguration().locale; return view; } @@ -291,19 +282,55 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { } private void displayMediaDetails() { - //Always load image from Internet to allow viewing the desc, license, and cats - setupImageView(); - title.setText(media.getDisplayTitle()); - desc.setHtmlText(media.getDescription()); - license.setText(media.getLicense()); - - Disposable disposable = mediaDataExtractor.fetchMediaDetails(media.getFilename(), media.getPageId()) + setTextFields(media); + compositeDisposable.addAll( + mediaDataExtractor.fetchDepictionIdsAndLabels(media) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) - .subscribe(this::setTextFields); - compositeDisposable.add(disposable); + .subscribe(this::onDepictionsLoaded, Timber::e), + mediaDataExtractor.checkDeletionRequestExists(media) + .subscribeOn(Schedulers.io()) + .observeOn(AndroidSchedulers.mainThread()) + .subscribe(this::onDeletionPageExists, Timber::e), + mediaDataExtractor.fetchDiscussion(media) + .subscribeOn(Schedulers.io()) + .observeOn(AndroidSchedulers.mainThread()) + .subscribe(this::onDiscussionLoaded, Timber::e), + mediaDataExtractor.refresh(media) + .subscribeOn(Schedulers.io()) + .observeOn(AndroidSchedulers.mainThread()) + .subscribe(this::onMediaRefreshed, Timber::e) + ); } + private void onMediaRefreshed(Media media) { + setTextFields(media); + compositeDisposable.addAll( + mediaDataExtractor.fetchDepictionIdsAndLabels(media) + .subscribeOn(Schedulers.io()) + .observeOn(AndroidSchedulers.mainThread()) + .subscribe(this::onDepictionsLoaded, Timber::e) + ); + } + + private void onDiscussionLoaded(String discussion) { + mediaDiscussion.setText(prettyDiscussion(discussion.trim())); + } + + private void onDeletionPageExists(Boolean deletionPageExists) { + if (deletionPageExists){ + delete.setVisibility(GONE); + nominatedForDeletion.setVisibility(VISIBLE); + } else if (!isCategoryImage) { + delete.setVisibility(VISIBLE); + nominatedForDeletion.setVisibility(GONE); + } + } + + private void onDepictionsLoaded(List idAndCaptions){ + depictsLayout.setVisibility(idAndCaptions.isEmpty() ? GONE : VISIBLE); + buildDepictionList(idAndCaptions); + } /** * The imageSpacer is Basically a transparent overlay for the SimpleDraweeView * which holds the image to be displayed( moreover this image is out of @@ -370,58 +397,45 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { } private void setTextFields(Media media) { - this.media = media; setupImageView(); + title.setText(media.getDisplayTitle()); desc.setHtmlText(prettyDescription(media)); license.setText(prettyLicense(media)); coordinates.setText(prettyCoordinates(media)); uploadedDate.setText(prettyUploadedDate(media)); - mediaDiscussion.setText(prettyDiscussion(media)); if (prettyCaption(media).equals(getContext().getString(R.string.detail_caption_empty))) { captionLayout.setVisibility(GONE); - } else mediaCaption.setText(prettyCaption(media)); + } else { + mediaCaption.setText(prettyCaption(media)); + } - - categoryNames.clear(); - categoryNames.addAll(media.getCategories()); - - depictions=media.getDepiction(); - - depictionLoaded = true; - - categoriesLoaded = true; - categoriesPresent = (categoryNames.size() > 0); - if (!categoriesPresent) { + final List categories = media.getCategories(); + if (categories.isEmpty()) { // Stick in a filler element. - categoryNames.add(getString(R.string.detail_panel_cats_none)); + categories.add(getString(R.string.detail_panel_cats_none)); } - rebuildCatList(); + rebuildCatList(categories); + - if(depictions != null) { - rebuildDepictionList(); - } - else depictsLayout.setVisibility(GONE); if (media.getCreator() == null || media.getCreator().equals("")) { authorLayout.setVisibility(GONE); } else { author.setText(media.getCreator()); } - - checkDeletion(media); } /** * Populates media details fragment with depiction list + * @param idAndCaptions */ - private void rebuildDepictionList() { + private void buildDepictionList(List idAndCaptions) { depictionContainer.removeAllViews(); - for (IdAndLabel depiction : depictions.getDepictions()) { - depictionContainer.addView( - buildDepictLabel( - depiction.getEntityLabel(), - depiction.getEntityId(), + for (IdAndCaptions idAndCaption : idAndCaptions) { + depictionContainer.addView(buildDepictLabel( + idAndCaption.getCaptions().values().iterator().next(), + idAndCaption.getId(), depictionContainer )); } @@ -446,7 +460,7 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { @OnClick(R.id.copyWikicode) public void onCopyWikicodeClicked(){ - String data = "[[" + media.getFilename() + "|thumb|" + media.getDescription() + "]]"; + String data = "[[" + media.getFilename() + "|thumb|" + media.getFallbackDescription() + "]]"; Utils.copy("wikiCode",data,getContext()); Timber.d("Generated wikidata copy code: %s", data); @@ -573,42 +587,37 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { } } - private void rebuildCatList() { + private void rebuildCatList(List categories) { categoryContainer.removeAllViews(); - // @fixme add the category items - - //As per issue #1826(see https://github.com/commons-app/apps-android-commons/issues/1826), some categories come suffixed with strings prefixed with |. As per the discussion - //that was meant for alphabetical sorting of the categories and can be safely removed. - for (int i = 0; i < categoryNames.size(); i++) { - String categoryName = categoryNames.get(i); - //Removed everything after '|' - int indexOfPipe = categoryName.indexOf('|'); - if (indexOfPipe != -1) { - categoryName = categoryName.substring(0, indexOfPipe); - //Set the updated category to the list as well - categoryNames.set(i, categoryName); - } - View catLabel = buildCatLabel(categoryName, categoryContainer); - categoryContainer.addView(catLabel); + for (String category : categories) { + categoryContainer.addView(buildCatLabel(sanitise(category), categoryContainer)); } } + //As per issue #1826(see https://github.com/commons-app/apps-android-commons/issues/1826), some categories come suffixed with strings prefixed with |. As per the discussion + //that was meant for alphabetical sorting of the categories and can be safely removed. + private String sanitise(String category) { + int indexOfPipe = category.indexOf('|'); + if (indexOfPipe != -1) { + //Removed everything after '|' + return category.substring(0, indexOfPipe); + } + return category; + } + /** * Add view to depictions obtained also tapping on depictions should open the url */ private View buildDepictLabel(String depictionName, String entityId, LinearLayout depictionContainer) { - final View item = LayoutInflater.from(getContext()).inflate(R.layout.detail_category_item, depictionContainer, false); + final View item = LayoutInflater.from(getContext()).inflate(R.layout.detail_category_item, depictionContainer,false); final TextView textView = item.findViewById(R.id.mediaDetailCategoryItemText); - textView.setText(depictionName); - if (depictionLoaded) { - item.setOnClickListener(view -> { - Intent intent = new Intent(getContext(), WikidataItemDetailsActivity.class); - intent.putExtra("wikidataItemName", depictionName); - intent.putExtra("entityId", entityId); - getContext().startActivity(intent); - }); - } + item.setOnClickListener(view -> { + Intent intent = new Intent(getContext(), WikidataItemDetailsActivity.class); + intent.putExtra("wikidataItemName", depictionName); + intent.putExtra("entityId", entityId); + getContext().startActivity(intent); + }); return item; } @@ -617,7 +626,7 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { final TextView textView = item.findViewById(R.id.mediaDetailCategoryItemText); textView.setText(catName); - if (categoriesLoaded && categoriesPresent) { + if(!getString(R.string.detail_panel_cats_none).equals(catName)) { textView.setOnClickListener(view -> { // Open Category Details page String selectedCategoryTitle = CATEGORY_PREFIX + catName; @@ -636,30 +645,36 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { * @return caption as string */ private String prettyCaption(Media media) { - String caption = media.getCaption().trim(); - if (caption.equals("")) { - return getString(R.string.detail_caption_empty); - } else { - return caption; + for (String caption : media.getCaptions().values()) { + if (caption.equals("")) { + return getString(R.string.detail_caption_empty); + } else { + return caption; + } } + return getString(R.string.detail_caption_empty); } private String prettyDescription(Media media) { - // @todo use UI language when multilingual descs are available - String desc = media.getDescription(); - if (desc.equals("")) { - return getString(R.string.detail_description_empty); - } else { - return desc; - } + final String description = chooseDescription(media); + return description.isEmpty() ? getString(R.string.detail_description_empty) + : description; } - private String prettyDiscussion(Media media) { - String disc = media.getDiscussion().trim(); - if (disc.equals("")) { - return getString(R.string.detail_discussion_empty); - } else { - return disc; + + private String chooseDescription(Media media) { + final Map descriptions = media.getDescriptions(); + final String multilingualDesc = descriptions.get(Locale.getDefault().getLanguage()); + if (multilingualDesc != null) { + return multilingualDesc; } + for (String description : descriptions.values()) { + return description; + } + return media.getFallbackDescription(); + } + + private String prettyDiscussion(String discussion) { + return discussion.isEmpty() ? getString(R.string.detail_discussion_empty) : discussion; } private String prettyLicense(Media media) { @@ -691,14 +706,4 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { return media.getCoordinates().getPrettyCoordinateString(); } - private void checkDeletion(Media media){ - if (media.isRequestedDeletion()){ - delete.setVisibility(GONE); - nominatedForDeletion.setVisibility(VISIBLE); - } else if (!isCategoryImage) { - delete.setVisibility(VISIBLE); - nominatedForDeletion.setVisibility(GONE); - } - } - } 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 aaec15136..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,7 +1,5 @@ package fr.free.nrw.commons.media; -import fr.free.nrw.commons.depictions.models.DepictionResponse; -import io.reactivex.Observable; import io.reactivex.Single; import java.util.Map; import org.wikipedia.dataclient.mwapi.MwQueryResponse; @@ -24,7 +22,7 @@ public interface MediaInterface { * @return */ @GET("w/api.php?action=query&format=json&formatversion=2") - Observable checkPageExistsUsingTitle(@Query("titles") String title); + Single checkPageExistsUsingTitle(@Query("titles") String title); /** * Check if file exists @@ -33,7 +31,7 @@ public interface MediaInterface { * @return */ @GET("w/api.php?action=query&format=json&formatversion=2&list=allimages") - Observable checkFileExistsUsingSha(@Query("aisha1") String aisha1); + Single checkFileExistsUsingSha(@Query("aisha1") String aisha1); /** * This method retrieves a list of Media objects filtered using image generator query @@ -46,7 +44,8 @@ public interface MediaInterface { @GET("w/api.php?action=query&format=json&formatversion=2" + //Basic parameters "&generator=categorymembers&gcmtype=file&gcmsort=timestamp&gcmdir=desc" + //Category parameters MEDIA_PARAMS) - Observable getMediaListFromCategory(@Query("gcmtitle") String category, @Query("gcmlimit") int itemLimit, @QueryMap Map continuation); + Single getMediaListFromCategory(@Query("gcmtitle") String category, @Query("gcmlimit") int itemLimit, @QueryMap Map continuation); + /** * This method retrieves a list of Media objects for a given user name @@ -58,7 +57,7 @@ public interface MediaInterface { */ @GET("w/api.php?action=query&format=json&formatversion=2" + //Basic parameters "&generator=allimages&gaisort=timestamp&gaidir=older" + MEDIA_PARAMS) - Observable getMediaListForUser(@Query("gaiuser") String username, + Single getMediaListForUser(@Query("gaiuser") String username, @Query("gailimit") int itemLimit, @QueryMap(encoded = true) Map continuation); /** @@ -82,7 +81,17 @@ public interface MediaInterface { */ @GET("w/api.php?action=query&format=json&formatversion=2" + MEDIA_PARAMS) - Observable getMedia(@Query("titles") String title); + Single getMedia(@Query("titles") String title); + + /** + * Fetches Media object from the imageInfo API + * + * @param pageIds the ids to be searched for + * @return + */ + @GET("w/api.php?action=query&format=json&formatversion=2" + + MEDIA_PARAMS) + Single getMediaById(@Query("pageids") String pageIds); /** * Fetches Media object from the imageInfo API @@ -93,21 +102,29 @@ public interface MediaInterface { */ @GET("w/api.php?action=query&format=json&formatversion=2&generator=images" + MEDIA_PARAMS) - Observable getMediaWithGenerator(@Query("titles") String title); + Single getMediaWithGenerator(@Query("titles") String title); @GET("w/api.php?format=json&action=parse&prop=text") - Observable getPageHtml(@Query("page") String title); + Single getPageHtml(@Query("page") String title); /** - * Fetches list of images from a depiction entity - * - * @param query depictionEntityId - * @param sroffset number od depictions already fetched, this is useful in implementing - * pagination - */ + * Fetches caption using file name + * + * @param filename name of the file to be used for fetching captions + * */ + @GET("w/api.php?action=wbgetentities&props=labels&format=json&languagefallback=1") + Single fetchCaptionByFilename(@Query("language") String language, @Query("titles") String filename); - @GET("w/api.php?action=query&list=search&format=json&srnamespace=6") - Observable fetchImagesForDepictedItem(@Query("srsearch") String query, - @Query("sroffset") String sroffset); + /** + * Fetches list of images from a depiction entity + * + * @param query depictionEntityId + * @param sroffset number od depictions already fetched, this is useful in implementing pagination + */ + + @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/main/java/fr/free/nrw/commons/review/ReviewActivity.java b/app/src/main/java/fr/free/nrw/commons/review/ReviewActivity.java index b614ad524..54057b8b9 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 @@ -14,18 +14,13 @@ 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 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.delete.DeleteHelper; @@ -35,6 +30,7 @@ import fr.free.nrw.commons.utils.ViewUtil; import io.reactivex.android.schedulers.AndroidSchedulers; import io.reactivex.disposables.CompositeDisposable; import io.reactivex.schedulers.Schedulers; +import javax.inject.Inject; public class ReviewActivity extends NavigationBaseActivity { @@ -144,10 +140,8 @@ public class ReviewActivity extends NavigationBaseActivity { .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .subscribe(media -> { - if (media != null) { - reviewPagerAdapter.disableButtons(); - updateImage(media); - } + reviewPagerAdapter.disableButtons(); + 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 8f078f82f..040231792 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,22 +1,19 @@ 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.Collections; -import java.util.Date; -import java.util.Random; - -import javax.inject.Inject; -import javax.inject.Singleton; - import fr.free.nrw.commons.Media; import fr.free.nrw.commons.media.MediaClient; import io.reactivex.Observable; import io.reactivex.Single; +import java.util.Collections; +import java.util.Date; +import java.util.Random; +import javax.inject.Inject; +import javax.inject.Singleton; +import org.apache.commons.lang3.StringUtils; +import org.wikipedia.dataclient.mwapi.MwQueryPage; +import org.wikipedia.dataclient.mwapi.RecentChange; +import org.wikipedia.util.DateUtil; @Singleton public class ReviewHelper { @@ -69,7 +66,6 @@ public class ReviewHelper { public Single getRandomMedia() { return getRecentChanges() .flatMapSingle(change -> getRandomMediaFromRecentChange(change)) - .onExceptionResumeNext(Observable.just(new Media(""))) .filter(media -> !StringUtils.isBlank(media.getFilename())) .firstOrError(); } @@ -86,7 +82,7 @@ public class ReviewHelper { .flatMap(change -> mediaClient.checkPageExistsUsingTitle("Commons:Deletion_requests/" + change.getTitle())) .flatMap(isDeleted -> { if (isDeleted) { - return Single.just(new Media("")); + return Single.error(new Exception(recentChange.getTitle() + " is deleted")); } return mediaClient.getMedia(recentChange.getTitle()); }); diff --git a/app/src/main/java/fr/free/nrw/commons/upload/PageContentsCreator.java b/app/src/main/java/fr/free/nrw/commons/upload/PageContentsCreator.java index 6079ddb11..ffb913367 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/PageContentsCreator.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/PageContentsCreator.java @@ -33,7 +33,7 @@ class PageContentsCreator { buffer .append("== {{int:filedesc}} ==\n") .append("{{Information\n") - .append("|description=").append(contribution.getDescription()).append("\n") + .append("|description=").append(contribution.getFallbackDescription()).append("\n") .append("|source=").append("{{own}}\n") .append("|author=[[User:").append(contribution.getCreator()).append("|") .append(contribution.getCreator()).append("]]\n"); diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadController.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadController.java index cf298f83b..6fee8a7c8 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadController.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadController.java @@ -13,7 +13,6 @@ import android.net.Uri; import android.os.IBinder; import android.provider.MediaStore; import android.text.TextUtils; -import fr.free.nrw.commons.Media; import fr.free.nrw.commons.R; import fr.free.nrw.commons.auth.SessionManager; import fr.free.nrw.commons.contributions.Contribution; @@ -110,8 +109,8 @@ public class UploadController { contribution.setCreator(sessionManager.getAuthorName()); } - if (contribution.getDescription() == null) { - contribution.setDescription(""); + if (contribution.getFallbackDescription() == null) { + contribution.setFallbackDescription(""); } final String license = store.getString(Prefs.DEFAULT_LICENSE, Prefs.Licenses.CC_BY_SA_3); @@ -164,7 +163,7 @@ public class UploadController { return mimeType; } - private long resolveDataLength(final ContentResolver contentResolver, final Media contribution) { + private long resolveDataLength(final ContentResolver contentResolver, final Contribution contribution) { try { if (contribution.getDataLength() <= 0) { Timber.d("UploadController/doInBackground, contribution.getLocalUri():%s", contribution.getLocalUri()); @@ -182,7 +181,7 @@ public class UploadController { return contribution.getDataLength(); } - private Date resolveDateTakenOrNow(final ContentResolver contentResolver, final Media contribution) { + private Date resolveDateTakenOrNow(final ContentResolver contentResolver, final Contribution contribution) { Timber.d("local uri %s", contribution.getLocalUri()); try(final Cursor cursor = dateTakenCursor(contentResolver, contribution)) { if (cursor != null && cursor.getCount() != 0 && cursor.getColumnCount() != 0) { @@ -196,7 +195,7 @@ public class UploadController { } } - private Cursor dateTakenCursor(final ContentResolver contentResolver, final Media contribution) { + private Cursor dateTakenCursor(final ContentResolver contentResolver, final Contribution contribution) { return contentResolver.query(contribution.getLocalUri(), new String[]{MediaStore.Images.ImageColumns.DATE_TAKEN}, null, null, null); } diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadMediaDetail.kt b/app/src/main/java/fr/free/nrw/commons/upload/UploadMediaDetail.kt index 30369f578..03f82af21 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadMediaDetail.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadMediaDetail.kt @@ -57,7 +57,7 @@ data class UploadMediaDetail constructor( * @return a string with the pattern of {{en|1=descriptionText}} */ @JvmStatic - fun formatList(descriptions: List) = + fun formatDescriptions(descriptions: List) = descriptions.filter { it.descriptionText.isNotEmpty() } .joinToString { "{{${it.languageCode}|1=${it.descriptionText}}}" } } diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java index e94a3df46..6bf18bde9 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java @@ -141,9 +141,6 @@ public class UploadModel { { final Contribution contribution = new Contribution( item, sessionManager, newListOf(selectedDepictions), newListOf(selectedCategories)); - Timber.d("Created timestamp while building contribution is %s, %s", - item.getCreatedTimestamp(), - new Date(item.getCreatedTimestamp())); if (item.getCreatedTimestamp() != -1L) { contribution.setDateCreated(new Date(item.getCreatedTimestamp())); contribution.setDateCreatedSource(item.getCreatedTimestampSource()); diff --git a/app/src/main/java/fr/free/nrw/commons/upload/WikidataPlace.kt b/app/src/main/java/fr/free/nrw/commons/upload/WikidataPlace.kt index 88f9d131c..8027f1d81 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/WikidataPlace.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/WikidataPlace.kt @@ -16,7 +16,7 @@ internal data class WikidataPlace( place.wikiDataEntityId!!, place.name, place.pic.takeIf { it.isNotBlank() }, - if (place.siteLinks.wikipediaLink == null) "" else place.siteLinks.wikipediaLink.toString() + place.siteLinks.wikipediaLink?.toString() ?: "" ) companion object { @@ -27,10 +27,6 @@ internal data class WikidataPlace( } fun getWikipediaPageTitle(): String? { - if (wikipediaArticle == null) { - return null - } - val split: Array = wikipediaArticle.split("/".toRegex()).toTypedArray() - return split[split.size - 1] + return wikipediaArticle?.substringAfterLast("/") } } diff --git a/app/src/test/kotlin/ModelFunctions.kt b/app/src/test/kotlin/ModelFunctions.kt index a0eaedd4e..684cab3ac 100644 --- a/app/src/test/kotlin/ModelFunctions.kt +++ b/app/src/test/kotlin/ModelFunctions.kt @@ -1,5 +1,6 @@ import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.whenever +import fr.free.nrw.commons.Media import fr.free.nrw.commons.category.CategoryItem import fr.free.nrw.commons.location.LatLng import fr.free.nrw.commons.nearby.Label @@ -7,6 +8,7 @@ import fr.free.nrw.commons.nearby.Place import fr.free.nrw.commons.nearby.Sitelinks import fr.free.nrw.commons.upload.structure.depictions.DepictedItem import org.wikipedia.wikidata.* +import java.util.* fun depictedItem( name: String = "label", @@ -29,6 +31,38 @@ fun depictedItem( fun categoryItem(name: String = "name", selected: Boolean = false) = CategoryItem(name, selected) +fun media( + thumbUrl: String? = "thumbUrl", + imageUrl: String? = "imageUrl", + filename: String? = "filename", + fallbackDescription: String? = "fallbackDescription", + dateUploaded: Date? = Date(), + license: String? = "license", + licenseUrl: String? = "licenseUrl", + creator: String? = "creator", + pageId: String = "pageId", + categories: List? = listOf("categories"), + coordinates: LatLng? = LatLng(0.0, 0.0, 0.0f), + captions: Map = mapOf("en" to "caption"), + descriptions: Map = mapOf("en" to "description"), + depictionIds: List = listOf("depictionId") +) = Media( + thumbUrl, + imageUrl, + filename, + fallbackDescription, + dateUploaded, + license, + licenseUrl, + creator, + pageId, + categories, + coordinates, + captions, + descriptions, + depictionIds +) + fun place( name: String = "name", label: Label? = null, diff --git a/app/src/test/kotlin/fr/free/nrw/commons/MediaTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/MediaTest.kt index df1adf81a..f55857edb 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/MediaTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/MediaTest.kt @@ -1,5 +1,6 @@ package fr.free.nrw.commons +import media import org.junit.Assert.assertEquals import org.junit.Test import org.junit.runner.RunWith @@ -11,13 +12,15 @@ import org.robolectric.annotation.Config class MediaTest { @Test fun displayTitleShouldStripExtension() { - val m = Media("File:Example.jpg") + val m = media(filename = "File:Example.jpg") assertEquals("Example", m.displayTitle) } @Test fun displayTitleShouldUseSpaceForUnderscore() { - val m = Media("File:Example 1_2.jpg") + val m = media(filename = "File:Example 1_2.jpg") assertEquals("Example 1 2", m.displayTitle) } } + + diff --git a/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/pictures/BookmarkPicturesControllerTest.java b/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/pictures/BookmarkPicturesControllerTest.java deleted file mode 100644 index 133f4807d..000000000 --- a/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/pictures/BookmarkPicturesControllerTest.java +++ /dev/null @@ -1,108 +0,0 @@ -package fr.free.nrw.commons.bookmarks.pictures; - -import android.net.Uri; - -import org.junit.Before; -import org.junit.Test; -import org.mockito.InjectMocks; -import org.mockito.Mock; -import org.mockito.MockitoAnnotations; - -import java.util.ArrayList; -import java.util.List; - -import fr.free.nrw.commons.Media; -import fr.free.nrw.commons.bookmarks.Bookmark; -import fr.free.nrw.commons.media.MediaClient; -import io.reactivex.Single; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.Mockito.when; - -/** - * Tests for bookmark pictures controller - */ -public class BookmarkPicturesControllerTest { - - @Mock - MediaClient mediaClient; - @Mock - BookmarkPicturesDao bookmarkDao; - - @InjectMocks - BookmarkPicturesController bookmarkPicturesController; - - - /** - * Init mocks - */ - @Before - public void setup() { - MockitoAnnotations.initMocks(this); - Media mockMedia = getMockMedia(); - when(bookmarkDao.getAllBookmarks()) - .thenReturn(getMockBookmarkList()); - when(mediaClient.getMedia(anyString())) - .thenReturn(Single.just(mockMedia)); - } - - /** - * Get mock bookmark list - * @return - */ - private List getMockBookmarkList() { - ArrayList list = new ArrayList<>(); - list.add(new Bookmark("File:Test1.jpg", "Maskaravivek", Uri.EMPTY)); - list.add(new Bookmark("File:Test2.jpg", "Maskaravivek", Uri.EMPTY)); - return list; - } - - /** - * Test case where all bookmark pictures are fetched and media is found against it - */ - @Test - public void loadBookmarkedPictures() { - List bookmarkedPictures = bookmarkPicturesController.loadBookmarkedPictures().blockingGet(); - assertEquals(2, bookmarkedPictures.size()); - } - - /** - * Test case where all bookmark pictures are fetched and only one media is found - */ - @Test - public void loadBookmarkedPicturesForNullMedia() { - when(mediaClient.getMedia("File:Test1.jpg")) - .thenReturn(Single.error(new NullPointerException("Error occurred"))); - when(mediaClient.getMedia("File:Test2.jpg")) - .thenReturn(Single.just(getMockMedia())); - List bookmarkedPictures = bookmarkPicturesController.loadBookmarkedPictures().blockingGet(); - assertEquals(1, bookmarkedPictures.size()); - } - - private Media getMockMedia() { - return new Media("File:Test.jpg"); - } - - /** - * Test case where current bookmarks don't match the bookmarks in DB - */ - @Test - public void needRefreshBookmarkedPictures() { - boolean needRefreshBookmarkedPictures = bookmarkPicturesController.needRefreshBookmarkedPictures(); - assertTrue(needRefreshBookmarkedPictures); - } - - /** - * Test case where the DB is up to date with the bookmarks loaded in the list - */ - @Test - public void doNotNeedRefreshBookmarkedPictures() { - List bookmarkedPictures = bookmarkPicturesController.loadBookmarkedPictures().blockingGet(); - assertEquals(2, bookmarkedPictures.size()); - boolean needRefreshBookmarkedPictures = bookmarkPicturesController.needRefreshBookmarkedPictures(); - assertFalse(needRefreshBookmarkedPictures); - } -} \ No newline at end of file diff --git a/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/pictures/BookmarkPicturesControllerTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/pictures/BookmarkPicturesControllerTest.kt new file mode 100644 index 000000000..ceba8f6c8 --- /dev/null +++ b/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/pictures/BookmarkPicturesControllerTest.kt @@ -0,0 +1,110 @@ +package fr.free.nrw.commons.bookmarks.pictures + +import android.net.Uri +import com.nhaarman.mockitokotlin2.whenever +import fr.free.nrw.commons.Media +import fr.free.nrw.commons.bookmarks.Bookmark +import fr.free.nrw.commons.media.MediaClient +import io.reactivex.Single +import media +import org.junit.Assert +import org.junit.Before +import org.junit.Test +import org.mockito.ArgumentMatchers +import org.mockito.InjectMocks +import org.mockito.Mock +import org.mockito.MockitoAnnotations +import java.util.* + +/** + * Tests for bookmark pictures controller + */ +class BookmarkPicturesControllerTest { + @Mock + var mediaClient: MediaClient? = null + + @Mock + var bookmarkDao: BookmarkPicturesDao? = null + + @InjectMocks + var bookmarkPicturesController: BookmarkPicturesController? = null + + /** + * Init mocks + */ + @Before + fun setup() { + MockitoAnnotations.initMocks(this) + val mockMedia = mockMedia + whenever(bookmarkDao!!.allBookmarks) + .thenReturn(mockBookmarkList) + whenever( + mediaClient!!.getMedia( + ArgumentMatchers.anyString() + ) + ) + .thenReturn(Single.just(mockMedia)) + } + + /** + * Get mock bookmark list + * @return + */ + private val mockBookmarkList: List + private get() { + val list = ArrayList() + list.add(Bookmark("File:Test1.jpg", "Maskaravivek", Uri.EMPTY)) + list.add(Bookmark("File:Test2.jpg", "Maskaravivek", Uri.EMPTY)) + return list + } + + /** + * Test case where all bookmark pictures are fetched and media is found against it + */ + @Test + fun loadBookmarkedPictures() { + val bookmarkedPictures = + bookmarkPicturesController!!.loadBookmarkedPictures().blockingGet() + Assert.assertEquals(2, bookmarkedPictures.size.toLong()) + } + + /** + * Test case where all bookmark pictures are fetched and only one media is found + */ + @Test + fun loadBookmarkedPicturesForNullMedia() { + whenever(mediaClient!!.getMedia("File:Test1.jpg")) + .thenReturn(Single.error(NullPointerException("Error occurred"))) + whenever(mediaClient!!.getMedia("File:Test2.jpg")) + .thenReturn(Single.just(mockMedia)) + val bookmarkedPictures = + bookmarkPicturesController!!.loadBookmarkedPictures().blockingGet() + Assert.assertEquals(1, bookmarkedPictures.size.toLong()) + } + + private val mockMedia: Media + private get() = media(filename="File:Test.jpg") + + /** + * Test case where current bookmarks don't match the bookmarks in DB + */ + @Test + fun needRefreshBookmarkedPictures() { + val needRefreshBookmarkedPictures = + bookmarkPicturesController!!.needRefreshBookmarkedPictures() + Assert.assertTrue(needRefreshBookmarkedPictures) + } + + /** + * Test case where the DB is up to date with the bookmarks loaded in the list + */ + @Test + fun doNotNeedRefreshBookmarkedPictures() { + val bookmarkedPictures = + bookmarkPicturesController!!.loadBookmarkedPictures().blockingGet() + Assert.assertEquals(2, bookmarkedPictures.size.toLong()) + val needRefreshBookmarkedPictures = + bookmarkPicturesController!!.needRefreshBookmarkedPictures() + Assert.assertFalse(needRefreshBookmarkedPictures) + } +} diff --git a/app/src/test/kotlin/fr/free/nrw/commons/delete/ReasonBuilderTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/delete/ReasonBuilderTest.kt index 6b21d15b6..e4bacb4d8 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/delete/ReasonBuilderTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/delete/ReasonBuilderTest.kt @@ -8,12 +8,12 @@ import fr.free.nrw.commons.auth.SessionManager import fr.free.nrw.commons.mwapi.OkHttpJsonApiClient import fr.free.nrw.commons.utils.ViewUtilWrapper import io.reactivex.Single +import media import org.junit.Before import org.junit.Test import org.mockito.ArgumentMatchers.anyInt import org.mockito.InjectMocks import org.mockito.Mock -import org.mockito.Mockito import org.mockito.Mockito.* import org.mockito.MockitoAnnotations import java.util.* @@ -55,11 +55,10 @@ class ReasonBuilderTest { `when`(okHttpJsonApiClient!!.getAchievements(anyString())) .thenReturn(Single.just(mock(FeedbackResponse::class.java))) - val media = Media("test_file") - media.dateUploaded=Date() + val media = media(filename="test_file", dateUploaded = Date()) reasonBuilder!!.getReason(media, "test") verify(sessionManager, times(0))!!.forceLogin(any(Context::class.java)) verify(okHttpJsonApiClient, times(1))!!.getAchievements(anyString()) } -} \ No newline at end of file +} 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 7cf05bbe9..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,10 +3,8 @@ 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.Observable import io.reactivex.Single import io.reactivex.schedulers.TestScheduler import org.junit.Before @@ -27,9 +25,6 @@ class DepictedImagesPresenterTest { @Mock lateinit var jsonKvStore: JsonKvStore - @Mock - lateinit var depictsClient: DepictsClient - @Mock lateinit var mediaClient: MediaClient @@ -40,7 +35,7 @@ class DepictedImagesPresenterTest { @Mock lateinit var mediaItem: Media - var testObservable: Observable>? = null + var testSingle: Single>? = null @Before @@ -49,28 +44,20 @@ class DepictedImagesPresenterTest { MockitoAnnotations.initMocks(this) testScheduler = TestScheduler() mediaList.add(mediaItem) - testObservable = Observable.just(mediaList) - depictedImagesPresenter = DepictedImagesPresenter(jsonKvStore, depictsClient, mediaClient, testScheduler, testScheduler) + testSingle = Single.just(mediaList) + 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(testObservable) + ).thenReturn(testSingle) depictedImagesPresenter.initList("rabbit") depictedImagesPresenter.handleSuccess(mediaList) verify(view)?.handleSuccess(mediaList) } - - @Test - fun replaceTitlesWithCaptions() { - var stringObservable: Single? = Single.just(String()) - Mockito.`when`(mediaClient.getCaptionByWikibaseIdentifier(ArgumentMatchers.anyString()))?.thenReturn(stringObservable) - depictedImagesPresenter.replaceTitlesWithCaptions("File:rabbit.jpg", 0) - testScheduler.triggerActions() - verify(view)?.handleLabelforImage("", 0) - } } diff --git a/app/src/test/kotlin/fr/free/nrw/commons/explore/BaseSearchPresenterTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/explore/BaseSearchPresenterTest.kt index 4be25522b..aa9996493 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/explore/BaseSearchPresenterTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/explore/BaseSearchPresenterTest.kt @@ -4,7 +4,9 @@ import androidx.arch.core.executor.testing.InstantTaskExecutorRule import androidx.lifecycle.LiveData import androidx.paging.PagedList import com.jraska.livedata.test -import com.nhaarman.mockitokotlin2.* +import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.verify +import com.nhaarman.mockitokotlin2.whenever import io.reactivex.processors.PublishProcessor import io.reactivex.schedulers.TestScheduler import org.junit.Before diff --git a/app/src/test/kotlin/fr/free/nrw/commons/explore/media/PageableMediaDataSourceTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/explore/media/PageableMediaDataSourceTest.kt index 41395fd8a..dabfd9a9a 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/explore/media/PageableMediaDataSourceTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/explore/media/PageableMediaDataSourceTest.kt @@ -1,10 +1,7 @@ package fr.free.nrw.commons.explore.media import com.nhaarman.mockitokotlin2.mock -import com.nhaarman.mockitokotlin2.verify import com.nhaarman.mockitokotlin2.whenever -import fr.free.nrw.commons.Media -import fr.free.nrw.commons.depictions.Media.DepictedImagesFragment.PAGE_ID_PREFIX import fr.free.nrw.commons.media.MediaClient import io.reactivex.Single import org.hamcrest.MatcherAssert.assertThat @@ -13,10 +10,6 @@ import org.junit.Before import org.junit.Test import org.mockito.Mock 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.wikidata.Entities class PageableMediaDataSourceTest { @Mock @@ -31,41 +24,10 @@ class PageableMediaDataSourceTest { @Test fun `loadFunction invokes mediaClient and has Label`() { - val (media, entity: Entities.Entity) = expectMediaAndEntity() - val label: Entities.Label = mock() - whenever(entity.labels()).thenReturn(mapOf(" " to label)) - whenever(label.value()).thenReturn("label") - val pageableMediaDataSource = PageableMediaDataSource(mock(), mediaConverter, mediaClient) - pageableMediaDataSource.onQueryUpdated("test") - assertThat(pageableMediaDataSource.loadFunction(0,1), `is`(listOf(media))) - verify(media).caption = "label" - } - - @Test - fun `loadFunction invokes mediaClient and does not have Label`() { - val (media, entity: Entities.Entity) = expectMediaAndEntity() - whenever(entity.labels()).thenReturn(mapOf()) - val pageableMediaDataSource = PageableMediaDataSource(mock(), mediaConverter, mediaClient) - pageableMediaDataSource.onQueryUpdated("test") - assertThat(pageableMediaDataSource.loadFunction(0,1), `is`(listOf(media))) - verify(media).caption = MediaClient.NO_CAPTION - } - - private fun expectMediaAndEntity(): Pair { - val queryResponse: MwQueryResponse = mock() whenever(mediaClient.getMediaListFromSearch("test", 0, 1)) - .thenReturn(Single.just(queryResponse)) - val queryResult: MwQueryResult = mock() - whenever(queryResponse.query()).thenReturn(queryResult) - val queryPage: MwQueryPage = mock() - whenever(queryResult.pages()).thenReturn(listOf(queryPage)) - val media = mock() - whenever(mediaConverter.convert(queryPage)).thenReturn(media) - whenever(media.pageId).thenReturn("1") - val entities: Entities = mock() - whenever(mediaClient.getEntities("${PAGE_ID_PREFIX}1")).thenReturn(Single.just(entities)) - val entity: Entities.Entity = mock() - whenever(entities.entities()).thenReturn(mapOf("" to entity)) - return Pair(media, entity) + .thenReturn(Single.just(emptyList())) + val pageableMediaDataSource = PageableMediaDataSource(mock(), mediaClient) + pageableMediaDataSource.onQueryUpdated("test") + assertThat(pageableMediaDataSource.loadFunction(0,1), `is`(emptyList())) } } diff --git a/app/src/test/kotlin/fr/free/nrw/commons/media/MediaClientTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/media/MediaClientTest.kt index 44fbc8303..4dfe7c433 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/media/MediaClientTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/media/MediaClientTest.kt @@ -1,11 +1,12 @@ package fr.free.nrw.commons.media +import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.whenever import fr.free.nrw.commons.Media +import fr.free.nrw.commons.explore.media.MediaConverter import fr.free.nrw.commons.media.model.PageMediaListItem import fr.free.nrw.commons.media.model.PageMediaListResponse import fr.free.nrw.commons.utils.CommonsDateUtil -import io.reactivex.Observable import io.reactivex.Single import junit.framework.Assert.* import org.junit.Before @@ -17,6 +18,7 @@ import org.wikipedia.dataclient.mwapi.MwQueryPage import org.wikipedia.dataclient.mwapi.MwQueryResponse import org.wikipedia.dataclient.mwapi.MwQueryResult import org.wikipedia.gallery.ImageInfo +import org.wikipedia.wikidata.Entities import java.util.* @@ -24,12 +26,14 @@ class MediaClientTest { @Mock internal var mediaInterface: MediaInterface? = null + @Mock + internal var mediaConverter: MediaConverter? = null + @Mock + internal var mediaDetailInterface: MediaDetailInterface? = null @Mock internal var pageMediaInterface: PageMediaInterface? = null - @Mock - internal var mediaDetailInterface: MediaDetailInterface? = null @InjectMocks var mediaClient: MediaClient? = null @@ -51,7 +55,7 @@ class MediaClientTest { `when`(mockResponse.query()).thenReturn(mwQueryResult) `when`(mediaInterface!!.checkPageExistsUsingTitle(ArgumentMatchers.anyString())) - .thenReturn(Observable.just(mockResponse)) + .thenReturn(Single.just(mockResponse)) val checkPageExistsUsingTitle = mediaClient!!.checkPageExistsUsingTitle("File:Test.jpg").blockingGet() @@ -69,7 +73,7 @@ class MediaClientTest { `when`(mockResponse.query()).thenReturn(mwQueryResult) `when`(mediaInterface!!.checkPageExistsUsingTitle(ArgumentMatchers.anyString())) - .thenReturn(Observable.just(mockResponse)) + .thenReturn(Single.just(mockResponse)) val checkPageExistsUsingTitle = mediaClient!!.checkPageExistsUsingTitle("File:Test.jpg").blockingGet() @@ -87,7 +91,7 @@ class MediaClientTest { `when`(mockResponse.query()).thenReturn(mwQueryResult) `when`(mediaInterface!!.checkFileExistsUsingSha(ArgumentMatchers.anyString())) - .thenReturn(Observable.just(mockResponse)) + .thenReturn(Single.just(mockResponse)) val checkFileExistsUsingSha = mediaClient!!.checkFileExistsUsingSha("abcde").blockingGet() assertTrue(checkFileExistsUsingSha) @@ -104,7 +108,7 @@ class MediaClientTest { `when`(mockResponse.query()).thenReturn(mwQueryResult) `when`(mediaInterface!!.checkFileExistsUsingSha(ArgumentMatchers.anyString())) - .thenReturn(Observable.just(mockResponse)) + .thenReturn(Single.just(mockResponse)) val checkFileExistsUsingSha = mediaClient!!.checkFileExistsUsingSha("abcde").blockingGet() assertFalse(checkFileExistsUsingSha) @@ -112,21 +116,12 @@ class MediaClientTest { @Test fun getMedia() { - val imageInfo = ImageInfo() - - val mwQueryPage = mock(MwQueryPage::class.java) - `when`(mwQueryPage.title()).thenReturn("Test") - `when`(mwQueryPage.imageInfo()).thenReturn(imageInfo) - - val mwQueryResult = mock(MwQueryResult::class.java) - `when`(mwQueryResult.firstPage()).thenReturn(mwQueryPage) - val mockResponse = mock(MwQueryResponse::class.java) - `when`(mockResponse.query()).thenReturn(mwQueryResult) + val (mockResponse, media: Media) = expectGetEntitiesAndMediaConversion() `when`(mediaInterface!!.getMedia(ArgumentMatchers.anyString())) - .thenReturn(Observable.just(mockResponse)) + .thenReturn(Single.just(mockResponse)) - assertEquals("Test", mediaClient!!.getMedia("abcde").blockingGet().filename) + mediaClient!!.getMedia("abcde").test().assertValue(media) } @Test @@ -143,34 +138,34 @@ class MediaClientTest { `when`(mockResponse.query()).thenReturn(mwQueryResult) `when`(mediaInterface!!.getMedia(ArgumentMatchers.anyString())) - .thenReturn(Observable.just(mockResponse)) - - assertEquals(Media.EMPTY, mediaClient!!.getMedia("abcde").blockingGet()) + .thenReturn(Single.just(mockResponse)) + mediaClient!!.getMedia("abcde").test().assertErrorMessage("empty list passed for ids") } - @Captor - private val filenameCaptor: ArgumentCaptor? = null - @Test fun getPictureOfTheDay() { val template = "Template:Potd/" + CommonsDateUtil.getIso8601DateFormatShort().format(Date()) - val imageInfo = ImageInfo() + val (mockResponse, media: Media) = expectGetEntitiesAndMediaConversion() + `when`(mediaInterface!!.getMediaWithGenerator(template)) + .thenReturn(Single.just(mockResponse)) + mediaClient!!.getPictureOfTheDay().test().assertValue(media) + } - val mwQueryPage = mock(MwQueryPage::class.java) - `when`(mwQueryPage.title()).thenReturn("Test") - `when`(mwQueryPage.imageInfo()).thenReturn(imageInfo) - - val mwQueryResult = mock(MwQueryResult::class.java) - `when`(mwQueryResult.firstPage()).thenReturn(mwQueryPage) + private fun expectGetEntitiesAndMediaConversion(): Pair { val mockResponse = mock(MwQueryResponse::class.java) - `when`(mockResponse.query()).thenReturn(mwQueryResult) - - `when`(mediaInterface!!.getMediaWithGenerator(filenameCaptor!!.capture())) - .thenReturn(Observable.just(mockResponse)) - - assertEquals("Test", mediaClient!!.pictureOfTheDay.blockingGet().filename) - assertEquals(template, filenameCaptor.value); + val queryResult: MwQueryResult = mock() + whenever(mockResponse.query()).thenReturn(queryResult) + val queryPage: MwQueryPage = mock() + whenever(queryResult.pages()).thenReturn(listOf(queryPage)) + whenever(queryPage.pageId()).thenReturn(0) + val entities: Entities = mock() + whenever(mediaDetailInterface!!.getEntity("M0")).thenReturn(Single.just(entities)) + val entity: Entities.Entity = mock() + whenever(entities.entities()).thenReturn(mapOf("id" to entity)) + val media: Media = mock() + whenever(mediaConverter!!.convert(queryPage, entity)).thenReturn(media) + return Pair(mockResponse, media) } @Captor @@ -179,17 +174,8 @@ class MediaClientTest { @Test fun getMediaListFromCategoryTwice() { val mockContinuation = mapOf(Pair("gcmcontinue", "test")) - val imageInfo = ImageInfo() - val mwQueryPage = mock(MwQueryPage::class.java) - `when`(mwQueryPage.title()).thenReturn("Test") - `when`(mwQueryPage.imageInfo()).thenReturn(imageInfo) - - val mwQueryResult = mock(MwQueryResult::class.java) - `when`(mwQueryResult.pages()).thenReturn(listOf(mwQueryPage)) - - val mockResponse = mock(MwQueryResponse::class.java) - `when`(mockResponse.query()).thenReturn(mwQueryResult) + val (mockResponse, media: Media) = expectGetEntitiesAndMediaConversion() `when`(mockResponse.continuation()).thenReturn(mockContinuation) `when`( @@ -198,31 +184,23 @@ class MediaClientTest { continuationCaptor!!.capture() ) ) - .thenReturn(Observable.just(mockResponse)) + .thenReturn(Single.just(mockResponse)) + val media1 = mediaClient!!.getMediaListFromCategory("abcde").blockingGet().get(0) val media2 = mediaClient!!.getMediaListFromCategory("abcde").blockingGet().get(0) assertEquals(continuationCaptor.allValues[0], emptyMap()) assertEquals(continuationCaptor.allValues[1], mockContinuation) - assertEquals(media1.filename, "Test") - assertEquals(media2.filename, "Test") + assertEquals(media1, media) + assertEquals(media2, media) } @Test fun getMediaListForUser() { val mockContinuation = mapOf("gcmcontinue" to "test") - val imageInfo = ImageInfo() - val mwQueryPage = mock(MwQueryPage::class.java) - whenever(mwQueryPage.title()).thenReturn("Test") - whenever(mwQueryPage.imageInfo()).thenReturn(imageInfo) - - val mwQueryResult = mock(MwQueryResult::class.java) - whenever(mwQueryResult.pages()).thenReturn(listOf(mwQueryPage)) - - val mockResponse = mock(MwQueryResponse::class.java) - whenever(mockResponse.query()).thenReturn(mwQueryResult) + val (mockResponse, media: Media) = expectGetEntitiesAndMediaConversion() whenever(mockResponse.continuation()).thenReturn(mockContinuation) whenever( @@ -231,7 +209,7 @@ class MediaClientTest { continuationCaptor!!.capture() ) ) - .thenReturn(Observable.just(mockResponse)) + .thenReturn(Single.just(mockResponse)) val media1 = mediaClient!!.getMediaListForUser("Test").blockingGet().get(0) val media2 = mediaClient!!.getMediaListForUser("Test").blockingGet().get(0) @@ -251,7 +229,7 @@ class MediaClientTest { mockResponse.setParse(mwParseResult) `when`(mediaInterface!!.getPageHtml(ArgumentMatchers.anyString())) - .thenReturn(Observable.just(mockResponse)) + .thenReturn(Single.just(mockResponse)) assertEquals("Test", mediaClient!!.getPageHtml("abcde").blockingGet()) } @@ -282,7 +260,7 @@ class MediaClientTest { mockResponse.setParse(null) `when`(mediaInterface!!.getPageHtml(ArgumentMatchers.anyString())) - .thenReturn(Observable.just(mockResponse)) + .thenReturn(Single.just(mockResponse)) assertEquals("", mediaClient!!.getPageHtml("abcde").blockingGet()) }