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

Faster element duplication #2066

Merged
merged 13 commits into from
Apr 13, 2021
Merged
4 changes: 2 additions & 2 deletions app/models/alchemy/content/factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ module Content::Factory
extend ActiveSupport::Concern

module ClassMethods
SKIPPED_ATTRIBUTES_ON_COPY = %w(position created_at updated_at creator_id updater_id id)
SKIPPED_ATTRIBUTES_ON_COPY = %w(position created_at updated_at creator_id updater_id element_id id)

# Builds a new content as descriped in the elements.yml file.
#
Expand Down Expand Up @@ -54,7 +54,7 @@ def create(attributes = {})
#
def copy(source, differences = {})
Content.new(
source.attributes.
source.attributes.with_indifferent_access.
except(*SKIPPED_ATTRIBUTES_ON_COPY).
merge(differences.with_indifferent_access)
).tap do |new_content|
Expand Down
42 changes: 1 addition & 41 deletions app/models/alchemy/element.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,6 @@ class Element < BaseRecord
"deprecated",
].freeze

SKIPPED_ATTRIBUTES_ON_COPY = [
"cached_tag_list",
"created_at",
"creator_id",
"id",
"folded",
"position",
"updated_at",
"updater_id",
].freeze

# All Elements that share the same page version and parent element and are fixed or not are considered a list.
#
# If parent_element_id is nil (typical case for a simple page),
Expand Down Expand Up @@ -163,26 +152,7 @@ def new(attributes = {})
# @copy.public? # => false
#
def copy(source_element, differences = {})
attributes = source_element.attributes.with_indifferent_access
.except(*SKIPPED_ATTRIBUTES_ON_COPY)
.merge(differences)
.merge({
autogenerate_contents: false,
autogenerate_nested_elements: false,
tag_list: source_element.tag_list,
})

new_element = create!(attributes)

if source_element.contents.any?
source_element.copy_contents_to(new_element)
end

if source_element.nested_elements.any?
source_element.copy_nested_elements_to(new_element)
end

new_element
DuplicateElement.new(source_element).call(differences)
end

def all_from_clipboard(clipboard)
Expand Down Expand Up @@ -313,16 +283,6 @@ def nestable_elements
definition.fetch("nestable_elements", [])
end

# Copy all nested elements from current element to given target element.
def copy_nested_elements_to(target_element)
nested_elements.map do |nested_element|
Element.copy(nested_element, {
parent_element_id: target_element.id,
page_version_id: target_element.page_version_id,
})
end
end

private

def generate_nested_elements
Expand Down
4 changes: 4 additions & 0 deletions app/models/alchemy/elements_repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ def limit(limit)
self.class.new elements[0..(limit.to_i - 1)]
end

def children_of(parent)
self.class.new(select { |e| e.parent_element_id == parent.id })
end

def each(&blk)
elements.each(&blk)
end
Expand Down
13 changes: 10 additions & 3 deletions app/models/alchemy/page/publisher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,16 @@ def publish!(public_on:)
version = public_version(public_on)
DeleteElements.new(version.elements).call

# We must not use .find_each here to not mess up the order of elements
page.draft_version.elements.not_nested.available.each do |element|
Element.copy(element, page_version_id: version.id)
repository = page.draft_version.element_repository
ActiveRecord::Base.no_touching do
Element.acts_as_list_no_update do
repository.visible.not_nested.each.with_index do |element, position|
tvdeyen marked this conversation as resolved.
Show resolved Hide resolved
Alchemy::DuplicateElement.new(element, repository: repository).call(
page_version_id: version.id,
position: position
)
end
end
end
end
end
Expand Down
4 changes: 4 additions & 0 deletions app/models/alchemy/page_version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ def still_public_for?(time = Time.current)
public_until.nil? || public_until >= time
end

def element_repository
ElementsRepository.new(elements.includes({ contents: :essence }, :tags))
end

private

def delete_elements
Expand Down
51 changes: 51 additions & 0 deletions app/services/alchemy/duplicate_element.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# frozen_string_literal: true

module Alchemy
class DuplicateElement
SKIPPED_ATTRIBUTES_ON_COPY = [
"cached_tag_list",
"created_at",
"creator_id",
"position",
"id",
"folded",
"updated_at",
"updater_id",
].freeze

attr_reader :source_element, :repository

def initialize(source_element, repository: source_element.page_version.element_repository)
@source_element = source_element
@repository = repository
end

def call(differences = {})
attributes = source_element.attributes.with_indifferent_access
.except(*SKIPPED_ATTRIBUTES_ON_COPY)
.merge(differences)
.merge(
autogenerate_contents: false,
autogenerate_nested_elements: false,
tags: source_element.tags,
)

