diff --git a/.idea/inspectionProfiles/Project_Default.xml b/.idea/inspectionProfiles/Project_Default.xml
index 936d6e5a6..a52bc1217 100644
--- a/.idea/inspectionProfiles/Project_Default.xml
+++ b/.idea/inspectionProfiles/Project_Default.xml
@@ -9,6 +9,7 @@
+
diff --git a/app/build.gradle b/app/build.gradle
index 596560b44..4aaad85c2 100644
--- a/app/build.gradle
+++ b/app/build.gradle
@@ -78,6 +78,8 @@ dependencies {
testImplementation "org.powermock:powermock-module-junit4:2.0.0-beta.5"
testImplementation "org.powermock:powermock-api-mockito2:2.0.0-beta.5"
testImplementation 'org.mockito:mockito-core:2.23.0'
+ testImplementation "com.jraska.livedata:testing-ktx:1.1.2"
+ testImplementation "androidx.arch.core:core-testing:2.1.0"
// Android testing
androidTestImplementation "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$KOTLIN_VERSION"
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 dcbf69a5f..f7cda5a7d 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
@@ -32,7 +32,7 @@ public class DepictsClient {
private final DepictsInterface depictsInterface;
private final MediaInterface mediaInterface;
- private static final String NO_DEPICTED_IMAGE = "No Image for Depiction";
+ public static final String NO_DEPICTED_IMAGE = "No Image for Depiction";
@Inject
public DepictsClient(DepictsInterface depictsInterface, MediaInterface mediaInterface) {
@@ -46,14 +46,16 @@ public class DepictsClient {
*/
public Observable searchForDepictions(String query, int limit, int offset) {
return depictsInterface.searchForDepicts(
- query,
- String.valueOf(limit),
- Locale.getDefault().getLanguage(),
- Locale.getDefault().getLanguage(),
- String.valueOf(offset)
+ query,
+ String.valueOf(limit),
+ Locale.getDefault().getLanguage(),
+ Locale.getDefault().getLanguage(),
+ String.valueOf(offset)
)
- .flatMap(depictSearchResponse ->Observable.fromIterable(depictSearchResponse.getSearch()))
- .map(DepictedItem::new);
+ .toObservable()
+ .flatMap( depictSearchResponse ->
+ Observable.fromIterable(depictSearchResponse.getSearch()))
+ .map(DepictedItem::new);
}
/**
@@ -80,15 +82,13 @@ public class DepictsClient {
.map(claimsResponse -> {
final List imageClaim = claimsResponse.getClaims()
.get(WikidataProperties.IMAGE.getPropertyName());
- if (imageClaim != null) {
final DataValueString dataValue = (DataValueString) imageClaim
.get(0)
.getMainSnak()
.getDataValue();
return getThumbnailUrl((dataValue.getValue()));
- }
- return NO_DEPICTED_IMAGE;
})
+ .onErrorReturn(throwable -> NO_DEPICTED_IMAGE)
.singleOrError();
}
diff --git a/app/src/main/java/fr/free/nrw/commons/explore/depictions/SearchDepictionsRenderer.java b/app/src/main/java/fr/free/nrw/commons/explore/depictions/SearchDepictionsRenderer.java
index 581858ef9..6e70e08cd 100644
--- a/app/src/main/java/fr/free/nrw/commons/explore/depictions/SearchDepictionsRenderer.java
+++ b/app/src/main/java/fr/free/nrw/commons/explore/depictions/SearchDepictionsRenderer.java
@@ -1,5 +1,7 @@
package fr.free.nrw.commons.explore.depictions;
+import static fr.free.nrw.commons.explore.depictions.DepictsClient.NO_DEPICTED_IMAGE;
+
import android.graphics.Bitmap;
import android.net.Uri;
import android.text.TextUtils;
@@ -45,9 +47,6 @@ public class SearchDepictionsRenderer extends Renderer {
private DepictCallback listener;
- int size = 0;
- private final static String NO_IMAGE_FOR_DEPICTION = "No Image for Depiction";
-
public SearchDepictionsRenderer(DepictCallback listener) {
this.listener = listener;
}
@@ -84,7 +83,7 @@ public class SearchDepictionsRenderer extends Renderer {
Timber.e("line86"+item.getImageUrl());
if (!TextUtils.isEmpty(item.getImageUrl())) {
- if (!item.getImageUrl().equals(NO_IMAGE_FOR_DEPICTION) && !item.getImageUrl().equals(""))
+ if (!item.getImageUrl().equals(NO_DEPICTED_IMAGE) && !item.getImageUrl().equals(""))
{
ImageRequest imageRequest = ImageRequestBuilder
.newBuilderWithSource(Uri.parse(item.getImageUrl()))
diff --git a/app/src/main/java/fr/free/nrw/commons/repository/UploadRemoteDataSource.java b/app/src/main/java/fr/free/nrw/commons/repository/UploadRemoteDataSource.java
index 7ea44ca68..61fd61736 100644
--- a/app/src/main/java/fr/free/nrw/commons/repository/UploadRemoteDataSource.java
+++ b/app/src/main/java/fr/free/nrw/commons/repository/UploadRemoteDataSource.java
@@ -1,14 +1,5 @@
package fr.free.nrw.commons.repository;
-import fr.free.nrw.commons.upload.ImageCoordinates;
-import java.io.IOException;
-import java.util.Comparator;
-import java.util.List;
-import java.util.Locale;
-
-import javax.inject.Inject;
-import javax.inject.Singleton;
-
import fr.free.nrw.commons.category.CategoriesModel;
import fr.free.nrw.commons.category.CategoryItem;
import fr.free.nrw.commons.contributions.Contribution;
@@ -16,14 +7,22 @@ import fr.free.nrw.commons.filepicker.UploadableFile;
import fr.free.nrw.commons.location.LatLng;
import fr.free.nrw.commons.nearby.NearbyPlaces;
import fr.free.nrw.commons.nearby.Place;
+import fr.free.nrw.commons.upload.ImageCoordinates;
import fr.free.nrw.commons.upload.SimilarImageInterface;
import fr.free.nrw.commons.upload.UploadController;
import fr.free.nrw.commons.upload.UploadModel;
import fr.free.nrw.commons.upload.UploadModel.UploadItem;
import fr.free.nrw.commons.upload.structure.depictions.DepictModel;
import fr.free.nrw.commons.upload.structure.depictions.DepictedItem;
+import io.reactivex.Flowable;
import io.reactivex.Observable;
import io.reactivex.Single;
+import java.io.IOException;
+import java.util.Comparator;
+import java.util.List;
+import java.util.Locale;
+import javax.inject.Inject;
+import javax.inject.Singleton;
/**
* This class would act as the data source for remote operations for UploadActivity
@@ -189,15 +188,13 @@ public class UploadRemoteDataSource {
* @param longitude
* @return
*/
- public Place getNearbyPlaces(double latitude, double longitude) {
- try {
- List fromWikidataQuery = nearbyPlaces.getFromWikidataQuery(new LatLng(latitude, longitude, 0.0f),
- Locale.getDefault().getLanguage(),
- NEARBY_RADIUS_IN_KILO_METERS);
- return fromWikidataQuery.size() > 0 ? fromWikidataQuery.get(0) : null;
- } catch (IOException e) {
- return null;
- }
+ public Place getNearbyPlaces(double latitude, double longitude) throws IOException {
+ List fromWikidataQuery = nearbyPlaces
+ .getFromWikidataQuery(new LatLng(latitude, longitude, 0.0f),
+ Locale.getDefault().getLanguage(),
+ NEARBY_RADIUS_IN_KILO_METERS);
+ return fromWikidataQuery.size() > 0 ? fromWikidataQuery.get(0) : null;
+
}
/**
@@ -220,9 +217,10 @@ public class UploadRemoteDataSource {
/**
* get all depictions
+ * @return
*/
- public Observable searchAllEntities(String query) {
+ public Flowable> searchAllEntities(String query) {
return depictModel.searchAllEntities(query);
}
diff --git a/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java b/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java
index 71d5ba506..9f4d9497a 100644
--- a/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java
+++ b/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java
@@ -1,6 +1,8 @@
package fr.free.nrw.commons.repository;
import fr.free.nrw.commons.upload.ImageCoordinates;
+import io.reactivex.Flowable;
+import java.io.IOException;
import java.util.Comparator;
import java.util.List;
@@ -284,7 +286,7 @@ public class UploadRepository {
* @return
*/
- public Observable searchAllEntities(String query) {
+ public Flowable> searchAllEntities(String query) {
return remoteDataSource.searchAllEntities(query);
}
@@ -294,7 +296,7 @@ public class UploadRepository {
* @param decLongitude
* @return
*/
- public Place checkNearbyPlaces(double decLatitude, double decLongitude) {
+ public Place checkNearbyPlaces(double decLatitude, double decLongitude) throws IOException {
return remoteDataSource.getNearbyPlaces(decLatitude, decLongitude);
}
diff --git a/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.kt b/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.kt
index 14ff8b126..adec4e7f5 100644
--- a/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.kt
+++ b/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.kt
@@ -209,7 +209,7 @@ class FileProcessor @Inject constructor(
.filter { it.size >= MIN_NEARBY_RESULTS }
.take(1)
.subscribe(
- { depictsModel.nearbyPlaces = it },
+ { depictsModel.nearbyPlaces.offer(it) },
{ Timber.e(it) }
)
}
diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadDepictsRenderer.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadDepictsRenderer.java
index cb5b8986c..f87f1b986 100644
--- a/app/src/main/java/fr/free/nrw/commons/upload/UploadDepictsRenderer.java
+++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadDepictsRenderer.java
@@ -1,34 +1,20 @@
package fr.free.nrw.commons.upload;
-import android.graphics.Bitmap;
import android.net.Uri;
import android.text.TextUtils;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
import android.widget.CheckBox;
-import android.widget.ImageView;
import android.widget.TextView;
-
-import androidx.annotation.Nullable;
-
-import com.facebook.common.executors.CallerThreadExecutor;
-import com.facebook.common.references.CloseableReference;
-import com.facebook.datasource.DataSource;
-import com.facebook.drawee.backends.pipeline.Fresco;
-import com.facebook.imagepipeline.core.ImagePipeline;
-import com.facebook.imagepipeline.datasource.BaseBitmapDataSubscriber;
-import com.facebook.imagepipeline.image.CloseableImage;
-import com.facebook.imagepipeline.request.ImageRequest;
-import com.facebook.imagepipeline.request.ImageRequestBuilder;
-import com.pedrogomez.renderers.Renderer;
-
import butterknife.BindView;
import butterknife.ButterKnife;
+import com.facebook.common.util.UriUtil;
+import com.facebook.drawee.view.SimpleDraweeView;
+import com.pedrogomez.renderers.Renderer;
import fr.free.nrw.commons.R;
import fr.free.nrw.commons.upload.structure.depictions.DepictedItem;
import fr.free.nrw.commons.upload.structure.depictions.UploadDepictsCallback;
-import timber.log.Timber;
/**
* Depicts Renderer for setting up inflating layout,
@@ -42,8 +28,7 @@ public class UploadDepictsRenderer extends Renderer {
TextView depictsLabel;
@BindView(R.id.description) TextView description;
@BindView(R.id.depicted_image)
- ImageView imageView;
- private final static String NO_IMAGE_FOR_DEPICTION="No Image for Depiction";
+ SimpleDraweeView imageView;
public UploadDepictsRenderer(UploadDepictsCallback listener) {
this.listener = listener;
@@ -91,45 +76,13 @@ public class UploadDepictsRenderer extends Renderer {
checkedView.setChecked(item.isSelected());
depictsLabel.setText(item.getName());
description.setText(item.getDescription());
- if (!TextUtils.isEmpty(item.getImageUrl())) {
- if (!item.getImageUrl().equals(NO_IMAGE_FOR_DEPICTION))
- setImageView(Uri.parse(item.getImageUrl()), imageView);
- }else{
- listener.fetchThumbnailUrlForEntity(item.getId(),item.getPosition());
+ final String imageUrl = item.getImageUrl();
+ if (TextUtils.isEmpty(imageUrl)) {
+ imageView.setImageURI(UriUtil.getUriForResourceId(R.drawable.ic_wikidata_logo_24dp));
+ listener.fetchThumbnailUrlForEntity(item);
+ } else {
+ imageView.setImageURI(Uri.parse(imageUrl));
}
}
- /**
- * Set thumbnail for the depicted item
- */
- private void setImageView(Uri imageUrl, ImageView imageView) {
- ImageRequest imageRequest = ImageRequestBuilder
- .newBuilderWithSource(imageUrl)
- .setAutoRotateEnabled(true)
- .build();
-
- ImagePipeline imagePipeline = Fresco.getImagePipeline();
- final DataSource>
- dataSource = imagePipeline.fetchDecodedImage(imageRequest, getContext());
-
- dataSource.subscribe(new BaseBitmapDataSubscriber() {
-
- @Override
- public void onNewResultImpl(@Nullable Bitmap bitmap) {
- if (dataSource.isFinished() && bitmap != null) {
- Timber.d("Bitmap loaded from url %s", imageUrl.toString());
- imageView.post(() -> imageView.setImageBitmap(Bitmap.createBitmap(bitmap)));
- dataSource.close();
- }
- }
-
- @Override
- public void onFailureImpl(DataSource dataSource) {
- Timber.d("Error getting bitmap from image url %s", imageUrl.toString());
- if (dataSource != null) {
- dataSource.close();
- }
- }
- }, CallerThreadExecutor.getInstance());
- }
}
diff --git a/app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsContract.java b/app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsContract.java
index 269f721ae..3256e373a 100644
--- a/app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsContract.java
+++ b/app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsContract.java
@@ -1,9 +1,10 @@
package fr.free.nrw.commons.upload.depicts;
-import java.util.List;
-
+import androidx.lifecycle.LiveData;
import fr.free.nrw.commons.BasePresenter;
import fr.free.nrw.commons.upload.structure.depictions.DepictedItem;
+import java.util.List;
+import org.jetbrains.annotations.NotNull;
/**
* The contract with which DepictsFragment and its presenter would talk to each other
@@ -41,10 +42,8 @@ public interface DepictsContract {
*/
void setDepictsList(List depictedItemList);
- /**
- * Set thumbnail image for depicted item
- */
- void onImageUrlFetched(String response, int position);
+
+ void onUrlFetched(@NotNull DepictedItem depictedItem, @NotNull String url);
}
interface UserActionListener extends BasePresenter {
@@ -71,11 +70,8 @@ public interface DepictsContract {
*/
void verifyDepictions();
- /**
- * Fetch thumbnail for the Wikidata Item
- * @param entityId entityId of the item
- * @param position position of the item
- */
- void fetchThumbnailForEntityId(String entityId, int position);
+ LiveData> getDepictedItems();
+
+ void fetchThumbnailForEntityId(DepictedItem depictedItem);
}
}
diff --git a/app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsFragment.java b/app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsFragment.java
index 7394a7d17..fb0abc1ed 100644
--- a/app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsFragment.java
+++ b/app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsFragment.java
@@ -1,6 +1,7 @@
package fr.free.nrw.commons.upload.depicts;
import android.os.Bundle;
+import android.util.Pair;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
@@ -30,6 +31,7 @@ import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.TimeUnit;
import javax.inject.Inject;
+import org.jetbrains.annotations.NotNull;
import timber.log.Timber;
@@ -66,6 +68,7 @@ public class DepictsFragment extends UploadBaseFragment implements DepictsContra
super.onViewCreated(view, savedInstanceState);
ButterKnife.bind(this, view);
init();
+ presenter.getDepictedItems().observe(getViewLifecycleOwner(), this::setDepictsList);
}
/**
@@ -125,9 +128,11 @@ public class DepictsFragment extends UploadBaseFragment implements DepictsContra
@Override
public void showError(Boolean value) {
- if (value)
- depictsSearchContainer.setError(getString(R.string.no_depiction_found));
- else depictsSearchContainer.setErrorEnabled(false);
+ if (value) {
+ depictsSearchContainer.setError(getString(R.string.no_depiction_found));
+ } else {
+ depictsSearchContainer.setErrorEnabled(false);
+ }
}
@Override
@@ -139,13 +144,24 @@ public class DepictsFragment extends UploadBaseFragment implements DepictsContra
}
}
- /**
- * Set thumbnail image for depicted item
- */
@Override
- public void onImageUrlFetched(String response, int position) {
- adapter.getItem(position).setImageUrl(response);
- adapter.notifyItemChanged(position);
+ public void onUrlFetched(@NotNull DepictedItem depictedItem, @NotNull String url) {
+ final Pair itemAndPosition = returnItemAndPosition(depictedItem);
+ if (itemAndPosition != null) {
+ itemAndPosition.first.setImageUrl(url);
+ adapter.notifyItemChanged(itemAndPosition.second);
+ }
+ }
+
+ @Nullable
+ private Pair returnItemAndPosition(@NotNull DepictedItem depictedItem) {
+ for (int i = 0; i < adapter.getItemCount(); i++) {
+ final DepictedItem item = adapter.getItem(i);
+ if(item.getId().equals(depictedItem.getId())){
+ return new Pair<>(item, i);
+ }
+ }
+ return null;
}
@OnClick(R.id.depicts_next)
@@ -167,8 +183,8 @@ public class DepictsFragment extends UploadBaseFragment implements DepictsContra
* Fetch thumbnail for the given entityId at the given position
*/
@Override
- public void fetchThumbnailUrlForEntity(String entityId, int position) {
- presenter.fetchThumbnailForEntityId(entityId,position);
+ public void fetchThumbnailUrlForEntity(DepictedItem depictedItem) {
+ presenter.fetchThumbnailForEntityId(depictedItem);
}
/**
diff --git a/app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsInterface.java b/app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsInterface.java
index e6df617b2..72a77aa3b 100644
--- a/app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsInterface.java
+++ b/app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsInterface.java
@@ -2,6 +2,7 @@ package fr.free.nrw.commons.upload.depicts;
import fr.free.nrw.commons.wikidata.model.DepictSearchResponse;
import io.reactivex.Observable;
+import io.reactivex.Single;
import org.wikipedia.wikidata.ClaimsResponse;
import retrofit2.http.GET;
import retrofit2.http.Query;
@@ -21,7 +22,7 @@ public interface DepictsInterface {
* @param offset number of depictions already fetched useful in implementing pagination
*/
@GET("/w/api.php?action=wbsearchentities&format=json&type=item&uselang=en")
- Observable searchForDepicts(@Query("search") String query, @Query("limit") String limit, @Query("language") String language, @Query("uselang") String uselang, @Query("continue") String offset);
+ Single searchForDepicts(@Query("search") String query, @Query("limit") String limit, @Query("language") String language, @Query("uselang") String uselang, @Query("continue") String offset);
@GET("/w/api.php?action=wbgetclaims&format=json&property=P18")
Observable getImageForEntity(@Query("entity") String entityId);
diff --git a/app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsPresenter.java b/app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsPresenter.java
deleted file mode 100644
index 756a65270..000000000
--- a/app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsPresenter.java
+++ /dev/null
@@ -1,152 +0,0 @@
-package fr.free.nrw.commons.upload.depicts;
-
-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.repository.UploadRepository;
-import fr.free.nrw.commons.upload.structure.depictions.DepictedItem;
-import io.reactivex.Observable;
-import io.reactivex.Scheduler;
-import io.reactivex.android.schedulers.AndroidSchedulers;
-import io.reactivex.disposables.CompositeDisposable;
-import io.reactivex.disposables.Disposable;
-import io.reactivex.schedulers.Schedulers;
-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 javax.inject.Singleton;
-import timber.log.Timber;
-
-/**
- * presenter for DepictsFragment
- */
-@Singleton
-public class DepictsPresenter implements DepictsContract.UserActionListener {
-
- private static final DepictsContract.View DUMMY = (DepictsContract.View) Proxy
- .newProxyInstance(
- DepictsContract.View.class.getClassLoader(),
- new Class[]{DepictsContract.View.class},
- (proxy, method, methodArgs) -> null);
-
-
- private final Scheduler ioScheduler;
- private final Scheduler mainThreadScheduler;
- private DepictsContract.View view = DUMMY;
- private UploadRepository repository;
- private DepictsClient depictsClient;
- private static int TIMEOUT_SECONDS = 15;
-
- private CompositeDisposable compositeDisposable;
-
- @Inject
- public DepictsPresenter(UploadRepository uploadRepository, @Named(IO_THREAD) Scheduler ioScheduler,
- @Named(MAIN_THREAD) Scheduler mainThreadScheduler, DepictsClient depictsClient) {
- this.repository = uploadRepository;
- this.ioScheduler = ioScheduler;
- this.mainThreadScheduler = mainThreadScheduler;
- this.depictsClient = depictsClient;
- compositeDisposable = new CompositeDisposable();
- }
-
- @Override
- public void onAttachView(DepictsContract.View view) {
- this.view = view;
- }
-
- @Override
- public void onDetachView() {
- this.view = DUMMY;
- }
-
- @Override
- public void onPreviousButtonClicked() {
- view.goToPreviousScreen();
- }
-
- @Override
- public void onDepictItemClicked(DepictedItem depictedItem) {
- repository.onDepictItemClicked(depictedItem);
- }
-
- /**
- * asks the repository to fetch depictions for the query
- * @param query
- */
- @Override
- public void searchForDepictions(String query) {
- List depictedItemList = new ArrayList<>();
- Observable distinctDepictsObservable = Observable
- .fromIterable(repository.getSelectedDepictions())
- .subscribeOn(ioScheduler)
- .observeOn(mainThreadScheduler)
- .doOnSubscribe(disposable -> {
- view.showProgress(true);
- view.setDepictsList(null);
- })
- .observeOn(ioScheduler)
- .concatWith(
- repository.searchAllEntities(query)
- )
- .distinct();
-
- Disposable searchDepictsDisposable = distinctDepictsObservable
- .observeOn(mainThreadScheduler)
- .subscribe(
- e -> {
- depictedItemList.add(e);
- },
- t -> {
- view.showProgress(false);
- view.showError(true);
- Timber.e(t);
- },
- () -> {
- view.showProgress(false);
-
- if (depictedItemList.isEmpty()) {
- view.showError(true);
- } else {
- view.showError(false);
- view.setDepictsList(depictedItemList);
- }
- }
- );
- compositeDisposable.add(searchDepictsDisposable);
- view.setDepictsList(depictedItemList);
- }
-
- /**
- * Check if depictions were selected
- * from the depiction list
- */
- @Override
- public void verifyDepictions() {
- List selectedDepictions = repository.getSelectedDepictions();
- if (selectedDepictions != null && !selectedDepictions.isEmpty()) {
- view.goToNextScreen();
- } else {
- view.noDepictionSelected();
- }
- }
-
- /**
- * Fetch thumbnail for the Wikidata Item
- * @param entityId entityId of the item
- * @param position position of the item
- */
- @Override
- public void fetchThumbnailForEntityId(String entityId, int position) {
- compositeDisposable.add(depictsClient.getP18ForItem(entityId)
- .subscribeOn(Schedulers.io())
- .observeOn(AndroidSchedulers.mainThread())
- .timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS)
- .subscribe(response -> {
- view.onImageUrlFetched(response,position);
- }));
- }
-}
diff --git a/app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsPresenter.kt b/app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsPresenter.kt
new file mode 100644
index 000000000..9d07ba342
--- /dev/null
+++ b/app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsPresenter.kt
@@ -0,0 +1,152 @@
+package fr.free.nrw.commons.upload.depicts
+
+import androidx.lifecycle.LiveData
+import androidx.lifecycle.MutableLiveData
+import fr.free.nrw.commons.di.CommonsApplicationModule
+import fr.free.nrw.commons.explore.depictions.DepictsClient
+import fr.free.nrw.commons.explore.depictions.DepictsClient.NO_DEPICTED_IMAGE
+import fr.free.nrw.commons.repository.UploadRepository
+import fr.free.nrw.commons.upload.structure.depictions.DepictedItem
+import io.reactivex.Flowable
+import io.reactivex.Scheduler
+import io.reactivex.Single
+import io.reactivex.disposables.CompositeDisposable
+import io.reactivex.functions.BiFunction
+import io.reactivex.processors.PublishProcessor
+import timber.log.Timber
+import java.lang.reflect.Proxy
+import javax.inject.Inject
+import javax.inject.Named
+import javax.inject.Singleton
+
+/**
+ * presenter for DepictsFragment
+ */
+@Singleton
+class DepictsPresenter @Inject constructor(
+ private val repository: UploadRepository,
+ @param:Named(CommonsApplicationModule.IO_THREAD) private val ioScheduler: Scheduler,
+ @param:Named(CommonsApplicationModule.MAIN_THREAD) private val mainThreadScheduler: Scheduler,
+ private val depictsClient: DepictsClient
+) : DepictsContract.UserActionListener {
+
+ companion object {
+ private val DUMMY = proxy()
+ }
+
+ private var view = DUMMY
+ private val compositeDisposable: CompositeDisposable = CompositeDisposable()
+ private val searchTerm: PublishProcessor = PublishProcessor.create()
+ private val depictedItems: MutableLiveData> = MutableLiveData()
+ private val idsToImageUrls = mutableMapOf()
+
+ override fun onAttachView(view: DepictsContract.View) {
+ this.view = view
+ compositeDisposable.add(
+ searchTerm
+ .observeOn(mainThreadScheduler)
+ .doOnNext { view.showProgress(true) }
+ .switchMap(::searchResultsWithTerm)
+ .observeOn(mainThreadScheduler)
+ .subscribe(
+ { (results, term) ->
+ view.showProgress(false)
+ view.showError(results.isEmpty() && term.isNotEmpty())
+ depictedItems.value = results
+ },
+ { t: Throwable? ->
+ view.showProgress(false)
+ view.showError(true)
+ Timber.e(t)
+ }
+ )
+ )
+ }
+
+ private fun searchResultsWithTerm(it: String): Flowable, String>> {
+ return Flowable.zip(
+ searchResults(it),
+ Flowable.just(it),
+ BiFunction { results: List, term: String -> Pair(results, term) }
+ )
+ }
+
+ private fun searchResults(it: String): Flowable> {
+ return repository.searchAllEntities(it)
+ .subscribeOn(ioScheduler)
+ .map { repository.selectedDepictions + it }
+ .map { it.distinctBy(DepictedItem::id) }
+ .map(::addImageUrlsFromCache)
+ }
+
+ private fun addImageUrlsFromCache(depictions: List) =
+ depictions.map { item ->
+ idsToImageUrls[item.id]?.let { item.copy(imageUrl = it) } ?: item
+ }
+
+ override fun onDetachView() {
+ view = DUMMY
+ compositeDisposable.dispose()
+ idsToImageUrls.clear()
+ }
+
+ override fun onPreviousButtonClicked() {
+ view.goToPreviousScreen()
+ }
+
+ override fun onDepictItemClicked(depictedItem: DepictedItem) {
+ repository.onDepictItemClicked(depictedItem)
+ }
+
+ override fun getDepictedItems(): LiveData> {
+ return depictedItems;
+ }
+
+ /**
+ * asks the repository to fetch depictions for the query
+ * @param query
+ */
+ override fun searchForDepictions(query: String) {
+ searchTerm.onNext(query)
+ }
+
+ /**
+ * Check if depictions were selected
+ * from the depiction list
+ */
+ override fun verifyDepictions() {
+ if (repository.selectedDepictions.isNotEmpty()) {
+ view.goToNextScreen()
+ } else {
+ view.noDepictionSelected()
+ }
+ }
+
+ /**
+ * Fetch thumbnail for the Wikidata Item
+ * @param entityId entityId of the item
+ * @param position position of the item
+ */
+ override fun fetchThumbnailForEntityId(depictedItem: DepictedItem) {
+ compositeDisposable.add(
+ imageUrlFromNetworkOrCache(depictedItem)
+ .observeOn(mainThreadScheduler)
+ .filter { it != NO_DEPICTED_IMAGE }
+ .subscribe(
+ { view.onUrlFetched(depictedItem, it) },
+ { Timber.e(it) }
+ )
+ )
+ }
+
+ private fun imageUrlFromNetworkOrCache(depictedItem: DepictedItem): Single =
+ if (idsToImageUrls.containsKey(depictedItem.id))
+ Single.just(idsToImageUrls[depictedItem.id])
+ else
+ depictsClient.getP18ForItem(depictedItem.id)
+ .subscribeOn(ioScheduler)
+ .doOnSuccess { idsToImageUrls[depictedItem.id] = it }
+}
+
+inline fun proxy() = Proxy
+ .newProxyInstance(T::class.java.classLoader, arrayOf(T::class.java)) { _, _, _ -> null } as T
diff --git a/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaPresenter.java b/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaPresenter.java
index 59cff4c8a..9f8ed530c 100644
--- a/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaPresenter.java
+++ b/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaPresenter.java
@@ -16,7 +16,7 @@ import fr.free.nrw.commons.upload.SimilarImageInterface;
import fr.free.nrw.commons.upload.UploadModel.UploadItem;
import fr.free.nrw.commons.upload.mediaDetails.UploadMediaDetailsContract.UserActionListener;
import fr.free.nrw.commons.upload.mediaDetails.UploadMediaDetailsContract.View;
-import io.reactivex.Observable;
+import io.reactivex.Maybe;
import io.reactivex.Scheduler;
import io.reactivex.disposables.CompositeDisposable;
import io.reactivex.disposables.Disposable;
@@ -84,7 +84,7 @@ public class UploadMediaPresenter implements UserActionListener, SimilarImageInt
checkNearbyPlaces(uploadItem);
}
},
- throwable -> Timber.e(throwable, "Error occurred in processing images"));
+ throwable -> Timber.e(throwable, "Error occurred in pre-processing images"));
compositeDisposable.add(uploadItemDisposable);
}
@@ -93,14 +93,18 @@ public class UploadMediaPresenter implements UserActionListener, SimilarImageInt
* @param uploadItem
*/
private void checkNearbyPlaces(UploadItem uploadItem) {
- Disposable checkNearbyPlaces = Observable.fromCallable(() -> repository
+ Disposable checkNearbyPlaces = Maybe.fromCallable(() -> repository
.checkNearbyPlaces(uploadItem.getGpsCoords().getDecLatitude(),
uploadItem.getGpsCoords().getDecLongitude()))
.subscribeOn(ioScheduler)
.observeOn(mainThreadScheduler)
- .subscribe(place -> view.onNearbyPlaceFound(uploadItem, place),
- throwable -> Timber.e(throwable, "Error occurred in processing images"));
- compositeDisposable.add(checkNearbyPlaces);
+ .subscribe(place -> {
+ if (place != null) {
+ view.onNearbyPlaceFound(uploadItem, place);
+ }
+ },
+ throwable -> Timber.e(throwable, "Error occurred in processing images"));
+ compositeDisposable.add(checkNearbyPlaces);
}
/**
diff --git a/app/src/main/java/fr/free/nrw/commons/upload/structure/depictions/DepictModel.kt b/app/src/main/java/fr/free/nrw/commons/upload/structure/depictions/DepictModel.kt
index 0d5604c5a..2e4283aed 100644
--- a/app/src/main/java/fr/free/nrw/commons/upload/structure/depictions/DepictModel.kt
+++ b/app/src/main/java/fr/free/nrw/commons/upload/structure/depictions/DepictModel.kt
@@ -2,7 +2,8 @@ package fr.free.nrw.commons.upload.structure.depictions
import fr.free.nrw.commons.nearby.Place
import fr.free.nrw.commons.upload.depicts.DepictsInterface
-import io.reactivex.Observable
+import io.reactivex.Flowable
+import io.reactivex.processors.BehaviorProcessor
import java.util.*
import javax.inject.Inject
import javax.inject.Singleton
@@ -13,33 +14,32 @@ import javax.inject.Singleton
@Singleton
class DepictModel @Inject constructor(private val depictsInterface: DepictsInterface) {
- var nearbyPlaces: MutableList? = null
+ var nearbyPlaces: BehaviorProcessor> = BehaviorProcessor.createDefault(emptyList())
- companion object {
- private const val SEARCH_DEPICTS_LIMIT = 25
- }
-
- /**
- * Search for depictions
- */
- fun searchAllEntities(query: String): Observable {
- if(query.isBlank()){
- return Observable.fromIterable(nearbyPlaces?.map { DepictedItem(it) } ?: emptyList())
+ companion object {
+ private const val SEARCH_DEPICTS_LIMIT = 25
}
- return networkItems(query)
- }
- private fun networkItems(query: String): Observable {
- val language = Locale.getDefault().language
- return depictsInterface.searchForDepicts(
- query, "$SEARCH_DEPICTS_LIMIT", language, language, "0"
- )
- .flatMap { Observable.fromIterable(it.search) }
- .map(::DepictedItem)
- }
+ /**
+ * Search for depictions
+ */
+ fun searchAllEntities(query: String): Flowable> {
+ if (query.isBlank()) {
+ return nearbyPlaces.map { it.map(::DepictedItem) }
+ }
+ return networkItems(query)
+ }
+
+ private fun networkItems(query: String): Flowable> {
+ val language = Locale.getDefault().language
+ return depictsInterface
+ .searchForDepicts(query, "$SEARCH_DEPICTS_LIMIT", language, language, "0")
+ .map { it.search.map(::DepictedItem) }
+ .toFlowable()
+ }
fun cleanUp() {
- nearbyPlaces = null
+ nearbyPlaces = BehaviorProcessor.createDefault(emptyList())
}
}
diff --git a/app/src/main/java/fr/free/nrw/commons/upload/structure/depictions/UploadDepictsCallback.java b/app/src/main/java/fr/free/nrw/commons/upload/structure/depictions/UploadDepictsCallback.java
index 35ea4992a..dc0361630 100644
--- a/app/src/main/java/fr/free/nrw/commons/upload/structure/depictions/UploadDepictsCallback.java
+++ b/app/src/main/java/fr/free/nrw/commons/upload/structure/depictions/UploadDepictsCallback.java
@@ -6,5 +6,5 @@ package fr.free.nrw.commons.upload.structure.depictions;
public interface UploadDepictsCallback {
void depictsClicked(DepictedItem item);
- void fetchThumbnailUrlForEntity(String entityId,int position);
+ void fetchThumbnailUrlForEntity(DepictedItem depictedItem);
}
diff --git a/app/src/main/res/layout/layout_upload_depicts_item.xml b/app/src/main/res/layout/layout_upload_depicts_item.xml
index 723c21ca5..9a0549236 100644
--- a/app/src/main/res/layout/layout_upload_depicts_item.xml
+++ b/app/src/main/res/layout/layout_upload_depicts_item.xml
@@ -17,14 +17,14 @@
app:layout_constraintLeft_toLeftOf="parent"
app:layout_constraintTop_toTopOf="parent" />
-
+ app:placeholderImage="@drawable/ic_wikidata_logo_24dp"/>
-
\ No newline at end of file
+
diff --git a/app/src/test/kotlin/fr/free/nrw/commons/upload/DepictsPresenterTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/upload/DepictsPresenterTest.kt
index 8ee2fa65e..c0484bec6 100644
--- a/app/src/test/kotlin/fr/free/nrw/commons/upload/DepictsPresenterTest.kt
+++ b/app/src/test/kotlin/fr/free/nrw/commons/upload/DepictsPresenterTest.kt
@@ -1,22 +1,32 @@
package fr.free.nrw.commons.upload
+import androidx.arch.core.executor.testing.InstantTaskExecutorRule
+import com.jraska.livedata.test
+import com.nhaarman.mockitokotlin2.never
+import com.nhaarman.mockitokotlin2.times
+import com.nhaarman.mockitokotlin2.verify
import com.nhaarman.mockitokotlin2.whenever
-import fr.free.nrw.commons.category.CategoryItem
+import fr.free.nrw.commons.explore.depictions.DepictsClient
+import fr.free.nrw.commons.explore.depictions.DepictsClient.NO_DEPICTED_IMAGE
import fr.free.nrw.commons.repository.UploadRepository
import fr.free.nrw.commons.upload.depicts.DepictsContract
-import fr.free.nrw.commons.upload.depicts.DepictsFragment
import fr.free.nrw.commons.upload.depicts.DepictsPresenter
import fr.free.nrw.commons.upload.structure.depictions.DepictedItem
-import io.reactivex.Observable
+import io.reactivex.Flowable
+import io.reactivex.Single
import io.reactivex.schedulers.TestScheduler
import org.junit.Before
+import org.junit.Rule
import org.junit.Test
-import org.mockito.ArgumentMatchers
import org.mockito.Mock
-import org.mockito.Mockito.verify
import org.mockito.MockitoAnnotations
+
class DepictsPresenterTest {
+
+ @get:Rule
+ var testRule = InstantTaskExecutorRule()
+
@Mock
internal lateinit var repository: UploadRepository
@@ -25,15 +35,10 @@ class DepictsPresenterTest {
private lateinit var depictsPresenter: DepictsPresenter
- private lateinit var depictsFragment: DepictsFragment
-
private lateinit var testScheduler: TestScheduler
- private val depictedItems: ArrayList = ArrayList()
-
@Mock
- lateinit var depictedItem: DepictedItem
-
+ lateinit var depictsClient: DepictsClient
/**
* initial setup
@@ -43,79 +48,143 @@ class DepictsPresenterTest {
fun setUp() {
MockitoAnnotations.initMocks(this)
testScheduler = TestScheduler()
- depictedItem = DepictedItem("label", "desc", "", false, "entityId")
- depictedItems.add(depictedItem)
- depictsPresenter = DepictsPresenter(repository, testScheduler, testScheduler, null)
- depictsFragment = DepictsFragment()
+ depictsPresenter = DepictsPresenter(repository, testScheduler, testScheduler, depictsClient)
depictsPresenter.onAttachView(view)
}
@Test
- fun searchEnglishDepictionsTest() {
- whenever(repository.sortBySimilarity(ArgumentMatchers.anyString())).thenReturn(Comparator { _, _ -> 1 })
- whenever(repository.selectedDepictions).thenReturn(depictedItems)
- whenever(repository.searchAllEntities(ArgumentMatchers.anyString())).thenReturn(Observable.empty())
- depictsPresenter.searchForDepictions("test")
- verify(view).showProgress(true)
- verify(view).setDepictsList(null)
+ fun `Search emission shows view progress`() {
+ depictsPresenter.searchForDepictions("")
testScheduler.triggerActions()
verify(view).showProgress(false)
}
@Test
- fun searchOtherLanguageDepictions() {
- whenever(repository.sortBySimilarity(ArgumentMatchers.anyString())).thenReturn(Comparator { _, _ -> 1 })
- whenever(repository.selectedDepictions).thenReturn(depictedItems)
- whenever(repository.searchAllEntities(ArgumentMatchers.anyString())).thenReturn(Observable.empty())
- depictsPresenter.searchForDepictions("वी")
- verify(view).showProgress(true)
- verify(view).setDepictsList(null)
+ fun `search results emission returns distinct results + selected items`() {
+ val searchResults = listOf(depictedItem(), depictedItem())
+ whenever(repository.searchAllEntities("")).thenReturn(Flowable.just(searchResults))
+ val selectedItem = depictedItem(id = "selected")
+ whenever(repository.selectedDepictions).thenReturn(listOf(selectedItem))
+ depictsPresenter.searchForDepictions("")
testScheduler.triggerActions()
verify(view).showProgress(false)
+ verify(view).showError(false)
+ depictsPresenter.depictedItems
+ .test()
+ .assertValue(listOf(selectedItem, depictedItem()))
}
@Test
- fun searchForNonExistingDepictions() {
- whenever(repository.sortBySimilarity(ArgumentMatchers.anyString())).thenReturn(Comparator { _, _ -> 1 })
- whenever(repository.selectedDepictions).thenReturn(depictedItems)
- whenever(repository.searchAllEntities(ArgumentMatchers.anyString())).thenReturn(Observable.empty())
- depictsPresenter.searchForDepictions("******")
- verify(view).showProgress(true)
- verify(view).setDepictsList(null)
+ fun `searchResults retrieve imageUrls from cache`() {
+ val depictedItem = depictedItem()
+ whenever(depictsClient.getP18ForItem(depictedItem.id)).thenReturn(Single.just("url"))
+ depictsPresenter.fetchThumbnailForEntityId(depictedItem)
+ testScheduler.triggerActions()
+ val searchResults = listOf(depictedItem(), depictedItem())
+ whenever(repository.searchAllEntities("")).thenReturn(Flowable.just(searchResults))
+ depictsPresenter.searchForDepictions("")
+ testScheduler.triggerActions()
+ depictsPresenter.depictedItems
+ .test()
+ .assertValue(listOf(depictedItem(imageUrl = "url")))
+ }
+
+ @Test
+ fun `empty search results with empty term do not show error`() {
+ whenever(repository.searchAllEntities("")).thenReturn(Flowable.just(emptyList()))
+ depictsPresenter.searchForDepictions("")
testScheduler.triggerActions()
- verify(view).setDepictsList(null)
verify(view).showProgress(false)
+ verify(view).showError(false)
+ depictsPresenter.depictedItems
+ .test()
+ .assertValue(emptyList())
}
@Test
- fun setSingleDepiction() {
- whenever(repository.sortBySimilarity(ArgumentMatchers.anyString())).thenReturn(Comparator { _, _ -> 1 })
- whenever(repository.selectedDepictions).thenReturn(depictedItems)
- whenever(repository.searchAllEntities(ArgumentMatchers.anyString())).thenReturn(Observable.empty())
- depictsPresenter.onDepictItemClicked(depictedItem)
- depictsPresenter.verifyDepictions()
- verify(view).goToNextScreen()
- }
-
- @Test
- fun setMultipleDepictions() {
- whenever(repository.sortBySimilarity(ArgumentMatchers.anyString())).thenReturn(Comparator { _, _ -> 1 })
- whenever(repository.selectedDepictions).thenReturn(depictedItems)
- whenever(repository.searchAllEntities(ArgumentMatchers.anyString())).thenReturn(Observable.empty())
- depictsPresenter.onDepictItemClicked(depictedItem)
- val depictedItem2 = DepictedItem("label2", "desc2", "", false, "entityid2")
- depictsPresenter.onDepictItemClicked(depictedItem2)
- depictsPresenter.verifyDepictions()
- verify(view).goToNextScreen()
- }
-
- @Test
- fun `on Search Exception Show Error And Stop Progress`() {
- whenever(repository.searchAllEntities(ArgumentMatchers.anyString()))
- .thenReturn(Observable.error(Exception()))
- depictsPresenter.searchForDepictions("******")
+ fun `empty search results with non empty term do show error`() {
+ whenever(repository.searchAllEntities("a")).thenReturn(Flowable.just(emptyList()))
+ depictsPresenter.searchForDepictions("a")
testScheduler.triggerActions()
+ verify(view).showProgress(false)
verify(view).showError(true)
+ depictsPresenter.depictedItems
+ .test()
+ .assertValue(emptyList())
+ }
+
+ @Test
+ fun `search error shows error`() {
+ whenever(repository.searchAllEntities("")).thenReturn(Flowable.error(Exception()))
+ depictsPresenter.searchForDepictions("")
+ testScheduler.triggerActions()
verify(view).showProgress(false)
+ verify(view).showError(true)
+ }
+
+ @Test
+ fun `onPreviousButtonClicked goes to previous screen`() {
+ depictsPresenter.onPreviousButtonClicked()
+ verify(view).goToPreviousScreen()
+ }
+
+ @Test
+ fun `onDepictItemClicked calls repository`() {
+ val depictedItem = depictedItem()
+ depictsPresenter.onDepictItemClicked(depictedItem)
+ verify(repository).onDepictItemClicked(depictedItem)
+ }
+
+ @Test
+ fun `verifyDepictions with non empty selectedDepictions goes to next screen`() {
+ whenever(repository.selectedDepictions).thenReturn(listOf(depictedItem()))
+ depictsPresenter.verifyDepictions()
+ verify(view).goToNextScreen()
+ }
+
+ @Test
+ fun `verifyDepictions with empty selectedDepictions goes to noDepictionSelected`() {
+ whenever(repository.selectedDepictions).thenReturn(emptyList())
+ depictsPresenter.verifyDepictions()
+ verify(view).noDepictionSelected()
+ }
+
+ @Test
+ fun `image urls fetched from network update the view`() {
+ val depictedItem = depictedItem()
+ whenever(depictsClient.getP18ForItem(depictedItem.id)).thenReturn(Single.just("url"))
+ depictsPresenter.fetchThumbnailForEntityId(depictedItem)
+ testScheduler.triggerActions()
+ verify(view).onUrlFetched(depictedItem, "url")
+ }
+
+ @Test
+ fun `image urls fetched from network filter NO_DEPICTED_IMAGE`() {
+ val depictedItem = depictedItem()
+ whenever(depictsClient.getP18ForItem(depictedItem.id))
+ .thenReturn(Single.just(NO_DEPICTED_IMAGE))
+ depictsPresenter.fetchThumbnailForEntityId(depictedItem)
+ testScheduler.triggerActions()
+ verify(view, never()).onUrlFetched(depictedItem, NO_DEPICTED_IMAGE)
+ }
+
+ @Test
+ fun `successive image urls fetched from cache`() {
+ val depictedItem = depictedItem()
+ whenever(depictsClient.getP18ForItem(depictedItem.id)).thenReturn(Single.just("url"))
+ depictsPresenter.fetchThumbnailForEntityId(depictedItem)
+ testScheduler.triggerActions()
+ verify(view).onUrlFetched(depictedItem, "url")
+ depictsPresenter.fetchThumbnailForEntityId(depictedItem)
+ testScheduler.triggerActions()
+ verify(view, times(2)).onUrlFetched(depictedItem, "url")
}
}
+
+fun depictedItem(
+ name: String = "label",
+ description: String = "desc",
+ imageUrl: String = "",
+ isSelected: Boolean = false,
+ id: String = "entityId"
+) = DepictedItem(name, description, imageUrl, isSelected, id)