diff --git a/app/controllers/concerns/notes_actions.rb b/app/controllers/concerns/notes_actions.rb
index 7b2cf131fce95f0678f092570cd8ff6e752256bc..4e34094b52c048841d2a7ed6b6ed661c79bfb4fd 100644
--- a/app/controllers/concerns/notes_actions.rb
+++ b/app/controllers/concerns/notes_actions.rb
@@ -11,6 +11,7 @@ module NotesActions
included do
before_action :set_polling_interval_header, only: [:index]
+ before_action :require_last_fetched_at_header!, only: [:index]
before_action :require_noteable!, only: [:index, :create]
before_action :authorize_admin_note!, only: [:update, :destroy]
before_action :note_project, only: [:create]
@@ -262,6 +263,12 @@ def require_noteable!
render_404 unless noteable
end
+ def require_last_fetched_at_header!
+ return if request.headers['X-Last-Fetched-At'].present? || Feature.disabled?(:require_notes_last_fetched_at)
+
+ render json: { message: 'X-Last-Fetched-At header is required' }, status: :bad_request
+ end
+
def last_fetched_at
microseconds = request.headers['X-Last-Fetched-At'].to_i
diff --git a/config/feature_flags/development/require_notes_last_fetched_at.yml b/config/feature_flags/development/require_notes_last_fetched_at.yml
new file mode 100644
index 0000000000000000000000000000000000000000..7ded6f16543e7edca367f1307660f193675fce4f
--- /dev/null
+++ b/config/feature_flags/development/require_notes_last_fetched_at.yml
@@ -0,0 +1,8 @@
+---
+name: require_notes_last_fetched_at
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/127763
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/419829
+milestone: '16.3'
+type: development
+group: group::project management
+default_enabled: false
diff --git a/ee/spec/controllers/groups/epics/notes_controller_spec.rb b/ee/spec/controllers/groups/epics/notes_controller_spec.rb
index f8854249e28db0b3942a5d151b21a826afab763a..493a95dc544899e5baad9b44e6267fe3de9188f9 100644
--- a/ee/spec/controllers/groups/epics/notes_controller_spec.rb
+++ b/ee/spec/controllers/groups/epics/notes_controller_spec.rb
@@ -28,6 +28,8 @@
group.add_developer(user)
sign_in(user)
note
+
+ request.headers['X-Last-Fetched-At'] = 0
end
it 'responds with array of notes' do
diff --git a/ee/spec/controllers/projects/security/vulnerabilities/notes_controller_spec.rb b/ee/spec/controllers/projects/security/vulnerabilities/notes_controller_spec.rb
index 0a59047e4398fe66b1f5ace7ebe39f2775fad539..c1b193bb4106b843b9244ab486e3e2405343fc12 100644
--- a/ee/spec/controllers/projects/security/vulnerabilities/notes_controller_spec.rb
+++ b/ee/spec/controllers/projects/security/vulnerabilities/notes_controller_spec.rb
@@ -15,6 +15,8 @@
describe 'GET index' do
subject(:view_all_notes) do
+ request.headers['X-Last-Fetched-At'] = 0
+
get :index, params: { namespace_id: project.namespace, project_id: project, vulnerability_id: vulnerability }
end
diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb
index 940f6fed906d83f121fba617f4d8feac1af660c9..3dcd6b040401fb28abf339ac436755bbd3f43879 100644
--- a/spec/controllers/projects/notes_controller_spec.rb
+++ b/spec/controllers/projects/notes_controller_spec.rb
@@ -18,7 +18,7 @@
}
end
- describe 'GET index' do
+ describe 'GET index', :freeze_time do
let(:request_params) do
{
namespace_id: project.namespace,
@@ -31,10 +31,13 @@
let(:parsed_response) { json_response.with_indifferent_access }
let(:note_json) { parsed_response[:notes].first }
+ let(:last_fetched_at) { Time.zone.at(3.hours.ago.to_i) }
before do
sign_in(user)
project.add_developer(user)
+
+ request.headers['X-Last-Fetched-At'] = microseconds(last_fetched_at)
end
specify { expect(get(:index, params: request_params)).to have_request_urgency(:medium) }
@@ -46,10 +49,6 @@
end
it 'passes last_fetched_at from headers to NotesFinder and MergeIntoNotesService' do
- last_fetched_at = Time.zone.at(3.hours.ago.to_i) # remove nanoseconds
-
- request.headers['X-Last-Fetched-At'] = microseconds(last_fetched_at)
-
expect(NotesFinder).to receive(:new)
.with(anything, hash_including(last_fetched_at: last_fetched_at))
.and_call_original
@@ -61,6 +60,28 @@
get :index, params: request_params
end
+ it 'returns status 400 when last_fetched_at is not present' do
+ request.headers['X-Last-Fetched-At'] = nil
+
+ get :index, params: request_params
+
+ expect(response).to have_gitlab_http_status(:bad_request)
+ end
+
+ context 'when require_notes_last_fetched_at is disabled' do
+ before do
+ stub_feature_flags(require_notes_last_fetched_at: false)
+ end
+
+ it 'returns status 200 when last_fetched_at is not present' do
+ request.headers['X-Last-Fetched-At'] = nil
+
+ get :index, params: request_params
+
+ expect(response).to have_gitlab_http_status(:ok)
+ end
+ end
+
context 'when user notes_filter is present' do
let(:notes_json) { parsed_response[:notes] }
let!(:comment) { create(:note, noteable: issue, project: project) }
diff --git a/spec/controllers/snippets/notes_controller_spec.rb b/spec/controllers/snippets/notes_controller_spec.rb
index 578973d5b3db526697465ecf2eae74bbd3575b3f..834415a5c87ab26d3e23e150ec2a077e3873085c 100644
--- a/spec/controllers/snippets/notes_controller_spec.rb
+++ b/spec/controllers/snippets/notes_controller_spec.rb
@@ -14,6 +14,10 @@
let(:note_on_public) { create(:note_on_personal_snippet, noteable: public_snippet) }
describe 'GET index' do
+ before do
+ request.headers['X-Last-Fetched-At'] = 0
+ end
+
context 'when a snippet is public' do
before do
note_on_public
diff --git a/spec/requests/projects/noteable_notes_spec.rb b/spec/requests/projects/noteable_notes_spec.rb
index 55540447da0633c5f0f9fc211b2c4825e4172565..a490e0596807bb283e8fd244d11073d114a43c52 100644
--- a/spec/requests/projects/noteable_notes_spec.rb
+++ b/spec/requests/projects/noteable_notes_spec.rb
@@ -14,6 +14,8 @@
let(:response_etag) { response.headers['ETag'] }
let(:stored_etag) { "W/\"#{etag_store.get(notes_path)}\"" }
+ let(:default_headers) { { 'X-Last-Fetched-At' => 0 } }
+
before do
login_as(user)
end
@@ -21,7 +23,7 @@
it 'does not set a Gitlab::EtagCaching ETag if there is a note' do
create(:note_on_merge_request, noteable: merge_request, project: merge_request.project)
- get notes_path
+ get notes_path, headers: default_headers
expect(response).to have_gitlab_http_status(:ok)
@@ -31,7 +33,7 @@
end
it 'sets a Gitlab::EtagCaching ETag if there is no note' do
- get notes_path
+ get notes_path, headers: default_headers
expect(response).to have_gitlab_http_status(:ok)
expect(response_etag).to eq(stored_etag)
@@ -68,7 +70,7 @@
)
)
- get notes_path, headers: { "if-none-match": stored_etag }
+ get notes_path, headers: default_headers.merge("if-none-match": stored_etag)
expect(response).to have_gitlab_http_status(:not_modified)
end