diff --git a/app/controllers/live_streams_controller.rb b/app/controllers/live_streams_controller.rb index f9669812..47f5b313 100644 --- a/app/controllers/live_streams_controller.rb +++ b/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 diff --git a/app/models/programming_group.rb b/app/models/programming_group.rb index 9f5e7745..5643ca3c 100644 --- a/app/models/programming_group.rb +++ b/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 diff --git a/app/models/runner.rb b/app/models/runner.rb index ddb4ab46..293a6c18 100644 --- a/app/models/runner.rb +++ b/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 diff --git a/app/models/user.rb b/app/models/user.rb index 3b5b62a7..fd630484 100644 --- a/app/models/user.rb +++ b/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 diff --git a/db/migrate/20230820182149_rename_user_to_contributor_in_runners.rb b/db/migrate/20230820182149_rename_user_to_contributor_in_runners.rb new file mode 100644 index 00000000..ac003d48 --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 22cc3410..4df6d717 100644 --- a/db/schema.rb +++ b/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| diff --git a/spec/factories/runner.rb b/spec/factories/runner.rb index 3ce0941f..7db65b18 100644 --- a/spec/factories/runner.rb +++ b/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 diff --git a/spec/models/runner_spec.rb b/spec/models/runner_spec.rb index 646d246c..ae2bd1d2 100644 --- a/spec/models/runner_spec.rb +++ b/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