Skip to content

Commit

Permalink
removal of generated methods in favor of hash lookup
Browse files Browse the repository at this point in the history
  • Loading branch information
klobuczek committed Apr 9, 2020
1 parent a3977f1 commit 2e68718
Show file tree
Hide file tree
Showing 13 changed files with 38 additions and 55 deletions.
2 changes: 1 addition & 1 deletion activegraph.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ DESCRIPTION
s.add_development_dependency('guard-rspec')
s.add_development_dependency('guard-rubocop')
s.add_development_dependency('neo4j-rake_tasks', '>= 0.3.0')
s.add_development_dependency("neo4j-#{ENV['driver'] == 'java' ? 'java' : 'ruby'}-driver", '>= 0.3.5')
s.add_development_dependency("neo4j-#{ENV['driver'] == 'java' ? 'java' : 'ruby'}-driver", '>= 0.4.0')
s.add_development_dependency('os')
s.add_development_dependency('pry')
s.add_development_dependency('railties', '>= 4.0')
Expand Down
13 changes: 5 additions & 8 deletions lib/active_graph/core/query_find_in_batches.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def find_in_batches(node_var, prop_var, options = {})

primary_key_var = ActiveGraph::Core::QueryClauses::Clause.from_key_and_single_value(node_var, prop_var)
records = query.where("#{primary_key_var} > $primary_key_offset")
.params(primary_key_offset: primary_key_offset).to_a
.params(primary_key_offset: primary_key_offset).to_a
end
end

Expand All @@ -36,13 +36,10 @@ def validate_find_in_batches_options!(options)
end

def primary_key_offset(last_record, node_var, prop_var)
last_record.send(node_var).send(prop_var)
rescue NoMethodError
begin
last_record.send(node_var).properties[prop_var.to_sym]
rescue NoMethodError
last_record.send("#{node_var}.#{prop_var}") # In case we're explicitly returning it
end
node = last_record[node_var]
return node.send(prop_var) if node&.respond_to?(prop_var)
return node.properties[prop_var.to_sym] if node&.respond_to?(:properties)
last_record["#{node_var}.#{prop_var}"] # In case we're explicitly returning it
end
end
end
Expand Down
15 changes: 0 additions & 15 deletions lib/active_graph/core/record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,6 @@ def wrap(value)
def wrap?
@wrap
end

# TODO: Remove this and :[]
# Too much unnecessary confusion and method names like `n.name`, `count(n)`
def method_missing(name, *args)
if respond_to_missing?(name)
raise ArgumentError if args.present?
self[name]
else
super
end
end

def respond_to_missing?(name, include_private = false)
keys.include?(name)
end
end
end
end
12 changes: 6 additions & 6 deletions lib/active_graph/core/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ def version
result = query('CALL dbms.components()', {}, skip_instrumentation: true)

# BTW: community / enterprise could be retrieved via `result.first.edition`
result.first.versions[0]
result.first[:versions][0]
end

def indexes
result = query('CALL db.indexes()', {}, skip_instrumentation: true)

result.map do |row|
{ type: row.type.to_sym, label: label(result, row), properties: properties(row), state: row.state.to_sym }
{ type: row[:type].to_sym, label: label(result, row), properties: properties(row), state: row[:state].to_sym }
end
end

Expand All @@ -27,15 +27,15 @@ def constraints
private

def v4_filter(row)
row.uniqueness == 'UNIQUE'
row[:uniqueness] == 'UNIQUE'
end

def v3_filter(row)
row.type == 'node_unique_property'
row[:type] == 'node_unique_property'
end

def label(result, row)
(v34?(result) ? row.label : (v4?(result) ? row.labelsOrTypes : row.tokenNames).first).to_sym
(v34?(result) ? row.label : (v4?(result) ? row[:labelsOrTypes] : row[:tokenNames]).first).to_sym
end

def v4?(result)
Expand All @@ -49,7 +49,7 @@ def v34?(result)
end

