From 6902848a9c54f9eb1bfd82fe173ad0d5d62fe2d5 Mon Sep 17 00:00:00 2001
From: Markus Koller <markus-koller@gmx.ch>
Date: Mon, 18 Sep 2017 15:03:24 +0200
Subject: [PATCH] Support custom attributes on projects

---
 app/finders/projects_finder.rb                |  3 +++
 app/models/project.rb                         |  1 +
 app/models/project_custom_attribute.rb        |  6 +++++
 ...111708_create_project_custom_attributes.rb | 15 +++++++++++++
 db/schema.rb                                  | 12 ++++++++++
 doc/api/custom_attributes.md                  | 22 ++++++++++++-------
 doc/api/projects.md                           |  6 +++++
 lib/api/helpers.rb                            |  1 +
 lib/api/projects.rb                           |  2 ++
 lib/gitlab/import_export/import_export.yml    |  1 +
 lib/gitlab/import_export/relation_factory.rb  |  3 ++-
 spec/factories/project_custom_attributes.rb   |  7 ++++++
 spec/lib/gitlab/import_export/all_models.yml  |  1 +
 spec/lib/gitlab/import_export/project.json    | 20 ++++++++++++++++-
 .../project_tree_restorer_spec.rb             |  4 ++++
 .../import_export/project_tree_saver_spec.rb  |  7 ++++++
 .../import_export/safe_model_attributes.yml   |  9 +++++++-
 spec/models/project_custom_attribute_spec.rb  | 16 ++++++++++++++
 spec/models/project_spec.rb                   |  1 +
 spec/requests/api/projects_spec.rb            |  5 +++++
 spec/requests/api/users_spec.rb               |  3 ++-
 .../api/custom_attributes_shared_examples.rb  |  5 ++++-
 22 files changed, 137 insertions(+), 13 deletions(-)
 create mode 100644 app/models/project_custom_attribute.rb
 create mode 100644 db/migrate/20170918111708_create_project_custom_attributes.rb
 create mode 100644 spec/factories/project_custom_attributes.rb
 create mode 100644 spec/models/project_custom_attribute_spec.rb

diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb
index eac6095d8dc..005612ededc 100644
--- a/app/finders/projects_finder.rb
+++ b/app/finders/projects_finder.rb
@@ -18,6 +18,8 @@
 #     non_archived: boolean
 #
 class ProjectsFinder < UnionFinder
+  include CustomAttributesFilter
+
   attr_accessor :params
   attr_reader :current_user, :project_ids_relation
 
@@ -44,6 +46,7 @@ class ProjectsFinder < UnionFinder
     collection = by_tags(collection)
     collection = by_search(collection)
     collection = by_archived(collection)
+    collection = by_custom_attributes(collection)
 
     sort(collection)
   end
diff --git a/app/models/project.rb b/app/models/project.rb
index b04aec550b1..38ac7b20b05 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -213,6 +213,7 @@ class Project < ActiveRecord::Base
   has_many :active_runners, -> { active }, through: :runner_projects, source: :runner, class_name: 'Ci::Runner'
 
   has_one :auto_devops, class_name: 'ProjectAutoDevops'
+  has_many :custom_attributes, class_name: 'ProjectCustomAttribute'
 
   accepts_nested_attributes_for :variables, allow_destroy: true
   accepts_nested_attributes_for :project_feature, update_only: true
diff --git a/app/models/project_custom_attribute.rb b/app/models/project_custom_attribute.rb
new file mode 100644
index 00000000000..3f1a7b86a82
--- /dev/null
+++ b/app/models/project_custom_attribute.rb
@@ -0,0 +1,6 @@
+class ProjectCustomAttribute < ActiveRecord::Base
+  belongs_to :project
+
+  validates :project, :key, :value, presence: true
+  validates :key, uniqueness: { scope: [:project_id] }
+end
diff --git a/db/migrate/20170918111708_create_project_custom_attributes.rb b/db/migrate/20170918111708_create_project_custom_attributes.rb
new file mode 100644
index 00000000000..b5bc90ec02e
--- /dev/null
+++ b/db/migrate/20170918111708_create_project_custom_attributes.rb
@@ -0,0 +1,15 @@
+class CreateProjectCustomAttributes < ActiveRecord::Migration
+  DOWNTIME = false
+
+  def change
+    create_table :project_custom_attributes do |t|
+      t.timestamps_with_timezone null: false
+      t.references :project, null: false, foreign_key: { on_delete: :cascade }
+      t.string :key, null: false
+      t.string :value, null: false
+
+      t.index [:project_id, :key], unique: true
+      t.index [:key, :value]
+    end
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 80d8ff92d6e..76fc92e2282 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1214,6 +1214,17 @@ ActiveRecord::Schema.define(version: 20171026082505) do
 
   add_index "project_auto_devops", ["project_id"], name: "index_project_auto_devops_on_project_id", unique: true, using: :btree
 
