Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Batch saving strategy should populate the right timestamps #15394

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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