Skip to content

Times called #7

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

Merged
merged 3 commits into from
Oct 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
172 changes: 160 additions & 12 deletions app/src/main/java/fr/free/nrw/commons/review/ReviewActivity.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,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 Down Expand Up @@ -62,9 +63,16 @@ public class ReviewActivity extends BaseActivity {

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
Expand Down Expand Up @@ -113,10 +121,21 @@ 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) {
// 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 {
// Start fresh review session with a random image
runRandomizer();
}

Expand Down Expand Up @@ -145,54 +164,173 @@ 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);

// 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())
.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() {
compositeDisposable.add(Observable.range(0, CACHE_SIZE)
.flatMap(i -> reviewHelper.getRandomMedia().toObservable())
.subscribeOn(Schedulers.io())
.observeOn(AndroidSchedulers.mainThread())
.toList()
.subscribe(mediaList -> {
cachedMedia.clear();
cachedMedia.addAll(mediaList);
updateLastCacheTime();
processNextCachedMedia();
}, this::handleError));
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();
}

Expand Down Expand Up @@ -305,10 +443,20 @@ 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);

}


Expand Down
26 changes: 19 additions & 7 deletions app/src/main/java/fr/free/nrw/commons/review/ReviewHelper.kt
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,11 @@ class ReviewHelper


/**
* Gets multiple random media items for review.
* - Fetches recent changes and filters them
* - Checks if files are nominated for deletion
* - Filters out already reviewed images
* Fetches recent changes from MediaWiki API
* Calls the API to get the latest 50 changes
* When more results are available, the query gets continued beyond this range
*
* @param count Number of media items to fetch
* @return Observable of Media items
* @return
*/
private fun getRecentChanges() =
reviewInterface
Expand Down Expand Up @@ -135,14 +133,28 @@ class ReviewHelper




/**
* Batch checks whether multiple files are being used in any wiki pages.
* This method processes a list of filenames in parallel using RxJava Observables.
*
* @param filenames A list of filenames to check for usage
* @return Observable emitting pairs of filename and usage status:
* - The String represents the filename
* - The Boolean indicates whether the file is used (true) or not (false)
* If an error occurs during processing, it will log the error and emit an empty Observable
*/
fun checkFileUsageBatch(filenames: List<String>): Observable<Pair<String, Boolean>> =
// Convert the list of filenames into an Observable stream
Observable.fromIterable(filenames)
// For each filename, check its usage and pair it with the result
.flatMap { filename ->
checkFileUsage(filename)
// Create a pair of the filename and its usage status
.map { isUsed -> Pair(filename, isUsed) }
}
// Handle any errors that occur during processing
.onErrorResumeNext { error: Throwable ->
// Log the error and continue with an empty Observable
Timber.e(error, "Error checking file usage batch")
Observable.empty()
}
Expand Down