Add latest changes from gitlab-org/gitlab@master

10 files changed, 145 insertions, 6 deletions

diff --git a/spec/factories/ml/model_version_metadata.rb b/spec/factories/ml/model_version_metadata.rb
new file mode 100644
index 00000000000..de13465e5bf
--- /dev/null
+++ b/spec/factories/ml/model_version_metadata.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :ml_model_version_metadata, class: '::Ml::ModelVersionMetadata' do
+    association :model_version, factory: :ml_model_versions
+    association :project, factory: :project
+
+    sequence(:name) { |n| "metadata_#{n}" }
+    sequence(:value) { |n| "value#{n}" }
+  end
+end
diff --git a/spec/fixtures/api/schemas/ml/get_latest_versions.json b/spec/fixtures/api/schemas/ml/get_latest_versions.json
index cb2308fa637..3bf9a43a795 100644
--- a/spec/fixtures/api/schemas/ml/get_latest_versions.json
+++ b/spec/fixtures/api/schemas/ml/get_latest_versions.json
@@ -21,7 +21,7 @@
             "run_id",
             "status",
             "status_message",
-            "metadata",
+            "tags",
             "run_link",
             "aliases"
           ],
@@ -59,7 +59,7 @@
             "status_message": {
               "type": "string"
             },
-            "metadata": {
+            "tags": {
               "type": "array",
               "items": {
               }
diff --git a/spec/fixtures/api/schemas/ml/get_model_version.json b/spec/fixtures/api/schemas/ml/get_model_version.json
index 214c993ed73..89779428edf 100644
--- a/spec/fixtures/api/schemas/ml/get_model_version.json
+++ b/spec/fixtures/api/schemas/ml/get_model_version.json
@@ -18,7 +18,7 @@
         "run_id",
         "status",
         "status_message",
-        "metadata",
+        "tags",
         "run_link",
         "aliases"
       ],
@@ -56,9 +56,22 @@
         "status_message": {
           "type": "string"
         },
-        "metadata": {
+        "tags": {
           "type": "array",
           "items": {
+            "type": "object",
+            "required": [
+              "key",
+              "value"
+            ],
+            "properties": {
+              "key": {
+                "type": "string"
+              },
+              "value": {
+                "type": "string"
+              }
+            }
           }
         },
         "run_link": {
diff --git a/spec/models/ml/candidate_metric_spec.rb b/spec/models/ml/candidate_metric_spec.rb
index 9f9a6e8e3ba..9ceaa83a6fa 100644
--- a/spec/models/ml/candidate_metric_spec.rb
+++ b/spec/models/ml/candidate_metric_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Ml::CandidateMetric do
+RSpec.describe Ml::CandidateMetric, feature_category: :mlops do
   describe 'associations' do
     it { is_expected.to belong_to(:candidate) }
   end
diff --git a/spec/models/ml/candidate_param_spec.rb b/spec/models/ml/candidate_param_spec.rb
index ff38e471219..89232b10855 100644
--- a/spec/models/ml/candidate_param_spec.rb
+++ b/spec/models/ml/candidate_param_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Ml::CandidateParam do
+RSpec.describe Ml::CandidateParam, feature_category: :mlops do
   describe 'associations' do
     it { is_expected.to belong_to(:candidate) }
   end
diff --git a/spec/models/ml/model_metadata_spec.rb b/spec/models/ml/model_metadata_spec.rb
index f06c7a2ce50..0afc7bb7a2e 100644
--- a/spec/models/ml/model_metadata_spec.rb
+++ b/spec/models/ml/model_metadata_spec.rb
@@ -5,6 +5,8 @@ require 'spec_helper'
 RSpec.describe Ml::ModelMetadata, feature_category: :mlops do
   describe 'associations' do
     it { is_expected.to belong_to(:model).required }
+    it { is_expected.to validate_length_of(:name).is_at_most(255) }
+    it { is_expected.to validate_length_of(:value).is_at_most(5000) }
   end
 
   describe 'validations' do
diff --git a/spec/models/ml/model_version_metadata_spec.rb b/spec/models/ml/model_version_metadata_spec.rb
new file mode 100644
index 00000000000..7c8ffb9b0d7
--- /dev/null
+++ b/spec/models/ml/model_version_metadata_spec.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ml::ModelVersionMetadata, feature_category: :mlops do
+  describe 'associations' do
+    it { is_expected.to belong_to(:model_version).required }
+    it { is_expected.to belong_to(:project).required }
+    it { is_expected.to validate_length_of(:name).is_at_most(255) }
+    it { is_expected.to validate_length_of(:value).is_at_most(5000) }
+  end
+
+  describe 'validations' do
+    let_it_be(:metadata) { create(:ml_model_version_metadata, name: 'some_metadata') }
+    let_it_be(:model_version) { metadata.model_version }
+
+    it 'is unique within the model version' do
+      expect do
+        model_version.metadata.create!(name: 'some_metadata', value: 'blah')
+      end.to raise_error.with_message(/Name 'some_metadata' already taken/)
+    end
+
+    it 'a model version is required' do
+      expect do
+        described_class.create!(name: 'some_metadata', value: 'blah')
+      end.to raise_error.with_message(/Model version must exist/)
+    end
+  end
+end
diff --git a/spec/models/ml/model_version_spec.rb b/spec/models/ml/model_version_spec.rb
index 95d4a545f52..9db9f7e34ab 100644
--- a/spec/models/ml/model_version_spec.rb
+++ b/spec/models/ml/model_version_spec.rb
@@ -19,6 +19,7 @@ RSpec.describe Ml::ModelVersion, feature_category: :mlops do
     it { is_expected.to belong_to(:model) }
     it { is_expected.to belong_to(:package).class_name('Packages::MlModel::Package') }
     it { is_expected.to have_one(:candidate).class_name('Ml::Candidate') }
+    it { is_expected.to have_many(:metadata) }
   end
 
   describe 'validation' do
@@ -99,6 +100,34 @@ RSpec.describe Ml::ModelVersion, feature_category: :mlops do
     end
   end
 
+  describe '#add_metadata' do
+    it 'accepts an array of metadata and persists it to the model version' do
+      input = [
+        { project_id: base_project.id, key: 'tag1', value: 'value1' },
+        { project_id: base_project.id, key: 'tag2', value: 'value2' }
+      ]
+
+      expect { model_version1.add_metadata(input) }.to change { model_version1.metadata.count }.by(2)
+    end
+
+    it 'raises an error when duplicate key names are supplied' do
+      input = [
+        { project_id: base_project.id, key: 'tag1', value: 'value1' },
+        { project_id: base_project.id, key: 'tag1', value: 'value2' }
+      ]
+
+      expect { model_version1.add_metadata(input) }.to raise_error(ActiveRecord::RecordInvalid)
+    end
+
+    it 'raises an error when validation fails' do
+      input = [
+        { project_id: base_project.id, key: nil, value: 'value1' }
+      ]
+
+      expect { model_version1.add_metadata(input) }.to raise_error(ActiveRecord::RecordInvalid)
+    end
+  end
+
   describe '#find_or_create!' do
     let_it_be(:existing_model_version) { create(:ml_model_versions, model: model1, version: '1.0.0') }
 
diff --git a/spec/requests/api/ml/mlflow/model_versions_spec.rb b/spec/requests/api/ml/mlflow/model_versions_spec.rb
index e62bccf1507..16990642764 100644
--- a/spec/requests/api/ml/mlflow/model_versions_spec.rb
+++ b/spec/requests/api/ml/mlflow/model_versions_spec.rb
@@ -164,6 +164,30 @@ RSpec.describe API::Ml::Mlflow::ModelVersions, feature_category: :mlops do
 
           it_behaves_like 'MLflow|Not Found - Resource Does Not Exist'
         end
+
+        # TODO: Ensure consisted error responses https://gitlab.com/gitlab-org/gitlab/-/issues/429731
+        context 'when a duplicate tag name is supplied' do
+          let(:params) do
+            { name: model_name, tags: [{ key: 'key1', value: 'value1' }, { key: 'key1', value: 'value2' }] }
+          end
+
+          it "returns a validation error", :aggregate_failures do
+            expect(json_response).to include({ 'error_code' => 'INVALID_PARAMETER_VALUE' })
+            expect(model.metadata.count).to be 0
+          end
+        end
+
+        # TODO: Ensure consisted error responses https://gitlab.com/gitlab-org/gitlab/-/issues/429731
+        context 'when an empty tag name is supplied' do
+          let(:params) do
+            { name: model_name, tags: [{ key: '', value: 'value1' }, { key: 'key1', value: 'value2' }] }
+          end
+
+          it "returns a validation error", :aggregate_failures do
+            expect(json_response).to include({ 'error_code' => 'INVALID_PARAMETER_VALUE' })
+            expect(model.metadata.count).to be 0
+          end
+        end
       end
 
       it_behaves_like 'MLflow|an authenticated resource'
diff --git a/spec/services/ml/create_model_version_service_spec.rb b/spec/services/ml/create_model_version_service_spec.rb
index b3aead4a92c..f9025fe8edb 100644
--- a/spec/services/ml/create_model_version_service_spec.rb
+++ b/spec/services/ml/create_model_version_service_spec.rb
@@ -75,5 +75,36 @@ RSpec.describe ::Ml::CreateModelVersionService, feature_category: :mlops do
       expect(model.reload.latest_version.package.name).to eq(model.name)
       expect(model.latest_version.package.version).to eq(model.latest_version.version)
     end
+
+    context 'when metadata are supplied, add them as metadata' do
+      let(:metadata) { [{ key: 'key1', value: 'value1' }, { key: 'key2', value: 'value2' }] }
+      let(:params) { { metadata: metadata } }
+
+      it 'creates metadata records', :aggregate_failures do
+        expect { service }.to change { Ml::ModelVersion.count }.by(1)
+
+        expect(service.metadata.count).to be 2
+      end
+    end
+
+    # TODO: Ensure consisted error responses https://gitlab.com/gitlab-org/gitlab/-/issues/429731
+    context 'for metadata with duplicate keys, it does not create duplicate records' do
+      let(:metadata) { [{ key: 'key1', value: 'value1' }, { key: 'key1', value: 'value2' }] }
+      let(:params) { { metadata: metadata } }
+
+      it 'raises an error', :aggregate_failures do
+        expect { service }.to raise_error(ActiveRecord::RecordInvalid)
+      end
+    end
+
+    # # TODO: Ensure consisted error responses https://gitlab.com/gitlab-org/gitlab/-/issues/429731
+    context 'for metadata with invalid keys, it does not create invalid records' do
+      let(:metadata) { [{ key: 'key1', value: 'value1' }, { key: '', value: 'value2' }] }
+      let(:params) { { metadata: metadata } }
+
+      it 'raises an error', :aggregate_failures do
+        expect { service }.to raise_error(ActiveRecord::RecordInvalid)
+      end
+    end
   end
 end