+  create_table "project_custom_attributes", force: :cascade do |t|
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.integer "project_id", null: false
+    t.string "key", null: false
+    t.string "value", null: false
+  end
+
+  add_index "project_custom_attributes", ["key", "value"], name: "index_project_custom_attributes_on_key_and_value", using: :btree
+  add_index "project_custom_attributes", ["project_id", "key"], name: "index_project_custom_attributes_on_project_id_and_key", unique: true, using: :btree
+
   create_table "project_features", force: :cascade do |t|
     t.integer "project_id"
     t.integer "merge_requests_access_level"
@@ -1859,6 +1870,7 @@ ActiveRecord::Schema.define(version: 20171026082505) do
   add_foreign_key "project_authorizations", "projects", on_delete: :cascade
   add_foreign_key "project_authorizations", "users", on_delete: :cascade
   add_foreign_key "project_auto_devops", "projects", on_delete: :cascade
+  add_foreign_key "project_custom_attributes", "projects", on_delete: :cascade
   add_foreign_key "project_features", "projects", name: "fk_18513d9b92", on_delete: :cascade
   add_foreign_key "project_group_links", "projects", name: "fk_daa8cee94c", on_delete: :cascade
   add_foreign_key "project_import_data", "projects", name: "fk_ffb9ee3a10", on_delete: :cascade
diff --git a/doc/api/custom_attributes.md b/doc/api/custom_attributes.md
index 8b26f7093ab..705f1d401cf 100644
--- a/doc/api/custom_attributes.md
+++ b/doc/api/custom_attributes.md
@@ -1,18 +1,21 @@
 # Custom Attributes API
 
 Every API call to custom attributes must be authenticated as administrator.
+Custom attributes are currently available on users and projects, which will
+be referred to as "resource" in this documentation.
 
 ## List custom attributes
 
-Get all custom attributes on a user.
+Get all custom attributes on a resource.
 
 ```
 GET /users/:id/custom_attributes
+GET /projects/:id/custom_attributes
 ```
 
 | Attribute | Type | Required | Description |
 | --------- | ---- | -------- | ----------- |
-| `id` | integer | yes | The ID of a user |
+| `id` | integer | yes | The ID of a resource |
 
 ```bash
 curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/users/42/custom_attributes
@@ -35,15 +38,16 @@ Example response:
 
 ## Single custom attribute
 
-Get a single custom attribute on a user.
+Get a single custom attribute on a resource.
 
 ```
 GET /users/:id/custom_attributes/:key
+GET /projects/:id/custom_attributes/:key
 ```
 
 | Attribute | Type | Required | Description |
 | --------- | ---- | -------- | ----------- |
-| `id` | integer | yes | The ID of a user |
+| `id` | integer | yes | The ID of a resource |
 | `key` | string | yes | The key of the custom attribute |
 
 ```bash
@@ -61,16 +65,17 @@ Example response:
 
 ## Set custom attribute
 
-Set a custom attribute on a user. The attribute will be updated if it already exists,
+Set a custom attribute on a resource. The attribute will be updated if it already exists,
 or newly created otherwise.
 
 ```
 PUT /users/:id/custom_attributes/:key
+PUT /projects/:id/custom_attributes/:key
 ```
 
 | Attribute | Type | Required | Description |
 | --------- | ---- | -------- | ----------- |
-| `id` | integer | yes | The ID of a user |
+| `id` | integer | yes | The ID of a resource |
 | `key` | string | yes | The key of the custom attribute |
 | `value` | string | yes | The value of the custom attribute |
 
@@ -89,15 +94,16 @@ Example response:
 
 ## Delete custom attribute
 
-Delete a custom attribute on a user.
+Delete a custom attribute on a resource.
 
 ```
 DELETE /users/:id/custom_attributes/:key
+DELETE /projects/:id/custom_attributes/:key
 ```
 
 | Attribute | Type | Required | Description |
 | --------- | ---- | -------- | ----------- |
-| `id` | integer | yes | The ID of a user |
+| `id` | integer | yes | The ID of a resource |
 | `key` | string | yes | The key of the custom attribute |
 
 ```bash
diff --git a/doc/api/projects.md b/doc/api/projects.md
index 07331d05231..5a403f7593a 100644
--- a/doc/api/projects.md
+++ b/doc/api/projects.md
@@ -192,6 +192,12 @@ GET /projects
 ]
 ```
 
