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.

Sign up for GitHub

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

Do not return error on not found for delete operations #628

Merged
merged 5 commits into from
Jun 28, 2024
Merged

Do not return error on not found for delete operations #628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
de086b5
Do not return error on not found for delete operations
miguelreiswildlife Jun 27, 2024
4e1a2a0
fix lint
miguelreiswildlife Jun 27, 2024
2136d07
fix tests
miguelreiswildlife Jun 28, 2024
e1af2f4
Add test for not found case
miguelreiswildlife Jun 28, 2024
bde788d
fix lint
miguelreiswildlife Jun 28, 2024
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
11 changes: 6 additions & 5 deletions internal/core/operations/rooms/remove/executor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ package remove

import (
"context"
"errors"
"fmt"
"sync"

Expand All @@ -32,6 +33,7 @@ import (
"github.com/topfreegames/maestro/internal/core/logs"
"github.com/topfreegames/maestro/internal/core/operations"
"github.com/topfreegames/maestro/internal/core/ports"
porterrors "github.com/topfreegames/maestro/internal/core/ports/errors"
"go.uber.org/zap"
"golang.org/x/sync/errgroup"
)
Expand Down Expand Up @@ -93,11 +95,7 @@ func (e *Executor) Execute(ctx context.Context, op *operation.Operation, definit
func (e *Executor) removeRoomsByIDs(ctx context.Context, schedulerName string, roomsIDs []string, op *operation.Operation) error {
rooms := make([]*game_room.GameRoom, 0, len(roomsIDs))
for _, roomID := range roomsIDs {
gameRoom, err := e.roomStorage.GetRoom(ctx, schedulerName, roomID)
if err != nil {
return err
}

gameRoom := &game_room.GameRoom{ID: roomID, SchedulerID: schedulerName}
rooms = append(rooms, gameRoom)
}

Expand Down Expand Up @@ -134,6 +132,9 @@ func (e *Executor) deleteRooms(ctx context.Context, rooms []*game_room.GameRoom,
msg := fmt.Sprintf("error removing room \"%v\". Reason => %v", room.ID, err.Error())
e.operationManager.AppendOperationEventToExecutionHistory(ctx, op, msg)
}
if errors.Is(err, porterrors.ErrNotFound) {
return nil
}
return err
})
}
Expand Down
64 changes: 21 additions & 43 deletions internal/core/operations/rooms/remove/executor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ import (
"fmt"
"testing"

serviceerrors "github.com/topfreegames/maestro/internal/core/services/errors"

"github.com/golang/mock/gomock"
"github.com/google/uuid"
"github.com/stretchr/testify/require"
porterrors "github.com/topfreegames/maestro/internal/core/ports/errors"
mockports "github.com/topfreegames/maestro/internal/core/ports/mock"
serviceerrors "github.com/topfreegames/maestro/internal/core/services/errors"

"github.com/topfreegames/maestro/internal/core/entities/game_room"
"github.com/topfreegames/maestro/internal/core/entities/operation"
Expand Down Expand Up @@ -151,7 +151,7 @@ func TestExecutor_Execute(t *testing.T) {
})

