From 9347d76a69557f1823b641dc46220349383bc76c Mon Sep 17 00:00:00 2001
From: Karol <git@koehn.pro>
Date: Mon, 16 Sep 2024 21:16:57 +0200
Subject: [PATCH 1/3] basic functionality of reusing files based on xml_id. Use
 xml_id_path to recreate proforma structure with tests and model_solutions
 containing multiple files

---
 .../convert_exercise_to_task.rb               | 93 ++++++++++++-------
 .../convert_task_to_exercise.rb               | 39 +++++---
 app/services/proforma_service/import.rb       |  8 +-
 .../20240903204319_add_xml_path_to_files.rb   |  7 ++
 db/schema.rb                                  |  1 +
 .../convert_exercise_to_task_spec.rb          | 79 ++++++++++++++--
 .../convert_task_to_exercise_spec.rb          | 40 ++++++--
 spec/services/proforma_service/import_spec.rb |  4 +-
 8 files changed, 197 insertions(+), 74 deletions(-)
 create mode 100644 db/migrate/20240903204319_add_xml_path_to_files.rb

diff --git a/app/services/proforma_service/convert_exercise_to_task.rb b/app/services/proforma_service/convert_exercise_to_task.rb
index 3eba296f6..db92ec6af 100644
--- a/app/services/proforma_service/convert_exercise_to_task.rb
+++ b/app/services/proforma_service/convert_exercise_to_task.rb
@@ -77,62 +77,84 @@ def uuid
     end
 
     def model_solutions
-      @exercise.files.filter {|file| file.role == 'reference_implementation' }.map do |file|
+      model_solutions_files = @exercise.files.filter {|file| file.role == 'reference_implementation' }.group_by {|file| xml_id_from_file(file).first }
+      model_solutions_files.map do |xml_id, files|
         ProformaXML::ModelSolution.new(
-          id: "ms-#{file.id}",
-          files: model_solution_file(file)
+          id: xml_id,
+          files: files.map {|file| model_solution_file(file) }
         )
       end
     end
 
     def model_solution_file(file)
-      [
-        task_file(file).tap do |ms_file|
-          ms_file.used_by_grader = false
-          ms_file.usage_by_lms = 'display'
-        end,
-      ]
+      task_file(file).tap do |ms_file|
+        ms_file.used_by_grader = false
+        ms_file.usage_by_lms = 'display'
+      end
     end
 
     def tests
-      @exercise.files.filter(&:teacher_defined_assessment?).map do |file|
+      @exercise.files.filter(&:teacher_defined_assessment?).group_by {|file| xml_id_from_file(file).first }.map do |xml_id, files|
         ProformaXML::Test.new(
-          id: file.id,
-          title: file.name,
-          files: test_file(file),
-          meta_data: test_meta_data(file)
+          id: xml_id,
+          title: files.first.name,
+          files: files.map {|file| test_file(file) },
+          meta_data: test_meta_data(files)
         )
       end
     end
 
-    def test_meta_data(file)
+    def xml_id_from_file(file)
+      type = if file.teacher_defined_assessment?
+               'test'
+             elsif file.reference_implementation?
+               'ms'
+             else
+               'file'
+             end
+      xml_id_path_parts = file.xml_id_path&.split('/')
+      xml_id_path  = []
+
+      xml_id_path << (xml_id_path_parts&.first || "co-#{type}-#{file.id}") unless type == 'file'
+      xml_id_path << (xml_id_path_parts&.last || file.id)
+
+      xml_id_path
+    end
+
+    def test_meta_data(files)
       {
         '@@order' => %w[test-meta-data],
         'test-meta-data' => {
-          '@@order' => %w[CodeOcean:feedback-message CodeOcean:weight CodeOcean:hidden-feedback],
+          '@@order' => %w[CodeOcean:test-file],
           '@xmlns' => {'CodeOcean' => 'codeocean.openhpi.de'},
-          'CodeOcean:feedback-message' => {
-            '@@order' => %w[$1],
-            '$1' => file.feedback_message,
-          },
-          'CodeOcean:weight' => {
-            '@@order' => %w[$1],
-            '$1' => file.weight,
-          },
-          'CodeOcean:hidden-feedback' => {
-            '@@order' => %w[$1],
-            '$1' => file.hidden_feedback,
-          },
+          'CodeOcean:test-file' => files.map do |file|
+            {
+              '@@order' => %w[CodeOcean:feedback-message CodeOcean:weight CodeOcean:hidden-feedback],
+              '@xmlns' => {'CodeOcean' => 'codeocean.openhpi.de'},
+              '@id' =>  xml_id_from_file(file).last,
+              '@name' => file.name,
+              'CodeOcean:feedback-message' => {
+                '@@order' => %w[$1],
+                '$1' => file.feedback_message,
+              },
+              'CodeOcean:weight' => {
+                '@@order' => %w[$1],
+                '$1' => file.weight,
+              },
+              'CodeOcean:hidden-feedback' => {
+                '@@order' => %w[$1],
+                '$1' => file.hidden_feedback,
+              },
+            }
+          end,
         },
       }
     end
 
     def test_file(file)
