Issue #171 - Fixing Lint Warnings in Category Package

Fixing Some Android Lint and SonarLint Warnings in this package:
* `fr.free.nrw.commons.category`

Some of the fixes were:
* Switching the try/catch/finally blocks for database querying in `CategoryDao` into try-with-resources blocks, with the Cursor object getting set as the closable resource
* Switching one field type in the sqlite "Create Table Statement" in CategoryDao from STRING to TEXT
* Throwing the new DAOException instead of a generic RuntimeException in CategoryDao
* Logging some Exceptions using Timber instead of `e.printStackTrace()`
* A few access modifier changes to be more restrictive
* Adding a private constructor to some static classes
* Made some constant String variables for recurring Strings
* Changed the switch cases with only one case into if/else statements
* Added some @nonnull and @nullable annotations to methods that inherit methods that use those annotations

Author: JasonSarwar

app/src/main/java/fr/free/nrw/commons/category/CategoriesAdapterFactory.java

@@ -6,7 +6,7 @@
 import java.util.Collections;
 import java.util.List;
 
-public class CategoriesAdapterFactory {
+class CategoriesAdapterFactory {
     private final CategoryClickedListener listener;
 
     public CategoriesAdapterFactory(CategoryClickedListener listener) {
@@ -17,7 +17,7 @@ public CategoryRendererAdapter create(List<CategoryItem> placeList) {
         RendererBuilder<CategoryItem> builder = new RendererBuilder<CategoryItem>()
                 .bind(CategoryItem.class, new CategoriesRenderer(listener));
         ListAdapteeCollection<CategoryItem> collection = new ListAdapteeCollection<>(
-                placeList != null ? placeList : Collections.<CategoryItem>emptyList());
+                placeList != null ? placeList : Collections.emptyList());
         return new CategoryRendererAdapter(builder, collection);
     }
 }

app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java

@@ -8,7 +8,7 @@
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
-
+import java.util.Map;
 import javax.inject.Inject;
 import javax.inject.Named;
 
@@ -21,15 +21,15 @@
 /**
  * The model class for categories in upload
  */
-public class CategoriesModel{
+public class CategoriesModel {
     private static final int SEARCH_CATS_LIMIT = 25;
 
     private final CategoryClient categoryClient;
     private final CategoryDao categoryDao;
     private final JsonKvStore directKvStore;
 
-    private HashMap<String, ArrayList<String>> categoriesCache;
-    private List<CategoryItem> selectedCategories;
+    private final Map<String, ArrayList<String>> categoriesCache;
+    private final List<CategoryItem> selectedCategories;
 
     @Inject GpsCategoryModel gpsCategoryModel;
     @Inject
@@ -82,7 +82,7 @@ public boolean containsYear(String item) {
      * Updates category count in category dao
      * @param item
      */
-    public void updateCategoryCount(CategoryItem item) {
+    private void updateCategoryCount(CategoryItem item) {
         Category category = categoryDao.find(item.getName());
 
         // Newly used category...
@@ -94,7 +94,7 @@ public void updateCategoryCount(CategoryItem item) {
         categoryDao.save(category);
     }
 
-    boolean cacheContainsKey(String term) {
+    private boolean cacheContainsKey(String term) {
         return categoriesCache.containsKey(term);
     }
     //endregion
@@ -111,7 +111,7 @@ public Observable<CategoryItem> searchAll(String term, List<String> imageTitleLi
             Observable<CategoryItem> categoryItemObservable = gpsCategories()
                     .concatWith(titleCategories(imageTitleList));
             if (hasDirectCategories()) {
-                categoryItemObservable.concatWith(directCategories().concatWith(recentCategories()));
+                categoryItemObservable = categoryItemObservable.concatWith(directCategories().concatWith(recentCategories()));
             }
             return categoryItemObservable;
         }
@@ -153,11 +153,11 @@ private boolean hasDirectCategories() {
     private Observable<CategoryItem> directCategories() {
         String directCategory = directKvStore.getString("Category", "");
         List<String> categoryList = new ArrayList<>();
-        Timber.d("Direct category found: " + directCategory);
+        Timber.d("Direct category found: %s", directCategory);
 
         if (!directCategory.equals("")) {
             categoryList.add(directCategory);
-            Timber.d("DirectCat does not equal emptyString. Direct Cat list has " + categoryList);
+            Timber.d("DirectCat does not equal emptyString. Direct Cat list has %s", categoryList);
         }
         return Observable.fromIterable(categoryList).map(name -> new CategoryItem(name, false));
     }
@@ -166,7 +166,7 @@ private Observable<CategoryItem> directCategories() {
      * Returns GPS categories
      * @return
      */
-    Observable<CategoryItem> gpsCategories() {
+    private Observable<CategoryItem> gpsCategories() {
         return Observable.fromIterable(gpsCategoryModel.getCategoryList())
                 .map(name -> new CategoryItem(name, false));
     }
@@ -217,15 +217,15 @@ public void onCategoryItemClicked(CategoryItem item) {
      * Select's category
      * @param item
      */
-    public void selectCategory(CategoryItem item) {
+    private void selectCategory(CategoryItem item) {
         selectedCategories.add(item);
     }
 
     /**
      * Unselect Category
      * @param item
      */
-    public void unselectCategory(CategoryItem item) {
+    private void unselectCategory(CategoryItem item) {
         selectedCategories.remove(item);
     }
 

app/src/main/java/fr/free/nrw/commons/category/CategoryContentProvider.java

@@ -32,6 +32,8 @@ public class CategoryContentProvider extends CommonsDaggerContentProvider {
 
     private static final UriMatcher uriMatcher = new UriMatcher(NO_MATCH);
 
+    private static final String UNKNOWN_URL = "Unknown URI: ";
+
     static {
         uriMatcher.addURI(BuildConfig.CATEGORY_AUTHORITY, BASE_PATH, CATEGORIES);
         uriMatcher.addURI(BuildConfig.CATEGORY_AUTHORITY, BASE_PATH + "/#", CATEGORIES_ID);
@@ -71,7 +73,7 @@ public Cursor query(@NonNull Uri uri, String[] projection, String selection,
                 );
                 break;
             default:
-                throw new IllegalArgumentException("Unknown URI" + uri);
+                throw new IllegalArgumentException(UNKNOWN_URL + uri);
         }
 
         cursor.setNotificationUri(getContext().getContentResolver(), uri);
@@ -90,12 +92,10 @@ public Uri insert(@NonNull Uri uri, ContentValues contentValues) {
         int uriType = uriMatcher.match(uri);
         SQLiteDatabase sqlDB = dbOpenHelper.getWritableDatabase();
         long id;
-        switch (uriType) {
-            case CATEGORIES:
-                id = sqlDB.insert(TABLE_NAME, null, contentValues);
-                break;
-            default:
-                throw new IllegalArgumentException("Unknown URI: " + uri);
+        if (uriType == CATEGORIES) {
+            id = sqlDB.insert(TABLE_NAME, null, contentValues);
+        } else {
+            throw new IllegalArgumentException(UNKNOWN_URL + uri);
         }
         getContext().getContentResolver().notifyChange(uri, null);
         return Uri.parse(BASE_URI + "/" + id);
@@ -113,15 +113,13 @@ public int bulkInsert(@NonNull Uri uri, @NonNull ContentValues[] values) {
         int uriType = uriMatcher.match(uri);
         SQLiteDatabase sqlDB = dbOpenHelper.getWritableDatabase();
         sqlDB.beginTransaction();
-        switch (uriType) {
-            case CATEGORIES:
-                for (ContentValues value : values) {
-                    Timber.d("Inserting! %s", value);
-                    sqlDB.insert(TABLE_NAME, null, value);
-                }
-                break;
-            default:
-                throw new IllegalArgumentException("Unknown URI: " + uri);
+        if (uriType == CATEGORIES) {
+            for (ContentValues value : values) {
+                Timber.d("Inserting! %s", value);
+                sqlDB.insert(TABLE_NAME, null, value);
+            }
+        } else {
+            throw new IllegalArgumentException(UNKNOWN_URL + uri);
         }
         sqlDB.setTransactionSuccessful();
         sqlDB.endTransaction();
@@ -145,21 +143,19 @@ outside world (exported="false"). Even then, we should make sure to sanitize
         int uriType = uriMatcher.match(uri);
         SQLiteDatabase sqlDB = dbOpenHelper.getWritableDatabase();
         int rowsUpdated;
-        switch (uriType) {
-            case CATEGORIES_ID:
-                if (TextUtils.isEmpty(selection)) {
-                    int id = Integer.valueOf(uri.getLastPathSegment());
-                    rowsUpdated = sqlDB.update(TABLE_NAME,
-                            contentValues,
-                            COLUMN_ID + " = ?",
-                            new String[]{String.valueOf(id)});
-                } else {
-                    throw new IllegalArgumentException(
-                            "Parameter `selection` should be empty when updating an ID");
-                }
-                break;
-            default:
-                throw new IllegalArgumentException("Unknown URI: " + uri + " with type " + uriType);
+        if (uriType == CATEGORIES_ID) {
+            if (TextUtils.isEmpty(selection)) {
+                int id = Integer.valueOf(uri.getLastPathSegment());
+                rowsUpdated = sqlDB.update(TABLE_NAME,
+                        contentValues,
+                        COLUMN_ID + " = ?",
+                        new String[]{String.valueOf(id)});
+            } else {
+                throw new IllegalArgumentException(
+                        "Parameter `selection` should be empty when updating an ID");
+            }
+        } else {
+            throw new IllegalArgumentException(UNKNOWN_URL + uri + " with type " + uriType);
         }
         getContext().getContentResolver().notifyChange(uri, null);
         return rowsUpdated;

app/src/main/java/fr/free/nrw/commons/category/CategoryDao.java

@@ -8,6 +8,7 @@
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 
+import fr.free.nrw.commons.data.DAOException;
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.List;
@@ -34,7 +35,7 @@ public void save(Category category) {
                 db.update(category.getContentUri(), toContentValues(category), null, null);
             }
         } catch (RemoteException e) {
-            throw new RuntimeException(e);
+            throw new DAOException(e);
         } finally {
             db.release();
         }
@@ -48,25 +49,21 @@ public void save(Category category) {
      */
     @Nullable
     Category find(String name) {
-        Cursor cursor = null;
         ContentProviderClient db = clientProvider.get();
-        try {
-            cursor = db.query(
+        try (Cursor cursor = db.query(
                     CategoryContentProvider.BASE_URI,
                     Table.ALL_FIELDS,
                     Table.COLUMN_NAME + "=?",
                     new String[]{name},
-                    null);
+                    null)) {
+
             if (cursor != null && cursor.moveToFirst()) {
                 return fromCursor(cursor);
             }
         } catch (RemoteException e) {
             // This feels lazy, but to hell with checked exceptions. :)
-            throw new RuntimeException(e);
+            throw new DAOException(e);
         } finally {
-            if (cursor != null) {
-                cursor.close();
-            }
             db.release();
         }
         return null;
@@ -80,26 +77,22 @@ Category find(String name) {
     @NonNull
     List<String> recentCategories(int limit) {
         List<String> items = new ArrayList<>();
-        Cursor cursor = null;
         ContentProviderClient db = clientProvider.get();
-        try {
-            cursor = db.query(
+        try (Cursor cursor = db.query(
                     CategoryContentProvider.BASE_URI,
                     Table.ALL_FIELDS,
                     null,
                     new String[]{},
-                    Table.COLUMN_LAST_USED + " DESC");
+                    Table.COLUMN_LAST_USED + " DESC")){
+
             // fixme add a limit on the original query instead of falling out of the loop?
             while (cursor != null && cursor.moveToNext()
                     && cursor.getPosition() < limit) {
                 items.add(fromCursor(cursor).getName());
             }
         } catch (RemoteException e) {
-            throw new RuntimeException(e);
+            throw new DAOException(e);
         } finally {
-            if (cursor != null) {
-                cursor.close();
-            }
             db.release();
         }
         return items;
@@ -133,7 +126,7 @@ public static class Table {
         static final String COLUMN_TIMES_USED = "times_used";
 
         // NOTE! KEEP IN SAME ORDER AS THEY ARE DEFINED UP THERE. HELPS HARD CODE COLUMN INDICES.
-        public static final String[] ALL_FIELDS = {
+        protected static final String[] ALL_FIELDS = {
                 COLUMN_ID,
                 COLUMN_NAME,
                 COLUMN_LAST_USED,
@@ -144,11 +137,14 @@ public static class Table {
 
         static final String CREATE_TABLE_STATEMENT = "CREATE TABLE " + TABLE_NAME + " ("
                 + COLUMN_ID + " INTEGER PRIMARY KEY,"
-                + COLUMN_NAME + " STRING,"
+                + COLUMN_NAME + " TEXT,"
                 + COLUMN_LAST_USED + " INTEGER,"
                 + COLUMN_TIMES_USED + " INTEGER"
                 + ");";
 
+        private Table() {
+        }
+
         public static void onCreate(SQLiteDatabase db) {
             db.execSQL(CREATE_TABLE_STATEMENT);
         }
@@ -178,7 +174,6 @@ public static void onUpdate(SQLiteDatabase db, int from, int to) {
             if (from == 5) {
                 from++;
                 onUpdate(db, from, to);
-                return;
             }
         }
     }