From a500660a41463c4bcaed1246def61db10fb8fd0f Mon Sep 17 00:00:00 2001 From: Marcus Hoffmann Date: Fri, 21 Feb 2020 14:45:46 +0100 Subject: [PATCH 1/6] allow displaying localized icons Move the logic of calculating the correct iconUrl from sql to java. Fixes #1460. --- .../fdroid/fdroid/nearby/SwapSuccessView.java | 2 +- .../org/fdroid/fdroid/NotificationHelper.java | 12 ++--- .../main/java/org/fdroid/fdroid/data/App.java | 25 +++++++++- .../org/fdroid/fdroid/data/AppProvider.java | 49 ------------------- .../java/org/fdroid/fdroid/data/DBHelper.java | 11 ++++- .../views/InstallConfirmActivity.java | 2 +- .../fdroid/views/AppDetailsActivity.java | 2 +- .../views/AppDetailsRecyclerViewAdapter.java | 2 +- .../views/apps/AppListItemController.java | 4 +- .../views/categories/AppCardController.java | 2 +- .../views/categories/CategoryController.java | 4 +- .../fdroid/fdroid/updater/AppIconsTest.java | 2 +- 12 files changed, 51 insertions(+), 66 deletions(-) diff --git a/app/src/full/java/org/fdroid/fdroid/nearby/SwapSuccessView.java b/app/src/full/java/org/fdroid/fdroid/nearby/SwapSuccessView.java index c6ef14113..14a343f26 100644 --- a/app/src/full/java/org/fdroid/fdroid/nearby/SwapSuccessView.java +++ b/app/src/full/java/org/fdroid/fdroid/nearby/SwapSuccessView.java @@ -309,7 +309,7 @@ public class SwapSuccessView extends SwapView implements LoaderManager.LoaderCal nameView.setText(app.name); } - ImageLoader.getInstance().displayImage(app.iconUrl, iconView, Utils.getRepoAppDisplayImageOptions()); + ImageLoader.getInstance().displayImage(app.getIconUrl(iconView.getContext()), iconView, Utils.getRepoAppDisplayImageOptions()); if (app.hasUpdates()) { btnInstall.setText(R.string.menu_upgrade); diff --git a/app/src/main/java/org/fdroid/fdroid/NotificationHelper.java b/app/src/main/java/org/fdroid/fdroid/NotificationHelper.java index 27c7525e9..ac5149a69 100644 --- a/app/src/main/java/org/fdroid/fdroid/NotificationHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/NotificationHelper.java @@ -493,7 +493,7 @@ class NotificationHelper { private Bitmap getLargeIconForEntry(AppUpdateStatusManager.AppUpdateStatus entry) { final Point largeIconSize = getLargeIconSize(); Bitmap iconLarge = null; - if (TextUtils.isEmpty(entry.app.iconUrl)) { + if (TextUtils.isEmpty(entry.app.getIconUrl(context))) { return null; } else if (entry.status == AppUpdateStatusManager.Status.Downloading || entry.status == AppUpdateStatusManager.Status.Installing) { @@ -505,12 +505,12 @@ class NotificationHelper { downloadIcon.draw(canvas); } return bitmap; - } else if (DiskCacheUtils.findInCache(entry.app.iconUrl, ImageLoader.getInstance().getDiskCache()) != null) { + } else if (DiskCacheUtils.findInCache(entry.app.getIconUrl(context), ImageLoader.getInstance().getDiskCache()) != null) { iconLarge = ImageLoader.getInstance().loadImageSync( - entry.app.iconUrl, new ImageSize(largeIconSize.x, largeIconSize.y)); + entry.app.getIconUrl(context), new ImageSize(largeIconSize.x, largeIconSize.y)); } else { // Load it for later! - ImageLoader.getInstance().loadImage(entry.app.iconUrl, new ImageSize(largeIconSize.x, largeIconSize.y), new ImageLoadingListener() { + ImageLoader.getInstance().loadImage(entry.app.getIconUrl(context), new ImageSize(largeIconSize.x, largeIconSize.y), new ImageLoadingListener() { AppUpdateStatusManager.AppUpdateStatus entry; @@ -536,8 +536,8 @@ class NotificationHelper { AppUpdateStatusManager.AppUpdateStatus oldEntry = appUpdateStatusManager.get(entry.getCanonicalUrl()); if (oldEntry != null && oldEntry.app != null - && oldEntry.app.iconUrl != null - && DiskCacheUtils.findInCache(oldEntry.app.iconUrl, ImageLoader.getInstance().getDiskCache()) != null) { + && oldEntry.app.getIconUrl(context) != null + && DiskCacheUtils.findInCache(oldEntry.app.getIconUrl(context), ImageLoader.getInstance().getDiskCache()) != null) { createNotification(oldEntry); // Update with new image! } } 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 f86bb71ba..d6f9c76a7 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/App.java +++ b/app/src/main/java/org/fdroid/fdroid/data/App.java @@ -19,9 +19,11 @@ import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.text.TextUtils; import android.util.Log; + import com.fasterxml.jackson.annotation.JacksonInject; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; + import org.apache.commons.io.filefilter.RegexFileFilter; import org.fdroid.fdroid.FDroidApp; import org.fdroid.fdroid.Preferences; @@ -217,7 +219,7 @@ public class App extends ValueObject implements Comparable, Parcelable { /** * URL to download the app's icon. */ - public String iconUrl; + private String iconUrl; public static String getIconName(String packageName, int versionCode) { return packageName + "_" + versionCode + ".png"; @@ -567,6 +569,10 @@ public class App extends ValueObject implements Comparable, Parcelable { if (!TextUtils.isEmpty(value)) { description = formatDescription(value); } + value = getLocalizedGraphicsEntry(localized, localesToUse, "icon"); + if (!TextUtils.isEmpty(value)) { + iconUrl = value; + } featureGraphic = getLocalizedGraphicsEntry(localized, localesToUse, "featureGraphic"); promoGraphic = getLocalizedGraphicsEntry(localized, localesToUse, "promoGraphic"); @@ -656,6 +662,23 @@ public class App extends ValueObject implements Comparable, Parcelable { return description.replace("\n", "
"); } + public String getIconUrl(Context context) { + Repo repo = RepoProvider.Helper.findById(context, repoId); + if (TextUtils.isEmpty(iconUrl)) { + if (TextUtils.isEmpty(icon)){ + return null; + } + String iconsDir; + if (repo.version >= Repo.VERSION_DENSITY_SPECIFIC_ICONS) { + iconsDir = Utils.getIconsDir(context, 1.0); + } else { + iconsDir = Utils.FALLBACK_ICONS_DIR; + } + return repo.address + iconsDir + icon; + } + return repo.address + "/" + packageName + "/" + iconUrl; + } + public String getFeatureGraphicUrl(Context context) { if (TextUtils.isEmpty(featureGraphic)) { return null; 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 7cd30f4f9..1b1939674 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java @@ -1007,8 +1007,6 @@ public class AppProvider extends FDroidProvider { updatePreferredMetadata(); updateCompatibleFlags(); updateSuggestedFromUpstream(null); - updateSuggestedFromLatest(null); - updateIconUrls(); } /** @@ -1182,51 +1180,4 @@ public class AppProvider extends FDroidProvider { LoggingQuery.execSQL(db(), updateSql, args); } - - private void updateIconUrls() { - final String appTable = getTableName(); - final String iconsDir = Utils.getIconsDir(getContext(), 1.0); - String repoVersion = Integer.toString(Repo.VERSION_DENSITY_SPECIFIC_ICONS); - Utils.debugLog(TAG, "Updating icon paths for apps belonging to repos with version >= " + repoVersion); - Utils.debugLog(TAG, "Using icons dir '" + iconsDir + "'"); - String query = getIconUpdateQuery(appTable); - final String[] params = { - repoVersion, iconsDir, Utils.FALLBACK_ICONS_DIR, - }; - db().execSQL(query, params); - } - - /** - * Returns a query which requires two parameters to be bound. These are (in order): - * 1) The repo version that introduced density specific icons - * 2) The dir to density specific icons for the current device. - */ - private static String getIconUpdateQuery(String app) { - - final String repo = RepoTable.NAME; - - final String iconUrlQuery = - "SELECT " + - - // Concatenate (using the "||" operator) the address, the - // icons directory (bound to the ? as the second parameter - // when executing the query) and the icon path. - "( " + - repo + "." + RepoTable.Cols.ADDRESS + - " || " + - - // If the repo has the relevant version, then use a more - // intelligent icons dir, otherwise revert to the default - // one - " CASE WHEN " + repo + "." + RepoTable.Cols.VERSION + " >= ? THEN ? ELSE ? END " + - - " || " + - app + "." + Cols.ICON + - ") " + - " FROM " + - repo + " WHERE " + repo + "." + RepoTable.Cols._ID + " = " + app + "." + Cols.REPO_ID; - return "UPDATE " + app + " SET " - + Cols.ICON_URL + " = ( " + iconUrlQuery + " )"; - } - } 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 11deee710..ec3cf3563 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java @@ -227,7 +227,7 @@ public class DBHelper extends SQLiteOpenHelper { + "primary key(" + ApkAntiFeatureJoinTable.Cols.APK_ID + ", " + ApkAntiFeatureJoinTable.Cols.ANTI_FEATURE_ID + ") " + " );"; - protected static final int DB_VERSION = 83; + protected static final int DB_VERSION = 84; private final Context context; @@ -456,6 +456,15 @@ public class DBHelper extends SQLiteOpenHelper { addIsLocalized(db, oldVersion); addTranslation(db, oldVersion); switchRepoArchivePriorities(db, oldVersion); + deleteOldIconUrls(db, oldVersion); + } + + private void deleteOldIconUrls(SQLiteDatabase db, int oldVersion) { + if (oldVersion >= 84) { + return; + } + Utils.debugLog(TAG, "Clearing iconUrl field to enable localized icons on next update"); + db.execSQL("UPDATE " + AppMetadataTable.NAME + " SET " + AppMetadataTable.Cols.ICON_URL + "= NULL"); } private void switchRepoArchivePriorities(SQLiteDatabase db, int oldVersion) { diff --git a/app/src/main/java/org/fdroid/fdroid/privileged/views/InstallConfirmActivity.java b/app/src/main/java/org/fdroid/fdroid/privileged/views/InstallConfirmActivity.java index fb450594f..051884d78 100644 --- a/app/src/main/java/org/fdroid/fdroid/privileged/views/InstallConfirmActivity.java +++ b/app/src/main/java/org/fdroid/fdroid/privileged/views/InstallConfirmActivity.java @@ -76,7 +76,7 @@ public class InstallConfirmActivity extends FragmentActivity implements OnCancel TabHost tabHost = (TabHost) findViewById(android.R.id.tabhost); appName.setText(app.name); - ImageLoader.getInstance().displayImage(app.iconUrl, appIcon, + ImageLoader.getInstance().displayImage(app.getIconUrl(this), appIcon, Utils.getRepoAppDisplayImageOptions()); tabHost.setup(); diff --git a/app/src/main/java/org/fdroid/fdroid/views/AppDetailsActivity.java b/app/src/main/java/org/fdroid/fdroid/views/AppDetailsActivity.java index 4af3ea8f5..94008a507 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/AppDetailsActivity.java +++ b/app/src/main/java/org/fdroid/fdroid/views/AppDetailsActivity.java @@ -149,7 +149,7 @@ public class AppDetailsActivity extends AppCompatActivity DisplayImageOptions displayImageOptions = Utils.getRepoAppDisplayImageOptions(); String featureGraphicUrl = app.getFeatureGraphicUrl(this); featureImage.loadImageAndDisplay(ImageLoader.getInstance(), displayImageOptions, - featureGraphicUrl, app.iconUrl); + featureGraphicUrl, app.getIconUrl(this)); } private String getPackageNameFromIntent(Intent intent) { diff --git a/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java b/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java index 6c8b24cb8..6304b9574 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java +++ b/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java @@ -480,7 +480,7 @@ public class AppDetailsRecyclerViewAdapter } public void bindModel() { - ImageLoader.getInstance().displayImage(app.iconUrl, iconView, Utils.getRepoAppDisplayImageOptions()); + ImageLoader.getInstance().displayImage(app.getIconUrl(iconView.getContext()), iconView, Utils.getRepoAppDisplayImageOptions()); titleView.setText(app.name); if (!TextUtils.isEmpty(app.authorName)) { authorView.setText(context.getString(R.string.by_author_format, app.authorName)); diff --git a/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java b/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java index ab30d3a6f..93cb0717b 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java +++ b/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java @@ -191,7 +191,7 @@ public abstract class AppListItemController extends RecyclerView.ViewHolder { if (actionButton != null) actionButton.setEnabled(true); - if (app.iconUrl == null) { + if (app.getIconUrl(icon.getContext()) == null) { try { icon.setImageDrawable(activity.getPackageManager().getApplicationIcon(app.packageName)); } catch (PackageManager.NameNotFoundException e) { @@ -201,7 +201,7 @@ public abstract class AppListItemController extends RecyclerView.ViewHolder { : null); } } else { - ImageLoader.getInstance().displayImage(app.iconUrl, icon, Utils.getRepoAppDisplayImageOptions()); + ImageLoader.getInstance().displayImage(app.getIconUrl(icon.getContext()), icon, Utils.getRepoAppDisplayImageOptions()); } // Figures out the current install/update/download/etc status for the app we are viewing. diff --git a/app/src/main/java/org/fdroid/fdroid/views/categories/AppCardController.java b/app/src/main/java/org/fdroid/fdroid/views/categories/AppCardController.java index ee1c87553..bb0c3cc02 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/categories/AppCardController.java +++ b/app/src/main/java/org/fdroid/fdroid/views/categories/AppCardController.java @@ -96,7 +96,7 @@ public class AppCardController extends RecyclerView.ViewHolder } } - ImageLoader.getInstance().displayImage(app.iconUrl, icon, Utils.getRepoAppDisplayImageOptions()); + ImageLoader.getInstance().displayImage(app.getIconUrl(icon.getContext()), icon, Utils.getRepoAppDisplayImageOptions()); } private boolean isConsideredNew(@NonNull App app) { diff --git a/app/src/main/java/org/fdroid/fdroid/views/categories/CategoryController.java b/app/src/main/java/org/fdroid/fdroid/views/categories/CategoryController.java index f06e25669..d2eab3b16 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/categories/CategoryController.java +++ b/app/src/main/java/org/fdroid/fdroid/views/categories/CategoryController.java @@ -154,6 +154,8 @@ public class CategoryController extends RecyclerView.ViewHolder implements Loade Schema.AppMetadataTable.Cols.Package.PACKAGE_NAME, Schema.AppMetadataTable.Cols.SUMMARY, Schema.AppMetadataTable.Cols.ICON_URL, + Schema.AppMetadataTable.Cols.ICON, + Schema.AppMetadataTable.Cols.REPO_ID, }, null, null, @@ -167,7 +169,7 @@ public class CategoryController extends RecyclerView.ViewHolder implements Loade int topAppsId = currentCategory.hashCode(); int countAllAppsId = topAppsId + 1; - // Anything other than these IDs indicates that the loader which just finished finished + // Anything other than these IDs indicates that the loader which just finished // is no longer the one this view holder is interested in, due to the user having // scrolled away already during the asynchronous query being run. if (loader.getId() == topAppsId) { diff --git a/app/src/test/java/org/fdroid/fdroid/updater/AppIconsTest.java b/app/src/test/java/org/fdroid/fdroid/updater/AppIconsTest.java index 195a1c262..b2d625278 100644 --- a/app/src/test/java/org/fdroid/fdroid/updater/AppIconsTest.java +++ b/app/src/test/java/org/fdroid/fdroid/updater/AppIconsTest.java @@ -68,7 +68,7 @@ public class AppIconsTest extends MultiIndexUpdaterTest { App app = AppProvider.Helper.findHighestPriorityMetadata(context.getContentResolver(), "org.adaway", new String[]{Schema.AppMetadataTable.Cols.ICON_URL}); - assertEquals(app.iconUrl, expectedUrl); + assertEquals(app.getIconUrl(context), expectedUrl); } } From 941d8a0b8bbd79895173e10e0bf71ca2e881a8fc Mon Sep 17 00:00:00 2001 From: Marcus Hoffmann Date: Fri, 21 Feb 2020 16:08:08 +0100 Subject: [PATCH 2/6] rename app.icon to app.iconFromApk This makes it clearer what this is actually referring to. --- .../fdroid/nearby/LocalRepoManager.java | 2 +- .../main/java/org/fdroid/fdroid/data/App.java | 19 ++++++++++--------- .../fdroid/fdroid/data/RepoXMLHandler.java | 2 +- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/app/src/full/java/org/fdroid/fdroid/nearby/LocalRepoManager.java b/app/src/full/java/org/fdroid/fdroid/nearby/LocalRepoManager.java index e5f8d7f13..336e2db0b 100644 --- a/app/src/full/java/org/fdroid/fdroid/nearby/LocalRepoManager.java +++ b/app/src/full/java/org/fdroid/fdroid/nearby/LocalRepoManager.java @@ -414,7 +414,7 @@ public final class LocalRepoManager { tag("lastupdated", app.lastUpdated); tag("name", app.name); tag("summary", app.summary); - tag("icon", app.icon); + tag("icon", app.iconFromApk); tag("desc", app.description); tag("license", "Unknown"); tag("categories", "LocalRepo," + Preferences.get().getLocalRepoName()); 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 d6f9c76a7..149552090 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/App.java +++ b/app/src/main/java/org/fdroid/fdroid/data/App.java @@ -118,7 +118,7 @@ public class App extends ValueObject implements Comparable, Parcelable { public String name = "Unknown"; public String summary = "Unknown application"; - public String icon; + public String iconFromApk; public String description; @@ -217,7 +217,8 @@ public class App extends ValueObject implements Comparable, Parcelable { public String[] requirements; /** - * URL to download the app's icon. + * URL to download the app's icon. (Set only from localized block, see also + * {@link #iconFromApk} and {@link #getIconUrl(Context)} */ private String iconUrl; @@ -260,7 +261,7 @@ public class App extends ValueObject implements Comparable, Parcelable { summary = cursor.getString(i); break; case Cols.ICON: - icon = cursor.getString(i); + iconFromApk = cursor.getString(i); break; case Cols.DESCRIPTION: description = cursor.getString(i); @@ -665,7 +666,7 @@ public class App extends ValueObject implements Comparable, Parcelable { public String getIconUrl(Context context) { Repo repo = RepoProvider.Helper.findById(context, repoId); if (TextUtils.isEmpty(iconUrl)) { - if (TextUtils.isEmpty(icon)){ + if (TextUtils.isEmpty(iconFromApk)){ return null; } String iconsDir; @@ -674,7 +675,7 @@ public class App extends ValueObject implements Comparable, Parcelable { } else { iconsDir = Utils.FALLBACK_ICONS_DIR; } - return repo.address + iconsDir + icon; + return repo.address + iconsDir + iconFromApk; } return repo.address + "/" + packageName + "/" + iconUrl; } @@ -779,7 +780,7 @@ public class App extends ValueObject implements Comparable, Parcelable { + ", last updated on " + this.lastUpdated + ")

