From f32c59034da5b1d758c05d9d03287d089abb3549 Mon Sep 17 00:00:00 2001 From: Saifuddin Date: Thu, 16 Jan 2025 18:00:39 +0530 Subject: [PATCH] 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) }