Commit 9b1e474a authored by Dylan Griffith's avatar Dylan Griffith

Merge branch '233844-elasticsearch-named-queries' into 'master'

Add `_name` to ES queries to improve readability

See merge request gitlab-org/gitlab!42021
parents 34761ab0 6a1017b4
...@@ -99,6 +99,7 @@ gem 'graphiql-rails', '~> 1.4.10' ...@@ -99,6 +99,7 @@ gem 'graphiql-rails', '~> 1.4.10'
gem 'apollo_upload_server', '~> 2.0.2' gem 'apollo_upload_server', '~> 2.0.2'
gem 'graphql-docs', '~> 1.6.0', group: [:development, :test] gem 'graphql-docs', '~> 1.6.0', group: [:development, :test]
gem 'hashie'
# Disable strong_params so that Mash does not respond to :permitted? # Disable strong_params so that Mash does not respond to :permitted?
gem 'hashie-forbidden_attributes' gem 'hashie-forbidden_attributes'
......
...@@ -1353,6 +1353,7 @@ DEPENDENCIES ...@@ -1353,6 +1353,7 @@ DEPENDENCIES
haml_lint (~> 0.34.0) haml_lint (~> 0.34.0)
hamlit (~> 2.11.0) hamlit (~> 2.11.0)
hangouts-chat (~> 0.0.5) hangouts-chat (~> 0.0.5)
hashie
hashie-forbidden_attributes hashie-forbidden_attributes
health_check (~> 3.0) health_check (~> 3.0)
hipchat (~> 1.5.0) hipchat (~> 1.5.0)
......
...@@ -5,6 +5,7 @@ module Elastic ...@@ -5,6 +5,7 @@ module Elastic
class ApplicationClassProxy < Elasticsearch::Model::Proxy::ClassMethodsProxy class ApplicationClassProxy < Elasticsearch::Model::Proxy::ClassMethodsProxy
include ClassProxyUtil include ClassProxyUtil
include Elastic::Latest::Routing include Elastic::Latest::Routing
include Elastic::Latest::QueryContext::Aware
def search(query, search_options = {}) def search(query, search_options = {})
es_options = routing_options(search_options) es_options = routing_options(search_options)
...@@ -55,13 +56,19 @@ module Elastic ...@@ -55,13 +56,19 @@ module Elastic
bool: { bool: {
must: [{ must: [{
simple_query_string: { simple_query_string: {
_name: context.name(self.es_type, :match, :search_terms),
fields: fields, fields: fields,
query: query, query: query,
default_operator: default_operator default_operator: default_operator
} }
}], }],
filter: [{ filter: [{
term: { type: self.es_type } term: {
type: {
_name: context.name(:doc, :is_a, self.es_type),
value: self.es_type
}
}
}] }]
} }
} }
...@@ -87,8 +94,8 @@ module Elastic ...@@ -87,8 +94,8 @@ module Elastic
query: { query: {
bool: { bool: {
filter: [ filter: [
{ term: { iid: iid } }, { term: { iid: { _name: context.name(self.es_type, :related, :iid), value: iid } } },
{ term: { type: self.es_type } } { term: { type: { _name: context.name(:doc, :is_a, self.es_type), value: self.es_type } } }
] ]
} }
} }
...@@ -98,22 +105,25 @@ module Elastic ...@@ -98,22 +105,25 @@ module Elastic
# Builds an elasticsearch query that will select child documents from a # Builds an elasticsearch query that will select child documents from a
# set of projects, taking user access rules into account. # set of projects, taking user access rules into account.
def project_ids_filter(query_hash, options) def project_ids_filter(query_hash, options)
project_query = project_ids_query( context.name(:project) do
options[:current_user], project_query = project_ids_query(
options[:project_ids], options[:current_user],
options[:public_and_internal_projects], options[:project_ids],
options[:features] options[:public_and_internal_projects],
) options[:features]
)
query_hash[:query][:bool][:filter] ||= []
query_hash[:query][:bool][:filter] << { query_hash[:query][:bool][:filter] ||= []
has_parent: { query_hash[:query][:bool][:filter] << {
parent_type: "project", has_parent: {
query: { _name: context.name,
bool: project_query parent_type: "project",
query: {
bool: project_query
}
} }
} }
} end
query_hash query_hash
end end
...@@ -153,17 +163,19 @@ module Elastic ...@@ -153,17 +163,19 @@ module Elastic
conditions = pick_projects_by_membership(scoped_project_ids, user, features) conditions = pick_projects_by_membership(scoped_project_ids, user, features)
if public_and_internal_projects if public_and_internal_projects
# Skip internal projects for anonymous and external users. context.name(:visibility) do
# Others are given access to all internal projects. # Skip internal projects for anonymous and external users.
# # Others are given access to all internal projects.
# Admins & auditors get access to internal projects even #
# if the feature is private. # Admins & auditors get access to internal projects even
conditions += pick_projects_by_visibility(Project::INTERNAL, user, features) if user && !user.external? # if the feature is private.
conditions += pick_projects_by_visibility(Project::INTERNAL, user, features) if user && !user.external?
# All users, including anonymous, can access public projects.
# Admins & auditors get access to public projects where the feature is # All users, including anonymous, can access public projects.
# private. # Admins & auditors get access to public projects where the feature is
conditions += pick_projects_by_visibility(Project::PUBLIC, user, features) # private.
conditions += pick_projects_by_visibility(Project::PUBLIC, user, features)
end
end end
{ should: conditions } { should: conditions }
...@@ -179,24 +191,32 @@ module Elastic ...@@ -179,24 +191,32 @@ module Elastic
def pick_projects_by_membership(project_ids, user, features = nil) def pick_projects_by_membership(project_ids, user, features = nil)
if features.nil? if features.nil?
if project_ids == :any if project_ids == :any
return [{ term: { visibility_level: Project::PRIVATE } }] return [{ term: { visibility_level: { _name: context.name(:any), value: Project::PRIVATE } } }]
else else
return [{ terms: { id: project_ids } }] return [{ terms: { _name: context.name(:membership, :id), id: project_ids } }]
end end
end end
Array(features).map do |feature| Array(features).map do |feature|
condition = condition =
if project_ids == :any if project_ids == :any
{ term: { visibility_level: Project::PRIVATE } } { term: { visibility_level: { _name: context.name(:any), value: Project::PRIVATE } } }
else else
{ terms: { id: filter_ids_by_feature(project_ids, user, feature) } } { terms: { _name: context.name(:membership, :id), id: filter_ids_by_feature(project_ids, user, feature) } }
end end
limit = limit = {
{ terms: { "#{feature}_access_level" => [::ProjectFeature::ENABLED, ::ProjectFeature::PRIVATE] } } terms: {
_name: context.name(feature, :enabled_or_private),
"#{feature}_access_level" => [::ProjectFeature::ENABLED, ::ProjectFeature::PRIVATE]
}
}
{ bool: { filter: [condition, limit] } } {
bool: {
filter: [condition, limit]
}
}
end end
end end
...@@ -205,9 +225,11 @@ module Elastic ...@@ -205,9 +225,11 @@ module Elastic
# If a project feature is specified, access is only granted if the feature # If a project feature is specified, access is only granted if the feature
# is enabled or, for admins & auditors, private. # is enabled or, for admins & auditors, private.
def pick_projects_by_visibility(visibility, user, features) def pick_projects_by_visibility(visibility, user, features)
condition = { term: { visibility_level: visibility } } context.name(visibility) do
condition = { term: { visibility_level: { _name: context.name, value: visibility } } }
limit_by_feature(condition, features, include_members_only: user&.can_read_all_resources?) limit_by_feature(condition, features, include_members_only: user&.can_read_all_resources?)
end
end end
# If a project feature(s) is specified, access is dependent on its visibility # If a project feature(s) is specified, access is dependent on its visibility
...@@ -225,14 +247,33 @@ module Elastic ...@@ -225,14 +247,33 @@ module Elastic
features = Array(features) features = Array(features)
features.map do |feature| features.map do |feature|
limit = context.name(feature, :access_level) do
if include_members_only limit =
{ terms: { "#{feature}_access_level" => [::ProjectFeature::ENABLED, ::ProjectFeature::PRIVATE] } } if include_members_only
else {
{ term: { "#{feature}_access_level" => ::ProjectFeature::ENABLED } } terms: {
end _name: context.name(:enabled_or_private),
"#{feature}_access_level" => [::ProjectFeature::ENABLED, ::ProjectFeature::PRIVATE]
}
}
else
{
term: {
"#{feature}_access_level" => {
_name: context.name(:enabled),
value: ::ProjectFeature::ENABLED
}
}
}
end
{ bool: { filter: [condition, limit] } } {
bool: {
_name: context.name,
filter: [condition, limit]
}
}
end
end end
end end
......
...@@ -43,8 +43,25 @@ module Elastic ...@@ -43,8 +43,25 @@ module Elastic
languages = [options[:language]].flatten languages = [options[:language]].flatten
filters = [] filters = []
filters << { terms: { "#{type}.rid" => repository_ids } } if repository_ids.any?
filters << { terms: { "#{type}.language" => languages } } if languages.any? if repository_ids.any?
filters << {
terms: {
_name: context.name(type, :related, :repositories),
"#{type}.rid" => repository_ids
}
}
end
if languages.any?
filters << {
terms: {
_name: context.name(type, :match, :languages),
"#{type}.language" => languages
}
}
end
filters << options[:additional_filter] if options[:additional_filter] filters << options[:additional_filter] if options[:additional_filter]
{ filter: filters } { filter: filters }
...@@ -55,7 +72,8 @@ module Elastic ...@@ -55,7 +72,8 @@ module Elastic
fields = %w(message^10 sha^5 author.name^2 author.email^2 committer.name committer.email).map {|i| "commit.#{i}"} fields = %w(message^10 sha^5 author.name^2 author.email^2 committer.name committer.email).map {|i| "commit.#{i}"}
query_with_prefix = query.split(/\s+/).map { |s| s.gsub(SHA_REGEX) { |sha| "#{sha}*" } }.join(' ') query_with_prefix = query.split(/\s+/).map { |s| s.gsub(SHA_REGEX) { |sha| "#{sha}*" } }.join(' ')
bool_expr = Gitlab::Elastic::BoolExpr.new bool_expr = ::Gitlab::Elastic::BoolExpr.new
query_hash = { query_hash = {
query: { bool: bool_expr }, query: { bool: bool_expr },
size: per, size: per,
...@@ -67,7 +85,7 @@ module Elastic ...@@ -67,7 +85,7 @@ module Elastic
# we need to do a project visibility check. # we need to do a project visibility check.
# #
# Note that `:current_user` might be `nil` for a anonymous user # Note that `:current_user` might be `nil` for a anonymous user
query_hash = project_ids_filter(query_hash, options) if options.key?(:current_user) query_hash = context.name(:commit, :authorized) { project_ids_filter(query_hash, options) } if options.key?(:current_user)
if query.blank? if query.blank?
bool_expr[:must] = { match_all: {} } bool_expr[:must] = { match_all: {} }
...@@ -75,6 +93,7 @@ module Elastic ...@@ -75,6 +93,7 @@ module Elastic
else else
bool_expr[:must] = { bool_expr[:must] = {
simple_query_string: { simple_query_string: {
_name: context.name(:commit, :match, :search_terms),
fields: fields, fields: fields,
query: query_with_prefix, query: query_with_prefix,
default_operator: :and default_operator: :and
...@@ -83,7 +102,14 @@ module Elastic ...@@ -83,7 +102,14 @@ module Elastic
end end
# add the document type filter # add the document type filter
bool_expr[:filter] << { term: { type: 'commit' } } bool_expr[:filter] << {
term: {
type: {
_name: context.name(:doc, :is_a, :commit),
value: 'commit'
}
}
}
# add filters extracted from the options # add filters extracted from the options
options_filter_context = options_filter_context(:commit, options) options_filter_context = options_filter_context(:commit, options)
...@@ -110,6 +136,7 @@ module Elastic ...@@ -110,6 +136,7 @@ module Elastic
} }
end end
# rubocop:disable Metrics/AbcSize
def search_blob(query, type: 'blob', page: 1, per: 20, options: {}) def search_blob(query, type: 'blob', page: 1, per: 20, options: {})
page ||= 1 page ||= 1
...@@ -131,6 +158,7 @@ module Elastic ...@@ -131,6 +158,7 @@ module Elastic
# add the term matching # add the term matching
bool_expr[:must] = { bool_expr[:must] = {
simple_query_string: { simple_query_string: {
_name: context.name(:blob, :match, :search_terms),
query: query.term, query: query.term,
default_operator: :and, default_operator: :and,
fields: %w[blob.content blob.file_name] fields: %w[blob.content blob.file_name]
...@@ -141,10 +169,17 @@ module Elastic ...@@ -141,10 +169,17 @@ module Elastic
# we need to do a project visibility check. # we need to do a project visibility check.
# #
# Note that `:current_user` might be `nil` for a anonymous user # Note that `:current_user` might be `nil` for a anonymous user
query_hash = project_ids_filter(query_hash, options) if options.key?(:current_user) query_hash = context.name(:blob, :authorized) { project_ids_filter(query_hash, options) } if options.key?(:current_user)
# add the document type filter # add the document type filter
bool_expr[:filter] << { term: { type: type } } bool_expr[:filter] << {
term: {
type: {
_name: context.name(:doc, :is_a, type),
value: type
}
}
}
# add filters extracted from the query # add filters extracted from the query
query_filter_context = query.elasticsearch_filter_context(:blob) query_filter_context = query.elasticsearch_filter_context(:blob)
......
...@@ -21,9 +21,11 @@ module Elastic ...@@ -21,9 +21,11 @@ module Elastic
end end
options[:features] = 'issues' options[:features] = 'issues'
query_hash = project_ids_filter(query_hash, options) context.name(:issue) do
query_hash = confidentiality_filter(query_hash, options) query_hash = context.name(:authorized) { project_ids_filter(query_hash, options) }
query_hash = state_filter(query_hash, options) query_hash = context.name(:confidentiality) { confidentiality_filter(query_hash, options) }
query_hash = context.name(:match) { state_filter(query_hash, options) }
end
query_hash = apply_sort(query_hash, options) query_hash = apply_sort(query_hash, options)
search(query_hash, options) search(query_hash, options)
...@@ -51,13 +53,13 @@ module Elastic ...@@ -51,13 +53,13 @@ module Elastic
return query_hash if authorized_project_ids.to_set == scoped_project_ids.to_set return query_hash if authorized_project_ids.to_set == scoped_project_ids.to_set
end end
filter = { term: { confidential: false } } filter = { term: { confidential: { _name: context.name(:non_confidential), value: false } } }
if current_user if current_user
filter = { filter = {
bool: { bool: {
should: [ should: [
{ term: { confidential: false } }, { term: { confidential: { _name: context.name(:non_confidential), value: false } } },
{ {
bool: { bool: {
must: [ must: [
...@@ -65,9 +67,9 @@ module Elastic ...@@ -65,9 +67,9 @@ module Elastic
{ {
bool: { bool: {
should: [ should: [
{ term: { author_id: current_user.id } }, { term: { author_id: { _name: context.name(:as_author), value: current_user.id } } },
{ term: { assignee_id: current_user.id } }, { term: { assignee_id: { _name: context.name(:as_assignee), value: current_user.id } } },
{ terms: { project_id: authorized_project_ids } } { terms: { _name: context.name(:project, :membership, :id), project_id: authorized_project_ids } }
] ]
} }
} }
......
...@@ -21,8 +21,10 @@ module Elastic ...@@ -21,8 +21,10 @@ module Elastic
end end
options[:features] = 'merge_requests' options[:features] = 'merge_requests'
query_hash = project_ids_filter(query_hash, options) context.name(:merge_request) do
query_hash = state_filter(query_hash, options) query_hash = context.name(:authorized) { project_ids_filter(query_hash, options) }
query_hash = context.name(:match) { state_filter(query_hash, options) }
end
query_hash = apply_sort(query_hash, options) query_hash = apply_sort(query_hash, options)
search(query_hash, options) search(query_hash, options)
......
...@@ -7,8 +7,7 @@ module Elastic ...@@ -7,8 +7,7 @@ module Elastic
options[:in] = %w(title^2 description) options[:in] = %w(title^2 description)
query_hash = basic_query_hash(options[:in], query) query_hash = basic_query_hash(options[:in], query)
query_hash = context.name(:milestone, :related) { project_ids_filter(query_hash, options) }
query_hash = project_ids_filter(query_hash, options)
search(query_hash, options) search(query_hash, options)
end end
......
...@@ -11,10 +11,12 @@ module Elastic ...@@ -11,10 +11,12 @@ module Elastic
def elastic_search(query, options: {}) def elastic_search(query, options: {})
options[:in] = ['note'] options[:in] = ['note']
query_hash = basic_query_hash(%w[note], query) query_hash = basic_query_hash(%w[note], query)
query_hash = project_ids_filter(query_hash, options)
query_hash = confidentiality_filter(query_hash, options) context.name(:note) do
query_hash = context.name(:authorized) { project_ids_filter(query_hash, options) }
query_hash = context.name(:confidentiality) { confidentiality_filter(query_hash, options) }
end
query_hash[:highlight] = highlight_options(options[:in]) query_hash[:highlight] = highlight_options(options[:in])
...@@ -35,18 +37,20 @@ module Elastic ...@@ -35,18 +37,20 @@ module Elastic
must: [ must: [
{ {
bool: { bool: {
should: [ _name: context.name(:issue, :not_confidential),
{ bool: { must_not: [{ exists: { field: :issue } }] } }, should: [
{ term: { "issue.confidential" => false } } { bool: { must_not: [{ exists: { field: :issue } }] } },
] { term: { "issue.confidential" => false } }
]
} }
}, },
{ {
bool: { bool: {
should: [ _name: context.name(:not_confidential),
{ bool: { must_not: [{ exists: { field: :confidential } }] } }, should: [
{ term: { confidential: false } } { bool: { must_not: [{ exists: { field: :confidential } }] } },
] { term: { confidential: false } }
]
} }
} }
] ]
...@@ -62,17 +66,17 @@ module Elastic ...@@ -62,17 +66,17 @@ module Elastic
{ {
bool: { bool: {
should: [ should: [
{ term: { "issue.confidential" => true } }, { term: { "issue.confidential" => { _name: context.name(:issue, :confidential), value: true } } },
{ term: { confidential: true } } { term: { confidential: { _name: context.name(:confidential), value: true } } }
] ]
} }
}, },
{ {
bool: { bool: {
should: [ should: [
{ term: { "issue.author_id" => current_user.id } }, { term: { "issue.author_id" => { _name: context.name(:as_author), value: current_user.id } } },
{ term: { "issue.assignee_id" => current_user.id } }, { term: { "issue.assignee_id" => { _name: context.name(:as_assignee), value: current_user.id } } },
{ terms: { project_id: authorized_project_ids(current_user, options) } } { terms: { _name: context.name(:project, :membership, :id), project_id: authorized_project_ids(current_user, options) } }
] ]
} }
} }
...@@ -98,15 +102,18 @@ module Elastic ...@@ -98,15 +102,18 @@ module Elastic
def project_ids_filter(query_hash, options) def project_ids_filter(query_hash, options)
query_hash[:query][:bool][:filter] ||= [] query_hash[:query][:bool][:filter] ||= []
project_query = project_ids_query( project_query = context.name(:project) do
options[:current_user], project_ids_query(
options[:project_ids], options[:current_user],
options[:public_and_internal_projects], options[:project_ids],
options[:features] options[:public_and_internal_projects],
) options[:features]
)
end
filters = { filters = {
bool: { bool: {
_name: context.name,
should: [] should: []
} }
} }
...@@ -130,7 +137,7 @@ module Elastic ...@@ -130,7 +137,7 @@ module Elastic
} }
} }
}, },
{ term: { noteable_type: noteable_type } } { term: { noteable_type: { _name: context.name(:noteable, :is_a, noteable_type), value: noteable_type } } }
] ]
} }
} }
...@@ -146,17 +153,19 @@ module Elastic ...@@ -146,17 +153,19 @@ module Elastic
override :pick_projects_by_membership override :pick_projects_by_membership
def pick_projects_by_membership(project_ids, user, _ = nil) def pick_projects_by_membership(project_ids, user, _ = nil)
noteable_type_to_feature.map do |noteable_type, feature| noteable_type_to_feature.map do |noteable_type, feature|
condition = context.name(feature) do
if project_ids == :any condition =
{ term: { visibility_level: Project::PRIVATE } } if project_ids == :any
else { term: { visibility_level: { _name: context.name(:any), value: Project::PRIVATE } } }
{ terms: { id: filter_ids_by_feature(project_ids, user, feature) } } else
end { terms: { _name: context.name(:membership, :id), id: filter_ids_by_feature(project_ids, user, feature) } }
end
limit =
{ terms: { "#{feature}_access_level" => [::ProjectFeature::ENABLED, ::ProjectFeature::PRIVATE] } } limit =
{ terms: { _name: context.name(:enabled_or_private), "#{feature}_access_level" => [::ProjectFeature::ENABLED, ::ProjectFeature::PRIVATE] } }
{ bool: { filter: [condition, limit] }, noteable_type: noteable_type }
{ bool: { _name: context.name, filter: [condition, limit] }, noteable_type: noteable_type }
end
end end
end end
...@@ -166,14 +175,16 @@ module Elastic ...@@ -166,14 +175,16 @@ module Elastic
override :limit_by_feature override :limit_by_feature
def limit_by_feature(condition, _ = nil, include_members_only:) def limit_by_feature(condition, _ = nil, include_members_only:)
noteable_type_to_feature.map do |noteable_type, feature| noteable_type_to_feature.map do |noteable_type, feature|
limit = context.name(feature) do
if include_members_only limit =
{ terms: { "#{feature}_access_level" => [::ProjectFeature::ENABLED, ::ProjectFeature::PRIVATE] } } if include_members_only
else { terms: { _name: context.name(:enabled_or_private), "#{feature}_access_level" => [::ProjectFeature::ENABLED, ::ProjectFeature::PRIVATE] } }
{ term: { "#{feature}_access_level" => ::ProjectFeature::ENABLED } } else
end { term: { "#{feature}_access_level" => { _name: context.name(:enabled), value: ::ProjectFeature::ENABLED } } }
end
{ bool: { filter: [condition, limit] }, noteable_type: noteable_type }
{ bool: { _name: context.name, filter: [condition, limit] }, noteable_type: noteable_type }
end
end end
end end
end end
......
...@@ -8,40 +8,45 @@ module Elastic ...@@ -8,40 +8,45 @@ module Elastic
query_hash = basic_query_hash(options[:in], query) query_hash = basic_query_hash(options[:in], query)
filters = [{ terms: { type: [es_type] } }] filters = [{ terms: { _name: context.name(:doc, :is_a, es_type), type: [es_type] } }]
if options[:namespace_id] context.name(:project) do
filters << { if options[:namespace_id]
terms: { filters << {
namespace_id: [options[:namespace_id]].flatten terms: {
_name: context.name(:related, :namespaces),
namespace_id: [options[:namespace_id]].flatten
}
} }
} end
end
if options[:non_archived]
if options[:non_archived] filters << {
filters << { terms: {
terms: { _name: context.name(:not_archived),
archived: [!options[:non_archived]].flatten archived: [!options[:non_archived]].flatten
}
} }
} end
end
if options[:visibility_levels]
filters << {
terms: {
_name: context.name(:visibility_level),
visibility_level: [options[:visibility_levels]].flatten
}
}
end
if options[:visibility_levels] if options[:project_ids]
filters << { filters << {
terms: { bool: project_ids_query(options[:current_user], options[:project_ids], options[:public_and_internal_projects])
visibility_level: [options[:visibility_levels]].flatten
} }
} end
end
if options[:project_ids] query_hash[:query][:bool][:filter] = filters
filters << {
bool: project_ids_query(options[:current_user], options[:project_ids], options[:public_and_internal_projects])
}
end end
query_hash[:query][:bool][:filter] = filters
search(query_hash, options) search(query_hash, options)
end end
end end
......
# frozen_string_literal: true
module Elastic
module Latest
class QueryContext
module Aware
def context
@context ||= QueryContext.new
end
end
def build_name(*args)
::Gitlab::Elastic::ExprName
.new(self)
.build(*contexts.last, *args)
end
def name(*args, &block)
name = build_name(*args)
return name.to_s unless block_given?
begin
contexts.push(name)
yield name.to_s
ensure
contexts.pop
end
end
private
def contexts
@contexts ||= []
end
end
end
end
...@@ -5,7 +5,7 @@ module Elastic ...@@ -5,7 +5,7 @@ module Elastic
class SnippetClassProxy < ApplicationClassProxy class SnippetClassProxy < ApplicationClassProxy
def elastic_search(query, options: {}) def elastic_search(query, options: {})
query_hash = basic_query_hash(%w(title description), query) query_hash = basic_query_hash(%w(title description), query)
query_hash = filter(query_hash, options) query_hash = context.name(:snippet, :authorized) { filter(query_hash, options) }
search(query_hash, options) search(query_hash, options)
end end
...@@ -27,6 +27,7 @@ module Elastic ...@@ -27,6 +27,7 @@ module Elastic
# Match any of the filter conditions, in addition to the existing conditions # Match any of the filter conditions, in addition to the existing conditions
query_hash[:query][:bool][:filter] << { query_hash[:query][:bool][:filter] << {
bool: { bool: {
_name: context.name,
should: filter_conditions should: filter_conditions
} }
} }
...@@ -40,6 +41,7 @@ module Elastic ...@@ -40,6 +41,7 @@ module Elastic
# Include accessible personal snippets # Include accessible personal snippets
filter_conditions << { filter_conditions << {
bool: { bool: {
_name: context.name(:personal),
filter: [ filter: [
{ terms: { visibility_level: Gitlab::VisibilityLevel.levels_for_user(user) } } { terms: { visibility_level: Gitlab::VisibilityLevel.levels_for_user(user) } }
], ],
...@@ -51,8 +53,9 @@ module Elastic ...@@ -51,8 +53,9 @@ module Elastic
if user if user
filter_conditions << { filter_conditions << {
bool: { bool: {
_name: context.name(:authored),
filter: [ filter: [
{ term: { author_id: user.id } } { term: { author_id: { _name: context.name(:as_author), value: user.id } } }
], ],
must_not: { exists: { field: 'project_id' } } must_not: { exists: { field: 'project_id' } }
} }
...@@ -76,6 +79,7 @@ module Elastic ...@@ -76,6 +79,7 @@ module Elastic
filter_conditions << { filter_conditions << {
bool: { bool: {
_name: context.name(:membership, :id),
must: [ must: [
{ terms: { project_id: project_ids } } { terms: { project_id: project_ids } }
] ]
......
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
module Elastic module Elastic
module Latest module Latest
module StateFilter module StateFilter
include QueryContext::Aware
private private
def state_filter(query_hash, options) def state_filter(query_hash, options)
...@@ -11,7 +13,7 @@ module Elastic ...@@ -11,7 +13,7 @@ module Elastic
return query_hash if state.blank? || state == 'all' return query_hash if state.blank? || state == 'all'
return query_hash unless API::Helpers::SearchHelpers.search_states.include?(state) return query_hash unless API::Helpers::SearchHelpers.search_states.include?(state)
filter = { match: { state: state } } filter = { match: { state: { _name: context.name(:state), query: state } } }
query_hash[:query][:bool][:filter] << filter query_hash[:query][:bool][:filter] << filter
query_hash query_hash
......
# frozen_string_literal: true
module Gitlab
module Elastic
class ExprName
def initialize(context)
@context = context
@values = []
end
def build(*context)
@values.concat(context)
self
end
def name(*context, &block)
@context.name(*context, &block)
end
def to_s
@values.map(&:to_s).join(":")
end
def to_a
@values
end
end
end
end
...@@ -2,13 +2,13 @@ ...@@ -2,13 +2,13 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Elastic::Latest::GitClassProxy do RSpec.describe Elastic::Latest::GitClassProxy, :elastic do
let_it_be(:project) { create(:project, :repository) } let_it_be(:project) { create(:project, :repository) }
let(:included_class) { Elastic::Latest::RepositoryClassProxy } let(:included_class) { Elastic::Latest::RepositoryClassProxy }
subject { included_class.new(project.repository) } subject { included_class.new(project.repository) }
describe '#elastic_search_as_found_blob', :elastic do describe '#elastic_search_as_found_blob' do
before do before do
stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true) stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
...@@ -31,4 +31,11 @@ RSpec.describe Elastic::Latest::GitClassProxy do ...@@ -31,4 +31,11 @@ RSpec.describe Elastic::Latest::GitClassProxy do
expect(result.project).to eq(project) expect(result.project).to eq(project)
end end
end end
it "names elasticsearch queries" do
subject.elastic_search_as_found_blob('*')
assert_named_queries('doc:is_a:blob',
'blob:match:search_terms')
end
end end
...@@ -2,12 +2,12 @@ ...@@ -2,12 +2,12 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Elastic::Latest::ProjectWikiClassProxy do RSpec.describe Elastic::Latest::ProjectWikiClassProxy, :elastic do
let_it_be(:project) { create(:project, :wiki_repo) } let_it_be(:project) { create(:project, :wiki_repo) }
subject { described_class.new(project.wiki.repository) } subject { described_class.new(project.wiki.repository) }
describe '#elastic_search_as_wiki_page', :elastic do describe '#elastic_search_as_wiki_page' do
let_it_be(:page) { create(:wiki_page, wiki: project.wiki) } let_it_be(:page) { create(:wiki_page, wiki: project.wiki) }
before do before do
...@@ -31,4 +31,11 @@ RSpec.describe Elastic::Latest::ProjectWikiClassProxy do ...@@ -31,4 +31,11 @@ RSpec.describe Elastic::Latest::ProjectWikiClassProxy do
expect(result.project).to eq(project) expect(result.project).to eq(project)
end end
end end
it 'names elasticsearch queries' do
subject.elastic_search_as_wiki_page('*')
assert_named_queries('doc:is_a:wiki_blob',
'blob:match:search_terms')
end
end end
...@@ -75,6 +75,14 @@ RSpec.describe Issue, :elastic do ...@@ -75,6 +75,14 @@ RSpec.describe Issue, :elastic do
expect(described_class.elastic_search('bla-bla', options: { project_ids: :any, public_and_internal_projects: true }).total_count).to eq(3) expect(described_class.elastic_search('bla-bla', options: { project_ids: :any, public_and_internal_projects: true }).total_count).to eq(3)
end end
it "names elasticsearch queries" do
described_class.elastic_search('*').total_count
assert_named_queries('doc:is_a:issue',
'issue:match:search_terms',
'issue:authorized:project')
end
it "searches by iid and scopes to type: issue only" do it "searches by iid and scopes to type: issue only" do
issue = nil issue = nil
......
...@@ -40,6 +40,14 @@ RSpec.describe MergeRequest, :elastic do ...@@ -40,6 +40,14 @@ RSpec.describe MergeRequest, :elastic do
expect(described_class.elastic_search('term3', options: { project_ids: :any, public_and_internal_projects: true }).total_count).to eq(1) expect(described_class.elastic_search('term3', options: { project_ids: :any, public_and_internal_projects: true }).total_count).to eq(1)
end end
it "names elasticsearch queries" do
described_class.elastic_search('*').total_count
assert_named_queries('doc:is_a:merge_request',
'merge_request:match:search_terms',
'merge_request:authorized:project')
end
it "searches by iid and scopes to type: merge_request only", :sidekiq_might_not_need_inline do it "searches by iid and scopes to type: merge_request only", :sidekiq_might_not_need_inline do
project = create :project, :public, :repository project = create :project, :public, :repository
merge_request = nil merge_request = nil
......
...@@ -55,6 +55,14 @@ RSpec.describe Note, :elastic do ...@@ -55,6 +55,14 @@ RSpec.describe Note, :elastic do
expect(described_class.elastic_search('bla-bla', options: { project_ids: :any }).records).to contain_exactly(outside_note) expect(described_class.elastic_search('bla-bla', options: { project_ids: :any }).records).to contain_exactly(outside_note)
end end
it "names elasticsearch queries" do
described_class.elastic_search('*').total_count
assert_named_queries("doc:is_a:note",
"note:match:search_terms",
"note:authorized")
end
it "indexes && searches diff notes" do it "indexes && searches diff notes" do
notes = [] notes = []
......
...@@ -198,6 +198,13 @@ RSpec.describe Project, :elastic do ...@@ -198,6 +198,13 @@ RSpec.describe Project, :elastic do
expect(described_class.elastic_search('tesla', options: { project_ids: project_ids }).total_count).to eq(2) expect(described_class.elastic_search('tesla', options: { project_ids: project_ids }).total_count).to eq(2)
end end
it "names elasticsearch queries" do
described_class.elastic_search('*').total_count
assert_named_queries('doc:is_a:project',
'project:match:search_terms')
end
it "returns json with all needed elements" do it "returns json with all needed elements" do
project = create :project project = create :project
......
...@@ -31,6 +31,20 @@ RSpec.describe Repository, :elastic do ...@@ -31,6 +31,20 @@ RSpec.describe Repository, :elastic do
expect(project.repository.elastic_search(partial_ref + '*')[:commits][:total_count]).to eq(1) expect(project.repository.elastic_search(partial_ref + '*')[:commits][:total_count]).to eq(1)
end end
it "names elasticsearch queries" do
project = create :project, :repository
project.repository.elastic_search('*')
assert_named_queries('doc:is_a:blob',
'blob:match:search_terms')
assert_named_queries('doc:is_a:wiki_blob',
'blob:match:search_terms')
assert_named_queries('doc:is_a:commit',
'commit:match:search_terms')
end
it 'can filter blobs' do it 'can filter blobs' do
project = create :project, :repository project = create :project, :repository
index!(project) index!(project)
......
...@@ -35,6 +35,14 @@ RSpec.describe Snippet, :elastic do ...@@ -35,6 +35,14 @@ RSpec.describe Snippet, :elastic do
expect(described_class.elastic_search('test snippet', options: options).total_count).to eq(1) expect(described_class.elastic_search('test snippet', options: options).total_count).to eq(1)
end end
it "names elasticsearch queries" do
described_class.elastic_search('*').total_count
assert_named_queries('doc:is_a:snippet',
'snippet:match:search_terms',
'snippet:authorized')
end
it 'returns json with all needed elements' do it 'returns json with all needed elements' do
snippet = create(:project_snippet) snippet = create(:project_snippet)
......
# frozen_string_literal: true
class ElasticQueryNameInspector
attr_reader :names
def initialize
@names = Set.new
end
def inspect(query)
query.extend(Hashie::Extensions::DeepFind)
@names += query.deep_find_all("_name")
end
def has_named_query?(*expected_names)
@names.superset?(expected_names.to_set)
end
end
# frozen_string_literal: true # frozen_string_literal: true
module ElasticsearchHelpers module ElasticsearchHelpers
def assert_named_queries(*expected_names)
es_host = Gitlab::CurrentSettings.elasticsearch_url.first
search_uri =
Addressable::Template.new("#{es_host}/{index}/doc/_search{?params*}")
ensure_names_present = lambda do |req|
payload = Gitlab::Json.parse(req.body)
query = payload["query"]
return false unless query.present?
inspector = ElasticQueryNameInspector.new
inspector.inspect(query)
inspector.has_named_query?(*expected_names)
rescue ::JSON::ParserError
false
end
a_named_query = a_request(:get, search_uri).with(&ensure_names_present)
message = "Expected a query with the following names: #{expected_names.inspect}"
expect(a_named_query).to have_been_made.at_least_once, message
end
def ensure_elasticsearch_index! def ensure_elasticsearch_index!
# Ensure that any enqueued updates are processed # Ensure that any enqueued updates are processed
Elastic::ProcessBookkeepingService.new.execute Elastic::ProcessBookkeepingService.new.execute
......
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