Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test and clean up PlayQueue #6345

Merged
merged 20 commits into from
May 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 60 additions & 44 deletions app/src/main/java/org/schabi/newpipe/player/playqueue/PlayQueue.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,29 +40,25 @@
*/
public abstract class PlayQueue implements Serializable {
public static final boolean DEBUG = MainActivity.DEBUG;

private ArrayList<PlayQueueItem> backup;
private ArrayList<PlayQueueItem> streams;

@NonNull
private final AtomicInteger queueIndex;
private final ArrayList<PlayQueueItem> history;
private final List<PlayQueueItem> history = new ArrayList<>();

private List<PlayQueueItem> backup;
private List<PlayQueueItem> streams;

private transient BehaviorSubject<PlayQueueEvent> eventBroadcast;
private transient Flowable<PlayQueueEvent> broadcastReceiver;

private transient boolean disposed;
private transient boolean disposed = false;

PlayQueue(final int index, final List<PlayQueueItem> 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;
}

/*//////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -137,18 +133,36 @@ public int getIndex() {
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));
}

Expand Down Expand Up @@ -180,8 +194,6 @@ public PlayQueueItem getItem(final int index) {
* @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);
}

Expand Down Expand Up @@ -410,34 +422,42 @@ public synchronized void unsetRecovery(final int index) {
}

/**
* Shuffles the current play queue.
* Shuffles the current play queue
* <p>
* 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.
* </p>
* <p>
* Will emit a {@link ReorderEvent} in any context.
* Will emit a {@link ReorderEvent} if shuffled.
* </p>
*
* @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));
}

/**
Expand All @@ -457,7 +477,6 @@ public synchronized void unshuffle() {
final int originIndex = getIndex();
final PlayQueueItem current = getItem();

streams.clear();
streams = backup;
backup = null;

Expand Down Expand Up @@ -500,22 +519,19 @@ public synchronized boolean previous() {
* 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() {
Expand Down
Original file line number Diff line number Diff line change
@@ -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<PlayQueueItem> 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<PlayQueueItem> 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<PlayQueueItem> 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<PlayQueueItem> 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<PlayQueueItem> streams1 = Collections.nCopies(5, item1);
final List<PlayQueueItem> 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<PlayQueueItem> streams1 = Collections.nCopies(5, item1);
final List<PlayQueueItem> streams2 = Collections.nCopies(6, item2);
final PlayQueue queue1 = makePlayQueue(0, streams1);
final PlayQueue queue2 = makePlayQueue(0, streams2);
assertNotEquals(queue1, queue2);
}
}
}