diff --git a/server/src/database/notes_observer.go b/server/src/database/notes_observer.go deleted file mode 100644 index 7e4b2576af..0000000000 --- a/server/src/database/notes_observer.go +++ /dev/null @@ -1,85 +0,0 @@ -package database - -import ( - "context" - "scrumlr.io/server/identifiers" - - "github.com/google/uuid" - "github.com/uptrace/bun" - "scrumlr.io/server/common/filter" -) - -type NotesObserver interface { - Observer - - // UpdatedNotes will be called if the notes of the board with the specified id were updated. - UpdatedNotes(board uuid.UUID, notes []Note) - - // DeletedNote will be called if a note has been deleted. - DeletedNote(user, board, note uuid.UUID, votes []Vote, deleteStack bool) -} - -var _ bun.AfterInsertHook = (*NoteInsert)(nil) -var _ bun.AfterUpdateHook = (*NoteUpdate)(nil) -var _ bun.AfterDeleteHook = (*Note)(nil) - -func (*NoteInsert) AfterInsert(ctx context.Context, _ *bun.InsertQuery) error { - return notifyNotesUpdated(ctx) -} - -func (*NoteUpdate) AfterUpdate(ctx context.Context, _ *bun.UpdateQuery) error { - return notifyNotesUpdated(ctx) -} - -func (*Note) AfterDelete(ctx context.Context, _ *bun.DeleteQuery) error { - result := ctx.Value("Result").(*[]Note) - if len(*result) > 0 { - return notifyNoteDeleted(ctx) - } - return nil -} - -func notifyNotesUpdated(ctx context.Context) error { - if ctx.Value("Database") == nil { - return nil - } - d := ctx.Value("Database").(*Database) - if len(d.observer) > 0 { - board := ctx.Value(identifiers.BoardIdentifier).(uuid.UUID) - notes, err := d.GetNotes(board) - if err != nil { - return err - } - for _, observer := range d.observer { - if o, ok := observer.(NotesObserver); ok { - o.UpdatedNotes(board, notes) - return nil - } - } - } - return nil -} - -func notifyNoteDeleted(ctx context.Context) error { - if ctx.Value("Database") == nil { - return nil - } - d := ctx.Value("Database").(*Database) - if len(d.observer) > 0 { - user := ctx.Value(identifiers.UserIdentifier).(uuid.UUID) - board := ctx.Value(identifiers.BoardIdentifier).(uuid.UUID) - note := ctx.Value(identifiers.NoteIdentifier).(uuid.UUID) - deleteStack := ctx.Value("DeleteStack").(bool) - votes, err := d.GetVotes(filter.VoteFilter{Board: board}) - if err != nil { - return err - } - for _, observer := range d.observer { - if o, ok := observer.(NotesObserver); ok { - o.DeletedNote(user, board, note, votes, deleteStack) - return nil - } - } - } - return nil -} diff --git a/server/src/database/notes_observer_test.go b/server/src/database/notes_observer_test.go deleted file mode 100644 index 343d0138e6..0000000000 --- a/server/src/database/notes_observer_test.go +++ /dev/null @@ -1,106 +0,0 @@ -package database - -import ( - "testing" - - "github.com/google/uuid" - "github.com/stretchr/testify/assert" -) - -type NotesObserverForTests struct { - t *testing.T - board *uuid.UUID - notes *[]Note - deletedNote *uuid.UUID -} - -func (o *NotesObserverForTests) UpdatedNotes(board uuid.UUID, notes []Note) { - o.board = &board - o.notes = ¬es -} - -func (o *NotesObserverForTests) DeletedNote(user, board, note uuid.UUID, votes []Vote, deleteStack bool) { - o.board = &board - o.deletedNote = ¬e -} - -func (o *NotesObserverForTests) Reset() { - o.board = nil - o.notes = nil - o.deletedNote = nil -} - -var notesObserver NotesObserverForTests -var notesObserverForTestNote Note - -func TestNotesObserver(t *testing.T) { - notesObserver = NotesObserverForTests{t: t} - testDb.AttachObserver(¬esObserver) - - t.Run("Test=1", testNotesObserverOnCreate) - notesObserver.Reset() - t.Run("Test=2", testNotesObserverOnUpdate) - notesObserver.Reset() - t.Run("Test=3", testNotesObserverOnDelete) - notesObserver.Reset() - t.Run("Test=4", testNotesObserverOnDeleteNotExisting) - - _, _ = testDb.DetachObserver(notesObserver) -} - -func testNotesObserverOnCreate(t *testing.T) { - board := fixture.MustRow("Board.notesObserverTestBoard").(*Board) - column := fixture.MustRow("Column.notesObserverTestColumn").(*Column) - user := fixture.MustRow("User.jack").(*User) - - note, err := testDb.CreateNote(NoteInsert{ - Author: user.ID, - Board: board.ID, - Column: column.ID, - Text: "I just wanna test this", - }) - - assert.Nil(t, err) - assert.NotNil(t, notesObserver.board) - assert.NotNil(t, notesObserver.notes) - - assert.Equal(t, 1, len(*notesObserver.notes)) - assert.Equal(t, note.Board, (*notesObserver.notes)[0].Board) - assert.Equal(t, note.Column, (*notesObserver.notes)[0].Column) - - notesObserverForTestNote = note -} -func testNotesObserverOnUpdate(t *testing.T) { - textUpdate := "I updated that thing" - user := fixture.MustRow("User.jack").(*User) - - note, err := testDb.UpdateNote(user.ID, NoteUpdate{ - ID: notesObserverForTestNote.ID, - Board: notesObserverForTestNote.Board, - Text: &textUpdate, - }) - - assert.Nil(t, err) - assert.NotNil(t, notesObserver.board) - assert.NotNil(t, notesObserver.notes) - - assert.Equal(t, 1, len(*notesObserver.notes)) - assert.Equal(t, note.Board, (*notesObserver.notes)[0].Board) - assert.Equal(t, note.Column, (*notesObserver.notes)[0].Column) -} -func testNotesObserverOnDelete(t *testing.T) { - user := fixture.MustRow("User.jack").(*User) - deleteStack = false - err := testDb.DeleteNote(user.ID, notesObserverForTestNote.Board, notesObserverForTestNote.ID, deleteStack) - assert.Nil(t, err) - assert.NotNil(t, notesObserver.board) - assert.NotNil(t, notesObserver.deletedNote) -} -func testNotesObserverOnDeleteNotExisting(t *testing.T) { - user := fixture.MustRow("User.jack").(*User) - deleteStack = false - err := testDb.DeleteNote(user.ID, notesObserverForTestNote.Board, notesObserverForTestNote.ID, deleteStack) - assert.Nil(t, err) - assert.Nil(t, notesObserver.board) - assert.Nil(t, notesObserver.deletedNote) -} diff --git a/server/src/realtime/board_sessions_requests.go b/server/src/realtime/board_sessions_requests.go index 4f1c0bf3f5..fd4f5bc630 100644 --- a/server/src/realtime/board_sessions_requests.go +++ b/server/src/realtime/board_sessions_requests.go @@ -17,11 +17,11 @@ const ( func (b *Broker) BroadcastUpdateOnBoardSessionRequest(board, user uuid.UUID, msg BoardSessionRequestEventType) error { logger.Get().Debugw("broadcasting to board session request", "board", board, "user", user, "msg", msg) - return b.con.Publish(requestSubject(board, user), msg) + return b.Con.Publish(requestSubject(board, user), msg) } func (b *Broker) GetBoardSessionRequestChannel(board, user uuid.UUID) chan *BoardSessionRequestEventType { - c, err := b.con.SubscribeToBoardSessionEvents(requestSubject(board, user)) + c, err := b.Con.SubscribeToBoardSessionEvents(requestSubject(board, user)) if err != nil { // TODO: Bubble up this error, so the caller can retry to establish this subscription logger.Get().Errorw("failed to subscribe to BoardSessionRequestChannel", "err", err) diff --git a/server/src/realtime/boards.go b/server/src/realtime/boards.go index 556ec1a2bf..b24b7b5a37 100644 --- a/server/src/realtime/boards.go +++ b/server/src/realtime/boards.go @@ -41,11 +41,11 @@ type BoardEvent struct { func (b *Broker) BroadcastToBoard(boardID uuid.UUID, msg BoardEvent) error { logger.Get().Debugw("broadcasting to board", "board", boardID, "msg", msg.Type) - return b.con.Publish(boardsSubject(boardID), msg) + return b.Con.Publish(boardsSubject(boardID), msg) } func (b *Broker) GetBoardChannel(boardID uuid.UUID) chan *BoardEvent { - c, err := b.con.SubscribeToBoardEvents(boardsSubject(boardID)) + c, err := b.Con.SubscribeToBoardEvents(boardsSubject(boardID)) if err != nil { // TODO: Bubble up this error, so the caller can retry to establish this subscription logger.Get().Errorw("failed to subscribe to BoardChannel", "err", err) diff --git a/server/src/realtime/broker.go b/server/src/realtime/broker.go index 716aa4c305..9af890cc7b 100644 --- a/server/src/realtime/broker.go +++ b/server/src/realtime/broker.go @@ -17,5 +17,5 @@ type Client interface { // The Broker enables a user to broadcast and receive events type Broker struct { - con Client + Con Client } diff --git a/server/src/realtime/health.go b/server/src/realtime/health.go index 6a0c9e7952..8711a0dc60 100644 --- a/server/src/realtime/health.go +++ b/server/src/realtime/health.go @@ -2,9 +2,9 @@ package realtime // IsHealthy returns true if the Broker is in a healthy state func (b *Broker) IsHealthy() bool { - if b == nil || b.con == nil { + if b == nil || b.Con == nil { return false } - err := b.con.Publish("health", "test") + err := b.Con.Publish("health", "test") return err == nil } diff --git a/server/src/realtime/nats.go b/server/src/realtime/nats.go index 0688491cff..bd3eb8d550 100644 --- a/server/src/realtime/nats.go +++ b/server/src/realtime/nats.go @@ -49,6 +49,6 @@ func NewNats(url string) (*Broker, error) { } return &Broker{ - con: &natsClient{con: c}, + Con: &natsClient{con: c}, }, nil } diff --git a/server/src/realtime/redis.go b/server/src/realtime/redis.go index 66dd18f028..0f1d30e6db 100644 --- a/server/src/realtime/redis.go +++ b/server/src/realtime/redis.go @@ -10,7 +10,7 @@ import ( ) func NewRedis(server RedisServer) (*Broker, error) { - return &Broker{con: connectRedis(server)}, nil + return &Broker{Con: connectRedis(server)}, nil } type redisClient struct { diff --git a/server/src/services/notes/notes.go b/server/src/services/notes/notes.go index 5ce19204e1..1444ddd3ac 100644 --- a/server/src/services/notes/notes.go +++ b/server/src/services/notes/notes.go @@ -3,14 +3,15 @@ package notes import ( "context" "database/sql" - "scrumlr.io/server/identifiers" "scrumlr.io/server/common" + "scrumlr.io/server/identifiers" "scrumlr.io/server/services" "github.com/google/uuid" "scrumlr.io/server/common/dto" + "scrumlr.io/server/common/filter" "scrumlr.io/server/realtime" "scrumlr.io/server/database" @@ -27,19 +28,18 @@ type Observer interface { } type DB interface { - Observer CreateNote(insert database.NoteInsert) (database.Note, error) GetNote(id uuid.UUID) (database.Note, error) GetNotes(board uuid.UUID, columns ...uuid.UUID) ([]database.Note, error) UpdateNote(caller uuid.UUID, update database.NoteUpdate) (database.Note, error) DeleteNote(caller uuid.UUID, board uuid.UUID, id uuid.UUID, deleteStack bool) error + GetVotes(f filter.VoteFilter) ([]database.Vote, error) } func NewNoteService(db DB, rt *realtime.Broker) services.Notes { b := new(NoteService) b.database = db b.realtime = rt - b.database.AttachObserver((database.NotesObserver)(b)) return b } @@ -50,6 +50,7 @@ func (s *NoteService) Create(ctx context.Context, body dto.NoteCreateRequest) (* log.Errorw("unable to create note", "board", body.Board, "user", body.User, "error", err) return nil, common.InternalServerError } + s.UpdatedNotes(body.Board) return new(dto.Note).From(note), err } @@ -98,18 +99,46 @@ func (s *NoteService) Update(ctx context.Context, body dto.NoteUpdateRequest) (* log.Errorw("unable to update note", "error", err, "note", body.ID) return nil, common.InternalServerError } + s.UpdatedNotes(body.Board) return new(dto.Note).From(note), err } func (s *NoteService) Delete(ctx context.Context, body dto.NoteDeleteRequest, id uuid.UUID) error { - return s.database.DeleteNote(ctx.Value(identifiers.UserIdentifier).(uuid.UUID), ctx.Value(identifiers.BoardIdentifier).(uuid.UUID), id, body.DeleteStack) + user := ctx.Value(identifiers.UserIdentifier).(uuid.UUID) + board := ctx.Value(identifiers.BoardIdentifier).(uuid.UUID) + note := ctx.Value(identifiers.NoteIdentifier).(uuid.UUID) + voteFilter := filter.VoteFilter{ + User: &user, + Board: board, + Note: ¬e, + } + + votes, vErr := s.database.GetVotes(voteFilter) + if vErr != nil { + logger.Get().Errorw("unable to retrieve votes for a note delete", "err", vErr) + } + + err := s.database.DeleteNote(user, board, id, body.DeleteStack) + if err != nil { + return err + } + + s.DeletedNote(user, board, note, votes, body.DeleteStack) + + return err } -func (s *NoteService) UpdatedNotes(board uuid.UUID, notes []database.Note) { +func (s *NoteService) UpdatedNotes(board uuid.UUID) { + notes, dbErr := s.database.GetNotes(board) + if dbErr != nil { + logger.Get().Errorw("unable to retrieve notes in UpdatedNotes call", "err", dbErr) + } + eventNotes := make([]dto.Note, len(notes)) for index, note := range notes { eventNotes[index] = *new(dto.Note).From(note) } + err := s.realtime.BroadcastToBoard(board, realtime.BoardEvent{ Type: realtime.BoardEventNotesUpdated, Data: eventNotes, diff --git a/server/src/services/notes/notes_test.go b/server/src/services/notes/notes_test.go index ae8510f8cc..1c67ed8760 100644 --- a/server/src/services/notes/notes_test.go +++ b/server/src/services/notes/notes_test.go @@ -2,14 +2,18 @@ package notes import ( "context" + + "scrumlr.io/server/identifiers" + "scrumlr.io/server/realtime" + "database/sql" "errors" "net/http" - "scrumlr.io/server/identifiers" "testing" "scrumlr.io/server/common" "scrumlr.io/server/common/dto" + "scrumlr.io/server/common/filter" "github.com/google/uuid" "github.com/stretchr/testify/assert" @@ -22,6 +26,25 @@ type NoteServiceTestSuite struct { suite.Suite } +type mockRtClient struct { + mock.Mock +} + +func (mc *mockRtClient) Publish(subject string, event interface{}) error { + args := mc.Called(subject, event) + return args.Error(0) +} + +func (mc *mockRtClient) SubscribeToBoardSessionEvents(subject string) (chan *realtime.BoardSessionRequestEventType, error) { + args := mc.Called(subject) + return args.Get(0).(chan *realtime.BoardSessionRequestEventType), args.Error(1) +} + +func (mc *mockRtClient) SubscribeToBoardEvents(subject string) (chan *realtime.BoardEvent, error) { + args := mc.Called(subject) + return args.Get(0).(chan *realtime.BoardEvent), args.Error(1) +} + type DBMock struct { DB mock.Mock @@ -52,6 +75,11 @@ func (m *DBMock) DeleteNote(caller uuid.UUID, board uuid.UUID, id uuid.UUID, del return args.Error(0) } +func (m *DBMock) GetVotes(f filter.VoteFilter) ([]database.Vote, error) { + args := m.Called(f) + return args.Get(0).([]database.Vote), args.Error(1) +} + func TestNoteServiceTestSuite(t *testing.T) { suite.Run(t, new(NoteServiceTestSuite)) } @@ -61,10 +89,21 @@ func (suite *NoteServiceTestSuite) TestCreate() { mock := new(DBMock) s.database = mock + clientMock := &mockRtClient{} + rtMock := &realtime.Broker{ + Con: clientMock, + } + s.realtime = rtMock + authorID, _ := uuid.NewRandom() boardID, _ := uuid.NewRandom() colID, _ := uuid.NewRandom() txt := "aaaaaaaaaaaaaaaaaaaa" + publishSubject := "board." + boardID.String() + publishEvent := realtime.BoardEvent{ + Type: realtime.BoardEventNotesUpdated, + Data: []dto.Note{}, + } mock.On("CreateNote", database.NoteInsert{ Author: authorID, @@ -72,6 +111,9 @@ func (suite *NoteServiceTestSuite) TestCreate() { Column: colID, Text: txt, }).Return(database.Note{}, nil) + mock.On("GetNotes", boardID).Return([]database.Note{}, nil) + + clientMock.On("Publish", publishSubject, publishEvent).Return(nil) _, err := s.Create(context.Background(), dto.NoteCreateRequest{ User: authorID, @@ -79,9 +121,9 @@ func (suite *NoteServiceTestSuite) TestCreate() { Column: colID, Text: txt, }) - - assert.Nil(suite.T(), err) + assert.NoError(suite.T(), err) mock.AssertExpectations(suite.T()) + clientMock.AssertExpectations(suite.T()) } @@ -96,9 +138,9 @@ func (suite *NoteServiceTestSuite) TestGetNote() { ID: noteID, }, nil) - get, err := s.Get(context.Background(), noteID) - assert.NotNil(suite.T(), get) - assert.Nil(suite.T(), err) + _, err := s.Get(context.Background(), noteID) + + assert.NoError(suite.T(), err) mock.AssertExpectations(suite.T()) } @@ -111,9 +153,9 @@ func (suite *NoteServiceTestSuite) TestGetNotes() { mock.On("GetNotes", boardID).Return([]database.Note{}, nil) - get, err := s.List(context.Background(), boardID) - assert.NotNil(suite.T(), get) - assert.Nil(suite.T(), err) + _, err := s.List(context.Background(), boardID) + + assert.NoError(suite.T(), err) mock.AssertExpectations(suite.T()) } @@ -121,6 +163,11 @@ func (suite *NoteServiceTestSuite) TestUpdateNote() { s := new(NoteService) mock := new(DBMock) s.database = mock + clientMock := &mockRtClient{} + rtMock := &realtime.Broker{ + Con: clientMock, + } + s.realtime = rtMock callerID, _ := uuid.NewRandom() noteID, _ := uuid.NewRandom() @@ -138,9 +185,18 @@ func (suite *NoteServiceTestSuite) TestUpdateNote() { Rank: 0, Stack: stackID, } + // Mocks for realtime + publishSubject := "board." + boardID.String() + publishEvent := realtime.BoardEvent{ + Type: realtime.BoardEventNotesUpdated, + Data: []dto.Note{}, + } + clientMock.On("Publish", publishSubject, publishEvent).Return(nil) + // Mock for the updatedNotes call, which internally calls GetNotes + mock.On("GetNotes", boardID).Return([]database.Note{}, nil) + ctx := context.Background() ctx = context.WithValue(ctx, identifiers.UserIdentifier, callerID) - mock.On("UpdateNote", callerID, database.NoteUpdate{ ID: noteID, Board: boardID, @@ -148,15 +204,13 @@ func (suite *NoteServiceTestSuite) TestUpdateNote() { Position: &posUpdate, }).Return(database.Note{}, nil) - update, err := s.Update(ctx, dto.NoteUpdateRequest{ + _, err := s.Update(ctx, dto.NoteUpdateRequest{ Text: &txt, ID: noteID, Board: boardID, Position: &pos, }) - - assert.NotNil(suite.T(), update) - assert.Nil(suite.T(), err) + assert.NoError(suite.T(), err) mock.AssertExpectations(suite.T()) } @@ -165,6 +219,12 @@ func (suite *NoteServiceTestSuite) TestDeleteNote() { mock := new(DBMock) s.database = mock + clientMock := &mockRtClient{} + rtMock := &realtime.Broker{ + Con: clientMock, + } + s.realtime = rtMock + callerID, _ := uuid.NewRandom() boardID, _ := uuid.NewRandom() noteID, _ := uuid.NewRandom() @@ -172,16 +232,39 @@ func (suite *NoteServiceTestSuite) TestDeleteNote() { body := dto.NoteDeleteRequest{ DeleteStack: deleteStack, } + voteFilter := filter.VoteFilter{ + User: &callerID, + Board: boardID, + Note: ¬eID, + } + + // Mocks only for the realtime stuff + publishSubject := "board." + boardID.String() + deletedNoteRealTimeUpdate := map[string]interface{}{ + "note": noteID, + "deleteStack": deleteStack, + } + publishEventNoteDeleted := realtime.BoardEvent{ + Type: realtime.BoardEventNoteDeleted, + Data: deletedNoteRealTimeUpdate, + } + publishEventVotesUpdated := realtime.BoardEvent{ + Type: realtime.BoardEventVotesUpdated, + Data: []*dto.Vote{}, + } + clientMock.On("Publish", publishSubject, publishEventNoteDeleted).Return(nil) + clientMock.On("Publish", publishSubject, publishEventVotesUpdated).Return(nil) ctx := context.Background() ctx = context.WithValue(ctx, identifiers.UserIdentifier, callerID) ctx = context.WithValue(ctx, identifiers.BoardIdentifier, boardID) + ctx = context.WithValue(ctx, identifiers.NoteIdentifier, noteID) + mock.On("GetVotes", voteFilter).Return([]database.Vote{}, nil) mock.On("DeleteNote", callerID, boardID, noteID, deleteStack).Return(nil) err := s.Delete(ctx, body, noteID) - - assert.Nil(suite.T(), err) + assert.NoError(suite.T(), err) mock.AssertExpectations(suite.T()) }