Commit 2b8d8316 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Merge branch 'add-extra-fields-to-the-application-context' into 'master'

Add extra fields to the application context

See merge request gitlab-org/gitlab!22792
parents 1df48218 040a742e
...@@ -302,7 +302,7 @@ gem 'sentry-raven', '~> 2.9' ...@@ -302,7 +302,7 @@ gem 'sentry-raven', '~> 2.9'
gem 'premailer-rails', '~> 1.10.3' gem 'premailer-rails', '~> 1.10.3'
# LabKit: Tracing and Correlation # LabKit: Tracing and Correlation
gem 'gitlab-labkit', '0.8.0' gem 'gitlab-labkit', '0.9.1'
# I18n # I18n
gem 'ruby_parser', '~> 3.8', require: false gem 'ruby_parser', '~> 3.8', require: false
......
...@@ -367,7 +367,7 @@ GEM ...@@ -367,7 +367,7 @@ GEM
github-markup (1.7.0) github-markup (1.7.0)
gitlab-chronic (0.10.5) gitlab-chronic (0.10.5)
numerizer (~> 0.2) numerizer (~> 0.2)
gitlab-labkit (0.8.0) gitlab-labkit (0.9.1)
actionpack (>= 5.0.0, < 6.1.0) actionpack (>= 5.0.0, < 6.1.0)
activesupport (>= 5.0.0, < 6.1.0) activesupport (>= 5.0.0, < 6.1.0)
grpc (~> 1.19) grpc (~> 1.19)
...@@ -1215,7 +1215,7 @@ DEPENDENCIES ...@@ -1215,7 +1215,7 @@ DEPENDENCIES
gitaly (~> 1.81.0) gitaly (~> 1.81.0)
github-markup (~> 1.7.0) github-markup (~> 1.7.0)
gitlab-chronic (~> 0.10.5) gitlab-chronic (~> 0.10.5)
gitlab-labkit (= 0.8.0) gitlab-labkit (= 0.9.1)
gitlab-license (~> 1.0) gitlab-license (~> 1.0)
gitlab-markup (~> 1.7.0) gitlab-markup (~> 1.7.0)
gitlab-net-dns (~> 0.9.1) gitlab-net-dns (~> 0.9.1)
......
...@@ -454,6 +454,7 @@ class ApplicationController < ActionController::Base ...@@ -454,6 +454,7 @@ class ApplicationController < ActionController::Base
user: -> { auth_user }, user: -> { auth_user },
project: -> { @project }, project: -> { @project },
namespace: -> { @group }, namespace: -> { @group },
caller_id: full_action_name,
&block) &block)
end end
...@@ -551,6 +552,10 @@ class ApplicationController < ActionController::Base ...@@ -551,6 +552,10 @@ class ApplicationController < ActionController::Base
end end
end end
def full_action_name
"#{self.class.name}##{action_name}"
end
# A user requires a role and have the setup_for_company attribute set when they are part of the experimental signup # A user requires a role and have the setup_for_company attribute set when they are part of the experimental signup
# flow (executed by the Growth team). Users are redirected to the welcome page when their role is required and the # flow (executed by the Growth team). Users are redirected to the welcome page when their role is required and the
# experiment is enabled for the current user. # experiment is enabled for the current user.
......
---
title: Add extra fields to the application context
merge_request: 22792
author:
type: added
...@@ -371,6 +371,9 @@ module EE ...@@ -371,6 +371,9 @@ module EE
end end
def generate_subscription def generate_subscription
return unless persisted?
return if ::Gitlab::Database.read_only?
create_gitlab_subscription( create_gitlab_subscription(
plan_code: plan&.name, plan_code: plan&.name,
trial: trial_active?, trial: trial_active?,
......
...@@ -160,7 +160,7 @@ module EE ...@@ -160,7 +160,7 @@ module EE
delegate :merge_pipelines_enabled, :merge_pipelines_enabled=, :merge_pipelines_enabled?, :merge_pipelines_were_disabled?, to: :ci_cd_settings delegate :merge_pipelines_enabled, :merge_pipelines_enabled=, :merge_pipelines_enabled?, :merge_pipelines_were_disabled?, to: :ci_cd_settings
delegate :merge_trains_enabled?, to: :ci_cd_settings delegate :merge_trains_enabled?, to: :ci_cd_settings
delegate :actual_limits, to: :namespace, allow_nil: true delegate :actual_limits, :actual_plan_name, to: :namespace, allow_nil: true
validates :repository_size_limit, validates :repository_size_limit,
numericality: { only_integer: true, greater_than_or_equal_to: 0, allow_nil: true } numericality: { only_integer: true, greater_than_or_equal_to: 0, allow_nil: true }
......
# frozen_string_literal: true
module EE
module Gitlab
module ApplicationContext
extend ::Gitlab::Utils::Override
override :to_lazy_hash
def to_lazy_hash
super.tap do |hash|
hash[:subscription_plan] = -> { subcription_plan_name } if include_namespace?
end
end
def subcription_plan_name
object = namespace || project
object&.actual_plan_name
end
end
end
end
...@@ -27,7 +27,7 @@ module EE ...@@ -27,7 +27,7 @@ module EE
retry_optimistic_lock(pipeline) do retry_optimistic_lock(pipeline) do
pipeline.drop!(:activity_limit_exceeded) pipeline.drop!(:activity_limit_exceeded)
limit.log_error!(project_id: project.id, plan: project.namespace.actual_plan_name) limit.log_error!(project_id: project.id, plan: project.actual_plan_name)
end end
end end
......
...@@ -27,7 +27,7 @@ module EE ...@@ -27,7 +27,7 @@ module EE
retry_optimistic_lock(pipeline) do retry_optimistic_lock(pipeline) do
pipeline.drop!(:job_activity_limit_exceeded) pipeline.drop!(:job_activity_limit_exceeded)
limit.log_error!(project_id: project.id, plan: project.namespace.actual_plan_name) limit.log_error!(project_id: project.id, plan: project.actual_plan_name)
end end
end end
......
...@@ -28,7 +28,7 @@ module EE ...@@ -28,7 +28,7 @@ module EE
pipeline.drop!(:size_limit_exceeded) pipeline.drop!(:size_limit_exceeded)
end end
limit.log_error!(project_id: project.id, plan: project.namespace.actual_plan_name) limit.log_error!(project_id: project.id, plan: project.actual_plan_name)
error(limit.message) error(limit.message)
end end
......
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::ApplicationContext do
describe '#to_lazy_hash' do
let(:user) { build(:user) }
let(:project) { build(:project) }
let(:namespace) { create(:group) }
let(:subgroup) { create(:group, parent: namespace) }
def result(context)
context.to_lazy_hash.transform_values { |v| v.respond_to?(:call) ? v.call : v }
end
it 'correctly loads the expected values' do
context = described_class.new(namespace: -> { subgroup })
expect(result(context))
.to include(root_namespace: namespace.full_path,
subscription_plan: 'free')
end
it 'falls back to a projects namespace plan when a project is passed but no namespace' do
create(:gitlab_subscription, :silver, namespace: project.namespace)
context = described_class.new(project: project)
expect(result(context))
.to include(project: project.full_path,
root_namespace: project.full_path_components.first,
subscription_plan: 'silver')
end
end
context 'only include values for which an option was specified' do
using RSpec::Parameterized::TableSyntax
where(:provided_options, :expected_context_keys) do
[:user, :namespace, :project] | [:user, :project, :root_namespace, :subscription_plan]
[:user, :project] | [:user, :project, :root_namespace, :subscription_plan]
[:user, :namespace] | [:user, :root_namespace, :subscription_plan]
[:user] | [:user]
[:caller_id] | [:caller_id]
[] | []
end
with_them do
it do
# Build a hash that has all `provided_options` as keys, and `nil` as value
provided_values = provided_options.map { |key| [key, nil] }.to_h
context = described_class.new(provided_values)
expect(context.to_lazy_hash.keys).to contain_exactly(*expected_context_keys)
end
end
end
end
...@@ -54,4 +54,46 @@ describe Namespace do ...@@ -54,4 +54,46 @@ describe Namespace do
expect(namespace.use_elasticsearch?).to eq(true) expect(namespace.use_elasticsearch?).to eq(true)
end end
end end
describe '#actual_plan_name' do
let(:namespace) { create(:namespace, plan: :gold_plan) }
subject { namespace.actual_plan_name }
context 'when DB is read-only' do
before do
expect(Gitlab::Database).to receive(:read_only?) { true }
end
it 'returns free plan' do
is_expected.to eq('free')
end
it 'does not create a gitlab_subscription' do
expect { subject }.not_to change(GitlabSubscription, :count)
end
end
context 'when namespace is not persisted' do
let(:namespace) { build(:namespace, plan: :gold_plan) }
it 'returns free plan' do
is_expected.to eq('free')
end
it 'does not create a gitlab_subscription' do
expect { subject }.not_to change(GitlabSubscription, :count)
end
end
context 'when DB is not read-only' do
it 'returns gold plan' do
is_expected.to eq('gold')
end
it 'creates a gitlab_subscription' do
expect { subject }.to change(GitlabSubscription, :count).by(1)
end
end
end
end end
...@@ -31,7 +31,7 @@ describe UpdateAllMirrorsWorker do ...@@ -31,7 +31,7 @@ describe UpdateAllMirrorsWorker do
inner_context = nil inner_context = nil
outer_context = nil outer_context = nil
Gitlab::ApplicationContext.with_context(project: build_stubbed(:project)) do Gitlab::ApplicationContext.with_context(project: build(:project)) do
outer_context = Labkit::Context.current.to_h outer_context = Labkit::Context.current.to_h
expect(worker).to receive(:schedule_mirrors!) do expect(worker).to receive(:schedule_mirrors!) do
......
...@@ -47,7 +47,8 @@ module API ...@@ -47,7 +47,8 @@ module API
Gitlab::ApplicationContext.push( Gitlab::ApplicationContext.push(
user: -> { current_user }, user: -> { current_user },
project: -> { @project }, project: -> { @project },
namespace: -> { @group } namespace: -> { @group },
caller_id: route.origin
) )
end end
......
...@@ -9,7 +9,8 @@ module API ...@@ -9,7 +9,8 @@ module API
before do before do
Gitlab::ApplicationContext.push( Gitlab::ApplicationContext.push(
user: -> { actor&.user }, user: -> { actor&.user },
project: -> { project } project: -> { project },
caller_id: route.origin
) )
end end
......
...@@ -5,12 +5,13 @@ module Gitlab ...@@ -5,12 +5,13 @@ module Gitlab
class ApplicationContext class ApplicationContext
include Gitlab::Utils::LazyAttributes include Gitlab::Utils::LazyAttributes
Attribute = Struct.new(:name, :type) Attribute = Struct.new(:name, :type, :evaluation)
APPLICATION_ATTRIBUTES = [ APPLICATION_ATTRIBUTES = [
Attribute.new(:project, Project), Attribute.new(:project, Project),
Attribute.new(:namespace, Namespace), Attribute.new(:namespace, Namespace),
Attribute.new(:user, User) Attribute.new(:user, User),
Attribute.new(:caller_id, String)
].freeze ].freeze
def self.with_context(args, &block) def self.with_context(args, &block)
...@@ -37,6 +38,7 @@ module Gitlab ...@@ -37,6 +38,7 @@ module Gitlab
hash[:user] = -> { username } if set_values.include?(:user) hash[:user] = -> { username } if set_values.include?(:user)
hash[:project] = -> { project_path } if set_values.include?(:project) hash[:project] = -> { project_path } if set_values.include?(:project)
hash[:root_namespace] = -> { root_namespace_path } if include_namespace? hash[:root_namespace] = -> { root_namespace_path } if include_namespace?
hash[:caller_id] = caller_id if set_values.include?(:caller_id)
end end
end end
...@@ -75,3 +77,5 @@ module Gitlab ...@@ -75,3 +77,5 @@ module Gitlab
end end
end end
end end
Gitlab::ApplicationContext.prepend_if_ee('EE::Gitlab::ApplicationContext')
...@@ -924,7 +924,7 @@ describe ApplicationController do ...@@ -924,7 +924,7 @@ describe ApplicationController do
end end
it 'sets the group if it was available' do it 'sets the group if it was available' do
group = build_stubbed(:group) group = build(:group)
controller.instance_variable_set(:@group, group) controller.instance_variable_set(:@group, group)
get :index, format: :json get :index, format: :json
...@@ -933,12 +933,18 @@ describe ApplicationController do ...@@ -933,12 +933,18 @@ describe ApplicationController do
end end
it 'sets the project if one was available' do it 'sets the project if one was available' do
project = build_stubbed(:project) project = build(:project)
controller.instance_variable_set(:@project, project) controller.instance_variable_set(:@project, project)
get :index, format: :json get :index, format: :json
expect(json_response['meta.project']).to eq(project.full_path) expect(json_response['meta.project']).to eq(project.full_path)
end end
it 'sets the caller_id as controller#action' do
get :index, format: :json
expect(json_response['meta.caller_id']).to eq('AnonymousController#index')
end
end end
end end
...@@ -43,11 +43,11 @@ describe Gitlab::ApplicationContext do ...@@ -43,11 +43,11 @@ describe Gitlab::ApplicationContext do
describe '#to_lazy_hash' do describe '#to_lazy_hash' do
let(:user) { build(:user) } let(:user) { build(:user) }
let(:project) { build(:project) } let(:project) { build(:project) }
let(:namespace) { build(:group) } let(:namespace) { create(:group) }
let(:subgroup) { build(:group, parent: namespace) } let(:subgroup) { create(:group, parent: namespace) }
def result(context) def result(context)
context.to_lazy_hash.transform_values { |v| v.call } context.to_lazy_hash.transform_values { |v| v.respond_to?(:call) ? v.call : v }
end end
it 'does not call the attributes until needed' do it 'does not call the attributes until needed' do
...@@ -78,27 +78,5 @@ describe Gitlab::ApplicationContext do ...@@ -78,27 +78,5 @@ describe Gitlab::ApplicationContext do
expect(result(context)) expect(result(context))
.to include(project: project.full_path, root_namespace: project.full_path_components.first) .to include(project: project.full_path, root_namespace: project.full_path_components.first)
end end
context 'only include values for which an option was specified' do
using RSpec::Parameterized::TableSyntax
where(:provided_options, :expected_context_keys) do
[:user, :namespace, :project] | [:user, :project, :root_namespace]
[:user, :project] | [:user, :project, :root_namespace]
[:user, :namespace] | [:user, :root_namespace]
[:user] | [:user]
[] | []
end
with_them do
it do
# Build a hash that has all `provided_options` as keys, and `nil` as value
provided_values = provided_options.map { |key| [key, nil] }.to_h
context = described_class.new(provided_values)
expect(context.to_lazy_hash.keys).to contain_exactly(*expected_context_keys)
end
end
end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment