Skip to content

Commit 6a2a456

Browse files
macgillsmacgills
committed
#3493 App freezes for 15 seconds when you press Next in UploadMediaDetailsFragment - add apropriate schedulers and convert justs to fromCallable
1 parent d925e32 commit 6a2a456

14 files changed

+124
-157
lines changed

‎app/src/main/java/fr/free/nrw/commons/repository/UploadRemoteDataSource.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -179,11 +179,10 @@ public Observable<UploadItem> preProcessImage(UploadableFile uploadableFile, Pla
179179
* ask the UplaodModel for the image quality of the UploadItem
180180
*
181181
* @param uploadItem
182-
* @param shouldValidateTitle
183182
* @return
184183
*/
185-
public Single<Integer> getImageQuality(UploadItem uploadItem, boolean shouldValidateTitle) {
186-
return uploadModel.getImageQuality(uploadItem, shouldValidateTitle);
184+
public Single<Integer> getImageQuality(UploadItem uploadItem) {
185+
return uploadModel.getImageQuality(uploadItem);
187186
}
188187

189188
/**

‎app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -187,11 +187,10 @@ public Observable<UploadItem> preProcessImage(UploadableFile uploadableFile, Pla
187187
* query the RemoteDataSource for image quality
188188
*
189189
* @param uploadItem
190-
* @param shouldValidateTitle
191190
* @return
192191
*/
193-
public Single<Integer> getImageQuality(UploadItem uploadItem, boolean shouldValidateTitle) {
194-
return remoteDataSource.getImageQuality(uploadItem, shouldValidateTitle);
192+
public Single<Integer> getImageQuality(UploadItem uploadItem) {
193+
return remoteDataSource.getImageQuality(uploadItem);
195194
}
196195

197196
/**

‎app/src/main/java/fr/free/nrw/commons/upload/ImageProcessingService.java

Lines changed: 41 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,21 @@
11
package fr.free.nrw.commons.upload;
22

3-
import android.content.Context;
4-
5-
import org.apache.commons.lang3.StringUtils;
6-
7-
import javax.inject.Inject;
8-
import javax.inject.Singleton;
3+
import static fr.free.nrw.commons.utils.ImageUtils.EMPTY_TITLE;
4+
import static fr.free.nrw.commons.utils.ImageUtils.FILE_NAME_EXISTS;
5+
import static fr.free.nrw.commons.utils.ImageUtils.IMAGE_OK;
96

7+
import android.content.Context;
108
import fr.free.nrw.commons.media.MediaClient;
119
import fr.free.nrw.commons.nearby.Place;
1210
import fr.free.nrw.commons.utils.ImageUtils;
1311
import fr.free.nrw.commons.utils.ImageUtilsWrapper;
1412
import io.reactivex.Single;
13+
import io.reactivex.schedulers.Schedulers;
14+
import javax.inject.Inject;
15+
import javax.inject.Singleton;
16+
import org.apache.commons.lang3.StringUtils;
1517
import timber.log.Timber;
1618

17-
import static fr.free.nrw.commons.utils.ImageUtils.EMPTY_TITLE;
18-
import static fr.free.nrw.commons.utils.ImageUtils.FILE_NAME_EXISTS;
19-
import static fr.free.nrw.commons.utils.ImageUtils.IMAGE_OK;
20-
2119
/**
2220
* Methods for pre-processing images to be uploaded
2321
*/
@@ -41,38 +39,33 @@ public ImageProcessingService(FileUtilsWrapper fileUtilsWrapper,
4139
this.mediaClient = mediaClient;
4240
}
4341

44-
/**
45-
* Check image quality before upload
46-
* - checks duplicate image
47-
* - checks dark image
48-
* - checks geolocation for image
49-
* - check for valid title
50-
*/
51-
Single<Integer> validateImage(UploadModel.UploadItem uploadItem, boolean checkTitle) {
52-
int currentImageQuality = uploadItem.getImageQuality();
53-
Timber.d("Current image quality is %d", currentImageQuality);
54-
if (currentImageQuality == ImageUtils.IMAGE_KEEP) {
55-
return Single.just(ImageUtils.IMAGE_OK);
56-
}
57-
Timber.d("Checking the validity of image");
58-
String filePath = uploadItem.getMediaUri().getPath();
59-
Single<Integer> duplicateImage = checkDuplicateImage(filePath);
60-
Single<Integer> wrongGeoLocation = checkImageGeoLocation(uploadItem.getPlace(), filePath);
61-
Single<Integer> darkImage = checkDarkImage(filePath);
62-
Single<Integer> itemTitle = checkTitle ? validateItemTitle(uploadItem) : Single.just(ImageUtils.IMAGE_OK);
63-
Single<Integer> checkFBMD = checkFBMD(filePath);
64-
Single<Integer> checkEXIF = checkEXIF(filePath);
65-
66-
Single<Integer> zipResult = Single.zip(duplicateImage, wrongGeoLocation, darkImage, itemTitle,
67-
(duplicate, wrongGeo, dark, title) -> {
68-
Timber.d("Result for duplicate: %d, geo: %d, dark: %d, title: %d", duplicate, wrongGeo, dark, title);
69-
return duplicate | wrongGeo | dark | title;
70-
});
71-
return Single.zip(zipResult, checkFBMD , checkEXIF , (zip , fbmd , exif)->{
72-
Timber.d("zip:" + zip + "fbmd:" + fbmd + "exif:" + exif);
73-
return zip | fbmd | exif;
74-
});
42+
/**
43+
* Check image quality before upload - checks duplicate image - checks dark image - checks
44+
* geolocation for image - check for valid title
45+
*/
46+
Single<Integer> validateImage(UploadModel.UploadItem uploadItem) {
47+
int currentImageQuality = uploadItem.getImageQuality();
48+
Timber.d("Current image quality is %d", currentImageQuality);
49+
if (currentImageQuality == ImageUtils.IMAGE_KEEP) {
50+
return Single.just(ImageUtils.IMAGE_OK);
7551
}
52+
Timber.d("Checking the validity of image");
53+
String filePath = uploadItem.getMediaUri().getPath();
54+
55+
return Single.zip(
56+
checkDuplicateImage(filePath),
57+
checkImageGeoLocation(uploadItem.getPlace(), filePath),
58+
checkDarkImage(filePath),
59+
validateItemTitle(uploadItem),
60+
checkFBMD(filePath),
61+
checkEXIF(filePath),
62+
(duplicateImage, wrongGeoLocation, darkImage, itemTitle, fbmd, exif) -> {
63+
Timber.d("duplicate: %d, geo: %d, dark: %d, title: %d" + "fbmd:" + fbmd + "exif:" + exif,
64+
duplicateImage, wrongGeoLocation, darkImage, itemTitle);
65+
return duplicateImage | wrongGeoLocation | darkImage | itemTitle | fbmd | exif;
66+
}
67+
);
68+
}
7669

7770
/**
7871
* We want to discourage users from uploading images to Commons that were taken from Facebook.
@@ -113,7 +106,8 @@ private Single<Integer> validateItemTitle(UploadModel.UploadItem uploadItem) {
113106
.map(doesFileExist -> {
114107
Timber.d("Result for valid title is %s", doesFileExist);
115108
return doesFileExist ? FILE_NAME_EXISTS : IMAGE_OK;
116-
});
109+
})
110+
.subscribeOn(Schedulers.io());
117111
}
118112

119113
/**
@@ -124,14 +118,14 @@ private Single<Integer> validateItemTitle(UploadModel.UploadItem uploadItem) {
124118
*/
125119
private Single<Integer> checkDuplicateImage(String filePath) {
126120
Timber.d("Checking for duplicate image %s", filePath);
127-
return Single.fromCallable(() ->
128-
fileUtilsWrapper.getFileInputStream(filePath))
121+
return Single.fromCallable(() -> fileUtilsWrapper.getFileInputStream(filePath))
129122
.map(fileUtilsWrapper::getSHA1)
130123
.flatMap(mediaClient::checkFileExistsUsingSha)
131124
.map(b -> {
132125
Timber.d("Result for duplicate image %s", b);
133126
return b ? ImageUtils.IMAGE_DUPLICATE : ImageUtils.IMAGE_OK;
134-
});
127+
})
128+
.subscribeOn(Schedulers.io());
135129
}
136130

137131
/**
@@ -164,7 +158,8 @@ private Single<Integer> checkImageGeoLocation(Place place, String filePath) {
164158
return Single.just(ImageUtils.IMAGE_OK);
165159
}
166160
return imageUtilsWrapper.checkImageGeolocationIsDifferent(geoLocation, place.getLocation());
167-
});
161+
})
162+
.subscribeOn(Schedulers.io());
168163
}
169164
}
170165

‎app/src/main/java/fr/free/nrw/commons/upload/ReadFBMD.java

Lines changed: 0 additions & 48 deletions
This file was deleted.

‎app/src/main/java/fr/free/nrw/commons/upload/ReadFBMD.kt

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package fr.free.nrw.commons.upload
2+
3+
import fr.free.nrw.commons.utils.ImageUtils
4+
import io.reactivex.Single
5+
import io.reactivex.schedulers.Schedulers
6+
import java.io.FileInputStream
7+
import java.io.IOException
8+
import javax.inject.Inject
9+
import javax.inject.Singleton
10+
11+
/**
12+
* We want to discourage users from uploading images to Commons that were taken from Facebook. This
13+
* attempts to detect whether an image was downloaded from Facebook by heuristically searching for
14+
* metadata that is specific to images that come from Facebook.
15+
*/
16+
@Singleton
17+
class ReadFBMD @Inject constructor() {
18+
fun processMetadata(path: String?) = Single.fromCallable {
19+
try {
20+
val fileStr = FileInputStream(path).use {
21+
ByteArray(4096).apply { it.read(this) }.toString()
22+
}
23+
if (isFbmd(fileStr.indexOf("8BIM"), fileStr.indexOf("FBMD")))
24+
return@fromCallable ImageUtils.FILE_FBMD
25+
} catch (e: IOException) {
26+
e.printStackTrace()
27+
}
28+
ImageUtils.IMAGE_OK
29+
}.subscribeOn(Schedulers.io())
30+
31+
private fun isFbmd(psBlockOffset: Int, fbmdOffset: Int) =
32+
psBlockOffset > 0 && fbmdOffset > 0 &&
33+
fbmdOffset > psBlockOffset &&
34+
fbmdOffset - psBlockOffset < 0x80
35+
}

‎app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,8 @@ public Observable<UploadItem> preProcessImage(UploadableFile uploadableFile,
119119
return Observable.just(getUploadItem(uploadableFile, place, source, similarImageInterface));
120120
}
121121

122-
public Single<Integer> getImageQuality(UploadItem uploadItem, boolean checkTitle) {
123-
return imageProcessingService.validateImage(uploadItem, checkTitle);
122+
public Single<Integer> getImageQuality(UploadItem uploadItem) {
123+
return imageProcessingService.validateImage(uploadItem);
124124
}
125125

126126
private UploadItem getUploadItem(UploadableFile uploadableFile,
@@ -378,4 +378,4 @@ public int hashCode() {
378378
}
379379
}
380380

381-
}
381+
}

‎app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragment.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ private void showInfoAlert(int titleStringID, int messageStringId) {
251251
@OnClick(R.id.btn_next)
252252
public void onNextButtonClicked() {
253253
uploadItem.setDescriptions(descriptionsAdapter.getDescriptions());
254-
presenter.verifyImageQuality(uploadItem, true);
254+
presenter.verifyImageQuality(uploadItem);
255255
}
256256

257257
@OnClick(R.id.btn_previous)

‎app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailsContract.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ interface UserActionListener extends BasePresenter<View> {
4343
void receiveImage(UploadableFile uploadableFile, @Contribution.FileSource String source,
4444
Place place);
4545

46-
void verifyImageQuality(UploadItem uploadItem, boolean validateTitle);
46+
void verifyImageQuality(UploadItem uploadItem);
4747

4848
void setUploadItem(int index, UploadItem uploadItem);
4949

‎app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaPresenter.java

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -111,14 +111,14 @@ private void checkNearbyPlaces(UploadItem uploadItem) {
111111
* asks the repository to verify image quality
112112
*
113113
* @param uploadItem
114-
* @param validateTitle
115114
*/
116115
@Override
117-
public void verifyImageQuality(UploadItem uploadItem, boolean validateTitle) {
116+
public void verifyImageQuality(UploadItem uploadItem) {
118117
view.showProgress(true);
119-
Disposable imageQualityDisposable = repository
120-
.getImageQuality(uploadItem, true)
121-
.subscribeOn(ioScheduler)
118+
119+
compositeDisposable.add(
120+
repository
121+
.getImageQuality(uploadItem)
122122
.observeOn(mainThreadScheduler)
123123
.subscribe(imageResult -> {
124124
view.showProgress(false);
@@ -129,9 +129,8 @@ public void verifyImageQuality(UploadItem uploadItem, boolean validateTitle) {
129129
view.showMessage("" + throwable.getLocalizedMessage(),
130130
R.color.color_error);
131131
Timber.e(throwable, "Error occurred while handling image");
132-
});
133-
134-
compositeDisposable.add(imageQualityDisposable);
132+
})
133+
);
135134
}
136135

137136
/**

‎app/src/main/java/fr/free/nrw/commons/utils/ImageUtilsWrapper.java

Lines changed: 0 additions & 31 deletions
This file was deleted.

‎app/src/main/java/fr/free/nrw/commons/utils/ImageUtilsWrapper.kt

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package fr.free.nrw.commons.utils
2+
3+
import fr.free.nrw.commons.location.LatLng
4+
import io.reactivex.Single
5+
import io.reactivex.schedulers.Schedulers
6+
import javax.inject.Inject
7+
import javax.inject.Singleton
8+
9+
@Singleton
10+
class ImageUtilsWrapper @Inject constructor() {
11+
fun checkIfImageIsTooDark(bitmapPath: String?) =
12+
Single.fromCallable { ImageUtils.checkIfImageIsTooDark(bitmapPath) }
13+
.subscribeOn(Schedulers.computation())
14+
15+
fun checkImageGeolocationIsDifferent(geolocationOfFileString: String?, latLng: LatLng?) =
16+
Single.just(ImageUtils.checkImageGeolocationIsDifferent(geolocationOfFileString, latLng))
17+
.subscribeOn(Schedulers.computation())
18+
.map { isDifferent: Boolean -> if (isDifferent) ImageUtils.IMAGE_GEOLOCATION_DIFFERENT else ImageUtils.IMAGE_OK }
19+
}

0 commit comments

Comments
 (0)