t.Run("should succeed - rooms are successfully removed => returns without error", func(t *testing.T) {
executor, roomsStorage, roomsManager, _ := testSetup(t)
executor, _, roomsManager, _ := testSetup(t)

firstRoomID := "first-room-id"
secondRoomID := "second-room-id"
Expand All @@ -163,26 +163,20 @@ func TestExecutor_Execute(t *testing.T) {
room := &game_room.GameRoom{
ID: firstRoomID,
SchedulerID: schedulerName,
Status: game_room.GameStatusReady,
Metadata: map[string]interface{}{},
}
secondRoom := &game_room.GameRoom{
ID: secondRoomID,
SchedulerID: schedulerName,
Status: game_room.GameStatusReady,
Metadata: map[string]interface{}{},
}
roomsStorage.EXPECT().GetRoom(gomock.Any(), schedulerName, firstRoomID).Return(room, nil)
roomsStorage.EXPECT().GetRoom(gomock.Any(), schedulerName, secondRoomID).Return(secondRoom, nil)
roomsManager.EXPECT().DeleteRoom(gomock.Any(), room).Return(nil)
roomsManager.EXPECT().DeleteRoom(gomock.Any(), secondRoom).Return(nil)

err := executor.Execute(context.Background(), op, definition)
require.Nil(t, err)
})

t.Run("when failed to get any room it returns with error", func(t *testing.T) {
executor, roomsStorage, _, _ := testSetup(t)
t.Run("when any room failed to delete with unexpected error it returns with error", func(t *testing.T) {
executor, _, roomsManager, operationManager := testSetup(t)

firstRoomID := "first-room-id"
secondRoomID := "second-room-id"
Expand All @@ -194,17 +188,22 @@ func TestExecutor_Execute(t *testing.T) {
room := &game_room.GameRoom{
ID: firstRoomID,
SchedulerID: schedulerName,
Status: game_room.GameStatusReady,
}
roomsStorage.EXPECT().GetRoom(gomock.Any(), schedulerName, firstRoomID).Return(room, nil)
roomsStorage.EXPECT().GetRoom(gomock.Any(), schedulerName, secondRoomID).Return(nil, fmt.Errorf("error on GetRoom"))
secondRoom := &game_room.GameRoom{
ID: secondRoomID,
SchedulerID: schedulerName,
}

roomsManager.EXPECT().DeleteRoom(gomock.Any(), room).Return(nil)
roomsManager.EXPECT().DeleteRoom(gomock.Any(), secondRoom).Return(fmt.Errorf("error on remove room"))
operationManager.EXPECT().AppendOperationEventToExecutionHistory(gomock.Any(), op, gomock.Any())

err := executor.Execute(context.Background(), op, definition)
require.ErrorContains(t, err, "error removing rooms by ids: error on GetRoom")
require.ErrorContains(t, err, "error removing rooms by ids: error on remove room")
})

t.Run("when any room failed to delete with unexpected error it returns with error", func(t *testing.T) {
executor, roomsStorage, roomsManager, operationManager := testSetup(t)
t.Run("when any room returns not found on delete it is ignored and returns without error", func(t *testing.T) {
executor, _, roomsManager, operationManager := testSetup(t)

firstRoomID := "first-room-id"
secondRoomID := "second-room-id"
Expand All @@ -216,26 +215,21 @@ func TestExecutor_Execute(t *testing.T) {
room := &game_room.GameRoom{
ID: firstRoomID,
SchedulerID: schedulerName,
Status: game_room.GameStatusReady,
}
secondRoom := &game_room.GameRoom{
ID: firstRoomID,
ID: secondRoomID,
SchedulerID: schedulerName,
Status: game_room.GameStatusReady,
}
roomsStorage.EXPECT().GetRoom(gomock.Any(), schedulerName, firstRoomID).Return(room, nil)
roomsStorage.EXPECT().GetRoom(gomock.Any(), schedulerName, secondRoomID).Return(secondRoom, nil)
roomsManager.EXPECT().DeleteRoom(gomock.Any(), room).Return(nil)

roomsManager.EXPECT().DeleteRoom(gomock.Any(), secondRoom).Return(fmt.Errorf("error on remove room"))
roomsManager.EXPECT().DeleteRoom(gomock.Any(), secondRoom).Return(porterrors.NewErrNotFound("not found"))
operationManager.EXPECT().AppendOperationEventToExecutionHistory(gomock.Any(), op, gomock.Any())

err := executor.Execute(context.Background(), op, definition)
require.ErrorContains(t, err, "error removing rooms by ids: error on remove room")
require.NoError(t, err)
})

t.Run("when any room failed to delete with timeout error it returns with error", func(t *testing.T) {
executor, roomsStorage, roomsManager, operationManager := testSetup(t)
executor, _, roomsManager, operationManager := testSetup(t)

firstRoomID := "first-room-id"
secondRoomID := "second-room-id"
Expand All @@ -247,15 +241,11 @@ func TestExecutor_Execute(t *testing.T) {
room := &game_room.GameRoom{
ID: firstRoomID,
SchedulerID: schedulerName,
Status: game_room.GameStatusReady,
}
secondRoom := &game_room.GameRoom{
ID: secondRoomID,
SchedulerID: schedulerName,
Status: game_room.GameStatusReady,
}
roomsStorage.EXPECT().GetRoom(gomock.Any(), schedulerName, firstRoomID).Return(room, nil)
roomsStorage.EXPECT().GetRoom(gomock.Any(), schedulerName, secondRoomID).Return(secondRoom, nil)
roomsManager.EXPECT().DeleteRoom(gomock.Any(), room).Return(nil)
roomsManager.EXPECT().DeleteRoom(gomock.Any(), secondRoom).Return(serviceerrors.NewErrGameRoomStatusWaitingTimeout("some error"))
operationManager.EXPECT().AppendOperationEventToExecutionHistory(gomock.Any(), op, gomock.Any())
Expand All @@ -278,7 +268,7 @@ func TestExecutor_Execute(t *testing.T) {
})

t.Run("should succeed - there are ids and amount => return without error", func(t *testing.T) {
executor, roomsStorage, roomsManager, _ := testSetup(t)
executor, _, roomsManager, _ := testSetup(t)

firstRoomID := "first-room-id"
secondRoomID := "second-room-id"
Expand All @@ -292,16 +282,6 @@ func TestExecutor_Execute(t *testing.T) {
}
op := &operation.Operation{ID: "random-uuid", SchedulerName: schedulerName}

room := &game_room.GameRoom{
ID: firstRoomID,
SchedulerID: schedulerName,
Status: game_room.GameStatusReady,
}
secondRoom := &game_room.GameRoom{
ID: secondRoomID,
SchedulerID: schedulerName,
Status: game_room.GameStatusReady,
}
thirdRoom := &game_room.GameRoom{
ID: thirdRoomID,
SchedulerID: schedulerName,
Expand All @@ -312,8 +292,6 @@ func TestExecutor_Execute(t *testing.T) {
SchedulerID: schedulerName,
Status: game_room.GameStatusReady,
}
roomsStorage.EXPECT().GetRoom(gomock.Any(), schedulerName, firstRoomID).Return(room, nil)
roomsStorage.EXPECT().GetRoom(gomock.Any(), schedulerName, secondRoomID).Return(secondRoom, nil)
roomsManager.EXPECT().DeleteRoom(gomock.Any(), gomock.Any()).Return(nil).Times(2)

availableRooms := []*game_room.GameRoom{thirdRoom, fourthRoom}
Expand Down
Loading