"; this.name = (String) appInfo.loadLabel(pm); - this.icon = getIconName(packageName, packageInfo.versionCode); + this.iconFromApk = getIconName(packageName, packageInfo.versionCode); this.installedVersionName = packageInfo.versionName; this.installedVersionCode = packageInfo.versionCode; this.compatible = true; @@ -966,7 +967,7 @@ public class App extends ValueObject implements Comparable, Parcelable { values.put(Cols.NAME, name); values.put(Cols.REPO_ID, repoId); values.put(Cols.SUMMARY, summary); - values.put(Cols.ICON, icon); + values.put(Cols.ICON, iconFromApk); values.put(Cols.ICON_URL, iconUrl); values.put(Cols.DESCRIPTION, description); values.put(Cols.WHATSNEW, whatsNew); @@ -1196,7 +1197,7 @@ public class App extends ValueObject implements Comparable, Parcelable { dest.writeString(this.name); dest.writeLong(this.repoId); dest.writeString(this.summary); - dest.writeString(this.icon); + dest.writeString(this.iconFromApk); dest.writeString(this.description); dest.writeString(this.whatsNew); dest.writeString(this.license); @@ -1247,7 +1248,7 @@ public class App extends ValueObject implements Comparable, Parcelable { this.name = in.readString(); this.repoId = in.readLong(); this.summary = in.readString(); - this.icon = in.readString(); + this.iconFromApk = in.readString(); this.description = in.readString(); this.whatsNew = in.readString(); this.license = in.readString(); diff --git a/app/src/main/java/org/fdroid/fdroid/data/RepoXMLHandler.java b/app/src/main/java/org/fdroid/fdroid/data/RepoXMLHandler.java index 6e5e55160..1110a2412 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/RepoXMLHandler.java +++ b/app/src/main/java/org/fdroid/fdroid/data/RepoXMLHandler.java @@ -214,7 +214,7 @@ public class RepoXMLHandler extends DefaultHandler { curapp.name = str; break; case "icon": - curapp.icon = str; + curapp.iconFromApk = str; break; case "description": // This is the old-style description. We'll read it From 7b5d7f8fed323e09e2256022a3d765974a919ea4 Mon Sep 17 00:00:00 2001 From: Marcus Hoffmann Date: Fri, 21 Feb 2020 17:16:07 +0100 Subject: [PATCH 3/6] fix tests after icon changes --- app/build.gradle | 3 ++- .../main/java/org/fdroid/fdroid/data/App.java | 1 + .../org/fdroid/fdroid/updater/AppIconsTest.java | 17 ++++++++++------- .../fdroid/updater/IndexV1UpdaterTest.java | 2 +- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index 9539e8cda..edc039c17 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -169,7 +169,8 @@ dependencies { testImplementation 'org.robolectric:robolectric:3.8' testImplementation "com.android.support.test:monitor:1.0.2" testImplementation 'org.bouncycastle:bcprov-jdk15on:1.60' - testImplementation 'junit:junit:4.12' + testImplementation 'junit:junit:4.13' + testImplementation 'org.hamcrest:hamcrest:2.2' testImplementation 'org.mockito:mockito-core:2.7.22' androidTestImplementation 'com.android.support:support-annotations:27.1.1' 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 149552090..130267336 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/App.java +++ b/app/src/main/java/org/fdroid/fdroid/data/App.java @@ -118,6 +118,7 @@ public class App extends ValueObject implements Comparable, Parcelable { public String name = "Unknown"; public String summary = "Unknown application"; + @JsonProperty("icon") public String iconFromApk; public String description; diff --git a/app/src/test/java/org/fdroid/fdroid/updater/AppIconsTest.java b/app/src/test/java/org/fdroid/fdroid/updater/AppIconsTest.java index b2d625278..50c4a8caf 100644 --- a/app/src/test/java/org/fdroid/fdroid/updater/AppIconsTest.java +++ b/app/src/test/java/org/fdroid/fdroid/updater/AppIconsTest.java @@ -1,6 +1,7 @@ package org.fdroid.fdroid.updater; import android.content.ContentValues; + import org.fdroid.fdroid.BuildConfig; import org.fdroid.fdroid.IndexUpdater; import org.fdroid.fdroid.data.App; @@ -14,7 +15,8 @@ import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; -import static org.junit.Assert.assertEquals; +import static org.hamcrest.MatcherAssert.assertThat; +import org.hamcrest.text.MatchesPattern; /** * Check whether app icons are loaded from the correct repository. The repository with the @@ -42,7 +44,7 @@ public class AppIconsTest extends MultiIndexUpdaterTest { updateMain(); updateArchive(); - assertIconUrl("https://f-droid.org/repo/icons/org.adaway.54.png"); + assertIconUrl("^https://f-droid.org/repo/icons-[0-9]{3}/org.adaway.54.png$"); } @Test @@ -53,7 +55,7 @@ public class AppIconsTest extends MultiIndexUpdaterTest { updateMain(); updateArchive(); - assertIconUrl("https://f-droid.org/archive/icons/org.adaway.54.png"); + assertIconUrl("^https://f-droid.org/archive/icons-[0-9]{3}/org.adaway.54.png$"); } private void setRepoPriority(String repoUri, int priority) { @@ -66,9 +68,10 @@ public class AppIconsTest extends MultiIndexUpdaterTest { private void assertIconUrl(String expectedUrl) { App app = AppProvider.Helper.findHighestPriorityMetadata(context.getContentResolver(), - "org.adaway", new String[]{Schema.AppMetadataTable.Cols.ICON_URL}); - - assertEquals(app.getIconUrl(context), expectedUrl); + "org.adaway", new String[]{ + Schema.AppMetadataTable.Cols.ICON_URL, + Schema.AppMetadataTable.Cols.ICON, + Schema.AppMetadataTable.Cols.REPO_ID}); + assertThat(app.getIconUrl(context), MatchesPattern.matchesPattern(expectedUrl)); } - } diff --git a/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java b/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java index c1205cc1e..3b617ed3d 100644 --- a/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java +++ b/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java @@ -312,7 +312,7 @@ public class IndexV1UpdaterTest extends FDroidProviderTest { "donate", "featureGraphic", "flattrID", - "icon", + "iconFromApk", "iconUrl", "issueTracker", "lastUpdated", From bc6e5e84336bf958fad1147f89ce151f152ab064 Mon Sep 17 00:00:00 2001 From: Marcus Hoffmann Date: Fri, 21 Feb 2020 17:57:46 +0100 Subject: [PATCH 4/6] add test for localized icon --- .../java/org/fdroid/fdroid/updater/AppIconsTest.java | 6 +++--- .../org/fdroid/fdroid/updater/IndexV1UpdaterTest.java | 9 +++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/app/src/test/java/org/fdroid/fdroid/updater/AppIconsTest.java b/app/src/test/java/org/fdroid/fdroid/updater/AppIconsTest.java index 50c4a8caf..ccf850ecf 100644 --- a/app/src/test/java/org/fdroid/fdroid/updater/AppIconsTest.java +++ b/app/src/test/java/org/fdroid/fdroid/updater/AppIconsTest.java @@ -9,6 +9,7 @@ import org.fdroid.fdroid.data.AppProvider; import org.fdroid.fdroid.data.Repo; import org.fdroid.fdroid.data.RepoProvider; import org.fdroid.fdroid.data.Schema; +import org.hamcrest.text.MatchesPattern; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -16,7 +17,6 @@ import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; import static org.hamcrest.MatcherAssert.assertThat; -import org.hamcrest.text.MatchesPattern; /** * Check whether app icons are loaded from the correct repository. The repository with the @@ -44,7 +44,7 @@ public class AppIconsTest extends MultiIndexUpdaterTest { updateMain(); updateArchive(); - assertIconUrl("^https://f-droid.org/repo/icons-[0-9]{3}/org.adaway.54.png$"); + assertIconUrl("^https://f-droid\\.org/repo/icons-[0-9]{3}/org\\.adaway\\.54\\.png$"); } @Test @@ -55,7 +55,7 @@ public class AppIconsTest extends MultiIndexUpdaterTest { updateMain(); updateArchive(); - assertIconUrl("^https://f-droid.org/archive/icons-[0-9]{3}/org.adaway.54.png$"); + assertIconUrl("^https://f-droid\\.org/archive/icons-[0-9]{3}/org\\.adaway\\.54.png$"); } private void setRepoPriority(String repoUri, int priority) { diff --git a/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java b/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java index 3b617ed3d..b8b95133e 100644 --- a/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java +++ b/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java @@ -27,6 +27,7 @@ import org.fdroid.fdroid.data.Repo; import org.fdroid.fdroid.data.RepoProvider; import org.fdroid.fdroid.data.RepoPushRequest; import org.fdroid.fdroid.data.RepoXMLHandlerTest; +import org.fdroid.fdroid.data.Schema; import org.fdroid.fdroid.mock.RepoDetails; import org.junit.Before; import org.junit.Test; @@ -143,6 +144,14 @@ public class IndexV1UpdaterTest extends FDroidProviderTest { assertTrue(requestedPermissions.contains(android.Manifest.permission.READ_EXTERNAL_STORAGE)); assertTrue(requestedPermissions.contains(android.Manifest.permission.WRITE_EXTERNAL_STORAGE)); assertFalse(requestedPermissions.contains(android.Manifest.permission.READ_CALENDAR)); + App app = AppProvider.Helper.findHighestPriorityMetadata(context.getContentResolver(), + "com.autonavi.minimap", new String[]{ + Schema.AppMetadataTable.Cols.ICON_URL, + Schema.AppMetadataTable.Cols.ICON, + Schema.AppMetadataTable.Cols.REPO_ID, + Schema.AppMetadataTable.Cols.Package.PACKAGE_NAME}); + assertEquals("localized icon takes precedence", TESTY_CANONICAL_URL + "/" + + app.packageName + "/en-US/icon.png", app.getIconUrl(context)); } @Test(expected = IndexUpdater.SigningException.class) From 56c05933a2767af4d8f88edc8fadaf4de75d44e7 Mon Sep 17 00:00:00 2001 From: Marcus Hoffmann Date: Mon, 24 Feb 2020 16:44:44 +0100 Subject: [PATCH 5/6] ignore xml icons They will never work, they should not be set by fdroidserver but we can be defensive about not returning them to any views here. --- app/src/main/java/org/fdroid/fdroid/data/App.java | 5 +++++ 1 file changed, 5 insertions(+) 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 130267336..77bf4560b 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/App.java +++ b/app/src/main/java/org/fdroid/fdroid/data/App.java @@ -670,6 +670,11 @@ public class App extends ValueObject implements Comparable, Parcelable { if (TextUtils.isEmpty(iconFromApk)){ return null; } + if (iconFromApk.endsWith(".xml")){ + // We cannot use xml ressources as icons. F-Droid server should not include them + // https://gitlab.com/fdroid/fdroidserver/issues/344 + return null; + } String iconsDir; if (repo.version >= Repo.VERSION_DENSITY_SPECIFIC_ICONS) { iconsDir = Utils.getIconsDir(context, 1.0); From 4a5bee3e84922ebbb70a2748f4a3daaee1a3ae02 Mon Sep 17 00:00:00 2001 From: Marcus Hoffmann Date: Mon, 24 Feb 2020 17:15:03 +0100 Subject: [PATCH 6/6] use icon from pm, when there's none from the metadata This was already done for list views because of the panic uninstall list but we can easily apply the same logic to the tile view and app detail view as well. --- .../main/java/org/fdroid/fdroid/Utils.java | 23 +++++++++++++++++++ .../views/AppDetailsRecyclerViewAdapter.java | 2 +- .../views/apps/AppListItemController.java | 13 +---------- .../views/categories/AppCardController.java | 3 +-- 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/Utils.java b/app/src/main/java/org/fdroid/fdroid/Utils.java index 12653ff42..431ae7a13 100644 --- a/app/src/main/java/org/fdroid/fdroid/Utils.java +++ b/app/src/main/java/org/fdroid/fdroid/Utils.java @@ -49,12 +49,15 @@ import android.util.Log; import android.util.TypedValue; import android.view.View; import android.view.ViewTreeObserver; +import android.widget.ImageView; import android.widget.Toast; import com.nostra13.universalimageloader.core.DisplayImageOptions; +import com.nostra13.universalimageloader.core.ImageLoader; import com.nostra13.universalimageloader.core.assist.ImageScaleType; import com.nostra13.universalimageloader.core.display.FadeInBitmapDisplayer; import com.nostra13.universalimageloader.utils.StorageUtils; import org.fdroid.fdroid.compat.FileCompat; +import org.fdroid.fdroid.data.App; import org.fdroid.fdroid.data.Repo; import org.fdroid.fdroid.data.SanitizedFile; import org.xml.sax.XMLReader; @@ -486,6 +489,26 @@ public final class Utils { return repoAppDisplayImageOptions; } + /** + * If app has an iconUrl we feed that to UIL, otherwise we ask the PackageManager which will + * return the app's icon directly when the app is installed. + * We fall back to the placeholder icon otherwise. + */ + public static void setIconFromRepoOrPM(@NonNull App app, ImageView iv, Context context) { + if (app.getIconUrl(iv.getContext()) == null) { + try { + iv.setImageDrawable(context.getPackageManager().getApplicationIcon(app.packageName)); + } catch (PackageManager.NameNotFoundException e) { + DisplayImageOptions options = Utils.getRepoAppDisplayImageOptions(); + iv.setImageDrawable(options.shouldShowImageForEmptyUri() + ? options.getImageForEmptyUri(FDroidApp.getInstance().getResources()) + : null); + } + } else { + ImageLoader.getInstance().displayImage(app.getIconUrl(iv.getContext()), iv, Utils.getRepoAppDisplayImageOptions()); + } + } + // this is all new stuff being added public static String hashBytes(byte[] input, String algo) { try { diff --git a/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java b/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java index 6304b9574..e6287e50f 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java +++ b/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java @@ -480,7 +480,7 @@ public class AppDetailsRecyclerViewAdapter } public void bindModel() { - ImageLoader.getInstance().displayImage(app.getIconUrl(iconView.getContext()), iconView, Utils.getRepoAppDisplayImageOptions()); + Utils.setIconFromRepoOrPM(app, iconView, iconView.getContext()); titleView.setText(app.name); if (!TextUtils.isEmpty(app.authorName)) { authorView.setText(context.getString(R.string.by_author_format, app.authorName)); diff --git a/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java b/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java index 93cb0717b..ac7b62a25 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java +++ b/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java @@ -191,18 +191,7 @@ public abstract class AppListItemController extends RecyclerView.ViewHolder { if (actionButton != null) actionButton.setEnabled(true); - if (app.getIconUrl(icon.getContext()) == null) { - try { - icon.setImageDrawable(activity.getPackageManager().getApplicationIcon(app.packageName)); - } catch (PackageManager.NameNotFoundException e) { - DisplayImageOptions options = Utils.getRepoAppDisplayImageOptions(); - icon.setImageDrawable(options.shouldShowImageForEmptyUri() - ? options.getImageForEmptyUri(FDroidApp.getInstance().getResources()) - : null); - } - } else { - ImageLoader.getInstance().displayImage(app.getIconUrl(icon.getContext()), icon, Utils.getRepoAppDisplayImageOptions()); - } + Utils.setIconFromRepoOrPM(app, icon, activity); // Figures out the current install/update/download/etc status for the app we are viewing. // Then, asks the view to update itself to reflect this status. diff --git a/app/src/main/java/org/fdroid/fdroid/views/categories/AppCardController.java b/app/src/main/java/org/fdroid/fdroid/views/categories/AppCardController.java index bb0c3cc02..00639e6fd 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/categories/AppCardController.java +++ b/app/src/main/java/org/fdroid/fdroid/views/categories/AppCardController.java @@ -95,8 +95,7 @@ public class AppCardController extends RecyclerView.ViewHolder newTag.setVisibility(View.GONE); } } - - ImageLoader.getInstance().displayImage(app.getIconUrl(icon.getContext()), icon, Utils.getRepoAppDisplayImageOptions()); + Utils.setIconFromRepoOrPM(app, icon, icon.getContext()); } private boolean isConsideredNew(@NonNull App app) {