+You can filter by [custom attributes](custom_attributes.md) with:
+
+```
+GET /projects?custom_attributes[key]=value&custom_attributes[other_key]=other_value
+```
+
 ## List user projects
 
 Get a list of visible projects for the given user. When accessed without
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 1c12166e434..5f9b94cc89c 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -328,6 +328,7 @@ module API
       finder_params[:archived] = params[:archived]
       finder_params[:search] = params[:search] if params[:search]
       finder_params[:user] = params.delete(:user) if params[:user]
+      finder_params[:custom_attributes] = params[:custom_attributes] if params[:custom_attributes]
       finder_params
     end
 
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index aab7a6c3f93..4cd7e714aa2 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -119,6 +119,8 @@ module API
     end
 
     resource :projects do
+      include CustomAttributesEndpoints
+
       desc 'Get a list of visible projects for authenticated user' do
         success Entities::BasicProjectDetails
       end
diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml
index 561779182bc..239dca4d43f 100644
--- a/lib/gitlab/import_export/import_export.yml
+++ b/lib/gitlab/import_export/import_export.yml
@@ -63,6 +63,7 @@ project_tree:
   - protected_tags:
     - :create_access_levels
   - :project_feature
+  - :custom_attributes
 
 # Only include the following attributes for the models specified.
 included_attributes:
diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb
index 469b230377d..aa35992bae4 100644
--- a/lib/gitlab/import_export/relation_factory.rb
+++ b/lib/gitlab/import_export/relation_factory.rb
@@ -17,7 +17,8 @@ module Gitlab
                     labels: :project_labels,
                     priorities: :label_priorities,
                     auto_devops: :project_auto_devops,
-                    label: :project_label }.freeze
+                    label: :project_label,
+                    custom_attributes: 'ProjectCustomAttribute' }.freeze
 
       USER_REFERENCES = %w[author_id assignee_id updated_by_id user_id created_by_id last_edited_by_id merge_user_id resolved_by_id].freeze
 
diff --git a/spec/factories/project_custom_attributes.rb b/spec/factories/project_custom_attributes.rb
new file mode 100644
index 00000000000..5eedeb86304
--- /dev/null
+++ b/spec/factories/project_custom_attributes.rb
@@ -0,0 +1,7 @@
+FactoryGirl.define do
+  factory :project_custom_attribute do
+    project
+    sequence(:key) { |n| "key#{n}" }
+    sequence(:value) { |n| "value#{n}" }
+  end
+end
diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index 6c6b9154a0a..2dba6550b5c 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -276,6 +276,7 @@ project:
 - root_of_fork_network
 - fork_network_member
 - fork_network
+- custom_attributes
 award_emoji:
 - awardable
 - user
diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json
index 9a68bbb379c..f7c90093bde 100644
--- a/spec/lib/gitlab/import_export/project.json
+++ b/spec/lib/gitlab/import_export/project.json
@@ -7408,5 +7408,23 @@
     "snippets_access_level": 20,
     "updated_at": "2016-09-23T11:58:28.000Z",
     "wiki_access_level": 20
-  }
+  },
+  "custom_attributes": [
+    {
+      "id": 1,
+      "created_at": "2017-10-19T15:36:23.466Z",
+      "updated_at": "2017-10-19T15:36:23.466Z",
+      "project_id": 5,
+      "key": "foo",
+      "value": "foo"
+    },
+    {
+      "id": 2,
+      "created_at": "2017-10-19T15:37:21.904Z",
+      "updated_at": "2017-10-19T15:37:21.904Z",
+      "project_id": 5,
+      "key": "bar",
+      "value": "bar"
+    }
+  ]
 }
diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
index 76b01b6a1ec..e4b4cf5ba85 100644
--- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
+++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
@@ -133,6 +133,10 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
         expect(@project.project_feature).not_to be_nil
       end
 
+      it 'has custom attributes' do
+        expect(@project.custom_attributes.count).to eq(2)
+      end
+
       it 'restores the correct service' do
         expect(CustomIssueTrackerService.first).not_to be_nil
       end
diff --git a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb
index 8da768ebd07..ee173afbd50 100644
--- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb
+++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb
@@ -168,6 +168,10 @@ describe Gitlab::ImportExport::ProjectTreeSaver do
         expect(project_feature["builds_access_level"]).to eq(ProjectFeature::PRIVATE)
       end
 
