From 8afc38b98451432003b15e22b0bf2570e9e5fb8f Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Tue, 13 Jun 2017 16:57:46 +0200 Subject: [PATCH 1/6] Batch saving strategy should populate the right timestamps Batch saving strategy should populate the right timestamps --- .../save_collection/saver/base.rb | 9 +++++++++ .../saver/concurrent_safe_batch.rb | 16 +++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/app/models/manager_refresh/save_collection/saver/base.rb b/app/models/manager_refresh/save_collection/saver/base.rb index 41810976fbf..604937221ac 100644 --- a/app/models/manager_refresh/save_collection/saver/base.rb +++ b/app/models/manager_refresh/save_collection/saver/base.rb @@ -94,6 +94,15 @@ def assert_referential_integrity(hash, inventory_object) end true end + + def time_now + # A rails friendly time getting config from ActiveRecord::Base.default_timezone (can be :local or :utc) + if ActiveRecord::Base.default_timezone == :utc + Time.now.utc + else + Time.now.localtime + end + end end end end diff --git a/app/models/manager_refresh/save_collection/saver/concurrent_safe_batch.rb b/app/models/manager_refresh/save_collection/saver/concurrent_safe_batch.rb index edd0f53ef13..e48aa383a3a 100644 --- a/app/models/manager_refresh/save_collection/saver/concurrent_safe_batch.rb +++ b/app/models/manager_refresh/save_collection/saver/concurrent_safe_batch.rb @@ -17,6 +17,10 @@ def save!(inventory_collection, association) all_attribute_keys.merge(attributes_index[index].keys) end + all_attribute_keys << :last_sync_on if inventory_collection.supports_last_sync_on? + all_attribute_keys += [:created_on, :updated_on] if inventory_collection.supports_timestamps_on_variant? + all_attribute_keys += [:created_at, :updated_at] if inventory_collection.supports_timestamps_at_variant? + inventory_collection_size = inventory_collection.size deleted_counter = 0 created_counter = 0 @@ -27,6 +31,7 @@ def save!(inventory_collection, association) # Records that are in the DB, we will be updating or deleting them. association.find_in_batches do |batch| + update_time = time_now batch.each do |record| next unless assert_distinct_relation(record) @@ -47,7 +52,12 @@ def save!(inventory_collection, association) next unless assert_referential_integrity(hash, inventory_object) inventory_object.id = record.id - hashes_for_update << hash.symbolize_keys.except(:id, :type) + hash_for_update = hash.symbolize_keys.except(:id, :type) + hash_for_update[:updated_on] = update_time if inventory_collection.supports_timestamps_on_variant? + hash_for_update[:updated_at] = update_time if inventory_collection.supports_timestamps_at_variant? + hash_for_update[:last_sync_on] = update_time if inventory_collection.supports_last_sync_on? + + hashes_for_update << hash_for_update end end @@ -106,9 +116,13 @@ def update_records!(inventory_collection, all_attribute_keys, hashes) def create_records!(inventory_collection, all_attribute_keys, batch, attributes_index) indexed_inventory_objects = {} hashes = [] + create_time = time_now batch.each do |index, inventory_object| hash = attributes_index.delete(index).symbolize_keys hash[:type] = inventory_collection.model_class.name if inventory_collection.supports_sti? && hash[:type].blank? + hash[:created_on] = create_time if inventory_collection.supports_timestamps_on_variant? + hash[:created_at] = create_time if inventory_collection.supports_timestamps_at_variant? + hash[:last_sync_on] = create_time if inventory_collection.supports_last_sync_on? next unless assert_referential_integrity(hash, inventory_object) hashes << hash From 87935b8656d766a386098574a736febb01d0e1a8 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Thu, 15 Jun 2017 12:43:00 +0200 Subject: [PATCH 2/6] Supports helpers for timestamp attributes Supports helpers for timestamp attributes --- .../manager_refresh/inventory_collection.rb | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/app/models/manager_refresh/inventory_collection.rb b/app/models/manager_refresh/inventory_collection.rb index 000960f10a9..9a969f32529 100644 --- a/app/models/manager_refresh/inventory_collection.rb +++ b/app/models/manager_refresh/inventory_collection.rb @@ -531,6 +531,29 @@ def supports_sti? @supports_sti_cache end + def supports_timestamps_on_variant? + if @supports_timestamps_on_variant.nil? + @supports_timestamps_on_variant = (model_class.column_names.include?("created_on") && + model_class.column_names.include?("updated_on") && + ActiveRecord::Base.record_timestamps) + end + @supports_timestamps_on_variant + end + + def supports_timestamps_at_variant? + if @supports_timestamps_at_variant.nil? + @supports_timestamps_at_variant = (model_class.column_names.include?("created_at") && + model_class.column_names.include?("updated_at") && + ActiveRecord::Base.record_timestamps) + end + @supports_timestamps_at_variant + end + + def supports_last_sync_on? + @supports_last_sync_on = model_class.column_names.include?("last_sync_on") if @supports_last_sync_on.nil? + @supports_last_sync_on + end + def targeted? targeted end From fe28f998e7eecab6608b2d88e0af3b94c9cde014 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Thu, 15 Jun 2017 12:44:06 +0200 Subject: [PATCH 3/6] Supports helper for remote_data_timetamp Supports helper for remote_data_timetamp --- app/models/manager_refresh/save_collection/saver/base.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/models/manager_refresh/save_collection/saver/base.rb b/app/models/manager_refresh/save_collection/saver/base.rb index 604937221ac..5e4130dd4ce 100644 --- a/app/models/manager_refresh/save_collection/saver/base.rb +++ b/app/models/manager_refresh/save_collection/saver/base.rb @@ -103,6 +103,10 @@ def time_now Time.now.localtime end end + + def supports_remote_data_timestamp?(all_attribute_keys) + all_attribute_keys.include?(:remote_data_timestamp) # include? on Set is O(1) + end end end end From 7672b5ae789deddb97ef58bdfa76959ec8d02822 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Thu, 15 Jun 2017 12:47:42 +0200 Subject: [PATCH 4/6] Use supports_remote_data_timestamp? in sql helper Use supports_remote_data_timestamp? in sql helper --- .../manager_refresh/save_collection/saver/sql_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/manager_refresh/save_collection/saver/sql_helper.rb b/app/models/manager_refresh/save_collection/saver/sql_helper.rb index 462b896514d..f2e16504797 100644 --- a/app/models/manager_refresh/save_collection/saver/sql_helper.rb +++ b/app/models/manager_refresh/save_collection/saver/sql_helper.rb @@ -33,7 +33,7 @@ def build_insert_query(inventory_collection, all_attribute_keys, hashes) # This conditional will avoid rewriting new data by old data. But we want it only when remote_data_timestamp is a # part of the data, since for the fake records, we just want to update ems_ref. - if all_attribute_keys.include?(:remote_data_timestamp) # include? on Set is O(1) + if supports_remote_data_timestamp?(all_attribute_keys) insert_query += %{ WHERE EXCLUDED.remote_data_timestamp IS NULL OR (EXCLUDED.remote_data_timestamp > #{table_name}.remote_data_timestamp) } @@ -72,7 +72,7 @@ def build_update_query(inventory_collection, all_attribute_keys, hashes) # This conditional will avoid rewriting new data by old data. But we want it only when remote_data_timestamp is a # part of the data, since for the fake records, we just want to update ems_ref. - if all_attribute_keys.include?(:remote_data_timestamp) # include? on Set is O(1) + if supports_remote_data_timestamp?(all_attribute_keys) update_query += %{ AND (updated_values.remote_data_timestamp IS NULL OR (updated_values.remote_data_timestamp > #{table_name}.remote_data_timestamp)) } From aa0a6da4126fd422c7e80bfe8fab48730d803eda Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Thu, 15 Jun 2017 13:11:41 +0200 Subject: [PATCH 5/6] Extract assignment of a common attributes to a base class Extract assignment of a common attributes to a base class --- .../manager_refresh/save_collection/saver/base.rb | 13 +++++++++++++ .../save_collection/saver/concurrent_safe.rb | 2 ++ .../save_collection/saver/concurrent_safe_batch.rb | 10 +++------- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/app/models/manager_refresh/save_collection/saver/base.rb b/app/models/manager_refresh/save_collection/saver/base.rb index 5e4130dd4ce..0d2b124c95a 100644 --- a/app/models/manager_refresh/save_collection/saver/base.rb +++ b/app/models/manager_refresh/save_collection/saver/base.rb @@ -107,6 +107,19 @@ def time_now def supports_remote_data_timestamp?(all_attribute_keys) all_attribute_keys.include?(:remote_data_timestamp) # include? on Set is O(1) end + + def assign_attributes_for_update!(hash, inventory_collection, update_time) + hash[:last_sync_on] = time if inventory_collection.supports_last_sync_on? + hash[:updated_on] = update_time if inventory_collection.supports_timestamps_on_variant? + hash[:updated_at] = update_time if inventory_collection.supports_timestamps_at_variant? + end + + def assign_attributes_for_create!(hash, inventory_collection, create_time) + hash[:type] = inventory_collection.model_class.name if inventory_collection.supports_sti? && hash[:type].blank? + hash[:last_sync_on] = time if inventory_collection.supports_last_sync_on? + hash[:created_on] = create_time if inventory_collection.supports_timestamps_on_variant? + hash[:created_at] = create_time if inventory_collection.supports_timestamps_at_variant? + end end end end diff --git a/app/models/manager_refresh/save_collection/saver/concurrent_safe.rb b/app/models/manager_refresh/save_collection/saver/concurrent_safe.rb index 719d5bf9bd2..ab79cce8549 100644 --- a/app/models/manager_refresh/save_collection/saver/concurrent_safe.rb +++ b/app/models/manager_refresh/save_collection/saver/concurrent_safe.rb @@ -53,6 +53,7 @@ def save!(inventory_collection, association) end def update_record!(inventory_collection, record, hash, inventory_object) + assign_attributes_for_update!(hash, inventory_collection, time_now) record.assign_attributes(hash.except(:id, :type)) if !inventory_object.inventory_collection.check_changed? || record.changed? @@ -73,6 +74,7 @@ def create_record!(inventory_collection, hash, inventory_object) all_attribute_keys = hash.keys hash = inventory_collection.model_class.new(hash).attributes.symbolize_keys + assign_attributes_for_create!(hash, inventory_collection, time_now) result_id = ActiveRecord::Base.connection.insert_sql( build_insert_query(inventory_collection, all_attribute_keys, [hash]) diff --git a/app/models/manager_refresh/save_collection/saver/concurrent_safe_batch.rb b/app/models/manager_refresh/save_collection/saver/concurrent_safe_batch.rb index e48aa383a3a..de9c382ba49 100644 --- a/app/models/manager_refresh/save_collection/saver/concurrent_safe_batch.rb +++ b/app/models/manager_refresh/save_collection/saver/concurrent_safe_batch.rb @@ -53,9 +53,7 @@ def save!(inventory_collection, association) inventory_object.id = record.id hash_for_update = hash.symbolize_keys.except(:id, :type) - hash_for_update[:updated_on] = update_time if inventory_collection.supports_timestamps_on_variant? - hash_for_update[:updated_at] = update_time if inventory_collection.supports_timestamps_at_variant? - hash_for_update[:last_sync_on] = update_time if inventory_collection.supports_last_sync_on? + assign_attributes_for_update!(hash_for_update, inventory_collection, update_time) hashes_for_update << hash_for_update end @@ -119,10 +117,8 @@ def create_records!(inventory_collection, all_attribute_keys, batch, attributes_ create_time = time_now batch.each do |index, inventory_object| hash = attributes_index.delete(index).symbolize_keys - hash[:type] = inventory_collection.model_class.name if inventory_collection.supports_sti? && hash[:type].blank? - hash[:created_on] = create_time if inventory_collection.supports_timestamps_on_variant? - hash[:created_at] = create_time if inventory_collection.supports_timestamps_at_variant? - hash[:last_sync_on] = create_time if inventory_collection.supports_last_sync_on? + assign_attributes_for_create!(hash, inventory_collection, create_time) + next unless assert_referential_integrity(hash, inventory_object) hashes << hash From 193dd122758bc141162893c6a35061bb31dfa77b Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Mon, 19 Jun 2017 11:29:03 +0200 Subject: [PATCH 6/6] Use Time.zone.now instead of localtime Use Time.zone.now instead of localtime --- app/models/manager_refresh/save_collection/saver/base.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/manager_refresh/save_collection/saver/base.rb b/app/models/manager_refresh/save_collection/saver/base.rb index 0d2b124c95a..245c080e2d1 100644 --- a/app/models/manager_refresh/save_collection/saver/base.rb +++ b/app/models/manager_refresh/save_collection/saver/base.rb @@ -100,7 +100,7 @@ def time_now if ActiveRecord::Base.default_timezone == :utc Time.now.utc else - Time.now.localtime + Time.zone.now end end