diff --git a/CHANGELOG.md b/CHANGELOG.md index 95235658..68c00990 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - Fix setting cooldown on penalize check [\#84](https://github.com/NodeFactoryIo/vedran/pull/84) ([MakMuftic](https://github.com/MakMuftic)) - Restructure penalizing for bad metrics [\#86](https://github.com/NodeFactoryIo/vedran/pull/86) ([MakMuftic](https://github.com/MakMuftic)) - Fix error saving downtime [\#100](https://github.com/NodeFactoryIo/vedran/pull/100) ([mpetrun5](https://github.com/mpetrun5)) +- Fix missing is node active check on new metrics [\#102](https://github.com/NodeFactoryIo/vedran/pull/102) ([MakMuftic](https://github.com/MakMuftic)) ### Changed - Use port from tunnel map for calling node [\#65](https://github.com/NodeFactoryIo/vedran/pull/65) ([mpetrun5](https://github.com/mpetrun5)) diff --git a/internal/controllers/metrics.go b/internal/controllers/metrics.go index 916b66e1..81e41f54 100644 --- a/internal/controllers/metrics.go +++ b/internal/controllers/metrics.go @@ -57,9 +57,11 @@ func (c ApiController) SaveMetricsHandler(w http.ResponseWriter, r *http.Request metricsRequest.BestBlockHeight, ) - err = active.ActivateNodeIfReady(requestContext.NodeId, c.repositories) - if err != nil { - log.Errorf("Unable to activate node %s, error: %v", requestContext.NodeId, err) - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + if !c.repositories.NodeRepo.IsNodeActive(requestContext.NodeId) { + err = active.ActivateNodeIfReady(requestContext.NodeId, c.repositories) + if err != nil { + log.Errorf("Unable to activate node %s, error: %v", requestContext.NodeId, err) + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + } } } diff --git a/internal/controllers/metrics_test.go b/internal/controllers/metrics_test.go index b3ee3903..c6ad027b 100644 --- a/internal/controllers/metrics_test.go +++ b/internal/controllers/metrics_test.go @@ -32,6 +32,8 @@ func TestApiController_SaveMetricsHandler(t *testing.T) { // NodeRepo.AddNodeToActive nodeRepoAddNodeToActiveError error nodeRepoAddNodeToActiveNumOfCalls int + // NodeRepo.IsNodeActive + nodeRepoIsNodeActiveReturn bool // MetricsRepo.FindByID metricsRepoFindByIDError error metricsRepoFindByIDReturn *models.Metrics @@ -61,6 +63,8 @@ func TestApiController_SaveMetricsHandler(t *testing.T) { // NodeRepo.AddNodeToActive nodeRepoAddNodeToActiveError: nil, nodeRepoAddNodeToActiveNumOfCalls: 1, + // NodeRepo.IsNodeActive + nodeRepoIsNodeActiveReturn: false, // MetricsRepo.FindByID metricsRepoFindByIDReturn: &models.Metrics{ NodeId: "1", @@ -82,6 +86,37 @@ func TestApiController_SaveMetricsHandler(t *testing.T) { metricsRepoSaveError: nil, metricsRepoSaveNumOfCalls: 1, }, + { + name: "Valid metrics save request and node should not be added to active nodes as it already is in active", + metricsRequest: MetricsRequest{ + PeerCount: 0, + BestBlockHeight: 1000, + FinalizedBlockHeight: 995, + ReadyTransactionCount: 0, + }, + nodeId: "1", + httpStatus: http.StatusOK, + // NodeRepo.FindByID + nodeRepoIsNodeOnCooldownReturns: false, + nodeRepoIsNodeOnCooldownError: nil, + nodeRepoIsNodeOnNumOfCalls: 0, + // NodeRepo.AddNodeToActive + nodeRepoAddNodeToActiveError: nil, + nodeRepoAddNodeToActiveNumOfCalls: 0, + // NodeRepo.IsNodeActive + nodeRepoIsNodeActiveReturn: true, + // MetricsRepo.FindByID + metricsRepoFindByIDReturn: nil, + metricsRepoFindByIDError: nil, + metricsRepoFindByIDNumOfCalls: 0, + // MetricsRepo.GetLatestBlockMetrics + metricsRepoGetLatestBlockMetricsReturn: nil, + metricsRepoGetLatestBlockMetricsError: nil, + metricsRepoGetLatestBlockMetricsNumOfCalls: 0, + // MetricsRepo.Save + metricsRepoSaveError: nil, + metricsRepoSaveNumOfCalls: 1, + }, { name: "Valid metrics save request and node should not be added to active nodes as it is penalized", metricsRequest: MetricsRequest{ @@ -99,6 +134,8 @@ func TestApiController_SaveMetricsHandler(t *testing.T) { // NodeRepo.AddNodeToActive nodeRepoAddNodeToActiveError: nil, nodeRepoAddNodeToActiveNumOfCalls: 0, + // NodeRepo.IsNodeActive + nodeRepoIsNodeActiveReturn: false, // MetricsRepo.FindByID metricsRepoFindByIDReturn: nil, metricsRepoFindByIDError: nil, @@ -128,6 +165,8 @@ func TestApiController_SaveMetricsHandler(t *testing.T) { // NodeRepo.AddNodeToActive nodeRepoAddNodeToActiveError: nil, nodeRepoAddNodeToActiveNumOfCalls: 0, + // NodeRepo.IsNodeActive + nodeRepoIsNodeActiveReturn: false, // MetricsRepo.FindByID metricsRepoFindByIDReturn: &models.Metrics{ NodeId: "1", @@ -184,6 +223,9 @@ func TestApiController_SaveMetricsHandler(t *testing.T) { nodeRepoMock.On("AddNodeToActive", test.nodeId).Return( test.nodeRepoAddNodeToActiveError, ) + nodeRepoMock.On("IsNodeActive", test.nodeId).Return( + test.nodeRepoIsNodeActiveReturn, + ) metricsRepoMock := mocks.MetricsRepository{} metricsRepoMock.On("FindByID", test.nodeId).Return( diff --git a/internal/repositories/node.go b/internal/repositories/node.go index aba55c07..a637d396 100644 --- a/internal/repositories/node.go +++ b/internal/repositories/node.go @@ -19,6 +19,7 @@ type NodeRepository interface { GetAll() (*[]models.Node, error) GetActiveNodes(selection string) *[]models.Node GetAllActiveNodes() *[]models.Node + IsNodeActive(ID string) bool RemoveNodeFromActive(ID string) error AddNodeToActive(ID string) error RewardNode(node models.Node) @@ -184,3 +185,13 @@ func (r *nodeRepo) IsNodeOnCooldown(ID string) (bool, error) { return node.Cooldown != 0, err } + +func (r *nodeRepo) IsNodeActive(ID string) bool { + for _, node := range *r.GetAllActiveNodes() { + if node.ID == ID { + return true + } + } + return false +} + diff --git a/mocks/actions/Actions.go b/mocks/actions/Actions.go index 5a1129e4..c2ef8acf 100644 --- a/mocks/actions/Actions.go +++ b/mocks/actions/Actions.go @@ -1,12 +1,10 @@ -// Code generated by mockery v2.2.1. DO NOT EDIT. +// Code generated by mockery v1.0.0. DO NOT EDIT. package mocks -import ( - models "github.com/NodeFactoryIo/vedran/internal/models" - repositories "github.com/NodeFactoryIo/vedran/internal/repositories" - mock "github.com/stretchr/testify/mock" -) +import mock "github.com/stretchr/testify/mock" +import models "github.com/NodeFactoryIo/vedran/internal/models" +import repositories "github.com/NodeFactoryIo/vedran/internal/repositories" // Actions is an autogenerated mock type for the Actions type type Actions struct { diff --git a/mocks/repositories/DowntimeRepository.go b/mocks/repositories/DowntimeRepository.go index 9900b537..0ed6f201 100644 --- a/mocks/repositories/DowntimeRepository.go +++ b/mocks/repositories/DowntimeRepository.go @@ -1,11 +1,9 @@ -// Code generated by mockery v2.2.1. DO NOT EDIT. +// Code generated by mockery v1.0.0. DO NOT EDIT. package mocks -import ( - models "github.com/NodeFactoryIo/vedran/internal/models" - mock "github.com/stretchr/testify/mock" -) +import mock "github.com/stretchr/testify/mock" +import models "github.com/NodeFactoryIo/vedran/internal/models" // DowntimeRepository is an autogenerated mock type for the DowntimeRepository type type DowntimeRepository struct { diff --git a/mocks/repositories/MetricsRepository.go b/mocks/repositories/MetricsRepository.go index 4ac93f4c..21a114ff 100644 --- a/mocks/repositories/MetricsRepository.go +++ b/mocks/repositories/MetricsRepository.go @@ -1,11 +1,9 @@ -// Code generated by mockery v2.2.1. DO NOT EDIT. +// Code generated by mockery v1.0.0. DO NOT EDIT. package mocks -import ( - models "github.com/NodeFactoryIo/vedran/internal/models" - mock "github.com/stretchr/testify/mock" -) +import mock "github.com/stretchr/testify/mock" +import models "github.com/NodeFactoryIo/vedran/internal/models" // MetricsRepository is an autogenerated mock type for the MetricsRepository type type MetricsRepository struct { diff --git a/mocks/repositories/NodeRepository.go b/mocks/repositories/NodeRepository.go index 7b5b8021..f14af1d3 100644 --- a/mocks/repositories/NodeRepository.go +++ b/mocks/repositories/NodeRepository.go @@ -1,11 +1,9 @@ -// Code generated by mockery v2.2.1. DO NOT EDIT. +// Code generated by mockery v1.0.0. DO NOT EDIT. package mocks -import ( - models "github.com/NodeFactoryIo/vedran/internal/models" - mock "github.com/stretchr/testify/mock" -) +import mock "github.com/stretchr/testify/mock" +import models "github.com/NodeFactoryIo/vedran/internal/models" // NodeRepository is an autogenerated mock type for the NodeRepository type type NodeRepository struct { @@ -127,6 +125,20 @@ func (_m *NodeRepository) IncreaseNodeCooldown(ID string) (*models.Node, error) return r0, r1 } +// IsNodeActive provides a mock function with given fields: ID +func (_m *NodeRepository) IsNodeActive(ID string) bool { + ret := _m.Called(ID) + + var r0 bool + if rf, ok := ret.Get(0).(func(string) bool); ok { + r0 = rf(ID) + } else { + r0 = ret.Get(0).(bool) + } + + return r0 +} + // IsNodeOnCooldown provides a mock function with given fields: ID func (_m *NodeRepository) IsNodeOnCooldown(ID string) (bool, error) { ret := _m.Called(ID) diff --git a/mocks/repositories/PingRepository.go b/mocks/repositories/PingRepository.go index 45fab75e..686e46b9 100644 --- a/mocks/repositories/PingRepository.go +++ b/mocks/repositories/PingRepository.go @@ -1,13 +1,11 @@ -// Code generated by mockery v2.2.1. DO NOT EDIT. +// Code generated by mockery v1.0.0. DO NOT EDIT. package mocks -import ( - models "github.com/NodeFactoryIo/vedran/internal/models" - mock "github.com/stretchr/testify/mock" +import mock "github.com/stretchr/testify/mock" +import models "github.com/NodeFactoryIo/vedran/internal/models" - time "time" -) +import time "time" // PingRepository is an autogenerated mock type for the PingRepository type type PingRepository struct { diff --git a/mocks/repositories/RecordRepository.go b/mocks/repositories/RecordRepository.go index 6506ecad..13e127a6 100644 --- a/mocks/repositories/RecordRepository.go +++ b/mocks/repositories/RecordRepository.go @@ -1,11 +1,9 @@ -// Code generated by mockery v2.2.1. DO NOT EDIT. +// Code generated by mockery v1.0.0. DO NOT EDIT. package mocks -import ( - models "github.com/NodeFactoryIo/vedran/internal/models" - mock "github.com/stretchr/testify/mock" -) +import mock "github.com/stretchr/testify/mock" +import models "github.com/NodeFactoryIo/vedran/internal/models" // RecordRepository is an autogenerated mock type for the RecordRepository type type RecordRepository struct {