Merge pull request neo4jrb#1034 from neo4jrb/classname_removal

_classname removal

CHANGELOG.md

@@ -3,7 +3,10 @@ All notable changes to this project will be documented in this file.
 This file should follow the standards specified on [http://keepachangelog.com/]
 This project adheres to [Semantic Versioning](http://semver.org/).
 
-## [Unreleaed]
+## [Unreleased]
+
+### Changed
+- `_classname` property has been completely removed, officially dropping support for Neo4j < 2.1.5.
 
 ### Added
 

lib/neo4j/active_node/has_n/association.rb

@@ -128,12 +128,6 @@ def relationship_class
           @relationship_class ||= @relationship_class_name && @relationship_class_name.constantize
         end
 
-        def inject_classname(properties)
-          return properties unless relationship_class
-          properties[Neo4j::Config.class_name_property] = relationship_class_name if relationship_class.cached_class?(true)
-          properties
-        end
-
         def unique?
           return relationship_class.unique? if rel_class?
           @origin ? origin_association.unique? : !!@unique

lib/neo4j/active_node/node_wrapper.rb

@@ -15,7 +15,7 @@ def wrapper
 
     def class_to_wrap
       load_classes_from_labels
-      (named_class || ::Neo4j::ActiveNode::Labels.model_for_labels(labels)).tap do |model_class|
+      Neo4j::ActiveNode::Labels.model_for_labels(labels).tap do |model_class|
         Neo4j::Node::Wrapper.populate_constants_for_labels_cache(model_class, labels)
       end
     end
@@ -48,11 +48,5 @@ def self.populate_constants_for_labels_cache(model_class, labels)
     def self.association_model_namespace
       Neo4j::Config.association_model_namespace_string
     end
-
-    def named_class
-      property = Neo4j::Config.class_name_property
-
-      Neo4j::Node::Wrapper.constant_for_label(self.props[property]) if self.props.is_a?(Hash) && self.props.key?(property)
-    end
   end
 end

lib/neo4j/active_node/query/query_proxy.rb

@@ -178,8 +178,6 @@ def create(other_nodes, properties)
           fail 'Can only create relationships on associations' if !@association
           other_nodes = _nodeify!(*other_nodes)
 
-          properties = @association.inject_classname(properties)
-
           Neo4j::Transaction.run do
             other_nodes.each do |other_node|
               other_node.save unless other_node.neo_id
@@ -221,7 +219,7 @@ def _create_relationship_with_rel_class(other_node_or_nodes, properties)
           Array(other_node_or_nodes).each do |other_node|
             node_props = (association.direction == :in) ? {from_node: other_node, to_node: @start_object} : {from_node: @start_object, to_node: other_node}
 
-            association.relationship_class.create(properties.except(:_classname).merge(node_props))
+            association.relationship_class.create(properties.merge(node_props))
           end
         end
 

lib/neo4j/active_rel/rel_wrapper.rb

@@ -3,7 +3,7 @@ module Wrapper
     def wrapper
       props.symbolize_keys!
       begin
-        most_concrete_class = sorted_wrapper_classes
+        most_concrete_class = class_from_type
         wrapped_rel = most_concrete_class.constantize.new
       rescue NameError
         return self
@@ -15,10 +15,6 @@ def wrapper
 
     private
 
-    def sorted_wrapper_classes
-      props[Neo4j::Config.class_name_property] || class_from_type
-    end
-
     def class_from_type
       Neo4j::ActiveRel::Types::WRAPPED_CLASSES[rel_type] || Neo4j::ActiveRel::Types::WRAPPED_CLASSES[rel_type] = rel_type.camelize
     end

lib/neo4j/active_rel/types.rb

@@ -15,9 +15,6 @@ module Types
       #
       # A model is added to WRAPPED_CLASSES when it is initalized AND when the `type` class method is called within a model. This means that
       # it's possible a model will be added twice: once with the rel_type version of the model name, again with the custom type. deal_with_it.gif.
-      #
-      # As an alternative to this, you can call the `set_classname` class method to insert a `_classname` property into your relationship,
-      # which will completely bypass this whole process.
       WRAPPED_CLASSES = {}
 
       included do
@@ -76,7 +73,7 @@ def rel_type?
 
         def assign_type!(given_type, auto)
           @rel_type = (auto ? decorated_rel_type(given_type) : given_type).tap do |type|
-            add_wrapped_class(type) unless uses_classname?
+            add_wrapped_class(type)
           end
         end
       end

lib/neo4j/config.rb

@@ -96,10 +96,6 @@ def to_yaml
         configuration.to_yaml
       end
 
-      def class_name_property
-        @_class_name_property = Neo4j::Config[CLASS_NAME_PROPERTY_KEY] || :_classname
-      end
-
       def include_root_in_json
         # we use ternary because a simple || will always evaluate true
         Neo4j::Config[:include_root_in_json].nil? ? true : Neo4j::Config[:include_root_in_json]

lib/neo4j/migration.rb

@@ -123,97 +123,5 @@ def new_id_for(model)
         end
       end
     end
-
-    class AddClassnames < Neo4j::Migration
-      def initialize(path = default_path)
-        @classnames_filename = 'add_classnames.yml'
-        @classnames_filepath = File.join(joined_path(path), classnames_filename)
-      end
-
-      def migrate
-        output 'Adding classnames. This make take some time.'
-        execute(true)
-      end
-
-      def test
-        output 'TESTING! No queries will be executed.'
-        execute(false)
-      end
-
-      def setup
-        output "Creating file #{classnames_filepath}. Please use this as the migration guide."
-        FileUtils.mkdir_p('db/neo4j-migrate')
-
-        return if File.file?(classnames_filepath)
-
-        source = File.join(File.dirname(__FILE__), '..', '..', 'config', 'neo4j', classnames_filename)
-        FileUtils.copy_file(source, classnames_filepath)
-      end
-
-      private
-
-      attr_reader :classnames_filename, :classnames_filepath, :model_map
-
-      def execute(migrate = false)
-        file_init
-        map = []
-        map.push :nodes         if model_map[:nodes]
-        map.push :relationships if model_map[:relationships]
-        map.each do |type|
-          model_map[type].each do |action, labels|
-            do_classnames(action, labels, type, migrate)
-          end
-        end
-      end
-
-      def do_classnames(action, labels, type, migrate = false)
-        method = type == :nodes ? :node_cypher : :rel_cypher
-        labels.each do |label|
-          output cypher = self.send(method, label, action)
-          execute_cypher(cypher) if migrate
-        end
-      end
-
-      def file_init
-        @model_map = ActiveSupport::HashWithIndifferentAccess.new(YAML.load_file(classnames_filepath))
-      end
-
-      def node_cypher(label, action)
-        where, phrase_start = action_variables(action, 'n')
-        output "#{phrase_start} _classname '#{label}' on nodes with matching label:"
-        "MATCH (n:`#{label}`) #{where} SET n._classname = '#{label}' RETURN COUNT(n) as modified"
-      end
-
-      def rel_cypher(hash, action)
-        label = hash[0]
-        value = hash[1]
-        from = value[:from]
-        fail "All relationships require a 'type'" unless value[:type]
-
-        from_cypher = from ? "(from:`#{from}`)" : '(from)'
-        to = value[:to]
-        to_cypher = to ? "(to:`#{to}`)" : '(to)'
-        type = "[r:`#{value[:type]}`]"
-        where, phrase_start = action_variables(action, 'r')
-        output "#{phrase_start} _classname '#{label}' where type is '#{value[:type]}' using cypher:"
-        "MATCH #{from_cypher}-#{type}->#{to_cypher} #{where} SET r._classname = '#{label}' return COUNT(r) as modified"
-      end
-
-      def execute_cypher(query_string)
-        output "Modified #{Neo4j::Session.query(query_string).first.modified} records"
-        output ''
-      end
-
-      def action_variables(action, identifier)
-        case action
-        when 'overwrite'
-          ['', 'Overwriting']
-        when 'add'
-          ["WHERE NOT HAS(#{identifier}._classname)", 'Adding']
-        else
-          fail "Invalid action #{action} specified"
-        end
-      end
-    end
   end
 end

lib/neo4j/shared/persistence.rb

@@ -2,8 +2,6 @@ module Neo4j::Shared
   module Persistence
     extend ActiveSupport::Concern
 
-    USES_CLASSNAME = []
-
     # @return [Hash] Given a node's state, will call the appropriate `props_for_{action}` method.
     def props_for_persistence
       _persisted_obj ? props_for_update : props_for_create
@@ -18,7 +16,6 @@ def update_model
     # Returns a hash containing:
     # * All properties and values for insertion in the database
     # * A `uuid` (or equivalent) key and value
-    # * A `_classname` property, if one is to be set
     # * Timestamps, if the class is set to include them.
     # Note that the UUID is added to the hash but is not set on the node.
     # The timestamps, by comparison, are set on the node prior to addition in this hash.
@@ -27,7 +24,6 @@ def props_for_create
       inject_timestamps!
       props_with_defaults = inject_defaults!(props)
       converted_props = props_for_db(props_with_defaults)
-      inject_classname!(converted_props)
       return converted_props unless self.class.respond_to?(:default_property_values)
       inject_primary_key!(converted_props)
     end
@@ -187,61 +183,16 @@ def update_magic_properties
       self.updated_at = DateTime.now if respond_to?(:updated_at=) && (updated_at.nil? || (changed? && !updated_at_changed?))
     end
 
-    # Inserts the _classname property into an object's properties during object creation.
-    def inject_classname!(props, check_version = true)
-      props[:_classname] = self.class.name if self.class.cached_class?(check_version)
-    end
-
-    def set_classname(props, check_version = true)
-      warning = 'This method has been replaced with `inject_classname!` and will be removed in a future version'.freeze
-      ActiveSupport::Deprecation.warn warning, caller
-      inject_classname!(props, check_version)
-    end
-
     def inject_timestamps!
       now = DateTime.now
       self.created_at ||= now if respond_to?(:created_at=)
       self.updated_at ||= now if respond_to?(:updated_at=)
     end
 
-
-
     def set_timestamps
       warning = 'This method has been replaced with `inject_timestamps!` and will be removed in a future version'.freeze
       ActiveSupport::Deprecation.warn warning, caller
       inject_timestamps!
     end
-
-    module ClassMethods
-      # Determines whether a model should insert a _classname property. This can be used to override the automatic matching of returned
-      # objects to models.
-      def cached_class?(check_version = true)
-        uses_classname? || (!!Neo4j::Config[:cache_class_names] && (check_version ? neo4j_session.version < '2.1.5' : true))
-      end
-
-      # @return [Boolean] status of whether this model will add a _classname property
-      def uses_classname?
-        Neo4j::Shared::Persistence::USES_CLASSNAME.include?(self.name)
-      end
-
-      # Adds this model to the USES_CLASSNAME array. When new rels/nodes are created, a _classname property will be added. This will override the
-      # automatic matching of label/rel type to model.
-      #
-      # You'd want to do this if you have multiple models for the same label or relationship type. When it comes to labels, there isn't really any
-      # reason to do this because you can have multiple labels; on the other hand, an argument can be made for doing this with relationships since
-      # rel type is a bit more restrictive.
-      #
-      # It could also be speculated that there's a slight performance boost to using _classname since the gem immediately knows what model is responsible
-      # for a returned object. At the same time, it is a bit restrictive and changing it can be a bit of a PITA. Use carefully!
-      def set_classname
-        Neo4j::Shared::Persistence::USES_CLASSNAME << self.name
-      end
-
-      # Removes this model from the USES_CLASSNAME array. When new rels/nodes are create, no _classname property will be injected. Upon returning of
-      # the object from the database, it will be matched to a model using its relationship type or labels.
-      def unset_classname
-        Neo4j::Shared::Persistence::USES_CLASSNAME.delete self.name
-      end
-    end
   end
 end

spec/e2e/active_model_spec.rb

@@ -328,75 +328,6 @@ def true_results?(node, verb)
     end
   end
 
-  describe 'cached classnames' do
-    after(:all) { Neo4j::Config[:cache_class_names] = true }
-    before do
-      stub_const('CacheTest', UniqueClass.create do
-        include Neo4j::ActiveNode
-      end)
-    end
-
-    context 'with cache_class set in config' do
-      before { Neo4j::Session.current.class.any_instance.stub(version: db_version) }
-
-      before do
-        Neo4j::Config[:cache_class_names] = true
-        @cached = CacheTest.create
-        @unwrapped = Neo4j::Node._load(@cached.neo_id)
-      end
-
-      context 'server version 2.1.4' do
-        let(:db_version) { '2.1.4' }
-
-        it 'responds true to :cached_class?' do
-          expect(CacheTest.cached_class?).to be_truthy
-        end
-
-        it 'sets _classname property equal to class name' do
-          expect(@unwrapped[:_classname]).to eq @cached.class.name
-        end
-
-        it 'removes the _classname property from the wrapped class' do
-          expect(@cached.props).to_not have_key(:_classname)
-        end
-      end
-
-      context 'server version 2.1.5' do
-        let(:db_version) { '2.1.5' }
-
-        it 'responds false to :cached_class?' do
-          expect(CacheTest.cached_class?).to be_falsey
-        end
-
-        it 'does not set _classname' do
-          expect(@unwrapped[:_classname]).to be_nil
-        end
-
-        it 'removes the _classname property from the wrapped class' do
-          expect(@cached.props).to_not have_key(:_classname)
-        end
-      end
-    end
-
-    context 'without cache_class set in model' do
-      before do
-        Neo4j::Config[:cache_class_names] = false
-        @uncached = CacheTest.create
-        @unwrapped = Neo4j::Node._load(@uncached.neo_id)
-      end
-
-      before { Neo4j::Config[:cache_class_names] = false }
-
-      it 'response false to :cached_class?' do
-        expect(CacheTest.cached_class?).to be_falsey
-      end
-
-      it 'does not set _classname on the node' do
-        expect(@unwrapped.props).to_not have_key(:_classname)
-      end
-    end
-  end
-
   before(:each) do
     stub_active_node_class('Person') do
       property :name