From 57c63a8e2a0d04c5e73b0d6f9451dc2836b14e1e Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Mon, 20 Jun 2016 22:18:19 +1000 Subject: [PATCH 1/4] Test coverage of anti feature parsing in RepoUpdater. --- .../org/fdroid/fdroid/RepoXMLHandlerTest.java | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/app/src/test/java/org/fdroid/fdroid/RepoXMLHandlerTest.java b/app/src/test/java/org/fdroid/fdroid/RepoXMLHandlerTest.java index e354320e8..1c7fd96fd 100644 --- a/app/src/test/java/org/fdroid/fdroid/RepoXMLHandlerTest.java +++ b/app/src/test/java/org/fdroid/fdroid/RepoXMLHandlerTest.java @@ -21,7 +21,11 @@ import java.io.BufferedInputStream; import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; @@ -30,6 +34,8 @@ import javax.xml.parsers.SAXParserFactory; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @Config(constants = BuildConfig.class) @@ -112,6 +118,65 @@ public class RepoXMLHandlerTest { expectedRepo.timestamp = 1412746769; RepoDetails actualDetails = getFromFile("largeRepo.xml"); handlerTestSuite(expectedRepo, actualDetails, 1211, 2381, 14, 12); + + // Generated using something like the following: + // sed 's,\(.*\).*,\1 \2,p' | sort | uniq + Map> expectedAntiFeatures = new HashMap<>(); + expectedAntiFeatures.put("org.fdroid.fdroid", new ArrayList()); + expectedAntiFeatures.put("org.adblockplus.android", Arrays.asList("Tracking", "Ads")); + expectedAntiFeatures.put("org.microg.nlp.backend.apple", Arrays.asList("Tracking", "NonFreeNet")); + expectedAntiFeatures.put("com.ds.avare", Collections.singletonList("NonFreeDep")); + expectedAntiFeatures.put("com.miracleas.bitcoin_spinner", Collections.singletonList("NonFreeAdd")); + expectedAntiFeatures.put("de.Cherubin7th.blackscreenpresentationremote", Collections.singletonList("Ads")); + expectedAntiFeatures.put("budo.budoist", Collections.singletonList("NonFreeNet")); + expectedAntiFeatures.put("no.rkkc.bysykkel", Collections.singletonList("NonFreeDep")); + expectedAntiFeatures.put("com.jadn.cc", Collections.singletonList("Tracking")); + expectedAntiFeatures.put("org.atai.TessUI", Collections.singletonList("NonFreeNet")); + expectedAntiFeatures.put("org.zephyrsoft.checknetwork", Collections.singletonList("Tracking")); + expectedAntiFeatures.put("de.bashtian.dashclocksunrise", Collections.singletonList("NonFreeDep")); + expectedAntiFeatures.put("org.geometerplus.zlibrary.ui.android", Collections.singletonList("NonFreeAdd")); + expectedAntiFeatures.put("org.mozilla.firefox", Arrays.asList("NonFreeAdd", "Tracking")); + expectedAntiFeatures.put("com.gmail.charleszq", Collections.singletonList("NonFreeDep")); + expectedAntiFeatures.put("it.andreascarpino.forvodroid", Arrays.asList("NonFreeNet", "NonFreeDep")); + expectedAntiFeatures.put("de.b0nk.fp1_epo_autoupdate", Collections.singletonList("NonFreeNet")); + expectedAntiFeatures.put("com.blogspot.tonyatkins.freespeech", Collections.singletonList("Tracking")); + expectedAntiFeatures.put("com.frostwire.android", Collections.singletonList("Tracking")); + expectedAntiFeatures.put("com.namsor.api.samples.gendre", Collections.singletonList("NonFreeNet")); + expectedAntiFeatures.put("com.github.mobile", Collections.singletonList("NonFreeNet")); + expectedAntiFeatures.put("com.cradle.iitc_mobile", Collections.singletonList("NonFreeNet")); + expectedAntiFeatures.put("com.matteopacini.katana", Collections.singletonList("Tracking")); + expectedAntiFeatures.put("de.enaikoon.android.keypadmapper3", Collections.singletonList("Tracking")); + expectedAntiFeatures.put("org.linphone", Collections.singletonList("NonFreeDep")); + expectedAntiFeatures.put("ch.rrelmy.android.locationcachemap", Collections.singletonList("NonFreeDep")); + expectedAntiFeatures.put("com.powerpoint45.lucidbrowser", Arrays.asList("Ads", "NonFreeDep")); + expectedAntiFeatures.put("org.mixare", Collections.singletonList("NonFreeDep")); + expectedAntiFeatures.put("apps.droidnotify", Collections.singletonList("NonFreeAdd")); + expectedAntiFeatures.put("com.numix.calculator", Collections.singletonList("NonFreeAdd")); + expectedAntiFeatures.put("com.numix.icons_circle", Collections.singletonList("NonFreeAdd")); + expectedAntiFeatures.put("com.gh4a", Collections.singletonList("NonFreeNet")); + expectedAntiFeatures.put("at.tomtasche.reader", Collections.singletonList("Tracking")); + expectedAntiFeatures.put("de.uni_potsdam.hpi.openmensa", Collections.singletonList("NonFreeNet")); + expectedAntiFeatures.put("net.osmand.plus", Collections.singletonList("Tracking")); + expectedAntiFeatures.put("byrne.utilities.pasteedroid", Collections.singletonList("NonFreeNet")); + expectedAntiFeatures.put("com.bwx.bequick", Collections.singletonList("NonFreeAdd")); + expectedAntiFeatures.put("be.geecko.QuickLyric", Collections.singletonList("Tracking")); + expectedAntiFeatures.put("com.wanghaus.remembeer", Collections.singletonList("NonFreeNet")); + expectedAntiFeatures.put("cri.sanity", Collections.singletonList("Ads")); + expectedAntiFeatures.put("com.showmehills", Collections.singletonList("Tracking")); + expectedAntiFeatures.put("com.akop.bach", Collections.singletonList("NonFreeNet")); + expectedAntiFeatures.put("org.dmfs.tasks", Collections.singletonList("NonFreeAdd")); + expectedAntiFeatures.put("org.telegram.messenger", Collections.singletonList("NonFreeNet")); + expectedAntiFeatures.put("com.danvelazco.fbwrapper", Collections.singletonList("Tracking")); + expectedAntiFeatures.put("org.zephyrsoft.trackworktime", Collections.singletonList("Tracking")); + expectedAntiFeatures.put("org.transdroid", Collections.singletonList("Tracking")); + expectedAntiFeatures.put("com.lonepulse.travisjr", Collections.singletonList("NonFreeNet")); + expectedAntiFeatures.put("com.twsitedapps.homemanager", Collections.singletonList("NonFreeAdd")); + expectedAntiFeatures.put("org.zeitgeist.movement", Collections.singletonList("NonFreeDep")); + expectedAntiFeatures.put("net.wigle.wigleandroid", Collections.singletonList("NonFreeNet")); + expectedAntiFeatures.put("org.nick.wwwjdic", Collections.singletonList("Tracking")); + + checkAntiFeatures(actualDetails.apps, expectedAntiFeatures); + /* * generated using: sed 's, apps, Map> expectedAntiFeatures) { + for (App app : apps) { + if (expectedAntiFeatures.containsKey(app.packageName)) { + List antiFeatures = expectedAntiFeatures.get(app.packageName); + if (antiFeatures.size() == 0) { + assertNull(app.antiFeatures); + } else { + List actualAntiFeatures = new ArrayList<>(); + for (String antiFeature : app.antiFeatures) { + actualAntiFeatures.add(antiFeature); + } + assertTrue(actualAntiFeatures.containsAll(antiFeatures)); + assertTrue(antiFeatures.containsAll(actualAntiFeatures)); + } + } + } + } + private void checkIncludedApps(List actualApps, String[] expctedAppIds) { assertNotNull(actualApps); assertNotNull(expctedAppIds); From d99b357a1fe8ca3fb2a673ad4177eb70f8b79167 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Mon, 20 Jun 2016 22:24:52 +1000 Subject: [PATCH 2/4] Replace `CommaSeparatedList` with `String[]`. This is a combination of: * `String[].split(",")` and * `TextUtils.join(",", values)` It seems a bit wastefull to have our own implementation of these two things which lightly wrap this code, and produce a datastructure which is non standard and foreign to Java developers. --- .../java/org/fdroid/fdroid/AppDetails.java | 6 +- .../java/org/fdroid/fdroid/AppFilter.java | 11 ++- .../fdroid/fdroid/CompatibilityChecker.java | 14 ++-- .../main/java/org/fdroid/fdroid/Utils.java | 84 ++++--------------- .../main/java/org/fdroid/fdroid/data/Apk.java | 8 +- .../main/java/org/fdroid/fdroid/data/App.java | 6 +- .../org/fdroid/fdroid/data/AppProvider.java | 6 +- .../org/fdroid/fdroid/RepoXMLHandlerTest.java | 4 +- .../fdroid/fdroid/data/ApkProviderTest.java | 8 +- 9 files changed, 51 insertions(+), 96 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails.java b/app/src/main/java/org/fdroid/fdroid/AppDetails.java index 69b03523e..5a0ad42a1 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails.java @@ -266,7 +266,7 @@ public class AppDetails extends AppCompatActivity { } if (Preferences.get().expertMode() && apk.nativecode != null) { - holder.nativecode.setText(apk.nativecode.toString().replaceAll(",", " ")); + holder.nativecode.setText(TextUtils.join(" ", apk.nativecode)); holder.nativecode.setVisibility(View.VISIBLE); } else { holder.nativecode.setVisibility(View.GONE); @@ -276,7 +276,7 @@ public class AppDetails extends AppCompatActivity { holder.incompatibleReasons.setText( getResources().getString( R.string.requires_features, - apk.incompatibleReasons.toPrettyString())); + TextUtils.join(", ", apk.incompatibleReasons))); holder.incompatibleReasons.setVisibility(View.VISIBLE); } else { holder.incompatibleReasons.setVisibility(View.GONE); @@ -1323,7 +1323,7 @@ public class AppDetails extends AppCompatActivity { // Categories TextView final TextView categories = (TextView) view.findViewById(R.id.categories); if (prefs.expertMode() && app.categories != null) { - categories.setText(app.categories.toString().replaceAll(",", ", ")); + categories.setText(TextUtils.join(", ", app.categories)); } else { categories.setVisibility(View.GONE); } diff --git a/app/src/main/java/org/fdroid/fdroid/AppFilter.java b/app/src/main/java/org/fdroid/fdroid/AppFilter.java index ee3397b02..9b1036ac9 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppFilter.java +++ b/app/src/main/java/org/fdroid/fdroid/AppFilter.java @@ -25,12 +25,15 @@ public class AppFilter { // Return true if the given app should be filtered out based on user // preferences, and false otherwise. public boolean filter(App app) { - if (app.requirements == null) { - return false; + if (app.requirements != null && !Preferences.get().filterAppsRequiringRoot()) { + for (String requirement : app.requirements) { + if (requirement.equals("root")) { + return true; + } + } } - return !Preferences.get().filterAppsRequiringRoot() - && app.requirements.contains("root"); + return false; } } diff --git a/app/src/main/java/org/fdroid/fdroid/CompatibilityChecker.java b/app/src/main/java/org/fdroid/fdroid/CompatibilityChecker.java index 306f38ced..532a317d7 100644 --- a/app/src/main/java/org/fdroid/fdroid/CompatibilityChecker.java +++ b/app/src/main/java/org/fdroid/fdroid/CompatibilityChecker.java @@ -6,6 +6,7 @@ import android.content.pm.FeatureInfo; import android.content.pm.PackageManager; import android.os.Build; import android.preference.PreferenceManager; +import android.support.annotation.Nullable; import org.fdroid.fdroid.compat.SupportedArchitectures; import org.fdroid.fdroid.data.Apk; @@ -69,13 +70,16 @@ public class CompatibilityChecker { cpuAbisDesc = builder.toString(); } - private boolean compatibleApi(Utils.CommaSeparatedList nativecode) { + private boolean compatibleApi(@Nullable String[] nativecode) { if (nativecode == null) { return true; } + for (final String cpuAbi : cpuAbis) { - if (nativecode.contains(cpuAbi)) { - return true; + for (String code : nativecode) { + if (code.equals(cpuAbi)) { + return true; + } } } return false; @@ -108,9 +112,7 @@ public class CompatibilityChecker { } } if (!compatibleApi(apk.nativecode)) { - for (final String code : apk.nativecode) { - incompatibleReasons.add(code); - } + Collections.addAll(incompatibleReasons, apk.nativecode); Utils.debugLog(TAG, apk.packageName + " vercode " + apk.versionCode + " only supports " + Utils.CommaSeparatedList.str(apk.nativecode) + " while your architectures are " + cpuAbisDesc); diff --git a/app/src/main/java/org/fdroid/fdroid/Utils.java b/app/src/main/java/org/fdroid/fdroid/Utils.java index 5e6d2f7ea..5cfd3eceb 100644 --- a/app/src/main/java/org/fdroid/fdroid/Utils.java +++ b/app/src/main/java/org/fdroid/fdroid/Utils.java @@ -58,6 +58,7 @@ import java.text.DateFormat; import java.text.ParseException; import java.text.SimpleDateFormat; import java.util.ArrayList; +import java.util.Arrays; import java.util.Date; import java.util.Formatter; import java.util.Iterator; @@ -402,84 +403,33 @@ public final class Utils { return new Locale(languageTag); } - public static final class CommaSeparatedList implements Iterable { - private final String value; - - private CommaSeparatedList(String list) { - value = list; - } - - public static CommaSeparatedList make(List list) { + public static final class CommaSeparatedList { + @Nullable + public static String[] make(List list) { if (list == null || list.isEmpty()) { return null; } - StringBuilder sb = new StringBuilder(); - for (int i = 0; i < list.size(); i++) { - if (i > 0) { - sb.append(','); - } - sb.append(list.get(i)); - } - return new CommaSeparatedList(sb.toString()); - } - - public static CommaSeparatedList make(String[] list) { - if (list == null || list.length == 0) { - return null; - } - StringBuilder sb = new StringBuilder(); - for (int i = 0; i < list.length; i++) { - if (i > 0) { - sb.append(','); - } - sb.append(list[i]); - } - return new CommaSeparatedList(sb.toString()); + return list.toArray(new String[list.size()]); } @Nullable - public static CommaSeparatedList make(@Nullable String list) { + public static String[] make(String[] list) { + if (list == null || list.length == 0) { + return null; + } + return list; + } + + @Nullable + public static String[] make(@Nullable String list) { if (TextUtils.isEmpty(list)) { return null; } - return new CommaSeparatedList(list); + return list.split(","); } - public static String str(CommaSeparatedList instance) { - return instance == null ? null : instance.toString(); - } - - @Override - public String toString() { - return value; - } - - public String toPrettyString() { - return value.replaceAll(",", ", "); - } - - @Override - public Iterator iterator() { - TextUtils.SimpleStringSplitter splitter = new TextUtils.SimpleStringSplitter(','); - splitter.setString(value); - return splitter.iterator(); - } - - public ArrayList toArrayList() { - ArrayList out = new ArrayList<>(); - for (String element : this) { - out.add(element); - } - return out; - } - - public boolean contains(String v) { - for (final String s : this) { - if (s.equals(v)) { - return true; - } - } - return false; + public static String str(@Nullable String[] values) { + return values == null ? null : TextUtils.join(",", values); } } diff --git a/app/src/main/java/org/fdroid/fdroid/data/Apk.java b/app/src/main/java/org/fdroid/fdroid/data/Apk.java index 48a02ea2f..66a280b6e 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/Apk.java +++ b/app/src/main/java/org/fdroid/fdroid/data/Apk.java @@ -29,11 +29,11 @@ public class Apk extends ValueObject implements Comparable { public int targetSdkVersion = SDK_VERSION_MIN_VALUE; // 0 if unknown public int maxSdkVersion = SDK_VERSION_MAX_VALUE; // "infinity" if not set public Date added; - public Utils.CommaSeparatedList permissions; // null if empty or + public String[] permissions; // null if empty or // unknown - public Utils.CommaSeparatedList features; // null if empty or unknown + public String[] features; // null if empty or unknown - public Utils.CommaSeparatedList nativecode; // null if empty or unknown + public String[] nativecode; // null if empty or unknown /** * ID (md5 sum of public key) of signature. Might be null, in the @@ -58,7 +58,7 @@ public class Apk extends ValueObject implements Comparable { public int repoVersion; public String repoAddress; - public Utils.CommaSeparatedList incompatibleReasons; + public String[] incompatibleReasons; public Apk() { } diff --git a/app/src/main/java/org/fdroid/fdroid/data/App.java b/app/src/main/java/org/fdroid/fdroid/data/App.java index 9bc6f32fb..4075fe2c6 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/App.java +++ b/app/src/main/java/org/fdroid/fdroid/data/App.java @@ -88,17 +88,17 @@ public class App extends ValueObject implements Comparable { /** * List of categories (as defined in the metadata documentation) or null if there aren't any. */ - public Utils.CommaSeparatedList categories; + public String[] categories; /** * List of anti-features (as defined in the metadata documentation) or null if there aren't any. */ - public Utils.CommaSeparatedList antiFeatures; + public String[] antiFeatures; /** * List of special requirements (such as root privileges) or null if there aren't any. */ - public Utils.CommaSeparatedList requirements; + public String[] requirements; /** * True if all updates for this app are to be ignored 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 396c6cf63..1e9cc1eeb 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java @@ -98,11 +98,9 @@ public class AppProvider extends FDroidProvider { cursor.moveToFirst(); while (!cursor.isAfterLast()) { final String categoriesString = cursor.getString(0); - Utils.CommaSeparatedList categoriesList = Utils.CommaSeparatedList.make(categoriesString); + String[] categoriesList = Utils.CommaSeparatedList.make(categoriesString); if (categoriesList != null) { - for (final String s : categoriesList) { - categorySet.add(s); - } + Collections.addAll(categorySet, categoriesList); } cursor.moveToNext(); } diff --git a/app/src/test/java/org/fdroid/fdroid/RepoXMLHandlerTest.java b/app/src/test/java/org/fdroid/fdroid/RepoXMLHandlerTest.java index 1c7fd96fd..27cc720a6 100644 --- a/app/src/test/java/org/fdroid/fdroid/RepoXMLHandlerTest.java +++ b/app/src/test/java/org/fdroid/fdroid/RepoXMLHandlerTest.java @@ -672,9 +672,7 @@ public class RepoXMLHandlerTest { assertNull(app.antiFeatures); } else { List actualAntiFeatures = new ArrayList<>(); - for (String antiFeature : app.antiFeatures) { - actualAntiFeatures.add(antiFeature); - } + Collections.addAll(actualAntiFeatures, app.antiFeatures); assertTrue(actualAntiFeatures.containsAll(antiFeatures)); assertTrue(antiFeatures.containsAll(actualAntiFeatures)); } diff --git a/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java b/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java index 03e7ebfa0..b1ba3a95d 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java @@ -289,7 +289,8 @@ public class ApkProviderTest extends FDroidProviderTest { assertEquals(0, apk.repoVersion); // But this should have saved correctly... - assertEquals("Some features", apk.features.toString()); + assertEquals(1, apk.features.length); + assertEquals("Some features", apk.features[0]); assertEquals("com.example.com", apk.packageName); assertEquals(1, apk.versionCode); assertEquals(10, apk.repo); @@ -435,7 +436,10 @@ public class ApkProviderTest extends FDroidProviderTest { assertNotNull(updatedApk.added); assertNotNull(updatedApk.hashType); - assertEquals("one,two,three", updatedApk.features.toString()); + 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()); From a19238931871e143518421ae584a05b3ca0dd5ac Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Tue, 21 Jun 2016 00:17:11 +1000 Subject: [PATCH 3/4] Completely removed `CommaSeparatedList` class, replaced with two helper methods in `Utils`. The two helper methods alleviate the need for copious null checks. They also provide consistent behaviour when there are zero elements (i.e. they return null, rather than an empty string or empty array, as was the case before). --- .../java/org/fdroid/fdroid/AppFilter.java | 2 +- .../fdroid/fdroid/CompatibilityChecker.java | 3 +- .../org/fdroid/fdroid/RepoXMLHandler.java | 12 ++--- .../main/java/org/fdroid/fdroid/Utils.java | 44 +++++-------------- .../main/java/org/fdroid/fdroid/data/Apk.java | 16 +++---- .../main/java/org/fdroid/fdroid/data/App.java | 21 +++++---- .../org/fdroid/fdroid/data/AppProvider.java | 2 +- .../org/fdroid/fdroid/data/RepoPersister.java | 2 +- .../fdroid/localrepo/LocalRepoManager.java | 4 +- .../fdroid/fdroid/data/ApkProviderTest.java | 3 +- 10 files changed, 42 insertions(+), 67 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/AppFilter.java b/app/src/main/java/org/fdroid/fdroid/AppFilter.java index 9b1036ac9..4983d922e 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppFilter.java +++ b/app/src/main/java/org/fdroid/fdroid/AppFilter.java @@ -27,7 +27,7 @@ public class AppFilter { public boolean filter(App app) { if (app.requirements != null && !Preferences.get().filterAppsRequiringRoot()) { for (String requirement : app.requirements) { - if (requirement.equals("root")) { + if ("root".equals(requirement)) { return true; } } diff --git a/app/src/main/java/org/fdroid/fdroid/CompatibilityChecker.java b/app/src/main/java/org/fdroid/fdroid/CompatibilityChecker.java index 532a317d7..74d3b0f2a 100644 --- a/app/src/main/java/org/fdroid/fdroid/CompatibilityChecker.java +++ b/app/src/main/java/org/fdroid/fdroid/CompatibilityChecker.java @@ -7,6 +7,7 @@ import android.content.pm.PackageManager; import android.os.Build; import android.preference.PreferenceManager; import android.support.annotation.Nullable; +import android.text.TextUtils; import org.fdroid.fdroid.compat.SupportedArchitectures; import org.fdroid.fdroid.data.Apk; @@ -114,7 +115,7 @@ public class CompatibilityChecker { if (!compatibleApi(apk.nativecode)) { Collections.addAll(incompatibleReasons, apk.nativecode); Utils.debugLog(TAG, apk.packageName + " vercode " + apk.versionCode - + " only supports " + Utils.CommaSeparatedList.str(apk.nativecode) + + " only supports " + TextUtils.join(", ", apk.nativecode) + " while your architectures are " + cpuAbisDesc); } diff --git a/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java b/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java index 8009fc403..f15b25a16 100644 --- a/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java +++ b/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java @@ -142,13 +142,13 @@ public class RepoXMLHandler extends DefaultHandler { curapk.added = Utils.parseDate(str, null); break; case "permissions": - curapk.permissions = Utils.CommaSeparatedList.make(str); + curapk.permissions = Utils.parseCommaSeparatedString(str); break; case "features": - curapk.features = Utils.CommaSeparatedList.make(str); + curapk.features = Utils.parseCommaSeparatedString(str); break; case "nativecode": - curapk.nativecode = Utils.CommaSeparatedList.make(str); + curapk.nativecode = Utils.parseCommaSeparatedString(str); break; } } else if (curapp != null) { @@ -218,13 +218,13 @@ public class RepoXMLHandler extends DefaultHandler { curapp.upstreamVersionCode = Utils.parseInt(str, -1); break; case "categories": - curapp.categories = Utils.CommaSeparatedList.make(str); + curapp.categories = Utils.parseCommaSeparatedString(str); break; case "antifeatures": - curapp.antiFeatures = Utils.CommaSeparatedList.make(str); + curapp.antiFeatures = Utils.parseCommaSeparatedString(str); break; case "requirements": - curapp.requirements = Utils.CommaSeparatedList.make(str); + curapp.requirements = Utils.parseCommaSeparatedString(str); break; } } else if ("description".equals(localName)) { diff --git a/app/src/main/java/org/fdroid/fdroid/Utils.java b/app/src/main/java/org/fdroid/fdroid/Utils.java index 5cfd3eceb..5a8610dc4 100644 --- a/app/src/main/java/org/fdroid/fdroid/Utils.java +++ b/app/src/main/java/org/fdroid/fdroid/Utils.java @@ -57,12 +57,8 @@ import java.security.cert.CertificateEncodingException; import java.text.DateFormat; import java.text.ParseException; import java.text.SimpleDateFormat; -import java.util.ArrayList; -import java.util.Arrays; import java.util.Date; import java.util.Formatter; -import java.util.Iterator; -import java.util.List; import java.util.Locale; public final class Utils { @@ -403,36 +399,6 @@ public final class Utils { return new Locale(languageTag); } - public static final class CommaSeparatedList { - @Nullable - public static String[] make(List list) { - if (list == null || list.isEmpty()) { - return null; - } - return list.toArray(new String[list.size()]); - } - - @Nullable - public static String[] make(String[] list) { - if (list == null || list.length == 0) { - return null; - } - return list; - } - - @Nullable - public static String[] make(@Nullable String list) { - if (TextUtils.isEmpty(list)) { - return null; - } - return list.split(","); - } - - public static String str(@Nullable String[] values) { - return values == null ? null : TextUtils.join(",", values); - } - } - public static DisplayImageOptions.Builder getImageLoadingOptions() { return new DisplayImageOptions.Builder() .cacheInMemory(true) @@ -510,6 +476,16 @@ public final class Utils { return result; } + @Nullable + public static String[] parseCommaSeparatedString(String values) { + return values == null || values.length() == 0 ? null : values.split(","); + } + + @Nullable + public static String serializeCommaSeparatedString(@Nullable String[] values) { + return values == null || values.length == 0 ? null : TextUtils.join(",", values); + } + private static Date parseDateFormat(DateFormat format, String str, Date fallback) { if (str == null || str.length() == 0) { return fallback; diff --git a/app/src/main/java/org/fdroid/fdroid/data/Apk.java b/app/src/main/java/org/fdroid/fdroid/data/Apk.java index 66a280b6e..36d5db4e8 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/Apk.java +++ b/app/src/main/java/org/fdroid/fdroid/data/Apk.java @@ -83,7 +83,7 @@ public class Apk extends ValueObject implements Comparable { added = Utils.parseDate(cursor.getString(i), null); break; case ApkProvider.DataColumns.FEATURES: - features = Utils.CommaSeparatedList.make(cursor.getString(i)); + features = Utils.parseCommaSeparatedString(cursor.getString(i)); break; case ApkProvider.DataColumns.PACKAGE_NAME: packageName = cursor.getString(i); @@ -104,13 +104,13 @@ public class Apk extends ValueObject implements Comparable { apkName = cursor.getString(i); break; case ApkProvider.DataColumns.PERMISSIONS: - permissions = Utils.CommaSeparatedList.make(cursor.getString(i)); + permissions = Utils.parseCommaSeparatedString(cursor.getString(i)); break; case ApkProvider.DataColumns.NATIVE_CODE: - nativecode = Utils.CommaSeparatedList.make(cursor.getString(i)); + nativecode = Utils.parseCommaSeparatedString(cursor.getString(i)); break; case ApkProvider.DataColumns.INCOMPATIBLE_REASONS: - incompatibleReasons = Utils.CommaSeparatedList.make(cursor.getString(i)); + incompatibleReasons = Utils.parseCommaSeparatedString(cursor.getString(i)); break; case ApkProvider.DataColumns.REPO_ID: repo = cursor.getInt(i); @@ -205,10 +205,10 @@ public class Apk extends ValueObject implements Comparable { values.put(ApkProvider.DataColumns.TARGET_SDK_VERSION, targetSdkVersion); values.put(ApkProvider.DataColumns.MAX_SDK_VERSION, maxSdkVersion); values.put(ApkProvider.DataColumns.ADDED_DATE, Utils.formatDate(added, "")); - values.put(ApkProvider.DataColumns.PERMISSIONS, Utils.CommaSeparatedList.str(permissions)); - values.put(ApkProvider.DataColumns.FEATURES, Utils.CommaSeparatedList.str(features)); - values.put(ApkProvider.DataColumns.NATIVE_CODE, Utils.CommaSeparatedList.str(nativecode)); - values.put(ApkProvider.DataColumns.INCOMPATIBLE_REASONS, Utils.CommaSeparatedList.str(incompatibleReasons)); + values.put(ApkProvider.DataColumns.PERMISSIONS, Utils.serializeCommaSeparatedString(permissions)); + values.put(ApkProvider.DataColumns.FEATURES, Utils.serializeCommaSeparatedString(features)); + values.put(ApkProvider.DataColumns.NATIVE_CODE, Utils.serializeCommaSeparatedString(nativecode)); + values.put(ApkProvider.DataColumns.INCOMPATIBLE_REASONS, Utils.serializeCommaSeparatedString(incompatibleReasons)); values.put(ApkProvider.DataColumns.IS_COMPATIBLE, compatible ? 1 : 0); return values; } diff --git a/app/src/main/java/org/fdroid/fdroid/data/App.java b/app/src/main/java/org/fdroid/fdroid/data/App.java index 4075fe2c6..355d1a8ee 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/App.java +++ b/app/src/main/java/org/fdroid/fdroid/data/App.java @@ -221,13 +221,13 @@ public class App extends ValueObject implements Comparable { lastUpdated = Utils.parseDate(cursor.getString(i), null); break; case AppProvider.DataColumns.CATEGORIES: - categories = Utils.CommaSeparatedList.make(cursor.getString(i)); + categories = Utils.parseCommaSeparatedString(cursor.getString(i)); break; case AppProvider.DataColumns.ANTI_FEATURES: - antiFeatures = Utils.CommaSeparatedList.make(cursor.getString(i)); + antiFeatures = Utils.parseCommaSeparatedString(cursor.getString(i)); break; case AppProvider.DataColumns.REQUIREMENTS: - requirements = Utils.CommaSeparatedList.make(cursor.getString(i)); + requirements = Utils.parseCommaSeparatedString(cursor.getString(i)); break; case AppProvider.DataColumns.IGNORE_ALLUPDATES: ignoreAllUpdates = cursor.getInt(i) == 1; @@ -334,7 +334,7 @@ public class App extends ValueObject implements Comparable { apk.targetSdkVersion = minTargetMax[1]; apk.maxSdkVersion = minTargetMax[2]; apk.packageName = this.packageName; - apk.permissions = Utils.CommaSeparatedList.make(packageInfo.requestedPermissions); + apk.permissions = packageInfo.requestedPermissions; apk.apkName = apk.packageName + "_" + apk.versionCode + ".apk"; apk.installedFile = apkFile; @@ -348,15 +348,14 @@ public class App extends ValueObject implements Comparable { abis.add(matcher.group(1)); } } - apk.nativecode = Utils.CommaSeparatedList.make(abis.toArray(new String[abis.size()])); + apk.nativecode = abis.toArray(new String[abis.size()]); final FeatureInfo[] features = packageInfo.reqFeatures; if (features != null && features.length > 0) { - final String[] featureNames = new String[features.length]; + apk.features = new String[features.length]; for (int i = 0; i < features.length; i++) { - featureNames[i] = features[i].name; + apk.features[i] = features[i].name; } - apk.features = Utils.CommaSeparatedList.make(featureNames); } final JarEntry aSignedEntry = (JarEntry) apkJar.getEntry("AndroidManifest.xml"); @@ -462,9 +461,9 @@ public class App extends ValueObject implements Comparable { values.put(AppProvider.DataColumns.SUGGESTED_VERSION_CODE, suggestedVersionCode); values.put(AppProvider.DataColumns.UPSTREAM_VERSION_NAME, upstreamVersionName); values.put(AppProvider.DataColumns.UPSTREAM_VERSION_CODE, upstreamVersionCode); - values.put(AppProvider.DataColumns.CATEGORIES, Utils.CommaSeparatedList.str(categories)); - values.put(AppProvider.DataColumns.ANTI_FEATURES, Utils.CommaSeparatedList.str(antiFeatures)); - values.put(AppProvider.DataColumns.REQUIREMENTS, Utils.CommaSeparatedList.str(requirements)); + values.put(AppProvider.DataColumns.CATEGORIES, Utils.serializeCommaSeparatedString(categories)); + values.put(AppProvider.DataColumns.ANTI_FEATURES, Utils.serializeCommaSeparatedString(antiFeatures)); + values.put(AppProvider.DataColumns.REQUIREMENTS, Utils.serializeCommaSeparatedString(requirements)); values.put(AppProvider.DataColumns.IS_COMPATIBLE, compatible ? 1 : 0); values.put(AppProvider.DataColumns.IGNORE_ALLUPDATES, ignoreAllUpdates ? 1 : 0); values.put(AppProvider.DataColumns.IGNORE_THISUPDATE, ignoreThisUpdate); 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 1e9cc1eeb..0b2457dad 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java @@ -98,7 +98,7 @@ public class AppProvider extends FDroidProvider { cursor.moveToFirst(); while (!cursor.isAfterLast()) { final String categoriesString = cursor.getString(0); - String[] categoriesList = Utils.CommaSeparatedList.make(categoriesString); + String[] categoriesList = Utils.parseCommaSeparatedString(categoriesString); if (categoriesList != null) { Collections.addAll(categorySet, categoriesList); } 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 8f5d2e037..cb7ce562a 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java +++ b/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java @@ -284,7 +284,7 @@ public class RepoPersister { final List reasons = checker.getIncompatibleReasons(apk); if (reasons.size() > 0) { apk.compatible = false; - apk.incompatibleReasons = Utils.CommaSeparatedList.make(reasons); + apk.incompatibleReasons = reasons.toArray(new String[reasons.size()]); } else { apk.compatible = true; apk.incompatibleReasons = null; diff --git a/app/src/main/java/org/fdroid/fdroid/localrepo/LocalRepoManager.java b/app/src/main/java/org/fdroid/fdroid/localrepo/LocalRepoManager.java index a22146ced..5cf5ca1fe 100644 --- a/app/src/main/java/org/fdroid/fdroid/localrepo/LocalRepoManager.java +++ b/app/src/main/java/org/fdroid/fdroid/localrepo/LocalRepoManager.java @@ -454,7 +454,7 @@ public final class LocalRepoManager { private void tagFeatures(App app) throws IOException { serializer.startTag("", "features"); if (app.installedApk.features != null) { - serializer.text(Utils.CommaSeparatedList.str(app.installedApk.features)); + serializer.text(TextUtils.join(",", app.installedApk.features)); } serializer.endTag("", "features"); } @@ -462,7 +462,7 @@ public final class LocalRepoManager { private void tagNativecode(App app) throws IOException { if (app.installedApk.nativecode != null) { serializer.startTag("", "nativecode"); - serializer.text(Utils.CommaSeparatedList.str(app.installedApk.nativecode)); + serializer.text(TextUtils.join(",", app.installedApk.nativecode)); serializer.endTag("", "nativecode"); } } diff --git a/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java b/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java index b1ba3a95d..d6fcd1b03 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java @@ -7,7 +7,6 @@ import android.net.Uri; import org.fdroid.fdroid.Assert; import org.fdroid.fdroid.BuildConfig; -import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.mock.MockApk; import org.fdroid.fdroid.mock.MockApp; import org.fdroid.fdroid.mock.MockRepo; @@ -410,7 +409,7 @@ public class ApkProviderTest extends FDroidProviderTest { assertNull(apk.added); assertNull(apk.hashType); - apk.features = Utils.CommaSeparatedList.make("one,two,three"); + apk.features = new String[] {"one", "two", "three" }; long dateTimestamp = System.currentTimeMillis(); apk.added = new Date(dateTimestamp); apk.hashType = "i'm a hash type"; From 243bb1948f3268a7c80c94d417c5568c1ef9d52f Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Tue, 21 Jun 2016 00:18:18 +1000 Subject: [PATCH 4/4] Added tests for two helper methods for comma separated string parsing. --- .../java/org/fdroid/fdroid/UtilsTest.java | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/app/src/test/java/org/fdroid/fdroid/UtilsTest.java b/app/src/test/java/org/fdroid/fdroid/UtilsTest.java index 7ec649ca3..4195b04f7 100644 --- a/app/src/test/java/org/fdroid/fdroid/UtilsTest.java +++ b/app/src/test/java/org/fdroid/fdroid/UtilsTest.java @@ -15,6 +15,8 @@ import java.io.IOException; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @Config(constants = BuildConfig.class) @@ -49,6 +51,29 @@ public class UtilsTest { String fingerprintLongByOneFingerprint = "59050C8155DCA377F23D5A15B77D37134000CDBD8B42FBFBE0E3F38096E68CECE"; String fingerprintLongByOnePubkey = "308203c5308202ada00302010202047b7cf549300d06092a864886f70d01010b0500308192310b30090603550406130255533111300f060355040813084e657720596f726b3111300f060355040713084e657720596f726b311d301b060355040a131454686520477561726469616e2050726f6a656374311f301d060355040b1316477561726469616e20462d44726f6964204275696c64311d301b06035504031314677561726469616e70726f6a6563742e696e666f301e170d3132313032393130323530305a170d3430303331363130323530305a308192310b30090603550406130255533111300f060355040813084e657720596f726b3111300f060355040713084e657720596f726b311d301b060355040a131454686520477561726469616e2050726f6a656374311f301d060355040b1316477561726469616e20462d44726f6964204275696c64311d301b06035504031314677561726469616e70726f6a6563742e696e666f30820122300d06092a864886f70d01010105000382010f003082010a0282010100b7f1f635fa3fce1a8042aaa960c2dc557e4ad2c082e5787488cba587fd26207cf59507919fc4dcebda5c8c0959d14146d0445593aa6c29dc639570b71712451fd5c231b0c9f5f0bec380503a1c2a3bc00048bc5db682915afa54d1ecf67b45e1e05c0934b3037a33d3a565899131f27a72c03a5de93df17a2376cc3107f03ee9d124c474dfab30d4053e8f39f292e2dcb6cc131bce12a0c5fc307985195d256bf1d7a2703d67c14bf18ed6b772bb847370b20335810e337c064fef7e2795a524c664a853cd46accb8494f865164dabfb698fa8318236432758bc40d52db00d5ce07fe2210dc06cd95298b4f09e6c9b7b7af61c1d62ea43ea36a2331e7b2d4e250203010001a321301f301d0603551d0e0416041404d763e981cf3a295b94a790d8536a783097232b300d06092a864886f70d01010b05000382010100654e6484ff032c54fed1d96d3c8e731302be9dbd7bb4fe635f2dac05b69f3ecbb5acb7c9fe405e2a066567a8f5c2beb8b199b5a4d5bb1b435cf02df026d4fb4edd9d8849078f085b00950083052d57467d65c6eebd98f037cff9b148d621cf8819c4f7dc1459bf8fc5c7d76f901495a7caf35d1e5c106e1d50610c4920c3c1b50adcfbd4ad83ce7353cdea7d856bba0419c224f89a2f3ebc203d20eb6247711ad2b55fd4737936dc42ced7a047cbbd24012079204a2883b6d55d5d5b66d9fd82fb51fca9a5db5fad9af8564cb380ff30ae8263dbbf01b46e01313f53279673daa3f893380285646b244359203e7eecde94ae141b7dfa8e6499bb8e7e0b25ab85"; + @Test + public void commaSeparatedStrings() { + assertNull(Utils.parseCommaSeparatedString(null)); + assertNull(Utils.parseCommaSeparatedString("")); + + String[] singleValue = Utils.parseCommaSeparatedString("single"); + assertNotNull(singleValue); + assertEquals(1, singleValue.length); + assertEquals("single", singleValue[0]); + + String[] tripleValue = Utils.parseCommaSeparatedString("One,TWO,three"); + assertNotNull(tripleValue); + assertEquals(3, tripleValue.length); + assertEquals("One", tripleValue[0]); + assertEquals("TWO", tripleValue[1]); + assertEquals("three", tripleValue[2]); + + assertNull(Utils.serializeCommaSeparatedString(null)); + assertNull(Utils.serializeCommaSeparatedString(new String[] {})); + assertEquals("Single", Utils.serializeCommaSeparatedString(new String[] {"Single"})); + assertEquals("One,TWO,three", Utils.serializeCommaSeparatedString(new String[] {"One", "TWO", "three"})); + } + @Test public void testFormatFingerprint() { Context context = RuntimeEnvironment.application;