Fix Runner access for programming groups

* We also rename the corresponding columns in the Runner model, so that for polymorphic association gets clear.

app/controllers/live_streams_controller.rb

@@ -15,7 +15,7 @@ class LiveStreamsController < ApplicationController
     redirect_back(fallback_location: root_path, allow_other_host: true, alert: t('exercises.download_file_tree.gone'))
   else
     desired_file = params[:filename].to_s
-    runner = Runner.for(current_user, @submission.exercise.execution_environment)
+    runner = Runner.for(current_contributor, @submission.exercise.execution_environment)
     fallback_location = implement_exercise_path(@submission.exercise)
     send_runner_file(runner, desired_file, fallback_location)
   end

app/models/programming_group.rb

@@ -7,6 +7,7 @@ class ProgrammingGroup < ApplicationRecord
   has_many :external_users, through: :programming_group_memberships, source_type: 'ExternalUser', source: :user
   has_many :internal_users, through: :programming_group_memberships, source_type: 'InternalUser', source: :user
   has_many :testruns, through: :submissions
+  has_many :runners, as: :contributor, dependent: :destroy
   belongs_to :exercise
 
   validate :group_size

app/models/runner.rb

@@ -1,11 +1,10 @@
 # frozen_string_literal: true
 
 class Runner < ApplicationRecord
-  include Creation
+  include ContributorCreation
   belongs_to :execution_environment
 
   before_validation :request_id
-
   validates :runner_id, presence: true
 
   attr_accessor :strategy
@@ -30,10 +29,10 @@ class Runner < ApplicationRecord
     end
   end
 
-  def self.for(user, execution_environment)
-    runner = find_by(user:, execution_environment:)
+  def self.for(contributor, execution_environment)
+    runner = find_by(contributor:, execution_environment:)
     if runner.nil?
-      runner = Runner.create(user:, execution_environment:)
+      runner = Runner.create(contributor:, execution_environment:)
       # The `strategy` is added through the before_validation hook `:request_id`.
       raise Runner::Error::Unknown.new("Runner could not be saved: #{runner.errors.inspect}") unless runner.persisted?
     else
@@ -84,7 +83,7 @@ class Runner < ApplicationRecord
   end
 
   def attach_to_execution(command, privileged_execution: false, &block)
-    Rails.logger.debug { "#{Time.zone.now.getutc.inspect}: Starting execution with Runner #{id} for #{user_type} #{user_id}." }
+    Rails.logger.debug { "#{Time.zone.now.getutc.inspect}: Starting execution with Runner #{id} for #{contributor_type} #{contributor_id}." }
     starting_time = Time.zone.now
     begin
       # As the EventMachine reactor is probably shared with other threads, we cannot use EventMachine.run with
@@ -101,7 +100,7 @@ class Runner < ApplicationRecord
       e.execution_duration = Time.zone.now - starting_time
       raise
     end
-    Rails.logger.debug { "#{Time.zone.now.getutc.inspect}: Stopped execution with Runner #{id} for #{user_type} #{user_id}." }
+    Rails.logger.debug { "#{Time.zone.now.getutc.inspect}: Stopped execution with Runner #{id} for #{contributor_type} #{contributor_id}." }
     Time.zone.now - starting_time # execution duration
   end
 

app/models/user.rb

@@ -20,6 +20,7 @@ class User < ApplicationRecord
   has_many :testruns, as: :user
   has_many :interventions, through: :user_exercise_interventions
   has_many :remote_evaluation_mappings, as: :user
+  has_many :runners, as: :contributor
   has_one :codeharbor_link, dependent: :destroy
   accepts_nested_attributes_for :user_proxy_exercise_exercises
 

db/migrate/20230820182149_rename_user_to_contributor_in_runners.rb

@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+class RenameUserToContributorInRunners < ActiveRecord::Migration[7.0]
+  def change
+    change_table :runners do |t|
+      t.rename :user_id, :contributor_id
+      t.rename :user_type, :contributor_type
+    end
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2023_08_19_084917) do
+ActiveRecord::Schema[7.0].define(version: 2023_08_20_182149) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
   enable_extension "pgcrypto"
