diff --git a/app/src/main/java/org/schabi/newpipe/player/playqueue/PlayQueue.java b/app/src/main/java/org/schabi/newpipe/player/playqueue/PlayQueue.java index 6131d8565..014c13339 100644 --- a/app/src/main/java/org/schabi/newpipe/player/playqueue/PlayQueue.java +++ b/app/src/main/java/org/schabi/newpipe/player/playqueue/PlayQueue.java @@ -40,29 +40,25 @@ import io.reactivex.rxjava3.subjects.BehaviorSubject; */ public abstract class PlayQueue implements Serializable { public static final boolean DEBUG = MainActivity.DEBUG; - - private ArrayList backup; - private ArrayList streams; - @NonNull private final AtomicInteger queueIndex; - private final ArrayList history; + private final List history = new ArrayList<>(); + + private List backup; + private List streams; private transient BehaviorSubject eventBroadcast; private transient Flowable broadcastReceiver; - - private transient boolean disposed; + private transient boolean disposed = false; PlayQueue(final int index, final List startWith) { - streams = new ArrayList<>(); - streams.addAll(startWith); - history = new ArrayList<>(); + streams = new ArrayList<>(startWith); + if (streams.size() > index) { history.add(streams.get(index)); } queueIndex = new AtomicInteger(index); - disposed = false; } /*////////////////////////////////////////////////////////////////////////// @@ -137,18 +133,36 @@ public abstract class PlayQueue implements Serializable { public synchronized void setIndex(final int index) { final int oldIndex = getIndex(); - int newIndex = index; + final int newIndex; + if (index < 0) { newIndex = 0; + } else if (index < streams.size()) { + // Regular assignment for index in bounds + newIndex = index; + } else if (streams.isEmpty()) { + // Out of bounds from here on + // Need to check if stream is empty to prevent arithmetic error and negative index + newIndex = 0; + } else if (isComplete()) { + // Circular indexing + newIndex = index % streams.size(); + } else { + // Index of last element + newIndex = streams.size() - 1; } - if (index >= streams.size()) { - newIndex = isComplete() ? index % streams.size() : streams.size() - 1; - } + + queueIndex.set(newIndex); + if (oldIndex != newIndex) { history.add(streams.get(newIndex)); } - queueIndex.set(newIndex); + /* + TODO: Documentation states that a SelectEvent will only be emitted if the new index is... + different from the old one but this is emitted regardless? Not sure what this what it does + exactly so I won't touch it + */ broadcast(new SelectEvent(oldIndex, newIndex)); } @@ -180,8 +194,6 @@ public abstract class PlayQueue implements Serializable { * @return the index of the given item */ public int indexOf(@NonNull final PlayQueueItem item) { - // referential equality, can't think of a better way to do this - // todo: better than this return streams.indexOf(item); } @@ -410,34 +422,42 @@ public abstract class PlayQueue implements Serializable { } /** - * Shuffles the current play queue. + * Shuffles the current play queue *

- * This method first backs up the existing play queue and item being played. - * Then a newly shuffled play queue will be generated along with currently - * playing item placed at the beginning of the queue. + * This method first backs up the existing play queue and item being played. Then a newly + * shuffled play queue will be generated along with currently playing item placed at the + * beginning of the queue. This item will also be added to the history. *

*

- * Will emit a {@link ReorderEvent} in any context. + * Will emit a {@link ReorderEvent} if shuffled. *

+ * + * @implNote Does nothing if the queue has a size <= 2 (the currently playing video must stay on + * top, so shuffling a size-2 list does nothing) */ public synchronized void shuffle() { + // Can't shuffle an list that's empty or only has one element + if (size() <= 2) { + return; + } + // Create a backup if it doesn't already exist if (backup == null) { backup = new ArrayList<>(streams); } - final int originIndex = getIndex(); - final PlayQueueItem current = getItem(); + + final int originalIndex = getIndex(); + final PlayQueueItem currentItem = getItem(); + Collections.shuffle(streams); - final int newIndex = streams.indexOf(current); - if (newIndex != -1) { - streams.add(0, streams.remove(newIndex)); - } + // Move currentItem to the head of the queue + streams.remove(currentItem); + streams.add(0, currentItem); queueIndex.set(0); - if (streams.size() > 0) { - history.add(streams.get(0)); - } - broadcast(new ReorderEvent(originIndex, queueIndex.get())); + history.add(currentItem); + + broadcast(new ReorderEvent(originalIndex, 0)); } /** @@ -457,7 +477,6 @@ public abstract class PlayQueue implements Serializable { final int originIndex = getIndex(); final PlayQueueItem current = getItem(); - streams.clear(); streams = backup; backup = null; @@ -500,22 +519,19 @@ public abstract class PlayQueue implements Serializable { * we don't have to do anything with new queue. * This method also gives a chance to track history of items in a queue in * VideoDetailFragment without duplicating items from two identical queues - * */ + */ @Override public boolean equals(@Nullable final Object obj) { - if (!(obj instanceof PlayQueue) - || getStreams().size() != ((PlayQueue) obj).getStreams().size()) { + if (!(obj instanceof PlayQueue)) { return false; } - final PlayQueue other = (PlayQueue) obj; - for (int i = 0; i < getStreams().size(); i++) { - if (!getItem(i).getUrl().equals(other.getItem(i).getUrl())) { - return false; - } - } + return streams.equals(other.streams); + } - return true; + @Override + public int hashCode() { + return streams.hashCode(); } public boolean isDisposed() { 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 new file mode 100644 index 000000000..43e090900 --- /dev/null +++ b/app/src/test/java/org/schabi/newpipe/player/playqueue/PlayQueueTest.java @@ -0,0 +1,183 @@ +package org.schabi.newpipe.player.playqueue; + +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.schabi.newpipe.extractor.stream.StreamInfoItem; +import org.schabi.newpipe.extractor.stream.StreamType; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Objects; + +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.assertTrue; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.spy; + +@SuppressWarnings("checkstyle:HideUtilityClassConstructor") +public class PlayQueueTest { + static PlayQueue makePlayQueue(final int index, final List streams) { + // I tried using Mockito, but it didn't work for some reason + return new PlayQueue(index, streams) { + @Override + public boolean isComplete() { + throw new UnsupportedOperationException(); + } + + @Override + public void fetch() { + throw new UnsupportedOperationException(); + } + }; + } + + static PlayQueueItem makeItemWithUrl(final String url) { + final StreamInfoItem infoItem = new StreamInfoItem( + 0, url, "", StreamType.VIDEO_STREAM + ); + return new PlayQueueItem(infoItem); + } + + public static class SetIndexTests { + private static final int SIZE = 5; + private PlayQueue nonEmptyQueue; + private PlayQueue emptyQueue; + + @Before + public void setup() { + final List streams = new ArrayList<>(5); + for (int i = 0; i < 5; ++i) { + streams.add(makeItemWithUrl("URL_" + i)); + } + nonEmptyQueue = spy(makePlayQueue(0, streams)); + emptyQueue = spy(makePlayQueue(0, new ArrayList<>())); + } + + @Test + public void negative() { + nonEmptyQueue.setIndex(-5); + assertEquals(0, nonEmptyQueue.getIndex()); + + emptyQueue.setIndex(-5); + assertEquals(0, nonEmptyQueue.getIndex()); + } + + @Test + public void inBounds() { + nonEmptyQueue.setIndex(2); + assertEquals(2, nonEmptyQueue.getIndex()); + + // emptyQueue not tested because 0 isn't technically inBounds + } + + @Test + public void outOfBoundIsComplete() { + doReturn(true).when(nonEmptyQueue).isComplete(); + nonEmptyQueue.setIndex(7); + assertEquals(2, nonEmptyQueue.getIndex()); + + doReturn(true).when(emptyQueue).isComplete(); + emptyQueue.setIndex(2); + assertEquals(0, emptyQueue.getIndex()); + } + + @Test + public void outOfBoundsNotComplete() { + doReturn(false).when(nonEmptyQueue).isComplete(); + nonEmptyQueue.setIndex(7); + assertEquals(SIZE - 1, nonEmptyQueue.getIndex()); + + doReturn(false).when(emptyQueue).isComplete(); + emptyQueue.setIndex(2); + assertEquals(0, emptyQueue.getIndex()); + } + + @Test + public void indexZero() { + nonEmptyQueue.setIndex(0); + assertEquals(0, nonEmptyQueue.getIndex()); + + doReturn(true).when(emptyQueue).isComplete(); + emptyQueue.setIndex(0); + assertEquals(0, emptyQueue.getIndex()); + + doReturn(false).when(emptyQueue).isComplete(); + emptyQueue.setIndex(0); + assertEquals(0, emptyQueue.getIndex()); + } + + @Test + public void addToHistory() { + nonEmptyQueue.setIndex(0); + assertFalse(nonEmptyQueue.previous()); + + nonEmptyQueue.setIndex(3); + assertTrue(nonEmptyQueue.previous()); + assertEquals("URL_0", Objects.requireNonNull(nonEmptyQueue.getItem()).getUrl()); + } + } + + public static class GetItemTests { + private static List streams; + private PlayQueue queue; + + @BeforeClass + public static void init() { + streams = new ArrayList<>(Collections.nCopies(5, makeItemWithUrl("OTHER_URL"))); + streams.set(3, makeItemWithUrl("TARGET_URL")); + } + + @Before + public void setup() { + queue = makePlayQueue(0, streams); + } + + @Test + public void inBounds() { + assertEquals("TARGET_URL", Objects.requireNonNull(queue.getItem(3)).getUrl()); + assertEquals("OTHER_URL", Objects.requireNonNull(queue.getItem(1)).getUrl()); + } + + @Test + public void outOfBounds() { + assertNull(queue.getItem(-1)); + assertNull(queue.getItem(5)); + } + } + + public static class EqualsTests { + private final PlayQueueItem item1 = makeItemWithUrl("URL_1"); + private final PlayQueueItem item2 = makeItemWithUrl("URL_2"); + + @Test + public void sameStreams() { + final List streams = Collections.nCopies(5, item1); + final PlayQueue queue1 = makePlayQueue(0, streams); + final PlayQueue queue2 = makePlayQueue(0, streams); + assertEquals(queue1, queue2); + } + + @Test + public void sameSizeDifferentItems() { + final List streams1 = Collections.nCopies(5, item1); + final List streams2 = Collections.nCopies(5, item2); + final PlayQueue queue1 = makePlayQueue(0, streams1); + final PlayQueue queue2 = makePlayQueue(0, streams2); + assertNotEquals(queue1, queue2); + } + + @Test + public void differentSizeStreams() { + final List streams1 = Collections.nCopies(5, item1); + final List streams2 = Collections.nCopies(6, item2); + final PlayQueue queue1 = makePlayQueue(0, streams1); + final PlayQueue queue2 = makePlayQueue(0, streams2); + assertNotEquals(queue1, queue2); + } + } +}