Browse Source

Follow up - ed8a152f28a18e6d070dadbba2e79e524a3b42f7 - Fixes #2911 - Performance: Improved touching assets of users, organizations and tickets. Rollback of indexed data behavior change (structure instead of name).

Rolf Schmidt 5 years ago
parent
commit
eafb6dd236

+ 3 - 3
app/models/application_model/can_lookup_search_index_attributes.rb

@@ -94,11 +94,11 @@ returns
 
     # get name of ref object
     value = nil
-    if respond_to?('search_index_data')
+    if respond_to?('name')
+      value = send('name')
+    elsif respond_to?('search_index_data')
       value = send('search_index_data')
       return if value == true
-    elsif respond_to?('name')
-      value = send('name')
     end
 
     value

+ 7 - 5
app/models/concerns/has_search_index_backend.rb

@@ -91,7 +91,9 @@ returns
 
     # start background job to transfer data to search index
     return true if !SearchIndexBackend.enabled?
-    return if search_index_value.blank?
+
+    new_search_index_value = search_index_value
+    return if new_search_index_value.blank?
 
     Models.indexable.each do |local_object|
       next if local_object == self.class
@@ -106,9 +108,9 @@ returns
       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
+        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?
@@ -116,7 +118,7 @@ returns
         next if attribute_class != self.class
 
         data = {
-          attribute_ref_name => search_index_value
+          attribute_ref_name => new_search_index_value,
         }
         where = {
           attribute_name => id

+ 3 - 7
spec/models/user/can_lookup_search_index_attributes_examples.rb

@@ -1,13 +1,11 @@
 RSpec.shared_examples 'CanLookupSearchIndexAttributes' do
   describe '.search_index_value_by_attribute' do
-    it 'returns hash of data' do
+    it 'returns search index value for attribute' 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)
+      expect(value).to eq('Tomato42')
     end
   end
 
@@ -16,9 +14,7 @@ RSpec.shared_examples 'CanLookupSearchIndexAttributes' 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)
+      expect(value).to eq('Tomato42')
     end
   end
 

+ 2 - 0
spec/models/user_spec.rb

@@ -8,6 +8,7 @@ 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/has_ticket_create_screen_impact_examples'
+require 'models/user/can_lookup_search_index_attributes_examples'
 
 RSpec.describe User, type: :model do
   subject(:user) { create(:user) }
@@ -25,6 +26,7 @@ RSpec.describe User, type: :model do
   it_behaves_like 'CanBeImported'
   it_behaves_like 'HasObjectManagerAttributesValidation'
   it_behaves_like 'HasTicketCreateScreenImpact'
+  it_behaves_like 'CanLookupSearchIndexAttributes'
 
   describe 'Class methods:' do
     describe '.authenticate' do

+ 31 - 22
spec/requests/search_spec.rb

@@ -352,15 +352,18 @@ RSpec.describe 'Search', type: :request, searchindex: true do
     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
+      post '/api/v1/search/User', params: { query: 'Tomato42' }, 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
+
+      post '/api/v1/search/User', params: { query: 'organization:Tomato42' }, as: :json
       expect(response).to have_http_status(:ok)
       expect(json_response).to be_a_kind_of(Hash)
       expect(json_response).to be_truthy
@@ -371,14 +374,16 @@ RSpec.describe 'Search', type: :request, searchindex: true do
       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
+      post '/api/v1/search/User', params: { query: 'Cucumber43' }, 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
+
+      post '/api/v1/search/User', params: { query: 'organization:Cucumber43' }, as: :json
       expect(response).to have_http_status(:ok)
       expect(json_response).to be_a_kind_of(Hash)
       expect(json_response).to be_truthy
@@ -387,13 +392,15 @@ RSpec.describe 'Search', type: :request, searchindex: true do
     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
+      post '/api/v1/search/Ticket', params: { query: 'Tomato42' }, 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
+
+      post '/api/v1/search/Ticket', params: { query: 'organization:Tomato42' }, as: :json
       expect(response).to have_http_status(:ok)
       expect(json_response).to be_a_kind_of(Hash)
       expect(json_response).to be_truthy
@@ -404,12 +411,14 @@ RSpec.describe 'Search', type: :request, searchindex: true do
       Scheduler.worker(true)
       SearchIndexBackend.refresh
 
-      params = {
-        query: 'Cucumber43',
-        limit: 10,
-      }
+      post '/api/v1/search/Ticket', params: { query: 'Cucumber43' }, 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
 
-      post '/api/v1/search/Ticket', params: params, as: :json
+      post '/api/v1/search/Ticket', params: { query: 'organization:Cucumber43' }, as: :json
       expect(response).to have_http_status(:ok)
       expect(json_response).to be_a_kind_of(Hash)
       expect(json_response).to be_truthy

+ 1 - 1
test/integration/elasticsearch_test.rb

@@ -104,7 +104,7 @@ class ElasticsearchTest < ActiveSupport::TestCase
     assert_equal('es-customer1@example.com', attributes['email'])
     assert(attributes['preferences'])
     assert_not(attributes['password'])
-    assert_equal({ 'name' => 'Customer Organization Update', 'note' => 'some note' }, attributes['organization'])
+    assert_equal('Customer Organization Update', attributes['organization'])
 
     # organization
     attributes = @organization1.search_index_data