Move execution request map to runners

api/runners.go

@@ -1,9 +1,7 @@
 package api
 
 import (
-	"errors"
 	"fmt"
-	"github.com/google/uuid"
 	"github.com/gorilla/mux"
 	"gitlab.hpi.de/codeocean/codemoon/poseidon/api/dto"
 	"gitlab.hpi.de/codeocean/codemoon/poseidon/config"
@@ -11,20 +9,8 @@ import (
 	"gitlab.hpi.de/codeocean/codemoon/poseidon/environment/pool"
 	"gitlab.hpi.de/codeocean/codemoon/poseidon/runner"
 	"net/http"
-	"sync"
 )
 
-var (
-	executions     = make(map[string]map[string]dto.ExecutionRequest)
-	executionsLock = sync.Mutex{}
-)
-
-func allocateExecutionMap(runner runner.Runner) {
-	executionsLock.Lock()
-	executions[runner.Id()] = make(map[string]dto.ExecutionRequest)
-	executionsLock.Unlock()
-}
-
 // provideRunner tries to respond with the id of a runner
 // This runner is then reserved for future use
 func provideRunner(writer http.ResponseWriter, request *http.Request) {
@@ -42,7 +28,6 @@ func provideRunner(writer http.ResponseWriter, request *http.Request) {
 		writeInternalServerError(writer, err, dto.ErrorNomadOverload)
 		return
 	}
-	allocateExecutionMap(runner)
 	sendJson(writer, &dto.RunnerResponse{Id: runner.Id()}, http.StatusOK)
 }
 
@@ -55,33 +40,23 @@ func executeCommand(router *mux.Router) func(w http.ResponseWriter, r *http.Requ
 			return
 		}
 
-        var scheme string
+		var scheme string
-	    if config.Config.Server.TLS {
+		if config.Config.Server.TLS {
-	    	scheme = "wss"
+			scheme = "wss"
-	    } else {
+		} else {
-	    	scheme = "ws"
+			scheme = "ws"
-	    }
+		}
-	    r, ok := runner.FromContext(request.Context())
+		r, ok := runner.FromContext(request.Context())
-	    if !ok {
+		if !ok {
-	    	log.Fatal("Expected runner in context! Something must be broken ...")
+			log.Fatal("Expected runner in context! Something must be broken ...")
-	    }
+		}
 
-		id, err := uuid.NewRandom()
+		id, err := r.AddExecution(*executionRequest)
 		if err != nil {
-			log.Printf("Error creating new execution id: %v", err)
 			writeInternalServerError(writer, err, dto.ErrorUnknown)
 			return
 		}
 
-		executionsLock.Lock()
-		runnerExecutions, ok := executions[r.Id()]
-		if !ok {
-			writeNotFound(writer, errors.New("runner has not been provided"))
-			return
-		}
-		runnerExecutions[id.String()] = *executionRequest
-		executionsLock.Unlock()
-
 		path, err := router.Get("runner-websocket").URL("runnerId", r.Id())
 		if err != nil {
 			log.Printf("Error creating runner websocket URL %v", err)

api/runners_test.go

@@ -75,7 +75,6 @@ func TestExecuteRoute(t *testing.T) {
 
 	testRunner := runner.NewExerciseRunner("testRunner")
 	runnerPool.AddRunner(testRunner)
-	allocateExecutionMap(testRunner)
 
 	path, err := router.Get("runner-execute").URL("runnerId", testRunner.Id())
 	if err != nil {
@@ -120,8 +119,10 @@ func TestExecuteRoute(t *testing.T) {
 				t.Fatal(err)
 			}
 			executionId := url.Query().Get("executionId")
+			storedExecutionRequest, ok := testRunner.Execution(runner.ExecutionId(executionId))
 
-			assert.Equal(t, executionRequest, executions[testRunner.Id()][executionId])
+			assert.True(t, ok, "No execution request with this id")
+			assert.Equal(t, executionRequest, storedExecutionRequest)
 		})
 	})
 

runner/runner.go

@@ -3,12 +3,20 @@ package runner
 import (
 	"context"
 	"encoding/json"
+	"github.com/google/uuid"
+	"gitlab.hpi.de/codeocean/codemoon/poseidon/api/dto"
 	"sync"
 )
 
+// Status is the type for the status of a Runner.
 type Status string
+
+// ContextKey is the type for keys in a request context.
 type ContextKey string
 
