From 942cfd3cd1ad243c7440110f1f974ba715b5d526 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Sat, 23 Apr 2016 15:29:13 +0100 Subject: [PATCH 01/15] AppDetails: guard against null fragment on destroy Fixes a crash reported by ACRA: java.lang.RuntimeException: Unable to destroy activity {org.fdroid.fdroid/org.fdroid.fdroid.AppDetails}: java.lang.NullPointerException at android.app.ActivityThread.performDestroyActivity(ActivityThread.java:3281) at android.app.ActivityThread.handleDestroyActivity(ActivityThread.java:3299) at android.app.ActivityThread.access$1200(ActivityThread.java:133) at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1256) at android.os.Handler.dispatchMessage(Handler.java:99) at android.os.Looper.loop(Looper.java:137) at android.app.ActivityThread.main(ActivityThread.java:4812) at java.lang.reflect.Method.invokeNative(Native Method) at java.lang.reflect.Method.invoke(Method.java:511) at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:792) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:559) at dalvik.system.NativeStart.main(Native Method) Caused by: java.lang.NullPointerException at org.fdroid.fdroid.AppDetails.cleanUpFinishedDownload(AppDetails.java:442) at org.fdroid.fdroid.AppDetails.onDestroy(AppDetails.java:567) at android.app.Activity.performDestroy(Activity.java:5366) at android.app.Instrumentation.callActivityOnDestroy(Instrumentation.java:1124) at android.app.ActivityThread.performDestroyActivity(ActivityThread.java:3268) ... 11 more --- app/src/main/java/org/fdroid/fdroid/AppDetails.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails.java b/app/src/main/java/org/fdroid/fdroid/AppDetails.java index 149065263..a431bd01b 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails.java @@ -439,7 +439,9 @@ public class AppDetails extends AppCompatActivity { */ private void cleanUpFinishedDownload() { activeDownloadUrlString = null; - headerFragment.removeProgress(); + if (headerFragment != null) { + headerFragment.removeProgress(); + } unregisterDownloaderReceivers(); } From 7f2a8115411e894453a64cbf5680f7746c5616d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Sat, 23 Apr 2016 15:37:34 +0100 Subject: [PATCH 02/15] RepoXMLHandler: make helper func static --- app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java b/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java index 474a42a9a..e5469633d 100644 --- a/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java +++ b/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java @@ -268,7 +268,7 @@ public class RepoXMLHandler extends DefaultHandler { curchars.setLength(0); } - private String cleanWhiteSpace(@Nullable String str) { + private static String cleanWhiteSpace(@Nullable String str) { return str == null ? null : str.replaceAll("\\s", " "); } } From 6a0eec1262111fa3543dba824a51dc38321b3a80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Sat, 23 Apr 2016 15:40:09 +0100 Subject: [PATCH 03/15] DBHelper: don't wrap entire func bodies in ifs This removes tons of unnecessary indenting. Do it in smaller functions too for consistency. --- .../java/org/fdroid/fdroid/data/DBHelper.java | 379 +++++++++--------- 1 file changed, 199 insertions(+), 180 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java index 9ec221939..073e585b7 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java @@ -116,80 +116,82 @@ class DBHelper extends SQLiteOpenHelper { } private void populateRepoNames(SQLiteDatabase db, int oldVersion) { - if (oldVersion < 37) { - Utils.debugLog(TAG, "Populating repo names from the url"); - final String[] columns = {"address", "_id"}; - Cursor cursor = db.query(TABLE_REPO, columns, - "name IS NULL OR name = ''", null, null, null, null); - if (cursor != null) { - if (cursor.getCount() > 0) { - cursor.moveToFirst(); - while (!cursor.isAfterLast()) { - String address = cursor.getString(0); - long id = cursor.getInt(1); - ContentValues values = new ContentValues(1); - String name = Repo.addressToName(address); - values.put("name", name); - final String[] args = {Long.toString(id)}; - Utils.debugLog(TAG, "Setting repo name to '" + name + "' for repo " + address); - db.update(TABLE_REPO, values, "_id = ?", args); - cursor.moveToNext(); - } + if (oldVersion >= 37) { + return; + } + Utils.debugLog(TAG, "Populating repo names from the url"); + final String[] columns = {"address", "_id"}; + Cursor cursor = db.query(TABLE_REPO, columns, + "name IS NULL OR name = ''", null, null, null, null); + if (cursor != null) { + if (cursor.getCount() > 0) { + cursor.moveToFirst(); + while (!cursor.isAfterLast()) { + String address = cursor.getString(0); + long id = cursor.getInt(1); + ContentValues values = new ContentValues(1); + String name = Repo.addressToName(address); + values.put("name", name); + final String[] args = {Long.toString(id)}; + Utils.debugLog(TAG, "Setting repo name to '" + name + "' for repo " + address); + db.update(TABLE_REPO, values, "_id = ?", args); + cursor.moveToNext(); } - cursor.close(); } + cursor.close(); } } private void renameRepoId(SQLiteDatabase db, int oldVersion) { - if (oldVersion < 36 && !columnExists(db, TABLE_REPO, "_id")) { - - Utils.debugLog(TAG, "Renaming " + TABLE_REPO + ".id to _id"); - db.beginTransaction(); - - try { - // http://stackoverflow.com/questions/805363/how-do-i-rename-a-column-in-a-sqlite-database-table#805508 - String tempTableName = TABLE_REPO + "__temp__"; - db.execSQL("ALTER TABLE " + TABLE_REPO + " RENAME TO " + tempTableName + ";"); - - // I realise this is available in the CREATE_TABLE_REPO above, - // however I have a feeling that it will need to be the same as the - // current structure of the table as of DBVersion 36, or else we may - // get into strife. For example, if there was a field that - // got removed, then it will break the "insert select" - // statement. Therefore, I've put a copy of CREATE_TABLE_REPO - // here that is the same as it was at DBVersion 36. - String createTableDdl = "create table " + TABLE_REPO + " (" - + "_id integer not null primary key, " - + "address text not null, " - + "name text, " - + "description text, " - + "inuse integer not null, " - + "priority integer not null, " - + "pubkey text, " - + "fingerprint text, " - + "maxage integer not null default 0, " - + "version integer not null default 0, " - + "lastetag text, " - + "lastUpdated string);"; - - db.execSQL(createTableDdl); - - String nonIdFields = "address, name, description, inuse, priority, " + - "pubkey, fingerprint, maxage, version, lastetag, lastUpdated"; - - String insertSql = "INSERT INTO " + TABLE_REPO + - "(_id, " + nonIdFields + " ) " + - "SELECT id, " + nonIdFields + " FROM " + tempTableName + ";"; - - db.execSQL(insertSql); - db.execSQL("DROP TABLE " + tempTableName + ";"); - db.setTransactionSuccessful(); - } catch (Exception e) { - Log.e(TAG, "Error renaming id to _id", e); - } - db.endTransaction(); + if (oldVersion >= 36 || columnExists(db, TABLE_REPO, "_id")) { + return; } + + Utils.debugLog(TAG, "Renaming " + TABLE_REPO + ".id to _id"); + db.beginTransaction(); + + try { + // http://stackoverflow.com/questions/805363/how-do-i-rename-a-column-in-a-sqlite-database-table#805508 + String tempTableName = TABLE_REPO + "__temp__"; + db.execSQL("ALTER TABLE " + TABLE_REPO + " RENAME TO " + tempTableName + ";"); + + // I realise this is available in the CREATE_TABLE_REPO above, + // however I have a feeling that it will need to be the same as the + // current structure of the table as of DBVersion 36, or else we may + // get into strife. For example, if there was a field that + // got removed, then it will break the "insert select" + // statement. Therefore, I've put a copy of CREATE_TABLE_REPO + // here that is the same as it was at DBVersion 36. + String createTableDdl = "create table " + TABLE_REPO + " (" + + "_id integer not null primary key, " + + "address text not null, " + + "name text, " + + "description text, " + + "inuse integer not null, " + + "priority integer not null, " + + "pubkey text, " + + "fingerprint text, " + + "maxage integer not null default 0, " + + "version integer not null default 0, " + + "lastetag text, " + + "lastUpdated string);"; + + db.execSQL(createTableDdl); + + String nonIdFields = "address, name, description, inuse, priority, " + + "pubkey, fingerprint, maxage, version, lastetag, lastUpdated"; + + String insertSql = "INSERT INTO " + TABLE_REPO + + "(_id, " + nonIdFields + " ) " + + "SELECT id, " + nonIdFields + " FROM " + tempTableName + ";"; + + db.execSQL(insertSql); + db.execSQL("DROP TABLE " + tempTableName + ";"); + db.setTransactionSuccessful(); + } catch (Exception e) { + Log.e(TAG, "Error renaming id to _id", e); + } + db.endTransaction(); } @Override @@ -299,36 +301,37 @@ class DBHelper extends SQLiteOpenHelper { * key in sqlite - table must be recreated). */ private void migrateRepoTable(SQLiteDatabase db, int oldVersion) { - if (oldVersion < 20) { - List oldrepos = new ArrayList<>(); - Cursor cursor = db.query(TABLE_REPO, - new String[] {"address", "inuse", "pubkey"}, - null, null, null, null, null); - if (cursor != null) { - if (cursor.getCount() > 0) { - cursor.moveToFirst(); - while (!cursor.isAfterLast()) { - Repo repo = new Repo(); - repo.address = cursor.getString(0); - repo.inuse = cursor.getInt(1) == 1; - repo.signingCertificate = cursor.getString(2); - oldrepos.add(repo); - cursor.moveToNext(); - } + if (oldVersion >= 20) { + return; + } + List oldrepos = new ArrayList<>(); + Cursor cursor = db.query(TABLE_REPO, + new String[] {"address", "inuse", "pubkey"}, + null, null, null, null, null); + if (cursor != null) { + if (cursor.getCount() > 0) { + cursor.moveToFirst(); + while (!cursor.isAfterLast()) { + Repo repo = new Repo(); + repo.address = cursor.getString(0); + repo.inuse = cursor.getInt(1) == 1; + repo.signingCertificate = cursor.getString(2); + oldrepos.add(repo); + cursor.moveToNext(); } - cursor.close(); - } - db.execSQL("drop table " + TABLE_REPO); - db.execSQL(CREATE_TABLE_REPO); - for (final Repo repo : oldrepos) { - ContentValues values = new ContentValues(); - values.put("address", repo.address); - values.put("inuse", repo.inuse); - values.put("priority", 10); - values.put("pubkey", repo.signingCertificate); - values.put("lastetag", (String) null); - db.insert(TABLE_REPO, null, values); } + cursor.close(); + } + db.execSQL("drop table " + TABLE_REPO); + db.execSQL(CREATE_TABLE_REPO); + for (final Repo repo : oldrepos) { + ContentValues values = new ContentValues(); + values.put("address", repo.address); + values.put("inuse", repo.inuse); + values.put("priority", 10); + values.put("pubkey", repo.signingCertificate); + values.put("lastetag", (String) null); + db.insert(TABLE_REPO, null, values); } } @@ -350,22 +353,23 @@ class DBHelper extends SQLiteOpenHelper { private void addNameAndDescriptionToRepo(SQLiteDatabase db, int oldVersion) { boolean nameExists = columnExists(db, TABLE_REPO, "name"); boolean descriptionExists = columnExists(db, TABLE_REPO, "description"); - if (oldVersion < 21 && !(nameExists && descriptionExists)) { - if (!nameExists) { - db.execSQL("alter table " + TABLE_REPO + " add column name text"); - } - if (!descriptionExists) { - db.execSQL("alter table " + TABLE_REPO + " add column description text"); - } - insertNameAndDescription(db, R.string.fdroid_repo_address, - R.string.fdroid_repo_name, R.string.fdroid_repo_description); - insertNameAndDescription(db, R.string.fdroid_archive_address, - R.string.fdroid_archive_name, R.string.fdroid_archive_description); - insertNameAndDescription(db, R.string.guardianproject_repo_address, - R.string.guardianproject_repo_name, R.string.guardianproject_repo_description); - insertNameAndDescription(db, R.string.guardianproject_archive_address, - R.string.guardianproject_archive_name, R.string.guardianproject_archive_description); + if (oldVersion >= 21 || (nameExists && descriptionExists)) { + return; } + if (!nameExists) { + db.execSQL("alter table " + TABLE_REPO + " add column name text"); + } + if (!descriptionExists) { + db.execSQL("alter table " + TABLE_REPO + " add column description text"); + } + insertNameAndDescription(db, R.string.fdroid_repo_address, + R.string.fdroid_repo_name, R.string.fdroid_repo_description); + insertNameAndDescription(db, R.string.fdroid_archive_address, + R.string.fdroid_archive_name, R.string.fdroid_archive_description); + insertNameAndDescription(db, R.string.guardianproject_repo_address, + R.string.guardianproject_repo_name, R.string.guardianproject_repo_description); + insertNameAndDescription(db, R.string.guardianproject_archive_address, + R.string.guardianproject_archive_name, R.string.guardianproject_archive_description); } @@ -374,115 +378,128 @@ class DBHelper extends SQLiteOpenHelper { * calculate its fingerprint and save it to the database. */ private void addFingerprintToRepo(SQLiteDatabase db, int oldVersion) { - if (oldVersion < 44) { - if (!columnExists(db, TABLE_REPO, "fingerprint")) { - db.execSQL("alter table " + TABLE_REPO + " add column fingerprint text"); - } - List oldrepos = new ArrayList<>(); - Cursor cursor = db.query(TABLE_REPO, - new String[] {"address", "pubkey"}, - null, null, null, null, null); - if (cursor != null) { - if (cursor.getCount() > 0) { - cursor.moveToFirst(); - while (!cursor.isAfterLast()) { - Repo repo = new Repo(); - repo.address = cursor.getString(0); - repo.signingCertificate = cursor.getString(1); - oldrepos.add(repo); - cursor.moveToNext(); - } + if (oldVersion >= 44) { + return; + } + if (!columnExists(db, TABLE_REPO, "fingerprint")) { + db.execSQL("alter table " + TABLE_REPO + " add column fingerprint text"); + } + List oldrepos = new ArrayList<>(); + Cursor cursor = db.query(TABLE_REPO, + new String[] {"address", "pubkey"}, + null, null, null, null, null); + if (cursor != null) { + if (cursor.getCount() > 0) { + cursor.moveToFirst(); + while (!cursor.isAfterLast()) { + Repo repo = new Repo(); + repo.address = cursor.getString(0); + repo.signingCertificate = cursor.getString(1); + oldrepos.add(repo); + cursor.moveToNext(); } - cursor.close(); - } - for (final Repo repo : oldrepos) { - ContentValues values = new ContentValues(); - values.put("fingerprint", Utils.calcFingerprint(repo.signingCertificate)); - db.update(TABLE_REPO, values, "address = ?", new String[] {repo.address}); } + cursor.close(); + } + for (final Repo repo : oldrepos) { + ContentValues values = new ContentValues(); + values.put("fingerprint", Utils.calcFingerprint(repo.signingCertificate)); + db.update(TABLE_REPO, values, "address = ?", new String[] {repo.address}); } } private void addMaxAgeToRepo(SQLiteDatabase db, int oldVersion) { - if (oldVersion < 30 && !columnExists(db, TABLE_REPO, "maxage")) { - db.execSQL("alter table " + TABLE_REPO + " add column maxage integer not null default 0"); + if (oldVersion >= 30 || columnExists(db, TABLE_REPO, "maxage")) { + return; } + db.execSQL("alter table " + TABLE_REPO + " add column maxage integer not null default 0"); } private void addVersionToRepo(SQLiteDatabase db, int oldVersion) { - if (oldVersion < 33 && !columnExists(db, TABLE_REPO, "version")) { - db.execSQL("alter table " + TABLE_REPO + " add column version integer not null default 0"); + if (oldVersion >= 33 || columnExists(db, TABLE_REPO, "version")) { + return; } + db.execSQL("alter table " + TABLE_REPO + " add column version integer not null default 0"); } private void addLastUpdatedToRepo(SQLiteDatabase db, int oldVersion) { - if (oldVersion < 35 && !columnExists(db, TABLE_REPO, "lastUpdated")) { - Utils.debugLog(TAG, "Adding lastUpdated column to " + TABLE_REPO); - db.execSQL("Alter table " + TABLE_REPO + " add column lastUpdated string"); + if (oldVersion >= 35 || columnExists(db, TABLE_REPO, "lastUpdated")) { + return; } + Utils.debugLog(TAG, "Adding lastUpdated column to " + TABLE_REPO); + db.execSQL("Alter table " + TABLE_REPO + " add column lastUpdated string"); } private void addIsSwapToRepo(SQLiteDatabase db, int oldVersion) { - if (oldVersion < 47 && !columnExists(db, TABLE_REPO, "isSwap")) { - Utils.debugLog(TAG, "Adding isSwap field to " + TABLE_REPO + " table in db."); - db.execSQL("alter table " + TABLE_REPO + " add column isSwap boolean default 0;"); + if (oldVersion >= 47 || columnExists(db, TABLE_REPO, "isSwap")) { + return; } + Utils.debugLog(TAG, "Adding isSwap field to " + TABLE_REPO + " table in db."); + db.execSQL("alter table " + TABLE_REPO + " add column isSwap boolean default 0;"); } private void addCredentialsToRepo(SQLiteDatabase db, int oldVersion) { - if (oldVersion < 52) { - if (!columnExists(db, TABLE_REPO, "username")) { - Utils.debugLog(TAG, "Adding username field to " + TABLE_REPO + " table in db."); - db.execSQL("alter table " + TABLE_REPO + " add column username string;"); - } + if (oldVersion >= 52) { + return; + } + if (!columnExists(db, TABLE_REPO, "username")) { + Utils.debugLog(TAG, "Adding username field to " + TABLE_REPO + " table in db."); + db.execSQL("alter table " + TABLE_REPO + " add column username string;"); + } - if (!columnExists(db, TABLE_REPO, "password")) { - Utils.debugLog(TAG, "Adding password field to " + TABLE_REPO + " table in db."); - db.execSQL("alter table " + TABLE_REPO + " add column password string;"); - } + if (!columnExists(db, TABLE_REPO, "password")) { + Utils.debugLog(TAG, "Adding password field to " + TABLE_REPO + " table in db."); + db.execSQL("alter table " + TABLE_REPO + " add column password string;"); } } private void addChangelogToApp(SQLiteDatabase db, int oldVersion) { - if (oldVersion < 48 && !columnExists(db, TABLE_APP, "changelogURL")) { - Utils.debugLog(TAG, "Adding changelogURL column to " + TABLE_APP); - db.execSQL("alter table " + TABLE_APP + " add column changelogURL text"); + if (oldVersion >= 48 || columnExists(db, TABLE_APP, "changelogURL")) { + return; } + Utils.debugLog(TAG, "Adding changelogURL column to " + TABLE_APP); + db.execSQL("alter table " + TABLE_APP + " add column changelogURL text"); } private void addIconUrlLargeToApp(SQLiteDatabase db, int oldVersion) { - if (oldVersion < 49 && !columnExists(db, TABLE_APP, "iconUrlLarge")) { - Utils.debugLog(TAG, "Adding iconUrlLarge columns to " + TABLE_APP); - db.execSQL("alter table " + TABLE_APP + " add column iconUrlLarge text"); + if (oldVersion >= 49 || columnExists(db, TABLE_APP, "iconUrlLarge")) { + return; } + Utils.debugLog(TAG, "Adding iconUrlLarge columns to " + TABLE_APP); + db.execSQL("alter table " + TABLE_APP + " add column iconUrlLarge text"); } private void updateIconUrlLarge(SQLiteDatabase db, int oldVersion) { - if (oldVersion < 50) { - Utils.debugLog(TAG, "Recalculating app icon URLs so that the newly added large icons will get updated."); - AppProvider.UpgradeHelper.updateIconUrls(context, db); - clearRepoEtags(db); + if (oldVersion >= 50) { + return; } + Utils.debugLog(TAG, "Recalculating app icon URLs so that the newly added large icons will get updated."); + AppProvider.UpgradeHelper.updateIconUrls(context, db); + clearRepoEtags(db); } private void addAuthorToApp(SQLiteDatabase db, int oldVersion) { - if (oldVersion < 53 && !columnExists(db, TABLE_APP, "author")) { + if (oldVersion >= 53) { + return; + } + if (!columnExists(db, TABLE_APP, "author")) { Utils.debugLog(TAG, "Adding author column to " + TABLE_APP); db.execSQL("alter table " + TABLE_APP + " add column author text"); } - if (oldVersion < 53 && !columnExists(db, TABLE_APP, "email")) { + if (!columnExists(db, TABLE_APP, "email")) { Utils.debugLog(TAG, "Adding email column to " + TABLE_APP); db.execSQL("alter table " + TABLE_APP + " add column email text"); } } private void useMaxValueInMaxSdkVersion(SQLiteDatabase db, int oldVersion) { - if (oldVersion < 54) { - Utils.debugLog(TAG, "Converting maxSdkVersion value 0 to " + Byte.MAX_VALUE); - ContentValues values = new ContentValues(); - values.put(ApkProvider.DataColumns.MAX_SDK_VERSION, Byte.MAX_VALUE); - db.update(TABLE_APK, values, ApkProvider.DataColumns.MAX_SDK_VERSION + " < 1", null); + if (oldVersion >= 54) { + return; } + Utils.debugLog(TAG, "Converting maxSdkVersion value 0 to " + Byte.MAX_VALUE); + ContentValues values = new ContentValues(); + values.put(ApkProvider.DataColumns.MAX_SDK_VERSION, Byte.MAX_VALUE); + db.update(TABLE_APK, values, ApkProvider.DataColumns.MAX_SDK_VERSION + " < 1", null); } /** @@ -501,14 +518,15 @@ class DBHelper extends SQLiteOpenHelper { // was is specified by the user. We don't want to weely-neely nuke that data. // and the new way to deal with changes to the table structure is to add a // if (oldVersion < x && !columnExists(...) and then alter the table as required. - if (oldVersion < 42) { - context.getSharedPreferences("FDroid", Context.MODE_PRIVATE).edit() - .putBoolean("triedEmptyUpdate", false).commit(); - db.execSQL("drop table " + TABLE_APP); - db.execSQL("drop table " + TABLE_APK); - clearRepoEtags(db); - createAppApk(db); + if (oldVersion >= 42) { + return; } + context.getSharedPreferences("FDroid", Context.MODE_PRIVATE).edit() + .putBoolean("triedEmptyUpdate", false).commit(); + db.execSQL("drop table " + TABLE_APP); + db.execSQL("drop table " + TABLE_APK); + clearRepoEtags(db); + createAppApk(db); } private static void createAppApk(SQLiteDatabase db) { @@ -527,10 +545,11 @@ class DBHelper extends SQLiteOpenHelper { // If any column was added or removed, just drop the table, create it // again and let the cache be filled from scratch again. private void recreateInstalledCache(SQLiteDatabase db, int oldVersion) { - if (oldVersion < 51) { - db.execSQL(DROP_TABLE_INSTALLED_APP); - createInstalledApp(db); + if (oldVersion >= 51) { + return; } + db.execSQL(DROP_TABLE_INSTALLED_APP); + createInstalledApp(db); } private static boolean columnExists(SQLiteDatabase db, From 32c67d05ece713b05ac3adaec3a4bf8c49c61cd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Sat, 23 Apr 2016 15:59:04 +0100 Subject: [PATCH 04/15] PMD: XML config file, enable most of unnecessary --- app/build.gradle | 12 ++---------- .../views/CaffeinatedScrollView.java | 1 + config/pmd/rules.xml | 18 ++++++++++++++++++ 3 files changed, 21 insertions(+), 10 deletions(-) create mode 100644 config/pmd/rules.xml diff --git a/app/build.gradle b/app/build.gradle index e4c4fe60f..7442af8bf 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -208,16 +208,8 @@ pmd { } task pmd(type: Pmd, dependsOn: assembleDebug) { - ruleSets = [ - //'java-basic', - 'java-unusedcode', - 'java-android', - 'java-clone', - 'java-finalizers', - 'java-imports', - 'java-migrating', - //'java-unnecessary', // too nitpicky with parenthesis - ] + ruleSetFiles = files("${project.rootDir}/config/pmd/rules.xml") + ruleSets = [] // otherwise defaults clash with the list in rules.xml source 'src/main/java', 'src/test/java', 'src/androidTest/java' include '**/*.java' } diff --git a/app/src/main/java/org/fdroid/fdroid/privileged/views/CaffeinatedScrollView.java b/app/src/main/java/org/fdroid/fdroid/privileged/views/CaffeinatedScrollView.java index f43dbc053..dd3e7837a 100644 --- a/app/src/main/java/org/fdroid/fdroid/privileged/views/CaffeinatedScrollView.java +++ b/app/src/main/java/org/fdroid/fdroid/privileged/views/CaffeinatedScrollView.java @@ -40,6 +40,7 @@ public class CaffeinatedScrollView extends ScrollView { /** * Make this visible so we can call it */ + @SuppressWarnings("PMD.UselessOverridingMethod") @Override public boolean awakenScrollBars() { return super.awakenScrollBars(); diff --git a/config/pmd/rules.xml b/config/pmd/rules.xml new file mode 100644 index 000000000..1cf4ca4a9 --- /dev/null +++ b/config/pmd/rules.xml @@ -0,0 +1,18 @@ + + + + + + + + + + + + + + From 5d3d8786e2f8cb810ed9ad4fe561d332c212f96d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Sat, 23 Apr 2016 16:06:41 +0100 Subject: [PATCH 05/15] PMD: Enable and obey PrematureDeclaration --- .../privileged/install/InstallExtensionDialogActivity.java | 6 +++--- .../fdroid/privileged/views/AppSecurityPermissions.java | 6 ++---- config/pmd/rules.xml | 1 + 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/privileged/install/InstallExtensionDialogActivity.java b/app/src/main/java/org/fdroid/fdroid/privileged/install/InstallExtensionDialogActivity.java index 26bb0c299..221cfb3b5 100644 --- a/app/src/main/java/org/fdroid/fdroid/privileged/install/InstallExtensionDialogActivity.java +++ b/app/src/main/java/org/fdroid/fdroid/privileged/install/InstallExtensionDialogActivity.java @@ -328,9 +328,6 @@ public class InstallExtensionDialogActivity extends FragmentActivity { * 3. Verify that install worked */ private void postInstall() { - // hack to get theme applied (which is not automatically applied due to activity's Theme.NoDisplay - ContextThemeWrapper theme = new ContextThemeWrapper(this, FDroidApp.getCurThemeResId()); - int isInstalledCorrectly = PrivilegedInstaller.isExtensionInstalledCorrectly(this); @@ -367,6 +364,9 @@ public class InstallExtensionDialogActivity extends FragmentActivity { throw new RuntimeException("unhandled return"); } + // hack to get theme applied (which is not automatically applied due to activity's Theme.NoDisplay + ContextThemeWrapper theme = new ContextThemeWrapper(this, FDroidApp.getCurThemeResId()); + AlertDialog.Builder builder = new AlertDialog.Builder(theme) .setTitle(title) .setMessage(message) diff --git a/app/src/main/java/org/fdroid/fdroid/privileged/views/AppSecurityPermissions.java b/app/src/main/java/org/fdroid/fdroid/privileged/views/AppSecurityPermissions.java index e16a42efa..23afc1e91 100644 --- a/app/src/main/java/org/fdroid/fdroid/privileged/views/AppSecurityPermissions.java +++ b/app/src/main/java/org/fdroid/fdroid/privileged/views/AppSecurityPermissions.java @@ -420,16 +420,14 @@ public class AppSecurityPermissions { final int base = pInfo.protectionLevel & PermissionInfo.PROTECTION_MASK_BASE; final boolean isNormal = base == PermissionInfo.PROTECTION_NORMAL; final boolean isDangerous = base == PermissionInfo.PROTECTION_DANGEROUS; - final boolean wasGranted = - (existingReqFlags & PackageInfo.REQUESTED_PERMISSION_GRANTED) != 0; // Dangerous and normal permissions are always shown to the user if (isNormal || isDangerous) { return true; } - final boolean isDevelopment = - (pInfo.protectionLevel & PermissionInfo.PROTECTION_FLAG_DEVELOPMENT) != 0; + final boolean isDevelopment = (pInfo.protectionLevel & PermissionInfo.PROTECTION_FLAG_DEVELOPMENT) != 0; + final boolean wasGranted = (existingReqFlags & PackageInfo.REQUESTED_PERMISSION_GRANTED) != 0; // Development permissions are only shown to the user if they are already // granted to the app -- if we are installing an app and they are not diff --git a/config/pmd/rules.xml b/config/pmd/rules.xml index 1cf4ca4a9..314281ed0 100644 --- a/config/pmd/rules.xml +++ b/config/pmd/rules.xml @@ -15,4 +15,5 @@ + From 857b0ea1c744c750df427d4f08415495b7769bc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Sat, 23 Apr 2016 16:14:05 +0100 Subject: [PATCH 06/15] PMD: Enable and obey AddEmptyString --- app/src/main/java/org/fdroid/fdroid/data/AppProvider.java | 2 +- app/src/main/java/org/fdroid/fdroid/net/LocalHTTPD.java | 6 +++--- .../org/fdroid/fdroid/net/bluetooth/BluetoothServer.java | 6 +++--- config/pmd/rules.xml | 1 + 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java index 4116638bb..96475ea42 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java @@ -572,7 +572,7 @@ public class AppProvider extends FDroidProvider { public static Uri getSearchUri(Repo repo, String query) { return getContentUri().buildUpon() .appendPath(PATH_SEARCH_REPO) - .appendPath(repo.id + "") + .appendPath(String.valueOf(repo.id)) .appendPath(query) .build(); } diff --git a/app/src/main/java/org/fdroid/fdroid/net/LocalHTTPD.java b/app/src/main/java/org/fdroid/fdroid/net/LocalHTTPD.java index 23937b9ec..d6d2f7633 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/LocalHTTPD.java +++ b/app/src/main/java/org/fdroid/fdroid/net/LocalHTTPD.java @@ -221,7 +221,7 @@ public class LocalHTTPD extends NanoHTTPD { try { // Calculate etag String etag = Integer - .toHexString((file.getAbsolutePath() + file.lastModified() + "" + file.length()) + .toHexString((file.getAbsolutePath() + file.lastModified() + String.valueOf(file.length())) .hashCode()); // Support (simple) skipping: @@ -268,7 +268,7 @@ public class LocalHTTPD extends NanoHTTPD { fis.skip(startFrom); res = createResponse(Response.Status.PARTIAL_CONTENT, mime, fis); - res.addHeader("Content-Length", "" + dataLen); + res.addHeader("Content-Length", String.valueOf(dataLen)); res.addHeader("Content-Range", "bytes " + startFrom + "-" + endAt + "/" + fileLen); res.addHeader("ETag", etag); @@ -278,7 +278,7 @@ public class LocalHTTPD extends NanoHTTPD { res = createResponse(Response.Status.NOT_MODIFIED, mime, ""); } else { res = createResponse(Response.Status.OK, mime, new FileInputStream(file)); - res.addHeader("Content-Length", "" + fileLen); + res.addHeader("Content-Length", String.valueOf(fileLen)); res.addHeader("ETag", etag); } } diff --git a/app/src/main/java/org/fdroid/fdroid/net/bluetooth/BluetoothServer.java b/app/src/main/java/org/fdroid/fdroid/net/bluetooth/BluetoothServer.java index 811508992..dc935199f 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/bluetooth/BluetoothServer.java +++ b/app/src/main/java/org/fdroid/fdroid/net/bluetooth/BluetoothServer.java @@ -259,7 +259,7 @@ public class BluetoothServer extends Thread { try { // Calculate etag String etag = Integer - .toHexString((file.getAbsolutePath() + file.lastModified() + "" + file.length()) + .toHexString((file.getAbsolutePath() + file.lastModified() + String.valueOf(file.length())) .hashCode()); // Support (simple) skipping: @@ -306,7 +306,7 @@ public class BluetoothServer extends Thread { fis.skip(startFrom); res = createResponse(NanoHTTPD.Response.Status.PARTIAL_CONTENT, mime, fis); - res.addHeader("Content-Length", "" + dataLen); + res.addHeader("Content-Length", String.valueOf(dataLen)); res.addHeader("Content-Range", "bytes " + startFrom + "-" + endAt + "/" + fileLen); res.addHeader("ETag", etag); @@ -316,7 +316,7 @@ public class BluetoothServer extends Thread { res = createResponse(NanoHTTPD.Response.Status.NOT_MODIFIED, mime, ""); } else { res = createResponse(NanoHTTPD.Response.Status.OK, mime, new FileInputStream(file)); - res.addHeader("Content-Length", "" + fileLen); + res.addHeader("Content-Length", String.valueOf(fileLen)); res.addHeader("ETag", etag); } } diff --git a/config/pmd/rules.xml b/config/pmd/rules.xml index 314281ed0..fbde9c171 100644 --- a/config/pmd/rules.xml +++ b/config/pmd/rules.xml @@ -16,4 +16,5 @@ + From f655f49aee34407e1cc61fad11c2e12aaa5c4d6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Sat, 23 Apr 2016 16:17:55 +0100 Subject: [PATCH 07/15] PMD: Enable and obey UnnecessaryConstructor --- .../main/java/org/fdroid/fdroid/localrepo/SwapService.java | 4 ---- config/pmd/rules.xml | 1 + 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/localrepo/SwapService.java b/app/src/main/java/org/fdroid/fdroid/localrepo/SwapService.java index 818b26725..a9aedff12 100644 --- a/app/src/main/java/org/fdroid/fdroid/localrepo/SwapService.java +++ b/app/src/main/java/org/fdroid/fdroid/localrepo/SwapService.java @@ -91,10 +91,6 @@ public class SwapService extends Service { @NonNull private final Set appsToSwap = new HashSet<>(); - public SwapService() { - super(); - } - /** * Where relevant, the state of the swap process will be saved to disk using preferences. * Note that this is not always useful, for example saving the "current wifi network" is diff --git a/config/pmd/rules.xml b/config/pmd/rules.xml index fbde9c171..cab42517c 100644 --- a/config/pmd/rules.xml +++ b/config/pmd/rules.xml @@ -17,4 +17,5 @@ + From c746a49b155638f6edf2c12a5063b6e98fa7bb6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Sat, 23 Apr 2016 16:22:47 +0100 Subject: [PATCH 08/15] PMD: Enable some design.xml rules we already obey --- config/pmd/rules.xml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/config/pmd/rules.xml b/config/pmd/rules.xml index cab42517c..34bdcfbb1 100644 --- a/config/pmd/rules.xml +++ b/config/pmd/rules.xml @@ -18,4 +18,11 @@ + + + + + + + From 68db3ae353d6adb7bd8838b72256573163f0e36d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Sat, 23 Apr 2016 16:25:26 +0100 Subject: [PATCH 09/15] PMD: Enable and obey ImmutableField --- app/src/androidTest/java/mock/MockFDroidResources.java | 2 +- .../androidTest/java/mock/MockInstallablePackageManager.java | 2 +- app/src/main/java/com/google/zxing/encode/QRCodeEncoder.java | 2 +- .../java/org/fdroid/fdroid/installer/PrivilegedInstaller.java | 2 +- config/pmd/rules.xml | 1 + 5 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/src/androidTest/java/mock/MockFDroidResources.java b/app/src/androidTest/java/mock/MockFDroidResources.java index b559bd83e..5cb33798c 100644 --- a/app/src/androidTest/java/mock/MockFDroidResources.java +++ b/app/src/androidTest/java/mock/MockFDroidResources.java @@ -7,7 +7,7 @@ import org.fdroid.fdroid.R; public class MockFDroidResources extends MockResources { - private Context getStringDelegatingContext; + private final Context getStringDelegatingContext; public MockFDroidResources(Context getStringDelegatingContext) { this.getStringDelegatingContext = getStringDelegatingContext; diff --git a/app/src/androidTest/java/mock/MockInstallablePackageManager.java b/app/src/androidTest/java/mock/MockInstallablePackageManager.java index 4ff3aef71..b7056711f 100644 --- a/app/src/androidTest/java/mock/MockInstallablePackageManager.java +++ b/app/src/androidTest/java/mock/MockInstallablePackageManager.java @@ -10,7 +10,7 @@ import java.util.List; public class MockInstallablePackageManager extends MockPackageManager { - private List info = new ArrayList<>(); + private final List info = new ArrayList<>(); @Override public List getInstalledPackages(int flags) { diff --git a/app/src/main/java/com/google/zxing/encode/QRCodeEncoder.java b/app/src/main/java/com/google/zxing/encode/QRCodeEncoder.java index 6200701f6..075209055 100755 --- a/app/src/main/java/com/google/zxing/encode/QRCodeEncoder.java +++ b/app/src/main/java/com/google/zxing/encode/QRCodeEncoder.java @@ -37,7 +37,7 @@ public final class QRCodeEncoder { private static final int WHITE = 0xFFFFFFFF; private static final int BLACK = 0xFF000000; - private int dimension = Integer.MIN_VALUE; + private final int dimension; private String contents; private String displayContents; private String title; diff --git a/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java b/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java index ed9419d4c..b24adb690 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java @@ -76,7 +76,7 @@ public class PrivilegedInstaller extends Installer { private static final String PRIVILEGED_EXTENSION_SERVICE_INTENT = "org.fdroid.fdroid.privileged.IPrivilegedService"; public static final String PRIVILEGED_EXTENSION_PACKAGE_NAME = "org.fdroid.fdroid.privileged"; - private Activity mActivity; + private final Activity mActivity; private static final int REQUEST_CONFIRM_PERMS = 0; diff --git a/config/pmd/rules.xml b/config/pmd/rules.xml index 34bdcfbb1..7af77cb40 100644 --- a/config/pmd/rules.xml +++ b/config/pmd/rules.xml @@ -25,4 +25,5 @@ + From 50b2e6f7a548e59b44f8a4ca8776c171aee13f23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Sat, 23 Apr 2016 17:25:04 +0100 Subject: [PATCH 10/15] PMD: Enable and obey SingularField --- app/src/androidTest/java/org/fdroid/fdroid/FileCompatTest.java | 3 +-- .../java/org/fdroid/fdroid/localrepo/type/BluetoothSwap.java | 3 +-- config/pmd/rules.xml | 1 + 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/app/src/androidTest/java/org/fdroid/fdroid/FileCompatTest.java b/app/src/androidTest/java/org/fdroid/fdroid/FileCompatTest.java index 97fe44ecf..819d8539b 100644 --- a/app/src/androidTest/java/org/fdroid/fdroid/FileCompatTest.java +++ b/app/src/androidTest/java/org/fdroid/fdroid/FileCompatTest.java @@ -30,14 +30,13 @@ public class FileCompatTest { private static final String TAG = "FileCompatTest"; - private File dir; private SanitizedFile sourceFile; private SanitizedFile destFile; @Before public void setUp() { Instrumentation instrumentation = InstrumentationRegistry.getInstrumentation(); - dir = TestUtils.getWriteableDir(instrumentation); + File dir = TestUtils.getWriteableDir(instrumentation); sourceFile = SanitizedFile.knownSanitized(TestUtils.copyAssetToDir(instrumentation.getContext(), "simpleIndex.jar", dir)); destFile = new SanitizedFile(dir, "dest-" + UUID.randomUUID() + ".testproduct"); assertFalse(destFile.exists()); diff --git a/app/src/main/java/org/fdroid/fdroid/localrepo/type/BluetoothSwap.java b/app/src/main/java/org/fdroid/fdroid/localrepo/type/BluetoothSwap.java index 31c191a52..8b09c4c6f 100644 --- a/app/src/main/java/org/fdroid/fdroid/localrepo/type/BluetoothSwap.java +++ b/app/src/main/java/org/fdroid/fdroid/localrepo/type/BluetoothSwap.java @@ -22,7 +22,6 @@ public final class BluetoothSwap extends SwapType { @NonNull private final BluetoothAdapter adapter; - private BroadcastReceiver receiver; private boolean isDiscoverable; @Nullable @@ -64,7 +63,7 @@ public final class BluetoothSwap extends SwapType { return; } - receiver = new BroadcastReceiver() { + BroadcastReceiver receiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { switch (intent.getIntExtra(BluetoothAdapter.EXTRA_SCAN_MODE, -1)) { diff --git a/config/pmd/rules.xml b/config/pmd/rules.xml index 7af77cb40..34775e15e 100644 --- a/config/pmd/rules.xml +++ b/config/pmd/rules.xml @@ -26,4 +26,5 @@ + From 145314a83a5e6bee1c718b8f9747dcb31ec0e336 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Sat, 23 Apr 2016 17:38:42 +0100 Subject: [PATCH 11/15] UpdateService: add missing cursor.close() Found by Android Studio. --- .../java/org/fdroid/fdroid/UpdateService.java | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/UpdateService.java b/app/src/main/java/org/fdroid/fdroid/UpdateService.java index a9a631a4a..67318670e 100644 --- a/app/src/main/java/org/fdroid/fdroid/UpdateService.java +++ b/app/src/main/java/org/fdroid/fdroid/UpdateService.java @@ -489,15 +489,18 @@ public class UpdateService extends IntentService implements ProgressListener { AppProvider.DataColumns.PACKAGE_NAME, AppProvider.DataColumns.SUGGESTED_VERSION_CODE, }, null, null, null); - cursor.moveToFirst(); - for (int i = 0; i < cursor.getCount(); i++) { - App app = new App(cursor); - Apk apk = ApkProvider.Helper.find(this, app.packageName, app.suggestedVersionCode, new String[]{ - ApkProvider.DataColumns.NAME, - }); - String urlString = Utils.getApkUrl(repoAddress, apk); - DownloaderService.queue(this, app.packageName, urlString); - cursor.moveToNext(); + if (cursor != null) { + cursor.moveToFirst(); + for (int i = 0; i < cursor.getCount(); i++) { + App app = new App(cursor); + Apk apk = ApkProvider.Helper.find(this, app.packageName, app.suggestedVersionCode, new String[]{ + ApkProvider.DataColumns.NAME, + }); + String urlString = Utils.getApkUrl(repoAddress, apk); + DownloaderService.queue(this, app.packageName, urlString); + cursor.moveToNext(); + } + cursor.close(); } } From f1230adfc13d2a371840ca08066cefc868076c31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Sat, 23 Apr 2016 17:47:35 +0100 Subject: [PATCH 12/15] Apply a few suggestions from Android Studio --- .../org/fdroid/fdroid/AndroidXMLDecompress.java | 14 ++++++-------- .../main/java/org/fdroid/fdroid/AppDetails.java | 3 +-- .../org/fdroid/fdroid/installer/Installer.java | 5 +---- .../org/fdroid/fdroid/net/DownloaderService.java | 2 +- 4 files changed, 9 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/AndroidXMLDecompress.java b/app/src/main/java/org/fdroid/fdroid/AndroidXMLDecompress.java index b543764b1..86ed380d3 100644 --- a/app/src/main/java/org/fdroid/fdroid/AndroidXMLDecompress.java +++ b/app/src/main/java/org/fdroid/fdroid/AndroidXMLDecompress.java @@ -68,7 +68,7 @@ import java.util.zip.ZipFile; * @see a binary XML parser */ public class AndroidXMLDecompress { - public static int startTag = 0x00100102; + public static final int START_TAG = 0x00100102; /** * Just get the XML attributes from the {@code } element. @@ -82,7 +82,7 @@ public class AndroidXMLDecompress { int stringTableOffset = stringIndexTableOffset + numbStrings * 4; int xmlTagOffset = littleEndianWord(binaryXml, 3 * 4); for (int i = xmlTagOffset; i < binaryXml.length - 4; i += 4) { - if (littleEndianWord(binaryXml, i) == startTag) { + if (littleEndianWord(binaryXml, i) == START_TAG) { xmlTagOffset = i; break; } @@ -92,11 +92,11 @@ public class AndroidXMLDecompress { while (offset < binaryXml.length) { int tag0 = littleEndianWord(binaryXml, offset); - if (tag0 == startTag) { + if (tag0 == START_TAG) { int numbAttrs = littleEndianWord(binaryXml, offset + 7 * 4); offset += 9 * 4; - HashMap attributes = new HashMap(3); + HashMap attributes = new HashMap<>(3); for (int i = 0; i < numbAttrs; i++) { int attributeNameStringIndex = littleEndianWord(binaryXml, offset + 1 * 4); int attributeValueStringIndex = littleEndianWord(binaryXml, offset + 2 * 4); @@ -117,7 +117,7 @@ public class AndroidXMLDecompress { // we only need the first start tag break; } - return new HashMap(0); + return new HashMap<>(0); } public static byte[] getManifestFromFilename(String filename) throws IOException { @@ -137,9 +137,7 @@ public class AndroidXMLDecompress { is.read(buf); is.close(); - if (zip != null) { - zip.close(); - } + zip.close(); return buf; } diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails.java b/app/src/main/java/org/fdroid/fdroid/AppDetails.java index a431bd01b..073a86a14 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails.java @@ -865,8 +865,7 @@ public class AppDetails extends AppCompatActivity { } private void startDownload(Apk apk, String repoAddress) { - String urlString = Utils.getApkUrl(repoAddress, apk); - activeDownloadUrlString = urlString; + activeDownloadUrlString = Utils.getApkUrl(repoAddress, apk); registerDownloaderReceivers(); headerFragment.startProgress(); DownloaderService.queue(this, apk.packageName, activeDownloadUrlString); diff --git a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java index 3ead44b3c..a0a549448 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java @@ -162,10 +162,7 @@ public abstract class Installer { return false; } Hasher hasher = new Hasher(hashType, apkFile); - if (hasher != null && hasher.match(hash)) { - return true; - } - return false; + return hasher.match(hash); } /** diff --git a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java index 2d31a6574..05e5cce37 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java @@ -94,7 +94,7 @@ public class DownloaderService extends Service { private static volatile Downloader downloader; private LocalBroadcastManager localBroadcastManager; - private static final HashMap QUEUE_WHATS = new HashMap(); + private static final HashMap QUEUE_WHATS = new HashMap<>(); private int what; private final class ServiceHandler extends Handler { From e860503141df1986aa849b3dcc4e82a5fd8102d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Sun, 24 Apr 2016 12:45:35 +0100 Subject: [PATCH 13/15] CONTRIBUTING: Add Android.mk gradlew note See #632. --- CONTRIBUTING.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7441a1282..595d555ab 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -58,6 +58,8 @@ To get all the logcat messages by F-Droid, you can run: * Use gradle with `--daemon` if you are going to build F-Droid multiple times. * If you get a message like `Could not find com.android.support:support-...`, make sure that you have the latest Android support maven repository. +* When building as part of AOSP with `Android.mk`, make sure you have a + recent version of Gradle installed as `gradlew` will not be used. ## Running the test suite From 70da6eaa1204540269a71f6e658c7f9efb084090 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Sun, 24 Apr 2016 14:57:32 +0100 Subject: [PATCH 14/15] Fix NPE when clicking on missing app links This happened to me when I clicked on an app link for a new app which was already in the repo, but I had not ran an index update yet since the app got built. Stack trace prior to the fix follows. java.lang.RuntimeException: Unable to destroy activity {org.fdroid.fdroid/org.fdroid.fdroid.AppDetails}: java.lang.NullPointerException: Attempt to invoke virtual method 'void android.support.v4.content.LocalBroadcastManager.unregisterReceiver(android.content.BroadcastReceiver)' on a null object reference at android.app.ActivityThread.performDestroyActivity(ActivityThread.java:3865) at android.app.ActivityThread.handleDestroyActivity(ActivityThread.java:3883) at android.app.ActivityThread.-wrap5(ActivityThread.java) at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1417) at android.os.Handler.dispatchMessage(Handler.java:102) at android.os.Looper.loop(Looper.java:148) at android.app.ActivityThread.main(ActivityThread.java:5461) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:726) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:616) Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'void android.support.v4.content.LocalBroadcastManager.unregisterReceiver(android.content.BroadcastReceiver)' on a null object reference at org.fdroid.fdroid.AppDetails.unregisterDownloaderReceivers(AppDetails.java:469) at org.fdroid.fdroid.AppDetails.onDestroy(AppDetails.java:569) at android.app.Activity.performDestroy(Activity.java:6422) at android.app.Instrumentation.callActivityOnDestroy(Instrumentation.java:1143) at android.app.ActivityThread.performDestroyActivity(ActivityThread.java:3852) ... 9 more --- app/src/main/java/org/fdroid/fdroid/AppDetails.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails.java b/app/src/main/java/org/fdroid/fdroid/AppDetails.java index 073a86a14..c29e6ae36 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails.java @@ -466,6 +466,9 @@ public class AppDetails extends AppCompatActivity { } private void unregisterDownloaderReceivers() { + if (localBroadcastManager == null) { + return; + } localBroadcastManager.unregisterReceiver(startedReceiver); localBroadcastManager.unregisterReceiver(progressReceiver); localBroadcastManager.unregisterReceiver(completeReceiver); From 87a4cfb27cce49323920dc105a2042531f9862d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Sun, 24 Apr 2016 15:06:11 +0100 Subject: [PATCH 15/15] Get rid of Log.d calls in src/main Also, Log.d in tests don't make much sense - replace by Log.i. This way it's easier to limit all Log.d calls to Utils.java. --- .../java/org/fdroid/fdroid/MultiRepoUpdaterTest.java | 4 ++-- app/src/androidTest/java/org/fdroid/fdroid/TestUtils.java | 2 +- .../main/java/org/fdroid/fdroid/data/RepoPersister.java | 3 +-- .../org/fdroid/fdroid/installer/ApkSignatureVerifier.java | 8 ++++---- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/app/src/androidTest/java/org/fdroid/fdroid/MultiRepoUpdaterTest.java b/app/src/androidTest/java/org/fdroid/fdroid/MultiRepoUpdaterTest.java index 7d0364623..ad72da4df 100644 --- a/app/src/androidTest/java/org/fdroid/fdroid/MultiRepoUpdaterTest.java +++ b/app/src/androidTest/java/org/fdroid/fdroid/MultiRepoUpdaterTest.java @@ -160,7 +160,7 @@ public class MultiRepoUpdaterTest extends InstrumentationTestCase { * repository indeed contains the apks that it said it would provide. */ private void assertExpected() { - Log.d(TAG, "Asserting all versions of each .apk are in index."); + Log.i(TAG, "Asserting all versions of each .apk are in index."); List repos = RepoProvider.Helper.all(context); assertEquals("Repos", 3, repos.size()); @@ -173,7 +173,7 @@ public class MultiRepoUpdaterTest extends InstrumentationTestCase { * */ private void assertSomewhatAcceptable() { - Log.d(TAG, "Asserting at least one versions of each .apk is in index."); + Log.i(TAG, "Asserting at least one versions of each .apk is in index."); List repos = RepoProvider.Helper.all(context); assertEquals("Repos", 3, repos.size()); diff --git a/app/src/androidTest/java/org/fdroid/fdroid/TestUtils.java b/app/src/androidTest/java/org/fdroid/fdroid/TestUtils.java index 7b6fc2abf..62e90189b 100644 --- a/app/src/androidTest/java/org/fdroid/fdroid/TestUtils.java +++ b/app/src/androidTest/java/org/fdroid/fdroid/TestUtils.java @@ -200,7 +200,7 @@ public class TestUtils { OutputStream output = null; try { tempFile = File.createTempFile(assetName + "-", ".testasset", directory); - Log.d(TAG, "Copying asset file " + assetName + " to directory " + directory); + Log.i(TAG, "Copying asset file " + assetName + " to directory " + directory); input = context.getAssets().open(assetName); output = new FileOutputStream(tempFile); Utils.copy(input, output); diff --git a/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java b/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java index ca05dd631..2138f8575 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java +++ b/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java @@ -8,7 +8,6 @@ import android.net.Uri; import android.os.RemoteException; import android.support.annotation.NonNull; import android.support.annotation.Nullable; -import android.util.Log; import org.fdroid.fdroid.CompatibilityChecker; import org.fdroid.fdroid.RepoUpdater; @@ -99,7 +98,7 @@ public class RepoPersister { } if (apksToSave.size() > 0 || appsToSave.size() > 0) { - Log.d(TAG, "Flushing details of up to " + MAX_APP_BUFFER + " apps and their packages to the database."); + Utils.debugLog(TAG, "Flushing details of up to " + MAX_APP_BUFFER + " apps and their packages to the database."); flushAppsToDbInBatch(); flushApksToDbInBatch(); apksToSave.clear(); diff --git a/app/src/main/java/org/fdroid/fdroid/installer/ApkSignatureVerifier.java b/app/src/main/java/org/fdroid/fdroid/installer/ApkSignatureVerifier.java index c13049956..27b6aeb3f 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/ApkSignatureVerifier.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/ApkSignatureVerifier.java @@ -24,8 +24,8 @@ import android.content.Context; import android.content.pm.PackageInfo; import android.content.pm.PackageManager; import android.content.pm.Signature; -import android.util.Log; +import org.fdroid.fdroid.Utils; import org.spongycastle.util.encoders.Hex; import java.io.ByteArrayOutputStream; @@ -58,9 +58,9 @@ public class ApkSignatureVerifier { return true; } - Log.d(TAG, "Signature mismatch!"); - Log.d(TAG, "APK sig: " + Hex.toHexString(getApkSignature(apkFile))); - Log.d(TAG, "F-Droid sig: " + Hex.toHexString(getFDroidSignature())); + Utils.debugLog(TAG, "Signature mismatch!"); + Utils.debugLog(TAG, "APK sig: " + Hex.toHexString(getApkSignature(apkFile))); + Utils.debugLog(TAG, "F-Droid sig: " + Hex.toHexString(getFDroidSignature())); return false; }