-      [
-        task_file(file).tap do |t_file|
-          t_file.used_by_grader = true
-        end,
-      ]
+      task_file(file).tap do |t_file|
+        t_file.used_by_grader = true
+      end
     end
 
     def exercise_files
@@ -168,8 +190,11 @@ def task_files
     end
 
     def task_file(file)
+      file.update(xml_id_path: xml_id_from_file(file).join('/')) if file.xml_id_path.blank?
+
+      xml_id = xml_id_from_file(file).last
       task_file = ProformaXML::TaskFile.new(
-        id: file.id,
+        id: xml_id,
         filename: filename(file),
         usage_by_lms: file.read_only ? 'display' : 'edit',
         visible: file.hidden ? 'no' : 'yes'
diff --git a/app/services/proforma_service/convert_task_to_exercise.rb b/app/services/proforma_service/convert_task_to_exercise.rb
index 6eda6e4ce..880915e36 100644
--- a/app/services/proforma_service/convert_task_to_exercise.rb
+++ b/app/services/proforma_service/convert_task_to_exercise.rb
@@ -60,44 +60,53 @@ def string_to_bool(str)
     end
 
     def files
-      model_solution_files + test_files + task_files.values.tap {|array| array.each {|file| file.role ||= 'regular_file' } }
+      model_solution_files + test_files + task_files
     end
 
     def test_files
-      @task.tests.map do |test_object|
-        task_files.delete(test_object.files.first.id).tap do |file|
-          file.weight = extract_meta_data(test_object.meta_data&.dig('test-meta-data'), 'weight').presence || 1.0
-          file.feedback_message = extract_meta_data(test_object.meta_data&.dig('test-meta-data'), 'feedback-message').presence || 'Feedback'
-          file.hidden_feedback = extract_meta_data(test_object.meta_data&.dig('test-meta-data'), 'hidden-feedback').presence || false
-          file.role ||= 'teacher_defined_test'
+      @task.tests.flat_map do |test|
+        test.files.map do |task_file|
+          codeocean_file_from_task_file(task_file, test).tap do |file|
+            file.weight = extract_meta_data(test.meta_data&.dig('test-meta-data'), 'test-file', task_file.id, 'weight').presence || 1.0
+            file.feedback_message = extract_meta_data(test.meta_data&.dig('test-meta-data'), 'test-file', task_file.id, 'feedback-message').presence || 'Feedback'
+            file.hidden_feedback = extract_meta_data(test.meta_data&.dig('test-meta-data'), 'test-file', task_file.id, 'hidden-feedback').presence || false
+            file.role = 'teacher_defined_test' unless file.teacher_defined_assessment?
+          end
         end
       end
     end
 
     def model_solution_files
-      @task.model_solutions.map do |model_solution_object|
-        task_files.delete(model_solution_object.files.first.id).tap do |file|
-          file.role ||= 'reference_implementation'
+      @task.model_solutions.flat_map do |model_solution|
+        model_solution.files.map do |task_file|
+          codeocean_file_from_task_file(task_file, model_solution).tap do |file|
+            file.role ||= 'reference_implementation'
+          end
         end
       end
     end
 
     def task_files
-      @task_files ||= @task.all_files.reject {|file| file.id == 'ms-placeholder-file' }.to_h do |task_file|
-        [task_file.id, codeocean_file_from_task_file(task_file)]
+      @task.files.reject {|file| file.id == 'ms-placeholder-file' }.map do |task_file|
+        codeocean_file_from_task_file(task_file).tap do |file|
+          file.role ||= 'regular_file'
+        end
       end
     end
 
-    def codeocean_file_from_task_file(file)
+    def codeocean_file_from_task_file(file, parent_object = nil)
       extension = File.extname(file.filename)
-      codeocean_file = CodeOcean::File.new(
+
+      codeocean_file = CodeOcean::File.where(context: @exercise).where('xml_id_path = ? OR xml_id_path LIKE ?', file.id, "%/#{file.id}").first_or_initialize(context: @exercise)
+      codeocean_file.assign_attributes(
         context: @exercise,
         file_type: file_type(extension),
         hidden: file.visible != 'yes', # hides 'delayed' and 'no'
         name: File.basename(file.filename, '.*'),
         read_only: file.usage_by_lms != 'edit',
         role: extract_meta_data(@task.meta_data&.dig('meta-data'), 'files', "CO-#{file.id}", 'role'),
-        path: File.dirname(file.filename).in?(['.', '']) ? nil : File.dirname(file.filename)
+        path: File.dirname(file.filename).in?(['.', '']) ? nil : File.dirname(file.filename),
+        xml_id_path: (parent_object.nil? ? '' : "#{parent_object.id}/") + file.id.to_s
       )
       if file.binary
         codeocean_file.native_file = FileIO.new(file.content.dup.force_encoding('UTF-8'), File.basename(file.filename))
diff --git a/app/services/proforma_service/import.rb b/app/services/proforma_service/import.rb
index 0547f8bcb..86e11d5fd 100644
--- a/app/services/proforma_service/import.rb
+++ b/app/services/proforma_service/import.rb
@@ -14,13 +14,7 @@ def execute
         import_result = importer.perform
         @task = import_result
 
-        exercise = base_exercise
-        exercise_files = exercise&.files&.to_a
-
-        exercise = ConvertTaskToExercise.call(task: @task, user: @user, exercise:)
-        exercise_files&.each(&:destroy) # feels suboptimal
-
-        exercise
+        ConvertTaskToExercise.call(task: @task, user: @user, exercise: base_exercise)
       else
         import_multi
       end
diff --git a/db/migrate/20240903204319_add_xml_path_to_files.rb b/db/migrate/20240903204319_add_xml_path_to_files.rb
new file mode 100644
index 000000000..680624c15
--- /dev/null
+++ b/db/migrate/20240903204319_add_xml_path_to_files.rb
@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class AddXmlPathToFiles < ActiveRecord::Migration[7.1]
+  def change
+    add_column :files, :xml_id_path, :string, null: true, default: nil
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 61ba96052..1cf5d14fc 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -315,6 +315,7 @@
     t.string "path"
     t.integer "file_template_id"
     t.boolean "hidden_feedback", default: false, null: false
+    t.string "xml_id_path"
     t.index ["context_id", "context_type"], name: "index_files_on_context_id_and_context_type"
   end
 
diff --git a/spec/services/proforma_service/convert_exercise_to_task_spec.rb b/spec/services/proforma_service/convert_exercise_to_task_spec.rb
index 73e68dc03..40f7c292a 100644
--- a/spec/services/proforma_service/convert_exercise_to_task_spec.rb
+++ b/spec/services/proforma_service/convert_exercise_to_task_spec.rb
@@ -79,11 +79,11 @@
 
     context 'when exercise has a mainfile' do
       let(:files) { [file] }
-      let(:file) { build(:file) }
+      let(:file) { create(:file) }
 
       it 'creates a task-file with the correct attributes' do
         expect(task.files.first).to have_attributes(
-          id: file.id,
+          id: file.id.to_s,
           content: file.content,
           filename: file.name_with_extension,
           used_by_grader: true,
@@ -109,6 +109,18 @@
           )
         )
       end
+
+      it 'sets xml_id_path to default' do
+        expect { convert_to_task.execute }.to change(file.reload, :xml_id_path).from(nil).to(file.id.to_s)
+      end
+
+      context 'when xml_id_path is set for file' do
+        let(:file) { create(:file, xml_id_path: 'foobar') }
+
+        it 'does not change xml_path_id' do
+          expect { convert_to_task.execute }.not_to change(file.reload, :xml_id_path)
+        end
+      end
     end
 
     context 'when exercise has a regular file' do
@@ -119,7 +131,7 @@
 
       it 'creates a task-file with the correct attributes' do
         expect(task.files.first).to have_attributes(
-          id: file.id,
+          id: file.id.to_s,
           content: file.content,
           filename: file.name_with_extension,
           used_by_grader: true,
@@ -185,14 +197,14 @@
 
       it 'creates a model-solution with one file' do
         expect(task.model_solutions.first).to have_attributes(
-          id: "ms-#{file.id}",
+          id: "co-ms-#{file.id}",
           files: have(1).item
         )
       end
 
       it 'creates a model-solution with one file with correct attributes' do
         expect(task.model_solutions.first.files.first).to have_attributes(
-          id: file.id,
+          id: file.id.to_s,
           content: file.content,
           filename: file.name_with_extension,
           used_by_grader: false,
@@ -210,6 +222,18 @@
       it 'creates a task with two model-solutions' do
         expect(task.model_solutions).to have(2).items
       end
+
+      context 'when reference_implementations belong to the same proforma model_solution' do
+        let(:files) { build_list(:file, 2, role: 'reference_implementation') {|file, i| file.xml_id_path = "proforma_ms/xml_id_#{i}" } }
+
+        it 'creates a task with one model-solution' do
+          expect(task.model_solutions).to have(1).items
+        end
+
+        it 'creates a task with a model-solution with two files' do
+          expect(task.model_solutions.first.files).to have(2).items
+        end
+      end
     end
 
     context 'when exercise has a test' do
@@ -223,14 +247,19 @@
 
       it 'creates a test with one file' do
         expect(task.tests.first).to have_attributes(
-          id: test_file.id,
+          id: "co-test-#{test_file.id}",
           title: test_file.name,
           files: have(1).item,
           meta_data: a_hash_including(
             'test-meta-data' => a_hash_including(
-              'CodeOcean:feedback-message' => {'$1' => 'feedback_message', '@@order' => ['$1']},
-              'CodeOcean:weight' => {'$1' => test_file.weight, '@@order' => ['$1']},
-              'CodeOcean:hidden-feedback' => {'$1' => test_file.hidden_feedback, '@@order' => ['$1']}
+              'CodeOcean:test-file' => contain_exactly(
+                a_hash_including(
+                  '@name' => test_file.name,
+                  'CodeOcean:feedback-message' => {'$1' => 'feedback_message', '@@order' => ['$1']},
+                  'CodeOcean:weight' => {'$1' => test_file.weight, '@@order' => ['$1']},
+                  'CodeOcean:hidden-feedback' => {'$1' => test_file.hidden_feedback, '@@order' => ['$1']}
+                )
+              )
             )
           )
         )
@@ -238,7 +267,7 @@
 
       it 'creates a test with one file with correct attributes' do
         expect(task.tests.first.files.first).to have_attributes(
-          id: test_file.id,
+          id: test_file.id.to_s,
           content: test_file.content,
           filename: test_file.name_with_extension,
           used_by_grader: true,
@@ -263,6 +292,36 @@
       it 'creates a task with two tests' do
         expect(task.tests).to have(2).items
       end
+
+      context 'when test_files belong to the same proforma test' do
+        let(:tests) { build_list(:test_file, 2) {|file, i| file.xml_id_path = "proforma_test/xml_id_#{i}" } }
+
+        it 'creates a test with two file' do
+          expect(task.tests.first).to have_attributes(
+            id: 'proforma_test',
+            title: tests.first.name,
+            files: have(2).item,
+            meta_data: a_hash_including(
+              'test-meta-data' => a_hash_including(
+                'CodeOcean:test-file' => contain_exactly(
+                  a_hash_including(
+                    '@name' => tests.first.name,
+                    'CodeOcean:feedback-message' => {'$1' => 'feedback_message', '@@order' => ['$1']},
+                    'CodeOcean:weight' => {'$1' => 1, '@@order' => ['$1']},
+                    'CodeOcean:hidden-feedback' => {'$1' => tests.first.hidden_feedback, '@@order' => ['$1']}
+                  ),
+                  a_hash_including(
+                    '@name' => tests.last.name,
+                    'CodeOcean:feedback-message' => {'$1' => 'feedback_message', '@@order' => ['$1']},
+                    'CodeOcean:weight' => {'$1' => 1, '@@order' => ['$1']},
+                    'CodeOcean:hidden-feedback' => {'$1' => tests.last.hidden_feedback, '@@order' => ['$1']}
+                  )
+                )
+              )
+            )
+          )
+        end
+      end
     end
   end
 end
diff --git a/spec/services/proforma_service/convert_task_to_exercise_spec.rb b/spec/services/proforma_service/convert_task_to_exercise_spec.rb
index 12438d9b3..400d35820 100644
--- a/spec/services/proforma_service/convert_task_to_exercise_spec.rb
+++ b/spec/services/proforma_service/convert_task_to_exercise_spec.rb
@@ -361,10 +361,18 @@
           files: test_files,
           meta_data: {
             'test-meta-data' => {
-              '@@order' => %w[CodeOcean:feedback-message CodeOcean:weight CodeOcean:hidden-feedback],
-              'CodeOcean:feedback-message' => {'$1' => 'feedback-message', '@@order' => ['$1']},
-              'CodeOcean:weight' => {'$1' => '0.7', '@@order' => ['$1']},
-              'CodeOcean:hidden-feedback' => {'$1' => 'true', '@@order' => ['$1']},
+              '@@order' => %w[CodeOcean:test-file],
+              '@xmlns' => {'CodeOcean' => 'codeocean.openhpi.de'},
+              'CodeOcean:test-file' => {
+                'CodeOcean:test_file_id' =>
+                {
+                  '@@order' => %w[CodeOcean:feedback-message CodeOcean:weight CodeOcean:hidden-feedback],
+                '@name' => 'test_file_name',
+                'CodeOcean:feedback-message' => {'$1' => 'feedback-message', '@@order' => ['$1']},
+                'CodeOcean:weight' => {'$1' => '0.7', '@@order' => ['$1']},
+                'CodeOcean:hidden-feedback' => {'$1' => 'true', '@@order' => ['$1']},
+                },
+              },
             },
           }
         )
@@ -506,7 +514,7 @@
         let(:test_files) { [test_file] }
         let(:test_file) do
           ProformaXML::TaskFile.new(
-            id: 'test_file_id',
+            id: 'test-file-id',
             content: 'testfile-content',
             filename: 'testfile.txt',
             used_by_grader: 'yes',
@@ -526,7 +534,7 @@
         let(:ms_files) { [ms_file] }
         let(:ms_file) do
           ProformaXML::TaskFile.new(
-            id: 'ms-file',
+            id: 'ms-file-id',
             content: 'ms-content',
             filename: 'filename.txt',
             used_by_grader: 'used_by_grader',
@@ -546,6 +554,26 @@
               .and(include(have_attributes(content: 'testfile-content', role: 'teacher_defined_test', hidden: true)))
           )
         end
+
+        context 'when the files with correct xml_id_paths already exist' do
+          let(:exercise) do
+            create(:dummy,
+              files: co_files,
+              title: 'exercise-title',
+              description: 'exercise-description')
+          end
+          let(:co_files) do
+            [create(:file, xml_id_path: 'id', role: 'regular_file'),
+             create(:file, xml_id_path: 'ms-id/ms-file-id', role: 'reference_implementation'),
+             create(:test_file, xml_id_path: 'test-id/test-file-id')]
+          end
+
+          it 'reuses existing file' do
+            convert_to_exercise_service
+
+            expect(exercise.reload.files).to match_array(co_files)
+          end
+        end
       end
     end
   end
diff --git a/spec/services/proforma_service/import_spec.rb b/spec/services/proforma_service/import_spec.rb
index 286d0a45f..6e224aa91 100644
--- a/spec/services/proforma_service/import_spec.rb
+++ b/spec/services/proforma_service/import_spec.rb
@@ -41,7 +41,7 @@
     let(:execution_environment) { build(:java) }
     let(:files) { [] }
     let(:tests) { [] }
-    let(:exporter) { ProformaService::ExportTask.call(exercise: exercise.reload).string }
+    let(:exporter) { ProformaService::ExportTask.call(exercise:).string }
 
     before do
       zip_file.write(exporter)
@@ -51,7 +51,7 @@
     it { is_expected.to be_an_equal_exercise_as exercise }
 
     it 'sets the correct user as owner of the exercise' do
-      expect(import_service.user).to be user
+      expect(import_service.user).to eq user
     end
 
     it 'sets the uuid' do

From 6b8c04b79407c4ba8dce87e4ec7c7ac69b74b226 Mon Sep 17 00:00:00 2001
From: Karol <git@koehn.pro>
Date: Tue, 24 Sep 2024 23:59:23 +0200
Subject: [PATCH 2/3] refactor xml_id_path to use array

---
 .../convert_exercise_to_task.rb               | 13 ++++++-----
 .../convert_task_to_exercise.rb               |  9 ++++----
 ...> 20241007204319_add_xml_path_to_files.rb} |  2 +-
 db/schema.rb                                  |  4 ++--
 .../convert_exercise_to_task_spec.rb          | 23 ++++++++++++++-----
 .../convert_task_to_exercise_spec.rb          | 21 +++++++++++++----
 spec/services/proforma_service/import_spec.rb |  2 +-
 7 files changed, 50 insertions(+), 24 deletions(-)
 rename db/migrate/{20240903204319_add_xml_path_to_files.rb => 20241007204319_add_xml_path_to_files.rb} (56%)

diff --git a/app/services/proforma_service/convert_exercise_to_task.rb b/app/services/proforma_service/convert_exercise_to_task.rb
index db92ec6af..67e290cbe 100644
--- a/app/services/proforma_service/convert_exercise_to_task.rb
+++ b/app/services/proforma_service/convert_exercise_to_task.rb
@@ -67,7 +67,7 @@ def create_task
 
     def proglang
       regex = %r{^openhpi/co_execenv_(?<language>[^:]*):(?<version>[^-]*)(?>-.*)?$}
-      match = regex.match @exercise.execution_environment.docker_image
+      match = regex.match @exercise&.execution_environment&.docker_image
       match ? {name: match[:language], version: match[:version]} : nil
     end
 
@@ -105,6 +105,9 @@ def tests
     end
 
     def xml_id_from_file(file)
+      xml_id_path = file.xml_id_path || []
+      return xml_id_path if xml_id_path&.any?
+
       type = if file.teacher_defined_assessment?
                'test'
              elsif file.reference_implementation?
@@ -112,11 +115,9 @@ def xml_id_from_file(file)
              else
                'file'
              end
-      xml_id_path_parts = file.xml_id_path&.split('/')
-      xml_id_path  = []
 
-      xml_id_path << (xml_id_path_parts&.first || "co-#{type}-#{file.id}") unless type == 'file'
-      xml_id_path << (xml_id_path_parts&.last || file.id)
+      xml_id_path << "co-#{type}-#{file.id}" unless type == 'file'
+      xml_id_path << file.id.to_s
 
       xml_id_path
     end
@@ -190,7 +191,7 @@ def task_files
     end
 
     def task_file(file)
-      file.update(xml_id_path: xml_id_from_file(file).join('/')) if file.xml_id_path.blank?
+      file.update(xml_id_path: xml_id_from_file(file)) if file.xml_id_path.blank?
 
       xml_id = xml_id_from_file(file).last
       task_file = ProformaXML::TaskFile.new(
diff --git a/app/services/proforma_service/convert_task_to_exercise.rb b/app/services/proforma_service/convert_task_to_exercise.rb
index 880915e36..4e64312b5 100644
--- a/app/services/proforma_service/convert_task_to_exercise.rb
+++ b/app/services/proforma_service/convert_task_to_exercise.rb
@@ -34,7 +34,7 @@ def import_task
 
     def extract_meta_data(meta_data, *path)
       current_level = meta_data
-      path.each {|attribute| current_level = current_level&.dig("CodeOcean:#{attribute}") }
+      path.each {|attribute| current_level = current_level.is_a?(Hash) ? current_level&.dig("CodeOcean:#{attribute}") : current_level&.find {|entry| entry['@id'] == attribute } } # || current_level
       current_level&.dig('$1')
     end
 
@@ -81,6 +81,7 @@ def model_solution_files
         model_solution.files.map do |task_file|
           codeocean_file_from_task_file(task_file, model_solution).tap do |file|
             file.role ||= 'reference_implementation'
+            file.feedback_message = nil
           end
         end
       end
@@ -90,14 +91,14 @@ def task_files
       @task.files.reject {|file| file.id == 'ms-placeholder-file' }.map do |task_file|
         codeocean_file_from_task_file(task_file).tap do |file|
           file.role ||= 'regular_file'
+          file.feedback_message = nil
         end
       end
     end
 
     def codeocean_file_from_task_file(file, parent_object = nil)
       extension = File.extname(file.filename)
-
-      codeocean_file = CodeOcean::File.where(context: @exercise).where('xml_id_path = ? OR xml_id_path LIKE ?', file.id, "%/#{file.id}").first_or_initialize(context: @exercise)
+      codeocean_file = @exercise.files.where('array_length(xml_id_path, 1) IS NOT NULL AND xml_id_path[array_length(xml_id_path, 1)] = ?', file.id).first_or_initialize
       codeocean_file.assign_attributes(
         context: @exercise,
         file_type: file_type(extension),
@@ -106,7 +107,7 @@ def codeocean_file_from_task_file(file, parent_object = nil)
         read_only: file.usage_by_lms != 'edit',
         role: extract_meta_data(@task.meta_data&.dig('meta-data'), 'files', "CO-#{file.id}", 'role'),
         path: File.dirname(file.filename).in?(['.', '']) ? nil : File.dirname(file.filename),
-        xml_id_path: (parent_object.nil? ? '' : "#{parent_object.id}/") + file.id.to_s
+        xml_id_path: (parent_object.nil? ? [file.id] : [parent_object.id, file.id]).map(&:to_s)
       )
       if file.binary
         codeocean_file.native_file = FileIO.new(file.content.dup.force_encoding('UTF-8'), File.basename(file.filename))
diff --git a/db/migrate/20240903204319_add_xml_path_to_files.rb b/db/migrate/20241007204319_add_xml_path_to_files.rb
similarity index 56%
rename from db/migrate/20240903204319_add_xml_path_to_files.rb
rename to db/migrate/20241007204319_add_xml_path_to_files.rb
index 680624c15..80c492b6b 100644
--- a/db/migrate/20240903204319_add_xml_path_to_files.rb
+++ b/db/migrate/20241007204319_add_xml_path_to_files.rb
@@ -2,6 +2,6 @@
 
 class AddXmlPathToFiles < ActiveRecord::Migration[7.1]
   def change
-    add_column :files, :xml_id_path, :string, null: true, default: nil
+    add_column :files, :xml_id_path, :string, array: true, default: [], null: true
   end
 end
diff --git a/db/schema.rb b/db/schema.rb
index 1cf5d14fc..a7ada58d3 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.2].define(version: 2024_09_12_101506) do
+ActiveRecord::Schema[7.2].define(version: 2024_10_07_204319) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
   enable_extension "pgcrypto"
@@ -315,7 +315,7 @@
     t.string "path"
     t.integer "file_template_id"
     t.boolean "hidden_feedback", default: false, null: false
-    t.string "xml_id_path"
+    t.string "xml_id_path", default: [], array: true
     t.index ["context_id", "context_type"], name: "index_files_on_context_id_and_context_type"
   end
 
diff --git a/spec/services/proforma_service/convert_exercise_to_task_spec.rb b/spec/services/proforma_service/convert_exercise_to_task_spec.rb
index 40f7c292a..6c38aa558 100644
--- a/spec/services/proforma_service/convert_exercise_to_task_spec.rb
+++ b/spec/services/proforma_service/convert_exercise_to_task_spec.rb
@@ -22,8 +22,10 @@
         execution_environment:,
         instructions: 'instruction',
         uuid: SecureRandom.uuid,
-        files: files + tests)
+        files: files + tests,
+        unpublished:)
     end
+    let(:unpublished) { false }
     let(:files) { [] }
     let(:tests) { [] }
     let(:execution_environment) { create(:java) }
@@ -77,6 +79,15 @@
       end
     end
 
+    context 'when exercise has no execution_environment' do
+      let(:execution_environment) { nil }
+      let(:unpublished) { true }
+
+      it 'creates a task with the correct proglang attribute' do
+        expect(task).to have_attributes(proglang: nil)
+      end
+    end
+
     context 'when exercise has a mainfile' do
       let(:files) { [file] }
       let(:file) { create(:file) }
@@ -111,13 +122,13 @@
       end
 
       it 'sets xml_id_path to default' do
-        expect { convert_to_task.execute }.to change(file.reload, :xml_id_path).from(nil).to(file.id.to_s)
+        expect { convert_to_task.execute }.to change(file.reload, :xml_id_path).from([]).to([file.id.to_s])
       end
 
       context 'when xml_id_path is set for file' do
-        let(:file) { create(:file, xml_id_path: 'foobar') }
+        let(:file) { create(:file, xml_id_path: ['foobar']) }
 
-        it 'does not change xml_path_id' do
+        it 'does not change xml_id_path' do
           expect { convert_to_task.execute }.not_to change(file.reload, :xml_id_path)
         end
       end
@@ -224,7 +235,7 @@
       end
 
       context 'when reference_implementations belong to the same proforma model_solution' do
-        let(:files) { build_list(:file, 2, role: 'reference_implementation') {|file, i| file.xml_id_path = "proforma_ms/xml_id_#{i}" } }
+        let(:files) { build_list(:file, 2, role: 'reference_implementation') {|file, i| file.xml_id_path = ['proforma_ms', "xml_id_#{i}"] } }
 
         it 'creates a task with one model-solution' do
           expect(task.model_solutions).to have(1).items
@@ -294,7 +305,7 @@
       end
 
       context 'when test_files belong to the same proforma test' do
-        let(:tests) { build_list(:test_file, 2) {|file, i| file.xml_id_path = "proforma_test/xml_id_#{i}" } }
+        let(:tests) { build_list(:test_file, 2) {|file, i| file.xml_id_path = ['proforma_test', "xml_id_#{i}"] } }
 
         it 'creates a test with two file' do
           expect(task.tests.first).to have_attributes(
diff --git a/spec/services/proforma_service/convert_task_to_exercise_spec.rb b/spec/services/proforma_service/convert_task_to_exercise_spec.rb
index 400d35820..68dcd5079 100644
--- a/spec/services/proforma_service/convert_task_to_exercise_spec.rb
+++ b/spec/services/proforma_service/convert_task_to_exercise_spec.rb
@@ -558,21 +558,34 @@
         context 'when the files with correct xml_id_paths already exist' do
           let(:exercise) do
             create(:dummy,
+              unpublished: true,
               files: co_files,
               title: 'exercise-title',
               description: 'exercise-description')
           end
           let(:co_files) do
-            [create(:file, xml_id_path: 'id', role: 'regular_file'),
-             create(:file, xml_id_path: 'ms-id/ms-file-id', role: 'reference_implementation'),
-             create(:test_file, xml_id_path: 'test-id/test-file-id')]
+            [create(:file, xml_id_path: ['id'], role: 'regular_file'),
+             create(:file, xml_id_path: %w[ms-id ms-file-id], role: 'reference_implementation'),
+             create(:test_file, xml_id_path: %w[test-id test-file-id])]
           end
 
           it 'reuses existing file' do
             convert_to_exercise_service
-
+            exercise.save!
             expect(exercise.reload.files).to match_array(co_files)
           end
+
+          context 'when files are move around' do
+            let(:files) { [test_file] }
+            let(:test_files) { [ms_file] }
+            let(:ms_files) { [file] }
+
+            it 'reuses existing file' do
+              convert_to_exercise_service
+              exercise.save!
+              expect(exercise.reload.files).to match_array(co_files)
+            end
+          end
         end
       end
     end
diff --git a/spec/services/proforma_service/import_spec.rb b/spec/services/proforma_service/import_spec.rb
index 6e224aa91..53290f889 100644
--- a/spec/services/proforma_service/import_spec.rb
+++ b/spec/services/proforma_service/import_spec.rb
@@ -127,7 +127,7 @@
     end
 
     context 'when exercise has multiple tests' do
-      let(:tests) { build_list(:test_file, 2) }
+      let(:tests) { [build(:test_file, xml_id_path: %w[test file1]), build(:test_file, xml_id_path: %w[test file2])] }
 
       it { is_expected.to be_an_equal_exercise_as exercise }
     end

From e6a3f2316c78c89f9fdc0a05edd87af83d59a905 Mon Sep 17 00:00:00 2001
From: kkoehn <git@koehn.pro>
Date: Mon, 30 Sep 2024 19:23:46 +0200
Subject: [PATCH 3/3] Update
 app/services/proforma_service/convert_exercise_to_task.rb

Co-authored-by: Sebastian Serth <MrSerth@users.noreply.github.com>
---
 app/services/proforma_service/convert_exercise_to_task.rb | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/app/services/proforma_service/convert_exercise_to_task.rb b/app/services/proforma_service/convert_exercise_to_task.rb
index 67e290cbe..0b8614109 100644
--- a/app/services/proforma_service/convert_exercise_to_task.rb
+++ b/app/services/proforma_service/convert_exercise_to_task.rb
@@ -77,8 +77,7 @@ def uuid
     end
 
     def model_solutions
-      model_solutions_files = @exercise.files.filter {|file| file.role == 'reference_implementation' }.group_by {|file| xml_id_from_file(file).first }
-      model_solutions_files.map do |xml_id, files|
+      @exercise.files.filter(&:reference_implementation?).group_by {|file| xml_id_from_file(file).first }.map do |xml_id, files|
         ProformaXML::ModelSolution.new(
           id: xml_id,
           files: files.map {|file| model_solution_file(file) }