Merge pull request #179 from camphor-/revert-169-create_next_api

Revert "曲のSkipのためのAPIを実装した"

docs/api.md

@@ -256,36 +256,6 @@ X-CSRF-Token: relaym
 
 [Spotify APIの不思議な挙動](sotify_api_problem.md)
 
-## PUT /sessions/:id/next
-
-### 概要
-
-指定したセッションを一曲進めます。
-
-### リクエスト
-
-空
-
-### レスポンス
-
-空
-
-| code  |   補足    |
-| ----- | -------- | 
-| 202   |          |
-
-非同期的にレスポンスを返すので、実際に状態が反映されたかWebSocketのメッセージか別のAPIリクエストを通して取得する必要があります。
-
-### エラー
-
-| code | message | 補足 |
-| ---- | -------- | -------- |
-| 400 | session is not allowed to control by others | 作成者以外によるstateの操作が許可されていない | 
-| 400 | requested state is not allowed | 許可されていないstateへの変更(許可されているstateの変更は[PRD](prd.md)を参照) |
-| 400 | next queue track not found | 次のキューが無いので次の曲に遷移できない |   
-| 403 | active device not found | アクティブなデバイスが存在しないので操作ができない |
-| 404 | session not found | 指定されたidのセッションが存在しない |
-
 ## POST /sessions/:id/queue
 
 ### 概要

domain/entity/session.go

