From 795c83f7b29c6d1056fde391e2a0bf6a595a465f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maximilian=20Pa=C3=9F?=
 <22845248+mpass99@users.noreply.github.com>
Date: Sun, 29 May 2022 15:47:04 +0200
Subject: [PATCH] Fix deleting non existent environments

that is an error caused by throwing a panic when an environment is not found and a nonexistent runner manager at the end of the chain is asked for it.
---
 cmd/poseidon/main.go                         |  2 +-
 internal/environment/abstract_manager.go     | 22 +++++++-----
 internal/environment/manager.go              |  1 +
 internal/environment/manager_handler_mock.go | 37 +++++++++++++++++---
 internal/runner/abstract_manager.go          |  2 +-
 5 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/cmd/poseidon/main.go b/cmd/poseidon/main.go
index b8cc282..0093dc2 100644
--- a/cmd/poseidon/main.go
+++ b/cmd/poseidon/main.go
@@ -113,7 +113,7 @@ func createAWSManager() (runnerManager runner.Manager, environmentManager enviro
 // initServer builds the http server and configures it with the chain of responsibility for multiple managers.
 func initServer(influxClient influxdb2API.WriteAPI) *http.Server {
 	runnerManager, environmentManager := createManagerHandler(createNomadManager, config.Config.Nomad.Enabled,
-		runner.NewAbstractManager(), &environment.AbstractManager{})
+		nil, nil)
 	runnerManager, environmentManager = createManagerHandler(createAWSManager, config.Config.AWS.Enabled,
 		runnerManager, environmentManager)
 
diff --git a/internal/environment/abstract_manager.go b/internal/environment/abstract_manager.go
index fb85998..7be3f8e 100644
--- a/internal/environment/abstract_manager.go
+++ b/internal/environment/abstract_manager.go
@@ -18,13 +18,17 @@ func (n *AbstractManager) SetNextHandler(next ManagerHandler) {
 }
 
 func (n *AbstractManager) NextHandler() ManagerHandler {
-	if n.nextHandler != nil {
+	if n.HasNextHandler() {
 		return n.nextHandler
 	} else {
 		return &AbstractManager{}
 	}
 }
 
+func (n *AbstractManager) HasNextHandler() bool {
+	return n.nextHandler != nil
+}
+
 func (n *AbstractManager) List(_ bool) ([]runner.ExecutionEnvironment, error) {
 	return []runner.ExecutionEnvironment{}, nil
 }
@@ -38,17 +42,17 @@ func (n *AbstractManager) CreateOrUpdate(_ dto.EnvironmentID, _ dto.ExecutionEnv
 }
 
 func (n *AbstractManager) Delete(id dto.EnvironmentID) (bool, error) {
+	if n.runnerManager == nil {
+		return false, nil
+	}
+
 	e, ok := n.runnerManager.GetEnvironment(id)
 	if !ok {
-		if n.nextHandler != nil {
-			isFound, err := n.NextHandler().Delete(id)
-			if err != nil {
-				return false, fmt.Errorf("aws wrapped: %w", err)
-			}
-			return isFound, nil
-		} else {
-			return false, nil
+		isFound, err := n.NextHandler().Delete(id)
+		if err != nil {
+			return false, fmt.Errorf("abstract wrapped: %w", err)
 		}
+		return isFound, nil
 	}
 
 	n.runnerManager.DeleteEnvironment(id)
diff --git a/internal/environment/manager.go b/internal/environment/manager.go
index 1ee9de2..4f92681 100644
--- a/internal/environment/manager.go
+++ b/internal/environment/manager.go
@@ -11,6 +11,7 @@ type ManagerHandler interface {
 	Manager
 	SetNextHandler(next ManagerHandler)
 	NextHandler() ManagerHandler
+	HasNextHandler() bool
 }
 
 // Manager encapsulates API calls to the executor API for creation and deletion of execution environments.
diff --git a/internal/environment/manager_handler_mock.go b/internal/environment/manager_handler_mock.go
index d81d4e0..36b7367 100644
--- a/internal/environment/manager_handler_mock.go
+++ b/internal/environment/manager_handler_mock.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.9.4. DO NOT EDIT.
+// Code generated by mockery v2.12.3. DO NOT EDIT.
 
 package environment
 
@@ -79,6 +79,20 @@ func (_m *ManagerHandlerMock) Get(id dto.EnvironmentID, fetch bool) (runner.Exec
 	return r0, r1
 }
 
+// HasNextHandler provides a mock function with given fields:
+func (_m *ManagerHandlerMock) HasNextHandler() bool {
+	ret := _m.Called()
+
+	var r0 bool
+	if rf, ok := ret.Get(0).(func() bool); ok {
+		r0 = rf()
+	} else {
+		r0 = ret.Get(0).(bool)
+	}
+
+	return r0
+}
+
 // List provides a mock function with given fields: fetch
 func (_m *ManagerHandlerMock) List(fetch bool) ([]runner.ExecutionEnvironment, error) {
 	ret := _m.Called(fetch)
@@ -118,9 +132,9 @@ func (_m *ManagerHandlerMock) NextHandler() ManagerHandler {
 	return r0
 }
 
-// SetNextHandler provides a mock function with given fields: m
-func (_m *ManagerHandlerMock) SetNextHandler(m ManagerHandler) {
-	_m.Called(m)
+// SetNextHandler provides a mock function with given fields: next
+func (_m *ManagerHandlerMock) SetNextHandler(next ManagerHandler) {
+	_m.Called(next)
 }
 
 // Statistics provides a mock function with given fields:
@@ -138,3 +152,18 @@ func (_m *ManagerHandlerMock) Statistics() map[dto.EnvironmentID]*dto.Statistica
 
 	return r0
 }
+
+type NewManagerHandlerMockT interface {
+	mock.TestingT
+	Cleanup(func())
+}
+
+// NewManagerHandlerMock creates a new instance of ManagerHandlerMock. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
+func NewManagerHandlerMock(t NewManagerHandlerMockT) *ManagerHandlerMock {
+	mock := &ManagerHandlerMock{}
+	mock.Mock.Test(t)
+
+	t.Cleanup(func() { mock.AssertExpectations(t) })
+
+	return mock
+}
diff --git a/internal/runner/abstract_manager.go b/internal/runner/abstract_manager.go
index 8e9c9cd..644be40 100644
--- a/internal/runner/abstract_manager.go
+++ b/internal/runner/abstract_manager.go
@@ -30,7 +30,7 @@ func (n *AbstractManager) SetNextHandler(next AccessorHandler) {
 }
 
 func (n *AbstractManager) NextHandler() AccessorHandler {
-	if n.nextHandler != nil {
+	if n.HasNextHandler() {
 		return n.nextHandler
 	} else {
 		return NewAbstractManager()