Fixes #2911 - Performance: Improved touching assets of users, organizations and tickets.

This commit is contained in:
Rolf Schmidt 2020-02-18 15:49:52 +01:00 committed by Thorsten Eckel
parent a01a2f3c91
commit ed8a152f28
23 changed files with 596 additions and 344 deletions

View file

@ -112,6 +112,8 @@ class ActionRow extends App.ObserverActionRow
class Object extends App.ObserverController
model: 'Organization'
observe:
member_ids: true
observeNot:
cid: true
created_at: true
@ -196,6 +198,7 @@ class Member extends App.ObserverController
login: true
email: true
active: true
image: true
globalRerender: false
render: (user) =>

View file

@ -43,15 +43,9 @@ class App.UserProfile extends App.Controller
new User(
object_id: user.id
el: elLocal.find('.js-name')
el: elLocal.find('.js-profileName')
)
if user.organization_id
new Organization(
object_id: user.organization_id
el: elLocal.find('.js-organization')
)
new Object(
el: elLocal.find('.js-object-container')
object_id: user.id
@ -206,9 +200,19 @@ class User extends App.ObserverController
observe:
firstname: true
lastname: true
organization_id: true
image: true
render: (user) =>
@html App.Utils.htmlEscape(user.displayName())
if user.organization_id
new Organization(
object_id: user.organization_id
el: @el.siblings('.js-organization')
)
@html App.view('user_profile/name')(
user: user
)
class Router extends App.ControllerPermanent
requiredPermission: 'ticket.agent'

View file

@ -1,6 +1,4 @@
class App.WidgetOrganization extends App.Controller
@extend App.PopoverProvidable
@registerPopovers 'User'
events:
'focusout [contenteditable]': 'update'
@ -44,10 +42,18 @@ class App.WidgetOrganization extends App.Controller
organizationData.push attributeConfig
# insert userData
@html App.view('widget/organization')(
elLocal = $(App.view('widget/organization')(
organization: organization
organizationData: organizationData
)
))
for user in organization.members
new User(
object_id: user.id
el: elLocal.find('div.userList-row[data-id=' + user.id + ']')
)
@html elLocal
@$('[contenteditable]').ce(
mode: 'textonly'
@ -55,8 +61,6 @@ class App.WidgetOrganization extends App.Controller
maxlength: 250
)
@renderPopovers()
update: (e) =>
name = $(e.target).attr('data-name')
value = $(e.target).html()
@ -66,3 +70,21 @@ class App.WidgetOrganization extends App.Controller
data[name] = value
org.updateAttributes(data)
@log 'notice', 'update', name, value, org
class User extends App.ObserverController
@extend App.PopoverProvidable
@registerPopovers 'User'
model: 'User'
observe:
firstname: true
lastname: true
image: true
render: (user) =>
@html App.view('organization_profile/member')(
user: user
el: @el,
)
@renderPopovers()

View file

@ -2,8 +2,7 @@
<div class="profile-window">
<div class="profile-section vertical centered">
<div class="align-right profile-action js-action"></div>
<%- @user.avatar("80") %>
<h1 class="js-name"></h1>
<div class="profile-name js-profileName text-center"></div>
<% if @user.organization: %>
<div class="profile-organization js-organization"></div>
<% end %>

View file

@ -0,0 +1,2 @@
<%- @user.avatar("80") %>
<h1><%= @user.displayName() %></h1>

View file

@ -27,12 +27,7 @@
<label><%- @T('Members') %></label>
<div class="userList">
<% for user in @organization.members: %>
<div class="userList-entry">
<%- user.avatar("40") %>
<a href="<%- user.uiUrl() %>" class="userList-name user-popover" data-id="<%- user.id %>">
<%= user.displayName() %>
</a>
</div>
<div class="userList-row" data-id="<%- user.id %>"></div>
<% end %>
</div>
</div>

View file

@ -6,11 +6,11 @@ class SearchIndexJob < ApplicationJob
}
def lock_key
# "SearchIndexJob/User/42"
"#{self.class.name}/#{arguments[0]}/#{arguments[1]}"
# "SearchIndexJob/User/42/true"
"#{self.class.name}/#{arguments[0]}/#{arguments[1]}/#{arguments[2]}"
end
def perform(object, o_id)
def perform(object, o_id, update_associations = true)
@object = object
@o_id = o_id
@ -18,6 +18,11 @@ class SearchIndexJob < ApplicationJob
return if !exists?(record)
record.search_index_update_backend
return if !update_associations
record.search_index_update_associations_delta
record.search_index_update_associations_full
end
private

View file

@ -19,48 +19,147 @@ returns
attributes = self.attributes
self.attributes.each do |key, value|
attribute_name = key.to_s
# ignore standard attribute if needed
if self.class.search_index_attribute_ignored?(attribute_name)
attributes.delete(attribute_name)
next
end
# need value for reference data
next if !value
# get attribute name
attribute_name = key.to_s
next if attribute_name[-3, 3] != '_id'
# check if we have a referenced object which we could include here
next if !search_index_attribute_method(attribute_name)
attribute_name = attribute_name[ 0, attribute_name.length - 3 ]
# get referenced attribute name
attribute_ref_name = self.class.search_index_attribute_ref_name(attribute_name)
next if !attribute_ref_name
# check if attribute method exists
next if !respond_to?(attribute_name)
# check if method has own class
relation_class = send(attribute_name).class
next if !relation_class
# lookup ref object
relation_model = relation_class.lookup(id: value)
next if !relation_model
# get name of ref object
value = nil
if relation_model.respond_to?('search_index_data')
value = relation_model.send('search_index_data')
end
if relation_model.respond_to?('name')
value = relation_model.send('name')
end
# ignore referenced attributes if needed
next if self.class.search_index_attribute_ignored?(attribute_ref_name)
# get referenced attribute value
value = search_index_value_by_attribute(attribute_name)
next if !value
# save name of ref object
attributes[ attribute_name ] = value
end
ignored_attributes = self.class.instance_variable_get(:@search_index_attributes_ignored) || []
return attributes if ignored_attributes.blank?
ignored_attributes.each do |attribute|
attributes.delete(attribute.to_s)
attributes[ attribute_ref_name ] = value
end
attributes
end
=begin
This function returns the relational search index value based on the attribute name.
organization = Organization.find(1)
value = organization.search_index_value_by_attribute('organization_id')
returns
value = {"name"=>"Zammad Foundation"}
=end
def search_index_value_by_attribute(attribute_name = '')
# get attribute name
relation_class = search_index_attribute_method(attribute_name)
return if !relation_class
# lookup ref object
relation_model = relation_class.lookup(id: attributes[attribute_name])
return if !relation_model
relation_model.search_index_value
end
=begin
This function returns the relational search value.
organization = Organization.find(1)
value = organization.search_index_value
returns
value = {"name"=>"Zammad Foundation"}
=end
def search_index_value
# get name of ref object
value = nil
if respond_to?('search_index_data')
value = send('search_index_data')
return if value == true
elsif respond_to?('name')
value = send('name')
end
value
end
=begin
This function returns the method for the relational search index attribute.
method = Ticket.new.search_index_attribute_method('organization_id')
returns
method = Organization (class)
=end
def search_index_attribute_method(attribute_name = '')
return if attribute_name[-3, 3] != '_id'
attribute_name = attribute_name[ 0, attribute_name.length - 3 ]
return if !respond_to?(attribute_name)
send(attribute_name).class
end
class_methods do
=begin
This function returns the relational search index attribute name for the given class.
attribute_ref_name = Organization.search_index_attribute_ref_name('user_id')
returns
attribute_ref_name = 'user'
=end
def search_index_attribute_ref_name(attribute_name)
attribute_name[ 0, attribute_name.length - 3 ]
end
=begin
This function returns if a search index attribute should be ignored.
ignored = Ticket.search_index_attribute_ignored?('organization_id')
returns
ignored = false
=end
def search_index_attribute_ignored?(attribute_name = '')
ignored_attributes = instance_variable_get(:@search_index_attributes_ignored) || []
return if ignored_attributes.blank?
ignored_attributes.include?(attribute_name.to_sym)
end
end
end

View file

@ -30,6 +30,86 @@ update search index, if configured - will be executed automatically
=begin
update search index, if configured - will be executed automatically
model = Organizations.find(123)
result = model.search_index_update_associations_full
returns
# Updates asscociation data for users and tickets of the organization in this example
result = true
=end
def search_index_update_associations_full
return if self.class.to_s != 'Organization'
# reindex all organization tickets for the given organization id
# we can not use the delta function for this because of the excluded
# ticket article attachments. see explain in delta function
Ticket.select('id').where(organization_id: id).order(id: :desc).limit(10_000).pluck(:id).each do |ticket_id|
SearchIndexJob.perform_later('Ticket', ticket_id, false)
end
end
=begin
update search index, if configured - will be executed automatically
model = Organizations.find(123)
result = model.search_index_update_associations_delta
returns
# Updates asscociation data for users and tickets of the organization in this example
result = true
=end
def search_index_update_associations_delta
# start background job to transfer data to search index
return true if !SearchIndexBackend.enabled?
return if search_index_value.blank?
Models.indexable.each do |local_object|
next if local_object == self.class
# delta update of associations is only possible for
# objects which are not containing modifications of the source
# https://github.com/zammad/zammad/blob/264853dcbe4e53addaf0f8e6df3735ceddc9de63/lib/tasks/search_index_es.rake#L266
# because of the exlusion of the article attachments for the ticket
# we dont have the attachment data available in the json store of the object.
# so the search index would lose the attachment information on the _update_by_query function
# https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-source-field.html
next if local_object.to_s == 'Ticket'
local_object.new.attributes.each do |key, _value|
attribute_name = key.to_s
attribute_ref_name = local_object.search_index_attribute_ref_name(attribute_name)
attribute_class = local_object.reflect_on_association(attribute_ref_name)&.klass
next if attribute_name.blank?
next if attribute_ref_name.blank?
next if attribute_class.blank?
next if attribute_class != self.class
data = {
attribute_ref_name => search_index_value
}
where = {
attribute_name => id
}
SearchIndexBackend.update_by_query(local_object.to_s, data, where)
end
end
true
end
=begin
delete search index object, will be executed automatically
model = Model.find(123)