+// ExecutionId is an id for a execution in a Runner.
+type ExecutionId string
+
 const (
 	StatusReady    Status = "ready"
 	StatusRunning  Status = "running"
@@ -20,32 +28,48 @@ const (
 )
 
 type Runner interface {
+	// SetStatus sets the status of the runner.
 	SetStatus(Status)
+
+	// Status gets the status of the runner.
 	Status() Status
+
+	// Id returns the id of the runner.
 	Id() string
+
+	// Execution looks up an ExecutionId for the runner and returns the associated RunnerRequest.
+	// If this request does not exits, ok is false, else true.
+	Execution(ExecutionId) (request dto.ExecutionRequest, ok bool)
+
+	// AddExecution saves the supplied ExecutionRequest for the runner and returns an ExecutionId to retrieve it again.
+	AddExecution(dto.ExecutionRequest) (ExecutionId, error)
+
+	// DeleteExecution deletes the execution of the runner with the specified id.
+	DeleteExecution(ExecutionId)
 }
 
-// ExerciseRunner is an abstraction to communicate with Nomad allocations
+// ExerciseRunner is an abstraction to communicate with Nomad allocations.
 type ExerciseRunner struct {
 	sync.Mutex
-	id     string
+	id         string
-	status Status
+	status     Status
-	ch     chan bool
+	ch         chan bool
+	executions map[ExecutionId]dto.ExecutionRequest
 }
 
-// NewExerciseRunner creates a new exercise runner with the provided id
+// NewExerciseRunner creates a new exercise runner with the provided id.
-// As default value the status is ready
 func NewExerciseRunner(id string) *ExerciseRunner {
 	return &ExerciseRunner{
-		id:     id,
+		id:         id,
-		status: StatusReady,
+		status:     StatusReady,
-		Mutex:  sync.Mutex{},
+		Mutex:      sync.Mutex{},
-		ch:     make(chan bool),
+		ch:         make(chan bool),
+		executions: make(map[ExecutionId]dto.ExecutionRequest),
 	}
 }
 
-// MarshalJSON implements json.Marshaler interface
+// MarshalJSON implements json.Marshaler interface.
-// This exports also private attributes like the id
+// This exports private attributes like the id too.
 func (r *ExerciseRunner) MarshalJSON() ([]byte, error) {
 	return json.Marshal(struct {
 		Id     string `json:"runnerId"`
@@ -56,29 +80,53 @@ func (r *ExerciseRunner) MarshalJSON() ([]byte, error) {
 	})
 }
 
-// SetStatus sets the status thread-safe
 func (r *ExerciseRunner) SetStatus(status Status) {
 	r.Lock()
 	defer r.Unlock()
 	r.status = status
 }
 
-// Status returns the status thread-safe
 func (r *ExerciseRunner) Status() Status {
 	r.Lock()
 	defer r.Unlock()
 	return r.status
 }
 
-// Id returns the id of the runner
 func (r *ExerciseRunner) Id() string {
 	return r.id
 }
 
+func (r *ExerciseRunner) Execution(id ExecutionId) (executionRequest dto.ExecutionRequest, ok bool) {
+	r.Lock()
+	defer r.Unlock()
+	executionRequest, ok = r.executions[id]
+	return
+}
+
+func (r *ExerciseRunner) AddExecution(request dto.ExecutionRequest) (ExecutionId, error) {
+	r.Lock()
+	defer r.Unlock()
+	idUuid, err := uuid.NewRandom()
+	if err != nil {
+		return ExecutionId(""), err
+	}
+	id := ExecutionId(idUuid.String())
+	r.executions[id] = request
+	return id, err
+}
+
+func (r *ExerciseRunner) DeleteExecution(id ExecutionId) {
+	r.Lock()
+	defer r.Unlock()
+	delete(r.executions, id)
+}
+
+// NewContext creates a context containing a runner.
 func NewContext(ctx context.Context, runner Runner) context.Context {
 	return context.WithValue(ctx, runnerContextKey, runner)
 }
 
+// FromContext returns a runner from a context.
 func FromContext(ctx context.Context) (Runner, bool) {
 	runner, ok := ctx.Value(runnerContextKey).(Runner)
 	return runner, ok

runner/runner_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"github.com/stretchr/testify/assert"
+	"gitlab.hpi.de/codeocean/codemoon/poseidon/api/dto"
 	"testing"
 )
 
@@ -32,6 +33,21 @@ func TestMarshalRunner(t *testing.T) {
 	assert.Equal(t, "{\"runnerId\":\"42\",\"status\":\"ready\"}", string(marshal))
 }
 
+func TestExecutionRequestIsStored(t *testing.T) {
+	runner := NewExerciseRunner("42")
+	executionRequest := dto.ExecutionRequest{
+		Command:     "command",
+		TimeLimit:   10,
+		Environment: nil,
+	}
+	id, err := runner.AddExecution(executionRequest)
+	storedExecutionRunner, ok := runner.Execution(id)
+
+	assert.NoError(t, err, "AddExecution should not produce an error")
+	assert.True(t, ok, "Getting an execution should not return ok false")
+	assert.Equal(t, executionRequest, storedExecutionRunner)
+}
+
 func TestNewContextReturnsNewContextWithRunner(t *testing.T) {
 	runner := NewExerciseRunner("testRunner")
 	ctx := context.Background()