Skip to content

Commit

Permalink
Merge pull request ManageIQ#15394 from Ladas/batch_saving_strategy_sh…
Browse files Browse the repository at this point in the history
…ould_populate_the_right_timestamps

Batch saving strategy should populate the right timestamps
  • Loading branch information
agrare committed Jun 20, 2017
2 parents e09b617 + 193dd12 commit c32abc7
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 4 deletions.
23 changes: 23 additions & 0 deletions app/models/manager_refresh/inventory_collection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 26 additions & 0 deletions app/models/manager_refresh/save_collection/saver/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,32 @@ 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.zone.now
end
end

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
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand All @@ -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])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)

Expand All @@ -47,7 +52,10 @@ 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)
assign_attributes_for_update!(hash_for_update, inventory_collection, update_time)

hashes_for_update << hash_for_update
end
end

Expand Down Expand Up @@ -106,9 +114,11 @@ 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?
assign_attributes_for_create!(hash, inventory_collection, create_time)

next unless assert_referential_integrity(hash, inventory_object)

hashes << hash
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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))
}
Expand Down

0 comments on commit c32abc7

Please sign in to comment.