@@ -71,7 +71,7 @@ func (s *Session) MoveToPlay() error {
 
 // MoveToPause はセッションのStateTypeをPauseに状態遷移します。
 func (s *Session) MoveToPause() error {
-	if s.StateType == Play || s.StateType == Pause || s.StateType == Stop {
+	if s.StateType == Play || s.StateType == Pause {
 		s.StateType = Pause
 		return nil
 	}
@@ -97,7 +97,6 @@ func (s *Session) IsCreator(userID string) bool {
 
 // GoNextTrack 次の曲の状態に進めます。
 func (s *Session) GoNextTrack() error {
-	s.SetProgressWhenPaused(0 * time.Second)
 	if len(s.QueueTracks) <= s.QueueHead+1 {
 		s.QueueHead++ // https://github.com/camphor-/relaym-server/blob/master/docs/definition.md#%E7%8F%BE%E5%9C%A8%E5%AF%BE%E8%B1%A1%E3%81%AE%E6%9B%B2%E3%81%AE%E3%82%A4%E3%83%B3%E3%83%87%E3%83%83%E3%82%AF%E3%82%B9-head
 		s.StateType = Stop
@@ -186,11 +185,6 @@ func (s *Session) canMoveFromStopToPlay() error {
 	return nil
 }
 
-// IsNextTrackExistWhenStateIsStop はstateがstopの時に次の曲が存在するかを調べます
-func (s *Session) IsNextTrackExistWhenStateIsStop() bool {
-	return len(s.QueueTracks) > s.QueueHead
-}
-
 // IsPlaying は現在のStateTypeがPlayかどうか返します。
 func (s *Session) IsPlaying() bool {
 	return s.StateType == Play

domain/entity/session_test.go

@@ -180,13 +180,6 @@ func TestSession_MoveToPause(t *testing.T) {
 			session: &Session{
 				StateType: Stop,
 			},
-			wantErr: false,
-		},
-		{
-			name: "Archived",
-			session: &Session{
-				StateType: Archived,
-			},
 			wantErr: true,
 		},
 	}

domain/entity/sync_check_timer.go

@@ -1,7 +1,6 @@
 package entity
 
 import (
-	"fmt"
 	"sync"
 	"time"
 
@@ -11,10 +10,8 @@ import (
 // SyncCheckTimer はSpotifyとの同期チェック用のタイマーです。タイマーが止まったことを確認するためのstopチャネルがあります。
 // ref : http://okzk.hatenablog.com/entry/2015/12/01/001924
 type SyncCheckTimer struct {
-	timer          *time.Timer
-	isTimerExpired bool
-	stopCh         chan struct{}
-	nextCh         chan struct{}
+	timer  *time.Timer
+	stopCh chan struct{}
 }
 
 // ExpireCh は指定設定された秒数経過したことを送るチャネルを返します。
@@ -27,44 +24,13 @@ func (s *SyncCheckTimer) StopCh() <-chan struct{} {
 	return s.stopCh
 }
 
-// NextCh は次の曲への遷移の指示を送るチャネルを返します。
-func (s *SyncCheckTimer) NextCh() <-chan struct{} {
-	return s.nextCh
-}
-
-// MakeIsTimerExpiredTrue はisTimerExpiredをtrueに変更します
-// <- s.ExpireCh でtimerから値を受け取った際に呼び出してください
-func (s *SyncCheckTimer) MakeIsTimerExpiredTrue() {
-	s.isTimerExpired = true
-}
-
-// newSyncCheckTimer はSyncCheckTimerを作成します
-// この段階ではtimerには空のtimerがセットされており、SetTimerを使用して正しいtimerのセットを行う必要があります
-func newSyncCheckTimer() *SyncCheckTimer {
-	timer := time.NewTimer(0)
-	//Expiredしたtimerを作成する
-	if !timer.Stop() {
-		<-timer.C
-	}
-
+func newSyncCheckTimer(d time.Duration) *SyncCheckTimer {
 	return &SyncCheckTimer{
-		stopCh:         make(chan struct{}, 2),
-		nextCh:         make(chan struct{}, 1),
-		isTimerExpired: true,
-		timer:          timer,
+		timer:  time.NewTimer(d),
+		stopCh: make(chan struct{}, 2),
 	}
 }
 
-// SetTimerはSyncCheckTimerにTimerをセットします
-func (s *SyncCheckTimer) SetDuration(d time.Duration) {
-	if !s.timer.Stop() && !s.isTimerExpired {
-		<-s.timer.C
-	}
-
-	s.isTimerExpired = false
-	s.timer.Reset(d)
-}
-
 // SyncCheckTimerManager はSpotifyとの同期チェック用のタイマーを一括して管理する構造体です。
 type SyncCheckTimerManager struct {
 	timers map[string]*SyncCheckTimer
@@ -78,9 +44,9 @@ func NewSyncCheckTimerManager() *SyncCheckTimerManager {
 	}
 }
 
-// CreateExpiredTimer は与えられたセッションの同期チェック用のタイマーを作成します。
+// CreateTimer は与えられたセッションの同期チェック用のタイマーを作成します。
 // 既存のタイマーが存在する場合はstopしてから新しいタイマーを作成します。
-func (m *SyncCheckTimerManager) CreateExpiredTimer(sessionID string) *SyncCheckTimer {
+func (m *SyncCheckTimerManager) CreateTimer(sessionID string, d time.Duration) *SyncCheckTimer {
 	logger := log.New()
 	m.mu.Lock()
 	defer m.mu.Unlock()
@@ -94,21 +60,23 @@ func (m *SyncCheckTimerManager) CreateExpiredTimer(sessionID string) *SyncCheckT
 		existing.timer.Stop()
 		close(existing.stopCh)
 	}
-	timer := newSyncCheckTimer()
+	timer := newSyncCheckTimer(d)
 	m.timers[sessionID] = timer
 	return timer
 }
 
-// DeleteTimer は与えられたセッションのタイマーをマップから削除します。
-// 既にタイマーがExpireして、そのチャネルの値を取り出してしまった後にマップから削除したいときに使います。
-func (m *SyncCheckTimerManager) DeleteTimer(sessionID string) {
+// StopTimer は与えられたセッションのタイマーを終了します。
+func (m *SyncCheckTimerManager) StopTimer(sessionID string) {
 	logger := log.New()
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
-	logger.Debugj(map[string]interface{}{"message": "delete timer", "sessionID": sessionID})
+	logger.Debugj(map[string]interface{}{"message": "stop timer", "sessionID": sessionID})
 
 	if timer, ok := m.timers[sessionID]; ok {
+		if !timer.timer.Stop() {
+			<-timer.timer.C
+		}
 		close(timer.stopCh)
 		delete(m.timers, sessionID)
 		return
@@ -117,44 +85,33 @@ func (m *SyncCheckTimerManager) DeleteTimer(sessionID string) {
 	logger.Debugj(map[string]interface{}{"message": "timer not existed", "sessionID": sessionID})
 }
 
-// GetTimer は与えられたセッションのタイマーを取得します。存在しない場合はfalseが返ります。
-func (m *SyncCheckTimerManager) GetTimer(sessionID string) (*SyncCheckTimer, bool) {
-	m.mu.Lock()
-	defer m.mu.Unlock()
-
-	if existing, ok := m.timers[sessionID]; ok {
-		return existing, true
-	}
-	return nil, false
-}
-
-// SendToNextCh は与えられたセッションのタイマーのNextChに通知を送ります
-func (m *SyncCheckTimerManager) SendToNextCh(sessionID string) error {
+// DeleteTimer は与えられたセッションのタイマーをマップから削除します。
+// StopTimerと異なり、タイマーのストップ処理は行いません。
+// 既にタイマーがExpireして、そのチャネルの値を取り出してしまった後にマップから削除したいときに使います。
+// <-timer.timer.Cを呼ぶと無限に待ちが発生してしまいます。(値を取り出すことは一生出来ないので)
+func (m *SyncCheckTimerManager) DeleteTimer(sessionID string) {
 	logger := log.New()
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
-	logger.Debugj(map[string]interface{}{"message": "call next ch", "sessionID": sessionID})
+	logger.Debugj(map[string]interface{}{"message": "delete timer", "sessionID": sessionID})
 
 	if timer, ok := m.timers[sessionID]; ok {
-		timer.nextCh <- struct{}{}
-		return nil
+		close(timer.stopCh)
+		delete(m.timers, sessionID)
+		return
 	}
 
-	logger.Debugj(map[string]interface{}{"message": "timer not existed on SendToNextCh", "sessionID": sessionID})
-	return fmt.Errorf("timer not existed")
+	logger.Debugj(map[string]interface{}{"message": "timer not existed", "sessionID": sessionID})
 }
 
-// IsTimerExpired は与えられたセッションのisTimerExpiredの値を返します
-func (m *SyncCheckTimerManager) IsTimerExpired(sessionID string) (bool, error) {
-	logger := log.New()
+// GetTimer は与えられたセッションのタイマーを取得します。存在しない場合はfalseが返ります。
+func (m *SyncCheckTimerManager) GetTimer(sessionID string) (*SyncCheckTimer, bool) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
 	if existing, ok := m.timers[sessionID]; ok {
-		return existing.isTimerExpired, nil
+		return existing, true
 	}
-
-	logger.Debugj(map[string]interface{}{"message": "timer not existed on IsRemainDuration", "sessionID": sessionID})
-	return false, fmt.Errorf("timer not existed")
+	return nil, false
 }

domain/entity/sync_check_timer_test.go

@@ -82,11 +82,10 @@ func TestSyncCheckTimer_StopCh(t *testing.T) {
 	}
 }
 
-func TestSyncCheckTimerManager_CreateExpiredTimer(t *testing.T) {
+func TestSyncCheckTimerManager_CreateTimer(t *testing.T) {
 	t.Parallel()
 
-	timer := newSyncCheckTimer()
-	timer.SetDuration(time.Second)
+	timer := newSyncCheckTimer(time.Second)
 
 	tests := []struct {
 		name      string
@@ -130,11 +129,67 @@ func TestSyncCheckTimerManager_CreateExpiredTimer(t *testing.T) {
 				return
 			}
 			opts := []cmp.Option{cmp.AllowUnexported(SyncCheckTimer{}), cmpopts.IgnoreUnexported(time.Timer{})}
-			got := m.CreateExpiredTimer(tt.sessionID)
-			got.SetDuration(tt.d)
-			if !cmp.Equal(got, tt.want, opts...) {
-				t.Errorf("CreateExpiredTimer() diff=%v", cmp.Diff(tt.want, got, opts...))
+			if got := m.CreateTimer(tt.sessionID, tt.d); !cmp.Equal(got, tt.want, opts...) {
+				t.Errorf("CreateTimer() diff=%v", cmp.Diff(tt.want, got, opts...))
+			}
+		})
+	}
+}
+
+func TestSyncCheckTimerManager_StopTimer(t *testing.T) {
+	t.Parallel()
+
+	timer := newSyncCheckTimer(time.Second)
+	timerForNotFound := newSyncCheckTimer(time.Second)
+
+	tests := []struct {
+		name      string
+		timer     *SyncCheckTimer
+		timers    map[string]*SyncCheckTimer
+		sessionID string
+		want      map[string]*SyncCheckTimer
+		wantPanic bool
+	}{
+		{
+			name:      "存在するセッションのタイマーが削除される",
+			timer:     timer,
+			timers:    map[string]*SyncCheckTimer{"sessionID": timer},
+			sessionID: "sessionID",
+			want:      map[string]*SyncCheckTimer{},
+			wantPanic: true,
+		},
+
+		{
+			name:      "存在しないセッションの場合は何も起こらない",
+			timer:     timerForNotFound,
+			timers:    map[string]*SyncCheckTimer{"sessionID": timerForNotFound},
+			sessionID: "not_found",
+			want:      map[string]*SyncCheckTimer{"sessionID": timerForNotFound},
+			wantPanic: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			defer func() {
+				err := recover()
+				if (err != nil) != tt.wantPanic {
+					t.Errorf("StopTimer() wantPanic=%v, but err=%v", tt.wantPanic, err)
+				}
+			}()
+
+			m := &SyncCheckTimerManager{
+				timers: tt.timers,
+				mu:     sync.Mutex{},
 			}
+			m.StopTimer(tt.sessionID)
+
+			opts := []cmp.Option{cmp.AllowUnexported(SyncCheckTimer{}), cmpopts.IgnoreUnexported(time.Timer{})}
+			if !cmp.Equal(m.timers, tt.want, opts...) {
+				t.Errorf("StopTimer() diff=%v", cmp.Diff(tt.want, m.timers, opts...))
+			}
+
+			// 既に閉じられているchannelに対してcloseするとpanicが起こることを利用して正しくcloseされているかチェックする
+			close(tt.timer.stopCh)
 		})
 	}
 }

domain/spotify/player.go

@@ -19,6 +19,5 @@ type Player interface {
 	Enqueue(ctx context.Context, trackURI string, deviceID string) error
 	SetRepeatMode(ctx context.Context, on bool, deviceID string) error
 	SetShuffleMode(ctx context.Context, on bool, deviceID string) error
-	DeleteAllTracksInQueue(ctx context.Context, deviceID string, trackURI string) error
-	GoNextTrack(ctx context.Context, deviceID string) error
+	SkipAllTracks(ctx context.Context, deviceID string, trackURI string) error
 }