new_element = Element.create(attributes)

source_element.contents.map do |content|
Content.copy(content, element: new_element)
end

nested_elements = repository.children_of(source_element)
nested_elements.each.with_index do |nested_element, position|
tvdeyen marked this conversation as resolved.
Show resolved Hide resolved
self.class.new(nested_element, repository: repository).call(
parent_element: new_element,
page_version: new_element.page_version,
position: position
)
end

new_element
end
end
end
2 changes: 1 addition & 1 deletion lib/alchemy/essence.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def acts_as_essence(options = {})
delegate :restricted?, to: :page, allow_nil: true
delegate :public?, to: :element, allow_nil: true

after_save :touch_element
after_update :touch_element

def acts_as_essence_class
#{name}
Expand Down
5 changes: 2 additions & 3 deletions spec/models/alchemy/attachment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,12 @@ module Alchemy
let(:attachment) { create(:alchemy_attachment) }

let(:content) do
create(:alchemy_content, :essence_file).tap do |content|
content.element.update_column(:updated_at, 3.hours.ago)
end
create(:alchemy_content, :essence_file)
end

before do
content.essence.update(attachment: attachment)
content.element.update_column(:updated_at, 3.hours.ago)
end

it "touches elements" do
Expand Down
6 changes: 6 additions & 0 deletions spec/models/alchemy/elements_repository_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -243,4 +243,10 @@

it_behaves_like "being chainable"
end

describe "#children_of" do
subject { repo.children_of(visible_element) }

it { is_expected.to match_array([nested_element]) }
end
end
10 changes: 10 additions & 0 deletions spec/models/alchemy/page_version_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -137,5 +137,15 @@
expect(Alchemy::EssenceRichtext.count).to be_zero
end
end

describe "#element_repository" do
let(:page_version) { create(:alchemy_page_version, :with_elements) }
subject { page_version.element_repository }

it "is an element repository containing the pages elements" do
expect(Alchemy::ElementsRepository).to receive(:new).with(page_version.elements).and_call_original
expect(subject).to be_a(Alchemy::ElementsRepository)
end
end
end
end
5 changes: 2 additions & 3 deletions spec/models/alchemy/picture_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -512,13 +512,12 @@ module Alchemy
let(:picture) { create(:alchemy_picture) }

let(:content) do
create(:alchemy_content, :essence_picture).tap do |content|
content.element.update_column(:updated_at, 3.hours.ago)
end
create(:alchemy_content, :essence_picture)
end

before do
content.essence.update(picture: picture)
content.element.update_column(:updated_at, 3.hours.ago)
end

it "touches elements" do
Expand Down
74 changes: 74 additions & 0 deletions spec/services/alchemy/duplicate_element_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# frozen_string_literal: true

require "rails_helper"

RSpec.describe Alchemy::DuplicateElement do
let(:element) do
create(:alchemy_element, :with_contents, tag_list: "red, yellow")
end
let(:differences) { {} }
subject { described_class.new(element).call(differences) }

it "should not create contents from scratch" do
expect(subject.contents.count).to eq(element.contents.count)
end

context "with differences" do
let(:new_page_version) { create(:alchemy_page_version) }
let(:differences) { { page_version_id: new_page_version.id } }

it "should create a new record with all attributes of source except given differences" do
expect(subject.page_version_id).to eq(new_page_version.id)
end
end

it "should make copies of all contents of source" do
expect(subject.contents).not_to be_empty
expect(subject.contents.pluck(:id)).not_to eq(element.contents.pluck(:id))
end

it "the copy should include source element tags" do
expect(subject.tag_list).to eq(element.tag_list)
end

context "with nested elements" do
let(:element) do
create(:alchemy_element, :with_contents, :with_nestable_elements, {
tag_list: "red, yellow",
page: create(:alchemy_page),
})
end

before do
element.nested_elements << create(:alchemy_element, name: "slide")
end

it "should copy nested elements" do
expect(subject.nested_elements).to_not be_empty
end

context "copy to new page version" do
let(:new_page_version) { create(:alchemy_page_version) }
let(:differences) { { page_version_id: new_page_version.id } }

it "should set page_version id to new page_version's id" do
subject.nested_elements.each do |nested_element|
expect(nested_element.page_version_id).to eq(new_page_version.id)
end
end
end

context "copy to new page version" do
let(:public_version) do
element.page.versions.create!(public_on: Time.current)
end
let(:differences) { { page_version_id: public_version.id } }

it "sets page_version id" do
subject.nested_elements.each do |nested_element|
expect(nested_element.page_version_id).to eq(public_version.id)
end
end
end
end
end