summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/models/concerns/atomic_internal_id.rb17
-rw-r--r--app/models/internal_id.rb36
-rw-r--r--changelogs/unreleased/1756-set-iid-via-api.yml5
-rw-r--r--doc/api/issues.md1
-rw-r--r--lib/api/issues.rb6
-rw-r--r--spec/models/internal_id_spec.rb66
-rw-r--r--spec/requests/api/issues_spec.rb32
-rw-r--r--spec/support/shared_examples/models/atomic_internal_id_spec.rb14
8 files changed, 169 insertions, 8 deletions
diff --git a/app/models/concerns/atomic_internal_id.rb b/app/models/concerns/atomic_internal_id.rb
index 4fef615e6e3..5e39676b24b 100644
--- a/app/models/concerns/atomic_internal_id.rb
+++ b/app/models/concerns/atomic_internal_id.rb
@@ -35,16 +35,21 @@ module AtomicInternalId
define_method("ensure_#{scope}_#{column}!") do
scope_value = association(scope).reader
+ value = read_attribute(column)
- if read_attribute(column).blank? && scope_value
- scope_attrs = { scope_value.class.table_name.singularize.to_sym => scope_value }
- usage = self.class.table_name.to_sym
+ return value unless scope_value
- new_iid = InternalId.generate_next(self, scope_attrs, usage, init)
- write_attribute(column, new_iid)
+ scope_attrs = { scope_value.class.table_name.singularize.to_sym => scope_value }
+ usage = self.class.table_name.to_sym
+
+ if value.present?
+ InternalId.track_greatest(self, scope_attrs, usage, value, init)
+ else
+ value = InternalId.generate_next(self, scope_attrs, usage, init)
+ write_attribute(column, value)
end
- read_attribute(column)
+ value
end
end
end
diff --git a/app/models/internal_id.rb b/app/models/internal_id.rb
index f50f28deffe..e5d0f94073c 100644
--- a/app/models/internal_id.rb
+++ b/app/models/internal_id.rb
@@ -1,6 +1,9 @@
# An InternalId is a strictly monotone sequence of integers
# generated for a given scope and usage.
#
+# The monotone sequence may be broken if an ID is explicitly provided
+# to `.track_greatest_and_save!` or `#track_greatest`.
+#
# For example, issues use their project to scope internal ids:
# In that sense, scope is "project" and usage is "issues".
# Generated internal ids for an issue are unique per project.
@@ -25,13 +28,34 @@ class InternalId < ActiveRecord::Base
# The operation locks the record and gathers a `ROW SHARE` lock (in PostgreSQL).
# As such, the increment is atomic and safe to be called concurrently.
def increment_and_save!
+ update_and_save { self.last_value = (last_value || 0) + 1 }
+ end
+
+ # Increments #last_value with new_value if it is greater than the current,
+ # and saves the record
+ #
+ # The operation locks the record and gathers a `ROW SHARE` lock (in PostgreSQL).
+ # As such, the increment is atomic and safe to be called concurrently.
+ def track_greatest_and_save!(new_value)
+ update_and_save { self.last_value = [last_value || 0, new_value].max }
+ end
+
+ private
+
+ def update_and_save(&block)
lock!
- self.last_value = (last_value || 0) + 1
+ yield
save!
last_value
end
class << self
+ def track_greatest(subject, scope, usage, new_value, init)
+ return new_value unless available?
+
+ InternalIdGenerator.new(subject, scope, usage, init).track_greatest(new_value)
+ end
+
def generate_next(subject, scope, usage, init)
# Shortcut if `internal_ids` table is not available (yet)
# This can be the case in other (unrelated) migration specs
@@ -94,6 +118,16 @@ class InternalId < ActiveRecord::Base
end
end
+ # Create a record in internal_ids if one does not yet exist
+ # and set its new_value if it is higher than the current last_value
+ #
+ # Note this will acquire a ROW SHARE lock on the InternalId record
+ def track_greatest(new_value)
+ subject.transaction do
+ (lookup || create_record).track_greatest_and_save!(new_value)
+ end
+ end
+
private
# Retrieve InternalId record for (project, usage) combination, if it exists
diff --git a/changelogs/unreleased/1756-set-iid-via-api.yml b/changelogs/unreleased/1756-set-iid-via-api.yml
new file mode 100644
index 00000000000..680a9464ab4
--- /dev/null
+++ b/changelogs/unreleased/1756-set-iid-via-api.yml
@@ -0,0 +1,5 @@
+---
+title: Allow issues API to receive an internal ID (iid) on create
+merge_request: 20626
+author: Jamie Schembri
+type: fixed
diff --git a/doc/api/issues.md b/doc/api/issues.md
index 92fb3e9c307..103eaa5655f 100644
--- a/doc/api/issues.md
+++ b/doc/api/issues.md
@@ -463,6 +463,7 @@ POST /projects/:id/issues
| Attribute | Type | Required | Description |
|-------------------------------------------|----------------|----------|--------------|
| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user |
+| `iid` | integer/string | no | The internal ID of the project's issue (requires admin or project owner rights) |
| `title` | string | yes | The title of an issue |
| `description` | string | no | The description of an issue |
| `confidential` | boolean | no | Set an issue to be confidential. Default is `false`. |
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index 25185d6edc8..bda05d1795b 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -162,6 +162,9 @@ module API
desc: 'The IID of a merge request for which to resolve discussions'
optional :discussion_to_resolve, type: String,
desc: 'The ID of a discussion to resolve, also pass `merge_request_to_resolve_discussions_of`'
+ optional :iid, type: Integer,
+ desc: 'The internal ID of a project issue. Available only for admins and project owners.'
+
use :issue_params
end
post ':id/issues' do
@@ -169,9 +172,10 @@ module API
authorize! :create_issue, user_project
- # Setting created_at time only allowed for admins and project owners
+ # Setting created_at time or iid only allowed for admins and project owners
unless current_user.admin? || user_project.owner == current_user
params.delete(:created_at)
+ params.delete(:iid)
end
issue_params = declared_params(include_missing: false)
diff --git a/spec/models/internal_id_spec.rb b/spec/models/internal_id_spec.rb
index 581fd0293cc..20600f5fa38 100644
--- a/spec/models/internal_id_spec.rb
+++ b/spec/models/internal_id_spec.rb
@@ -79,6 +79,46 @@ describe InternalId do
end
end
+ describe '.track_greatest' do
+ let(:value) { 9001 }
+ subject { described_class.track_greatest(issue, scope, usage, value, init) }
+
+ context 'in the absence of a record' do
+ it 'creates a record if not yet present' do
+ expect { subject }.to change { described_class.count }.from(0).to(1)
+ end
+ end
+
+ it 'stores record attributes' do
+ subject
+
+ described_class.first.tap do |record|
+ expect(record.project).to eq(project)
+ expect(record.usage).to eq(usage.to_s)
+ expect(record.last_value).to eq(value)
+ end
+ end
+
+ context 'with existing issues' do
+ before do
+ create(:issue, project: project)
+ described_class.delete_all
+ end
+
+ it 'still returns the last value to that of the given value' do
+ expect(subject).to eq(value)
+ end
+ end
+
+ context 'when value is less than the current last_value' do
+ it 'returns the current last_value' do
+ described_class.create!(**scope, usage: usage, last_value: 10_001)
+
+ expect(subject).to eq 10_001
+ end
+ end
+ end
+
describe '#increment_and_save!' do
let(:id) { create(:internal_id) }
subject { id.increment_and_save! }
@@ -103,4 +143,30 @@ describe InternalId do
end
end
end
+
+ describe '#track_greatest_and_save!' do
+ let(:id) { create(:internal_id) }
+ let(:new_last_value) { 9001 }
+ subject { id.track_greatest_and_save!(new_last_value) }
+
+ it 'returns new last value' do
+ expect(subject).to eq new_last_value
+ end
+
+ it 'saves the record' do
+ subject
+
+ expect(id.changed?).to be_falsey
+ end
+
+ context 'when new last value is lower than the max' do
+ it 'does not update the last value' do
+ id.update!(last_value: 10_001)
+
+ subject
+
+ expect(id.reload.last_value).to eq 10_001
+ end
+ end
+ end
end
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index 66eb18229fa..28ba00c7293 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -1002,6 +1002,38 @@ describe API::Issues do
end
end
+ context 'an internal ID is provided' do
+ context 'by an admin' do
+ it 'sets the internal ID on the new issue' do
+ post api("/projects/#{project.id}/issues", admin),
+ title: 'new issue', iid: 9001
+
+ expect(response).to have_gitlab_http_status(201)
+ expect(json_response['iid']).to eq 9001
+ end
+ end
+
+ context 'by an owner' do
+ it 'sets the internal ID on the new issue' do
+ post api("/projects/#{project.id}/issues", user),
+ title: 'new issue', iid: 9001
+
+ expect(response).to have_gitlab_http_status(201)
+ expect(json_response['iid']).to eq 9001
+ end
+ end
+
+ context 'by another user' do
+ it 'ignores the given internal ID' do
+ post api("/projects/#{project.id}/issues", user2),
+ title: 'new issue', iid: 9001
+
+ expect(response).to have_gitlab_http_status(201)
+ expect(json_response['iid']).not_to eq 9001
+ end
+ end
+ end
+
it 'creates a new project issue' do
post api("/projects/#{project.id}/issues", user),
title: 'new issue', labels: 'label, label2', weight: 3,
diff --git a/spec/support/shared_examples/models/atomic_internal_id_spec.rb b/spec/support/shared_examples/models/atomic_internal_id_spec.rb
index 7ab1041d17c..c659be8f13a 100644
--- a/spec/support/shared_examples/models/atomic_internal_id_spec.rb
+++ b/spec/support/shared_examples/models/atomic_internal_id_spec.rb
@@ -60,6 +60,20 @@ shared_examples_for 'AtomicInternalId' do |validate_presence: true|
expect { subject }.not_to change { instance.public_send(internal_id_attribute) }
end
+
+ context 'when the instance has an internal ID set' do
+ let(:internal_id) { 9001 }
+
+ it 'calls InternalId.update_last_value and sets the `last_value` to that of the instance' do
+ instance.send("#{internal_id_attribute}=", internal_id)
+
+ expect(InternalId)
+ .to receive(:track_greatest)
+ .with(instance, scope_attrs, usage, internal_id, any_args)
+ .and_return(internal_id)
+ subject
+ end
+ end
end
end
end
View Lorry Controller Status