Change exposed_ports to array

app/controllers/execution_environments_controller.rb

@@ -107,8 +107,15 @@ class ExecutionEnvironmentsController < ApplicationController
 
   def execution_environment_params
     if params[:execution_environment].present?
-      params[:execution_environment].permit(:docker_image, :exposed_ports, :editor_mode, :file_extension, :file_type_id, :help, :indent_size, :memory_limit, :cpu_limit, :name, :network_enabled, :permitted_execution_time, :pool_size, :run_command, :test_command, :testing_framework).merge(
+      exposed_ports = if params[:execution_environment][:exposed_ports_list].present?
-        user_id: current_user.id, user_type: current_user.class.name
+                        # Transform the `exposed_ports_list` to `exposed_ports` array
+                        params[:execution_environment].delete(:exposed_ports_list).scan(/\d+/)
+                      else
+                        []
+                      end
+
+      params[:execution_environment].permit(:docker_image, :editor_mode, :file_extension, :file_type_id, :help, :indent_size, :memory_limit, :cpu_limit, :name, :network_enabled, :permitted_execution_time, :pool_size, :run_command, :test_command, :testing_framework).merge(
+        user_id: current_user.id, user_type: current_user.class.name, exposed_ports: exposed_ports
       )
     end
   end

app/models/execution_environment.rb

@@ -28,7 +28,8 @@ class ExecutionEnvironment < ApplicationRecord
   validates :pool_size, numericality: {only_integer: true}, presence: true
   validates :run_command, presence: true
   validates :cpu_limit, presence: true, numericality: {greater_than: 0, only_integer: true}
-  validates :exposed_ports, format: {with: /\A(([[:digit:]]{1,5},)*([[:digit:]]{1,5}))?\z/}
+  before_validation :clean_exposed_ports
+  validates :exposed_ports, array: {numericality: {greater_than_or_equal_to: 0, less_than: 65_536, only_integer: true}}
 
   def set_default_values
     set_default_values_if_present(permitted_execution_time: 60, pool_size: 0)
@@ -47,14 +48,19 @@ class ExecutionEnvironment < ApplicationRecord
       cpuLimit: cpu_limit,
       memoryLimit: memory_limit,
       networkAccess: network_enabled,
-      exposedPorts: exposed_ports_list,
+      exposedPorts: exposed_ports,
     }.to_json
   end
 
   def exposed_ports_list
-    (exposed_ports || '').split(',').map(&:to_i)
+    exposed_ports.join(', ')
   end
 
