Merge branch 'master' into travis-green-docker

took all the newer versions. Only thing I deleted is the part #to pass the test "it executes the run command" ...
We need to shortly discuss this yqbk.

Conflicts:
	lib/docker_client.rb

app/controllers/exercises_controller.rb

@@ -118,7 +118,7 @@ class ExercisesController < ApplicationController
   private :user_by_code_harbor_token
 
   def exercise_params
-    params[:exercise].permit(:description, :execution_environment_id, :file_id, :instructions, :public, :hide_file_tree, :allow_file_creation, :title, files_attributes: file_attributes).merge(user_id: current_user.id, user_type: current_user.class.name)
+    params[:exercise].permit(:description, :execution_environment_id, :file_id, :instructions, :public, :hide_file_tree, :allow_file_creation, :allow_auto_completion, :title, files_attributes: file_attributes).merge(user_id: current_user.id, user_type: current_user.class.name)
   end
   private :exercise_params
 
@@ -247,23 +247,34 @@ class ExercisesController < ApplicationController
   end
 
   def redirect_after_submit
-    Rails.logger.debug('Score ' + @submission.normalized_score.to_s)
+    Rails.logger.debug('Redirecting user with score:s ' + @submission.normalized_score.to_s)
     if @submission.normalized_score == 1.0
-      # if user has an own rfc, redirect to it and message him to clean up and accept the answer.
+      # if user is external and has an own rfc, redirect to it and message him to clean up and accept the answer. (we need to check that the user is external,
+      # otherwise an internal user could be shown a false rfc here, since current_user.id is polymorphic, but only makes sense for external users when used with rfcs.)
+      if current_user.respond_to? :external_id
+        if rfc = RequestForComment.unsolved.where(exercise_id: @submission.exercise, user_id: current_user.id).first
+          # set a message that informs the user that his own RFC should be closed.
+          flash[:notice] = I18n.t('exercises.submit.full_score_redirect_to_own_rfc')
+          flash.keep(:notice)
 
-      # else: show open rfc for same exercise
-      if rfc = RequestForComment.unsolved.where(exercise_id: @submission.exercise).order("RANDOM()").first
+          respond_to do |format|
+            format.html { redirect_to(rfc) }
+            format.json { render(json: {redirect: url_for(rfc)}) }
+          end
+          return
 
-        # set a message that informs the user that his score was perfect and help in RFC is greatly appreciated.
-        flash[:notice] = I18n.t('exercises.submit.full_score_redirect_to_rfc')
-        flash.keep(:notice)
+        # else: show open rfc for same exercise if available
+        elsif rfc = RequestForComment.unsolved.where(exercise_id: @submission.exercise).order("RANDOM()").first
+          # set a message that informs the user that his score was perfect and help in RFC is greatly appreciated.
+          flash[:notice] = I18n.t('exercises.submit.full_score_redirect_to_rfc')
+          flash.keep(:notice)
 
-        respond_to do |format|
-          format.html { redirect_to(rfc) }
-          format.json { render(json: {redirect: url_for(rfc)}) }
+          respond_to do |format|
+            format.html { redirect_to(rfc) }
+            format.json { render(json: {redirect: url_for(rfc)}) }
+          end
+          return
         end
-
-        return
       end
     end
     redirect_to_lti_return_path

app/controllers/request_for_comments_controller.rb

@@ -82,6 +82,7 @@ class RequestForCommentsController < ApplicationController
 
     # Never trust parameters from the scary internet, only allow the white list through.
     def request_for_comment_params
+      # we are using the current_user.id here, since internal users are not able to create comments. The external_user.id is a primary key and does not require the consumer_id to be unique.
       params.require(:request_for_comment).permit(:exercise_id, :file_id, :question, :requested_at, :solved, :submission_id).merge(user_id: current_user.id, user_type: current_user.class.name)
     end
 end

app/controllers/submissions_controller.rb

@@ -129,7 +129,7 @@ class SubmissionsController < ApplicationController
 
         socket.on :message do |event|
           Rails.logger.info( Time.now.getutc.to_s + ": Docker sending: " + event.data)
-          handle_message(event.data, tubesock)
+          handle_message(event.data, tubesock, result[:container])
         end
 
         socket.on :close do |event|
@@ -139,12 +139,12 @@ class SubmissionsController < ApplicationController
         tubesock.onmessage do |data|
           Rails.logger.info(Time.now.getutc.to_s + ": Client sending: " + data)
           # Check whether the client send a JSON command and kill container
-          # if the command is 'exit', send it to docker otherwise.
+          # if the command is 'client_exit', send it to docker otherwise.
           begin
             parsed = JSON.parse(data)
-            if parsed['cmd'] == 'exit'
+            if parsed['cmd'] == 'client_kill'
               Rails.logger.debug("Client exited container.")
-              @docker_client.exit_container(result[:container])
+              @docker_client.kill_container(result[:container])
             else
               socket.send data
               Rails.logger.debug('Sent the received client data to docker:' + data)
@@ -171,10 +171,11 @@ class SubmissionsController < ApplicationController
     tubesock.close
   end
 
-  def handle_message(message, tubesock)
+  def handle_message(message, tubesock, container)
     # Handle special commands first
     if (/^exit/.match(message))
       kill_socket(tubesock)
+      @docker_client.exit_container(container)
     else
       # Filter out information about run_command, test_command, user or working directory
       run_command = @submission.execution_environment.run_command % command_substitutions(params[:filename])
@@ -231,7 +232,13 @@ class SubmissionsController < ApplicationController
     hijack do |tubesock|
       Thread.new { EventMachine.run } unless EventMachine.reactor_running? && EventMachine.reactor_thread.alive?
       # tubesock is the socket to the client
-      tubesock.send_data JSON.dump(score_submission(@submission))
+
+      # the score_submission call will end up calling docker exec, which is blocking.
+      # to ensure responsiveness, we therefore open a thread here.
+      Thread.new {
+        tubesock.send_data JSON.dump(score_submission(@submission))
+        tubesock.send_data JSON.dump({'cmd' => 'exit'})
+      }
     end
   end
 
@@ -291,6 +298,7 @@ class SubmissionsController < ApplicationController
 
       # tubesock is the socket to the client
       tubesock.send_data JSON.dump(output)
+      tubesock.send_data JSON.dump('cmd' => 'exit')
     end
   end