[FEATURE] Fixed bug when progress of the playback was calculated wron…

…gly (#71) Fixed bug when progress was calculated wrongly because playStartTime, lastPauseTime weren't persisted to repository. Added NOT FRAGILE unit tests for playback progress calculation by inctroducing Clock object.
Project-Sonata · Aug 4, 2024 · 16b9dc2 · 16b9dc2
1 parent a3eba97
commit 16b9dc2
Show file tree

Hide file tree

Showing 12 changed files with 335 additions and 42 deletions.
diff --git a/src/main/java/com/odeyalo/sonata/connect/entity/PlayerStateEntity.java b/src/main/java/com/odeyalo/sonata/connect/entity/PlayerStateEntity.java
@@ -19,10 +19,12 @@
 public class PlayerStateEntity {
     Long id;
     boolean playing;
-    @Nullable
-    RepeatState repeatState;
     @NotNull
-    ShuffleMode shuffleState;
+    @Builder.Default
+    RepeatState repeatState = RepeatState.OFF;
+    @NotNull
+    @Builder.Default
+    ShuffleMode shuffleState = ShuffleMode.OFF;
     @Builder.Default
     Long progressMs = 0L;
     @Nullable
@@ -36,21 +38,15 @@ public class PlayerStateEntity {
     PlayableItemEntity currentlyPlayingItem;
     @NotNull
     Volume volume;
-    @Getter(value = AccessLevel.PRIVATE)
-    @Setter(value = AccessLevel.PRIVATE)
     long playStartTime = 0;
-    @Getter(value = AccessLevel.PRIVATE)
-    @Setter(value = AccessLevel.PRIVATE)
     long lastPauseTime = 0;
 
-    public static final boolean SHUFFLE_ENABLED = true;
-    public static final boolean SHUFFLE_DISABLED = false;
-
     @NotNull
     public ShuffleMode getShuffleState() {
         return shuffleState;
     }
 
+    @Nullable
     public PlayingType getCurrentlyPlayingType() {
         return playingType;
     }
@@ -60,6 +56,7 @@ public PlayableItemEntity getCurrentlyPlayingItem() {
         return currentlyPlayingItem;
     }
 
+    @NotNull
     public DevicesEntity getDevices() {
         return devicesEntity;
     }

diff --git a/src/main/java/com/odeyalo/sonata/connect/entity/factory/DefaultPlayerStateEntityFactory.java b/src/main/java/com/odeyalo/sonata/connect/entity/factory/DefaultPlayerStateEntityFactory.java
@@ -31,6 +31,7 @@ public PlayerStateEntity create(@NotNull final CurrentPlayerState state) {
                 .shuffleState(state.getShuffleState())
                 .user(UserEntity.builder().id(state.getUser().getId()).build())
                 .devicesEntity(DevicesEntity.fromCollection(devices))
+                .progressMs(state.getProgressMs())
                 .lastPauseTime(state.getLastPauseTime())
                 .playStartTime(state.getPlayStartTime());
 

diff --git a/src/main/java/com/odeyalo/sonata/connect/model/CurrentPlayerState.java b/src/main/java/com/odeyalo/sonata/connect/model/CurrentPlayerState.java
@@ -2,6 +2,8 @@
 
 import com.odeyalo.sonata.connect.service.player.TargetDeactivationDevice;
 import com.odeyalo.sonata.connect.service.player.TargetDevice;
+import com.odeyalo.sonata.connect.support.time.Clock;
+import com.odeyalo.sonata.connect.support.time.JavaClock;
 import lombok.AllArgsConstructor;
 import lombok.Builder;
 import lombok.Value;
@@ -45,6 +47,9 @@ public class CurrentPlayerState {
     long lastPauseTime = 0;
     @Builder.Default
     long playStartTime = 0;
+    @NotNull
+    @Builder.Default
+    Clock clock = JavaClock.instance();
 
     @NotNull
     public static CurrentPlayerState emptyFor(@NotNull final User user) {
@@ -54,6 +59,11 @@ public static CurrentPlayerState emptyFor(@NotNull final User user) {
                 .build();
     }
 
+    @NotNull
+    public CurrentPlayerState useClock(@NotNull final Clock clock) {
+        return withClock(clock);
+    }
+
     @NotNull
     public ShuffleMode getShuffleState() {
         return shuffleState;
@@ -102,9 +112,16 @@ public long getProgressMs() {
             return -1L;
         }
 
+        final long currentProgress = getCurrentProgressMs();
+
+        if ( playableItem.getDuration().isExceeded(currentProgress) ) {
+            return playableItem.getDuration().asMilliseconds();
+        }
+
         if ( isPlaying() ) {
-            return progressMs + calculateProgress();
+            return currentProgress;
         }
+
         return progressMs;
     }
 
@@ -139,7 +156,7 @@ public CurrentPlayerState play(@NotNull final PlayableItem item) {
                 .playing(true)
                 .playableItem(item)
                 .playingType(PlayingType.valueOf(item.getItemType().name()))
-                .playStartTime(System.currentTimeMillis())
+                .playStartTime(clock.currentTimeMillis())
                 .progressMs(0L)
                 .build();
     }
@@ -148,7 +165,7 @@ public CurrentPlayerState play(@NotNull final PlayableItem item) {
     public CurrentPlayerState resumePlayback() {
         return this.toBuilder()
                 .playing(true)
-                .playStartTime(System.currentTimeMillis())
+                .playStartTime(clock.currentTimeMillis())
                 .build();
     }
 
@@ -157,17 +174,21 @@ public CurrentPlayerState pause() {
         if ( isPlaying() ) {
             return this.toBuilder()
                     .playing(false)
-                    .lastPauseTime(System.currentTimeMillis())
-                    .progressMs(progressMs + calculateProgress())
+                    .lastPauseTime(clock.currentTimeMillis())
+                    .progressMs(getCurrentProgressMs())
                     .build();
         }
 
         return this;
     }
 
-    private long calculateProgress() {
+    private long getCurrentProgressMs() {
+        return progressMs + computeElapsedTime();
+    }
+
+    private long computeElapsedTime() {
         if ( isPlaying() ) {
-            return System.currentTimeMillis() - playStartTime;
+            return clock.currentTimeMillis() - playStartTime;
         } else {
             return lastPauseTime - playStartTime;
         }

diff --git a/src/main/java/com/odeyalo/sonata/connect/model/PlayableItem.java b/src/main/java/com/odeyalo/sonata/connect/model/PlayableItem.java
@@ -14,7 +14,9 @@ public interface PlayableItem {
     @NotNull
     PlayableItemType getItemType();
 
-
     @NotNull
     ContextUri getContextUri();
+
+    @NotNull
+    PlayableItemDuration getDuration();
 }
diff --git a/src/main/java/com/odeyalo/sonata/connect/model/PlayableItemDuration.java b/src/main/java/com/odeyalo/sonata/connect/model/PlayableItemDuration.java
@@ -3,8 +3,11 @@
 import lombok.AccessLevel;
 import lombok.Getter;
 import lombok.Value;
+import org.jetbrains.annotations.NotNull;
 import org.springframework.util.Assert;
 
+import java.time.Duration;
+
 /**
  * Represent a duration of the {@link PlayableItem}
  */
@@ -22,15 +25,31 @@ public static PlayableItemDuration ofMilliseconds(long durationMs) {
         return new PlayableItemDuration(durationMs);
     }
 
+    @NotNull
     public static PlayableItemDuration ofSeconds(long durationSec) {
         return ofMilliseconds(durationSec * 1000);
     }
 
+    @NotNull
+    public static PlayableItemDuration fromJavaDuration(@NotNull final Duration itemDuration) {
+        return ofMilliseconds(itemDuration.toMillis());
+    }
+
     public long asMilliseconds() {
         return durationMs;
     }
 
     public long asSeconds() {
         return durationMs / 1000;
     }
+
+    /**
+     * Check if the provided millis exceed the item duration
+     * @param millisToCompare - millis to compare with item duration
+     * @return {@code true} if provided millis exceed the playable item duration
+     * {@code false} otherwise
+     */
+    public boolean isExceeded(final long millisToCompare) {
+        return asMilliseconds() < millisToCompare;
+    }
 }
diff --git a/src/main/java/com/odeyalo/sonata/connect/support/time/Clock.java b/src/main/java/com/odeyalo/sonata/connect/support/time/Clock.java
@@ -0,0 +1,17 @@
+package com.odeyalo.sonata.connect.support.time;
+
+import org.jetbrains.annotations.NotNull;
+
+import java.time.Instant;
+
+/**
+ * A wrapper around the system clock to allow custom implementations to be used in unit tests where we want to fake or control the clock behavior.
+ */
+public interface Clock {
+
+    @NotNull
+    Instant now();
+
+    long currentTimeMillis();
+
+}
diff --git a/src/main/java/com/odeyalo/sonata/connect/support/time/JavaClock.java b/src/main/java/com/odeyalo/sonata/connect/support/time/JavaClock.java
@@ -0,0 +1,27 @@
+package com.odeyalo.sonata.connect.support.time;
+
+import org.jetbrains.annotations.NotNull;
+
+import java.time.Instant;
+
+public final class JavaClock implements Clock {
+    private static final JavaClock INSTANCE = new JavaClock();
+
+    private JavaClock() {}
+
+    @NotNull
+    public static JavaClock instance() {
+        return INSTANCE;
+    }
+
+    @Override
+    @NotNull
+    public Instant now() {
+        return Instant.now();
+    }
+
+    @Override
+    public long currentTimeMillis() {
+        return now().toEpochMilli();
+    }
+}