From fd3b07cafbbf09bba8e210d0442924d4634da85c Mon Sep 17 00:00:00 2001 From: Herbert Reiter <46045854+damoasda@users.noreply.github.com> Date: Tue, 5 Jan 2021 16:32:15 +0100 Subject: [PATCH 1/4] Use any image for local folders --- .../core/feed/LocalFeedUpdater.java | 55 ++++++++---- .../core/feed/LocalFeedUpdaterTest.java | 85 +++++++++++++++++++ 2 files changed, 124 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/de/danoeh/antennapod/core/feed/LocalFeedUpdater.java b/core/src/main/java/de/danoeh/antennapod/core/feed/LocalFeedUpdater.java index 4e59fd750..7eb6618b5 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/feed/LocalFeedUpdater.java +++ b/core/src/main/java/de/danoeh/antennapod/core/feed/LocalFeedUpdater.java @@ -6,15 +6,13 @@ import android.media.MediaMetadataRetriever; import android.net.Uri; import android.text.TextUtils; +import androidx.annotation.NonNull; import androidx.documentfile.provider.DocumentFile; -import org.apache.commons.lang3.StringUtils; - import java.io.IOException; import java.text.ParseException; import java.text.SimpleDateFormat; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.Date; import java.util.HashSet; @@ -34,6 +32,8 @@ import de.danoeh.antennapod.core.util.DownloadError; public class LocalFeedUpdater { + static final String[] PREFERRED_FEED_IMAGE_FILENAMES = { "folder.jpg", "Folder.jpg", "folder.png", "Folder.png" }; + public static void updateFeed(Feed feed, Context context) { try { tryUpdateFeed(feed, context); @@ -97,18 +97,7 @@ public class LocalFeedUpdater { } } - List iconLocations = Arrays.asList("folder.jpg", "Folder.jpg", "folder.png", "Folder.png"); - for (String iconLocation : iconLocations) { - DocumentFile image = documentFolder.findFile(iconLocation); - if (image != null) { - feed.setImageUrl(image.getUri().toString()); - break; - } - } - if (StringUtils.isBlank(feed.getImageUrl())) { - // set default feed image - feed.setImageUrl(getDefaultIconUrl(context)); - } + feed.setImageUrl(getImageUrl(context, documentFolder)); feed.getPreferences().setAutoDownload(false); feed.getPreferences().setAutoDeleteAction(FeedPreferences.AutoDeleteAction.NO); @@ -122,6 +111,40 @@ public class LocalFeedUpdater { DBTasks.updateFeed(context, feed, removeUnlistedItems); } + /** + * Returns the image URL for the local feed. + */ + @NonNull + static String getImageUrl(@NonNull Context context, @NonNull DocumentFile documentFolder) { + String imageUrl = null; + + // look for special file names + for (String iconLocation : PREFERRED_FEED_IMAGE_FILENAMES) { + DocumentFile image = documentFolder.findFile(iconLocation); + if (image != null) { + imageUrl = image.getUri().toString(); + break; + } + } + + // use the first image in the folder if existing + if (imageUrl == null) { + for (DocumentFile file : documentFolder.listFiles()) { + String mime = file.getType(); + if (mime != null && (mime.startsWith("image/jpeg") || mime.startsWith("image/png"))) { + imageUrl = file.getUri().toString(); + break; + } + } + } + + // use default icon as fallback + if (imageUrl == null) { + imageUrl = getDefaultIconUrl(context); + } + return imageUrl; + } + /** * Returns the URL of the default icon for a local feed. The URL refers to an app resource file. */ @@ -161,7 +184,7 @@ public class LocalFeedUpdater { return item; } - private static void loadMetadata(FeedItem item, DocumentFile file, Context context) throws Exception { + private static void loadMetadata(FeedItem item, DocumentFile file, Context context) { MediaMetadataRetriever mediaMetadataRetriever = new MediaMetadataRetriever(); mediaMetadataRetriever.setDataSource(context, file.getUri()); diff --git a/core/src/test/java/de/danoeh/antennapod/core/feed/LocalFeedUpdaterTest.java b/core/src/test/java/de/danoeh/antennapod/core/feed/LocalFeedUpdaterTest.java index fbe4c6ace..56d762924 100644 --- a/core/src/test/java/de/danoeh/antennapod/core/feed/LocalFeedUpdaterTest.java +++ b/core/src/test/java/de/danoeh/antennapod/core/feed/LocalFeedUpdaterTest.java @@ -3,6 +3,7 @@ package de.danoeh.antennapod.core.feed; import android.app.Application; import android.content.Context; import android.media.MediaMetadataRetriever; +import android.net.Uri; import android.webkit.MimeTypeMap; import androidx.annotation.NonNull; @@ -33,6 +34,8 @@ import de.danoeh.antennapod.core.storage.DBReader; import de.danoeh.antennapod.core.storage.DBWriter; import de.danoeh.antennapod.core.storage.PodDBAdapter; +import static org.hamcrest.CoreMatchers.endsWith; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.any; @@ -180,6 +183,66 @@ public class LocalFeedUpdaterTest { assertEquals(24, calendar.get(Calendar.SECOND)); } + @Test + public void testGetImageUrl_NoImage() { + String defaultImageName = context.getResources().getResourceEntryName(R.raw.local_feed_default_icon); + { + // empty folder + DocumentFile documentFolder = mockDocumentFolder(); + String imageUrl = LocalFeedUpdater.getImageUrl(context, documentFolder); + assertThat(imageUrl, endsWith(defaultImageName)); + } + { + // no image file, with audio file + DocumentFile documentFolder = mockDocumentFolder(mockDocumentFile("audio.mp3", "audio/mp3")); + String imageUrl = LocalFeedUpdater.getImageUrl(context, documentFolder); + assertThat(imageUrl, endsWith(defaultImageName)); + } + } + + @Test + public void testGetImageUrl_PreferredImagesFilenames() { + for (String filename : LocalFeedUpdater.PREFERRED_FEED_IMAGE_FILENAMES) { + DocumentFile documentFolder = mockDocumentFolder(mockDocumentFile("audio.mp3", "audio/mp3"), + mockDocumentFile(filename, "image/jpeg")); // image MIME type doesn't matter + String imageUrl = LocalFeedUpdater.getImageUrl(context, documentFolder); + assertThat(imageUrl, endsWith(filename)); + } + } + + @Test + public void testGetImageUrl_OtherImageFilename() { + { + // .jpg file + DocumentFile documentFolder = mockDocumentFolder(mockDocumentFile("audio.mp3", "audio/mp3"), + mockDocumentFile("my-image.jpg", "image/jpeg")); + String imageUrl = LocalFeedUpdater.getImageUrl(context, documentFolder); + assertThat(imageUrl, endsWith("my-image.jpg")); + } + { + // .jpeg file + DocumentFile documentFolder = mockDocumentFolder(mockDocumentFile("audio.mp3", "audio/mp3"), + mockDocumentFile("my-image.jpeg", "image/jpeg")); + String imageUrl = LocalFeedUpdater.getImageUrl(context, documentFolder); + assertThat(imageUrl, endsWith("my-image.jpeg")); + } + { + // .png file + DocumentFile documentFolder = mockDocumentFolder(mockDocumentFile("audio.mp3", "audio/mp3"), + mockDocumentFile("my-image.png", "image/png")); + String imageUrl = LocalFeedUpdater.getImageUrl(context, documentFolder); + assertThat(imageUrl, endsWith("my-image.png")); + } + { + // unsupported image type + DocumentFile documentFolder = mockDocumentFolder(mockDocumentFile("audio.mp3", "audio/mp3"), + mockDocumentFile("my-image.svg", "image/svg+xml")); + String imageUrl = LocalFeedUpdater.getImageUrl(context, documentFolder); + String defaultImageName = context.getResources().getResourceEntryName(R.raw.local_feed_default_icon); + assertThat(imageUrl, endsWith(defaultImageName)); + } + } + /** * Fill ShadowMediaMetadataRetriever with dummy duration and title. * @@ -238,4 +301,26 @@ public class LocalFeedUpdaterTest { List feedItems = DBReader.getFeedItemList(feed); assertEquals(expectedItemCount, feedItems.size()); } + + /** + * Create a DocumentFile mock object. + */ + @NonNull + private static DocumentFile mockDocumentFile(@NonNull String fileName, @NonNull String mimeType) { + DocumentFile file = mock(DocumentFile.class); + when(file.getName()).thenReturn(fileName); + when(file.getUri()).thenReturn(Uri.parse("file:///path/" + fileName)); + when(file.getType()).thenReturn(mimeType); + return file; + } + + /** + * Create a DocumentFile folder mock object with a list of files. + */ + @NonNull + private static DocumentFile mockDocumentFolder(DocumentFile... files) { + DocumentFile documentFolder = mock(DocumentFile.class); + when(documentFolder.listFiles()).thenReturn(files); + return documentFolder; + } } From 1f329cdde06000e8cfca295db48bdb447be9fe51 Mon Sep 17 00:00:00 2001 From: Herbert Reiter <46045854+damoasda@users.noreply.github.com> Date: Tue, 5 Jan 2021 16:39:37 +0100 Subject: [PATCH 2/4] Avoid assertTrue() for better error messages --- .../antennapod/core/feed/LocalFeedUpdaterTest.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/core/src/test/java/de/danoeh/antennapod/core/feed/LocalFeedUpdaterTest.java b/core/src/test/java/de/danoeh/antennapod/core/feed/LocalFeedUpdaterTest.java index 56d762924..765b61c5e 100644 --- a/core/src/test/java/de/danoeh/antennapod/core/feed/LocalFeedUpdaterTest.java +++ b/core/src/test/java/de/danoeh/antennapod/core/feed/LocalFeedUpdaterTest.java @@ -35,9 +35,10 @@ import de.danoeh.antennapod.core.storage.DBWriter; import de.danoeh.antennapod.core.storage.PodDBAdapter; import static org.hamcrest.CoreMatchers.endsWith; +import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.empty; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -96,7 +97,7 @@ public class LocalFeedUpdaterTest { public void testUpdateFeed_AddNewFeed() { // check for empty database List feedListBefore = DBReader.getFeedList(); - assertTrue(feedListBefore.isEmpty()); + assertThat(feedListBefore, is(empty())); callUpdateFeed(LOCAL_FEED_DIR2); @@ -142,7 +143,7 @@ public class LocalFeedUpdaterTest { callUpdateFeed(LOCAL_FEED_DIR2); Feed feedAfter = verifySingleFeedInDatabase(); - assertTrue(feedAfter.getImageUrl().contains("local-feed2/folder.png")); + assertThat(feedAfter.getImageUrl(), endsWith("local-feed2/folder.png")); } /** @@ -154,7 +155,7 @@ public class LocalFeedUpdaterTest { Feed feedAfter = verifySingleFeedInDatabase(); String resourceEntryName = context.getResources().getResourceEntryName(R.raw.local_feed_default_icon); - assertTrue(feedAfter.getImageUrl().contains(resourceEntryName)); + assertThat(feedAfter.getImageUrl(), endsWith(resourceEntryName)); } /** From 5d1ef31a4ace38bf33a9c609cd48d755929ba327 Mon Sep 17 00:00:00 2001 From: Herbert Reiter <46045854+damoasda@users.noreply.github.com> Date: Wed, 13 Jan 2021 21:11:17 +0100 Subject: [PATCH 3/4] Refactor method: Use early return instead of late return to make the code more readable --- .../core/feed/LocalFeedUpdater.java | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/de/danoeh/antennapod/core/feed/LocalFeedUpdater.java b/core/src/main/java/de/danoeh/antennapod/core/feed/LocalFeedUpdater.java index 7eb6618b5..d0e15d591 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/feed/LocalFeedUpdater.java +++ b/core/src/main/java/de/danoeh/antennapod/core/feed/LocalFeedUpdater.java @@ -116,33 +116,24 @@ public class LocalFeedUpdater { */ @NonNull static String getImageUrl(@NonNull Context context, @NonNull DocumentFile documentFolder) { - String imageUrl = null; - // look for special file names for (String iconLocation : PREFERRED_FEED_IMAGE_FILENAMES) { DocumentFile image = documentFolder.findFile(iconLocation); if (image != null) { - imageUrl = image.getUri().toString(); - break; + return image.getUri().toString(); } } // use the first image in the folder if existing - if (imageUrl == null) { - for (DocumentFile file : documentFolder.listFiles()) { - String mime = file.getType(); - if (mime != null && (mime.startsWith("image/jpeg") || mime.startsWith("image/png"))) { - imageUrl = file.getUri().toString(); - break; - } + for (DocumentFile file : documentFolder.listFiles()) { + String mime = file.getType(); + if (mime != null && (mime.startsWith("image/jpeg") || mime.startsWith("image/png"))) { + return file.getUri().toString(); } } // use default icon as fallback - if (imageUrl == null) { - imageUrl = getDefaultIconUrl(context); - } - return imageUrl; + return getDefaultIconUrl(context); } /** From e56f203c6d9bb5813df56fb7d72bca7dcb8db13e Mon Sep 17 00:00:00 2001 From: Herbert Reiter <46045854+damoasda@users.noreply.github.com> Date: Wed, 13 Jan 2021 21:20:16 +0100 Subject: [PATCH 4/4] Refactoring: Split test methods to remove anonymous blocks --- .../core/feed/LocalFeedUpdaterTest.java | 85 +++++++++---------- 1 file changed, 42 insertions(+), 43 deletions(-) diff --git a/core/src/test/java/de/danoeh/antennapod/core/feed/LocalFeedUpdaterTest.java b/core/src/test/java/de/danoeh/antennapod/core/feed/LocalFeedUpdaterTest.java index 765b61c5e..b38f8586d 100644 --- a/core/src/test/java/de/danoeh/antennapod/core/feed/LocalFeedUpdaterTest.java +++ b/core/src/test/java/de/danoeh/antennapod/core/feed/LocalFeedUpdaterTest.java @@ -185,20 +185,19 @@ public class LocalFeedUpdaterTest { } @Test - public void testGetImageUrl_NoImage() { + public void testGetImageUrl_EmptyFolder() { + DocumentFile documentFolder = mockDocumentFolder(); + String imageUrl = LocalFeedUpdater.getImageUrl(context, documentFolder); String defaultImageName = context.getResources().getResourceEntryName(R.raw.local_feed_default_icon); - { - // empty folder - DocumentFile documentFolder = mockDocumentFolder(); - String imageUrl = LocalFeedUpdater.getImageUrl(context, documentFolder); - assertThat(imageUrl, endsWith(defaultImageName)); - } - { - // no image file, with audio file - DocumentFile documentFolder = mockDocumentFolder(mockDocumentFile("audio.mp3", "audio/mp3")); - String imageUrl = LocalFeedUpdater.getImageUrl(context, documentFolder); - assertThat(imageUrl, endsWith(defaultImageName)); - } + assertThat(imageUrl, endsWith(defaultImageName)); + } + + @Test + public void testGetImageUrl_NoImageButAudioFiles() { + DocumentFile documentFolder = mockDocumentFolder(mockDocumentFile("audio.mp3", "audio/mp3")); + String imageUrl = LocalFeedUpdater.getImageUrl(context, documentFolder); + String defaultImageName = context.getResources().getResourceEntryName(R.raw.local_feed_default_icon); + assertThat(imageUrl, endsWith(defaultImageName)); } @Test @@ -212,36 +211,36 @@ public class LocalFeedUpdaterTest { } @Test - public void testGetImageUrl_OtherImageFilename() { - { - // .jpg file - DocumentFile documentFolder = mockDocumentFolder(mockDocumentFile("audio.mp3", "audio/mp3"), - mockDocumentFile("my-image.jpg", "image/jpeg")); - String imageUrl = LocalFeedUpdater.getImageUrl(context, documentFolder); - assertThat(imageUrl, endsWith("my-image.jpg")); - } - { - // .jpeg file - DocumentFile documentFolder = mockDocumentFolder(mockDocumentFile("audio.mp3", "audio/mp3"), - mockDocumentFile("my-image.jpeg", "image/jpeg")); - String imageUrl = LocalFeedUpdater.getImageUrl(context, documentFolder); - assertThat(imageUrl, endsWith("my-image.jpeg")); - } - { - // .png file - DocumentFile documentFolder = mockDocumentFolder(mockDocumentFile("audio.mp3", "audio/mp3"), - mockDocumentFile("my-image.png", "image/png")); - String imageUrl = LocalFeedUpdater.getImageUrl(context, documentFolder); - assertThat(imageUrl, endsWith("my-image.png")); - } - { - // unsupported image type - DocumentFile documentFolder = mockDocumentFolder(mockDocumentFile("audio.mp3", "audio/mp3"), - mockDocumentFile("my-image.svg", "image/svg+xml")); - String imageUrl = LocalFeedUpdater.getImageUrl(context, documentFolder); - String defaultImageName = context.getResources().getResourceEntryName(R.raw.local_feed_default_icon); - assertThat(imageUrl, endsWith(defaultImageName)); - } + public void testGetImageUrl_OtherImageFilenameJpg() { + DocumentFile documentFolder = mockDocumentFolder(mockDocumentFile("audio.mp3", "audio/mp3"), + mockDocumentFile("my-image.jpg", "image/jpeg")); + String imageUrl = LocalFeedUpdater.getImageUrl(context, documentFolder); + assertThat(imageUrl, endsWith("my-image.jpg")); + } + + @Test + public void testGetImageUrl_OtherImageFilenameJpeg() { + DocumentFile documentFolder = mockDocumentFolder(mockDocumentFile("audio.mp3", "audio/mp3"), + mockDocumentFile("my-image.jpeg", "image/jpeg")); + String imageUrl = LocalFeedUpdater.getImageUrl(context, documentFolder); + assertThat(imageUrl, endsWith("my-image.jpeg")); + } + + @Test + public void testGetImageUrl_OtherImageFilenamePng() { + DocumentFile documentFolder = mockDocumentFolder(mockDocumentFile("audio.mp3", "audio/mp3"), + mockDocumentFile("my-image.png", "image/png")); + String imageUrl = LocalFeedUpdater.getImageUrl(context, documentFolder); + assertThat(imageUrl, endsWith("my-image.png")); + } + + @Test + public void testGetImageUrl_OtherImageFilenameUnsupportedMimeType() { + DocumentFile documentFolder = mockDocumentFolder(mockDocumentFile("audio.mp3", "audio/mp3"), + mockDocumentFile("my-image.svg", "image/svg+xml")); + String imageUrl = LocalFeedUpdater.getImageUrl(context, documentFolder); + String defaultImageName = context.getResources().getResourceEntryName(R.raw.local_feed_default_icon); + assertThat(imageUrl, endsWith(defaultImageName)); } /**