diff options
9 files changed, 48 insertions, 19 deletions
diff --git a/app/controllers/concerns/uploads_actions.rb b/app/controllers/concerns/uploads_actions.rb index 6d9ee39f841..b87779c22d3 100644 --- a/app/controllers/concerns/uploads_actions.rb +++ b/app/controllers/concerns/uploads_actions.rb @@ -34,7 +34,7 @@ module UploadsActions headers['Pragma'] = '' ttl, directives = *cache_settings - ttl ||= 6.months + ttl ||= 0 directives ||= { private: true, must_revalidate: true } expires_in ttl, directives diff --git a/changelogs/unreleased/35337-images-stopped-loading-signed-urls-of-uploads-expire-earlier-than-r.yml b/changelogs/unreleased/35337-images-stopped-loading-signed-urls-of-uploads-expire-earlier-than-r.yml new file mode 100644 index 00000000000..649a341f538 --- /dev/null +++ b/changelogs/unreleased/35337-images-stopped-loading-signed-urls-of-uploads-expire-earlier-than-r.yml @@ -0,0 +1,6 @@ +--- +title: Disable upload HTTP caching to fix case when object storage is enabled and + proxy_download is disabled +merge_request: 19494 +author: +type: fixed diff --git a/db/migrate/20190805140353_remove_rendundant_index_from_releases.rb b/db/migrate/20190805140353_remove_rendundant_index_from_releases.rb index a0dcd68e8b4..477f8a850f8 100644 --- a/db/migrate/20190805140353_remove_rendundant_index_from_releases.rb +++ b/db/migrate/20190805140353_remove_rendundant_index_from_releases.rb @@ -15,7 +15,7 @@ class RemoveRendundantIndexFromReleases < ActiveRecord::Migration[5.2] remove_concurrent_index_by_name :releases, 'index_releases_on_project_id' # This is an extra index that is not present in db/schema.rb but known to exist on some installs - remove_concurrent_index_by_name :releases, 'releases_project_id_idx' + remove_concurrent_index_by_name :releases, 'releases_project_id_idx' if index_exists_by_name?(:releases, 'releases_project_id_idx') end def down diff --git a/lib/gitlab/cycle_analytics/group_stage_summary.rb b/lib/gitlab/cycle_analytics/group_stage_summary.rb index a1fc941495d..26eaaf7df83 100644 --- a/lib/gitlab/cycle_analytics/group_stage_summary.rb +++ b/lib/gitlab/cycle_analytics/group_stage_summary.rb @@ -3,18 +3,17 @@ module Gitlab module CycleAnalytics class GroupStageSummary - attr_reader :group, :from, :current_user, :options + attr_reader :group, :current_user, :options def initialize(group, options:) @group = group - @from = options[:from] @current_user = options[:current_user] @options = options end def data - [serialize(Summary::Group::Issue.new(group: group, from: from, current_user: current_user, options: options)), - serialize(Summary::Group::Deploy.new(group: group, from: from, options: options))] + [serialize(Summary::Group::Issue.new(group: group, current_user: current_user, options: options)), + serialize(Summary::Group::Deploy.new(group: group, options: options))] end private diff --git a/lib/gitlab/cycle_analytics/summary/group/base.rb b/lib/gitlab/cycle_analytics/summary/group/base.rb index 48d8164bde1..f1d20d5aefa 100644 --- a/lib/gitlab/cycle_analytics/summary/group/base.rb +++ b/lib/gitlab/cycle_analytics/summary/group/base.rb @@ -5,11 +5,10 @@ module Gitlab module Summary module Group class Base - attr_reader :group, :from, :options + attr_reader :group, :options - def initialize(group:, from:, options:) + def initialize(group:, options:) @group = group - @from = from @options = options end diff --git a/lib/gitlab/cycle_analytics/summary/group/deploy.rb b/lib/gitlab/cycle_analytics/summary/group/deploy.rb index 78d677cf558..11a9152cf0c 100644 --- a/lib/gitlab/cycle_analytics/summary/group/deploy.rb +++ b/lib/gitlab/cycle_analytics/summary/group/deploy.rb @@ -20,7 +20,8 @@ module Gitlab def find_deployments deployments = Deployment.joins(:project).merge(Project.inside_path(group.full_path)) deployments = deployments.where(projects: { id: options[:projects] }) if options[:projects] - deployments = deployments.where("deployments.created_at > ?", from) + deployments = deployments.where("deployments.created_at > ?", options[:from]) + deployments = deployments.where("deployments.created_at < ?", options[:to]) if options[:to] deployments.success.count end end diff --git a/lib/gitlab/cycle_analytics/summary/group/issue.rb b/lib/gitlab/cycle_analytics/summary/group/issue.rb index 9daae8531d8..4d5ee1d43ca 100644 --- a/lib/gitlab/cycle_analytics/summary/group/issue.rb +++ b/lib/gitlab/cycle_analytics/summary/group/issue.rb @@ -5,11 +5,10 @@ module Gitlab module Summary module Group class Issue < Group::Base - attr_reader :group, :from, :current_user, :options + attr_reader :group, :current_user, :options - def initialize(group:, from:, current_user:, options:) + def initialize(group:, current_user:, options:) @group = group - @from = from @current_user = current_user @options = options end @@ -25,10 +24,19 @@ module Gitlab private def find_issues - issues = IssuesFinder.new(current_user, group_id: group.id, include_subgroups: true, created_after: from).execute + issues = IssuesFinder.new(current_user, finder_params).execute issues = issues.where(projects: { id: options[:projects] }) if options[:projects] issues.count end + + def finder_params + { + group_id: group.id, + include_subgroups: true, + created_after: options[:from], + created_before: options[:to] + }.compact + end end end end diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index dd7ab4f9d47..1bcf3bb106b 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -7,9 +7,9 @@ shared_examples 'content 5 min private cached with revalidation' do end end -shared_examples 'content long term private cached with revalidation' do +shared_examples 'content not cached' do it 'ensures content will not be cached without revalidation' do - expect(subject['Cache-Control']).to eq('max-age=15778476, private, must-revalidate') + expect(subject['Cache-Control']).to eq('max-age=0, private, must-revalidate') end end @@ -490,7 +490,7 @@ describe UploadsController do expect(response).to have_gitlab_http_status(200) end - it_behaves_like 'content long term private cached with revalidation' do + it_behaves_like 'content not cached' do subject do get :show, params: { model: 'note', mounted_as: 'attachment', id: note.id, filename: 'dk.png' } @@ -510,7 +510,7 @@ describe UploadsController do expect(response).to have_gitlab_http_status(200) end - it_behaves_like 'content long term private cached with revalidation' do + it_behaves_like 'content not cached' do subject do get :show, params: { model: 'note', mounted_as: 'attachment', id: note.id, filename: 'dk.png' } @@ -563,7 +563,7 @@ describe UploadsController do expect(response).to have_gitlab_http_status(200) end - it_behaves_like 'content long term private cached with revalidation' do + it_behaves_like 'content not cached' do subject do get :show, params: { model: 'note', mounted_as: 'attachment', id: note.id, filename: 'dk.png' } diff --git a/spec/lib/gitlab/cycle_analytics/group_stage_summary_spec.rb b/spec/lib/gitlab/cycle_analytics/group_stage_summary_spec.rb index d5c2f7cc579..664009f140f 100644 --- a/spec/lib/gitlab/cycle_analytics/group_stage_summary_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/group_stage_summary_spec.rb @@ -44,6 +44,14 @@ describe Gitlab::CycleAnalytics::GroupStageSummary do expect(subject.first[:value]).to eq(2) end end + + context 'when `from` and `to` parameters are provided' do + subject { described_class.new(group, options: { from: 10.days.ago, to: Time.now, current_user: user }).data } + + it 'finds issues from 5 days ago' do + expect(subject.first[:value]).to eq(2) + end + end end context 'with other projects' do @@ -97,6 +105,14 @@ describe Gitlab::CycleAnalytics::GroupStageSummary do expect(subject.second[:value]).to eq(2) end end + + context 'when `from` and `to` parameters are provided' do + subject { described_class.new(group, options: { from: 10.days.ago, to: Time.now, current_user: user }).data } + + it 'finds deployments from 5 days ago' do + expect(subject.second[:value]).to eq(2) + end + end end context 'with other projects' do |