View file

@ -1,43 +0,0 @@
# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
class Observer::Organization::RefObjectTouch < ActiveRecord::Observer
observe 'organization'
def after_create(record)
ref_object_touch(record)
end
def after_update(record)
ref_object_touch(record)
end
def after_destroy(record)
ref_object_touch(record)
end
def ref_object_touch(record)
# return if we run import mode
return true if Setting.get('import_mode')
# feature used for different purpose; do not touch references
return true if User.where(organization_id: record.id).count > 100
# touch organizations tickets
Ticket.select('id').where(organization_id: record.id).pluck(:id).each do |ticket_id|
ticket = Ticket.find(ticket_id)
ticket.with_lock do
ticket.touch # rubocop:disable Rails/SkipsModelValidations
end
end
# touch current members
User.select('id').where(organization_id: record.id).pluck(:id).each do |user_id|
user = User.find(user_id)
user.with_lock do
user.touch # rubocop:disable Rails/SkipsModelValidations
end
end
true
end
end

View file

@ -20,37 +20,22 @@ class Observer::User::RefObjectTouch < ActiveRecord::Observer
# return if we run import mode
return true if Setting.get('import_mode')
# touch old organization if changed
member_ids = []
organization_id_changed = record.saved_changes['organization_id']
if organization_id_changed && organization_id_changed[0] != organization_id_changed[1]
if organization_id_changed[0]
return true if !organization_id_changed
# featrue used for different propose, do not touch references
if User.where(organization_id: organization_id_changed[0]).count < 100
organization = Organization.find(organization_id_changed[0])
organization.touch # rubocop:disable Rails/SkipsModelValidations
member_ids = organization.member_ids
end
end
return true if organization_id_changed[0] == organization_id_changed[1]
# touch old organization
if organization_id_changed[0]
organization = Organization.find(organization_id_changed[0])
organization.touch # rubocop:disable Rails/SkipsModelValidations
end
# touch new/current organization
if record.organization
# featrue used for different propose, do not touch references
if User.where(organization_id: record.organization_id).count < 100
record.organization.touch # rubocop:disable Rails/SkipsModelValidations
member_ids += record.organization.member_ids
end
if record&.organization
record.organization.touch # rubocop:disable Rails/SkipsModelValidations
end
# touch old/current customer
member_ids.uniq.each do |user_id|
next if user_id == record.id
User.find(user_id).touch # rubocop:disable Rails/SkipsModelValidations
end
true
end
end

View file

@ -48,7 +48,6 @@ module Zammad
'observer::_user::_ref_object_touch',
'observer::_user::_ticket_organization',
'observer::_user::_geo',
'observer::_organization::_ref_object_touch',
'observer::_sla::_ticket_rebuild_escalation',
'observer::_transaction'

View file

