Commit d121e73b authored by Bob Van Landuyt's avatar Bob Van Landuyt Committed by Mayra Cabrera

Don't pass undefined context-values to Labkit

Don't pass values on to labkit that weren't explicityly
defined. Otherwise, labkit will unintentionally overwrite any existing
values with nil.

Consider the following example:

  ApplicationContext.with_context(user: <User "jane-doe">) do
    ApplicationContext.with_context(
      project: <Project "gitlab-org/gitlab">
    ) do
      # The inner context lives here
    end
  end

We want the inner context for Labkit to contain this hash:

  {
    user: "jane-doe",
    project: "gitlab-org/gitlab",
    root_namespace: "gitlab-org"
  }

But before this change, we'd overwrite the `user` value when creating
the inner context, even though the inner context did not contain any
information that should override the user info.
parent 9834e063
...@@ -5,6 +5,14 @@ module Gitlab ...@@ -5,6 +5,14 @@ module Gitlab
class ApplicationContext class ApplicationContext
include Gitlab::Utils::LazyAttributes include Gitlab::Utils::LazyAttributes
Attribute = Struct.new(:name, :type)
APPLICATION_ATTRIBUTES = [
Attribute.new(:project, Project),
Attribute.new(:namespace, Namespace),
Attribute.new(:user, User)
].freeze
def self.with_context(args, &block) def self.with_context(args, &block)
application_context = new(**args) application_context = new(**args)
Labkit::Context.with_context(application_context.to_lazy_hash, &block) Labkit::Context.with_context(application_context.to_lazy_hash, &block)
...@@ -15,21 +23,36 @@ module Gitlab ...@@ -15,21 +23,36 @@ module Gitlab
Labkit::Context.push(application_context.to_lazy_hash) Labkit::Context.push(application_context.to_lazy_hash)
end end
def initialize(user: nil, project: nil, namespace: nil) def initialize(**args)
@user, @project, @namespace = user, project, namespace unknown_attributes = args.keys - APPLICATION_ATTRIBUTES.map(&:name)
raise ArgumentError, "#{unknown_attributes} are not known keys" if unknown_attributes.any?
@set_values = args.keys
assign_attributes(args)
end end
def to_lazy_hash def to_lazy_hash
{ user: -> { username }, {}.tap do |hash|
project: -> { project_path }, hash[:user] = -> { username } if set_values.include?(:user)
root_namespace: -> { root_namespace_path } } hash[:project] = -> { project_path } if set_values.include?(:project)
hash[:root_namespace] = -> { root_namespace_path } if include_namespace?
end
end end
private private
lazy_attr_reader :user, type: User attr_reader :set_values
lazy_attr_reader :project, type: Project
lazy_attr_reader :namespace, type: Namespace APPLICATION_ATTRIBUTES.each do |attr|
lazy_attr_reader attr.name, type: attr.type
end
def assign_attributes(values)
values.slice(*APPLICATION_ATTRIBUTES.map(&:name)).each do |name, value|
instance_variable_set("@#{name}", value)
end
end
def project_path def project_path
project&.full_path project&.full_path
...@@ -46,5 +69,9 @@ module Gitlab ...@@ -46,5 +69,9 @@ module Gitlab
project&.full_path_components&.first project&.full_path_components&.first
end end
end end
def include_namespace?
set_values.include?(:namespace) || set_values.include?(:project)
end
end end
end end
...@@ -28,7 +28,7 @@ describe Gitlab::ApplicationContext do ...@@ -28,7 +28,7 @@ describe Gitlab::ApplicationContext do
describe '.push' do describe '.push' do
it 'passes the expected context on to labkit' do it 'passes the expected context on to labkit' do
fake_proc = duck_type(:call) fake_proc = duck_type(:call)
expected_context = hash_including(user: fake_proc, project: fake_proc, root_namespace: fake_proc) expected_context = { user: fake_proc }
expect(Labkit::Context).to receive(:push).with(expected_context) expect(Labkit::Context).to receive(:push).with(expected_context)
...@@ -78,5 +78,27 @@ describe Gitlab::ApplicationContext do ...@@ -78,5 +78,27 @@ 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