+  def clean_exposed_ports
+    self.exposed_ports = exposed_ports.uniq.sort
+  end
+  private :clean_exposed_ports
+
   def valid_test_setup?
     if test_command? ^ testing_framework?
       errors.add(:test_command,

app/views/execution_environments/_form.html.slim

@@ -14,9 +14,9 @@
     = f.text_field(:docker_image, class: 'alternative-input form-control', disabled: true)
     .help-block.form-text == t('.hints.docker_image')
   .form-group
-    = f.label(:exposed_ports)
+    = f.label(:exposed_ports_list)
-    = f.text_field(:exposed_ports, class: 'form-control', placeholder: '3000,4000', pattern: '^((\d{1,5},)*(\d{1,5}))?$')
+    = f.text_field(:exposed_ports_list, class: 'form-control', placeholder: '3000, 4000', pattern: '^(\s*(\d{1,5},\s*)*(\d{1,5}\s*))?$')
-    .help-block.form-text == t('.hints.exposed_ports')
+    .help-block.form-text = t('.hints.exposed_ports_list')
   .form-group
     = f.label(:memory_limit)
     = f.number_field(:memory_limit, class: 'form-control', min: DockerClient::MINIMUM_MEMORY_LIMIT, value: f.object.memory_limit || DockerClient::DEFAULT_MEMORY_LIMIT)

app/views/execution_environments/show.html.slim

@@ -5,7 +5,7 @@ h1
 = row(label: 'execution_environment.name', value: @execution_environment.name)
 = row(label: 'execution_environment.user', value: link_to_if(policy(@execution_environment.author).show?, @execution_environment.author, @execution_environment.author))
 = row(label: 'execution_environment.file_type', value: @execution_environment.file_type.present? ? link_to(@execution_environment.file_type, @execution_environment.file_type) : nil)
-- [:docker_image, :exposed_ports, :memory_limit, :cpu_limit, :network_enabled, :permitted_execution_time, :pool_size].each do |attribute|
+- [:docker_image, :exposed_ports_list, :memory_limit, :cpu_limit, :network_enabled, :permitted_execution_time, :pool_size].each do |attribute|
   = row(label: "execution_environment.#{attribute}", value: @execution_environment.send(attribute))
 - [:run_command, :test_command].each do |attribute|
   = row(label: "execution_environment.#{attribute}") do

config/locales/de.yml

@@ -10,6 +10,7 @@ de:
       execution_environment:
         docker_image: Docker-Image
         exposed_ports: Zugängliche Ports
+        exposed_ports_list: Zugängliche Ports
         file_type: Standard-Dateityp
         file_type_id: Standard-Dateityp
         help: Hilfetext
@@ -282,7 +283,7 @@ de:
       hints:
         command: <em>filename</em> wird automatisch durch den richtigen Dateinamen ersetzt.
         docker_image: 'Wählen Sie ein Docker-Image aus der Liste oder fügen Sie ein neues hinzu, welches über <a href="https://hub.docker.com/" target="_blank">DockerHub</a> verfügbar ist.'
-        exposed_ports: Während der Ausführung sind diese Ports für den Nutzer zugänglich. Die Portnummern müssen mit Komma, aber ohne Leerzeichen voneinander getrennt sein.
+        exposed_ports_list: Während der Ausführung sind diese Ports für den Nutzer zugänglich. Die Portnummern müssen nummerisch und mit Komma voneinander getrennt sein.
       errors:
         not_synced_to_runner_management: Die Ausführungsumgebung wurde erstellt, aber aufgrund eines Fehlers nicht zum Runnermanagement synchronisiert.
     index:

config/locales/en.yml

@@ -10,6 +10,7 @@ en:
       execution_environment:
         docker_image: Docker Image
         exposed_ports: Exposed Ports
+        exposed_ports_list: Exposed Ports
         file_type: Default File Type
         file_type_id: Default File Type
         help: Help Text
@@ -282,7 +283,7 @@ en:
       hints:
         command: <em>filename</em> is automatically replaced with the correct filename.
         docker_image: Pick a Docker image listed above or add a new one which is available via <a href="https://hub.docker.com/" target="_blank">DockerHub</a>.
-        exposed_ports: During code execution these ports are accessible for the user. Port numbers must be separated by a comma but no space.
+        exposed_ports_list: During code execution these ports are accessible for the user. Port numbers must be numeric and separated by a comma.
       errors:
         not_synced_to_runner_management: The execution environment was created but not synced to the runner management due to an error.
     index:

db/migrate/20210602071834_change_type_of_exposed_ports_in_execution_environment.rb

@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+class ChangeTypeOfExposedPortsInExecutionEnvironment < ActiveRecord::Migration[6.1]
+  # rubocop:disable Rails/SkipsModelValidations:
+  def up
+    rename_column :execution_environments, :exposed_ports, :exposed_ports_migration
+    add_column :execution_environments, :exposed_ports, :integer, array: true, default: [], nil: true
+
+    ExecutionEnvironment.all.each do |execution_environment|
+      next if execution_environment.exposed_ports_migration.nil?
+
+      cleaned = execution_environment.exposed_ports_migration.scan(/\d+/)
+      list = cleaned.map(&:to_i).uniq.sort
+      execution_environment.update_columns(exposed_ports: list)
+    end
+
+    remove_column :execution_environments, :exposed_ports_migration
+  end
+
+  def down
+    rename_column :execution_environments, :exposed_ports, :exposed_ports_migration
+    add_column :execution_environments, :exposed_ports, :string
+
+    ExecutionEnvironment.all.each do |execution_environment|
+      next if execution_environment.exposed_ports_migration.empty?
+
+      list = execution_environment.exposed_ports_migration
+      if list.empty?
+        execution_environment.update_columns(exposed_ports: nil)
+      else
+        execution_environment.update_columns(exposed_ports: list.join(','))
+      end
+    end
+    remove_column :execution_environments, :exposed_ports_migration
+  end
+  # rubocop:enable Rails/SkipsModelValidations:
+end

db/migrate/20210602071834_clean_exposed_ports_in_execution_environment.rb

@@ -1,17 +0,0 @@
-# frozen_string_literal: true
-
-class CleanExposedPortsInExecutionEnvironment < ActiveRecord::Migration[6.1]
-  def change
-    ExecutionEnvironment.all.each do |execution_environment|
-      next if execution_environment.exposed_ports.nil?
-
-      cleaned = execution_environment.exposed_ports.gsub(/[[:space:]]/, '')
-      list = cleaned.split(',').map(&:to_i).uniq
-      if list.empty?
-        execution_environment.update(exposed_ports: nil)
-      else
-        execution_environment.update(exposed_ports: list.join(','))
-      end
-    end
-  end
-end

db/schema.rb

@@ -104,7 +104,6 @@ ActiveRecord::Schema.define(version: 2021_06_02_071834) do
     t.string "test_command", limit: 255
     t.string "testing_framework", limit: 255
     t.text "help"
-    t.string "exposed_ports", limit: 255
     t.integer "permitted_execution_time"
     t.integer "user_id"
     t.string "user_type", limit: 255
@@ -113,6 +112,7 @@ ActiveRecord::Schema.define(version: 2021_06_02_071834) do
     t.integer "memory_limit"
     t.boolean "network_enabled"
     t.integer "cpu_limit", default: 20, null: false
+    t.integer "exposed_ports", default: [], array: true
   end
 
   create_table "exercise_collection_items", id: :serial, force: :cascade do |t|

lib/docker_client.rb

@@ -448,7 +448,7 @@ container_execution_time: nil}
   end
 
   def self.mapped_ports(execution_environment)
-    (execution_environment.exposed_ports || '').gsub(/\s/, '').split(',').map do |port|
+    execution_environment.exposed_ports.map do |port|
       ["#{port}/tcp", [{'HostPort' => PortPool.available_port.to_s}]]
     end.to_h
   end

spec/factories/execution_environment.rb

@@ -122,7 +122,7 @@ FactoryBot.define do
     default_cpu_limit
     docker_image { 'hklement/ubuntu-sinatra:latest' }
     file_type { association :dot_rb, user: user }
-    exposed_ports { '4567' }
+    exposed_ports { [4567] }
     help
     name { 'Sinatra' }
     network_enabled { true }

spec/lib/docker_client_spec.rb

@@ -332,7 +332,7 @@ describe DockerClient, docker: true do
 
   describe '.mapped_ports' do
     context 'with exposed ports' do
-      before { execution_environment.exposed_ports = '3000' }
+      before { execution_environment.exposed_ports = [3000] }
 
       it 'returns a mapping' do
         expect(described_class.mapped_ports(execution_environment)).to be_a(Hash)

spec/models/execution_environment_spec.rb

@@ -178,17 +178,17 @@ describe ExecutionEnvironment do
   end
 
   describe '#exposed_ports_list' do
-    it 'returns an empty array if no ports are exposed' do
+    it 'returns an empty string if no ports are exposed' do
-      execution_environment.exposed_ports = nil
+      execution_environment.exposed_ports = []
-      expect(execution_environment.exposed_ports_list).to eq([])
+      expect(execution_environment.exposed_ports_list).to eq('')
     end
 
-    it 'returns an array of integers representing the exposed ports' do
+    it 'returns an string with comma-separated integers representing the exposed ports' do
-      execution_environment.exposed_ports = '1,2,3'
+      execution_environment.exposed_ports = [1, 2, 3]
-      expect(execution_environment.exposed_ports_list).to eq([1, 2, 3])
+      expect(execution_environment.exposed_ports_list).to eq('1, 2, 3')
 
-      execution_environment.exposed_ports_list.each do |port|
+      execution_environment.exposed_ports.each do |port|
-        expect(execution_environment.exposed_ports).to include(port.to_s)
+        expect(execution_environment.exposed_ports_list).to include(port.to_s)
       end
     end
   end