@@ -416,12 +416,12 @@ ActiveRecord::Schema[7.0].define(version: 2023_08_19_084917) do
   create_table "runners", force: :cascade do |t|
     t.string "runner_id"
     t.bigint "execution_environment_id"
-    t.string "user_type"
-    t.bigint "user_id"
+    t.string "contributor_type"
+    t.bigint "contributor_id"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
+    t.index ["contributor_type", "contributor_id"], name: "index_runners_on_user"
     t.index ["execution_environment_id"], name: "index_runners_on_execution_environment_id"
-    t.index ["user_type", "user_id"], name: "index_runners_on_user"
   end
 
   create_table "searches", id: :serial, force: :cascade do |t|

spec/factories/runner.rb

@@ -5,6 +5,6 @@ FactoryBot.define do
   factory :runner do
     runner_id { SecureRandom.uuid }
     execution_environment factory: :ruby
-    user factory: :external_user
+    contributor factory: :external_user
   end
 end

spec/models/runner_spec.rb

@@ -22,9 +22,9 @@ describe Runner do
       expect(runner.errors[:execution_environment]).to be_present
     end
 
-    it 'validates the presence of a user' do
-      runner.update(user: nil)
-      expect(runner.errors[:user]).to be_present
+    it 'validates the presence of a contributor' do
+      runner.update(contributor: nil)
+      expect(runner.errors[:contributor]).to be_present
     end
   end
 
@@ -159,9 +159,9 @@ describe Runner do
   end
 
   describe 'creation' do
-    let(:user) { create(:external_user) }
+    let(:contributor) { create(:external_user) }
     let(:execution_environment) { create(:ruby) }
-    let(:create_action) { -> { described_class.create(user:, execution_environment:) } }
+    let(:create_action) { -> { described_class.create(contributor:, execution_environment:) } }
 
     it 'requests a runner id from the runner management' do
       expect(strategy_class).to receive(:request_from_management)
@@ -184,7 +184,7 @@ describe Runner do
     it 'does not call the runner management again while a runner id is set' do
       expect(strategy_class).to receive(:request_from_management).and_return(runner_id).once
       runner = create_action.call
-      runner.update(user: create(:external_user))
+      runner.update(contributor: create(:external_user))
     end
   end
 
@@ -237,27 +237,27 @@ describe Runner do
   end
 
   describe '::for' do
-    let(:user) { create(:external_user) }
+    let(:contributor) { create(:external_user) }
     let(:exercise) { create(:fibonacci) }
 
     context 'when the runner could not be saved' do
       before { allow(strategy_class).to receive(:request_from_management).and_return(nil) }
 
       it 'raises an error' do
-        expect { described_class.for(user, exercise.execution_environment) }.to raise_error(Runner::Error::Unknown, /could not be saved/)
+        expect { described_class.for(contributor, exercise.execution_environment) }.to raise_error(Runner::Error::Unknown, /could not be saved/)
       end
     end
 
     context 'when a runner already exists' do
-      let!(:existing_runner) { create(:runner, user:, execution_environment: exercise.execution_environment) }
+      let!(:existing_runner) { create(:runner, contributor:, execution_environment: exercise.execution_environment) }
 
       it 'returns the existing runner' do
-        new_runner = described_class.for(user, exercise.execution_environment)
+        new_runner = described_class.for(contributor, exercise.execution_environment)
         expect(new_runner).to eq(existing_runner)
       end
 
       it 'sets the strategy' do
-        runner = described_class.for(user, exercise.execution_environment)
+        runner = described_class.for(contributor, exercise.execution_environment)
         expect(runner.strategy).to be_present
       end
     end
@@ -266,7 +266,7 @@ describe Runner do
       before { allow(strategy_class).to receive(:request_from_management).and_return(runner_id) }
 
       it 'returns a new runner' do
-        runner = described_class.for(user, exercise.execution_environment)
+        runner = described_class.for(contributor, exercise.execution_environment)
         expect(runner).to be_valid
       end
     end