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

Performance Improvements #1109

Merged
merged 1 commit into from
Jul 18, 2016
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
1 change: 1 addition & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ Metrics/ClassLength:
- 'lib/active_fedora/reflection.rb'
- 'lib/active_fedora/orders/ordered_list.rb'
- 'lib/active_fedora/solr_service.rb'
- 'lib/active_fedora/associations/orders_association.rb'

Metrics/MethodLength:
Enabled: false
Expand Down
14 changes: 11 additions & 3 deletions lib/active_fedora/associations/builder/orders.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ def self.define_readers(mixin, name)
mixin.redefine_method(target_accessor(name)) do
association(name).target_reader
end
mixin.redefine_method("#{target_accessor(name, pluralize: false)}_ids") do
association(name).target_ids_reader
end
mixin.redefine_method("#{target_accessor(name)}=") do |nodes|
association(name).target_writer(nodes)
end
Expand All @@ -27,7 +30,7 @@ def self.build(model, name, options)
model.send(:define_method, :apply_first_and_last) do
source = send(options[:through])
source.save
return if head.map(&:rdf_subject) == source.head_id && tail.map(&:rdf_subject) == source.tail_id
return if head_ids == source.head_id && tail_ids == source.tail_id
self.head = source.head_id
self.tail = source.tail_id
save! if changed?
Expand All @@ -50,8 +53,13 @@ def save!(*args)
end
end

def self.target_accessor(name)
name.to_s.gsub("_proxies", "").pluralize
def self.target_accessor(name, pluralize: true)
name = name.to_s.gsub("_proxies", "")
if pluralize
name.pluralize
else
name
end
end
private_class_method :target_accessor

Expand Down
4 changes: 4 additions & 0 deletions lib/active_fedora/associations/orders_association.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ def target_reader
@target_proxy ||= ActiveFedora::Orders::TargetProxy.new(self)
end

def target_ids_reader
target_reader.ids
end

def find_reflection
reflection
end
Expand Down
2 changes: 1 addition & 1 deletion lib/active_fedora/fedora_attributes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def resource
# Appending the graph at the end is necessary because adding it as the
# parent leaves behind triples not related to the ldp_source's rdf
# subject.
@resource ||= self.class.resource_class.new(@ldp_source.graph.rdf_subject, @ldp_source.graph) << @ldp_source.graph
@resource ||= self.class.resource_class.new(@ldp_source.graph.rdf_subject, data: @ldp_source.graph.send(:graph).data)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should a ticket be opened upstream to change the visibility to public?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@no-reply Should we make AT::Resource's graph public, or just delegate data?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

end

# You can set the URI to use for the rdf_label on ClassMethods.rdf_label, then on
Expand Down
4 changes: 2 additions & 2 deletions lib/active_fedora/orders/list_node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class ListNode
attr_accessor :prev, :next, :target
attr_writer :next_uri, :prev_uri
attr_accessor :proxy_in, :proxy_for
def initialize(node_cache, rdf_subject, graph = ActiveTriples::Resource.new)
def initialize(node_cache, rdf_subject, graph = RDF::Repository.new)
@rdf_subject = rdf_subject
@graph = graph
@node_cache = node_cache
Expand Down Expand Up @@ -144,7 +144,7 @@ def populate(instance)
private

def resource
@resource ||= Resource.new(uri, graph)
@resource ||= Resource.new(uri, data: graph)
end
end

Expand Down
6 changes: 5 additions & 1 deletion lib/active_fedora/orders/ordered_list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ class OrderedList
# @param [::RDF::URI] head_subject URI of head node in list.
# @param [::RDF::URI] tail_subject URI of tail node in list.
def initialize(graph, head_subject, tail_subject)
@graph = graph
@graph = if graph.respond_to?(:graph, true)
graph.send(:graph).data
else
graph
end
@head_subject = head_subject
@tail_subject = tail_subject
@node_cache ||= NodeCache.new
Expand Down
4 changes: 4 additions & 0 deletions lib/active_fedora/orders/target_proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ def insert_at(loc, record)
self
end

def ids
association.reader.map(&:target_id)
end

# Deletes the element at the specified index, returning that element, or nil if
# the index is out of range.
def delete_at(loc)
Expand Down
1 change: 1 addition & 0 deletions spec/unit/ordered_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ class Image < ActiveFedora::Base
subject.save
subject.reload
expect(subject.ordered_members).to eq [member, member]
expect(subject.ordered_member_ids).to eq [member.id, member.id]
expect(subject.list_source.resource.query([nil, ::RDF::Vocab::ORE.proxyIn, subject.resource.rdf_subject]).to_a.length).to eq 2
expect(subject.head_ids).to eq subject.list_source.head_id
expect(subject.tail_ids).to eq subject.list_source.tail_id
Expand Down