From c7bb9bcbceeb34a6bcca25b4de48cf82969dfd7f Mon Sep 17 00:00:00 2001 From: Saifuddin Date: Thu, 16 Jan 2025 18:00:39 +0530 Subject: [PATCH 01/11] Rename .java to .kt --- ...nsContentProvider.java => BookmarkLocationsContentProvider.kt} | 0 ...arkLocationsController.java => BookmarkLocationsController.kt} | 0 .../{BookmarkLocationsDao.java => BookmarkLocationsDao.kt} | 0 ...ookmarkLocationsFragment.java => BookmarkLocationsFragment.kt} | 0 4 files changed, 0 insertions(+), 0 deletions(-) rename app/src/main/java/fr/free/nrw/commons/bookmarks/locations/{BookmarkLocationsContentProvider.java => BookmarkLocationsContentProvider.kt} (100%) rename app/src/main/java/fr/free/nrw/commons/bookmarks/locations/{BookmarkLocationsController.java => BookmarkLocationsController.kt} (100%) rename app/src/main/java/fr/free/nrw/commons/bookmarks/locations/{BookmarkLocationsDao.java => BookmarkLocationsDao.kt} (100%) rename app/src/main/java/fr/free/nrw/commons/bookmarks/locations/{BookmarkLocationsFragment.java => BookmarkLocationsFragment.kt} (100%) diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsContentProvider.java b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsContentProvider.kt similarity index 100% rename from app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsContentProvider.java rename to app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsContentProvider.kt diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsController.java b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsController.kt similarity index 100% rename from app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsController.java rename to app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsController.kt diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.java b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.kt similarity index 100% rename from app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.java rename to app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.kt diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.java b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt similarity index 100% rename from app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.java rename to app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt From 313f61538f9eaaac5b1f5d061064ed8de68670e7 Mon Sep 17 00:00:00 2001 From: Saifuddin Date: Thu, 16 Jan 2025 18:00:39 +0530 Subject: [PATCH 02/11] Refactor: Migrate bookmark location logic to Kotlin This commit migrates the bookmark location logic to Kotlin, enhancing code maintainability and readability. - Removes the `BookmarkLocationsContentProvider`, `BookmarkLocationsController`, and `BookmarkLocationsDao` Java classes. - Creates `BookmarkLocationsDao.kt` and `BookmarkLocationsContentProvider.kt` in Kotlin. - Migrates the logic from `BookmarkLocationsFragment.java` to `BookmarkLocationsFragment.kt`. - Updates test files to reflect these changes. - Addresses associated code review comments. --- .../BookmarkLocationsContentProvider.kt | 211 +++++----- .../locations/BookmarkLocationsController.kt | 27 +- .../locations/BookmarkLocationsDao.kt | 385 ++++++++---------- .../locations/BookmarkLocationsFragment.kt | 228 ++++++----- .../locations/BookMarkLocationDaoTest.kt | 13 +- .../BookmarkLocationControllerTest.kt | 2 +- .../NearbyParentFragmentPresenterTest.kt | 2 +- 7 files changed, 423 insertions(+), 445 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsContentProvider.kt b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsContentProvider.kt index 8c9b559d4..e63325f2d 100644 --- a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsContentProvider.kt +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsContentProvider.kt @@ -1,119 +1,126 @@ -package fr.free.nrw.commons.bookmarks.locations; +package fr.free.nrw.commons.bookmarks.locations -import android.content.ContentValues; -import android.database.Cursor; -import android.database.sqlite.SQLiteDatabase; -import android.database.sqlite.SQLiteQueryBuilder; -// We can get uri using java.Net.Uri, but andoid implimentation is faster (but it's forgiving with handling exceptions though) -import android.net.Uri; -import android.text.TextUtils; +// We can get uri using java.Net.Uri, but android implementation is faster +// (but it's forgiving with handling exceptions though) +import android.content.ContentValues +import android.database.Cursor +import android.database.sqlite.SQLiteQueryBuilder +import android.net.Uri +import fr.free.nrw.commons.BuildConfig +import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.COLUMN_NAME +import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.TABLE_NAME +import fr.free.nrw.commons.data.DBOpenHelper +import fr.free.nrw.commons.di.CommonsDaggerContentProvider +import timber.log.Timber +import javax.inject.Inject -import androidx.annotation.NonNull; - -import javax.inject.Inject; - -import fr.free.nrw.commons.BuildConfig; -import fr.free.nrw.commons.data.DBOpenHelper; -import fr.free.nrw.commons.di.CommonsDaggerContentProvider; -import timber.log.Timber; - -import static fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.COLUMN_NAME; -import static fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.TABLE_NAME; /** * Handles private storage for Bookmark locations */ -public class BookmarkLocationsContentProvider extends CommonsDaggerContentProvider { +class BookmarkLocationsContentProvider : CommonsDaggerContentProvider() { - private static final String BASE_PATH = "bookmarksLocations"; - public static final Uri BASE_URI = Uri.parse("content://" + BuildConfig.BOOKMARK_LOCATIONS_AUTHORITY + "/" + BASE_PATH); + companion object { + private const val BASE_PATH = "bookmarksLocations" + val BASE_URI: Uri = + Uri.parse("content://${BuildConfig.BOOKMARK_LOCATIONS_AUTHORITY}/$BASE_PATH") - /** - * Append bookmark locations name to the base uri - */ - public static Uri uriForName(String name) { - return Uri.parse(BASE_URI.toString() + "/" + name); - } - - @Inject DBOpenHelper dbOpenHelper; - - @Override - public String getType(@NonNull Uri uri) { - return null; - } - - /** - * Queries the SQLite database for the bookmark locations - * @param uri : contains the uri for bookmark locations - * @param projection - * @param selection : handles Where - * @param selectionArgs : the condition of Where clause - * @param sortOrder : ascending or descending - */ - @SuppressWarnings("ConstantConditions") - @Override - public Cursor query(@NonNull Uri uri, String[] projection, String selection, - String[] selectionArgs, String sortOrder) { - SQLiteQueryBuilder queryBuilder = new SQLiteQueryBuilder(); - queryBuilder.setTables(TABLE_NAME); - - SQLiteDatabase db = dbOpenHelper.getReadableDatabase(); - Cursor cursor = queryBuilder.query(db, projection, selection, selectionArgs, null, null, sortOrder); - cursor.setNotificationUri(getContext().getContentResolver(), uri); - - return cursor; - } - - /** - * Handles the update query of local SQLite Database - * @param uri : contains the uri for bookmark locations - * @param contentValues : new values to be entered to db - * @param selection : handles Where - * @param selectionArgs : the condition of Where clause - */ - @SuppressWarnings("ConstantConditions") - @Override - public int update(@NonNull Uri uri, ContentValues contentValues, String selection, - String[] selectionArgs) { - SQLiteDatabase sqlDB = dbOpenHelper.getWritableDatabase(); - int rowsUpdated; - if (TextUtils.isEmpty(selection)) { - int id = Integer.valueOf(uri.getLastPathSegment()); - rowsUpdated = sqlDB.update(TABLE_NAME, - contentValues, - COLUMN_NAME + " = ?", - new String[]{String.valueOf(id)}); - } else { - throw new IllegalArgumentException( - "Parameter `selection` should be empty when updating an ID"); + /** + * Append bookmark locations name to the base URI. + */ + fun uriForName(name: String): Uri { + return Uri.parse("$BASE_URI/$name") } - getContext().getContentResolver().notifyChange(uri, null); - return rowsUpdated; + } + + @Inject + lateinit var dbOpenHelper: DBOpenHelper + + override fun getType(uri: Uri): String? = null + + /** + * Queries the SQLite database for the bookmark locations. + */ + override fun query( + uri: Uri, + projection: Array?, + selection: String?, + selectionArgs: Array?, + sortOrder: String? + ): Cursor { + val queryBuilder = SQLiteQueryBuilder().apply { + tables = TABLE_NAME + } + + val db = dbOpenHelper.readableDatabase + val cursor = queryBuilder.query( + db, + projection, + selection, + selectionArgs, + null, + null, + sortOrder + ) + cursor.setNotificationUri(context?.contentResolver, uri) + return cursor } /** - * Handles the insertion of new bookmark locations record to local SQLite Database + * Handles the update query of local SQLite database. */ - @SuppressWarnings("ConstantConditions") - @Override - public Uri insert(@NonNull Uri uri, ContentValues contentValues) { - SQLiteDatabase sqlDB = dbOpenHelper.getWritableDatabase(); - long id = sqlDB.insert(BookmarkLocationsDao.Table.TABLE_NAME, null, contentValues); - getContext().getContentResolver().notifyChange(uri, null); - return Uri.parse(BASE_URI + "/" + id); + override fun update( + uri: Uri, + contentValues: ContentValues?, + selection: String?, + selectionArgs: Array? + ): Int { + val db = dbOpenHelper.writableDatabase + val rowsUpdated: Int + + if (selection.isNullOrEmpty()) { + val id = uri.lastPathSegment?.toIntOrNull() + ?: throw IllegalArgumentException("Invalid ID in URI") + rowsUpdated = db.update( + TABLE_NAME, + contentValues, + "$COLUMN_NAME = ?", + arrayOf(id.toString()) + ) + } else { + throw IllegalArgumentException( + "Parameter `selection` should be empty when updating an ID" + ) + } + + context?.contentResolver?.notifyChange(uri, null) + return rowsUpdated } - @SuppressWarnings("ConstantConditions") - @Override - public int delete(@NonNull Uri uri, String s, String[] strings) { - int rows; - SQLiteDatabase db = dbOpenHelper.getReadableDatabase(); - Timber.d("Deleting bookmark name %s", uri.getLastPathSegment()); - rows = db.delete(TABLE_NAME, - "location_name = ?", - new String[]{uri.getLastPathSegment()} - ); - getContext().getContentResolver().notifyChange(uri, null); - return rows; + /** + * Handles the insertion of a new bookmark locations record to the local SQLite database. + */ + override fun insert(uri: Uri, contentValues: ContentValues?): Uri { + val db = dbOpenHelper.writableDatabase + val id = db.insert(TABLE_NAME, null, contentValues) + context?.contentResolver?.notifyChange(uri, null) + return Uri.parse("$BASE_URI/$id") + } + + /** + * Handles the deletion of bookmark locations from the local SQLite database. + */ + override fun delete(uri: Uri, selection: String?, selectionArgs: Array?): Int { + val db = dbOpenHelper.readableDatabase + Timber.d("Deleting bookmark name %s", uri.lastPathSegment) + + val rows = db.delete( + TABLE_NAME, + "location_name = ?", + arrayOf(uri.lastPathSegment) + ) + + context?.contentResolver?.notifyChange(uri, null) + return rows } } diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsController.kt b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsController.kt index 6e4c17c2e..ad059c414 100644 --- a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsController.kt +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsController.kt @@ -1,26 +1,17 @@ -package fr.free.nrw.commons.bookmarks.locations; +package fr.free.nrw.commons.bookmarks.locations -import java.util.List; - -import javax.inject.Inject; -import javax.inject.Singleton; - -import fr.free.nrw.commons.nearby.Place; +import fr.free.nrw.commons.nearby.Place +import javax.inject.Inject +import javax.inject.Singleton @Singleton -public class BookmarkLocationsController { - - @Inject - BookmarkLocationsDao bookmarkLocationDao; - - @Inject - public BookmarkLocationsController() {} +class BookmarkLocationsController @Inject constructor( + private val bookmarkLocationDao: BookmarkLocationsDao +) { /** - * Load from DB the bookmarked locations + * Load bookmarked locations from the database. * @return a list of Place objects. */ - public List loadFavoritesLocations() { - return bookmarkLocationDao.getAllBookmarksLocations(); - } + fun loadFavoritesLocations(): List = bookmarkLocationDao.getAllBookmarksLocations() } diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.kt b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.kt index fe4f603f4..eb86909c8 100644 --- a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.kt +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.kt @@ -1,313 +1,286 @@ -package fr.free.nrw.commons.bookmarks.locations; +package fr.free.nrw.commons.bookmarks.locations -import android.annotation.SuppressLint; -import android.content.ContentProviderClient; -import android.content.ContentValues; -import android.database.Cursor; -import android.database.sqlite.SQLiteDatabase; -import android.database.sqlite.SQLiteException; -import android.os.RemoteException; -import androidx.annotation.NonNull; +import android.annotation.SuppressLint +import android.content.ContentProviderClient +import android.content.ContentValues +import android.database.Cursor +import android.database.sqlite.SQLiteDatabase +import android.database.sqlite.SQLiteException +import android.os.RemoteException +import androidx.annotation.NonNull +import fr.free.nrw.commons.location.LatLng +import fr.free.nrw.commons.nearby.Label +import fr.free.nrw.commons.nearby.NearbyController +import fr.free.nrw.commons.nearby.Place +import fr.free.nrw.commons.nearby.Sitelinks +import timber.log.Timber +import javax.inject.Inject +import javax.inject.Named +import javax.inject.Provider -import fr.free.nrw.commons.nearby.NearbyController; -import java.util.ArrayList; -import java.util.List; -import javax.inject.Inject; -import javax.inject.Named; -import javax.inject.Provider; - -import fr.free.nrw.commons.location.LatLng; -import fr.free.nrw.commons.nearby.Label; -import fr.free.nrw.commons.nearby.Place; -import fr.free.nrw.commons.nearby.Sitelinks; -import timber.log.Timber; - -import static fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsContentProvider.BASE_URI; - -public class BookmarkLocationsDao { - - private final Provider clientProvider; - - @Inject - public BookmarkLocationsDao(@Named("bookmarksLocation") Provider clientProvider) { - this.clientProvider = clientProvider; - } +class BookmarkLocationsDao @Inject constructor( + @Named("bookmarksLocation") private val clientProvider: Provider +) { /** - * Find all persisted locations bookmarks on database - * + * Find all persisted location bookmarks in the database * @return list of Place */ - @NonNull - public List getAllBookmarksLocations() { - List items = new ArrayList<>(); - Cursor cursor = null; - ContentProviderClient db = clientProvider.get(); + fun getAllBookmarksLocations(): List { + val items = mutableListOf() + var cursor: Cursor? = null + val db = clientProvider.get() try { cursor = db.query( BookmarkLocationsContentProvider.BASE_URI, Table.ALL_FIELDS, null, - new String[]{}, - null); - while (cursor != null && cursor.moveToNext()) { - items.add(fromCursor(cursor)); + emptyArray(), + null + ) + cursor?.let { + while (it.moveToNext()) { + items.add(fromCursor(it)) + } } - } catch (RemoteException e) { - throw new RuntimeException(e); + } catch (e: RemoteException) { + throw RuntimeException(e) } finally { - if (cursor != null) { - cursor.close(); - } - db.release(); + cursor?.close() + db.release() } - return items; + return items } /** * Look for a place in bookmarks table in order to insert or delete it - * * @param bookmarkLocation : Place object - * @return is Place now fav ? + * @return boolean : is Place now fav ? */ - public boolean updateBookmarkLocation(Place bookmarkLocation) { - boolean bookmarkExists = findBookmarkLocation(bookmarkLocation); + fun updateBookmarkLocation(bookmarkLocation: Place): Boolean { + val bookmarkExists = findBookmarkLocation(bookmarkLocation) if (bookmarkExists) { - deleteBookmarkLocation(bookmarkLocation); - NearbyController.updateMarkerLabelListBookmark(bookmarkLocation, false); + deleteBookmarkLocation(bookmarkLocation) + NearbyController.updateMarkerLabelListBookmark(bookmarkLocation, false) } else { - addBookmarkLocation(bookmarkLocation); - NearbyController.updateMarkerLabelListBookmark(bookmarkLocation, true); + addBookmarkLocation(bookmarkLocation) + NearbyController.updateMarkerLabelListBookmark(bookmarkLocation, true) } - return !bookmarkExists; + return !bookmarkExists } /** * Add a Place to bookmarks table - * * @param bookmarkLocation : Place to add */ - private void addBookmarkLocation(Place bookmarkLocation) { - ContentProviderClient db = clientProvider.get(); + private fun addBookmarkLocation(bookmarkLocation: Place) { + val db = clientProvider.get() try { - db.insert(BASE_URI, toContentValues(bookmarkLocation)); - } catch (RemoteException e) { - throw new RuntimeException(e); + db.insert(BookmarkLocationsContentProvider.BASE_URI, toContentValues(bookmarkLocation)) + } catch (e: RemoteException) { + throw RuntimeException(e) } finally { - db.release(); + db.release() } } /** * Delete a Place from bookmarks table - * * @param bookmarkLocation : Place to delete */ - private void deleteBookmarkLocation(Place bookmarkLocation) { - ContentProviderClient db = clientProvider.get(); + private fun deleteBookmarkLocation(bookmarkLocation: Place) { + val db = clientProvider.get() try { - db.delete(BookmarkLocationsContentProvider.uriForName(bookmarkLocation.name), null, null); - } catch (RemoteException e) { - throw new RuntimeException(e); + db.delete( + BookmarkLocationsContentProvider.uriForName(bookmarkLocation.name), + null, + null + ) + } catch (e: RemoteException) { + throw RuntimeException(e) } finally { - db.release(); + db.release() } } /** * Find a Place from database based on its name - * * @param bookmarkLocation : Place to find * @return boolean : is Place in database ? */ - public boolean findBookmarkLocation(Place bookmarkLocation) { - Cursor cursor = null; - ContentProviderClient db = clientProvider.get(); + fun findBookmarkLocation(bookmarkLocation: Place): Boolean { + var cursor: Cursor? = null + val db = clientProvider.get() try { cursor = db.query( BookmarkLocationsContentProvider.BASE_URI, Table.ALL_FIELDS, - Table.COLUMN_NAME + "=?", - new String[]{bookmarkLocation.name}, - null); - if (cursor != null && cursor.moveToFirst()) { - return true; - } - } catch (RemoteException e) { - // This feels lazy, but to hell with checked exceptions. :) - throw new RuntimeException(e); + "${Table.COLUMN_NAME}=?", + arrayOf(bookmarkLocation.name), + null + ) + return cursor?.moveToFirst() == true + } catch (e: RemoteException) { + throw RuntimeException(e) } finally { - if (cursor != null) { - cursor.close(); - } - db.release(); + cursor?.close() + db.release() } - return false; } @SuppressLint("Range") @NonNull - Place fromCursor(final Cursor cursor) { - final LatLng location = new LatLng(cursor.getDouble(cursor.getColumnIndex(Table.COLUMN_LAT)), - cursor.getDouble(cursor.getColumnIndex(Table.COLUMN_LONG)), 1F); + fun fromCursor(cursor: Cursor): Place { + val location = LatLng( + cursor.getDouble(cursor.getColumnIndex(Table.COLUMN_LAT)), + cursor.getDouble(cursor.getColumnIndex(Table.COLUMN_LONG)), + 1F + ) - final Sitelinks.Builder builder = new Sitelinks.Builder(); - builder.setWikipediaLink(cursor.getString(cursor.getColumnIndex(Table.COLUMN_WIKIPEDIA_LINK))); - builder.setWikidataLink(cursor.getString(cursor.getColumnIndex(Table.COLUMN_WIKIDATA_LINK))); - builder.setCommonsLink(cursor.getString(cursor.getColumnIndex(Table.COLUMN_COMMONS_LINK))); + val builder = Sitelinks.Builder().apply { + setWikipediaLink(cursor.getString(cursor.getColumnIndex(Table.COLUMN_WIKIPEDIA_LINK))) + setWikidataLink(cursor.getString(cursor.getColumnIndex(Table.COLUMN_WIKIDATA_LINK))) + setCommonsLink(cursor.getString(cursor.getColumnIndex(Table.COLUMN_COMMONS_LINK))) + } - return new Place( + return Place( cursor.getString(cursor.getColumnIndex(Table.COLUMN_LANGUAGE)), cursor.getString(cursor.getColumnIndex(Table.COLUMN_NAME)), - Label.fromText((cursor.getString(cursor.getColumnIndex(Table.COLUMN_LABEL_TEXT)))), + Label.fromText(cursor.getString(cursor.getColumnIndex(Table.COLUMN_LABEL_TEXT))), cursor.getString(cursor.getColumnIndex(Table.COLUMN_DESCRIPTION)), location, cursor.getString(cursor.getColumnIndex(Table.COLUMN_CATEGORY)), builder.build(), cursor.getString(cursor.getColumnIndex(Table.COLUMN_PIC)), - Boolean.parseBoolean(cursor.getString(cursor.getColumnIndex(Table.COLUMN_EXISTS))) - ); + cursor.getString(cursor.getColumnIndex(Table.COLUMN_EXISTS))?.toBoolean() ?: false + ) } - private ContentValues toContentValues(Place bookmarkLocation) { - ContentValues cv = new ContentValues(); - cv.put(BookmarkLocationsDao.Table.COLUMN_NAME, bookmarkLocation.getName()); - cv.put(BookmarkLocationsDao.Table.COLUMN_LANGUAGE, bookmarkLocation.getLanguage()); - cv.put(BookmarkLocationsDao.Table.COLUMN_DESCRIPTION, bookmarkLocation.getLongDescription()); - cv.put(BookmarkLocationsDao.Table.COLUMN_CATEGORY, bookmarkLocation.getCategory()); - cv.put(BookmarkLocationsDao.Table.COLUMN_LABEL_TEXT, bookmarkLocation.getLabel()!=null ? bookmarkLocation.getLabel().getText() : ""); - cv.put(BookmarkLocationsDao.Table.COLUMN_LABEL_ICON, bookmarkLocation.getLabel()!=null ? bookmarkLocation.getLabel().getIcon() : null); - cv.put(BookmarkLocationsDao.Table.COLUMN_WIKIPEDIA_LINK, bookmarkLocation.siteLinks.getWikipediaLink().toString()); - cv.put(BookmarkLocationsDao.Table.COLUMN_WIKIDATA_LINK, bookmarkLocation.siteLinks.getWikidataLink().toString()); - cv.put(BookmarkLocationsDao.Table.COLUMN_COMMONS_LINK, bookmarkLocation.siteLinks.getCommonsLink().toString()); - cv.put(BookmarkLocationsDao.Table.COLUMN_LAT, bookmarkLocation.location.getLatitude()); - cv.put(BookmarkLocationsDao.Table.COLUMN_LONG, bookmarkLocation.location.getLongitude()); - cv.put(BookmarkLocationsDao.Table.COLUMN_PIC, bookmarkLocation.pic); - cv.put(BookmarkLocationsDao.Table.COLUMN_EXISTS, bookmarkLocation.exists.toString()); - return cv; + private fun toContentValues(bookmarkLocation: Place): ContentValues { + return ContentValues().apply { + put(Table.COLUMN_NAME, bookmarkLocation.name) + put(Table.COLUMN_LANGUAGE, bookmarkLocation.language) + put(Table.COLUMN_DESCRIPTION, bookmarkLocation.longDescription) + put(Table.COLUMN_CATEGORY, bookmarkLocation.category) + put(Table.COLUMN_LABEL_TEXT, bookmarkLocation.label?.text ?: "") + put(Table.COLUMN_LABEL_ICON, bookmarkLocation.label?.icon) + put(Table.COLUMN_WIKIPEDIA_LINK, bookmarkLocation.siteLinks.wikipediaLink.toString()) + put(Table.COLUMN_WIKIDATA_LINK, bookmarkLocation.siteLinks.wikidataLink.toString()) + put(Table.COLUMN_COMMONS_LINK, bookmarkLocation.siteLinks.commonsLink.toString()) + put(Table.COLUMN_LAT, bookmarkLocation.location.latitude) + put(Table.COLUMN_LONG, bookmarkLocation.location.longitude) + put(Table.COLUMN_PIC, bookmarkLocation.pic) + put(Table.COLUMN_EXISTS, bookmarkLocation.exists.toString()) + } } - public static class Table { - public static final String TABLE_NAME = "bookmarksLocations"; + object Table { + const val TABLE_NAME = "bookmarksLocations" - static final String COLUMN_NAME = "location_name"; - static final String COLUMN_LANGUAGE = "location_language"; - static final String COLUMN_DESCRIPTION = "location_description"; - static final String COLUMN_LAT = "location_lat"; - static final String COLUMN_LONG = "location_long"; - static final String COLUMN_CATEGORY = "location_category"; - static final String COLUMN_LABEL_TEXT = "location_label_text"; - static final String COLUMN_LABEL_ICON = "location_label_icon"; - static final String COLUMN_IMAGE_URL = "location_image_url"; - static final String COLUMN_WIKIPEDIA_LINK = "location_wikipedia_link"; - static final String COLUMN_WIKIDATA_LINK = "location_wikidata_link"; - static final String COLUMN_COMMONS_LINK = "location_commons_link"; - static final String COLUMN_PIC = "location_pic"; - static final String COLUMN_EXISTS = "location_exists"; + const val COLUMN_NAME = "location_name" + const val COLUMN_LANGUAGE = "location_language" + const val COLUMN_DESCRIPTION = "location_description" + const val COLUMN_LAT = "location_lat" + const val COLUMN_LONG = "location_long" + const val COLUMN_CATEGORY = "location_category" + const val COLUMN_LABEL_TEXT = "location_label_text" + const val COLUMN_LABEL_ICON = "location_label_icon" + const val COLUMN_IMAGE_URL = "location_image_url" + const val COLUMN_WIKIPEDIA_LINK = "location_wikipedia_link" + const val COLUMN_WIKIDATA_LINK = "location_wikidata_link" + const val COLUMN_COMMONS_LINK = "location_commons_link" + const val COLUMN_PIC = "location_pic" + const val COLUMN_EXISTS = "location_exists" // NOTE! KEEP IN SAME ORDER AS THEY ARE DEFINED UP THERE. HELPS HARD CODE COLUMN INDICES. - public static final String[] ALL_FIELDS = { - COLUMN_NAME, - COLUMN_LANGUAGE, - COLUMN_DESCRIPTION, - COLUMN_CATEGORY, - COLUMN_LABEL_TEXT, - COLUMN_LABEL_ICON, - COLUMN_LAT, - COLUMN_LONG, - COLUMN_IMAGE_URL, - COLUMN_WIKIPEDIA_LINK, - COLUMN_WIKIDATA_LINK, - COLUMN_COMMONS_LINK, - COLUMN_PIC, - COLUMN_EXISTS, - }; + val ALL_FIELDS = arrayOf( + COLUMN_NAME, COLUMN_LANGUAGE, COLUMN_DESCRIPTION, COLUMN_CATEGORY, COLUMN_LABEL_TEXT, COLUMN_LABEL_ICON, + COLUMN_LAT, COLUMN_LONG, COLUMN_IMAGE_URL, COLUMN_WIKIPEDIA_LINK, COLUMN_WIKIDATA_LINK, COLUMN_COMMONS_LINK, + COLUMN_PIC, COLUMN_EXISTS + ) - static final String DROP_TABLE_STATEMENT = "DROP TABLE IF EXISTS " + TABLE_NAME; + const val DROP_TABLE_STATEMENT = "DROP TABLE IF EXISTS $TABLE_NAME" - static final String CREATE_TABLE_STATEMENT = "CREATE TABLE " + TABLE_NAME + " (" - + COLUMN_NAME + " STRING PRIMARY KEY," - + COLUMN_LANGUAGE + " STRING," - + COLUMN_DESCRIPTION + " STRING," - + COLUMN_CATEGORY + " STRING," - + COLUMN_LABEL_TEXT + " STRING," - + COLUMN_LABEL_ICON + " INTEGER," - + COLUMN_LAT + " DOUBLE," - + COLUMN_LONG + " DOUBLE," - + COLUMN_IMAGE_URL + " STRING," - + COLUMN_WIKIPEDIA_LINK + " STRING," - + COLUMN_WIKIDATA_LINK + " STRING," - + COLUMN_COMMONS_LINK + " STRING," - + COLUMN_PIC + " STRING," - + COLUMN_EXISTS + " STRING" - + ");"; + const val CREATE_TABLE_STATEMENT = """ + CREATE TABLE $TABLE_NAME ( + $COLUMN_NAME STRING PRIMARY KEY, + $COLUMN_LANGUAGE STRING, + $COLUMN_DESCRIPTION STRING, + $COLUMN_CATEGORY STRING, + $COLUMN_LABEL_TEXT STRING, + $COLUMN_LABEL_ICON INTEGER, + $COLUMN_LAT DOUBLE, + $COLUMN_LONG DOUBLE, + $COLUMN_IMAGE_URL STRING, + $COLUMN_WIKIPEDIA_LINK STRING, + $COLUMN_WIKIDATA_LINK STRING, + $COLUMN_COMMONS_LINK STRING, + $COLUMN_PIC STRING, + $COLUMN_EXISTS STRING + ); + """ - public static void onCreate(SQLiteDatabase db) { - db.execSQL(CREATE_TABLE_STATEMENT); + fun onCreate(db: SQLiteDatabase) { + db.execSQL(CREATE_TABLE_STATEMENT) } - public static void onDelete(SQLiteDatabase db) { - db.execSQL(DROP_TABLE_STATEMENT); - onCreate(db); + fun onDelete(db: SQLiteDatabase) { + db.execSQL(DROP_TABLE_STATEMENT) + onCreate(db) } - public static void onUpdate(final SQLiteDatabase db, int from, final int to) { - Timber.d("bookmarksLocations db is updated from:"+from+", to:"+to); + @SuppressLint("SQLiteString") + fun onUpdate(db: SQLiteDatabase, from: Int, to: Int) { + Timber.d("bookmarksLocations db is updated from:$from, to:$to") + var currFrom = from + if (from == to) { - return; + return } if (from < 7) { - // doesn't exist yet - from++; - onUpdate(db, from, to); - return; + onUpdate(db, ++currFrom, to) + return } if (from == 7) { - // table added in version 8 - onCreate(db); - from++; - onUpdate(db, from, to); - return; + onCreate(db) + onUpdate(db, ++currFrom, to) + return } if (from < 10) { - from++; - onUpdate(db, from, to); - return; + onUpdate(db, ++currFrom, to) + return } if (from == 10) { - //This is safe, and can be called clean, as we/I do not remember the appropriate version for this - //We are anyways switching to room, these things won't be necessary then try { - db.execSQL("ALTER TABLE bookmarksLocations ADD COLUMN location_pic STRING;"); - }catch (SQLiteException exception){ - Timber.e(exception);// + db.execSQL("ALTER TABLE $TABLE_NAME ADD COLUMN location_pic STRING;") + } catch (exception: SQLiteException) { + Timber.e(exception) } - return; + return } if (from >= 12) { try { - db.execSQL( - "ALTER TABLE bookmarksLocations ADD COLUMN location_destroyed STRING;"); - } catch (SQLiteException exception) { - Timber.e(exception); + db.execSQL("ALTER TABLE $TABLE_NAME ADD COLUMN location_destroyed STRING;") + } catch (exception: SQLiteException) { + Timber.e(exception) } } - if (from >= 13){ + if (from >= 13) { try { - db.execSQL("ALTER TABLE bookmarksLocations ADD COLUMN location_language STRING;"); - } catch (SQLiteException exception){ - Timber.e(exception); + db.execSQL("ALTER TABLE $TABLE_NAME ADD COLUMN location_language STRING;") + } catch (exception: SQLiteException) { + Timber.e(exception) } } - if (from >= 14){ + if (from >= 14) { try { - db.execSQL("ALTER TABLE bookmarksLocations ADD COLUMN location_exists STRING;"); - } catch (SQLiteException exception){ - Timber.e(exception); + db.execSQL("ALTER TABLE $TABLE_NAME ADD COLUMN location_exists STRING;") + } catch (exception: SQLiteException) { + Timber.e(exception) } } } } -} \ No newline at end of file +} diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt index f5ce556c4..228ec2599 100644 --- a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt @@ -1,137 +1,145 @@ -package fr.free.nrw.commons.bookmarks.locations; +package fr.free.nrw.commons.bookmarks.locations -import android.Manifest.permission; -import android.content.Intent; -import android.os.Bundle; -import android.view.LayoutInflater; -import android.view.View; -import android.view.ViewGroup; -import androidx.activity.result.ActivityResultCallback; -import androidx.activity.result.ActivityResultLauncher; -import androidx.activity.result.contract.ActivityResultContracts; -import androidx.activity.result.contract.ActivityResultContracts.StartActivityForResult; -import androidx.annotation.NonNull; -import androidx.annotation.Nullable; -import androidx.recyclerview.widget.LinearLayoutManager; -import dagger.android.support.DaggerFragment; -import fr.free.nrw.commons.R; -import fr.free.nrw.commons.contributions.ContributionController; -import fr.free.nrw.commons.databinding.FragmentBookmarksLocationsBinding; -import fr.free.nrw.commons.nearby.Place; -import fr.free.nrw.commons.nearby.fragments.CommonPlaceClickActions; -import fr.free.nrw.commons.nearby.fragments.PlaceAdapter; -import java.util.List; -import java.util.Map; -import javax.inject.Inject; -import kotlin.Unit; +import android.Manifest.permission +import android.os.Bundle +import android.view.LayoutInflater +import android.view.View +import android.view.ViewGroup +import androidx.activity.result.ActivityResultLauncher +import androidx.activity.result.contract.ActivityResultContracts.RequestMultiplePermissions +import androidx.activity.result.contract.ActivityResultContracts.StartActivityForResult +import androidx.recyclerview.widget.LinearLayoutManager +import dagger.android.support.DaggerFragment +import fr.free.nrw.commons.R +import fr.free.nrw.commons.contributions.ContributionController +import fr.free.nrw.commons.databinding.FragmentBookmarksLocationsBinding +import fr.free.nrw.commons.filepicker.FilePicker +import fr.free.nrw.commons.nearby.fragments.CommonPlaceClickActions +import fr.free.nrw.commons.nearby.fragments.PlaceAdapter +import javax.inject.Inject -public class BookmarkLocationsFragment extends DaggerFragment { - public FragmentBookmarksLocationsBinding binding; +class BookmarkLocationsFragment : DaggerFragment() { - @Inject BookmarkLocationsController controller; - @Inject ContributionController contributionController; - @Inject BookmarkLocationsDao bookmarkLocationDao; - @Inject CommonPlaceClickActions commonPlaceClickActions; - private PlaceAdapter adapter; + private var binding: FragmentBookmarksLocationsBinding? = null - private final ActivityResultLauncher cameraPickLauncherForResult = - registerForActivityResult(new StartActivityForResult(), - result -> { - contributionController.handleActivityResultWithCallback(requireActivity(), callbacks -> { - contributionController.onPictureReturnedFromCamera(result, requireActivity(), callbacks); - }); - }); + @Inject lateinit var controller: BookmarkLocationsController + @Inject lateinit var contributionController: ContributionController + @Inject lateinit var bookmarkLocationDao: BookmarkLocationsDao + @Inject lateinit var commonPlaceClickActions: CommonPlaceClickActions - private final ActivityResultLauncher galleryPickLauncherForResult = - registerForActivityResult(new StartActivityForResult(), - result -> { - contributionController.handleActivityResultWithCallback(requireActivity(), callbacks -> { - contributionController.onPictureReturnedFromGallery(result, requireActivity(), callbacks); - }); - }); + private lateinit var inAppCameraLocationPermissionLauncher: + ActivityResultLauncher> + private lateinit var adapter: PlaceAdapter - private ActivityResultLauncher inAppCameraLocationPermissionLauncher = registerForActivityResult(new ActivityResultContracts.RequestMultiplePermissions(), new ActivityResultCallback>() { - @Override - public void onActivityResult(Map result) { - boolean areAllGranted = true; - for(final boolean b : result.values()) { - areAllGranted = areAllGranted && b; - } + private val cameraPickLauncherForResult = + registerForActivityResult(StartActivityForResult()) { result -> + contributionController.handleActivityResultWithCallback( + requireActivity(), + object: FilePicker.HandleActivityResult { + override fun onHandleActivityResult(callbacks: FilePicker.Callbacks) { + contributionController.onPictureReturnedFromCamera( + result, + requireActivity(), + callbacks + ) + } + } + ) + } - if (areAllGranted) { - contributionController.locationPermissionCallback.onLocationPermissionGranted(); - } else { - if (shouldShowRequestPermissionRationale(permission.ACCESS_FINE_LOCATION)) { - contributionController.handleShowRationaleFlowCameraLocation(getActivity(), inAppCameraLocationPermissionLauncher, cameraPickLauncherForResult); + private val galleryPickLauncherForResult = + registerForActivityResult(StartActivityForResult()) { result -> + contributionController.handleActivityResultWithCallback( + requireActivity(), + object: FilePicker.HandleActivityResult { + override fun onHandleActivityResult(callbacks: FilePicker.Callbacks) { + contributionController.onPictureReturnedFromGallery( + result, + requireActivity(), + callbacks + ) + } + } + ) + } + + companion object { + fun newInstance(): BookmarkLocationsFragment { + return BookmarkLocationsFragment() + } + } + + override fun onCreateView( + inflater: LayoutInflater, + container: ViewGroup?, + savedInstanceState: Bundle? + ): View? { + binding = FragmentBookmarksLocationsBinding.inflate(inflater, container, false) + return binding?.root + } + + override fun onViewCreated(view: View, savedInstanceState: Bundle?) { + super.onViewCreated(view, savedInstanceState) + binding?.loadingImagesProgressBar?.visibility = View.VISIBLE + binding?.listView?.layoutManager = LinearLayoutManager(context) + + inAppCameraLocationPermissionLauncher = + registerForActivityResult(RequestMultiplePermissions()) { result -> + val areAllGranted = result.values.all { it } + + if (areAllGranted) { + contributionController.locationPermissionCallback.onLocationPermissionGranted() } else { - contributionController.locationPermissionCallback.onLocationPermissionDenied(getActivity().getString(R.string.in_app_camera_location_permission_denied)); + if (shouldShowRequestPermissionRationale(permission.ACCESS_FINE_LOCATION)) { + contributionController.handleShowRationaleFlowCameraLocation( + activity, + inAppCameraLocationPermissionLauncher, + cameraPickLauncherForResult + ) + } else { + contributionController.locationPermissionCallback + .onLocationPermissionDenied( + getString(R.string.in_app_camera_location_permission_denied) + ) + } } } - } - }); - /** - * Create an instance of the fragment with the right bundle parameters - * @return an instance of the fragment - */ - public static BookmarkLocationsFragment newInstance() { - return new BookmarkLocationsFragment(); - } - - @Override - public View onCreateView( - @NonNull LayoutInflater inflater, - ViewGroup container, - Bundle savedInstanceState - ) { - binding = FragmentBookmarksLocationsBinding.inflate(inflater, container, false); - return binding.getRoot(); - } - - @Override - public void onViewCreated(View view, @Nullable Bundle savedInstanceState) { - super.onViewCreated(view, savedInstanceState); - binding.loadingImagesProgressBar.setVisibility(View.VISIBLE); - binding.listView.setLayoutManager(new LinearLayoutManager(getContext())); - adapter = new PlaceAdapter(bookmarkLocationDao, - place -> Unit.INSTANCE, - (place, isBookmarked) -> { - adapter.remove(place); - return Unit.INSTANCE; + adapter = PlaceAdapter( + bookmarkLocationDao, + { }, + { place, _ -> + adapter.remove(place) }, commonPlaceClickActions, inAppCameraLocationPermissionLauncher, galleryPickLauncherForResult, cameraPickLauncherForResult - ); - binding.listView.setAdapter(adapter); + ) + binding?.listView?.adapter = adapter } - @Override - public void onResume() { - super.onResume(); - initList(); + override fun onResume() { + super.onResume() + initList() } - /** - * Initialize the recycler view with bookmarked locations - */ - private void initList() { - List places = controller.loadFavoritesLocations(); - adapter.setItems(places); - binding.loadingImagesProgressBar.setVisibility(View.GONE); - if (places.size() <= 0) { - binding.statusMessage.setText(R.string.bookmark_empty); - binding.statusMessage.setVisibility(View.VISIBLE); + private fun initList() { + val places = controller.loadFavoritesLocations() + adapter.items = places + binding?.loadingImagesProgressBar?.visibility = View.GONE + if (places.isEmpty()) { + binding?.statusMessage?.text = getString(R.string.bookmark_empty) + binding?.statusMessage?.visibility = View.VISIBLE } else { - binding.statusMessage.setVisibility(View.GONE); + binding?.statusMessage?.visibility = View.GONE } } - @Override - public void onDestroy() { - super.onDestroy(); - binding = null; + override fun onDestroy() { + super.onDestroy() + // Make sure to null out the binding to avoid memory leaks + binding = null } } diff --git a/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookMarkLocationDaoTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookMarkLocationDaoTest.kt index 391dd8d17..e677bde5d 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookMarkLocationDaoTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookMarkLocationDaoTest.kt @@ -18,7 +18,6 @@ import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.verify import com.nhaarman.mockitokotlin2.whenever import fr.free.nrw.commons.TestCommonsApplication -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsContentProvider.BASE_URI import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.COLUMN_CATEGORY import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.COLUMN_COMMONS_LINK import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.COLUMN_DESCRIPTION @@ -149,7 +148,7 @@ class BookMarkLocationDaoTest { fun getAllLocationBookmarks() { whenever(client.query(any(), any(), anyOrNull(), any(), anyOrNull())).thenReturn(createCursor(14)) - var result = testObject.allBookmarksLocations + var result = testObject.getAllBookmarksLocations() assertEquals(14, result.size) } @@ -157,19 +156,19 @@ class BookMarkLocationDaoTest { @Test(expected = RuntimeException::class) fun getAllLocationBookmarksTranslatesExceptions() { whenever(client.query(any(), any(), anyOrNull(), any(), anyOrNull())).thenThrow(RemoteException("")) - testObject.allBookmarksLocations + testObject.getAllBookmarksLocations() } @Test fun getAllLocationBookmarksReturnsEmptyList_emptyCursor() { whenever(client.query(any(), any(), anyOrNull(), any(), anyOrNull())).thenReturn(createCursor(0)) - assertTrue(testObject.allBookmarksLocations.isEmpty()) + assertTrue(testObject.getAllBookmarksLocations().isEmpty()) } @Test fun getAllLocationBookmarksReturnsEmptyList_nullCursor() { whenever(client.query(any(), any(), anyOrNull(), any(), anyOrNull())).thenReturn(null) - assertTrue(testObject.allBookmarksLocations.isEmpty()) + assertTrue(testObject.getAllBookmarksLocations().isEmpty()) } @Test @@ -178,7 +177,7 @@ class BookMarkLocationDaoTest { whenever(client.query(any(), any(), anyOrNull(), any(), anyOrNull())).thenReturn(mockCursor) whenever(mockCursor.moveToFirst()).thenReturn(false) - testObject.allBookmarksLocations + testObject.getAllBookmarksLocations() verify(mockCursor).close() } @@ -189,7 +188,7 @@ class BookMarkLocationDaoTest { whenever(client.query(any(), any(), any(), any(), anyOrNull())).thenReturn(null) assertTrue(testObject.updateBookmarkLocation(examplePlaceBookmark)) - verify(client).insert(eq(BASE_URI), captor.capture()) + verify(client).insert(eq(BookmarkLocationsContentProvider.BASE_URI), captor.capture()) captor.firstValue.let { cv -> assertEquals(13, cv.size()) assertEquals(examplePlaceBookmark.name, cv.getAsString(COLUMN_NAME)) diff --git a/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationControllerTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationControllerTest.kt index 3fd21c25f..262489ba5 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationControllerTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationControllerTest.kt @@ -20,7 +20,7 @@ class BookmarkLocationControllerTest { @Before fun setup() { MockitoAnnotations.initMocks(this) - whenever(bookmarkDao!!.allBookmarksLocations) + whenever(bookmarkDao!!.getAllBookmarksLocations()) .thenReturn(mockBookmarkList) } diff --git a/app/src/test/kotlin/fr/free/nrw/commons/nearby/NearbyParentFragmentPresenterTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/nearby/NearbyParentFragmentPresenterTest.kt index 80e9a53f9..12b2319d0 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/nearby/NearbyParentFragmentPresenterTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/nearby/NearbyParentFragmentPresenterTest.kt @@ -463,7 +463,7 @@ class NearbyParentFragmentPresenterTest { nearbyPlacesInfo.searchLatLng = latestLocation nearbyPlacesInfo.placeList = emptyList() - whenever(bookmarkLocationsDao.allBookmarksLocations).thenReturn(Collections.emptyList()) + whenever(bookmarkLocationsDao.getAllBookmarksLocations()).thenReturn(Collections.emptyList()) nearbyPresenter.updateMapMarkers(nearbyPlacesInfo.placeList, latestLocation, null) Mockito.verify(nearbyParentFragmentView).setProgressBarVisibility(false) } From 54d0f652947d01ff6e2a119285255f301a0c12d4 Mon Sep 17 00:00:00 2001 From: Saifuddin Date: Thu, 16 Jan 2025 23:11:10 +0530 Subject: [PATCH 03/11] Refactor: Migrate to Room Database for Bookmark Locations This commit migrates the bookmark locations functionality from a custom content provider to Room database. Key changes: * **Removal of `BookmarkLocationsContentProvider`:** This class, which previously handled data storage, has been removed. * **Introduction of `BookmarksLocations`:** This data class now represents a bookmarked location, serving as the Room entity. * **Creation of `BookmarkLocationsDao`:** This Room DAO handles database interactions for bookmark locations, including: * Adding, deleting, and querying bookmarked locations. * Checking if a location is already bookmarked. * Updating the bookmark status of a location. * Retrieving all bookmarked locations as `Place` objects. * **`BookmarkLocationsViewModel`:** Added to manage the data layer for bookmark locations * **`NearbyUtil`:** Created a Util class for Nearby to manage the bookmark locations. * **Updates in `PlaceAdapter` and `PlaceAdapterDelegate`:** These classes have been modified to work with the new Room-based data layer. * **Updates in `AppDatabase`:** The database now includes `BookmarksLocations` as an entity and exposes the `bookmarkLocationsDao`. * **Updates in `FragmentBuilderModule` and `CommonsApplicationModule`**: for DI * **Removal of `DBOpenHelper` upgrade for locations**: as it is no longer needed * **Updates in `NearbyParentFragmentPresenter`**: refactored the logic to use the dao functions * **Updates in `NearbyParentFragment`**: refactored the logic to use the util and dao functions * **Update in `BookmarkLocationsController`**: removed as its no longer needed * **Add `toPlace` and `toBookmarksLocations`**: extension functions to map between data class and entities * **Update in `CommonsApplication`**: to remove old db table. --- .../fr/free/nrw/commons/CommonsApplication.kt | 1 - .../BookmarkLocationsContentProvider.kt | 126 -------- .../locations/BookmarkLocationsController.kt | 2 +- .../locations/BookmarkLocationsDao.kt | 305 ++---------------- .../locations/BookmarkLocationsFragment.kt | 2 + .../locations/BookmarkLocationsViewModel.kt | 15 + .../bookmarks/locations/BookmarksLocations.kt | 72 +++++ .../fr/free/nrw/commons/data/DBOpenHelper.kt | 2 - .../fr/free/nrw/commons/db/AppDatabase.kt | 6 +- .../commons/di/CommonsApplicationModule.kt | 5 + .../di/ContentProviderBuilderModule.kt | 4 - .../nrw/commons/di/FragmentBuilderModule.kt | 3 + .../fr/free/nrw/commons/nearby/NearbyUtil.kt | 22 ++ .../commons/nearby/PlaceAdapterDelegate.kt | 24 +- .../NearbyParentFragmentContract.java | 2 +- .../fragments/NearbyParentFragment.java | 15 +- .../commons/nearby/fragments/PlaceAdapter.kt | 3 + .../NearbyParentFragmentPresenter.kt | 30 +- 18 files changed, 210 insertions(+), 429 deletions(-) delete mode 100644 app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsContentProvider.kt create mode 100644 app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsViewModel.kt create mode 100644 app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarksLocations.kt create mode 100644 app/src/main/java/fr/free/nrw/commons/nearby/NearbyUtil.kt diff --git a/app/src/main/java/fr/free/nrw/commons/CommonsApplication.kt b/app/src/main/java/fr/free/nrw/commons/CommonsApplication.kt index 9ed19d686..b8a26a12f 100644 --- a/app/src/main/java/fr/free/nrw/commons/CommonsApplication.kt +++ b/app/src/main/java/fr/free/nrw/commons/CommonsApplication.kt @@ -253,7 +253,6 @@ class CommonsApplication : MultiDexApplication() { Timber.e(e) } BookmarkPicturesDao.Table.onDelete(db) - BookmarkLocationsDao.Table.onDelete(db) BookmarkItemsDao.Table.onDelete(db) } diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsContentProvider.kt b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsContentProvider.kt deleted file mode 100644 index e63325f2d..000000000 --- a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsContentProvider.kt +++ /dev/null @@ -1,126 +0,0 @@ -package fr.free.nrw.commons.bookmarks.locations - -// We can get uri using java.Net.Uri, but android implementation is faster -// (but it's forgiving with handling exceptions though) -import android.content.ContentValues -import android.database.Cursor -import android.database.sqlite.SQLiteQueryBuilder -import android.net.Uri -import fr.free.nrw.commons.BuildConfig -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.COLUMN_NAME -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.TABLE_NAME -import fr.free.nrw.commons.data.DBOpenHelper -import fr.free.nrw.commons.di.CommonsDaggerContentProvider -import timber.log.Timber -import javax.inject.Inject - - -/** - * Handles private storage for Bookmark locations - */ -class BookmarkLocationsContentProvider : CommonsDaggerContentProvider() { - - companion object { - private const val BASE_PATH = "bookmarksLocations" - val BASE_URI: Uri = - Uri.parse("content://${BuildConfig.BOOKMARK_LOCATIONS_AUTHORITY}/$BASE_PATH") - - /** - * Append bookmark locations name to the base URI. - */ - fun uriForName(name: String): Uri { - return Uri.parse("$BASE_URI/$name") - } - } - - @Inject - lateinit var dbOpenHelper: DBOpenHelper - - override fun getType(uri: Uri): String? = null - - /** - * Queries the SQLite database for the bookmark locations. - */ - override fun query( - uri: Uri, - projection: Array?, - selection: String?, - selectionArgs: Array?, - sortOrder: String? - ): Cursor { - val queryBuilder = SQLiteQueryBuilder().apply { - tables = TABLE_NAME - } - - val db = dbOpenHelper.readableDatabase - val cursor = queryBuilder.query( - db, - projection, - selection, - selectionArgs, - null, - null, - sortOrder - ) - cursor.setNotificationUri(context?.contentResolver, uri) - return cursor - } - - /** - * Handles the update query of local SQLite database. - */ - override fun update( - uri: Uri, - contentValues: ContentValues?, - selection: String?, - selectionArgs: Array? - ): Int { - val db = dbOpenHelper.writableDatabase - val rowsUpdated: Int - - if (selection.isNullOrEmpty()) { - val id = uri.lastPathSegment?.toIntOrNull() - ?: throw IllegalArgumentException("Invalid ID in URI") - rowsUpdated = db.update( - TABLE_NAME, - contentValues, - "$COLUMN_NAME = ?", - arrayOf(id.toString()) - ) - } else { - throw IllegalArgumentException( - "Parameter `selection` should be empty when updating an ID" - ) - } - - context?.contentResolver?.notifyChange(uri, null) - return rowsUpdated - } - - /** - * Handles the insertion of a new bookmark locations record to the local SQLite database. - */ - override fun insert(uri: Uri, contentValues: ContentValues?): Uri { - val db = dbOpenHelper.writableDatabase - val id = db.insert(TABLE_NAME, null, contentValues) - context?.contentResolver?.notifyChange(uri, null) - return Uri.parse("$BASE_URI/$id") - } - - /** - * Handles the deletion of bookmark locations from the local SQLite database. - */ - override fun delete(uri: Uri, selection: String?, selectionArgs: Array?): Int { - val db = dbOpenHelper.readableDatabase - Timber.d("Deleting bookmark name %s", uri.lastPathSegment) - - val rows = db.delete( - TABLE_NAME, - "location_name = ?", - arrayOf(uri.lastPathSegment) - ) - - context?.contentResolver?.notifyChange(uri, null) - return rows - } -} diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsController.kt b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsController.kt index ad059c414..3509f4e73 100644 --- a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsController.kt +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsController.kt @@ -13,5 +13,5 @@ class BookmarkLocationsController @Inject constructor( * Load bookmarked locations from the database. * @return a list of Place objects. */ - fun loadFavoritesLocations(): List = bookmarkLocationDao.getAllBookmarksLocations() + fun loadFavoritesLocations(): List = listOf() } diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.kt b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.kt index eb86909c8..b7096fd64 100644 --- a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.kt +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.kt @@ -1,286 +1,49 @@ package fr.free.nrw.commons.bookmarks.locations - -import android.annotation.SuppressLint -import android.content.ContentProviderClient -import android.content.ContentValues -import android.database.Cursor -import android.database.sqlite.SQLiteDatabase -import android.database.sqlite.SQLiteException -import android.os.RemoteException -import androidx.annotation.NonNull -import fr.free.nrw.commons.location.LatLng -import fr.free.nrw.commons.nearby.Label +import androidx.room.Dao +import androidx.room.Delete +import androidx.room.Insert +import androidx.room.OnConflictStrategy +import androidx.room.Query import fr.free.nrw.commons.nearby.NearbyController import fr.free.nrw.commons.nearby.Place -import fr.free.nrw.commons.nearby.Sitelinks -import timber.log.Timber -import javax.inject.Inject -import javax.inject.Named -import javax.inject.Provider +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.flow +@Dao +abstract class BookmarkLocationsDao { -class BookmarkLocationsDao @Inject constructor( - @Named("bookmarksLocation") private val clientProvider: Provider -) { + @Insert(onConflict = OnConflictStrategy.REPLACE) + abstract suspend fun addBookmarkLocation(bookmarkLocation: BookmarksLocations) - /** - * Find all persisted location bookmarks in the database - * @return list of Place - */ - fun getAllBookmarksLocations(): List { - val items = mutableListOf() - var cursor: Cursor? = null - val db = clientProvider.get() - try { - cursor = db.query( - BookmarkLocationsContentProvider.BASE_URI, - Table.ALL_FIELDS, - null, - emptyArray(), - null + @Query("SELECT * FROM bookmarks_locations") + abstract suspend fun getAllBookmarksLocations(): List + + @Query("SELECT EXISTS (SELECT 1 FROM bookmarks_locations WHERE location_name = :name)") + abstract suspend fun findBookmarkLocation(name: String): Boolean + + @Delete + abstract suspend fun deleteBookmarkLocation(bookmarkLocation: BookmarksLocations) + + suspend fun updateBookmarkLocation(bookmarkLocation: Place): Boolean { + val bookmarkLocationExists = findBookmarkLocation(bookmarkLocation.name) + + if(bookmarkLocationExists) { + deleteBookmarkLocation( + bookmarkLocation.toBookmarksLocations() ) - cursor?.let { - while (it.moveToNext()) { - items.add(fromCursor(it)) - } - } - } catch (e: RemoteException) { - throw RuntimeException(e) - } finally { - cursor?.close() - db.release() - } - return items - } - - /** - * Look for a place in bookmarks table in order to insert or delete it - * @param bookmarkLocation : Place object - * @return boolean : is Place now fav ? - */ - fun updateBookmarkLocation(bookmarkLocation: Place): Boolean { - val bookmarkExists = findBookmarkLocation(bookmarkLocation) - if (bookmarkExists) { - deleteBookmarkLocation(bookmarkLocation) NearbyController.updateMarkerLabelListBookmark(bookmarkLocation, false) } else { - addBookmarkLocation(bookmarkLocation) + addBookmarkLocation( + bookmarkLocation.toBookmarksLocations() + ) NearbyController.updateMarkerLabelListBookmark(bookmarkLocation, true) } - return !bookmarkExists + + return !bookmarkLocationExists } - /** - * Add a Place to bookmarks table - * @param bookmarkLocation : Place to add - */ - private fun addBookmarkLocation(bookmarkLocation: Place) { - val db = clientProvider.get() - try { - db.insert(BookmarkLocationsContentProvider.BASE_URI, toContentValues(bookmarkLocation)) - } catch (e: RemoteException) { - throw RuntimeException(e) - } finally { - db.release() - } + fun getAllBookmarksLocationsPlace(): Flow> { + return flow { getAllBookmarksLocations().map { it.toPlace() } } } - - /** - * Delete a Place from bookmarks table - * @param bookmarkLocation : Place to delete - */ - private fun deleteBookmarkLocation(bookmarkLocation: Place) { - val db = clientProvider.get() - try { - db.delete( - BookmarkLocationsContentProvider.uriForName(bookmarkLocation.name), - null, - null - ) - } catch (e: RemoteException) { - throw RuntimeException(e) - } finally { - db.release() - } - } - - /** - * Find a Place from database based on its name - * @param bookmarkLocation : Place to find - * @return boolean : is Place in database ? - */ - fun findBookmarkLocation(bookmarkLocation: Place): Boolean { - var cursor: Cursor? = null - val db = clientProvider.get() - try { - cursor = db.query( - BookmarkLocationsContentProvider.BASE_URI, - Table.ALL_FIELDS, - "${Table.COLUMN_NAME}=?", - arrayOf(bookmarkLocation.name), - null - ) - return cursor?.moveToFirst() == true - } catch (e: RemoteException) { - throw RuntimeException(e) - } finally { - cursor?.close() - db.release() - } - } - - @SuppressLint("Range") - @NonNull - fun fromCursor(cursor: Cursor): Place { - val location = LatLng( - cursor.getDouble(cursor.getColumnIndex(Table.COLUMN_LAT)), - cursor.getDouble(cursor.getColumnIndex(Table.COLUMN_LONG)), - 1F - ) - - val builder = Sitelinks.Builder().apply { - setWikipediaLink(cursor.getString(cursor.getColumnIndex(Table.COLUMN_WIKIPEDIA_LINK))) - setWikidataLink(cursor.getString(cursor.getColumnIndex(Table.COLUMN_WIKIDATA_LINK))) - setCommonsLink(cursor.getString(cursor.getColumnIndex(Table.COLUMN_COMMONS_LINK))) - } - - return Place( - cursor.getString(cursor.getColumnIndex(Table.COLUMN_LANGUAGE)), - cursor.getString(cursor.getColumnIndex(Table.COLUMN_NAME)), - Label.fromText(cursor.getString(cursor.getColumnIndex(Table.COLUMN_LABEL_TEXT))), - cursor.getString(cursor.getColumnIndex(Table.COLUMN_DESCRIPTION)), - location, - cursor.getString(cursor.getColumnIndex(Table.COLUMN_CATEGORY)), - builder.build(), - cursor.getString(cursor.getColumnIndex(Table.COLUMN_PIC)), - cursor.getString(cursor.getColumnIndex(Table.COLUMN_EXISTS))?.toBoolean() ?: false - ) - } - - private fun toContentValues(bookmarkLocation: Place): ContentValues { - return ContentValues().apply { - put(Table.COLUMN_NAME, bookmarkLocation.name) - put(Table.COLUMN_LANGUAGE, bookmarkLocation.language) - put(Table.COLUMN_DESCRIPTION, bookmarkLocation.longDescription) - put(Table.COLUMN_CATEGORY, bookmarkLocation.category) - put(Table.COLUMN_LABEL_TEXT, bookmarkLocation.label?.text ?: "") - put(Table.COLUMN_LABEL_ICON, bookmarkLocation.label?.icon) - put(Table.COLUMN_WIKIPEDIA_LINK, bookmarkLocation.siteLinks.wikipediaLink.toString()) - put(Table.COLUMN_WIKIDATA_LINK, bookmarkLocation.siteLinks.wikidataLink.toString()) - put(Table.COLUMN_COMMONS_LINK, bookmarkLocation.siteLinks.commonsLink.toString()) - put(Table.COLUMN_LAT, bookmarkLocation.location.latitude) - put(Table.COLUMN_LONG, bookmarkLocation.location.longitude) - put(Table.COLUMN_PIC, bookmarkLocation.pic) - put(Table.COLUMN_EXISTS, bookmarkLocation.exists.toString()) - } - } - - object Table { - const val TABLE_NAME = "bookmarksLocations" - - const val COLUMN_NAME = "location_name" - const val COLUMN_LANGUAGE = "location_language" - const val COLUMN_DESCRIPTION = "location_description" - const val COLUMN_LAT = "location_lat" - const val COLUMN_LONG = "location_long" - const val COLUMN_CATEGORY = "location_category" - const val COLUMN_LABEL_TEXT = "location_label_text" - const val COLUMN_LABEL_ICON = "location_label_icon" - const val COLUMN_IMAGE_URL = "location_image_url" - const val COLUMN_WIKIPEDIA_LINK = "location_wikipedia_link" - const val COLUMN_WIKIDATA_LINK = "location_wikidata_link" - const val COLUMN_COMMONS_LINK = "location_commons_link" - const val COLUMN_PIC = "location_pic" - const val COLUMN_EXISTS = "location_exists" - - // NOTE! KEEP IN SAME ORDER AS THEY ARE DEFINED UP THERE. HELPS HARD CODE COLUMN INDICES. - val ALL_FIELDS = arrayOf( - COLUMN_NAME, COLUMN_LANGUAGE, COLUMN_DESCRIPTION, COLUMN_CATEGORY, COLUMN_LABEL_TEXT, COLUMN_LABEL_ICON, - COLUMN_LAT, COLUMN_LONG, COLUMN_IMAGE_URL, COLUMN_WIKIPEDIA_LINK, COLUMN_WIKIDATA_LINK, COLUMN_COMMONS_LINK, - COLUMN_PIC, COLUMN_EXISTS - ) - - const val DROP_TABLE_STATEMENT = "DROP TABLE IF EXISTS $TABLE_NAME" - - const val CREATE_TABLE_STATEMENT = """ - CREATE TABLE $TABLE_NAME ( - $COLUMN_NAME STRING PRIMARY KEY, - $COLUMN_LANGUAGE STRING, - $COLUMN_DESCRIPTION STRING, - $COLUMN_CATEGORY STRING, - $COLUMN_LABEL_TEXT STRING, - $COLUMN_LABEL_ICON INTEGER, - $COLUMN_LAT DOUBLE, - $COLUMN_LONG DOUBLE, - $COLUMN_IMAGE_URL STRING, - $COLUMN_WIKIPEDIA_LINK STRING, - $COLUMN_WIKIDATA_LINK STRING, - $COLUMN_COMMONS_LINK STRING, - $COLUMN_PIC STRING, - $COLUMN_EXISTS STRING - ); - """ - - fun onCreate(db: SQLiteDatabase) { - db.execSQL(CREATE_TABLE_STATEMENT) - } - - fun onDelete(db: SQLiteDatabase) { - db.execSQL(DROP_TABLE_STATEMENT) - onCreate(db) - } - - @SuppressLint("SQLiteString") - fun onUpdate(db: SQLiteDatabase, from: Int, to: Int) { - Timber.d("bookmarksLocations db is updated from:$from, to:$to") - var currFrom = from - - if (from == to) { - return - } - if (from < 7) { - onUpdate(db, ++currFrom, to) - return - } - if (from == 7) { - onCreate(db) - onUpdate(db, ++currFrom, to) - return - } - if (from < 10) { - onUpdate(db, ++currFrom, to) - return - } - if (from == 10) { - try { - db.execSQL("ALTER TABLE $TABLE_NAME ADD COLUMN location_pic STRING;") - } catch (exception: SQLiteException) { - Timber.e(exception) - } - return - } - if (from >= 12) { - try { - db.execSQL("ALTER TABLE $TABLE_NAME ADD COLUMN location_destroyed STRING;") - } catch (exception: SQLiteException) { - Timber.e(exception) - } - } - if (from >= 13) { - try { - db.execSQL("ALTER TABLE $TABLE_NAME ADD COLUMN location_language STRING;") - } catch (exception: SQLiteException) { - Timber.e(exception) - } - } - if (from >= 14) { - try { - db.execSQL("ALTER TABLE $TABLE_NAME ADD COLUMN location_exists STRING;") - } catch (exception: SQLiteException) { - Timber.e(exception) - } - } - } - } -} +} \ No newline at end of file diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt index 228ec2599..00538c572 100644 --- a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt @@ -8,6 +8,7 @@ import android.view.ViewGroup import androidx.activity.result.ActivityResultLauncher import androidx.activity.result.contract.ActivityResultContracts.RequestMultiplePermissions import androidx.activity.result.contract.ActivityResultContracts.StartActivityForResult +import androidx.lifecycle.lifecycleScope import androidx.recyclerview.widget.LinearLayoutManager import dagger.android.support.DaggerFragment import fr.free.nrw.commons.R @@ -108,6 +109,7 @@ class BookmarkLocationsFragment : DaggerFragment() { adapter = PlaceAdapter( bookmarkLocationDao, + scope = lifecycleScope, { }, { place, _ -> adapter.remove(place) diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsViewModel.kt b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsViewModel.kt new file mode 100644 index 000000000..2ee924423 --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsViewModel.kt @@ -0,0 +1,15 @@ +package fr.free.nrw.commons.bookmarks.locations + +import androidx.lifecycle.ViewModel +import fr.free.nrw.commons.nearby.Place +import kotlinx.coroutines.flow.Flow + +class BookmarkLocationsViewModel( + private val bookmarkLocationsDao: BookmarkLocationsDao +): ViewModel() { + + fun getAllBookmarkLocations(): Flow> { + return bookmarkLocationsDao.getAllBookmarksLocationsPlace() + } + +} \ No newline at end of file diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarksLocations.kt b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarksLocations.kt new file mode 100644 index 000000000..66d670169 --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarksLocations.kt @@ -0,0 +1,72 @@ +package fr.free.nrw.commons.bookmarks.locations + +import androidx.room.ColumnInfo +import androidx.room.Entity +import androidx.room.PrimaryKey +import fr.free.nrw.commons.location.LatLng +import fr.free.nrw.commons.nearby.Label +import fr.free.nrw.commons.nearby.Place +import fr.free.nrw.commons.nearby.Sitelinks + +@Entity(tableName = "bookmarks_locations") +data class BookmarksLocations( + @PrimaryKey @ColumnInfo(name = "location_name") val locationName: String, + @ColumnInfo(name = "location_language") val locationLanguage: String, + @ColumnInfo(name = "location_description") val locationDescription: String, + @ColumnInfo(name = "location_lat") val locationLat: Double, + @ColumnInfo(name = "location_long") val locationLong: Double, + @ColumnInfo(name = "location_category") val locationCategory: String, + @ColumnInfo(name = "location_label_text") val locationLabelText: String, + @ColumnInfo(name = "location_label_icon") val locationLabelIcon: Int?, + @ColumnInfo(name = "location_image_url") val locationImageUrl: String, + @ColumnInfo(name = "location_wikipedia_link") val locationWikipediaLink: String, + @ColumnInfo(name = "location_wikidata_link") val locationWikidataLink: String, + @ColumnInfo(name = "location_commons_link") val locationCommonsLink: String, + @ColumnInfo(name = "location_pic") val locationPic: String, + @ColumnInfo(name = "location_exists") val locationExists: Boolean +) + +fun BookmarksLocations.toPlace(): Place { + val location = LatLng( + locationLat, + locationLong, + 1F + ) + + val builder = Sitelinks.Builder().apply { + setWikipediaLink(locationWikipediaLink) + setWikidataLink(locationWikidataLink) + setCommonsLink(locationCommonsLink) + } + + return Place( + locationLanguage, + locationName, + Label.fromText(locationLabelText), + locationDescription, + location, + locationCategory, + builder.build(), + locationPic, + locationExists + ) +} + +fun Place.toBookmarksLocations(): BookmarksLocations { + return BookmarksLocations( + locationName = name, + locationLanguage = language, + locationDescription = longDescription, + locationCategory = category, + locationLat = location.latitude, + locationLong = location.longitude, + locationLabelText = label?.text ?: "", + locationLabelIcon = label?.icon, + locationImageUrl = pic, + locationWikipediaLink = siteLinks.wikipediaLink.toString(), + locationWikidataLink = siteLinks.wikidataLink.toString(), + locationCommonsLink = siteLinks.commonsLink.toString(), + locationPic = pic, + locationExists = exists + ) +} \ No newline at end of file diff --git a/app/src/main/java/fr/free/nrw/commons/data/DBOpenHelper.kt b/app/src/main/java/fr/free/nrw/commons/data/DBOpenHelper.kt index 1377ae281..660fd2a4a 100644 --- a/app/src/main/java/fr/free/nrw/commons/data/DBOpenHelper.kt +++ b/app/src/main/java/fr/free/nrw/commons/data/DBOpenHelper.kt @@ -30,7 +30,6 @@ class DBOpenHelper( override fun onCreate(db: SQLiteDatabase) { CategoryDao.Table.onCreate(db) BookmarkPicturesDao.Table.onCreate(db) - BookmarkLocationsDao.Table.onCreate(db) BookmarkItemsDao.Table.onCreate(db) RecentSearchesDao.Table.onCreate(db) RecentLanguagesDao.Table.onCreate(db) @@ -39,7 +38,6 @@ class DBOpenHelper( override fun onUpgrade(db: SQLiteDatabase, from: Int, to: Int) { CategoryDao.Table.onUpdate(db, from, to) BookmarkPicturesDao.Table.onUpdate(db, from, to) - BookmarkLocationsDao.Table.onUpdate(db, from, to) BookmarkItemsDao.Table.onUpdate(db, from, to) RecentSearchesDao.Table.onUpdate(db, from, to) RecentLanguagesDao.Table.onUpdate(db, from, to) diff --git a/app/src/main/java/fr/free/nrw/commons/db/AppDatabase.kt b/app/src/main/java/fr/free/nrw/commons/db/AppDatabase.kt index 0c34bbdec..ee4da8ee7 100644 --- a/app/src/main/java/fr/free/nrw/commons/db/AppDatabase.kt +++ b/app/src/main/java/fr/free/nrw/commons/db/AppDatabase.kt @@ -5,6 +5,8 @@ import androidx.room.RoomDatabase import androidx.room.TypeConverters import fr.free.nrw.commons.bookmarks.category.BookmarkCategoriesDao import fr.free.nrw.commons.bookmarks.category.BookmarksCategoryModal +import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao +import fr.free.nrw.commons.bookmarks.locations.BookmarksLocations import fr.free.nrw.commons.contributions.Contribution import fr.free.nrw.commons.contributions.ContributionDao import fr.free.nrw.commons.customselector.database.NotForUploadStatus @@ -23,7 +25,7 @@ import fr.free.nrw.commons.upload.depicts.DepictsDao * */ @Database( - entities = [Contribution::class, Depicts::class, UploadedStatus::class, NotForUploadStatus::class, ReviewEntity::class, Place::class, BookmarksCategoryModal::class], + entities = [Contribution::class, Depicts::class, UploadedStatus::class, NotForUploadStatus::class, ReviewEntity::class, Place::class, BookmarksCategoryModal::class, BookmarksLocations::class], version = 19, exportSchema = false, ) @@ -42,4 +44,6 @@ abstract class AppDatabase : RoomDatabase() { abstract fun ReviewDao(): ReviewDao abstract fun bookmarkCategoriesDao(): BookmarkCategoriesDao + + abstract fun bookmarkLocationsDao(): BookmarkLocationsDao } diff --git a/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.kt b/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.kt index 58d9039d5..324c8e1e5 100644 --- a/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.kt +++ b/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.kt @@ -16,6 +16,7 @@ import fr.free.nrw.commons.BuildConfig import fr.free.nrw.commons.R import fr.free.nrw.commons.auth.SessionManager import fr.free.nrw.commons.bookmarks.category.BookmarkCategoriesDao +import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao import fr.free.nrw.commons.contributions.ContributionDao import fr.free.nrw.commons.customselector.database.NotForUploadStatusDao import fr.free.nrw.commons.customselector.database.UploadedStatusDao @@ -206,6 +207,10 @@ open class CommonsApplicationModule(private val applicationContext: Context) { fun providesPlaceDao(appDatabase: AppDatabase): PlaceDao = appDatabase.PlaceDao() + @Provides + fun providesBookmarkLocationsDao(appDatabase: AppDatabase): BookmarkLocationsDao = + appDatabase.bookmarkLocationsDao() + @Provides fun providesDepictDao(appDatabase: AppDatabase): DepictsDao = appDatabase.DepictsDao() diff --git a/app/src/main/java/fr/free/nrw/commons/di/ContentProviderBuilderModule.kt b/app/src/main/java/fr/free/nrw/commons/di/ContentProviderBuilderModule.kt index 1882f77a9..80a0626cc 100644 --- a/app/src/main/java/fr/free/nrw/commons/di/ContentProviderBuilderModule.kt +++ b/app/src/main/java/fr/free/nrw/commons/di/ContentProviderBuilderModule.kt @@ -3,7 +3,6 @@ package fr.free.nrw.commons.di import dagger.Module import dagger.android.ContributesAndroidInjector import fr.free.nrw.commons.bookmarks.items.BookmarkItemsContentProvider -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsContentProvider import fr.free.nrw.commons.bookmarks.pictures.BookmarkPicturesContentProvider import fr.free.nrw.commons.category.CategoryContentProvider import fr.free.nrw.commons.explore.recentsearches.RecentSearchesContentProvider @@ -26,9 +25,6 @@ abstract class ContentProviderBuilderModule { @ContributesAndroidInjector abstract fun bindBookmarkContentProvider(): BookmarkPicturesContentProvider - @ContributesAndroidInjector - abstract fun bindBookmarkLocationContentProvider(): BookmarkLocationsContentProvider - @ContributesAndroidInjector abstract fun bindBookmarkItemContentProvider(): BookmarkItemsContentProvider diff --git a/app/src/main/java/fr/free/nrw/commons/di/FragmentBuilderModule.kt b/app/src/main/java/fr/free/nrw/commons/di/FragmentBuilderModule.kt index 0ef34e355..9920fef9f 100644 --- a/app/src/main/java/fr/free/nrw/commons/di/FragmentBuilderModule.kt +++ b/app/src/main/java/fr/free/nrw/commons/di/FragmentBuilderModule.kt @@ -101,6 +101,9 @@ abstract class FragmentBuilderModule { @ContributesAndroidInjector abstract fun bindBookmarkCategoriesListFragment(): BookmarkCategoriesFragment + @ContributesAndroidInjector + abstract fun bindBookmarkLocationsListFragment(): BookmarkLocationsFragment + @ContributesAndroidInjector abstract fun bindReviewOutOfContextFragment(): ReviewImageFragment diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyUtil.kt b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyUtil.kt new file mode 100644 index 000000000..d92796101 --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyUtil.kt @@ -0,0 +1,22 @@ +package fr.free.nrw.commons.nearby + +import androidx.lifecycle.LifecycleCoroutineScope +import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao +import fr.free.nrw.commons.bookmarks.locations.BookmarksLocations +import kotlinx.coroutines.launch + +object NearbyUtil { + + fun getBookmarkLocationExists( + bookmarksLocationsDao: BookmarkLocationsDao, + name: String, + scope: LifecycleCoroutineScope? + ): Boolean { + var isBookmarked = false + scope?.launch { + isBookmarked = bookmarksLocationsDao.findBookmarkLocation(name) + } + + return isBookmarked + } +} \ No newline at end of file diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/PlaceAdapterDelegate.kt b/app/src/main/java/fr/free/nrw/commons/nearby/PlaceAdapterDelegate.kt index 7156568b6..d9a76c25d 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/PlaceAdapterDelegate.kt +++ b/app/src/main/java/fr/free/nrw/commons/nearby/PlaceAdapterDelegate.kt @@ -7,6 +7,7 @@ import android.view.View.INVISIBLE import android.view.View.VISIBLE import android.widget.RelativeLayout import androidx.activity.result.ActivityResultLauncher +import androidx.lifecycle.LifecycleCoroutineScope import androidx.recyclerview.widget.LinearLayoutManager import androidx.recyclerview.widget.RecyclerView import androidx.transition.TransitionManager @@ -16,9 +17,11 @@ import com.hannesdorfmann.adapterdelegates4.dsl.adapterDelegateViewBinding import fr.free.nrw.commons.R import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao import fr.free.nrw.commons.databinding.ItemPlaceBinding +import kotlinx.coroutines.launch fun placeAdapterDelegate( bookmarkLocationDao: BookmarkLocationsDao, + scope: LifecycleCoroutineScope?, onItemClick: ((Place) -> Unit)? = null, onCameraClicked: (Place, ActivityResultLauncher>, ActivityResultLauncher) -> Unit, onCameraLongPressed: () -> Boolean, @@ -61,7 +64,10 @@ fun placeAdapterDelegate( nearbyButtonLayout.galleryButton.setOnClickListener { onGalleryClicked(item, galleryPickLauncherForResult) } nearbyButtonLayout.galleryButton.setOnLongClickListener { onGalleryLongPressed() } bookmarkButtonImage.setOnClickListener { - val isBookmarked = bookmarkLocationDao.updateBookmarkLocation(item) + var isBookmarked = false + scope?.launch { + isBookmarked = bookmarkLocationDao.updateBookmarkLocation(item) + } bookmarkButtonImage.setImageResource( if (isBookmarked) R.drawable.ic_round_star_filled_24px else R.drawable.ic_round_star_border_24px, ) @@ -93,13 +99,15 @@ fun placeAdapterDelegate( GONE } - bookmarkButtonImage.setImageResource( - if (bookmarkLocationDao.findBookmarkLocation(item)) { - R.drawable.ic_round_star_filled_24px - } else { - R.drawable.ic_round_star_border_24px - }, - ) + scope?.launch { + bookmarkButtonImage.setImageResource( + if (bookmarkLocationDao.findBookmarkLocation(item.name)) { + R.drawable.ic_round_star_filled_24px + } else { + R.drawable.ic_round_star_border_24px + }, + ) + } } nearbyButtonLayout.directionsButton.setOnLongClickListener { onDirectionsLongPressed() } } diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/contract/NearbyParentFragmentContract.java b/app/src/main/java/fr/free/nrw/commons/nearby/contract/NearbyParentFragmentContract.java index e46e95353..1d59fcd34 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/contract/NearbyParentFragmentContract.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/contract/NearbyParentFragmentContract.java @@ -134,7 +134,7 @@ public interface NearbyParentFragmentContract { void setAdvancedQuery(String query); - void toggleBookmarkedStatus(Place place); + void toggleBookmarkedStatus(Place place, LifecycleCoroutineScope scope); void handleMapScrolled(LifecycleCoroutineScope scope, boolean isNetworkAvailable); } diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java b/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java index f3224de7f..9ba0d8982 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java @@ -90,6 +90,7 @@ import fr.free.nrw.commons.nearby.MarkerPlaceGroup; import fr.free.nrw.commons.nearby.NearbyController; import fr.free.nrw.commons.nearby.NearbyFilterSearchRecyclerViewAdapter; import fr.free.nrw.commons.nearby.NearbyFilterState; +import fr.free.nrw.commons.nearby.NearbyUtil; import fr.free.nrw.commons.nearby.Place; import fr.free.nrw.commons.nearby.PlacesRepository; import fr.free.nrw.commons.nearby.Sitelinks; @@ -126,6 +127,7 @@ import java.util.concurrent.TimeUnit; import javax.inject.Inject; import javax.inject.Named; import kotlin.Unit; +import kotlinx.coroutines.BuildersKt; import org.jetbrains.annotations.NotNull; import org.osmdroid.api.IGeoPoint; import org.osmdroid.events.MapEventsReceiver; @@ -577,13 +579,14 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment binding.bottomSheetNearby.rvNearbyList.setLayoutManager( new LinearLayoutManager(getContext())); adapter = new PlaceAdapter(bookmarkLocationDao, + scope, place -> { moveCameraToPosition( new GeoPoint(place.location.getLatitude(), place.location.getLongitude())); return Unit.INSTANCE; }, (place, isBookmarked) -> { - presenter.toggleBookmarkedStatus(place); + presenter.toggleBookmarkedStatus(place, scope); return Unit.INSTANCE; }, commonPlaceClickActions, @@ -2135,7 +2138,11 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment private void updateBookmarkButtonImage(final Place place) { final int bookmarkIcon; - if (bookmarkLocationDao.findBookmarkLocation(place)) { + if (NearbyUtil.INSTANCE.getBookmarkLocationExists( + bookmarkLocationDao, + place.getName(), + scope + )) { bookmarkIcon = R.drawable.ic_round_star_filled_24px; } else { bookmarkIcon = R.drawable.ic_round_star_border_24px; @@ -2301,11 +2308,11 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment BottomSheetItem item = dataList.get(position); switch (item.getImageResourceId()) { case R.drawable.ic_round_star_border_24px: - presenter.toggleBookmarkedStatus(selectedPlace); + presenter.toggleBookmarkedStatus(selectedPlace, scope); updateBookmarkButtonImage(selectedPlace); break; case R.drawable.ic_round_star_filled_24px: - presenter.toggleBookmarkedStatus(selectedPlace); + presenter.toggleBookmarkedStatus(selectedPlace, scope); updateBookmarkButtonImage(selectedPlace); break; case R.drawable.ic_directions_black_24dp: diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/fragments/PlaceAdapter.kt b/app/src/main/java/fr/free/nrw/commons/nearby/fragments/PlaceAdapter.kt index e5cc92667..adb687014 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/fragments/PlaceAdapter.kt +++ b/app/src/main/java/fr/free/nrw/commons/nearby/fragments/PlaceAdapter.kt @@ -2,6 +2,7 @@ package fr.free.nrw.commons.nearby.fragments import android.content.Intent import androidx.activity.result.ActivityResultLauncher +import androidx.lifecycle.LifecycleCoroutineScope import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao import fr.free.nrw.commons.nearby.Place import fr.free.nrw.commons.nearby.placeAdapterDelegate @@ -9,6 +10,7 @@ import fr.free.nrw.commons.upload.categories.BaseDelegateAdapter class PlaceAdapter( bookmarkLocationsDao: BookmarkLocationsDao, + scope: LifecycleCoroutineScope? = null, onPlaceClicked: ((Place) -> Unit)? = null, onBookmarkClicked: (Place, Boolean) -> Unit, commonPlaceClickActions: CommonPlaceClickActions, @@ -18,6 +20,7 @@ class PlaceAdapter( ) : BaseDelegateAdapter( placeAdapterDelegate( bookmarkLocationsDao, + scope, onPlaceClicked, commonPlaceClickActions.onCameraClicked(), commonPlaceClickActions.onCameraLongPressed(), diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/presenter/NearbyParentFragmentPresenter.kt b/app/src/main/java/fr/free/nrw/commons/nearby/presenter/NearbyParentFragmentPresenter.kt index ce268f7a3..2a5b2f67d 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/presenter/NearbyParentFragmentPresenter.kt +++ b/app/src/main/java/fr/free/nrw/commons/nearby/presenter/NearbyParentFragmentPresenter.kt @@ -26,6 +26,7 @@ import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.delay import kotlinx.coroutines.ensureActive import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withContext import timber.log.Timber import java.lang.reflect.InvocationHandler @@ -133,16 +134,25 @@ class NearbyParentFragmentPresenter * @param place The place whose bookmarked status is to be toggled. If the place is `null`, * the operation is skipped. */ - override fun toggleBookmarkedStatus(place: Place?) { + override fun toggleBookmarkedStatus( + place: Place?, + scope: LifecycleCoroutineScope? + ) { if (place == null) return - val nowBookmarked = bookmarkLocationDao.updateBookmarkLocation(place) + var nowBookmarked: Boolean? = null + scope?.launch { + nowBookmarked = bookmarkLocationDao.updateBookmarkLocation(place) + + } bookmarkChangedPlaces.add(place) val placeIndex = NearbyController.markerLabelList.indexOfFirst { it.place.location == place.location } - NearbyController.markerLabelList[placeIndex] = MarkerPlaceGroup( - nowBookmarked, - NearbyController.markerLabelList[placeIndex].place - ) + NearbyController.markerLabelList[placeIndex] = nowBookmarked?.let { + MarkerPlaceGroup( + it, + NearbyController.markerLabelList[placeIndex].place + ) + } nearbyParentFragmentView.setFilterState() } @@ -334,7 +344,7 @@ class NearbyParentFragmentPresenter for (i in 0..updatedGroups.lastIndex) { val repoPlace = placesRepository.fetchPlace(updatedGroups[i].place.entityID) if (repoPlace != null && repoPlace.name != null && repoPlace.name != ""){ - updatedGroups[i].isBookmarked = bookmarkLocationDao.findBookmarkLocation(repoPlace) + updatedGroups[i].isBookmarked = bookmarkLocationDao.findBookmarkLocation(repoPlace.name) updatedGroups[i].place.apply { name = repoPlace.name isMonument = repoPlace.isMonument @@ -372,7 +382,7 @@ class NearbyParentFragmentPresenter collectResults.send( fetchedPlaces.mapIndexed { index, place -> Pair(indices[index], MarkerPlaceGroup( - bookmarkLocationDao.findBookmarkLocation(place), + bookmarkLocationDao.findBookmarkLocation(place.name), place )) } @@ -435,7 +445,7 @@ class NearbyParentFragmentPresenter if (bookmarkChangedPlacesBacklog.containsKey(group.place.location)) { updatedGroups[index] = MarkerPlaceGroup( bookmarkLocationDao - .findBookmarkLocation(updatedGroups[index].place), + .findBookmarkLocation(updatedGroups[index].place.name), updatedGroups[index].place ) } @@ -545,7 +555,7 @@ class NearbyParentFragmentPresenter ).sortedBy { it.getDistanceInDouble(mapFocus) }.take(NearbyController.MAX_RESULTS) .map { MarkerPlaceGroup( - bookmarkLocationDao.findBookmarkLocation(it), it + bookmarkLocationDao.findBookmarkLocation(it.name), it ) } ensureActive() From 8295edeb0af5b5ac05a6c7350b542e9960dc8f00 Mon Sep 17 00:00:00 2001 From: Saifuddin Date: Fri, 17 Jan 2025 17:38:45 +0530 Subject: [PATCH 04/11] Refactor: Improve bookmark location handling and update database version This commit includes the following changes: - Updates the database version to 20. - Refactors bookmark location handling within `NearbyParentFragment` to improve logic and efficiency. - Introduces `getBookmarkLocationExists` in `NearbyUtil` to directly update bookmark icons in the bottom sheet adapter. - Removes unused provider. - Adjusts `BookmarkLocationsFragment`'s `PlaceAdapter` to use `lifecycleScope` for better coroutine management. - Refactors `updateBookmarkLocation` in `NearbyParentFragmentPresenter`. --- app/src/main/AndroidManifest.xml | 6 ------ .../locations/BookmarkLocationsFragment.kt | 2 +- .../fr/free/nrw/commons/db/AppDatabase.kt | 2 +- .../commons/di/CommonsApplicationModule.kt | 5 ----- .../nrw/commons/di/FragmentBuilderModule.kt | 3 --- .../fr/free/nrw/commons/nearby/NearbyUtil.kt | 19 ++++++++++------- .../fragments/NearbyParentFragment.java | 13 ++++-------- .../NearbyParentFragmentPresenter.kt | 21 ++++++++----------- 8 files changed, 27 insertions(+), 44 deletions(-) diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index fb776920e..d56a874b5 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -232,12 +232,6 @@ android:exported="false" android:label="@string/provider_bookmarks" android:syncable="false" /> - adapter.remove(place) diff --git a/app/src/main/java/fr/free/nrw/commons/db/AppDatabase.kt b/app/src/main/java/fr/free/nrw/commons/db/AppDatabase.kt index ee4da8ee7..d51e53dbe 100644 --- a/app/src/main/java/fr/free/nrw/commons/db/AppDatabase.kt +++ b/app/src/main/java/fr/free/nrw/commons/db/AppDatabase.kt @@ -26,7 +26,7 @@ import fr.free.nrw.commons.upload.depicts.DepictsDao */ @Database( entities = [Contribution::class, Depicts::class, UploadedStatus::class, NotForUploadStatus::class, ReviewEntity::class, Place::class, BookmarksCategoryModal::class, BookmarksLocations::class], - version = 19, + version = 20, exportSchema = false, ) @TypeConverters(Converters::class) diff --git a/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.kt b/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.kt index 324c8e1e5..b25998ee5 100644 --- a/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.kt +++ b/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.kt @@ -111,11 +111,6 @@ open class CommonsApplicationModule(private val applicationContext: Context) { fun provideBookmarkContentProviderClient(context: Context): ContentProviderClient? = context.contentResolver.acquireContentProviderClient(BuildConfig.BOOKMARK_AUTHORITY) - @Provides - @Named("bookmarksLocation") - fun provideBookmarkLocationContentProviderClient(context: Context): ContentProviderClient? = - context.contentResolver.acquireContentProviderClient(BuildConfig.BOOKMARK_LOCATIONS_AUTHORITY) - @Provides @Named("bookmarksItem") fun provideBookmarkItemContentProviderClient(context: Context): ContentProviderClient? = diff --git a/app/src/main/java/fr/free/nrw/commons/di/FragmentBuilderModule.kt b/app/src/main/java/fr/free/nrw/commons/di/FragmentBuilderModule.kt index 9920fef9f..0ef34e355 100644 --- a/app/src/main/java/fr/free/nrw/commons/di/FragmentBuilderModule.kt +++ b/app/src/main/java/fr/free/nrw/commons/di/FragmentBuilderModule.kt @@ -101,9 +101,6 @@ abstract class FragmentBuilderModule { @ContributesAndroidInjector abstract fun bindBookmarkCategoriesListFragment(): BookmarkCategoriesFragment - @ContributesAndroidInjector - abstract fun bindBookmarkLocationsListFragment(): BookmarkLocationsFragment - @ContributesAndroidInjector abstract fun bindReviewOutOfContextFragment(): ReviewImageFragment diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyUtil.kt b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyUtil.kt index d92796101..d844c7c21 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyUtil.kt +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyUtil.kt @@ -1,8 +1,9 @@ package fr.free.nrw.commons.nearby +import android.util.Log import androidx.lifecycle.LifecycleCoroutineScope +import fr.free.nrw.commons.R import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao -import fr.free.nrw.commons.bookmarks.locations.BookmarksLocations import kotlinx.coroutines.launch object NearbyUtil { @@ -10,13 +11,17 @@ object NearbyUtil { fun getBookmarkLocationExists( bookmarksLocationsDao: BookmarkLocationsDao, name: String, - scope: LifecycleCoroutineScope? - ): Boolean { - var isBookmarked = false + scope: LifecycleCoroutineScope?, + bottomSheetAdapter: BottomSheetAdapter, + ) { scope?.launch { - isBookmarked = bookmarksLocationsDao.findBookmarkLocation(name) + val isBookmarked = bookmarksLocationsDao.findBookmarkLocation(name) + Log.d("isBookmarked", isBookmarked.toString()) + if (isBookmarked) { + bottomSheetAdapter.updateBookmarkIcon(R.drawable.ic_round_star_filled_24px) + } else { + bottomSheetAdapter.updateBookmarkIcon(R.drawable.ic_round_star_border_24px) + } } - - return isBookmarked } } \ No newline at end of file diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java b/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java index 9ba0d8982..bba19b181 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java @@ -2137,17 +2137,12 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment } private void updateBookmarkButtonImage(final Place place) { - final int bookmarkIcon; - if (NearbyUtil.INSTANCE.getBookmarkLocationExists( + NearbyUtil.INSTANCE.getBookmarkLocationExists( bookmarkLocationDao, place.getName(), - scope - )) { - bookmarkIcon = R.drawable.ic_round_star_filled_24px; - } else { - bookmarkIcon = R.drawable.ic_round_star_border_24px; - } - bottomSheetAdapter.updateBookmarkIcon(bookmarkIcon); + scope, + bottomSheetAdapter + ); } @Override diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/presenter/NearbyParentFragmentPresenter.kt b/app/src/main/java/fr/free/nrw/commons/nearby/presenter/NearbyParentFragmentPresenter.kt index 2a5b2f67d..7b6292e28 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/presenter/NearbyParentFragmentPresenter.kt +++ b/app/src/main/java/fr/free/nrw/commons/nearby/presenter/NearbyParentFragmentPresenter.kt @@ -139,29 +139,26 @@ class NearbyParentFragmentPresenter scope: LifecycleCoroutineScope? ) { if (place == null) return - var nowBookmarked: Boolean? = null + var nowBookmarked: Boolean scope?.launch { nowBookmarked = bookmarkLocationDao.updateBookmarkLocation(place) - - } - bookmarkChangedPlaces.add(place) - val placeIndex = - NearbyController.markerLabelList.indexOfFirst { it.place.location == place.location } - NearbyController.markerLabelList[placeIndex] = nowBookmarked?.let { - MarkerPlaceGroup( - it, + bookmarkChangedPlaces.add(place) + val placeIndex = + NearbyController.markerLabelList.indexOfFirst { it.place.location == place.location } + NearbyController.markerLabelList[placeIndex] = MarkerPlaceGroup( + nowBookmarked, NearbyController.markerLabelList[placeIndex].place ) + nearbyParentFragmentView.setFilterState() } - nearbyParentFragmentView.setFilterState() } override fun attachView(view: NearbyParentFragmentContract.View) { - this.nearbyParentFragmentView = view + nearbyParentFragmentView = view } override fun detachView() { - this.nearbyParentFragmentView = DUMMY + nearbyParentFragmentView = DUMMY } override fun removeNearbyPreferences(applicationKvStore: JsonKvStore) { From 97037858a360bd0fd000fdde4ea1592ee766f0ff Mon Sep 17 00:00:00 2001 From: Saifuddin Date: Fri, 17 Jan 2025 18:26:42 +0530 Subject: [PATCH 05/11] Toggle bookmark icon in BottomSheetAdapter instead of finding the location each time in the bookmark * Update bookmark button image in `BottomSheetAdapter` * Add new toggle function to `BottomSheetAdapter` * Call the toggle function in `NearbyParentFragment` --- .../nrw/commons/nearby/BottomSheetAdapter.kt | 16 +++++++++++++++- .../nearby/fragments/NearbyParentFragment.java | 8 ++++++-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/BottomSheetAdapter.kt b/app/src/main/java/fr/free/nrw/commons/nearby/BottomSheetAdapter.kt index 8bcc21e40..a83d49f75 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/BottomSheetAdapter.kt +++ b/app/src/main/java/fr/free/nrw/commons/nearby/BottomSheetAdapter.kt @@ -68,7 +68,21 @@ class BottomSheetAdapter( item.imageResourceId == R.drawable.ic_round_star_border_24px ) { item.imageResourceId = icon - this.notifyItemChanged(index) + notifyItemChanged(index) + return + } + } + } + + fun toggleBookmarkIcon() { + itemList.forEachIndexed { index, item -> + if(item.imageResourceId == R.drawable.ic_round_star_filled_24px) { + item.imageResourceId = R.drawable.ic_round_star_border_24px + notifyItemChanged(index) + return + } else if(item.imageResourceId == R.drawable.ic_round_star_border_24px){ + item.imageResourceId = R.drawable.ic_round_star_filled_24px + notifyItemChanged(index) return } } diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java b/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java index bba19b181..05a662c71 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java @@ -2145,6 +2145,10 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment ); } + private void toggleBookmarkButtonImage() { + bottomSheetAdapter.toggleBookmarkIcon(); + } + @Override public void onAttach(final Context context) { super.onAttach(context); @@ -2304,11 +2308,11 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment switch (item.getImageResourceId()) { case R.drawable.ic_round_star_border_24px: presenter.toggleBookmarkedStatus(selectedPlace, scope); - updateBookmarkButtonImage(selectedPlace); + toggleBookmarkButtonImage(); break; case R.drawable.ic_round_star_filled_24px: presenter.toggleBookmarkedStatus(selectedPlace, scope); - updateBookmarkButtonImage(selectedPlace); + toggleBookmarkButtonImage(); break; case R.drawable.ic_directions_black_24dp: Utils.handleGeoCoordinates(this.getContext(), selectedPlace.getLocation()); From 8d8b722d30df0d0fda9cb46b50af59bc7a9ca513 Mon Sep 17 00:00:00 2001 From: Saifuddin Date: Sat, 18 Jan 2025 11:33:16 +0530 Subject: [PATCH 06/11] Refactor: Load bookmarked locations using Flow * `BookmarkLocationsController`: Changed to use `Flow` to load bookmarked locations. * `BookmarkLocationsDao`: Changed to return `Flow` of bookmark location instead of a list. * `BookmarkLocationsFragment`: Used `LifecycleScope` to collect data from flow and display it. * Removed unused `getAllBookmarkLocations()` from `BookmarkLocationsViewModel`. * Used `map` in `BookmarkLocationsDao` to convert from `BookmarksLocations` to `Place` list. --- .../bookmarks/locations/BookmarkLocationsController.kt | 5 ++++- .../bookmarks/locations/BookmarkLocationsDao.kt | 5 +++-- .../bookmarks/locations/BookmarkLocationsFragment.kt | 10 +++++++++- .../bookmarks/locations/BookmarkLocationsViewModel.kt | 6 +++--- 4 files changed, 19 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsController.kt b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsController.kt index 3509f4e73..7739c20a7 100644 --- a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsController.kt +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsController.kt @@ -1,6 +1,8 @@ package fr.free.nrw.commons.bookmarks.locations import fr.free.nrw.commons.nearby.Place +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.flow import javax.inject.Inject import javax.inject.Singleton @@ -13,5 +15,6 @@ class BookmarkLocationsController @Inject constructor( * Load bookmarked locations from the database. * @return a list of Place objects. */ - fun loadFavoritesLocations(): List = listOf() + fun loadFavoritesLocations(): Flow> = + bookmarkLocationDao.getAllBookmarksLocationsPlace() } diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.kt b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.kt index b7096fd64..d2c0a6fe8 100644 --- a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.kt +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.kt @@ -9,6 +9,7 @@ import fr.free.nrw.commons.nearby.NearbyController import fr.free.nrw.commons.nearby.Place import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.flow.map @Dao abstract class BookmarkLocationsDao { @@ -17,7 +18,7 @@ abstract class BookmarkLocationsDao { abstract suspend fun addBookmarkLocation(bookmarkLocation: BookmarksLocations) @Query("SELECT * FROM bookmarks_locations") - abstract suspend fun getAllBookmarksLocations(): List + abstract fun getAllBookmarksLocations(): Flow> @Query("SELECT EXISTS (SELECT 1 FROM bookmarks_locations WHERE location_name = :name)") abstract suspend fun findBookmarkLocation(name: String): Boolean @@ -44,6 +45,6 @@ abstract class BookmarkLocationsDao { } fun getAllBookmarksLocationsPlace(): Flow> { - return flow { getAllBookmarksLocations().map { it.toPlace() } } + return flow { getAllBookmarksLocations().map { it.map { it1 -> it1.toPlace() } } } } } \ No newline at end of file diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt index 64213ae1d..e334d8bbf 100644 --- a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt @@ -15,8 +15,10 @@ import fr.free.nrw.commons.R import fr.free.nrw.commons.contributions.ContributionController import fr.free.nrw.commons.databinding.FragmentBookmarksLocationsBinding import fr.free.nrw.commons.filepicker.FilePicker +import fr.free.nrw.commons.nearby.Place import fr.free.nrw.commons.nearby.fragments.CommonPlaceClickActions import fr.free.nrw.commons.nearby.fragments.PlaceAdapter +import kotlinx.coroutines.launch import javax.inject.Inject @@ -128,7 +130,13 @@ class BookmarkLocationsFragment : DaggerFragment() { } private fun initList() { - val places = controller.loadFavoritesLocations() + var places: List = listOf() + viewLifecycleOwner.lifecycleScope.launch { + controller.loadFavoritesLocations().collect { + adapter.items = it + places = it + } + } adapter.items = places binding?.loadingImagesProgressBar?.visibility = View.GONE if (places.isEmpty()) { diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsViewModel.kt b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsViewModel.kt index 2ee924423..b22723c0f 100644 --- a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsViewModel.kt +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsViewModel.kt @@ -8,8 +8,8 @@ class BookmarkLocationsViewModel( private val bookmarkLocationsDao: BookmarkLocationsDao ): ViewModel() { - fun getAllBookmarkLocations(): Flow> { - return bookmarkLocationsDao.getAllBookmarksLocationsPlace() - } +// fun getAllBookmarkLocations(): List { +// return bookmarkLocationsDao.getAllBookmarksLocationsPlace() +// } } \ No newline at end of file From 4bf00b8b2d1d0b7dc90de281c402d07ec662e04d Mon Sep 17 00:00:00 2001 From: Saifuddin Date: Sat, 18 Jan 2025 12:43:28 +0530 Subject: [PATCH 07/11] Loading locations data in fragment * BookmarkLocationsController: Changed `loadFavoritesLocations` to be a suspend function. * BookmarkLocationsFragment: Updated the `initList` function to call the controller's suspend function. * BookmarkLocationsDao: Changed `getAllBookmarksLocations` and `getAllBookmarksLocationsPlace` to be suspend functions. --- .../locations/BookmarkLocationsController.kt | 2 +- .../bookmarks/locations/BookmarkLocationsDao.kt | 6 +++--- .../bookmarks/locations/BookmarkLocationsFragment.kt | 11 ++++++----- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsController.kt b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsController.kt index 7739c20a7..81ec80214 100644 --- a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsController.kt +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsController.kt @@ -15,6 +15,6 @@ class BookmarkLocationsController @Inject constructor( * Load bookmarked locations from the database. * @return a list of Place objects. */ - fun loadFavoritesLocations(): Flow> = + suspend fun loadFavoritesLocations(): List = bookmarkLocationDao.getAllBookmarksLocationsPlace() } diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.kt b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.kt index d2c0a6fe8..025655daf 100644 --- a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.kt +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.kt @@ -18,7 +18,7 @@ abstract class BookmarkLocationsDao { abstract suspend fun addBookmarkLocation(bookmarkLocation: BookmarksLocations) @Query("SELECT * FROM bookmarks_locations") - abstract fun getAllBookmarksLocations(): Flow> + abstract suspend fun getAllBookmarksLocations(): List @Query("SELECT EXISTS (SELECT 1 FROM bookmarks_locations WHERE location_name = :name)") abstract suspend fun findBookmarkLocation(name: String): Boolean @@ -44,7 +44,7 @@ abstract class BookmarkLocationsDao { return !bookmarkLocationExists } - fun getAllBookmarksLocationsPlace(): Flow> { - return flow { getAllBookmarksLocations().map { it.map { it1 -> it1.toPlace() } } } + suspend fun getAllBookmarksLocationsPlace(): List { + return getAllBookmarksLocations().map { it.toPlace() } } } \ No newline at end of file diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt index e334d8bbf..a51d02b10 100644 --- a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt @@ -130,13 +130,14 @@ class BookmarkLocationsFragment : DaggerFragment() { } private fun initList() { - var places: List = listOf() + var places: List viewLifecycleOwner.lifecycleScope.launch { - controller.loadFavoritesLocations().collect { - adapter.items = it - places = it - } + places = controller.loadFavoritesLocations() + updateUIList(places) } + } + + private fun updateUIList(places: List) { adapter.items = places binding?.loadingImagesProgressBar?.visibility = View.GONE if (places.isEmpty()) { From f86af685086259a149134a5aa9e32aee4279426e Mon Sep 17 00:00:00 2001 From: Saifuddin Date: Mon, 20 Jan 2025 11:50:33 +0530 Subject: [PATCH 08/11] Refactor BookmarkLocationControllerTest and related files to use coroutines * Migrated `bookmarkDao!!.getAllBookmarksLocations()` to `bookmarkDao!!.getAllBookmarksLocationsPlace()` and added `runBlocking` * Added `runBlocking` for `loadBookmarkedLocations()` and `testInitNonEmpty()` * Added `runBlocking` for `getAllBookmarksLocations()` and update `updateMapMarkers` These changes improve the test structure by ensuring the database functions are executed within a coroutine context. --- .../locations/BookmarkLocationsDao.kt | 41 ++- .../locations/BookMarkLocationDaoTest.kt | 312 +++--------------- .../BookmarkLocationControllerTest.kt | 11 +- .../BookmarkLocationFragmentUnitTests.kt | 5 +- .../NearbyParentFragmentPresenterTest.kt | 5 +- 5 files changed, 83 insertions(+), 291 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.kt b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.kt index 025655daf..2fa65b2d9 100644 --- a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.kt +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.kt @@ -7,44 +7,59 @@ import androidx.room.OnConflictStrategy import androidx.room.Query import fr.free.nrw.commons.nearby.NearbyController import fr.free.nrw.commons.nearby.Place -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.flow -import kotlinx.coroutines.flow.map +/** + * DAO for managing bookmark locations in the database. + */ @Dao abstract class BookmarkLocationsDao { + /** + * Adds or updates a bookmark location in the database. + */ @Insert(onConflict = OnConflictStrategy.REPLACE) abstract suspend fun addBookmarkLocation(bookmarkLocation: BookmarksLocations) + /** + * Fetches all bookmark locations from the database. + */ @Query("SELECT * FROM bookmarks_locations") abstract suspend fun getAllBookmarksLocations(): List + /** + * Checks if a bookmark location exists by name. + */ @Query("SELECT EXISTS (SELECT 1 FROM bookmarks_locations WHERE location_name = :name)") abstract suspend fun findBookmarkLocation(name: String): Boolean + /** + * Deletes a bookmark location from the database. + */ @Delete abstract suspend fun deleteBookmarkLocation(bookmarkLocation: BookmarksLocations) + /** + * Adds or removes a bookmark location and updates markers. + * @return `true` if added, `false` if removed. + */ suspend fun updateBookmarkLocation(bookmarkLocation: Place): Boolean { - val bookmarkLocationExists = findBookmarkLocation(bookmarkLocation.name) + val exists = findBookmarkLocation(bookmarkLocation.name) - if(bookmarkLocationExists) { - deleteBookmarkLocation( - bookmarkLocation.toBookmarksLocations() - ) + if (exists) { + deleteBookmarkLocation(bookmarkLocation.toBookmarksLocations()) NearbyController.updateMarkerLabelListBookmark(bookmarkLocation, false) } else { - addBookmarkLocation( - bookmarkLocation.toBookmarksLocations() - ) + addBookmarkLocation(bookmarkLocation.toBookmarksLocations()) NearbyController.updateMarkerLabelListBookmark(bookmarkLocation, true) } - return !bookmarkLocationExists + return !exists } + /** + * Fetches all bookmark locations as `Place` objects. + */ suspend fun getAllBookmarksLocationsPlace(): List { return getAllBookmarksLocations().map { it.toPlace() } } -} \ No newline at end of file +} diff --git a/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookMarkLocationDaoTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookMarkLocationDaoTest.kt index e677bde5d..f1c3f9798 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookMarkLocationDaoTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookMarkLocationDaoTest.kt @@ -7,6 +7,8 @@ import android.database.MatrixCursor import android.database.sqlite.SQLiteDatabase import android.net.Uri import android.os.RemoteException +import androidx.room.Room +import androidx.test.core.app.ApplicationProvider import com.nhaarman.mockitokotlin2.any import com.nhaarman.mockitokotlin2.anyOrNull import com.nhaarman.mockitokotlin2.argumentCaptor @@ -18,35 +20,20 @@ import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.verify import com.nhaarman.mockitokotlin2.whenever import fr.free.nrw.commons.TestCommonsApplication -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.COLUMN_CATEGORY -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.COLUMN_COMMONS_LINK -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.COLUMN_DESCRIPTION -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.COLUMN_EXISTS -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.COLUMN_IMAGE_URL -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.COLUMN_LABEL_ICON -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.COLUMN_LABEL_TEXT -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.COLUMN_LANGUAGE -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.COLUMN_LAT -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.COLUMN_LONG -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.COLUMN_NAME -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.COLUMN_PIC -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.COLUMN_WIKIDATA_LINK -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.COLUMN_WIKIPEDIA_LINK -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.CREATE_TABLE_STATEMENT -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.DROP_TABLE_STATEMENT -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.onCreate -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.onDelete -import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao.Table.onUpdate +import fr.free.nrw.commons.db.AppDatabase import fr.free.nrw.commons.location.LatLng import fr.free.nrw.commons.nearby.Label import fr.free.nrw.commons.nearby.Place import fr.free.nrw.commons.nearby.Sitelinks +import kotlinx.coroutines.runBlocking +import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import org.mockito.Mock import org.mockito.Mockito.verifyNoInteractions import org.robolectric.RobolectricTestRunner import org.robolectric.annotation.Config @@ -54,28 +41,12 @@ import org.robolectric.annotation.Config @RunWith(RobolectricTestRunner::class) @Config(sdk = [21], application = TestCommonsApplication::class) class BookMarkLocationDaoTest { - private val columns = - arrayOf( - COLUMN_NAME, - COLUMN_LANGUAGE, - COLUMN_DESCRIPTION, - COLUMN_CATEGORY, - COLUMN_LABEL_TEXT, - COLUMN_LABEL_ICON, - COLUMN_IMAGE_URL, - COLUMN_WIKIPEDIA_LINK, - COLUMN_WIKIDATA_LINK, - COLUMN_COMMONS_LINK, - COLUMN_LAT, - COLUMN_LONG, - COLUMN_PIC, - COLUMN_EXISTS, - ) - private val client: ContentProviderClient = mock() - private val database: SQLiteDatabase = mock() - private val captor = argumentCaptor() - private lateinit var testObject: BookmarkLocationsDao + @Mock + var bookmarkLocationsDao: BookmarkLocationsDao? = null + + private lateinit var database: AppDatabase + private lateinit var examplePlaceBookmark: Place private lateinit var exampleLabel: Label private lateinit var exampleUri: Uri @@ -88,10 +59,18 @@ class BookMarkLocationDaoTest { exampleUri = Uri.parse("wikimedia/uri") exampleLocation = LatLng(40.0, 51.4, 1f) - builder = Sitelinks.Builder() - builder.setWikipediaLink("wikipediaLink") - builder.setWikidataLink("wikidataLink") - builder.setCommonsLink("commonsLink") + database = Room.inMemoryDatabaseBuilder( + ApplicationProvider.getApplicationContext(), + AppDatabase::class.java + ).allowMainThreadQueries().build() + + bookmarkLocationsDao = database.bookmarkLocationsDao() + + builder = Sitelinks.Builder().apply { + setWikipediaLink("wikipediaLink") + setWikidataLink("wikidataLink") + setCommonsLink("commonsLink") + } examplePlaceBookmark = Place( @@ -105,236 +84,25 @@ class BookMarkLocationDaoTest { "picName", false, ) - testObject = BookmarkLocationsDao { client } + } + + @After + fun tearDown() { + database.close() } @Test - fun createTable() { - onCreate(database) - verify(database).execSQL(CREATE_TABLE_STATEMENT) + fun insertAndRetrieveBookmark() = runBlocking { + // Insert a bookmark + bookmarkLocationsDao?.addBookmarkLocation(examplePlaceBookmark.toBookmarksLocations()) + + // Retrieve all bookmarks + val bookmarks = bookmarkLocationsDao?.getAllBookmarksLocations() + + // Assert the bookmark exists + assertEquals(1, bookmarks?.size) + val retrievedBookmark = bookmarks?.first() + assertEquals(examplePlaceBookmark.name, retrievedBookmark?.locationName) + assertEquals(examplePlaceBookmark.language, retrievedBookmark?.locationLanguage) } - - @Test - fun deleteTable() { - onDelete(database) - inOrder(database) { - verify(database).execSQL(DROP_TABLE_STATEMENT) - verify(database).execSQL(CREATE_TABLE_STATEMENT) - } - } - - @Test - fun createFromCursor() { - createCursor(1).let { cursor -> - cursor.moveToFirst() - testObject.fromCursor(cursor).let { - assertEquals("en", it.language) - assertEquals("placeName", it.name) - assertEquals(Label.FOREST, it.label) - assertEquals("placeDescription", it.longDescription) - assertEquals(40.0, it.location.latitude, 0.001) - assertEquals(51.4, it.location.longitude, 0.001) - assertEquals("placeCategory", it.category) - assertEquals(builder.build().wikipediaLink, it.siteLinks.wikipediaLink) - assertEquals(builder.build().wikidataLink, it.siteLinks.wikidataLink) - assertEquals(builder.build().commonsLink, it.siteLinks.commonsLink) - assertEquals("picName", it.pic) - assertEquals(false, it.exists) - } - } - } - - @Test - fun getAllLocationBookmarks() { - whenever(client.query(any(), any(), anyOrNull(), any(), anyOrNull())).thenReturn(createCursor(14)) - - var result = testObject.getAllBookmarksLocations() - - assertEquals(14, result.size) - } - - @Test(expected = RuntimeException::class) - fun getAllLocationBookmarksTranslatesExceptions() { - whenever(client.query(any(), any(), anyOrNull(), any(), anyOrNull())).thenThrow(RemoteException("")) - testObject.getAllBookmarksLocations() - } - - @Test - fun getAllLocationBookmarksReturnsEmptyList_emptyCursor() { - whenever(client.query(any(), any(), anyOrNull(), any(), anyOrNull())).thenReturn(createCursor(0)) - assertTrue(testObject.getAllBookmarksLocations().isEmpty()) - } - - @Test - fun getAllLocationBookmarksReturnsEmptyList_nullCursor() { - whenever(client.query(any(), any(), anyOrNull(), any(), anyOrNull())).thenReturn(null) - assertTrue(testObject.getAllBookmarksLocations().isEmpty()) - } - - @Test - fun cursorsAreClosedAfterGetAllLocationBookmarksQuery() { - val mockCursor: Cursor = mock() - whenever(client.query(any(), any(), anyOrNull(), any(), anyOrNull())).thenReturn(mockCursor) - whenever(mockCursor.moveToFirst()).thenReturn(false) - - testObject.getAllBookmarksLocations() - - verify(mockCursor).close() - } - - @Test - fun updateNewLocationBookmark() { - whenever(client.insert(any(), any())).thenReturn(Uri.EMPTY) - whenever(client.query(any(), any(), any(), any(), anyOrNull())).thenReturn(null) - - assertTrue(testObject.updateBookmarkLocation(examplePlaceBookmark)) - verify(client).insert(eq(BookmarkLocationsContentProvider.BASE_URI), captor.capture()) - captor.firstValue.let { cv -> - assertEquals(13, cv.size()) - assertEquals(examplePlaceBookmark.name, cv.getAsString(COLUMN_NAME)) - assertEquals(examplePlaceBookmark.language, cv.getAsString(COLUMN_LANGUAGE)) - assertEquals(examplePlaceBookmark.longDescription, cv.getAsString(COLUMN_DESCRIPTION)) - assertEquals(examplePlaceBookmark.label.text, cv.getAsString(COLUMN_LABEL_TEXT)) - assertEquals(examplePlaceBookmark.category, cv.getAsString(COLUMN_CATEGORY)) - assertEquals(examplePlaceBookmark.location.latitude, cv.getAsDouble(COLUMN_LAT), 0.001) - assertEquals(examplePlaceBookmark.location.longitude, cv.getAsDouble(COLUMN_LONG), 0.001) - assertEquals(examplePlaceBookmark.siteLinks.wikipediaLink.toString(), cv.getAsString(COLUMN_WIKIPEDIA_LINK)) - assertEquals(examplePlaceBookmark.siteLinks.wikidataLink.toString(), cv.getAsString(COLUMN_WIKIDATA_LINK)) - assertEquals(examplePlaceBookmark.siteLinks.commonsLink.toString(), cv.getAsString(COLUMN_COMMONS_LINK)) - assertEquals(examplePlaceBookmark.pic, cv.getAsString(COLUMN_PIC)) - assertEquals(examplePlaceBookmark.exists.toString(), cv.getAsString(COLUMN_EXISTS)) - } - } - - @Test - fun updateExistingLocationBookmark() { - whenever(client.delete(isA(), isNull(), isNull())).thenReturn(1) - whenever(client.query(any(), any(), any(), any(), anyOrNull())).thenReturn(createCursor(1)) - - assertFalse(testObject.updateBookmarkLocation(examplePlaceBookmark)) - verify(client).delete(eq(BookmarkLocationsContentProvider.uriForName(examplePlaceBookmark.name)), isNull(), isNull()) - } - - @Test - fun findExistingLocationBookmark() { - whenever(client.query(any(), any(), any(), any(), anyOrNull())).thenReturn(createCursor(1)) - assertTrue(testObject.findBookmarkLocation(examplePlaceBookmark)) - } - - @Test(expected = RuntimeException::class) - fun findLocationBookmarkTranslatesExceptions() { - whenever(client.query(any(), any(), anyOrNull(), any(), anyOrNull())).thenThrow(RemoteException("")) - testObject.findBookmarkLocation(examplePlaceBookmark) - } - - @Test - fun findNotExistingLocationBookmarkReturnsNull_emptyCursor() { - whenever(client.query(any(), any(), any(), any(), anyOrNull())).thenReturn(createCursor(0)) - assertFalse(testObject.findBookmarkLocation(examplePlaceBookmark)) - } - - @Test - fun findNotExistingLocationBookmarkReturnsNull_nullCursor() { - whenever(client.query(any(), any(), any(), any(), anyOrNull())).thenReturn(null) - assertFalse(testObject.findBookmarkLocation(examplePlaceBookmark)) - } - - @Test - fun cursorsAreClosedAfterFindLocationBookmarkQuery() { - val mockCursor: Cursor = mock() - whenever(client.query(any(), any(), anyOrNull(), any(), anyOrNull())).thenReturn(mockCursor) - whenever(mockCursor.moveToFirst()).thenReturn(false) - - testObject.findBookmarkLocation(examplePlaceBookmark) - - verify(mockCursor).close() - } - - @Test - fun migrateTableVersionFrom_v1_to_v2() { - onUpdate(database, 1, 2) - // Table didn't exist before v5 - verifyNoInteractions(database) - } - - @Test - fun migrateTableVersionFrom_v2_to_v3() { - onUpdate(database, 2, 3) - // Table didn't exist before v5 - verifyNoInteractions(database) - } - - @Test - fun migrateTableVersionFrom_v3_to_v4() { - onUpdate(database, 3, 4) - // Table didnt exist before v5 - verifyNoInteractions(database) - } - - @Test - fun migrateTableVersionFrom_v4_to_v5() { - onUpdate(database, 4, 5) - // Table didnt change in version 5 - verifyNoInteractions(database) - } - - @Test - fun migrateTableVersionFrom_v5_to_v6() { - onUpdate(database, 5, 6) - // Table didnt change in version 6 - verifyNoInteractions(database) - } - - @Test - fun migrateTableVersionFrom_v6_to_v7() { - onUpdate(database, 6, 7) - // Table didnt change in version 7 - verifyNoInteractions(database) - } - - @Test - fun migrateTableVersionFrom_v7_to_v8() { - onUpdate(database, 7, 8) - verify(database).execSQL(CREATE_TABLE_STATEMENT) - } - - @Test - fun migrateTableVersionFrom_v12_to_v13() { - onUpdate(database, 12, 13) - verify(database).execSQL("ALTER TABLE bookmarksLocations ADD COLUMN location_destroyed STRING;") - } - - @Test - fun migrateTableVersionFrom_v13_to_v14() { - onUpdate(database, 13, 14) - verify(database).execSQL("ALTER TABLE bookmarksLocations ADD COLUMN location_language STRING;") - } - - @Test - fun migrateTableVersionFrom_v14_to_v15() { - onUpdate(database, 14, 15) - verify(database).execSQL("ALTER TABLE bookmarksLocations ADD COLUMN location_exists STRING;") - } - - private fun createCursor(rows: Int): Cursor = - MatrixCursor(columns, rows).apply { - repeat(rows) { - newRow().apply { - add("placeName") - add("en") - add("placeDescription") - add("placeCategory") - add(Label.FOREST.text) - add(Label.FOREST.icon) - add("placeImage") - add("wikipediaLink") - add("wikidataLink") - add("commonsLink") - add(40.0) - add(51.4) - add("picName") - add(false) - } - } - } } diff --git a/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationControllerTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationControllerTest.kt index 262489ba5..a7afc6b14 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationControllerTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationControllerTest.kt @@ -2,6 +2,7 @@ package fr.free.nrw.commons.bookmarks.locations import com.nhaarman.mockitokotlin2.whenever import fr.free.nrw.commons.nearby.Place +import kotlinx.coroutines.runBlocking import org.junit.Assert import org.junit.Before import org.junit.Test @@ -19,9 +20,11 @@ class BookmarkLocationControllerTest { @Before fun setup() { - MockitoAnnotations.initMocks(this) - whenever(bookmarkDao!!.getAllBookmarksLocations()) - .thenReturn(mockBookmarkList) + MockitoAnnotations.openMocks(this) + runBlocking { + whenever(bookmarkDao!!.getAllBookmarksLocationsPlace()) + .thenReturn(mockBookmarkList) + } } /** @@ -66,7 +69,7 @@ class BookmarkLocationControllerTest { * Test case where all bookmark locations are fetched and media is found against it */ @Test - fun loadBookmarkedLocations() { + fun loadBookmarkedLocations() = runBlocking { val bookmarkedLocations = bookmarkLocationsController.loadFavoritesLocations() Assert.assertEquals(2, bookmarkedLocations.size.toLong()) diff --git a/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationFragmentUnitTests.kt b/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationFragmentUnitTests.kt index 24693dc85..d3d3f98be 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationFragmentUnitTests.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationFragmentUnitTests.kt @@ -22,6 +22,7 @@ import fr.free.nrw.commons.nearby.Place import fr.free.nrw.commons.nearby.fragments.CommonPlaceClickActions import fr.free.nrw.commons.nearby.fragments.PlaceAdapter import fr.free.nrw.commons.profile.ProfileActivity +import kotlinx.coroutines.runBlocking import org.junit.Assert import org.junit.Before import org.junit.Test @@ -129,7 +130,9 @@ class BookmarkLocationFragmentUnitTests { */ @Test fun testInitNonEmpty() { - whenever(controller.loadFavoritesLocations()).thenReturn(mockBookmarkList) + runBlocking { + whenever(controller.loadFavoritesLocations()).thenReturn(mockBookmarkList) + } val method: Method = BookmarkLocationsFragment::class.java.getDeclaredMethod("initList") method.isAccessible = true diff --git a/app/src/test/kotlin/fr/free/nrw/commons/nearby/NearbyParentFragmentPresenterTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/nearby/NearbyParentFragmentPresenterTest.kt index 12b2319d0..96098e2a0 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/nearby/NearbyParentFragmentPresenterTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/nearby/NearbyParentFragmentPresenterTest.kt @@ -8,6 +8,7 @@ import fr.free.nrw.commons.location.LatLng import fr.free.nrw.commons.location.LocationServiceManager.LocationChangeType import fr.free.nrw.commons.nearby.contract.NearbyParentFragmentContract import fr.free.nrw.commons.nearby.presenter.NearbyParentFragmentPresenter +import kotlinx.coroutines.runBlocking import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before @@ -463,7 +464,9 @@ class NearbyParentFragmentPresenterTest { nearbyPlacesInfo.searchLatLng = latestLocation nearbyPlacesInfo.placeList = emptyList() - whenever(bookmarkLocationsDao.getAllBookmarksLocations()).thenReturn(Collections.emptyList()) + runBlocking { + whenever(bookmarkLocationsDao.getAllBookmarksLocations()).thenReturn(Collections.emptyList()) + } nearbyPresenter.updateMapMarkers(nearbyPlacesInfo.placeList, latestLocation, null) Mockito.verify(nearbyParentFragmentView).setProgressBarVisibility(false) } From 907724a58be0120de35f3d68831826784b271a35 Mon Sep 17 00:00:00 2001 From: Saifuddin Date: Mon, 20 Jan 2025 12:37:00 +0530 Subject: [PATCH 09/11] Refactor BookmarkLocationsFragment and add tests for BookmarkLocationsDao * Moved `initList` to `BookmarkLocationsFragment` for better lifecycle handling. * Added test case for `BookmarkLocationsFragment`'s onResume. * Added test cases for `BookmarkLocationsDao` operations like adding, retrieving, finding, deleting and updating bookmarks. --- .../locations/BookmarkLocationsFragment.kt | 2 +- .../locations/BookMarkLocationDaoTest.kt | 73 ++++++++++++++++--- .../BookmarkLocationFragmentUnitTests.kt | 8 +- 3 files changed, 69 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt index a51d02b10..212c95105 100644 --- a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt @@ -129,7 +129,7 @@ class BookmarkLocationsFragment : DaggerFragment() { initList() } - private fun initList() { + fun initList() { var places: List viewLifecycleOwner.lifecycleScope.launch { places = controller.loadFavoritesLocations() diff --git a/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookMarkLocationDaoTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookMarkLocationDaoTest.kt index f1c3f9798..4808d8518 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookMarkLocationDaoTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookMarkLocationDaoTest.kt @@ -42,8 +42,7 @@ import org.robolectric.annotation.Config @Config(sdk = [21], application = TestCommonsApplication::class) class BookMarkLocationDaoTest { - @Mock - var bookmarkLocationsDao: BookmarkLocationsDao? = null + private lateinit var bookmarkLocationsDao: BookmarkLocationsDao private lateinit var database: AppDatabase @@ -92,17 +91,67 @@ class BookMarkLocationDaoTest { } @Test - fun insertAndRetrieveBookmark() = runBlocking { - // Insert a bookmark - bookmarkLocationsDao?.addBookmarkLocation(examplePlaceBookmark.toBookmarksLocations()) + fun testForAddAndGetAllBookmarkLocations() = runBlocking { + bookmarkLocationsDao.addBookmarkLocation(examplePlaceBookmark.toBookmarksLocations()) - // Retrieve all bookmarks - val bookmarks = bookmarkLocationsDao?.getAllBookmarksLocations() + val bookmarks = bookmarkLocationsDao.getAllBookmarksLocations() - // Assert the bookmark exists - assertEquals(1, bookmarks?.size) - val retrievedBookmark = bookmarks?.first() - assertEquals(examplePlaceBookmark.name, retrievedBookmark?.locationName) - assertEquals(examplePlaceBookmark.language, retrievedBookmark?.locationLanguage) + assertEquals(1, bookmarks.size) + val retrievedBookmark = bookmarks.first() + assertEquals(examplePlaceBookmark.name, retrievedBookmark.locationName) + assertEquals(examplePlaceBookmark.language, retrievedBookmark.locationLanguage) + } + + @Test + fun testFindBookmarkByNameForTrue() = runBlocking { + bookmarkLocationsDao.addBookmarkLocation(examplePlaceBookmark.toBookmarksLocations()) + + val exists = bookmarkLocationsDao.findBookmarkLocation(examplePlaceBookmark.name) + assertTrue(exists) + } + + @Test + fun testFindBookmarkByNameForFalse() = runBlocking { + bookmarkLocationsDao.addBookmarkLocation(examplePlaceBookmark.toBookmarksLocations()) + + val exists = bookmarkLocationsDao.findBookmarkLocation("xyz") + assertFalse(exists) + } + + @Test + fun testDeleteBookmark() = runBlocking { + val bookmarkLocation = examplePlaceBookmark.toBookmarksLocations() + bookmarkLocationsDao.addBookmarkLocation(bookmarkLocation) + + bookmarkLocationsDao.deleteBookmarkLocation(bookmarkLocation) + + val bookmarks = bookmarkLocationsDao.getAllBookmarksLocations() + assertTrue(bookmarks.isEmpty()) + } + + @Test + fun testUpdateBookmarkForTrue() = runBlocking { + val exists = bookmarkLocationsDao.updateBookmarkLocation(examplePlaceBookmark) + + assertTrue(exists) + } + + @Test + fun testUpdateBookmarkForFalse() = runBlocking { + val newBookmark = examplePlaceBookmark.toBookmarksLocations() + bookmarkLocationsDao.addBookmarkLocation(newBookmark) + + val exists = bookmarkLocationsDao.updateBookmarkLocation(examplePlaceBookmark) + assertFalse(exists) + } + + @Test + fun testGetAllBookmarksLocationsPlace() = runBlocking { + val bookmarkLocation = examplePlaceBookmark.toBookmarksLocations() + bookmarkLocationsDao.addBookmarkLocation(bookmarkLocation) + + val bookmarks = bookmarkLocationsDao.getAllBookmarksLocationsPlace() + assertEquals(1, bookmarks.size) + assertEquals(examplePlaceBookmark.name, bookmarks.first().name) } } diff --git a/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationFragmentUnitTests.kt b/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationFragmentUnitTests.kt index d3d3f98be..d824c0005 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationFragmentUnitTests.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationFragmentUnitTests.kt @@ -10,6 +10,7 @@ import androidx.fragment.app.FragmentManager import androidx.fragment.app.FragmentTransaction import androidx.recyclerview.widget.RecyclerView import androidx.test.core.app.ApplicationProvider +import com.nhaarman.mockitokotlin2.verify import com.nhaarman.mockitokotlin2.whenever import fr.free.nrw.commons.OkHttpConnectionFactory import fr.free.nrw.commons.R @@ -171,7 +172,12 @@ class BookmarkLocationFragmentUnitTests { */ @Test @Throws(Exception::class) - fun testOnResume() { + fun testOnResume() = runBlocking { + whenever(controller.loadFavoritesLocations()).thenReturn(mockBookmarkList) + fragment.onResume() + + verify(fragment).initList() + verify(adapter).items = mockBookmarkList } } From abfc897ce862ceb7aa63b27a6967b5b912f9875e Mon Sep 17 00:00:00 2001 From: Saifuddin Date: Mon, 20 Jan 2025 16:13:47 +0530 Subject: [PATCH 10/11] Refactor BookmarkLocationsFragment to load favorites only when view is not null * BookmarkLocationsFragment: load favorites locations only when view is not null. * BookmarkLocationsFragmentTest: added spy and verify to test onResume() call initList() method. --- .../locations/BookmarkLocationsFragment.kt | 11 ++++++++--- .../BookmarkLocationFragmentUnitTests.kt | 17 +++++++++-------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt index 212c95105..27fd6830d 100644 --- a/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsFragment.kt @@ -1,6 +1,7 @@ package fr.free.nrw.commons.bookmarks.locations import android.Manifest.permission +import android.annotation.SuppressLint import android.os.Bundle import android.view.LayoutInflater import android.view.View @@ -8,7 +9,9 @@ import android.view.ViewGroup import androidx.activity.result.ActivityResultLauncher import androidx.activity.result.contract.ActivityResultContracts.RequestMultiplePermissions import androidx.activity.result.contract.ActivityResultContracts.StartActivityForResult +import androidx.lifecycle.Lifecycle import androidx.lifecycle.lifecycleScope +import androidx.lifecycle.repeatOnLifecycle import androidx.recyclerview.widget.LinearLayoutManager import dagger.android.support.DaggerFragment import fr.free.nrw.commons.R @@ -131,9 +134,11 @@ class BookmarkLocationsFragment : DaggerFragment() { fun initList() { var places: List - viewLifecycleOwner.lifecycleScope.launch { - places = controller.loadFavoritesLocations() - updateUIList(places) + if(view != null) { + viewLifecycleOwner.lifecycleScope.launch { + places = controller.loadFavoritesLocations() + updateUIList(places) + } } } diff --git a/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationFragmentUnitTests.kt b/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationFragmentUnitTests.kt index d824c0005..52121bf84 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationFragmentUnitTests.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationFragmentUnitTests.kt @@ -23,12 +23,14 @@ import fr.free.nrw.commons.nearby.Place import fr.free.nrw.commons.nearby.fragments.CommonPlaceClickActions import fr.free.nrw.commons.nearby.fragments.PlaceAdapter import fr.free.nrw.commons.profile.ProfileActivity +import kotlinx.coroutines.delay import kotlinx.coroutines.runBlocking import org.junit.Assert import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mock +import org.mockito.Mockito.spy import org.mockito.MockitoAnnotations import org.powermock.reflect.Whitebox import org.robolectric.Robolectric @@ -133,11 +135,11 @@ class BookmarkLocationFragmentUnitTests { fun testInitNonEmpty() { runBlocking { whenever(controller.loadFavoritesLocations()).thenReturn(mockBookmarkList) + val method: Method = + BookmarkLocationsFragment::class.java.getDeclaredMethod("initList") + method.isAccessible = true + method.invoke(fragment) } - val method: Method = - BookmarkLocationsFragment::class.java.getDeclaredMethod("initList") - method.isAccessible = true - method.invoke(fragment) } /** @@ -173,11 +175,10 @@ class BookmarkLocationFragmentUnitTests { @Test @Throws(Exception::class) fun testOnResume() = runBlocking { + val fragmentSpy = spy(fragment) whenever(controller.loadFavoritesLocations()).thenReturn(mockBookmarkList) - fragment.onResume() - - verify(fragment).initList() - verify(adapter).items = mockBookmarkList + fragmentSpy.onResume() + verify(fragmentSpy).initList() } } From d7d3e44214f8bd22ddc11631fd3b3f386cab785c Mon Sep 17 00:00:00 2001 From: Saifuddin Date: Mon, 20 Jan 2025 16:48:59 +0530 Subject: [PATCH 11/11] Refactor database and add migration * `AppDatabase`: Updated to use room migration. * `CommonsApplicationModule`: added migration from version 19 to 20 to `appDatabase` --- .../fr/free/nrw/commons/db/AppDatabase.kt | 4 ++ .../commons/di/CommonsApplicationModule.kt | 46 ++++++++++++++++++- 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/fr/free/nrw/commons/db/AppDatabase.kt b/app/src/main/java/fr/free/nrw/commons/db/AppDatabase.kt index d51e53dbe..74ec9bc89 100644 --- a/app/src/main/java/fr/free/nrw/commons/db/AppDatabase.kt +++ b/app/src/main/java/fr/free/nrw/commons/db/AppDatabase.kt @@ -1,8 +1,12 @@ package fr.free.nrw.commons.db +import android.content.Context import androidx.room.Database +import androidx.room.Room import androidx.room.RoomDatabase import androidx.room.TypeConverters +import androidx.room.migration.Migration +import androidx.sqlite.db.SupportSQLiteDatabase import fr.free.nrw.commons.bookmarks.category.BookmarkCategoriesDao import fr.free.nrw.commons.bookmarks.category.BookmarksCategoryModal import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao diff --git a/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.kt b/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.kt index b25998ee5..d777f1a56 100644 --- a/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.kt +++ b/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.kt @@ -192,7 +192,10 @@ open class CommonsApplicationModule(private val applicationContext: Context) { applicationContext, AppDatabase::class.java, "commons_room.db" - ).addMigrations(MIGRATION_1_2).fallbackToDestructiveMigration().build() + ).addMigrations( + MIGRATION_1_2, + MIGRATION_19_TO_20 + ).fallbackToDestructiveMigration().build() @Provides fun providesContributionsDao(appDatabase: AppDatabase): ContributionDao = @@ -246,5 +249,46 @@ open class CommonsApplicationModule(private val applicationContext: Context) { ) } } + + private val MIGRATION_19_TO_20 = object : Migration(19, 20) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL( + """ + CREATE TABLE IF NOT EXISTS bookmarks_locations ( + location_name TEXT NOT NULL PRIMARY KEY, + location_language TEXT NOT NULL, + location_description TEXT NOT NULL, + location_lat REAL NOT NULL, + location_long REAL NOT NULL, + location_category TEXT NOT NULL, + location_label_text TEXT NOT NULL, + location_label_icon INTEGER, + location_image_url TEXT NOT NULL, + location_wikipedia_link TEXT NOT NULL, + location_wikidata_link TEXT NOT NULL, + location_commons_link TEXT NOT NULL, + location_pic TEXT NOT NULL, + location_exists INTEGER NOT NULL CHECK(location_exists IN (0, 1)) + ) + """ + ) + db.execSQL(""" + INSERT INTO bookmarks_locations ( + location_name, location_language, location_description, location_category, + location_label_text, location_label_icon, location_lat, location_long, + location_image_url, location_wikipedia_link, location_wikidata_link, + location_commons_link, location_pic, location_exists + ) + SELECT + location_name, location_language, location_description, location_category, + location_label_text, location_label_icon, location_lat, location_long, + location_image_url, location_wikipedia_link, location_wikidata_link, + location_commons_link, location_pic, location_exists + FROM bookmarksLocations + """) + + db.execSQL("DROP TABLE IF EXISTS bookmarkLocations") + } + } } }