def properties(row)
row.properties.map(&:to_sym)
row[:properties].map(&:to_sym)
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions lib/active_graph/node/persistence.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def create_model
# @return [ActiveGraph::Node] A CypherNode or EmbeddedNode
def _create_node(node_props, labels = labels_for_create)
query = "CREATE (n:`#{Array(labels).join('`:`')}`) SET n = $props RETURN n"
neo4j_query(query, {props: node_props}, wrap: false).to_a[0].n
neo4j_query(query, {props: node_props}, wrap: false).to_a[0][:n]
end

# As the name suggests, this inserts the primary key (id property) into the properties hash.
Expand Down Expand Up @@ -157,7 +157,7 @@ def find_or_initialize_by(attributes)
def load_entity(id)
query = query_base_for(id, :n).return(:n)
result = neo4j_query(query).first
result && result.n
result && result[:n]
end

def query_base_for(neo_id, var = :n)
Expand Down
4 changes: 2 additions & 2 deletions lib/active_graph/node/query/query_proxy_find_in_batches.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ module Query
module QueryProxyFindInBatches
def find_in_batches(options = {})
query.return(identity).find_in_batches(identity, @model.primary_key, options) do |batch|
yield batch.map(&identity)
yield batch.map { |record| record[identity] }
end
end

def find_each(options = {})
query.return(identity).find_each(identity, @model.primary_key, options) do |result|
yield result.send(identity)
yield result[identity]
end
end
end
Expand Down
3 changes: 2 additions & 1 deletion lib/active_graph/node/query/query_proxy_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ def include?(other, target = nil)
"#{var}.#{association_id_key} = $other_node_id"
end
node_id = other.respond_to?(:neo_id) ? other.neo_id : other
self.where(where_filter).params(other_node_id: node_id).query.reorder.return("count(#{var}) as count").first.count > 0
self.where(where_filter).params(other_node_id: node_id).query.reorder.return("count(#{var}) as count")
.first[:count] > 0
end
end

Expand Down
6 changes: 3 additions & 3 deletions lib/active_graph/node/query_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def last
def count(distinct = nil)
fail(ActiveGraph::InvalidParameterError, ':count accepts the `:distinct` symbol or nil as a parameter') unless distinct.nil? || distinct == :distinct
q = distinct.nil? ? 'n' : 'DISTINCT n'
self.query_as(:n).return("count(#{q}) AS count").first.count
self.query_as(:n).return("count(#{q}) AS count").first[:count]
end

alias size count
Expand All @@ -39,13 +39,13 @@ def empty?

def find_in_batches(options = {})
self.query_as(:n).return(:n).find_in_batches(:n, primary_key, options) do |batch|
yield batch.map(&:n)
yield batch.map { |record| record[:n] }
end
end

def find_each(options = {})
self.query_as(:n).return(:n).find_each(:n, primary_key, options) do |batch|
yield batch.n
yield batch[:n]
end
end

Expand Down
6 changes: 3 additions & 3 deletions lib/active_graph/relationship/persistence/query_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def build!
node_before_callbacks! do
res = query_factory(rel, rel_id, iterative_query).query.unwrapped.return(*unpersisted_return_ids).first
node_symbols.each { |n| wrap!(send(n), res, n) }
@unwrapped_rel = res.send(rel_id)
@unwrapped_rel = res[rel_id]
end
end

Expand Down Expand Up @@ -83,8 +83,8 @@ def unpersisted_return_ids
# in, making the object reflect as "persisted".
# @param [Symbol] key :from_node or :to_node, the object to request from the response.
def wrap!(node, res, key)
return if node.persisted? || !res.respond_to?(key)
unwrapped = res.send(key)
return if node.persisted? || !res.keys.include?(key)
unwrapped = res[key]
node.init_on_load(unwrapped, unwrapped.props)
end

Expand Down
2 changes: 1 addition & 1 deletion lib/active_graph/relationship/query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def find_by_id(key)
query = ActiveGraph::Base.new_query
result = query.match('()-[r]-()').where('ID(r)' => key.to_i).limit(1).return(:r).first
fail RecordNotFound.new("Couldn't find #{name} with 'id'=#{key.inspect}", name, key) if result.blank?
result.r
result[:r]
end

