From 3c6a72affdc978786890d771f01a0f8f086f298b Mon Sep 17 00:00:00 2001 From: Stypox Date: Tue, 20 Jul 2021 16:38:35 +0200 Subject: [PATCH 1/3] Remove equals() method from PlayQueueItem --- .../newpipe/player/playqueue/PlayQueueItem.java | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/player/playqueue/PlayQueueItem.java b/app/src/main/java/org/schabi/newpipe/player/playqueue/PlayQueueItem.java index 1c38ebbca..182a91e59 100644 --- a/app/src/main/java/org/schabi/newpipe/player/playqueue/PlayQueueItem.java +++ b/app/src/main/java/org/schabi/newpipe/player/playqueue/PlayQueueItem.java @@ -64,20 +64,6 @@ public class PlayQueueItem implements Serializable { this.recoveryPosition = RECOVERY_UNSET; } - @Override - public boolean equals(final Object o) { - if (o instanceof PlayQueueItem) { - return url.equals(((PlayQueueItem) o).url); - } else { - return false; - } - } - - @Override - public int hashCode() { - return url.hashCode(); - } - @NonNull public String getTitle() { return title; From 2851f80026b796df4853b651a102b08fe5f57f50 Mon Sep 17 00:00:00 2001 From: Stypox Date: Wed, 21 Jul 2021 18:07:26 +0200 Subject: [PATCH 2/3] Use url comparison between queue items when disabling preloading From #4562: Disable player stream preloading only if the current stream is going to be replaced for sure (see this). equals() was implemented for PlayQueueItems, so that (only) the url is compared when checking them. --- .../schabi/newpipe/fragments/detail/VideoDetailFragment.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java index 18e5bb7bf..14d023dc7 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java @@ -813,7 +813,7 @@ public final class VideoDetailFragment @NonNull final String newTitle, @Nullable final PlayQueue newQueue) { if (isPlayerAvailable() && newQueue != null && playQueue != null - && !Objects.equals(newQueue.getItem(), playQueue.getItem())) { + && playQueue.getItem() != null && !playQueue.getItem().getUrl().equals(newUrl)) { // Preloading can be disabled since playback is surely being replaced. player.disablePreloadingOfCurrentTrack(); } From 05e2c4898b5a646f6184a72f166bcdea15890526 Mon Sep 17 00:00:00 2001 From: Stypox Date: Wed, 21 Jul 2021 18:22:17 +0200 Subject: [PATCH 3/3] Add tests for play queue items' equals() --- .../player/playqueue/PlayQueueItemTest.java | 19 +++++++++++++++++++ .../player/playqueue/PlayQueueTest.java | 18 ++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 app/src/test/java/org/schabi/newpipe/player/playqueue/PlayQueueItemTest.java diff --git a/app/src/test/java/org/schabi/newpipe/player/playqueue/PlayQueueItemTest.java b/app/src/test/java/org/schabi/newpipe/player/playqueue/PlayQueueItemTest.java new file mode 100644 index 000000000..d10d33f7e --- /dev/null +++ b/app/src/test/java/org/schabi/newpipe/player/playqueue/PlayQueueItemTest.java @@ -0,0 +1,19 @@ +package org.schabi.newpipe.player.playqueue; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; + +public class PlayQueueItemTest { + + public static final String URL = "MY_URL"; + + @Test + public void equalsMustNotBeOverloaded() { + final PlayQueueItem a = PlayQueueTest.makeItemWithUrl(URL); + final PlayQueueItem b = PlayQueueTest.makeItemWithUrl(URL); + assertEquals(a, a); + assertNotEquals(a, b); // they should compare different even if they have the same data + } +} diff --git a/app/src/test/java/org/schabi/newpipe/player/playqueue/PlayQueueTest.java b/app/src/test/java/org/schabi/newpipe/player/playqueue/PlayQueueTest.java index 43e090900..f92863eab 100644 --- a/app/src/test/java/org/schabi/newpipe/player/playqueue/PlayQueueTest.java +++ b/app/src/test/java/org/schabi/newpipe/player/playqueue/PlayQueueTest.java @@ -15,6 +15,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.spy; @@ -148,6 +149,15 @@ public class PlayQueueTest { assertNull(queue.getItem(-1)); assertNull(queue.getItem(5)); } + + @Test + public void itemsAreNotCloned() { + final PlayQueueItem item = makeItemWithUrl("A url"); + final PlayQueue playQueue = makePlayQueue(0, Collections.singletonList(item)); + + // make sure that items are not cloned when added to the queue + assertSame(playQueue.getItem(), item); + } } public static class EqualsTests { @@ -162,6 +172,14 @@ public class PlayQueueTest { assertEquals(queue1, queue2); } + @Test + public void sameStreamsDifferentIndex() { + final List streams = Collections.nCopies(5, item1); + final PlayQueue queue1 = makePlayQueue(1, streams); + final PlayQueue queue2 = makePlayQueue(4, streams); + assertEquals(queue1, queue2); + } + @Test public void sameSizeDifferentItems() { final List streams1 = Collections.nCopies(5, item1);