Skip to content
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

Fixes #5743 Reduce delay between peer reviews #5897

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
254 changes: 227 additions & 27 deletions app/src/main/java/fr/free/nrw/commons/review/ReviewActivity.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand All @@ -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;
u7805486 marked this conversation as resolved.
Show resolved Hide resolved
// 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);
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any way to improve these comments rather than just remove them?

Choose a reason for hiding this comment

The 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 -> {
Expand Down Expand Up @@ -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));
}

/**
Expand All @@ -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;
Expand All @@ -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);
}

Expand Down Expand Up @@ -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
Expand Down
Loading