# Performs a very basic match on the relationship.
Expand Down
20 changes: 10 additions & 10 deletions spec/active_graph/base_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,19 @@
append 'CREATE (n:Label2) RETURN n'
end

expect(result[0].to_a[0].n).to be_a(Neo4j::Driver::Types::Node)
expect(result[1].to_a[0].n).to be_a(Neo4j::Driver::Types::Node)
expect(result[0].to_a[0].n.labels.to_a).to eq([:Label1])
expect(result[1].to_a[0].n.labels).to eq([:Label2])
expect(result[0].to_a[0][:n]).to be_a(Neo4j::Driver::Types::Node)
expect(result[1].to_a[0][:n]).to be_a(Neo4j::Driver::Types::Node)
expect(result[0].to_a[0][:n].labels.to_a).to eq([:Label1])
expect(result[1].to_a[0][:n].labels).to eq([:Label2])
end

it 'allows for building with Query API' do
result = subject.queries do
append query.create(n: {Label1: {}}).return(:n)
end

expect(result[0].to_a[0].n).to be_a(Neo4j::Driver::Types::Node)
expect(result[0].to_a[0].n.labels).to eq([:Label1])
expect(result[0].to_a[0][:n]).to be_a(Neo4j::Driver::Types::Node)
expect(result[0].to_a[0][:n].labels).to eq([:Label1])
end
end

Expand All @@ -45,7 +45,7 @@ def create_object_by_id(id, tx)

def get_object_by_id(id)
first = subject.query('MATCH (t:Temporary {id: $id}) RETURN t', id: id).first
first && first.t
first && first[:t]
end

it 'logs one query per query_set in transaction' do
Expand Down Expand Up @@ -219,11 +219,11 @@ def get_object_by_id(id)

expect(result).to be_a(Enumerable)
expect(result.count).to be(1)
expect(result.to_a[0].obj).to eq(a: 1)
expect(result.to_a[0][:obj]).to eq(a: 1)
end

describe 'parameter input and output' do
subject { ActiveGraph::Base.query('WITH $param AS param RETURN param', param: param).first.param }
subject { ActiveGraph::Base.query('WITH $param AS param RETURN param', param: param).first[:param] }

[
# Integers
Expand Down Expand Up @@ -273,7 +273,7 @@ def get_object_by_id(id)
"MERGE path=(n:Foo {a: 1})-[r:foo {b: 2}]->(b:Foo)
RETURN #{return_clause} AS result"
end
subject { described_class.query(query, {}, wrap: wrap).to_a[0].result }
subject { described_class.query(query, {}, wrap: wrap).to_a[0][:result] }

[true, false].each do |type|
let_context wrap: type do
Expand Down
4 changes: 2 additions & 2 deletions spec/e2e/node_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ def true_results?(node, verb)

person2 = neo4j_query('MATCH (p:Person) WHERE ID(p) = $neo_id RETURN p',
{neo_id: person.neo_id},
wrap: false).first.p
wrap: false).first[:p]
expect(person2.props).to match hash_including age: 22, name: 'andreas'
end

Expand Down Expand Up @@ -719,7 +719,7 @@ def true_results?(node, verb)
query = new_query.match(p: :Person)
.where(p: {neo_id: person.neo_id})
.return('p.datetime AS datetime')
ActiveGraph::Base.query(query).first.datetime
ActiveGraph::Base.query(query).first[:datetime]
end

it 'saves as date/time string by default' do
Expand Down
2 changes: 1 addition & 1 deletion spec/e2e/relationship_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ class ActiveRelSpecTypesInheritedRelClass < ActiveRelSpecTypesAutomaticRelType
end

it 'should use the Relationship class' do
expect(result[0].value).to eq('default_value')
expect(result[0][:value]).to eq('default_value')
end
end

Expand Down

0 comments on commit 2e68718

Please sign in to comment.