From 895b343f81ca7d2c46d158bda68e0f370ebab8cb Mon Sep 17 00:00:00 2001 From: Sean Mac Gillicuddy Date: Mon, 23 Mar 2020 11:30:02 +0000 Subject: [PATCH] #3222 Merge master into Structured Data branch, fix conflicts - review fixes --- .../free/nrw/commons/MediaDataExtractor.java | 105 +++++++++--------- .../category/CategoryImagesListFragment.java | 21 ++-- .../contributions/ContributionViewHolder.java | 4 +- .../fr/free/nrw/commons/db/Converters.java | 3 + .../commons/depictions/GridViewAdapter.java | 2 +- .../Media/DepictedImagesContract.java | 2 +- .../Media/DepictedImagesFragment.java | 36 +++--- .../Media/DepictedImagesPresenter.java | 32 ++---- .../SubClass/SubDepictionListPresenter.java | 33 ++---- .../WikidataItemDetailsActivity.java | 14 +-- .../free/nrw/commons/di/NetworkingModule.java | 9 +- .../explore/depictions/DepictsClient.java | 2 +- .../SearchDepictionsFragmentPresenter.java | 30 ++--- .../explore/images/SearchImageFragment.java | 7 +- .../free/nrw/commons/media/MediaClient.java | 59 ++++------ .../commons/wikidata/WikidataEditService.java | 4 +- .../depictions/DepictedImagesPresenterTest.kt | 5 +- 17 files changed, 163 insertions(+), 205 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/MediaDataExtractor.java b/app/src/main/java/fr/free/nrw/commons/MediaDataExtractor.java index 973eae0d7..96d8c30da 100644 --- a/app/src/main/java/fr/free/nrw/commons/MediaDataExtractor.java +++ b/app/src/main/java/fr/free/nrw/commons/MediaDataExtractor.java @@ -1,20 +1,18 @@ package fr.free.nrw.commons; +import static fr.free.nrw.commons.depictions.Media.DepictedImagesFragment.PAGE_ID_PREFIX; + import androidx.core.text.HtmlCompat; - - import com.google.gson.JsonArray; import com.google.gson.JsonObject; - +import fr.free.nrw.commons.media.MediaClient; +import io.reactivex.Single; import java.util.ArrayList; import java.util.HashMap; import java.util.Map; - import javax.inject.Inject; import javax.inject.Singleton; - -import fr.free.nrw.commons.media.MediaClient; -import io.reactivex.Single; +import org.jetbrains.annotations.NotNull; import timber.log.Timber; /** @@ -25,10 +23,15 @@ import timber.log.Timber; */ @Singleton public class MediaDataExtractor { - private final MediaClient mediaClient; + + private static final int LABEL_BEGIN_INDEX = 3; + private static final int LABEL_END_OFFSET = 3; + private static final int ID_BEGIN_INDEX = 1; + private static final int ID_END_OFFSET = 1; + private final MediaClient mediaClient; @Inject - public MediaDataExtractor(MediaClient mediaClient) { + public MediaDataExtractor(final MediaClient mediaClient) { this.mediaClient = mediaClient; } @@ -38,31 +41,35 @@ public class MediaDataExtractor { * @param filename for which the details are to be fetched * @return full Media object with all details including deletion status and talk page */ - public Single fetchMediaDetails(String filename, String pageId) { - Single mediaSingle = getMediaFromFileName(filename); - Single pageExistsSingle = mediaClient.checkPageExistsUsingTitle("Commons:Deletion_requests/" + filename); - Single discussionSingle = getDiscussion(filename); - Single captionSingle = getCaption("M"+pageId); - Single depictionSingle = getDepictions(filename); - return Single.zip(mediaSingle, pageExistsSingle, discussionSingle, captionSingle, depictionSingle, (media, deletionStatus, discussion, caption, depiction) -> { - media.setDiscussion(discussion); - media.setCaption(caption); - media.setDepiction(formatDepictions(depiction)); - if (deletionStatus) { - media.setRequestedDeletion(); - } - return media; - }); + public Single fetchMediaDetails(final String filename, final String pageId) { + return Single.zip(getMediaFromFileName(filename), + mediaClient.checkPageExistsUsingTitle("Commons:Deletion_requests/" + filename), + getDiscussion(filename), + getCaption(PAGE_ID_PREFIX + pageId), + getDepictions(filename), + this::combineToMedia); } - /** + @NotNull + private Media combineToMedia(final Media media, final Boolean deletionStatus, final String discussion, + final String caption, final JsonObject depiction) { + media.setDiscussion(discussion); + media.setCaption(caption); + media.setDepiction(formatDepictions(depiction)); + if (deletionStatus) { + media.setRequestedDeletion(); + } + return media; + } + + /** * 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 Single getCaption(String wikibaseIdentifier) { + private Single getCaption(final String wikibaseIdentifier) { return mediaClient.getCaptionByWikibaseIdentifier(wikibaseIdentifier); } @@ -72,27 +79,23 @@ public class MediaDataExtractor { * @return List containing map for depictions, the map has two keys, * first key is for the label and second is for the url of the item */ - private ArrayList> formatDepictions(JsonObject mediaResponse) { + private ArrayList> formatDepictions(final JsonObject mediaResponse) { try { - JsonArray depictionArray = (JsonArray) mediaResponse.get("Depiction"); - ArrayList> depictedItemList = new ArrayList<>(); - try { - for (int i = 0; i depictedObject = new HashMap<>(); - String label = depictedItem.get("label").toString(); - String id = depictedItem.get("id").toString(); - String transformedLabel = label.substring(3, label.length()-3); - String transformedId = id.substring(1,id.length() - 1); - depictedObject.put("label", transformedLabel); //remove the additional characters obtained in label and ID object to extract the relevant string (since the string also contains extra quites that are not required) - depictedObject.put("id", transformedId); - depictedItemList.add(depictedObject); - } - return depictedItemList; - } catch (NullPointerException e) { - return new ArrayList<>(); + final JsonArray depictionArray = (JsonArray) mediaResponse.get("Depiction"); + final ArrayList> depictedItemList = new ArrayList<>(); + for (int i = 0; i depictedObject = new HashMap<>(); + final String label = depictedItem.get("label").toString(); + final String id = depictedItem.get("id").toString(); + final String transformedLabel = label.substring(LABEL_BEGIN_INDEX, label.length()- LABEL_END_OFFSET); + final String transformedId = id.substring(ID_BEGIN_INDEX,id.length() - ID_END_OFFSET); + depictedObject.put("label", transformedLabel); //remove the additional characters obtained in label and ID object to extract the relevant string (since the string also contains extra quites that are not required) + depictedObject.put("id", transformedId); + depictedItemList.add(depictedObject); } - } catch (ClassCastException c) { + return depictedItemList; + } catch (final ClassCastException | NullPointerException ignore) { return new ArrayList<>(); } } @@ -102,13 +105,9 @@ public class MediaDataExtractor { * @param filename the filename we will return the caption for * @return a map containing caption and depictions (empty string in the map if no caption/depictions) */ - private Single getDepictions(String filename) { + private Single getDepictions(final String filename) { return mediaClient.getCaptionAndDepictions(filename) - .map(mediaResponse -> { - return mediaResponse; - }).doOnError(throwable -> { - Timber.e(throwable+ "error while fetching depictions"); - }); + .doOnError(throwable -> Timber.e(throwable, "error while fetching depictions")); } /** @@ -116,7 +115,7 @@ public class MediaDataExtractor { * @param filename Eg. File:Test.jpg * @return return data rich Media object */ - public Single getMediaFromFileName(String filename) { + public Single getMediaFromFileName(final String filename) { return mediaClient.getMedia(filename); } @@ -125,7 +124,7 @@ public class MediaDataExtractor { * @param filename * @return */ - private Single getDiscussion(String filename) { + private Single getDiscussion(final String filename) { return mediaClient.getPageHtml(filename.replace("File", "File talk")) .map(discussion -> HtmlCompat.fromHtml(discussion, HtmlCompat.FROM_HTML_MODE_LEGACY).toString()) .onErrorReturn(throwable -> { 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 b5f81ecf7..571d1c628 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 @@ -1,5 +1,9 @@ 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; import android.view.LayoutInflater; @@ -12,15 +16,7 @@ import android.widget.ListAdapter; import android.widget.ProgressBar; import android.widget.RelativeLayout; import android.widget.TextView; - import androidx.annotation.Nullable; - -import java.util.List; -import java.util.concurrent.TimeUnit; - -import javax.inject.Inject; -import javax.inject.Named; - import butterknife.BindView; import butterknife.ButterKnife; import dagger.android.support.DaggerFragment; @@ -33,11 +29,12 @@ import fr.free.nrw.commons.utils.ViewUtil; import io.reactivex.android.schedulers.AndroidSchedulers; import io.reactivex.disposables.CompositeDisposable; import io.reactivex.schedulers.Schedulers; +import java.util.List; +import java.util.concurrent.TimeUnit; +import javax.inject.Inject; +import javax.inject.Named; import timber.log.Timber; -import static android.view.View.GONE; -import static android.view.View.VISIBLE; - /** * Displays images for a particular category with load more on scrolling incorporated */ @@ -261,7 +258,7 @@ public class CategoryImagesListFragment extends DaggerFragment { isLoading = false; statusTextView.setVisibility(GONE); for (Media m : collection) { - replaceTitlesWithCaptions("M"+m.getPageId(), mediaSize++); + replaceTitlesWithCaptions(PAGE_ID_PREFIX + m.getPageId(), mediaSize++); } } 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 c3633aa27..983a60080 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,5 +1,7 @@ package fr.free.nrw.commons.contributions; +import static fr.free.nrw.commons.depictions.Media.DepictedImagesFragment.PAGE_ID_PREFIX; + import android.graphics.Color; import android.graphics.drawable.ColorDrawable; import android.net.Uri; @@ -117,7 +119,7 @@ public class ContributionViewHolder extends RecyclerView.ViewHolder { titleView.setText(contribution.getDisplayTitle()); } else { Timber.d("Fetching caption for %s", contribution.getFilename()); - String wikibaseMediaId = "M"+contribution.getPageId(); // 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 + String wikibaseMediaId = PAGE_ID_PREFIX + contribution.getPageId(); // 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()) 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 7ce736f49..7a824922c 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 @@ -12,6 +12,9 @@ import java.util.Date; import java.util.HashMap; import java.util.Map; +/** + * This class supplies converters to write/read types to/from the database. + */ public class Converters { public static Gson getGson() { 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 17bd115e2..56cb73728 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 @@ -57,7 +57,7 @@ public class GridViewAdapter extends ArrayAdapter { data = new ArrayList<>(); return false; } - if (data.size() <= 0) { + if (data.size() == 0) { return false; } String fileName = data.get(0).getFilename(); 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 132dd4659..67e59390c 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 @@ -34,7 +34,7 @@ public interface DepictedImagesContract { /** * Seat caption to the image at the given position */ - void handleLabelforImage(String s, int position); + void handleLabelforImage(String caption, int position); /** * Display snackbar 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 d63ad4903..34184cb5d 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 @@ -1,5 +1,8 @@ package fr.free.nrw.commons.depictions.Media; +import static android.view.View.GONE; +import static android.view.View.VISIBLE; + import android.os.Bundle; import android.view.LayoutInflater; import android.view.View; @@ -11,26 +14,20 @@ import android.widget.ListAdapter; import android.widget.ProgressBar; import android.widget.RelativeLayout; import android.widget.TextView; - import androidx.annotation.NonNull; import androidx.annotation.Nullable; - -import java.util.List; - -import javax.inject.Inject; - import butterknife.BindView; import butterknife.ButterKnife; import dagger.android.support.DaggerFragment; import fr.free.nrw.commons.Media; import fr.free.nrw.commons.R; -import fr.free.nrw.commons.depictions.WikidataItemDetailsActivity; import fr.free.nrw.commons.depictions.GridViewAdapter; +import fr.free.nrw.commons.depictions.WikidataItemDetailsActivity; import fr.free.nrw.commons.utils.NetworkUtils; import fr.free.nrw.commons.utils.ViewUtil; - -import static android.view.View.GONE; -import static android.view.View.VISIBLE; +import java.util.List; +import javax.inject.Inject; +import timber.log.Timber; /** * Fragment for showing image list after selected an item from SearchActivity In Explore @@ -38,6 +35,7 @@ import static android.view.View.VISIBLE; public class DepictedImagesFragment extends DaggerFragment implements DepictedImagesContract.View { + public static final String PAGE_ID_PREFIX = "M"; @BindView(R.id.statusMessage) TextView statusTextView; @BindView(R.id.loadingImagesProgressBar) @@ -87,7 +85,9 @@ public class DepictedImagesFragment extends DaggerFragment implements DepictedIm presenter.initList(entityId); if (!NetworkUtils.isInternetConnectionEstablished(getContext())) { handleNoInternet(); - } else presenter.initList(entityId); + } else { + presenter.initList(entityId); + } } /** @@ -150,9 +150,9 @@ public class DepictedImagesFragment extends DaggerFragment implements DepictedIm * Seat caption to the image at the given position */ @Override - public void handleLabelforImage(String s, int position) { - if (!s.trim().equals(getString(R.string.detail_caption_empty))) { - gridAdapter.getItem(position).setThumbnailTitle(s); + public void handleLabelforImage(String caption, int position) { + if (!caption.trim().equals(getString(R.string.detail_caption_empty))) { + gridAdapter.getItem(position).setThumbnailTitle(caption); gridAdapter.notifyDataSetChanged(); } } @@ -250,15 +250,15 @@ public class DepictedImagesFragment extends DaggerFragment implements DepictedIm try { ((WikidataItemDetailsActivity) getContext()).viewPagerNotifyDataSetChanged(); - } catch (Exception e) { - e.printStackTrace(); + } catch (RuntimeException e) { + Timber.e(e); } } progressBar.setVisibility(GONE); isLoading = false; statusTextView.setVisibility(GONE); - for (Media m : collection) { - presenter.replaceTitlesWithCaptions("M"+m.getPageId(), mediaSize++); + for (Media media : collection) { + presenter.replaceTitlesWithCaptions(PAGE_ID_PREFIX +media.getPageId(), 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 bf408e9db..372aa60a4 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 @@ -1,26 +1,22 @@ package fr.free.nrw.commons.depictions.Media; +import static fr.free.nrw.commons.di.CommonsApplicationModule.IO_THREAD; +import static fr.free.nrw.commons.di.CommonsApplicationModule.MAIN_THREAD; + import android.annotation.SuppressLint; - -import java.lang.reflect.Proxy; -import java.util.ArrayList; -import java.util.List; -import java.util.concurrent.TimeUnit; - -import javax.inject.Inject; -import javax.inject.Named; - 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; import io.reactivex.disposables.CompositeDisposable; +import java.lang.reflect.Proxy; +import java.util.ArrayList; +import java.util.List; +import javax.inject.Inject; +import javax.inject.Named; import timber.log.Timber; -import static fr.free.nrw.commons.di.CommonsApplicationModule.IO_THREAD; -import static fr.free.nrw.commons.di.CommonsApplicationModule.MAIN_THREAD; - /** * Presenter for DepictedImagesFragment */ @@ -31,7 +27,6 @@ public class DepictedImagesPresenter implements DepictedImagesContract.UserActio DepictedImagesContract.View.class.getClassLoader(), new Class[]{DepictedImagesContract.View.class}, (proxy, method, methodArgs) -> null); - private static int TIMEOUT_SECONDS = 15; DepictsClient depictsClient; MediaClient mediaClient; @Named("default_preferences") @@ -76,10 +71,9 @@ public class DepictedImagesPresenter implements DepictedImagesContract.UserActio view.setLoadingStatus(true); view.progressBarVisible(true); view.setIsLastPage(false); - compositeDisposable.add(depictsClient.fetchImagesForDepictedItem(entityId, 25, 0) + compositeDisposable.add(depictsClient.fetchImagesForDepictedItem(entityId, 0) .subscribeOn(ioScheduler) .observeOn(mainThreadScheduler) - .timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) .subscribe(this::handleSuccess, this::handleError)); } @@ -90,10 +84,9 @@ public class DepictedImagesPresenter implements DepictedImagesContract.UserActio @Override public void fetchMoreImages() { view.progressBarVisible(true); - compositeDisposable.add(depictsClient.fetchImagesForDepictedItem(entityId, 25, queryList.size()) + compositeDisposable.add(depictsClient.fetchImagesForDepictedItem(entityId, queryList.size()) .subscribeOn(ioScheduler) .observeOn(mainThreadScheduler) - .timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) .subscribe(this::handlePaginationSuccess, this::handleError)); } @@ -150,9 +143,8 @@ public class DepictedImagesPresenter implements DepictedImagesContract.UserActio compositeDisposable.add(mediaClient.getCaptionByWikibaseIdentifier(wikibaseIdentifier) .subscribeOn(ioScheduler) .observeOn(mainThreadScheduler) - .timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) - .subscribe(subscriber -> { - view.handleLabelforImage(subscriber, position); + .subscribe(caption -> { + view.handleLabelforImage(caption, position); })); } diff --git a/app/src/main/java/fr/free/nrw/commons/depictions/SubClass/SubDepictionListPresenter.java b/app/src/main/java/fr/free/nrw/commons/depictions/SubClass/SubDepictionListPresenter.java index 0eae3f586..9781a7f46 100644 --- a/app/src/main/java/fr/free/nrw/commons/depictions/SubClass/SubDepictionListPresenter.java +++ b/app/src/main/java/fr/free/nrw/commons/depictions/SubClass/SubDepictionListPresenter.java @@ -1,14 +1,7 @@ package fr.free.nrw.commons.depictions.SubClass; -import java.io.IOException; -import java.lang.reflect.Proxy; -import java.util.ArrayList; -import java.util.Date; -import java.util.List; -import java.util.concurrent.TimeUnit; - -import javax.inject.Inject; -import javax.inject.Named; +import static fr.free.nrw.commons.di.CommonsApplicationModule.IO_THREAD; +import static fr.free.nrw.commons.di.CommonsApplicationModule.MAIN_THREAD; import fr.free.nrw.commons.explore.depictions.DepictsClient; import fr.free.nrw.commons.explore.recentsearches.RecentSearch; @@ -16,14 +9,17 @@ import fr.free.nrw.commons.explore.recentsearches.RecentSearchesDao; import fr.free.nrw.commons.mwapi.OkHttpJsonApiClient; import fr.free.nrw.commons.upload.structure.depictions.DepictedItem; import io.reactivex.Scheduler; -import io.reactivex.android.schedulers.AndroidSchedulers; import io.reactivex.disposables.CompositeDisposable; -import io.reactivex.schedulers.Schedulers; +import java.io.IOException; +import java.lang.reflect.Proxy; +import java.util.ArrayList; +import java.util.Date; +import java.util.List; +import java.util.concurrent.TimeUnit; +import javax.inject.Inject; +import javax.inject.Named; import timber.log.Timber; -import static fr.free.nrw.commons.di.CommonsApplicationModule.IO_THREAD; -import static fr.free.nrw.commons.di.CommonsApplicationModule.MAIN_THREAD; - /** * Presenter for parent classes and child classes of Depicted items in Explore */ @@ -155,13 +151,8 @@ public class SubDepictionListPresenter implements SubDepictionListContract.UserA */ private void handleError(Throwable throwable) { Timber.e(throwable, "Error occurred while loading queried depictions"); - try { - view.initErrorView(); - view.showSnackbar(); - } catch (Exception e) { - e.printStackTrace(); - } - + view.initErrorView(); + view.showSnackbar(); } } diff --git a/app/src/main/java/fr/free/nrw/commons/depictions/WikidataItemDetailsActivity.java b/app/src/main/java/fr/free/nrw/commons/depictions/WikidataItemDetailsActivity.java index a9dc53e77..e9b858763 100644 --- a/app/src/main/java/fr/free/nrw/commons/depictions/WikidataItemDetailsActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/depictions/WikidataItemDetailsActivity.java @@ -33,7 +33,7 @@ import fr.free.nrw.commons.upload.structure.depictions.DepictedItem; public class WikidataItemDetailsActivity extends NavigationBaseActivity implements MediaDetailPagerFragment.MediaDetailProvider, AdapterView.OnItemClickListener { private FragmentManager supportFragmentManager; private DepictedImagesFragment depictionImagesListFragment; - private MediaDetailPagerFragment mediaDetails; + private MediaDetailPagerFragment mediaDetailPagerFragment; /** * Name of the depicted item * Ex: Rabbit @@ -78,8 +78,8 @@ public class WikidataItemDetailsActivity extends NavigationBaseActivity implemen * The viewpager will notified that number of items have changed. */ public void viewPagerNotifyDataSetChanged() { - if (mediaDetails!=null){ - mediaDetails.notifyDataSetChanged(); + if (mediaDetailPagerFragment !=null){ + mediaDetailPagerFragment.notifyDataSetChanged(); } } @@ -129,18 +129,18 @@ public class WikidataItemDetailsActivity extends NavigationBaseActivity implemen tabLayout.setVisibility(View.GONE); viewPager.setVisibility(View.GONE); mediaContainer.setVisibility(View.VISIBLE); - if (mediaDetails == null || !mediaDetails.isVisible()) { + if (mediaDetailPagerFragment == null || !mediaDetailPagerFragment.isVisible()) { // set isFeaturedImage true for featured images, to include author field on media detail - mediaDetails = new MediaDetailPagerFragment(false, true); + mediaDetailPagerFragment = new MediaDetailPagerFragment(false, true); FragmentManager supportFragmentManager = getSupportFragmentManager(); supportFragmentManager .beginTransaction() - .replace(R.id.mediaContainer, mediaDetails) + .replace(R.id.mediaContainer, mediaDetailPagerFragment) .addToBackStack(null) .commit(); supportFragmentManager.executePendingTransactions(); } - mediaDetails.showImage(position); + mediaDetailPagerFragment.showImage(position); forceInitBackButton(); } diff --git a/app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java b/app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java index 169a16feb..f85c69c36 100644 --- a/app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java +++ b/app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java @@ -54,7 +54,6 @@ public class NetworkingModule { public static final String NAMED_COMMONS_WIKI_SITE = "commons-wikisite"; private static final String NAMED_WIKI_DATA_WIKI_SITE = "wikidata-wikisite"; - private static final String NAMED_COMMONS_WIKI = "commonswiki"; public static final String NAMED_COMMONS_CSRF = "commons-csrf"; @@ -138,12 +137,6 @@ public class NetworkingModule { return new WikiSite(BuildConfig.WIKIDATA_URL); } - @Provides - @Singleton - @Named(NAMED_COMMONS_WIKI) - public WikiSite provideCommonsWiki() { - return new WikiSite(BuildConfig.COMMONS_URL); - } /** * Gson objects are very heavy. The app should ideally be using just one instance of it instead of creating new instances everywhere. @@ -230,7 +223,7 @@ public class NetworkingModule { @Provides @Singleton - public MediaDetailInterface providesMediaDetailInterface(@Named(NAMED_COMMONS_WIKI) WikiSite commonsWikisite) { + public MediaDetailInterface providesMediaDetailInterface(@Named(NAMED_COMMONS_WIKI_SITE) WikiSite commonsWikisite) { return ServiceFactory.get(commonsWikisite, BuildConfig.COMMONS_URL, MediaDetailInterface.class); } diff --git a/app/src/main/java/fr/free/nrw/commons/explore/depictions/DepictsClient.java b/app/src/main/java/fr/free/nrw/commons/explore/depictions/DepictsClient.java index 5ad2ab72f..e4d7f86c7 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/depictions/DepictsClient.java +++ b/app/src/main/java/fr/free/nrw/commons/explore/depictions/DepictsClient.java @@ -105,7 +105,7 @@ public class DepictsClient { /** * @return list of images for a particular depict entity */ - public Observable> fetchImagesForDepictedItem(String query, int limit, int sroffset) { + public Observable> fetchImagesForDepictedItem(String query, int sroffset) { return mediaInterface.fetchImagesForDepictedItem("haswbstatement:" + BuildConfig.DEPICTS_PROPERTY + "=" + query, String.valueOf(sroffset)) .map(mwQueryResponse -> { List mediaList = new ArrayList<>(); diff --git a/app/src/main/java/fr/free/nrw/commons/explore/depictions/SearchDepictionsFragmentPresenter.java b/app/src/main/java/fr/free/nrw/commons/explore/depictions/SearchDepictionsFragmentPresenter.java index bf0d66521..805e3f44f 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/depictions/SearchDepictionsFragmentPresenter.java +++ b/app/src/main/java/fr/free/nrw/commons/explore/depictions/SearchDepictionsFragmentPresenter.java @@ -1,27 +1,24 @@ package fr.free.nrw.commons.explore.depictions; -import java.lang.reflect.Proxy; -import java.util.ArrayList; -import java.util.Date; -import java.util.List; -import java.util.concurrent.TimeUnit; - -import javax.inject.Inject; -import javax.inject.Named; +import static fr.free.nrw.commons.di.CommonsApplicationModule.IO_THREAD; +import static fr.free.nrw.commons.di.CommonsApplicationModule.MAIN_THREAD; import fr.free.nrw.commons.di.CommonsDaggerSupportFragment; import fr.free.nrw.commons.explore.recentsearches.RecentSearch; import fr.free.nrw.commons.explore.recentsearches.RecentSearchesDao; import fr.free.nrw.commons.kvstore.JsonKvStore; import fr.free.nrw.commons.upload.structure.depictions.DepictedItem; - import io.reactivex.Scheduler; import io.reactivex.disposables.CompositeDisposable; +import java.lang.reflect.Proxy; +import java.util.ArrayList; +import java.util.Date; +import java.util.List; +import java.util.concurrent.TimeUnit; +import javax.inject.Inject; +import javax.inject.Named; import timber.log.Timber; -import static fr.free.nrw.commons.di.CommonsApplicationModule.IO_THREAD; -import static fr.free.nrw.commons.di.CommonsApplicationModule.MAIN_THREAD; - /** * The presenter class for SearchDepictionsFragment */ @@ -105,13 +102,8 @@ public class SearchDepictionsFragmentPresenter extends CommonsDaggerSupportFragm */ private void handleError(Throwable throwable) { Timber.e(throwable, "Error occurred while loading queried depictions"); - try { - view.initErrorView(); - view.showSnackbar(); - } catch (Exception e) { - e.printStackTrace(); - } - + view.initErrorView(); + view.showSnackbar(); } /** diff --git a/app/src/main/java/fr/free/nrw/commons/explore/images/SearchImageFragment.java b/app/src/main/java/fr/free/nrw/commons/explore/images/SearchImageFragment.java index ede6a059b..bc7d221b4 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/images/SearchImageFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/explore/images/SearchImageFragment.java @@ -41,6 +41,7 @@ import timber.log.Timber; import static android.view.View.GONE; import static android.view.View.VISIBLE; +import static fr.free.nrw.commons.depictions.Media.DepictedImagesFragment.PAGE_ID_PREFIX; /** * Displays the image search screen. @@ -205,7 +206,7 @@ public class SearchImageFragment extends CommonsDaggerSupportFragment { imagesAdapter.notifyDataSetChanged(); ((SearchActivity)getContext()).viewPagerNotifyDataSetChanged(); for (Media m : mediaList) { - replaceTitlesWithCaptions("M"+m.getPageId(), mediaSize++); + replaceTitlesWithCaptions(PAGE_ID_PREFIX + m.getPageId(), mediaSize++); } } } @@ -215,13 +216,13 @@ public class SearchImageFragment extends CommonsDaggerSupportFragment { * When captions are retrieved they replace title */ - public void replaceTitlesWithCaptions(String wikibaseIdentifier, int i) { + public void replaceTitlesWithCaptions(String wikibaseIdentifier, int position) { compositeDisposable.add(mediaClient.getCaptionByWikibaseIdentifier(wikibaseIdentifier) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) .subscribe(subscriber -> { - handleLabelforImage(subscriber, i); + handleLabelforImage(subscriber, position); })); } diff --git a/app/src/main/java/fr/free/nrw/commons/media/MediaClient.java b/app/src/main/java/fr/free/nrw/commons/media/MediaClient.java index e92e277b5..fcbc7a22e 100644 --- a/app/src/main/java/fr/free/nrw/commons/media/MediaClient.java +++ b/app/src/main/java/fr/free/nrw/commons/media/MediaClient.java @@ -2,36 +2,28 @@ package fr.free.nrw.commons.media; import android.annotation.SuppressLint; - import androidx.annotation.NonNull; - -import org.wikipedia.dataclient.mwapi.MwQueryResponse; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Locale; -import java.util.Map; - -import javax.inject.Inject; -import javax.inject.Singleton; - import com.google.gson.JsonArray; import com.google.gson.JsonElement; import com.google.gson.JsonObject; import com.google.gson.JsonPrimitive; import com.google.gson.internal.LinkedTreeMap; - -import java.util.Date; - - import fr.free.nrw.commons.BuildConfig; import fr.free.nrw.commons.Media; import fr.free.nrw.commons.utils.CommonsDateUtil; import io.reactivex.Observable; import io.reactivex.Single; import io.reactivex.schedulers.Schedulers; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Date; +import java.util.HashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import javax.inject.Inject; +import javax.inject.Singleton; +import org.wikipedia.dataclient.mwapi.MwQueryResponse; import timber.log.Timber; /** @@ -186,26 +178,23 @@ public class MediaClient { public Single getCaptionByWikibaseIdentifier(String wikibaseIdentifier) { return mediaDetailInterface.getCaptionForImage(Locale.getDefault().getLanguage(), wikibaseIdentifier) .map(mediaDetailResponse -> { - if (mediaDetailResponse != null && mediaDetailResponse.getSuccess() != null && mediaDetailResponse.getSuccess() == 1 && mediaDetailResponse.getEntities() != null) { - Map entities = mediaDetailResponse.getEntities(); - try { - Map.Entry entry = entities.entrySet().iterator().next(); - CommonsWikibaseItem commonsWikibaseItem = entry.getValue(); - Map labels = commonsWikibaseItem.getLabels(); - Map.Entry captionEntry = labels.entrySet().iterator().next(); - Caption caption = captionEntry.getValue(); + if (isSuccess(mediaDetailResponse)) { + for (CommonsWikibaseItem wikibaseItem : mediaDetailResponse.getEntities().values()) { + for (Caption caption : wikibaseItem.getLabels().values()) { return caption.getValue(); - - } catch (Exception e) { - return NO_CAPTION; + } } } - return NO_CAPTION; - + return NO_CAPTION; }) .singleOrError(); } + private boolean isSuccess(MediaDetailResponse response) { + return response != null && response.getSuccess() != null + && response.getSuccess() == 1 && response.getEntities() != null; + } + /** * Fetches Structured data from API * @@ -214,9 +203,7 @@ public class MediaClient { */ public Single getCaptionAndDepictions(String filename) { return mediaDetailInterface.fetchStructuredDataByFilename(Locale.getDefault().getLanguage(), filename) - .map(mediaDetailResponse -> { - return fetchCaptionandDepictionsFromMediaDetailResponse(mediaDetailResponse); - }) + .map(this::fetchCaptionandDepictionsFromMediaDetailResponse) .singleOrError(); } @@ -228,7 +215,7 @@ public class MediaClient { @SuppressLint("CheckResult") private JsonObject fetchCaptionandDepictionsFromMediaDetailResponse(MediaDetailResponse mediaDetailResponse) { JsonObject mediaDetails = new JsonObject(); - if (mediaDetailResponse != null && mediaDetailResponse.getSuccess() != null && mediaDetailResponse.getSuccess() == 1 && mediaDetailResponse.getEntities() != null) { + if (isSuccess(mediaDetailResponse)) { Map entities = mediaDetailResponse.getEntities(); try { Map.Entry entry = entities.entrySet().iterator().next(); @@ -247,7 +234,6 @@ public class MediaClient { try { LinkedTreeMap statements = (LinkedTreeMap) commonsWikibaseItem.getStatements(); ArrayList depictsItemList = (ArrayList) statements.get(BuildConfig.DEPICTS_PROPERTY); - String depictions = null; JsonArray jsonArray = new JsonArray(); for (int i = 0; i < depictsItemList.size(); i++) { LinkedTreeMap depictedItem = depictsItemList.get(i); @@ -268,7 +254,6 @@ public class MediaClient { } catch (Exception e) { JsonElement jsonElement = new JsonPrimitive(NO_CAPTION); mediaDetails.add("Caption", jsonElement); - jsonElement = null; jsonElement = new JsonPrimitive(NO_DEPICTION); mediaDetails.add("Depiction", jsonElement); } diff --git a/app/src/main/java/fr/free/nrw/commons/wikidata/WikidataEditService.java b/app/src/main/java/fr/free/nrw/commons/wikidata/WikidataEditService.java index c6790b8fb..e74473cd5 100644 --- a/app/src/main/java/fr/free/nrw/commons/wikidata/WikidataEditService.java +++ b/app/src/main/java/fr/free/nrw/commons/wikidata/WikidataEditService.java @@ -1,5 +1,7 @@ package fr.free.nrw.commons.wikidata; +import static fr.free.nrw.commons.depictions.Media.DepictedImagesFragment.PAGE_ID_PREFIX; + import android.annotation.SuppressLint; import android.content.Context; import androidx.annotation.NonNull; @@ -198,7 +200,7 @@ public class WikidataEditService { String data = jsonData.toString(); Observable.defer((Callable>) () -> - wikiBaseClient.postEditEntity("M" + fileEntityId, data)) + wikiBaseClient.postEditEntity(PAGE_ID_PREFIX + fileEntityId, data)) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .subscribe(success -> { 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 bb1e34f5a..daa3841f2 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 @@ -55,7 +55,8 @@ class DepictedImagesPresenterTest { @Test fun initList() { - Mockito.`when`(depictsClient?.fetchImagesForDepictedItem(ArgumentMatchers.anyString(), ArgumentMatchers.anyInt(), ArgumentMatchers.anyInt())).thenReturn(testObservable) + Mockito.`when`(depictsClient?.fetchImagesForDepictedItem(ArgumentMatchers.anyString(), + ArgumentMatchers.anyInt())).thenReturn(testObservable) depictedImagesPresenter?.initList("rabbit") depictedImagesPresenter?.handleSuccess(mediaList) verify(view)?.handleSuccess(mediaList) @@ -69,4 +70,4 @@ class DepictedImagesPresenterTest { testScheduler?.triggerActions() verify(view)?.handleLabelforImage("", 0) } -} \ No newline at end of file +}