Save per-apk anti features to join table + tests.

Also use a newer version of testy.at.or.at index for the index-v1 test,
because it includes apks with "KnownVuln" anti features whereas the
older version did not.

app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java

@@ -9,6 +9,10 @@ import android.net.Uri;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 import android.util.Log;
+
+import org.fdroid.fdroid.Utils;
+import org.fdroid.fdroid.data.Schema.AntiFeatureTable;
+import org.fdroid.fdroid.data.Schema.ApkAntiFeatureJoinTable;
 import org.fdroid.fdroid.data.Schema.ApkTable;
 import org.fdroid.fdroid.data.Schema.ApkTable.Cols;
 import org.fdroid.fdroid.data.Schema.AppMetadataTable;
@@ -17,8 +21,10 @@ import org.fdroid.fdroid.data.Schema.RepoTable;
 
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 @SuppressWarnings("LineLength")
 public class ApkProvider extends FDroidProvider {
@@ -266,6 +272,10 @@ public class ApkProvider extends FDroidProvider {
         return ApkTable.NAME;
     }
 
+    protected String getApkAntiFeatureJoinTableName() {
+        return ApkAntiFeatureJoinTable.NAME;
+    }
+
     protected String getAppTableName() {
         return AppMetadataTable.NAME;
     }
@@ -283,6 +293,18 @@ public class ApkProvider extends FDroidProvider {
     private class Query extends QueryBuilder {
 
         private boolean repoTableRequired;
+        private boolean antiFeaturesRequested;
+
+        /**
+         * If the query includes anti features, then we group by apk id. This is because joining onto the anti-features
+         * table will result in multiple result rows for each apk (potentially), so we will GROUP_CONCAT each of the
+         * anti features into a single comma separated list for each apk. If we are _not_ including anti features, then
+         * don't group by apk, because when doing a COUNT(*) this will result in the wrong result.
+         */
+        @Override
+        protected String groupBy() {
+            return antiFeaturesRequested ? "apk." + Cols.ROW_ID : null;
+        }
 
         @Override
         protected String getRequiredTables() {
@@ -301,6 +323,9 @@ public class ApkProvider extends FDroidProvider {
                 addPackageField(PACKAGE_FIELDS.get(field), field);
             } else if (REPO_FIELDS.containsKey(field)) {
                 addRepoField(REPO_FIELDS.get(field), field);
+            } else if (Cols.AntiFeatures.ANTI_FEATURES.equals(field)) {
+                antiFeaturesRequested = true;
+                addAntiFeatures();
             } else if (field.equals(Cols._ID)) {
                 appendField("rowid", "apk", "_id");
             } else if (field.equals(Cols._COUNT)) {
@@ -324,6 +349,18 @@ public class ApkProvider extends FDroidProvider {
             appendField(field, "repo", alias);
         }
 
+        private void addAntiFeatures() {
+            String apkAntiFeature = "apkAntiFeatureJoin";
+            String antiFeature = "antiFeature";
+
+            leftJoin(getApkAntiFeatureJoinTableName(), apkAntiFeature,
+                    "apk." + Cols.ROW_ID + " = " + apkAntiFeature + "." + ApkAntiFeatureJoinTable.Cols.APK_ID);
+
+            leftJoin(AntiFeatureTable.NAME, antiFeature,
+                    apkAntiFeature + "." + ApkAntiFeatureJoinTable.Cols.ANTI_FEATURE_ID + " = " + antiFeature + "." + AntiFeatureTable.Cols.ROW_ID);
+
+            appendField("group_concat(" + antiFeature + "." + AntiFeatureTable.Cols.NAME + ") as " + Cols.AntiFeatures.ANTI_FEATURES);
+        }
     }
 
     private QuerySelection queryPackage(String packageName) {
@@ -508,15 +545,73 @@ public class ApkProvider extends FDroidProvider {
 
     @Override
     public Uri insert(Uri uri, ContentValues values) {
+        boolean saveAntiFeatures = false;
+        String[] antiFeatures = null;
+        if (values.containsKey(Cols.AntiFeatures.ANTI_FEATURES)) {
+            saveAntiFeatures = true;
+            String antiFeaturesString = values.getAsString(Cols.AntiFeatures.ANTI_FEATURES);
+            antiFeatures = Utils.parseCommaSeparatedString(antiFeaturesString);
+            values.remove(Cols.AntiFeatures.ANTI_FEATURES);
+        }
+
         removeFieldsFromOtherTables(values);
         validateFields(Cols.ALL, values);
         long newId = db().insertOrThrow(getTableName(), null, values);
+
+        if (saveAntiFeatures) {
+            ensureAntiFeatures(antiFeatures, newId);
+        }
+
         if (!isApplyingBatch()) {
             getContext().getContentResolver().notifyChange(uri, null);
         }
         return getApkUri(newId);
     }
 
+    protected void ensureAntiFeatures(String[] antiFeatures, long apkId) {
+        db().delete(getApkAntiFeatureJoinTableName(), ApkAntiFeatureJoinTable.Cols.APK_ID + " = ?", new String[] {Long.toString(apkId)});
+        if (antiFeatures != null) {
+            Set<String> antiFeatureSet = new HashSet<>();
+            for (String antiFeatureName : antiFeatures) {
+
+                // There is nothing stopping a server repeating a category name in the metadata of
+                // an app. In order to prevent unique constraint violations, only insert once into
+                // the join table.
+                if (antiFeatureSet.contains(antiFeatureName)) {
+                    continue;
+                }
+
+                antiFeatureSet.add(antiFeatureName);
+
+                long antiFeatureId = ensureAntiFeature(antiFeatureName);
+                ContentValues categoryValues = new ContentValues(2);
+                categoryValues.put(ApkAntiFeatureJoinTable.Cols.APK_ID, apkId);
+                categoryValues.put(ApkAntiFeatureJoinTable.Cols.ANTI_FEATURE_ID, antiFeatureId);
+                db().insert(getApkAntiFeatureJoinTableName(), null, categoryValues);
+            }
+        }
+    }
+
+    protected long ensureAntiFeature(String antiFeatureName) {
+        long antiFeatureId = 0;
+        Cursor cursor = db().query(AntiFeatureTable.NAME, new String[] {AntiFeatureTable.Cols.ROW_ID}, AntiFeatureTable.Cols.NAME + " = ?", new String[]{antiFeatureName}, null, null, null);
+        if (cursor != null) {
+            if (cursor.getCount() > 0) {
+                cursor.moveToFirst();
+                antiFeatureId = cursor.getLong(0);
+            }
+            cursor.close();
+        }
+
+        if (antiFeatureId <= 0) {
+            ContentValues values = new ContentValues(1);
+            values.put(AntiFeatureTable.Cols.NAME, antiFeatureName);
+            antiFeatureId = db().insert(AntiFeatureTable.NAME, null, values);
+        }
+
+        return antiFeatureId;
+    }
+
     @Override
     public int delete(Uri uri, String where, String[] whereArgs) {
 
@@ -549,6 +644,15 @@ public class ApkProvider extends FDroidProvider {
             throw new UnsupportedOperationException("Cannot update anything other than a single apk.");
         }
 
+        boolean saveAntiFeatures = false;
+        String[] antiFeatures = null;
+        if (values.containsKey(Cols.AntiFeatures.ANTI_FEATURES)) {
+            saveAntiFeatures = true;
+            String antiFeaturesString = values.getAsString(Cols.AntiFeatures.ANTI_FEATURES);
+            antiFeatures = Utils.parseCommaSeparatedString(antiFeaturesString);
+            values.remove(Cols.AntiFeatures.ANTI_FEATURES);
+        }
+
         validateFields(Cols.ALL, values);
         removeFieldsFromOtherTables(values);
 
@@ -556,6 +660,19 @@ public class ApkProvider extends FDroidProvider {
         query = query.add(querySingleWithAppId(uri));
 
         int numRows = db().update(getTableName(), values, query.getSelection(), query.getArgs());
+
+        if (saveAntiFeatures) {
+            // Get the database ID of the row we just updated, so that we can join relevant anti features to it.
+            Cursor result = db().query(getTableName(), new String[]{Cols.ROW_ID},
+                    query.getSelection(), query.getArgs(), null, null, null);
+            if (result != null) {
+                result.moveToFirst();
+                long apkId = result.getLong(0);
+                ensureAntiFeatures(antiFeatures, apkId);
+                result.close();
+            }
+        }
+
         if (!isApplyingBatch()) {
             getContext().getContentResolver().notifyChange(uri, null);
         }

app/src/main/java/org/fdroid/fdroid/data/DBHelper.java

@@ -200,7 +200,7 @@ class DBHelper extends SQLiteOpenHelper {
             + AntiFeatureTable.Cols.NAME + " TEXT NOT NULL "
             + " );";
 
-    private static final String CREATE_TABLE_APK_ANTI_FEATURE_JOIN = "CREATE TABLE " + ApkAntiFeatureJoinTable.NAME
+    static final String CREATE_TABLE_APK_ANTI_FEATURE_JOIN = "CREATE TABLE " + ApkAntiFeatureJoinTable.NAME
             + " ( "
             + ApkAntiFeatureJoinTable.Cols.APK_ID + " INT NOT NULL, "
             + ApkAntiFeatureJoinTable.Cols.ANTI_FEATURE_ID + " INT NOT NULL, "

app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java

@@ -36,6 +36,11 @@ public class TempApkProvider extends ApkProvider {
         return TABLE_TEMP_APK;
     }
 
+    @Override
+    protected String getApkAntiFeatureJoinTableName() {
+        return TempAppProvider.TABLE_TEMP_APK_ANTI_FEATURE_JOIN;
+    }
+
     @Override
     protected String getAppTableName() {
         return TempAppProvider.TABLE_TEMP_APP;
@@ -93,11 +98,23 @@ public class TempApkProvider extends ApkProvider {
         final SQLiteDatabase db = db();
         final String memoryDbName = TempAppProvider.DB;
         db.execSQL(DBHelper.CREATE_TABLE_APK.replaceFirst(ApkTable.NAME, memoryDbName + "." + getTableName()));
+        db.execSQL(DBHelper.CREATE_TABLE_APK_ANTI_FEATURE_JOIN.replaceFirst(Schema.ApkAntiFeatureJoinTable.NAME, memoryDbName + "." + getApkAntiFeatureJoinTableName()));
 
         String where = ApkTable.NAME + "." + Cols.REPO_ID + " != ?";
         String[] whereArgs = new String[]{Long.toString(repoIdBeingUpdated)};
         db.execSQL(TempAppProvider.copyData(Cols.ALL_COLS, ApkTable.NAME, memoryDbName + "." + getTableName(), where), whereArgs);
 
+        String antiFeaturesWhere =
+                Schema.ApkAntiFeatureJoinTable.NAME + "." + Schema.ApkAntiFeatureJoinTable.Cols.APK_ID + " IN " +
+                "(SELECT innerApk." + Cols.ROW_ID + " FROM " + ApkTable.NAME + " AS innerApk " +
+                "WHERE innerApk." + Cols.REPO_ID + " != ?)";
+
+        db.execSQL(TempAppProvider.copyData(
+                Schema.ApkAntiFeatureJoinTable.Cols.ALL_COLS,
+                Schema.ApkAntiFeatureJoinTable.NAME,
+                memoryDbName + "." + getApkAntiFeatureJoinTableName(),
+                antiFeaturesWhere), whereArgs);
+
         db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_appId on " + getTableName() + " (" + Cols.APP_ID + ");");
         db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_compatible ON " + getTableName() + " (" + Cols.IS_COMPATIBLE + ");");
     }

app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java

@@ -30,6 +30,7 @@ public class TempAppProvider extends AppProvider {
     private static final String PROVIDER_NAME = "TempAppProvider";
 
     static final String TABLE_TEMP_APP = "temp_" + AppMetadataTable.NAME;
+    static final String TABLE_TEMP_APK_ANTI_FEATURE_JOIN = "temp_" + Schema.ApkAntiFeatureJoinTable.NAME;
     static final String TABLE_TEMP_CAT_JOIN = "temp_" + CatJoinTable.NAME;
 
     private static final String PATH_INIT = "init";
@@ -218,6 +219,7 @@ public class TempAppProvider extends AppProvider {
             final String tempApp = DB + "." + TABLE_TEMP_APP;
             final String tempApk = DB + "." + TempApkProvider.TABLE_TEMP_APK;
             final String tempCatJoin = DB + "." + TABLE_TEMP_CAT_JOIN;
+            final String tempAntiFeatureJoin = DB + "." + TABLE_TEMP_APK_ANTI_FEATURE_JOIN;
 
             final String[] repoArgs = new String[]{Long.toString(repoIdToCommit)};
 
@@ -230,6 +232,16 @@ public class TempAppProvider extends AppProvider {
             db.execSQL("DELETE FROM " + CatJoinTable.NAME + " WHERE " + getCatRepoWhere(CatJoinTable.NAME), repoArgs);
             db.execSQL(copyData(CatJoinTable.Cols.ALL_COLS, tempCatJoin, CatJoinTable.NAME, getCatRepoWhere(tempCatJoin)), repoArgs);
 
+            db.execSQL(
+                    "DELETE FROM " + Schema.ApkAntiFeatureJoinTable.NAME + " " +
+                    "WHERE " + getAntiFeatureRepoWhere(Schema.ApkAntiFeatureJoinTable.NAME), repoArgs);
+
+            db.execSQL(copyData(
+                    Schema.ApkAntiFeatureJoinTable.Cols.ALL_COLS,
+                    tempAntiFeatureJoin,
+                    Schema.ApkAntiFeatureJoinTable.NAME,
+                    getAntiFeatureRepoWhere(tempAntiFeatureJoin)));
+
             db.setTransactionSuccessful();
 
             getContext().getContentResolver().notifyChange(AppProvider.getContentUri(), null);
@@ -250,4 +262,13 @@ public class TempAppProvider extends AppProvider {
 
         return CatJoinTable.Cols.ROW_ID + " IN (" + catRepoSubquery + ")";
     }
+
+    private String getAntiFeatureRepoWhere(String antiFeatureTable) {
+        String subquery =
+                "SELECT innerApk." + ApkTable.Cols.ROW_ID + " " +
+                "FROM " + ApkTable.NAME + " AS innerApk " +
+                "WHERE innerApk." + ApkTable.Cols.REPO_ID + " != ?";
+
+        return antiFeatureTable + "." + Schema.ApkAntiFeatureJoinTable.Cols.APK_ID + " IN (" + subquery + ")";
+    }
 }

app/src/test/java/org/fdroid/fdroid/AntiFeaturesTest.java

@@ -0,0 +1,67 @@
+package org.fdroid.fdroid;
+
+import android.app.Application;
+import android.content.ContentValues;
+
+import org.fdroid.fdroid.data.Apk;
+import org.fdroid.fdroid.data.ApkProvider;
+import org.fdroid.fdroid.data.App;
+import org.fdroid.fdroid.data.FDroidProviderTest;
+import org.fdroid.fdroid.data.Schema;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.annotation.Config;
+
+import java.io.IOException;
+import java.util.List;
+
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+
+@Config(constants = BuildConfig.class, application = Application.class, sdk = 24)
+@RunWith(RobolectricTestRunner.class)
+public class AntiFeaturesTest extends FDroidProviderTest {
+
+    @Test
+    public void testPerApkAntiFeatures() throws IOException, RepoUpdater.UpdateException {
+        ContentValues vulnValues = new ContentValues(1);
+        vulnValues.put(Schema.ApkTable.Cols.AntiFeatures.ANTI_FEATURES, "KnownVuln,ContainsGreenButtons");
+
+        App vulnAtV2 = Assert.insertApp(context, "com.vuln", "Fixed it");
+        Assert.insertApk(context, vulnAtV2, 1);
+        Assert.insertApk(context, vulnAtV2, 2, vulnValues);
+        Assert.insertApk(context, vulnAtV2, 3);
+
+        App notVuln = Assert.insertApp(context, "com.not-vuln", "It's Fine");
+        Assert.insertApk(context, notVuln, 5);
+        Assert.insertApk(context, notVuln, 10);
+        Assert.insertApk(context, notVuln, 15);
+
+        App allVuln = Assert.insertApp(context, "com.all-vuln", "Oops");
+        Assert.insertApk(context, allVuln, 100, vulnValues);
+        Assert.insertApk(context, allVuln, 101, vulnValues);
+        Assert.insertApk(context, allVuln, 105, vulnValues);
+
+        List<Apk> notVulnApks = ApkProvider.Helper.findByPackageName(context, notVuln.packageName);
+        assertEquals(3, notVulnApks.size());
+
+        List<Apk> allVulnApks = ApkProvider.Helper.findByPackageName(context, allVuln.packageName);
+        assertEquals(3, allVulnApks.size());
+        for (Apk apk : allVulnApks) {
+            assertArrayEquals(new String[]{"KnownVuln", "ContainsGreenButtons"}, apk.antiFeatures);
+        }
+
+        List<Apk> vulnAtV2Apks = ApkProvider.Helper.findByPackageName(context, vulnAtV2.packageName);
+        assertEquals(3, vulnAtV2Apks.size());
+        for (Apk apk : vulnAtV2Apks) {
+            if (apk.versionCode == 2) {
+                assertArrayEquals(new String[]{"KnownVuln", "ContainsGreenButtons"}, apk.antiFeatures);
+            } else {
+                assertNull(apk.antiFeatures);
+            }
+        }
+    }
+
+}

app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java

@@ -22,6 +22,7 @@ import java.util.List;
 import static org.fdroid.fdroid.Assert.assertCantDelete;
 import static org.fdroid.fdroid.Assert.assertResultCount;
 import static org.fdroid.fdroid.Assert.insertApp;
+import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
@@ -309,10 +310,12 @@ public class ApkProviderTest extends FDroidProviderTest {
         assertEquals("com.example", apk.packageName);
         assertEquals(10, apk.versionCode);
 
+        assertNull(apk.antiFeatures);
         assertNull(apk.features);
         assertNull(apk.added);
         assertNull(apk.hashType);
 
+        apk.antiFeatures = new String[] {"KnownVuln", "Other anti feature"};
         apk.features = new String[] {"one", "two", "three" };
         long dateTimestamp = System.currentTimeMillis();
         apk.added = new Date(dateTimestamp);
@@ -335,14 +338,8 @@ public class ApkProviderTest extends FDroidProviderTest {
         assertEquals("com.example", updatedApk.packageName);
         assertEquals(10, updatedApk.versionCode);
 
-        assertNotNull(updatedApk.features);
+        assertArrayEquals(new String[]{"KnownVuln", "Other anti feature"}, updatedApk.antiFeatures);
-        assertNotNull(updatedApk.added);
+        assertArrayEquals(new String[]{"one", "two", "three"}, updatedApk.features);
-        assertNotNull(updatedApk.hashType);
-
-        assertEquals(3, updatedApk.features.length);
-        assertEquals("one", updatedApk.features[0]);
-        assertEquals("two", updatedApk.features[1]);
-        assertEquals("three", updatedApk.features[2]);
         assertEquals(new Date(dateTimestamp).getYear(), updatedApk.added.getYear());
         assertEquals(new Date(dateTimestamp).getMonth(), updatedApk.added.getMonth());
         assertEquals(new Date(dateTimestamp).getDay(), updatedApk.added.getDay());

app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java

@@ -91,7 +91,7 @@ public class IndexV1UpdaterTest extends FDroidProviderTest {
         updater.processIndexV1(indexInputStream, indexEntry, "fakeEtag");
         IOUtils.closeQuietly(indexInputStream);
         List<App> apps = AppProvider.Helper.all(context.getContentResolver());
-        assertEquals("53 apps present", 53, apps.size());
+        assertEquals("63 apps present", 63, apps.size());
 
         String[] packages = {
                 "fake.app.one",
@@ -110,7 +110,7 @@ public class IndexV1UpdaterTest extends FDroidProviderTest {
         repos = RepoProvider.Helper.all(context);
         assertEquals("One repo", 1, repos.size());
         Repo repoFromDb = repos.get(0);
-        assertEquals("repo.timestamp should be set", 1481222111, repoFromDb.timestamp);
+        assertEquals("repo.timestamp should be set", 1497639511, repoFromDb.timestamp);
         assertEquals("repo.address should be the same", repo.address, repoFromDb.address);
         assertEquals("repo.name should be set", "non-public test repo", repoFromDb.name);
         assertEquals("repo.maxage should be set", 0, repoFromDb.maxage);