Commit d8616582 authored by Nick Thomas's avatar Nick Thomas

Merge branch '2358-elasticsearch-project-snippets' into 'master'

Support ES searches for project snippets

See merge request gitlab-org/gitlab!18459
parents dc5e100c a5b8eff2
# frozen_string_literal: true
module Search
class SnippetService
attr_accessor :current_user, :params
def initialize(user, params)
@current_user, @params = user, params.dup
end
class SnippetService < Search::GlobalService
def execute
Gitlab::SnippetSearchResults.new(current_user, params[:search])
end
......
---
title: Support ES searches for project snippets
merge_request: 18459
author:
type: fixed
......@@ -4,7 +4,7 @@ module Elastic
extend ActiveSupport::Concern
FORWARDABLE_INSTANCE_METHODS = [:es_id, :es_parent].freeze
FORWARDABLE_CLASS_METHODS = [:elastic_search, :es_import, :nested?, :es_type, :index_name, :document_type, :mapping, :mappings, :settings, :import].freeze
FORWARDABLE_CLASS_METHODS = [:elastic_search, :es_import, :es_type, :index_name, :document_type, :mapping, :mappings, :settings, :import].freeze
def __elasticsearch__(&block)
@__elasticsearch__ ||= ::Elastic::MultiVersionInstanceProxy.new(self)
......
......@@ -9,7 +9,7 @@ module EE
def execute
return super unless use_elasticsearch?
::Gitlab::Elastic::SnippetSearchResults.new(current_user, params[:search])
::Gitlab::Elastic::SnippetSearchResults.new(current_user, params[:search], elastic_projects, nil, true)
end
# This method is used in the top-level SearchService, so cannot be in-lined into #execute
......
......@@ -15,7 +15,7 @@ module Elastic
record.__elasticsearch__.client = client
import(record, record.class.nested?, indexing)
import(record, indexing)
initial_index_project(record) if record.class == Project && indexing
......@@ -65,12 +65,12 @@ module Elastic
raise ImportError.new(errors.inspect)
end
def import(record, nested, indexing)
def import(record, indexing)
operation = indexing ? 'index_document' : 'update_document'
response = nil
IMPORT_RETRY_COUNT.times do
response = if nested
response = if record.es_parent
record.__elasticsearch__.__send__ operation, routing: record.es_parent # rubocop:disable GitlabSecurity/PublicSend
else
record.__elasticsearch__.__send__ operation # rubocop:disable GitlabSecurity/PublicSend
......
......@@ -18,12 +18,12 @@ class ElasticIndexerWorker
options
)
when /delete/
if klass.nested?
if options['es_parent']
client.delete(
index: klass.index_name,
type: klass.document_type,
id: es_id,
routing: options["es_parent"]
routing: options['es_parent']
)
else
clear_project_data(record_id, es_id) if klass == Project
......
......@@ -5,11 +5,6 @@ module Elastic
class ApplicationClassProxy < Elasticsearch::Model::Proxy::ClassMethodsProxy
include ClassProxyUtil
# Should be overridden for all nested models
def nested?
false
end
def es_type
target.name.underscore
end
......
......@@ -20,14 +20,17 @@ module Elastic
private
def generic_attributes
{
'join_field' => {
attributes = { 'type' => es_type }
if es_parent
attributes['join_field'] = {
'name' => es_type,
'parent' => es_parent
},
'type' => es_type
}
end
attributes
end
end
end
end
......@@ -66,6 +66,7 @@ module Elastic
blob
wiki_blob
commit
snippet
)
}
# ES6 requires a single type per index, so we implement our own "type"
......
......@@ -3,10 +3,6 @@
module Elastic
module Latest
class IssueClassProxy < ApplicationClassProxy
def nested?
true
end
def elastic_search(query, options: {})
query_hash =
if query =~ /#(\d+)\z/
......
......@@ -3,10 +3,6 @@
module Elastic
module Latest
class MergeRequestClassProxy < ApplicationClassProxy
def nested?
true
end
def elastic_search(query, options: {})
query_hash =
if query =~ /\!(\d+)\z/
......
......@@ -3,10 +3,6 @@
module Elastic
module Latest
class MilestoneClassProxy < ApplicationClassProxy
def nested?
true
end
def elastic_search(query, options: {})
options[:in] = %w(title^2 description)
......
......@@ -7,10 +7,6 @@ module Elastic
'note'
end
def nested?
true
end
def elastic_search(query, options: {})
options[:in] = ['note']
......
......@@ -16,9 +16,9 @@ module Elastic
if noteable.is_a?(Issue)
data['issue'] = {
assignee_id: noteable.assignee_ids,
author_id: noteable.author_id,
confidential: noteable.confidential
'assignee_id' => noteable.assignee_ids,
'author_id' => noteable.author_id,
'confidential' => noteable.confidential
}
end
......
......@@ -5,14 +5,14 @@ module Elastic
class SnippetClassProxy < ApplicationClassProxy
def elastic_search(query, options: {})
query_hash = basic_query_hash(%w(title file_name), query)
query_hash = filter(query_hash, options[:user])
query_hash = filter(query_hash, options)
search(query_hash)
end
def elastic_search_code(query, options: {})
query_hash = basic_query_hash(%w(content), query)
query_hash = filter(query_hash, options[:user])
query_hash = filter(query_hash, options)
search(query_hash)
end
......@@ -23,50 +23,91 @@ module Elastic
private
def filter(query_hash, user)
return query_hash if user && user.full_private_access?
def filter(query_hash, options)
user = options[:current_user]
return query_hash if user&.full_private_access?
filter =
if user
{
filter_conditions =
filter_personal_snippets(user, options) +
filter_project_snippets(user, options)
# Match any of the filter conditions, in addition to the existing conditions
query_hash[:query][:bool][:filter] << {
bool: {
should: [
{ term: { author_id: user.id } },
{ terms: { project_id: authorized_project_ids_for_user(user) } },
{
should: filter_conditions
}
}
query_hash
end
def filter_personal_snippets(user, options)
filter_conditions = []
# Include accessible personal snippets
filter_conditions << {
bool: {
filter: [
{ terms: { visibility_level: [Snippet::PUBLIC, Snippet::INTERNAL] } },
{ term: { type: self.es_type } }
{ terms: { visibility_level: Gitlab::VisibilityLevel.levels_for_user(user) } }
],
must_not: { exists: { field: 'project_id' } }
}
}
]
}
}
else
{
# Include authored personal snippets
if user
filter_conditions << {
bool: {
filter: [
{ term: { visibility_level: Snippet::PUBLIC } },
{ term: { type: self.es_type } }
{ term: { author_id: user.id } }
],
must_not: { exists: { field: 'project_id' } }
}
}
end
query_hash[:query][:bool][:filter] = filter
query_hash
filter_conditions
end
def authorized_project_ids_for_user(user)
if Ability.allowed?(user, :read_cross_project)
user.authorized_projects.pluck_primary_key
else
[]
def filter_project_snippets(user, options)
return [] unless Ability.allowed?(user, :read_cross_project)
filter_conditions = []
# Include public/internal project snippets for accessible projects
filter_conditions << {
bool: {
filter: [
{ terms: { visibility_level: Gitlab::VisibilityLevel.levels_for_user(user) } },
{
has_parent: {
parent_type: 'project',
query: {
bool: project_ids_query(
user,
options[:project_ids],
options[:public_and_internal_projects],
'snippets'
)
}
}
}
]
}
}
# Include all project snippets for authorized projects
if user
filter_conditions << {
bool: {
must: [
{ terms: { project_id: user.authorized_projects(Gitlab::Access::GUEST).pluck_primary_key } }
]
}
}
end
filter_conditions
end
end
end
......
......@@ -28,10 +28,7 @@ module Elastic
data['content'] = data['content'].mb_chars.limit(MAX_INDEX_SIZE).to_s # rubocop: disable CodeReuse/ActiveRecord
end
# ES6 is now single-type per index, so we implement our own typing
data['type'] = es_type
data
data.merge(generic_attributes)
end
end
end
......
......@@ -2,7 +2,18 @@
module Gitlab
module Elastic
class SnippetSearchResults < ::Gitlab::SnippetSearchResults
class SnippetSearchResults < Gitlab::Elastic::SearchResults
def objects(scope, page = 1)
page = (page || 1).to_i
case scope
when 'snippet_titles'
eager_load(snippet_titles, page, eager: { project: [:route, :namespace] })
when 'snippet_blobs'
eager_load(snippet_blobs, page, eager: { project: [:route, :namespace] })
end
end
def formatted_count(scope)
case scope
when 'snippet_titles'
......@@ -25,11 +36,11 @@ module Gitlab
private
def snippet_titles
Snippet.elastic_search(query, options: search_params)
Snippet.elastic_search(query, options: base_options)
end
def snippet_blobs
Snippet.elastic_search_code(query, options: search_params)
Snippet.elastic_search_code(query, options: base_options)
end
def limited_snippet_titles_count
......@@ -43,10 +54,6 @@ module Gitlab
def paginated_objects(relation, page)
super.records
end
def search_params
{ user: current_user }
end
end
end
end
......@@ -3,33 +3,111 @@
require 'spec_helper'
describe 'Snippet elastic search', :js, :elastic do
let(:user) { create(:user) }
let(:project) { create(:project, namespace: user.namespace) }
let(:public_project) { create(:project, :public) }
let(:authorized_user) { create(:user) }
let(:authorized_project) { create(:project, namespace: authorized_user.namespace) }
before do
stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
project.add_maintainer(user)
sign_in(user)
end
authorized_project.add_maintainer(authorized_user)
create(:personal_snippet, :public, content: 'public personal snippet')
create(:project_snippet, :public, content: 'public project snippet', project: public_project)
create(:personal_snippet, :internal, content: 'internal personal snippet')
create(:project_snippet, :internal, content: 'internal project snippet', project: public_project)
create(:personal_snippet, :private, content: 'private personal snippet')
create(:project_snippet, :private, content: 'private project snippet', project: public_project)
describe 'searching' do
it 'finds a personal snippet' do
create(:personal_snippet, author: user, content: 'Test searching for personal snippets')
create(:personal_snippet, :private, content: 'authorized personal snippet', author: authorized_user)
create(:project_snippet, :private, content: 'authorized project snippet', project: authorized_project)
Gitlab::Elastic::Helper.refresh_index
sign_in(current_user) if current_user
visit explore_snippets_path
submit_search('Test')
submit_search('snippet')
end
context 'as anonymous user' do
let(:current_user) { nil }
it 'finds only public snippets' do
within('.results') do
expect(page).to have_content('public personal snippet')
expect(page).to have_content('public project snippet')
expect(page).to have_selector('.results', text: 'Test searching for personal snippets')
expect(page).not_to have_content('internal personal snippet')
expect(page).not_to have_content('internal project snippet')
expect(page).not_to have_content('authorized personal snippet')
expect(page).not_to have_content('authorized project snippet')
expect(page).not_to have_content('private personal snippet')
expect(page).not_to have_content('private project snippet')
end
end
end
it 'finds a project snippet' do
create(:project_snippet, project: project, content: 'Test searching for personal snippets')
context 'as logged in user' do
let(:current_user) { create(:user) }
visit explore_snippets_path
submit_search('Test')
it 'finds only public and internal snippets' do
within('.results') do
expect(page).to have_content('public personal snippet')
expect(page).to have_content('public project snippet')
expect(page).to have_content('internal personal snippet')
expect(page).to have_content('internal project snippet')
expect(page).not_to have_content('private personal snippet')
expect(page).not_to have_content('private project snippet')
expect(page).not_to have_content('authorized personal snippet')
expect(page).not_to have_content('authorized project snippet')
end
end
end
context 'as authorized user' do
let(:current_user) { authorized_user }
it 'finds only public, internal, and authorized private snippets' do
within('.results') do
expect(page).to have_content('public personal snippet')
expect(page).to have_content('public project snippet')
expect(page).to have_content('internal personal snippet')
expect(page).to have_content('internal project snippet')
expect(page).not_to have_content('private personal snippet')
expect(page).not_to have_content('private project snippet')
expect(page).to have_content('authorized personal snippet')
expect(page).to have_content('authorized project snippet')
end
end
end
context 'as administrator' do
let(:current_user) { create(:admin) }
expect(page).to have_selector('.results', text: 'Test searching for personal snippets')
it 'finds all snippets' do
within('.results') do
expect(page).to have_content('public personal snippet')
expect(page).to have_content('public project snippet')
expect(page).to have_content('internal personal snippet')
expect(page).to have_content('internal project snippet')
expect(page).to have_content('private personal snippet')
expect(page).to have_content('private project snippet')
expect(page).to have_content('authorized personal snippet')
expect(page).to have_content('authorized project snippet')
end
end
end
end
......@@ -4,7 +4,7 @@ require 'spec_helper'
describe Gitlab::Elastic::SnippetSearchResults, :elastic do
let(:snippet) { create(:personal_snippet, content: 'foo', file_name: 'foo') }
let(:results) { described_class.new(snippet.author, 'foo') }
let(:results) { described_class.new(snippet.author, 'foo', []) }
before do
stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
......@@ -26,7 +26,7 @@ describe Gitlab::Elastic::SnippetSearchResults, :elastic do
end
context 'when user is not author' do
let(:results) { described_class.new(create(:user), 'foo') }
let(:results) { described_class.new(create(:user), 'foo', []) }
it 'returns nothing' do
expect(results.snippet_titles_count).to eq(0)
......@@ -35,7 +35,7 @@ describe Gitlab::Elastic::SnippetSearchResults, :elastic do
end
context 'when user is nil' do
let(:results) { described_class.new(nil, 'foo') }
let(:results) { described_class.new(nil, 'foo', []) }
it 'returns nothing' do
expect(results.snippet_titles_count).to eq(0)
......@@ -54,7 +54,7 @@ describe Gitlab::Elastic::SnippetSearchResults, :elastic do
context 'when user has full_private_access' do
let(:user) { create(:admin) }
let(:results) { described_class.new(user, 'foo') }
let(:results) { described_class.new(user, 'foo', :any) }
it 'returns matched snippets' do
expect(results.snippet_titles_count).to eq(1)
......@@ -67,8 +67,8 @@ describe Gitlab::Elastic::SnippetSearchResults, :elastic do
let(:snippet) { create(:personal_snippet, :public, content: content) }
it 'indexes up to a limit' do
expect(described_class.new(nil, 'abc').snippet_blobs_count).to eq(1)
expect(described_class.new(nil, 'xyz').snippet_blobs_count).to eq(0)
expect(described_class.new(nil, 'abc', []).snippet_blobs_count).to eq(1)
expect(described_class.new(nil, 'xyz', []).snippet_blobs_count).to eq(0)
end
end
end
......@@ -101,15 +101,23 @@ describe Issue, :elastic do
assignee = create(:user)
issue = create :issue, project: project, assignees: [assignee]
expected_hash = issue.attributes.extract!('id', 'iid', 'title', 'description', 'created_at',
'updated_at', 'state', 'project_id', 'author_id',
'confidential')
.merge({
expected_hash = issue.attributes.extract!(
'id',
'iid',
'title',
'description',
'created_at',
'updated_at',
'state',
'project_id',
'author_id',
'confidential'
).merge({
'type' => issue.es_type,
'join_field' => {
'name' => issue.es_type,
'parent' => issue.es_parent
},
'type' => issue.es_type
}
})
expected_hash['assignee_id'] = [assignee.id]
......
......@@ -79,11 +79,11 @@ describe MergeRequest, :elastic do
'target_project_id',
'author_id'
).merge({
'type' => merge_request.es_type,
'join_field' => {
'name' => merge_request.es_type,
'parent' => merge_request.es_parent
},
'type' => merge_request.es_type
}
})
expect(merge_request.__elasticsearch__.as_indexed_json).to eq(expected_hash)
......
......@@ -48,11 +48,11 @@ describe Milestone, :elastic do
'created_at',
'updated_at'
).merge({
'type' => milestone.es_type,
'join_field' => {
'name' => milestone.es_type,
'parent' => milestone.es_parent
},
'type' => milestone.es_type
}
})
expect(milestone.__elasticsearch__.as_indexed_json).to eq(expected_hash)
......
......@@ -75,22 +75,32 @@ describe Note, :elastic do
end
it "returns json with all needed elements" do
note = create :note
expected_hash_keys = %w(
id
note
project_id
noteable_type
noteable_id
created_at
updated_at
issue
join_field
type
)
expect(note.__elasticsearch__.as_indexed_json.keys).to eq(expected_hash_keys)
assignee = create(:user)
issue = create(:issue, assignees: [assignee])
note = create(:note, noteable: issue, project: issue.project)
expected_hash = note.attributes.extract!(
'id',
'note',
'project_id',
'noteable_type',
'noteable_id',
'created_at',
'updated_at'
).merge({
'issue' => {
'assignee_id' => issue.assignee_ids,
'author_id' => issue.author_id,
'confidential' => issue.confidential
},
'type' => note.es_type,
'join_field' => {
'name' => note.es_type,
'parent' => note.es_parent
}
})
expect(note.__elasticsearch__.as_indexed_json).to eq(expected_hash)
end
it "does not create ElasticIndexerWorker job for system messages" do
......
......@@ -34,7 +34,7 @@ describe Snippet, :elastic do
end
it 'returns only public snippets when user is blank' do
result = described_class.elastic_search_code('password', options: { user: nil })
result = described_class.elastic_search_code('password', options: { current_user: nil })
expect(result.total_count).to eq(1)
expect(result.records).to match_array [public_snippet]
......@@ -43,7 +43,7 @@ describe Snippet, :elastic do
it 'returns only public and internal personal snippets for non-members' do
non_member = create(:user)
result = described_class.elastic_search_code('password', options: { user: non_member })
result = described_class.elastic_search_code('password', options: { current_user: non_member })
expect(result.total_count).to eq(2)
expect(result.records).to match_array [public_snippet, internal_snippet]
......@@ -53,14 +53,14 @@ describe Snippet, :elastic do
member = create(:user)
project.add_developer(member)
result = described_class.elastic_search_code('password', options: { user: member })
result = described_class.elastic_search_code('password', options: { current_user: member })
expect(result.total_count).to eq(5)
expect(result.records).to match_array [public_snippet, internal_snippet, project_public_snippet, project_internal_snippet, project_private_snippet]
end
it 'returns private snippets where the user is the author' do
result = described_class.elastic_search_code('password', options: { user: author })
result = described_class.elastic_search_code('password', options: { current_user: author })
expect(result.total_count).to eq(3)
expect(result.records).to match_array [public_snippet, internal_snippet, private_snippet]
......@@ -70,7 +70,7 @@ describe Snippet, :elastic do
member = create(:user)
project.add_reporter(member)
result = described_class.elastic_search_code('password +(123 | 789)', options: { user: member })
result = described_class.elastic_search_code('password +(123 | 789)', options: { current_user: member })
expect(result.total_count).to eq(2)
expect(result.records).to match_array [project_public_snippet, project_private_snippet]
......@@ -80,7 +80,7 @@ describe Snippet, :elastic do
it "returns all snippets for #{user_type}" do
superuser = create(user_type)
result = described_class.elastic_search_code('password', options: { user: superuser })
result = described_class.elastic_search_code('password', options: { current_user: superuser })
expect(result.total_count).to eq(6)
expect(result.records).to match_array [public_snippet, internal_snippet, private_snippet, project_public_snippet, project_internal_snippet, project_private_snippet]
......@@ -97,7 +97,7 @@ describe Snippet, :elastic do
project.add_developer(member)
expect(Ability).to receive(:allowed?).with(member, :read_cross_project) { false }
result = described_class.elastic_search_code('password', options: { user: member })
result = described_class.elastic_search_code('password', options: { current_user: member })
expect(result.records).to match_array [public_snippet, internal_snippet]
end
......@@ -116,7 +116,7 @@ describe Snippet, :elastic do
Gitlab::Elastic::Helper.refresh_index
end
options = { user: user }
options = { current_user: user }
expect(described_class.elastic_search('home', options: options).total_count).to eq(1)
expect(described_class.elastic_search('index.php', options: options).total_count).to eq(1)
......@@ -136,7 +136,13 @@ describe Snippet, :elastic do
'project_id',
'author_id',
'visibility_level'
).merge({ 'type' => snippet.es_type })
).merge({
'type' => snippet.es_type,
'join_field' => {
'name' => snippet.es_type,
'parent' => snippet.es_parent
}
})
expect(snippet.__elasticsearch__.as_indexed_json).to eq(expected_hash)
end
......
......@@ -78,8 +78,32 @@ describe Elastic::IndexRecordService, :elastic do
end
Gitlab::Elastic::Helper.refresh_index
## All database objects + data from repository. The absolute value does not matter
expect(Elasticsearch::Model.search('*').total_count).to be > 40
# Fetch all child documents
children = Elasticsearch::Model.search(
size: 100,
query: {
has_parent: {
parent_type: 'project',
query: {
term: { id: project.id }
}
}
}
)
# The absolute value does not matter
expect(children.total_count).to be > 40
# Make sure all types are present
expect(children.pluck(:_source).pluck(:type).uniq).to contain_exactly(
'blob',
'commit',
'issue',
'merge_request',
'milestone',
'note',
'snippet'
)
end
it 'does not index records not associated with the project' do
......
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