+      it 'has custom attributes' do
+        expect(saved_project_json['custom_attributes'].count).to eq(2)
+      end
+
       it 'does not complain about non UTF-8 characters in MR diffs' do
         ActiveRecord::Base.connection.execute("UPDATE merge_request_diffs SET st_diffs = '---\n- :diff: !binary |-\n    LS0tIC9kZXYvbnVsbAorKysgYi9pbWFnZXMvbnVjb3IucGRmCkBAIC0wLDAg\n    KzEsMTY3OSBAQAorJVBERi0xLjUNJeLjz9MNCisxIDAgb2JqDTw8L01ldGFk\n    YXR'")
 
@@ -279,6 +283,9 @@ describe Gitlab::ImportExport::ProjectTreeSaver do
     create(:event, :created, target: milestone, project: project, author: user)
     create(:service, project: project, type: 'CustomIssueTrackerService', category: 'issue_tracker')
 
+    create(:project_custom_attribute, project: project)
+    create(:project_custom_attribute, project: project)
+
     project
   end
 
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index 89d30407077..02f06f9bb0b 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -508,4 +508,11 @@ ProjectAutoDevops:
 - updated_at
 IssueAssignee:
 - user_id
-- issue_id
\ No newline at end of file
+- issue_id
+ProjectCustomAttribute:
+- id
+- created_at
+- updated_at
+- project_id
+- key
+- value
diff --git a/spec/models/project_custom_attribute_spec.rb b/spec/models/project_custom_attribute_spec.rb
new file mode 100644
index 00000000000..669de5506bc
--- /dev/null
+++ b/spec/models/project_custom_attribute_spec.rb
@@ -0,0 +1,16 @@
+require 'spec_helper'
+
+describe ProjectCustomAttribute do
+  describe 'assocations' do
+    it { is_expected.to belong_to(:project) }
+  end
+
+  describe 'validations' do
+    subject { build :project_custom_attribute }
+
+    it { is_expected.to validate_presence_of(:project) }
+    it { is_expected.to validate_presence_of(:key) }
+    it { is_expected.to validate_presence_of(:value) }
+    it { is_expected.to validate_uniqueness_of(:key).scoped_to(:project_id) }
+  end
+end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index e8588975118..88987bae4a5 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -79,6 +79,7 @@ describe Project do
     it { is_expected.to have_many(:pipeline_schedules) }
     it { is_expected.to have_many(:members_and_requesters) }
     it { is_expected.to have_one(:cluster) }
+    it { is_expected.to have_many(:custom_attributes).class_name('ProjectCustomAttribute') }
 
     context 'after initialized' do
       it "has a project_feature" do
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index e095ba2af5d..abe367d4e11 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -1856,4 +1856,9 @@ describe API::Projects do
       end
     end
   end
+
+  it_behaves_like 'custom attributes endpoints', 'projects' do
+    let(:attributable) { project }
+    let(:other_attributable) { project2 }
+  end
 end
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index 634c8dae0ba..2aeae6f9ec7 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -1880,7 +1880,8 @@ describe API::Users do
     end
   end
 
-  include_examples 'custom attributes endpoints', 'users' do
+  it_behaves_like 'custom attributes endpoints', 'users' do
     let(:attributable) { user }
+    let(:other_attributable) { admin }
   end
 end
diff --git a/spec/support/shared_examples/requests/api/custom_attributes_shared_examples.rb b/spec/support/shared_examples/requests/api/custom_attributes_shared_examples.rb
index 6bc39f2f279..4e18804b937 100644
--- a/spec/support/shared_examples/requests/api/custom_attributes_shared_examples.rb
+++ b/spec/support/shared_examples/requests/api/custom_attributes_shared_examples.rb
@@ -3,7 +3,9 @@ shared_examples 'custom attributes endpoints' do |attributable_name|
   let!(:custom_attribute2) { attributable.custom_attributes.create key: 'bar', value: 'bar' }
 
   describe "GET /#{attributable_name} with custom attributes filter" do
-    let!(:other_attributable) { create attributable.class.name.underscore }
+    before do
+      other_attributable
+    end
 
     context 'with an unauthorized user' do
       it 'does not filter by custom attributes' do
@@ -11,6 +13,7 @@ shared_examples 'custom attributes endpoints' do |attributable_name|
 
         expect(response).to have_gitlab_http_status(200)
         expect(json_response.size).to be 2
+        expect(json_response.map { |r| r['id'] }).to contain_exactly attributable.id, other_attributable.id
       end
     end