-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fixes #5743 Reduce delay between peer reviews #5897
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
913d2e6
4521b5f
febed89
66f917b
55e59b8
54aeeb0
7ffa271
e617507
36dcc43
617957e
549d943
62a2a33
f8c092f
9dc81df
3a98976
d7c883d
8ec6c2f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
import android.annotation.SuppressLint; | ||
import android.content.Context; | ||
import android.content.Intent; | ||
import android.content.SharedPreferences; | ||
import android.graphics.PorterDuff; | ||
import android.graphics.drawable.Drawable; | ||
import android.os.Bundle; | ||
|
@@ -11,6 +12,7 @@ | |
import android.view.MenuItem; | ||
import android.view.MotionEvent; | ||
import android.view.View; | ||
import androidx.core.util.Pair; | ||
import androidx.fragment.app.Fragment; | ||
import androidx.fragment.app.FragmentManager; | ||
import fr.free.nrw.commons.Media; | ||
|
@@ -22,19 +24,25 @@ | |
import fr.free.nrw.commons.theme.BaseActivity; | ||
import fr.free.nrw.commons.utils.DialogUtil; | ||
import fr.free.nrw.commons.utils.ViewUtil; | ||
import io.reactivex.Observable; | ||
import io.reactivex.android.schedulers.AndroidSchedulers; | ||
import io.reactivex.disposables.CompositeDisposable; | ||
import io.reactivex.schedulers.Schedulers; | ||
import java.util.ArrayList; | ||
import java.util.List; | ||
import javax.inject.Inject; | ||
|
||
public class ReviewActivity extends BaseActivity { | ||
|
||
|
||
private ActivityReviewBinding binding; | ||
|
||
MediaDetailFragment mediaDetailFragment; | ||
public ReviewPagerAdapter reviewPagerAdapter; | ||
public ReviewController reviewController; | ||
|
||
|
||
|
||
|
||
@Inject | ||
ReviewHelper reviewHelper; | ||
@Inject | ||
|
@@ -53,6 +61,20 @@ public class ReviewActivity extends BaseActivity { | |
final String SAVED_MEDIA = "saved_media"; | ||
private Media media; | ||
|
||
private List<Media> cachedMedia = new ArrayList<>(); | ||
|
||
/** Constants for managing media cache in the review activity */ | ||
// Name of SharedPreferences file for storing review activity preferences | ||
private static final String PREF_NAME = "ReviewActivityPrefs"; | ||
// Key for storing the timestamp of last cache update | ||
private static final String LAST_CACHE_TIME_KEY = "lastCacheTime"; | ||
|
||
// Maximum number of media files to store in cache | ||
private static final int CACHE_SIZE = 5; | ||
// Cache expiration time in milliseconds (24 hours) | ||
|
||
private static final long CACHE_EXPIRY_TIME = 24 * 60 * 60 * 1000; | ||
|
||
@Override | ||
protected void onSaveInstanceState(Bundle outState) { | ||
super.onSaveInstanceState(outState); | ||
|
@@ -99,11 +121,22 @@ protected void onCreate(Bundle savedInstanceState) { | |
Drawable d[]=binding.skipImage.getCompoundDrawablesRelative(); | ||
d[2].setColorFilter(getApplicationContext().getResources().getColor(R.color.button_blue), PorterDuff.Mode.SRC_IN); | ||
|
||
/** | ||
* Restores the previous state of the activity or initializes a new review session. | ||
* Checks if there's a saved media state from a previous session (e.g., before screen rotation). | ||
* If found, restores the last viewed image and its detail view. | ||
* Otherwise, starts a new random image review session. | ||
* | ||
* @param savedInstanceState Bundle containing the activity's previously saved state, if any | ||
*/ | ||
if (savedInstanceState != null && savedInstanceState.getParcelable(SAVED_MEDIA) != null) { | ||
updateImage(savedInstanceState.getParcelable(SAVED_MEDIA)); // Use existing media if we have one | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any way to improve these comments rather than just remove them? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, I have added java doc to explain why I changed this part, but my teammates pushed a old version, sorry about that. the new version included java doc and kdoc for all changed part. |
||
// Restore the previously viewed image if state exists | ||
updateImage(savedInstanceState.getParcelable(SAVED_MEDIA)); | ||
// Restore media detail view (handles configuration changes like screen rotation) | ||
setUpMediaDetailOnOrientation(); | ||
} else { | ||
runRandomizer(); //Run randomizer whenever everything is ready so that a first random image will be added | ||
// Start fresh review session with a random image | ||
runRandomizer(); | ||
} | ||
|
||
binding.skipImage.setOnClickListener(view -> { | ||
|
@@ -131,36 +164,190 @@ public boolean onSupportNavigateUp() { | |
return true; | ||
} | ||
|
||
/** | ||
* Initiates the process of loading a random media file for review. | ||
* This method: | ||
* - Resets the UI state | ||
* - Shows loading indicator | ||
* - Manages media cache | ||
* - Either loads from cache or fetches new media | ||
* | ||
* The method is annotated with @SuppressLint("CheckResult") as the Observable | ||
* subscription is handled through CompositeDisposable in the implementation. | ||
* | ||
* @return true indicating successful initiation of the randomization process | ||
*/ | ||
@SuppressLint("CheckResult") | ||
public boolean runRandomizer() { | ||
// Reset flag for tracking presence of non-hidden categories | ||
hasNonHiddenCategories = false; | ||
// Display loading indicator while fetching media | ||
binding.pbReviewImage.setVisibility(View.VISIBLE); | ||
// Reset view pager to first page | ||
binding.viewPagerReview.setCurrentItem(0); | ||
// Finds non-hidden categories from Media instance | ||
compositeDisposable.add(reviewHelper.getRandomMedia() | ||
|
||
// Check cache status and determine source of next media | ||
if (cachedMedia.isEmpty() || isCacheExpired()) { | ||
// Fetch and cache new media if cache is empty or expired | ||
fetchAndCacheMedia(); | ||
} else { | ||
// Use next media file from existing cache | ||
processNextCachedMedia(); | ||
} | ||
return true; | ||
} | ||
|
||
/** | ||
* Batch checks whether multiple files from the cache are used in wikis. | ||
* This is a more efficient way to process multiple files compared to checking them one by one. | ||
* | ||
* @param mediaList List of Media objects to check for usage | ||
*/ | ||
/** | ||
* Batch checks whether multiple files from the cache are used in wikis. | ||
* This is a more efficient way to process multiple files compared to checking them one by one. | ||
* | ||
* @param mediaList List of Media objects to check for usage | ||
*/ | ||
private void batchCheckFilesUsage(List<Media> mediaList) { | ||
// Extract filenames from media objects | ||
List<String> filenames = new ArrayList<>(); | ||
for (Media media : mediaList) { | ||
if (media.getFilename() != null) { | ||
filenames.add(media.getFilename()); | ||
} | ||
} | ||
|
||
compositeDisposable.add( | ||
reviewHelper.checkFileUsageBatch(filenames) | ||
.subscribeOn(Schedulers.io()) | ||
.observeOn(AndroidSchedulers.mainThread()) | ||
.subscribe(this::checkWhetherFileIsUsedInWikis)); | ||
return true; | ||
.toList() | ||
.subscribe(results -> { | ||
// Process each result | ||
for (kotlin.Pair<String, Boolean> result : results) { | ||
String filename = result.getFirst(); | ||
Boolean isUsed = result.getSecond(); | ||
|
||
// Find corresponding media object | ||
for (Media media : mediaList) { | ||
if (filename.equals(media.getFilename())) { | ||
if (!isUsed) { | ||
// If file is not used, proceed with category check | ||
findNonHiddenCategories(media); | ||
} | ||
break; | ||
} | ||
} | ||
} | ||
}, this::handleError)); | ||
} | ||
|
||
|
||
/** | ||
* Fetches and caches new media files for review. | ||
* Uses RxJava to: | ||
* - Generate a range of indices for the desired cache size | ||
* - Fetch random media files asynchronously | ||
* - Handle thread scheduling between IO and UI operations | ||
* - Store the fetched media in cache | ||
* | ||
* The operation is added to compositeDisposable for proper lifecycle management. | ||
*/ | ||
private void fetchAndCacheMedia() { | ||
u7805486 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
compositeDisposable.add( | ||
Observable.range(0, CACHE_SIZE) | ||
.flatMap(i -> reviewHelper.getRandomMedia().toObservable()) | ||
.subscribeOn(Schedulers.io()) | ||
.observeOn(AndroidSchedulers.mainThread()) | ||
.toList() | ||
.subscribe(mediaList -> { | ||
// Clear existing cache | ||
cachedMedia.clear(); | ||
|
||
// Start batch check process | ||
batchCheckFilesUsage(mediaList); | ||
|
||
// Update cache with new media | ||
cachedMedia.addAll(mediaList); | ||
updateLastCacheTime(); | ||
|
||
// Process first media item if available | ||
if (!cachedMedia.isEmpty()) { | ||
processNextCachedMedia(); | ||
} | ||
}, this::handleError)); | ||
} | ||
|
||
/** | ||
* Processes the next media file from the cache. | ||
* If cache is not empty, removes and processes the first media file. | ||
* If cache is empty, triggers a new fetch operation. | ||
* | ||
* This method ensures continuous flow of media files for review | ||
* while maintaining the cache mechanism. | ||
*/ | ||
private void processNextCachedMedia() { | ||
if (!cachedMedia.isEmpty()) { | ||
// Remove and get the first media from cache | ||
Media media = cachedMedia.remove(0); | ||
|
||
checkWhetherFileIsUsedInWikis(media); | ||
} else { | ||
// Refill cache if empty | ||
fetchAndCacheMedia(); | ||
} | ||
} | ||
|
||
/** | ||
* Checks if the current cache has expired. | ||
* Cache expiration is determined by comparing the last cache time | ||
* with the current time against the configured expiry duration. | ||
* | ||
* @return true if cache has expired, false otherwise | ||
*/ | ||
private boolean isCacheExpired() { | ||
// Get shared preferences instance | ||
SharedPreferences prefs = getSharedPreferences(PREF_NAME, Context.MODE_PRIVATE); | ||
|
||
long lastCacheTime = prefs.getLong(LAST_CACHE_TIME_KEY, 0); | ||
|
||
long currentTime = System.currentTimeMillis(); | ||
|
||
return (currentTime - lastCacheTime) > CACHE_EXPIRY_TIME; | ||
} | ||
|
||
|
||
/** | ||
* Updates the timestamp of the last cache operation. | ||
* Stores the current time in SharedPreferences to track | ||
* cache freshness for future operations. | ||
*/ | ||
private void updateLastCacheTime() { | ||
|
||
SharedPreferences prefs = getSharedPreferences(PREF_NAME, Context.MODE_PRIVATE); | ||
|
||
SharedPreferences.Editor editor = prefs.edit(); | ||
// Store current timestamp as last cache time | ||
editor.putLong(LAST_CACHE_TIME_KEY, System.currentTimeMillis()); | ||
// Apply changes asynchronously | ||
editor.apply(); | ||
} | ||
|
||
/** | ||
* Check whether media is used or not in any Wiki Page | ||
*/ | ||
@SuppressLint("CheckResult") | ||
private void checkWhetherFileIsUsedInWikis(final Media media) { | ||
compositeDisposable.add(reviewHelper.checkFileUsage(media.getFilename()) | ||
.subscribeOn(Schedulers.io()) | ||
.observeOn(AndroidSchedulers.mainThread()) | ||
.subscribe(result -> { | ||
// result false indicates media is not used in any wiki | ||
if (!result) { | ||
// Finds non-hidden categories from Media instance | ||
findNonHiddenCategories(media); | ||
} else { | ||
runRandomizer(); | ||
processNextCachedMedia(); | ||
} | ||
})); | ||
}, this::handleError)); | ||
} | ||
|
||
/** | ||
|
@@ -181,7 +368,6 @@ private void findNonHiddenCategories(Media media) { | |
updateImage(media); | ||
} | ||
|
||
@SuppressLint("CheckResult") | ||
private void updateImage(Media media) { | ||
reviewHelper.addViewedImagesToDB(media.getPageId()); | ||
this.media = media; | ||
|
@@ -191,27 +377,26 @@ private void updateImage(Media media) { | |
return; | ||
} | ||
|
||
//If The Media User and Current Session Username is same then Skip the Image | ||
if (media.getUser() != null && media.getUser().equals(AccountUtil.getUserName(getApplicationContext()))) { | ||
runRandomizer(); | ||
processNextCachedMedia(); | ||
return; | ||
} | ||
|
||
binding.reviewImageView.setImageURI(media.getImageUrl()); | ||
|
||
reviewController.onImageRefreshed(media); //file name is updated | ||
reviewController.onImageRefreshed(media); | ||
compositeDisposable.add(reviewHelper.getFirstRevisionOfFile(fileName) | ||
.subscribeOn(Schedulers.io()) | ||
.observeOn(AndroidSchedulers.mainThread()) | ||
.subscribe(revision -> { | ||
reviewController.firstRevision = revision; | ||
reviewPagerAdapter.updateFileInformation(); | ||
@SuppressLint({"StringFormatInvalid", "LocalSuppress"}) String caption = String.format(getString(R.string.review_is_uploaded_by), fileName, revision.getUser()); | ||
binding.tvImageCaption.setText(caption); | ||
binding.pbReviewImage.setVisibility(View.GONE); | ||
reviewImageFragment = getInstanceOfReviewImageFragment(); | ||
reviewImageFragment.enableButtons(); | ||
})); | ||
.subscribeOn(Schedulers.io()) | ||
.observeOn(AndroidSchedulers.mainThread()) | ||
.subscribe(revision -> { | ||
reviewController.firstRevision = revision; | ||
reviewPagerAdapter.updateFileInformation(); | ||
String caption = String.format(getString(R.string.review_is_uploaded_by), fileName, revision.getUser()); | ||
binding.tvImageCaption.setText(caption); | ||
binding.pbReviewImage.setVisibility(View.GONE); | ||
reviewImageFragment = getInstanceOfReviewImageFragment(); | ||
reviewImageFragment.enableButtons(); | ||
}, this::handleError)); | ||
binding.viewPagerReview.setCurrentItem(0); | ||
} | ||
|
||
|
@@ -258,6 +443,21 @@ public void showReviewImageInfo() { | |
null, | ||
null); | ||
} | ||
/** | ||
* Handles errors that occur during media processing operations. | ||
* This is a generic error handler that: | ||
* - Hides the loading indicator | ||
* - Shows a user-friendly error message via Snackbar | ||
* | ||
* Used as error callback for RxJava operations and other async tasks. | ||
* | ||
* @param error The Throwable that was caught during operation | ||
*/ | ||
private void handleError(Throwable error) { | ||
binding.pbReviewImage.setVisibility(View.GONE); | ||
// Show error message to user via Snackbar | ||
ViewUtil.showShortSnackbar(binding.drawerLayout, R.string.error_review); | ||
} | ||
|
||
|
||
@Override | ||
|
Uh oh!
There was an error while loading. Please reload this page.