From f02fe69b987ad0325b5dcaa11cfa7dd4be3216bd Mon Sep 17 00:00:00 2001 From: Ivan Kupalov Date: Mon, 17 Feb 2020 16:23:34 +0100 Subject: [PATCH] Fix emoji download (#1691) * Update OkHttp to 4.3.1 * Fix downloading emoji fonts OkHttp strips away content length info when compression is used. Even though this behavior is old, we didn't observe it until OkHttp was updated in d05bd4b751a71af5469135188a4e3e673f6a7bd5. We get it from the original network response header. It should be compressed length. * Reformat EmojiCompatFont and EmojiPreference --- app/build.gradle | 2 +- .../keylesspalace/tusky/EmojiPreference.java | 106 +++++++++--------- .../tusky/util/EmojiCompatFont.java | 92 ++++++++++----- 3 files changed, 115 insertions(+), 85 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index 8666369c..b3a850a8 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -98,7 +98,7 @@ project.tasks.withType(org.jetbrains.kotlin.gradle.tasks.KotlinCompile).all { ext.lifecycleVersion = "2.1.0" ext.roomVersion = '2.2.3' ext.retrofitVersion = '2.6.0' -ext.okhttpVersion = '4.2.2' +ext.okhttpVersion = '4.3.1' ext.glideVersion = '4.10.0' ext.daggerVersion = '2.25.3' diff --git a/app/src/main/java/com/keylesspalace/tusky/EmojiPreference.java b/app/src/main/java/com/keylesspalace/tusky/EmojiPreference.java index a884736c..cdf09002 100644 --- a/app/src/main/java/com/keylesspalace/tusky/EmojiPreference.java +++ b/app/src/main/java/com/keylesspalace/tusky/EmojiPreference.java @@ -1,14 +1,10 @@ package com.keylesspalace.tusky; import android.app.AlarmManager; -import androidx.appcompat.app.AlertDialog; import android.app.PendingIntent; import android.content.Context; import android.content.Intent; import android.os.Build; - -import androidx.preference.Preference; -import androidx.preference.PreferenceManager; import android.util.AttributeSet; import android.util.Log; import android.view.LayoutInflater; @@ -20,6 +16,10 @@ import android.widget.RadioButton; import android.widget.TextView; import android.widget.Toast; +import androidx.appcompat.app.AlertDialog; +import androidx.preference.Preference; +import androidx.preference.PreferenceManager; + import com.keylesspalace.tusky.util.EmojiCompatFont; import java.util.ArrayList; @@ -44,7 +44,6 @@ public class EmojiPreference extends Preference { private boolean updated, currentNeedsUpdate; - public EmojiPreference(Context context, AttributeSet attrs) { super(context, attrs); @@ -63,7 +62,7 @@ public class EmojiPreference extends Preference { View view = LayoutInflater.from(getContext()).inflate(R.layout.dialog_emojicompat, null); - for(int i = 0; i < viewIds.length; i++) { + for (int i = 0; i < viewIds.length; i++) { setupItem(view.findViewById(viewIds[i]), FONTS[i]); } @@ -96,13 +95,13 @@ public class EmojiPreference extends Preference { // Set actions download.setOnClickListener((downloadButton) -> - startDownload(font, container)); + startDownload(font, container)); cancel.setOnClickListener((cancelButton) -> - cancelDownload(font, container)); + cancelDownload(font, container)); radio.setOnClickListener((radioButton) -> - select(font, (RadioButton) radioButton)); + select(font, (RadioButton) radioButton)); container.setOnClickListener((containterView) -> select(font, @@ -111,11 +110,11 @@ public class EmojiPreference extends Preference { } private void startDownload(EmojiCompatFont font, View container) { - ImageButton download = container.findViewById(R.id.emojicompat_download); - TextView caption = container.findViewById(R.id.emojicompat_caption); + ImageButton download = container.findViewById(R.id.emojicompat_download); + TextView caption = container.findViewById(R.id.emojicompat_caption); ProgressBar progressBar = container.findViewById(R.id.emojicompat_progress); - ImageButton cancel = container.findViewById(R.id.emojicompat_download_cancel); + ImageButton cancel = container.findViewById(R.id.emojicompat_download_cancel); // Switch to downloading style download.setVisibility(View.GONE); @@ -136,8 +135,7 @@ public class EmojiPreference extends Preference { progress *= progressBar.getMax(); if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { progressBar.setProgress((int) progress, true); - } - else { + } else { progressBar.setProgress((int) progress); } } @@ -167,14 +165,15 @@ public class EmojiPreference extends Preference { /** * Select a font both visually and logically - * @param font The font to be selected + * + * @param font The font to be selected * @param radio The radio button associated with it's visual item */ private void select(EmojiCompatFont font, RadioButton radio) { selected = font; // Uncheck all the other buttons - for(RadioButton other : radioButtons) { - if(other != radio) { + for (RadioButton other : radioButtons) { + if (other != radio) { other.setChecked(false); } } @@ -183,31 +182,31 @@ public class EmojiPreference extends Preference { /** * Called when a "consistent" state is reached, i.e. it's not downloading the font - * @param font The font to be displayed + * + * @param font The font to be displayed * @param container The ConstraintLayout containing the item */ private void updateItem(EmojiCompatFont font, View container) { // Assignments - ImageButton download = container.findViewById(R.id.emojicompat_download); - TextView caption = container.findViewById(R.id.emojicompat_caption); + ImageButton download = container.findViewById(R.id.emojicompat_download); + TextView caption = container.findViewById(R.id.emojicompat_caption); ProgressBar progress = container.findViewById(R.id.emojicompat_progress); - ImageButton cancel = container.findViewById(R.id.emojicompat_download_cancel); + ImageButton cancel = container.findViewById(R.id.emojicompat_download_cancel); - RadioButton radio = container.findViewById(R.id.emojicompat_radio); + RadioButton radio = container.findViewById(R.id.emojicompat_radio); // There's no download going on progress.setVisibility(View.GONE); cancel.setVisibility(View.GONE); caption.setVisibility(View.VISIBLE); - if(font.isDownloaded(getContext())) { + if (font.isDownloaded(getContext())) { // Make it selectable download.setVisibility(View.GONE); radio.setVisibility(View.VISIBLE); container.setClickable(true); - } - else { + } else { // Make it downloadable download.setVisibility(View.VISIBLE); radio.setVisibility(View.GONE); @@ -215,14 +214,13 @@ public class EmojiPreference extends Preference { } // Select it if necessary - if(font == selected) { + if (font == selected) { radio.setChecked(true); // Update available if (!font.isDownloaded(getContext())) { currentNeedsUpdate = true; } - } - else { + } else { radio.setChecked(false); } } @@ -248,33 +246,33 @@ public class EmojiPreference extends Preference { * That means, the selected font can be saved (if the user hit OK) */ private void onDialogOk() { - saveSelectedFont(); + saveSelectedFont(); if (selected != original || updated) { - new AlertDialog.Builder(getContext()) - .setTitle(R.string.restart_required) - .setMessage(R.string.restart_emoji) - .setNegativeButton(R.string.later, null) - .setPositiveButton(R.string.restart, ((dialog, which) -> { - // Restart the app - // From https://stackoverflow.com/a/17166729/5070653 - Intent launchIntent = new Intent(getContext(), SplashActivity.class); - PendingIntent mPendingIntent = PendingIntent.getActivity( - getContext(), - // This is the codepoint of the party face emoji :D - 0x1f973, - launchIntent, - PendingIntent.FLAG_CANCEL_CURRENT); - AlarmManager mgr = - (AlarmManager) getContext().getSystemService(Context.ALARM_SERVICE); - if (mgr != null) { - mgr.set( - AlarmManager.RTC, - System.currentTimeMillis() + 100, - mPendingIntent); - } - System.exit(0); - })).show(); - } + new AlertDialog.Builder(getContext()) + .setTitle(R.string.restart_required) + .setMessage(R.string.restart_emoji) + .setNegativeButton(R.string.later, null) + .setPositiveButton(R.string.restart, ((dialog, which) -> { + // Restart the app + // From https://stackoverflow.com/a/17166729/5070653 + Intent launchIntent = new Intent(getContext(), SplashActivity.class); + PendingIntent mPendingIntent = PendingIntent.getActivity( + getContext(), + // This is the codepoint of the party face emoji :D + 0x1f973, + launchIntent, + PendingIntent.FLAG_CANCEL_CURRENT); + AlarmManager mgr = + (AlarmManager) getContext().getSystemService(Context.ALARM_SERVICE); + if (mgr != null) { + mgr.set( + AlarmManager.RTC, + System.currentTimeMillis() + 100, + mPendingIntent); + } + System.exit(0); + })).show(); + } } diff --git a/app/src/main/java/com/keylesspalace/tusky/util/EmojiCompatFont.java b/app/src/main/java/com/keylesspalace/tusky/util/EmojiCompatFont.java index 627893f2..ea047982 100644 --- a/app/src/main/java/com/keylesspalace/tusky/util/EmojiCompatFont.java +++ b/app/src/main/java/com/keylesspalace/tusky/util/EmojiCompatFont.java @@ -27,6 +27,7 @@ import de.c1710.filemojicompat.FileEmojiCompatConfig; import okhttp3.OkHttpClient; import okhttp3.Request; import okhttp3.Response; +import okhttp3.ResponseBody; import okio.BufferedSink; import okio.Okio; import okio.Source; @@ -81,7 +82,7 @@ public class EmojiCompatFont { R.drawable.ic_notoemoji, "https://tusky.app/hosted/emoji/NotoEmojiCompat.ttf", "11.0.0" - ); + ); /** * This array stores all available EmojiCompat fonts. @@ -109,14 +110,14 @@ public class EmojiCompatFont { /** * Returns the Emoji font associated with this ID + * * @param id the ID of this font * @return the corresponding font. Will default to SYSTEM_DEFAULT if not in range. */ public static EmojiCompatFont byId(int id) { - if(id >= 0 && id < FONTS.length) { + if (id >= 0 && id < FONTS.length) { return FONTS[id]; - } - else { + } else { return SYSTEM_DEFAULT; } } @@ -157,15 +158,15 @@ public class EmojiCompatFont { /** * This method will return the actual font file (regardless of its existence) for * the current version (not necessarily the latest!). + * * @return The font (TTF) file or null if called on SYSTEM_FONT */ @Nullable private File getFont(Context context) { - if(this != SYSTEM_DEFAULT) { + if (this != SYSTEM_DEFAULT) { File directory = new File(context.getExternalFilesDir(null), DIRECTORY); return new File(directory, this.getName() + this.getVersion() + ".ttf"); - } - else { + } else { return null; } } @@ -185,6 +186,7 @@ public class EmojiCompatFont { /** * Checks whether there is already a font version that satisfies the current version, i.e. it * has a higher or equal version code. + * * @param context The Context * @return Whether there is a font file with a higher or equal version code to the current */ @@ -199,10 +201,11 @@ public class EmojiCompatFont { /** * Downloads the TTF file for this font + * * @param listeners The listeners which will be notified when the download has been finished */ public void downloadFont(Context context, Downloader.EmojiDownloadListener... listeners) { - if(this != SYSTEM_DEFAULT) { + if (this != SYSTEM_DEFAULT) { // Additionally run a cleanup process after the download has been successful. Downloader.EmojiDownloadListener cleanup = font -> deleteOldVersions(context); @@ -216,9 +219,8 @@ public class EmojiCompatFont { this, allListeners.toArray(allListenersA)) .execute(getFont(context)); - } - else { - for(Downloader.EmojiDownloadListener listener: listeners) { + } else { + for (Downloader.EmojiDownloadListener listener : listeners) { // The system emoji font is always downloaded... listener.onDownloaded(this); } @@ -227,6 +229,7 @@ public class EmojiCompatFont { /** * Deletes any older version of a font + * * @param context The current Context */ private void deleteOldVersions(Context context) { @@ -236,11 +239,11 @@ public class EmojiCompatFont { Log.d(TAG, String.format("deleteOldVersions: Found %d other font files", existingFontFiles.size())); for (Pair fileExists : existingFontFiles) { if (compareVersions(fileExists.second, getVersionCode()) < 0) { - File file = fileExists.first; - // Uses side effects! - Log.d(TAG, String.format("Deleted %s successfully: %s", file.getAbsolutePath(), - file.delete())); - } + File file = fileExists.first; + // Uses side effects! + Log.d(TAG, String.format("Deleted %s successfully: %s", file.getAbsolutePath(), + file.delete())); + } } } @@ -250,6 +253,7 @@ public class EmojiCompatFont { /** * Loads all font files that are inside the files directory into an ArrayList with the information * on whether they are older than the currently available version or not. + * * @param context The Context */ private void loadExistingFontFiles(Context context) { @@ -274,7 +278,7 @@ public class EmojiCompatFont { this.existingFontFiles = new ArrayList<>(existingFontFiles.length); - for(File file : existingFontFiles) { + for (File file : existingFontFiles) { Matcher matcher = fontRegex.matcher(file.getName()); if (matcher.matches()) { String version = matcher.group(1); @@ -294,6 +298,7 @@ public class EmojiCompatFont { /** * Returns the current or latest version of this font file (if there is any) + * * @param context The Context * @return The file for this font with the current or (if not existent) highest version code or null if there is no file for this font. */ @@ -380,7 +385,7 @@ public class EmojiCompatFont { * Stops downloading the font. If no one started a font download, nothing happens. */ public void cancelDownload() { - if(fontDownloader != null) { + if (fontDownloader != null) { fontDownloader.cancel(false); fontDownloader = null; } @@ -407,7 +412,7 @@ public class EmojiCompatFont { } @Override - protected File doInBackground(File... files){ + protected File doInBackground(File... files) { // Only download to one file... File downloadFile = files[0]; try { @@ -428,7 +433,7 @@ public class EmojiCompatFont { // Download! if (response.body() != null && response.isSuccessful() - && (size = response.body().contentLength()) > 0) { + && (size = networkResponseLength(response)) > 0) { float progress = 0; source = response.body().source(); try { @@ -448,14 +453,13 @@ public class EmojiCompatFont { Log.e(TAG, "Status code: " + response.code()); failed = true; } - } - finally { - if(source != null) { + } finally { + if (source != null) { source.close(); } sink.close(); // This 'if' uses side effects to delete the File. - if(isCancelled() && !downloadFile.delete()) { + if (isCancelled() && !downloadFile.delete()) { Log.e(TAG, "Could not delete file " + downloadFile); } } @@ -468,28 +472,27 @@ public class EmojiCompatFont { @Override public void onProgressUpdate(Float... progress) { - for(EmojiDownloadListener listener: listeners) { + for (EmojiDownloadListener listener : listeners) { listener.onProgress(progress[0]); } } @Override public void onPostExecute(File downloadedFile) { - if(!failed && downloadedFile.exists()) { + if (!failed && downloadedFile.exists()) { for (EmojiDownloadListener listener : listeners) { listener.onDownloaded(font); } - } - else { + } else { fail(downloadedFile); } } private void fail(File failedFile) { - if(failedFile.exists() && !failedFile.delete()) { + if (failedFile.exists() && !failedFile.delete()) { Log.e(TAG, "Could not delete file " + failedFile); } - for(EmojiDownloadListener listener : listeners) { + for (EmojiDownloadListener listener : listeners) { listener.onFailed(); } } @@ -500,11 +503,13 @@ public class EmojiCompatFont { public interface EmojiDownloadListener { /** * Called after successfully finishing a download. + * * @param font The font related to this download. This will help identifying the download */ void onDownloaded(EmojiCompatFont font); // TODO: Add functionality + /** * Called when something went wrong with the download. * This one won't be called when the download has been cancelled though. @@ -515,12 +520,39 @@ public class EmojiCompatFont { /** * Called whenever the progress changed + * * @param Progress A value between 0 and 1 representing the current progress */ default void onProgress(float Progress) { // ARE WE THERE YET? } } + + + /** + * This method is needed because when transparent compression is used OkHttp reports + * {@link ResponseBody#contentLength()} as -1. We try to get the header which server sent + * us manually here. + * + * @see OkHttp issue 259 + */ + private long networkResponseLength(Response response) { + Response networkResponse = response.networkResponse(); + if (networkResponse == null) { + // In case it's a fully cached response + ResponseBody body = response.body(); + return body == null ? -1 : body.contentLength(); + } + String header = networkResponse.header("Content-Length"); + if (header == null) { + return -1; + } + try { + return Integer.parseInt(header); + } catch (NumberFormatException e) { + return -1; + } + } } @Override