Fix missing is node active check on new metrics

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))

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)
+		}
 	}
 }

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(

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
+}
+