@ -119,7 +119,7 @@ create/update/delete index
def self.index(data)
url = build_url(data[:name], nil, false, false)
url = build_url(type: data[:name], with_pipeline: false, with_document_type: false)
return if url.blank?
if data[:action] && data[:action] == 'delete'
@ -139,7 +139,7 @@ add new object to search index
def self.add(type, data)
url = build_url(type, data['id'])
url = build_url(type: type, object_id: data['id'])
return if url.blank?
make_request_and_validate(url, data: data, method: :post)
@ -147,6 +147,48 @@ add new object to search index
=begin
This function updates specifc attributes of an index based on a query.
data = {
organization: {
name: "Zammad Foundation"
}
}
where = {
organization_id: 1
}
SearchIndexBackend.update_by_query('Ticket', data, where)
=end
def self.update_by_query(type, data, where)
return if data.blank?
return if where.blank?
url = build_url(type: type, action: '_update_by_query', with_pipeline: false, with_document_type: false, url_params: { conflicts: 'proceed' })
return if url.blank?
script_list = []
data.each do |key, _value|
script_list.push("ctx._source.#{key}=params.#{key}")
end
data = {
script: {
lang: 'painless',
source: script_list.join(';'),
params: data,
},
query: {
term: where,
},
}
make_request_and_validate(url, data: data, method: :post, read_timeout: 10.minutes)
end
=begin
remove whole data from index
SearchIndexBackend.remove('Ticket', 123)
@ -157,9 +199,9 @@ remove whole data from index
def self.remove(type, o_id = nil)
url = if o_id
build_url(type, o_id, false, true)
build_url(type: type, object_id: o_id, with_pipeline: false, with_document_type: true)
else
build_url(type, o_id, false, false)
build_url(type: type, object_id: o_id, with_pipeline: false, with_document_type: false)
end
return if url.blank?
@ -237,11 +279,9 @@ remove whole data from index
def self.search_by_index(query, index, options = {})
return [] if query.blank?
url = build_url
url = build_url(type: index, action: '_search', with_pipeline: false, with_document_type: true)
return [] if url.blank?
url += build_search_url(index)
# real search condition
condition = {
'query_string' => {
@ -389,11 +429,9 @@ example for aggregations within one year
def self.selectors(index, selectors = nil, options = {}, aggs_interval = nil)
raise 'no selectors given' if !selectors
url = build_url(nil, nil, false, false)
url = build_url(type: index, action: '_search', with_pipeline: false, with_document_type: true)
return if url.blank?
url += build_search_url(index)
data = selector2query(selectors, options, aggs_interval)
response = make_request(url, data: data)
@ -626,107 +664,92 @@ return true if backend is configured
true
end
def self.build_index_name(index)
def self.build_index_name(index = nil)
local_index = "#{Setting.get('es_index')}_#{Rails.env}"
return local_index if index.blank?
return "#{local_index}/#{index}" if lower_equal_es56?
"#{local_index}_#{index.underscore.tr('/', '_')}"
end
=begin
return true if the elastic search version is lower equal 5.6
result = SearchIndexBackend.lower_equal_es56?
returns
result = true
=end
def self.lower_equal_es56?
Setting.get('es_multi_index') == false
end
=begin
generate url for index or document access (only for internal use)
# url to access single document in index (in case with_pipeline or not)
url = SearchIndexBackend.build_url('User', 123, with_pipeline)
url = SearchIndexBackend.build_url(type: 'User', object_id: 123, with_pipeline: true)
# url to access whole index
url = SearchIndexBackend.build_url('User')
url = SearchIndexBackend.build_url(type: 'User')
# url to access document definition in index (only es6 and higher)
url = SearchIndexBackend.build_url('User', nil, false, true)
url = SearchIndexBackend.build_url(type: 'User', with_pipeline: false, with_document_type: true)
# base url
url = SearchIndexBackend.build_url
=end
def self.build_url(type = nil, o_id = nil, with_pipeline = true, with_document_type = true)
# rubocop:disable Metrics/ParameterLists
def self.build_url(type: nil, action: nil, object_id: nil, with_pipeline: true, with_document_type: true, url_params: {})
# rubocop:enable Metrics/ParameterLists
return if !SearchIndexBackend.enabled?
# for elasticsearch 5.6 and lower
index = "#{Setting.get('es_index')}_#{Rails.env}"
if Setting.get('es_multi_index') == false
url = Setting.get('es_url')
url = if type
if with_pipeline == true
url_pipline = Setting.get('es_pipeline')
if url_pipline.present?
url_pipline = "?pipeline=#{url_pipline}"
end
end
if o_id
"#{url}/#{index}/#{type}/#{o_id}#{url_pipline}"
else
"#{url}/#{index}/#{type}#{url_pipline}"
end
else
"#{url}/#{index}"
end
return url
end
# set index
index = build_index_name(type)
# for elasticsearch 6.x and higher
url = Setting.get('es_url')
if with_pipeline == true
# add pipeline if needed
if index && with_pipeline == true
url_pipline = Setting.get('es_pipeline')
if url_pipline.present?
url_pipline = "?pipeline=#{url_pipline}"
url_params['pipeline'] = url_pipline
end
end
if type
index = build_index_name(type)
# access (e. g. creating or dropping) whole index
if with_document_type == false
return "#{url}/#{index}"
end
# access single document in index (e. g. drop or add document)
if o_id
return "#{url}/#{index}/_doc/#{o_id}#{url_pipline}"
end
# access document type (e. g. creating or dropping document mapping)
return "#{url}/#{index}/_doc#{url_pipline}"
end
"#{url}/"
end
=begin
generate url searchaccess (only for internal use)
# url search access with single index
url = SearchIndexBackend.build_search_url('User')
# url to access all over es
url = SearchIndexBackend.build_search_url
=end
def self.build_search_url(index = nil)
# for elasticsearch 5.6 and lower
if Setting.get('es_multi_index') == false
if index
return "/#{index}/_search"
end
return '/_search'
# prepare url params
params_string = ''
if url_params.present?
params_string = '?' + url_params.map { |key, value| "#{key}=#{value}" }.join('&')
end
# for elasticsearch 6.x and higher
"#{build_index_name(index)}/_doc/_search"
url = Setting.get('es_url')
return "#{url}#{params_string}" if index.blank?
# add type information
url = "#{url}/#{index}"
# add document type
if with_document_type && !lower_equal_es56?
url = "#{url}/_doc"
end
# add action
if action
url = "#{url}/#{action}"
end
# add object id
if object_id.present?
url = "#{url}/#{object_id}"
end
"#{url}#{params_string}"
end
def self.humanized_error(verb:, url:, payload: nil, response:)

View file

@ -2,7 +2,7 @@ $LOAD_PATH << './lib'
require 'rubygems'
namespace :searchindex do
task :drop, [:opts] => :environment do |_t, _args|
task :drop, [:opts] => %i[environment searchindex:version_supported] do |_t, _args|
print 'drop indexes...'
# drop indexes
@ -23,7 +23,7 @@ namespace :searchindex do
Rake::Task['searchindex:drop_pipeline'].execute
end
task :create, [:opts] => :environment do |_t, _args|
task :create, [:opts] => %i[environment searchindex:version_supported] do |_t, _args|
print 'create indexes...'
if es_multi_index?
@ -67,7 +67,7 @@ namespace :searchindex do
Rake::Task['searchindex:create_pipeline'].execute
end
task :create_pipeline, [:opts] => :environment do |_t, _args|
task :create_pipeline, [:opts] => %i[environment searchindex:version_supported] do |_t, _args|
if !es_pipeline?
Setting.set('es_pipeline', '')
next
@ -124,7 +124,7 @@ namespace :searchindex do
puts 'done'
end
task :drop_pipeline, [:opts] => :environment do |_t, _args|
task :drop_pipeline, [:opts] => %i[environment searchindex:version_supported] do |_t, _args|
next if !es_pipeline?
# update processors
@ -142,7 +142,7 @@ namespace :searchindex do
puts 'done'
end
task :reload, [:opts] => :environment do |_t, _args|
task :reload, [:opts] => %i[environment searchindex:version_supported] do |_t, _args|
puts 'reload data...'
Models.indexable.each do |model_class|
@ -156,17 +156,23 @@ namespace :searchindex do
end
task :refresh, [:opts] => :environment do |_t, _args|
task :refresh, [:opts] => %i[environment searchindex:version_supported] do |_t, _args|
print 'refresh all indexes...'
SearchIndexBackend.refresh
end
task :rebuild, [:opts] => :environment do |_t, _args|
task :rebuild, [:opts] => %i[environment searchindex:version_supported] do |_t, _args|
Rake::Task['searchindex:drop'].execute
Rake::Task['searchindex:create'].execute
Rake::Task['searchindex:reload'].execute
end
task :version_supported, [:opts] => :environment do |_t, _args|
next if es_version_supported?
abort "Your elastic search version is not supported! Please update your version to a greater equal than 5.6.0 (Your current version: #{es_version})."
end
end
=begin
@ -300,6 +306,16 @@ def es_version
end
end
def es_version_supported?
version_split = es_version.split('.')
version = "#{version_split[0]}#{format('%03d', version_split[1])}#{format('%03d', version_split[2])}".to_i
# only versions greater/equal than 5.6.0 are supported
return if version < 5_006_000
true
end
# no es_pipeline for elasticsearch 5.5 and lower
def es_pipeline?
number = es_version

View file

@ -31,39 +31,12 @@ RSpec.describe Organization, type: :model do
let!(:member) { create(:customer_user, organization: organization) }
let!(:member_ticket) { create(:ticket, customer: member) }
context 'when basic attributes are updated' do
it 'touches its members and their tickets' do
expect { organization.update(name: 'foo') }
.to change { member.reload.updated_at }
.and change { member_ticket.reload.updated_at }
end
end
context 'when member associations are added' do
let(:user) { create(:customer_user) }
it 'is touched, and touches its other members (but not their tickets)' do
expect { organization.members.push(user) }
.to change { organization.reload.updated_at }
.and change { member.reload.updated_at }
.and not_change { member_ticket.reload.updated_at }
end
end
context 'with 100+ members' do
let!(:members) { create_list(:user, 101, organization: organization) }
let!(:member_ticket) { create(:ticket, customer: members.first) }
# This _should_ be split into two separate examples,
# but setup is slow and expensive.
it 'does not perform any association updates' do
expect { organization.update(name: 'foo') }
.to not_change { members.map(&:reload).map(&:updated_at) }
.and not_change { member_ticket.reload.updated_at }
expect { organization.members.push(member) }
.to not_change { organization.reload.updated_at }
.and not_change { members.map(&:reload).map(&:updated_at) }
end
end
end

View file

@ -852,20 +852,6 @@ RSpec.describe Ticket, type: :model do
.and change { other_organization.reload.updated_at }
end
end
context 'when organization has 100+ members' do
let!(:other_members) { create_list(:user, 100, organization: organization) }
context 'and customer association is changed' do
it 'touches both old and new customer, and their organizations' do
expect { ticket.update(customer: other_customer) }
.to change { customer.reload.updated_at }
.and change { organization.reload.updated_at }
.and change { other_customer.reload.updated_at }
.and change { other_organization.reload.updated_at }
end
end
end
end
describe 'Association & attachment management:' do

View file

@ -0,0 +1,38 @@
RSpec.shared_examples 'CanLookupSearchIndexAttributes' do
describe '.search_index_value_by_attribute' do
it 'returns hash of data' do
organization = create(:organization, name: 'Tomato42', note: 'special recipe')
user = create(:agent_user, organization: organization)
value = user.search_index_value_by_attribute('organization_id')
expect_value = { 'name' => 'Tomato42', 'note' => 'special recipe' }
expect(value).to be_a_kind_of(Hash)
expect(value).to eq(expect_value)
end
end
describe '.search_index_value' do
it 'returns correct value' do
organization = create(:organization, name: 'Tomato42', note: 'special recipe')
value = organization.search_index_value
expect_value = { 'name' => 'Tomato42', 'note' => 'special recipe' }
expect(value).to be_a_kind_of(Hash)
expect(value).to eq(expect_value)
end
end
describe '.search_index_attribute_ref_name' do
it 'returns correct value' do
attribute_ref_name = User.search_index_attribute_ref_name('organization_id')
expect(attribute_ref_name).to eq('organization')
end
end
describe '.search_index_attribute_ignored?' do
it 'returns correct value' do
ignored = User.search_index_attribute_ignored?('password')
expect(ignored).to be true
end
end
end

View file

@ -7,6 +7,7 @@ require 'models/concerns/has_groups_permissions_examples'
require 'models/concerns/has_xss_sanitized_note_examples'
require 'models/concerns/can_be_imported_examples'
require 'models/concerns/has_object_manager_attributes_validation_examples'
require 'models/user/can_lookup_search_index_attributes_examples'
RSpec.describe User, type: :model do
subject(:user) { create(:user) }
@ -23,6 +24,7 @@ RSpec.describe User, type: :model do
it_behaves_like 'HasGroups and Permissions', group_access_no_permission_factory: :user
it_behaves_like 'CanBeImported'
it_behaves_like 'HasObjectManagerAttributesValidation'
it_behaves_like 'CanLookupSearchIndexAttributes'
describe 'Class methods:' do
describe '.authenticate' do
@ -1142,28 +1144,16 @@ RSpec.describe User, type: :model do
end
describe 'Touching associations on update:' do
subject(:user) { create(:customer_user, organization: organization) }
subject!(:user) { create(:customer_user) }
let(:organization) { create(:organization) }
let(:other_customer) { create(:customer_user) }
let!(:organization) { create(:organization) }
context 'when basic attributes are updated' do
context 'when a customer gets a organization ' do
it 'touches its organization' do
expect { user.update(firstname: 'foo') }
expect { user.update(organization: organization) }
.to change { organization.reload.updated_at }
end
end
context 'when organization has 100+ other members' do
let!(:other_members) { create_list(:user, 100, organization: organization) }
context 'and basic attributes are updated' do
it 'does not touch its organization' do
expect { user.update(firstname: 'foo') }
.to not_change { organization.reload.updated_at }
end
end
end
end
describe 'Cti::CallerId syncing:' do

View file

@ -106,24 +106,27 @@ RSpec.describe 'Organization', type: :request, searchindex: true do
get "/api/v1/organizations/search?query=#{CGI.escape('Zammad')}", params: {}, as: :json
expect(response).to have_http_status(:ok)
expect(json_response).to be_a_kind_of(Array)
expect(json_response[0]['name']).to eq('Zammad Foundation')
expect(json_response[0]['member_ids']).to be_truthy
expect(json_response[0]['members']).to be_falsey
organization = json_response.detect { |object| object['name'] == 'Zammad Foundation' }
expect(organization['name']).to eq('Zammad Foundation')
expect(organization['member_ids']).to be_truthy
expect(organization['members']).to be_falsey
get "/api/v1/organizations/search?query=#{CGI.escape('Zammad')}&expand=true", params: {}, as: :json
expect(response).to have_http_status(:ok)
expect(json_response).to be_a_kind_of(Array)
expect(json_response[0]['name']).to eq('Zammad Foundation')
expect(json_response[0]['member_ids']).to be_truthy
expect(json_response[0]['members']).to be_truthy
organization = json_response.detect { |object| object['name'] == 'Zammad Foundation' }
expect(organization['name']).to eq('Zammad Foundation')
expect(organization['member_ids']).to be_truthy
expect(organization['members']).to be_truthy
get "/api/v1/organizations/search?query=#{CGI.escape('Zammad')}&label=true", params: {}, as: :json
expect(response).to have_http_status(:ok)
expect(json_response).to be_a_kind_of(Array)
expect(json_response[0]['label']).to eq('Zammad Foundation')
expect(json_response[0]['value']).to eq('Zammad Foundation')
expect(json_response[0]['member_ids']).to be_falsey
expect(json_response[0]['members']).to be_falsey
organization = json_response.detect { |object| object['label'] == 'Zammad Foundation' }
expect(organization['label']).to eq('Zammad Foundation')
expect(organization['value']).to eq('Zammad Foundation')
expect(organization['member_ids']).to be_falsey
expect(organization['members']).to be_falsey
end
it 'does index with customer1' do

View file

@ -27,6 +27,12 @@ RSpec.describe 'Search', type: :request, searchindex: true do
let!(:organization5) do
create(:organization, name: 'ABC_D Org')
end
let!(:organization_nested) do
create(:organization, name: 'Tomato42 Ltd.', note: 'Tomato42 Ltd.')
end
let!(:customer_user_nested) do
create(:customer_user, organization: organization_nested)
end
let!(:customer_user2) do
create(:customer_user, organization: organization1)
end
@ -42,6 +48,9 @@ RSpec.describe 'Search', type: :request, searchindex: true do
let!(:ticket3) do
create(:ticket, title: 'test 1234-2', customer: customer_user3, group: group)
end
let!(:ticket_nested) do
create(:ticket, title: 'vegetable request', customer: customer_user_nested, group: group)
end
let!(:article1) do
create(:ticket_article, ticket_id: ticket1.id)
end
@ -51,6 +60,20 @@ RSpec.describe 'Search', type: :request, searchindex: true do
let!(:article3) do
create(:ticket_article, ticket_id: ticket3.id)
end
let!(:article_nested) do
article = create(:ticket_article, ticket_id: ticket_nested.id)
Store.add(
object: 'Ticket::Article',
o_id: article.id,
data: File.binread(Rails.root.join('test', 'data', 'elasticsearch', 'es-normal.txt')),
filename: 'es-normal.txt',
preferences: {},
created_by_id: 1,
)
article
end
before do
configure_elasticsearch do
@ -327,5 +350,100 @@ RSpec.describe 'Search', type: :request, searchindex: true do
target_id = json_response['result'][0]['id']
expect(json_response['assets']['Organization'][target_id.to_s]['name']).to eq('ABC_D Org')
end
it 'does find the user of the nested organization and also even if the organization name changes' do
params = {
query: 'Tomato42',
limit: 10,
}
# because of the initial relation between user and organization
# both user and organization will be found as result
authenticated_as(agent_user)
post '/api/v1/search/User', params: params, as: :json
expect(response).to have_http_status(:ok)
expect(json_response).to be_a_kind_of(Hash)
expect(json_response).to be_truthy
expect(json_response['assets']['Organization'][organization_nested.id.to_s]).to be_truthy
expect(json_response['assets']['User'][customer_user_nested.id.to_s]).to be_truthy
organization_nested.update(name: 'Cucumber43 Ltd.')
Scheduler.worker(true)
SearchIndexBackend.refresh
params = {
query: 'Cucumber43',
limit: 10,
}
# even after a change of the organization name we should find
# the customer user because of the nested organization data
post '/api/v1/search/User', params: params, as: :json
expect(response).to have_http_status(:ok)
expect(json_response).to be_a_kind_of(Hash)
expect(json_response).to be_truthy
expect(json_response['assets']['Organization'][organization_nested.id.to_s]).to be_truthy
expect(json_response['assets']['User'][customer_user_nested.id.to_s]).to be_truthy
end
it 'does find the ticket by organization name even if the organization name changes' do
params = {
query: 'Tomato42',
limit: 10,
}
authenticated_as(agent_user)
post '/api/v1/search/Ticket', params: params, as: :json
expect(response).to have_http_status(:ok)
expect(json_response).to be_a_kind_of(Hash)
expect(json_response).to be_truthy
expect(json_response['assets']['Organization'][organization_nested.id.to_s]).to be_truthy
expect(json_response['assets']['Ticket'][ticket_nested.id.to_s]).to be_truthy
organization_nested.update(name: 'Cucumber43 Ltd.')
Scheduler.worker(true)
SearchIndexBackend.refresh
params = {
query: 'Cucumber43',
limit: 10,
}
post '/api/v1/search/Ticket', params: params, as: :json
expect(response).to have_http_status(:ok)
expect(json_response).to be_a_kind_of(Hash)
expect(json_response).to be_truthy
expect(json_response['assets']['Organization'][organization_nested.id.to_s]).to be_truthy
expect(json_response['assets']['Ticket'][ticket_nested.id.to_s]).to be_truthy
end
it 'does find the ticket by attachment even after ticket reindex' do
params = {
query: 'text66',
limit: 10,
}
authenticated_as(agent_user)
post '/api/v1/search/Ticket', params: params, as: :json
expect(response).to have_http_status(:ok)
expect(json_response).to be_a_kind_of(Hash)
expect(json_response).to be_truthy
expect(json_response['assets']['Ticket'][ticket_nested.id.to_s]).to be_truthy
organization_nested.update(name: 'Cucumber43 Ltd.')
Scheduler.worker(true)
SearchIndexBackend.refresh
params = {
query: 'text66',
limit: 10,
}
post '/api/v1/search/Ticket', params: params, as: :json
expect(response).to have_http_status(:ok)
expect(json_response).to be_a_kind_of(Hash)
expect(json_response).to be_truthy
expect(json_response['assets']['Ticket'][ticket_nested.id.to_s]).to be_truthy
end
end
end

View file

@ -89,24 +89,27 @@ RSpec.describe 'User Organization', type: :request, searchindex: true do
get "/api/v1/organizations/search?query=#{CGI.escape('Zammad')}", params: {}, as: :json
expect(response).to have_http_status(:ok)
expect(json_response.class).to eq(Array)
expect(json_response[0]['name']).to eq('Zammad Foundation')
expect(json_response[0]['member_ids']).to be_truthy
expect(json_response[0]['members']).to be_falsey
organization = json_response.detect { |object| object['name'] == 'Zammad Foundation' }
expect(organization['name']).to eq('Zammad Foundation')
expect(organization['member_ids']).to be_truthy
expect(organization['members']).to be_falsey
get "/api/v1/organizations/search?query=#{CGI.escape('Zammad')}&expand=true", params: {}, as: :json
expect(response).to have_http_status(:ok)
expect(json_response.class).to eq(Array)
expect(json_response[0]['name']).to eq('Zammad Foundation')
expect(json_response[0]['member_ids']).to be_truthy
expect(json_response[0]['members']).to be_truthy
organization = json_response.detect { |object| object['name'] == 'Zammad Foundation' }
expect(organization['name']).to eq('Zammad Foundation')
expect(organization['member_ids']).to be_truthy
expect(organization['members']).to be_truthy
get "/api/v1/organizations/search?query=#{CGI.escape('Zammad')}&label=true", params: {}, as: :json
expect(response).to have_http_status(:ok)
expect(json_response.class).to eq(Array)
expect(json_response[0]['label']).to eq('Zammad Foundation')
expect(json_response[0]['value']).to eq('Zammad Foundation')
expect(json_response[0]['member_ids']).to be_falsey
expect(json_response[0]['members']).to be_falsey
organization = json_response.detect { |object| object['label'] == 'Zammad Foundation' }
expect(organization['label']).to eq('Zammad Foundation')
expect(organization['value']).to eq('Zammad Foundation')
expect(organization['member_ids']).to be_falsey
expect(organization['members']).to be_falsey
end
it 'does organization index with customer1' do

View file

@ -74,6 +74,7 @@ class ElasticsearchTest < ActiveSupport::TestCase
# execute background jobs to index created/changed objects
Scheduler.worker(true)
SearchIndexBackend.refresh
end
@ -103,7 +104,7 @@ class ElasticsearchTest < ActiveSupport::TestCase
assert_equal('es-customer1@example.com', attributes['email'])
assert(attributes['preferences'])
assert_not(attributes['password'])
assert_equal('Customer Organization Update', attributes['organization'])
assert_equal({ 'name' => 'Customer Organization Update', 'note' => 'some note' }, attributes['organization'])
# organization
attributes = @organization1.search_index_data
@ -174,6 +175,7 @@ class ElasticsearchTest < ActiveSupport::TestCase
# execute background jobs
Scheduler.worker(true)
SearchIndexBackend.refresh
ticket1 = Ticket.create!(
title: "some title\n äöüß",
@ -295,7 +297,7 @@ class ElasticsearchTest < ActiveSupport::TestCase
# execute background jobs
Scheduler.worker(true)
sleep 2 # for ES to come ready/indexed
SearchIndexBackend.refresh
# search as @agent
@ -433,7 +435,7 @@ class ElasticsearchTest < ActiveSupport::TestCase
# execute background jobs
Scheduler.worker(true)
sleep 2 # for ES to come ready/indexed
SearchIndexBackend.refresh
# search for tags
result = Ticket.search(

View file

@ -94,60 +94,10 @@ class UserAssetsTest < ActiveSupport::TestCase
attributes.delete('authorization_ids')
assert(diff(attributes, assets[:User][user3.id]), 'check assets')
# touch org, check if user1 has changed
travel 2.seconds
org2 = Organization.find(org1.id)
org2.note = "some note...#{rand(9_999_999_999_999)}"
org2.save!
attributes = org2.attributes_with_association_ids
attributes.delete('user_ids')
assert_not(diff(attributes, assets[:Organization][org2.id]), 'check assets')
user1_new = User.find(user1.id)
attributes = user1_new.attributes_with_association_ids
attributes['accounts'] = {}
attributes.delete('password')
attributes.delete('token_ids')
attributes.delete('authorization_ids')
assert_not(diff(attributes, assets[:User][user1_new.id]), 'check assets')
# check new assets lookup
assets = user3.assets({})
attributes = org2.attributes_with_association_ids
attributes.delete('user_ids')
assert(diff(attributes, assets[:Organization][org1.id]), 'check assets')
user1 = User.find(user1.id)
attributes = user1.attributes_with_association_ids
attributes['accounts'] = {}
attributes.delete('password')
attributes.delete('token_ids')
attributes.delete('authorization_ids')
assert(diff(attributes, assets[:User][user1.id]), 'check assets')
user2 = User.find(user2.id)
attributes = user2.attributes_with_association_ids
attributes['accounts'] = {}
attributes.delete('password')
attributes.delete('token_ids')
attributes.delete('authorization_ids')
assert(diff(attributes, assets[:User][user2.id]), 'check assets')
user3 = User.find(user3.id)
attributes = user3.attributes_with_association_ids
attributes['accounts'] = {}
attributes.delete('password')
attributes.delete('token_ids')
attributes.delete('authorization_ids')
assert(diff(attributes, assets[:User][user3.id]), 'check assets')
travel_back
user3.destroy!
user2.destroy!
user1.destroy!
org1.destroy!
assert_not(Organization.find_by(id: org2.id))
end
def diff(object1, object2)