Fixes Schema Synchronzation for at least neo4j >= 4.3 (#1683)

* Fixed schema synchronize

* fixed spec for ruby 3.2

* removed jruby-9.4.0.0 from build matrix. Too low quality of the jruby release.

* made test password 8 characters long

* upgraded jruby and neo4j

* adapted to fixed driver requires

* version bump and changelog

---------

Co-authored-by: klobuczek

.github/workflows/test.yml

@@ -17,14 +17,14 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        ruby: [ jruby-9.3.9.0, ruby-3.1.3 ]
-        neo4j: [ 3.5.35, 4.0.12, 4.1.12, 4.2.19, 4.3.22, 4.4.14, 5.3.0 ]
+        ruby: [ jruby-9.3.10.0, ruby-3.1.3, ruby-3.2.0 ]
+        neo4j: [ 3.5.35, 4.0.12, 4.1.12, 4.2.19, 4.3.23, 4.4.17, 5.4.0 ]
     env:
       NEO4J_VERSION: ${{ matrix.neo4j }}
       JRUBY_OPTS: --debug -J-Xmx1280m -Xcompile.invokedynamic=false -J-XX:+TieredCompilation -J-XX:TieredStopAtLevel=1 -J-noverify -Xcompile.mode=OFF
     steps:
       - name: Start neo4j
-        run: docker run --name neo4j --env NEO4J_AUTH=neo4j/pass --env NEO4J_ACCEPT_LICENSE_AGREEMENT=yes --env NEO4J_dbms_security_auth__minimum__password__length=4 --env NEO4J_dbms_directories_import= -p7687:7687 -p7474:7474 -v `pwd`/tmp:/var/lib/neo4j/import --rm neo4j:${{ matrix.neo4j }}-enterprise &
+        run: docker run --name neo4j --env NEO4J_AUTH=neo4j/password --env NEO4J_ACCEPT_LICENSE_AGREEMENT=yes --env NEO4J_dbms_directories_import= -p7687:7687 -p7474:7474 -v `pwd`/tmp:/var/lib/neo4j/import --rm neo4j:${{ matrix.neo4j }}-enterprise &
 
       - uses: actions/checkout@v3
 

CHANGELOG.md

@@ -3,6 +3,12 @@ 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/).
 
+## [11.2.0] 2023-02-06
+
+## Added
+
+- support for neo4j:schema:dump and :load for neo4j 4 and 5. No automatic migration of schema on major neo4j upgrades. Schema must be regenerated with neo4j:schema:dump or manually adjusted to new syntax on those upgrades.
+
 ## [11.1.0] 2023-01-10
 
 ## Added

docs/activegraph.rb

@@ -1,15 +1,13 @@
 # Usage: rails new myapp -m activegraph.rb
 
-gem 'activegraph', '~> 10.0.1'
-gem 'neo4j-ruby-driver', '~> 1.7.0'
+gem 'activegraph', '=> 11.1'
 
 gem_group :development do
   gem 'neo4j-rake_tasks'
 end
 
 inject_into_file 'config/application.rb', before: '# Require the gems listed in Gemfile' do <<END
 require 'active_graph/railtie'
-require 'neo4j_ruby_driver'
 
 END
 end

lib/active_graph/core.rb

@@ -5,7 +5,7 @@
 require 'active_graph/core/record'
 require 'active_graph/core/wrappable'
 require 'active_graph/transaction'
-require 'neo4j_ruby_driver'
+require 'neo4j/driver'
 
 Neo4j::Driver::Types::Entity.include ActiveGraph::Core::Wrappable
 Neo4j::Driver::Types::Entity.prepend ActiveGraph::Core::Entity

lib/active_graph/core/schema.rb

@@ -19,17 +19,22 @@ def version?(requirement)
       end
 
       def indexes
-        raw_indexes.map do |row|
-          definition(row).merge(type: row[:type].to_sym, state: row[:state].to_sym)
+        normalize(raw_indexes, *%i[type state])
+      end
+
+      def normalize(result, *extra)
+        result.map do |row|
+          definition(row, version?('<4') ? :index_cypher_v3 : :index_cypher)
+            .merge(extra.to_h { |key| [key, row[key].to_sym] })
         end
       end
 
       def constraints
         if version?('<4.3')
-          raw_indexes.select(&method(:filter))
+          raw_indexes.select(&method(:constraint_owned?))
         else
           raw_constraints.select(&method(:constraint_filter))
-        end.map { |row| definition(row).merge(type: :uniqueness) }
+        end.map { |row| definition(row, :constraint_cypher).merge(type: :uniqueness) }
       end
 
       private def raw_constraints
@@ -45,22 +50,39 @@ def raw_indexes
         end
       end
 
+      def constraint_owned?(record)
+        FILTER[major]&.then { |(key, value)| record[key] == value } || record[:owningConstraint]
+      end
+
       private
 
       def major
         @major ||= version.segments.first
       end
 
-      def filter(record)
-        FILTER[major].then { |(key, value)| record[key] == value }
+      def constraint_filter(record)
+        %w[UNIQUENESS RELATIONSHIP_PROPERTY_EXISTENCE NODE_PROPERTY_EXISTENCE NODE_KEY].include?(record[:type])
       end
 
-      def constraint_filter(record)
-        record[:type] == 'UNIQUENESS'
+      def index_cypher_v3(label, properties)
+        "INDEX ON :#{label}#{com_sep(properties, nil)}"
+      end
+
+      def index_cypher(label, properties)
+        "INDEX FOR (n:#{label}) ON #{com_sep(properties)}"
+      end
+
+      def constraint_cypher(label, properties)
+        "CONSTRAINT ON (n:#{label}) ASSERT #{com_sep(properties)} IS UNIQUE"
+      end
+
+      def com_sep(properties, prefix = 'n.')
+        "(#{properties.map { |prop| "#{prefix}#{prop}" }.join(', ')})"
       end
 
-      def definition(row)
-        { label: label(row), properties: properties(row), name: row[:name] }
+      def definition(row, template)
+        { label: label(row), properties: properties(row), name: row[:name],
+          create_statement: row[:createStatement] || send(template,label(row), row[:properties]) }
       end
 
       def label(row)

lib/active_graph/migrations/schema.rb

@@ -3,62 +3,46 @@ module Migrations
     module Schema
       class << self
         def fetch_schema_data
-          { constraints: fetch_constraint_descriptions.sort, indexes: fetch_index_descriptions.sort }
+          %i[constraints indexes].to_h { |schema_elem| [schema_elem, send("fetch_#{schema_elem}_descriptions").keys] }
         end
 
         def synchronize_schema_data(schema_data, remove_missing)
-          queries = []
-          ActiveGraph::Base.read_transaction do
-            queries += drop_and_create_queries(fetch_constraint_descriptions, schema_data[:constraints], remove_missing)
-            queries += drop_and_create_queries(fetch_index_descriptions, schema_data[:indexes], remove_missing)
-          end
+          queries =
+            ActiveGraph::Base.read_transaction do
+              drop_and_create_queries(fetch_constraints_descriptions, schema_data[:constraints], 'CONSTRAINT', remove_missing) +
+                drop_and_create_queries(fetch_indexes_descriptions, schema_data[:indexes], 'INDEX', remove_missing)
+            end
           ActiveGraph::Base.write_transaction do
             queries.each(&ActiveGraph::Base.method(:query))
           end
         end
 
         private
 
-        def fetch_constraint_descriptions
-          ActiveGraph::Base.query('CALL db.constraints() YIELD description').map(&:first)
+        def fetch_indexes_descriptions
+            ActiveGraph::Base.raw_indexes.reject(&ActiveGraph::Base.method(:constraint_owned?))
+                             .then(&ActiveGraph::Base.method(:normalize)).then(&method(:fetch_descriptions))
         end
 
-        def fetch_index_descriptions
-          ActiveGraph::Base.raw_indexes do |keys, result|
-            if keys.include?(:description)
-              v3_indexes(result)
-            else
-              v4_indexes(result)
-            end
-          end
+        def fetch_constraints_descriptions
+          fetch_descriptions(ActiveGraph::Base.constraints)
         end
 
-        def v3_indexes(result)
-          result.reject do |row|
-            # These indexes are created automagically when the corresponding constraints are created
-            row[:type] == 'node_unique_property'
-          end.map { |row| row[:description] }
+        def fetch_descriptions(results)
+          results.map { |definition| definition.values_at(:create_statement, :name) }.sort.to_h
         end
 
-        def v4_indexes(result)
-          result.reject do |row|
-            # These indexes are created automagically when the corresponding constraints are created
-            row[:uniqueness] == 'UNIQUE'
-          end.map(&method(:description))
+        def drop_and_create_queries(existing, specified, schema_elem, remove_missing)
+          (remove_missing ? existing.except(*specified).map { |stmt, name| drop_statement(schema_elem, stmt, name) } : []) +
+            (specified - existing.keys).map(&method(:create_statement))
         end
 
-        def description(row)
-          "INDEX FOR (n:#{row[:labelsOrTypes].first}) ON (#{row[:properties].map { |prop| "n.#{prop}" }.join(', ')})"
+        def drop_statement(schema_elem, create_statement, name)
+          "DROP #{name&.then { |name| "#{schema_elem} #{name}" } || create_statement}"
         end
 
-        def drop_and_create_queries(existing, specified, remove_missing)
-          [].tap do |queries|
-            if remove_missing
-              (existing - specified).each { |description| queries << "DROP #{description}" }
-            end
-
-            (specified - existing).each { |description| queries << "CREATE #{description}" }
-          end
+        def create_statement(stmt)
+          stmt.start_with?('CREATE ') ? stmt : "CREATE #{stmt}"
         end
       end
     end

lib/active_graph/version.rb

@@ -1,3 +1,3 @@
 module ActiveGraph
-  VERSION = '11.1.0'
+  VERSION = '11.2.0'
 end

spec/spec_helper.rb

@@ -190,7 +190,7 @@ def let_context(*args, &block)
 def set_default_driver
   server_url = ENV['NEO4J_URL'] || 'bolt://localhost:7687'
   ActiveGraph::Base.driver =
-    Neo4j::Driver::GraphDatabase.driver(server_url, Neo4j::Driver::AuthTokens.basic('neo4j', 'pass'))
+    Neo4j::Driver::GraphDatabase.driver(server_url, Neo4j::Driver::AuthTokens.basic('neo4j', 'password'))
 end
 
 set_default_driver

spec/unit/migrations/schema_spec.rb

@@ -0,0 +1,126 @@
+require 'active_graph/migrations/schema'
+
+describe ActiveGraph::Migrations::Schema do
+  before { delete_schema }
+
+  subject do
+    described_class.synchronize_schema_data(schema_data, remove_missing)
+    described_class.fetch_schema_data
+  end
+
+  let(:schema_data) { { indexes: indexes, constraints: constraints } }
+  let(:remove_missing) { false }
+  let(:all_indexes) { [range_index, point_index, fulltext_index, text_index].compact.sort }
+  let(:all_constraints) { [unique_constraint, not_null_rel_prop_constraint, not_null_node_prop_constraint, node_key_constraint].compact.sort }
+  let(:indexes) { [] }
+  let(:constraints) { [] }
+
+  let(:range_index) { 'CREATE RANGE INDEX `range_index` FOR (n:`Person`) ON (n.`nickname`)' }
+  let(:point_index) { "CREATE POINT INDEX `point_index` FOR (n:`Person`) ON (n.`location`) OPTIONS {indexConfig: {`spatial.cartesian-3d.max`: [1000000.0, 1000000.0, 1000000.0],`spatial.cartesian-3d.min`: [-1000000.0, -1000000.0, -1000000.0],`spatial.cartesian.max`: [1000000.0, 1000000.0],`spatial.cartesian.min`: [-1000000.0, -1000000.0],`spatial.wgs-84-3d.max`: [180.0, 90.0, 1000000.0],`spatial.wgs-84-3d.min`: [-180.0, -90.0, -1000000.0],`spatial.wgs-84.max`: [180.0, 90.0],`spatial.wgs-84.min`: [-180.0, -90.0]}, indexProvider: 'point-1.0'}" }
+  let(:fulltext_index) { "CREATE FULLTEXT INDEX `fulltext_index` FOR (n:`Friend`) ON EACH [n.`name`] OPTIONS {indexConfig: {`fulltext.analyzer`: 'swedish',`fulltext.eventually_consistent`: false}, indexProvider: 'fulltext-1.0'}" }
+  let(:text_index) { "CREATE TEXT INDEX `text_index` FOR ()-[r:`KNOWS`]-() ON (r.`city`) OPTIONS {indexConfig: {}, indexProvider: 'text-2.0'}" }
+
+  let(:unique_constraint) { "CREATE CONSTRAINT `unique_constraint` FOR (n:`Person`) REQUIRE (n.`name`) IS UNIQUE OPTIONS {indexConfig: {}, indexProvider: 'range-1.0'}" }
+  let(:not_null_rel_prop_constraint) { 'CREATE CONSTRAINT `not_null_rel_prop_constraint` FOR ()-[r:`LIKED`]-() REQUIRE (r.`when`) IS NOT NULL' }
+  let(:not_null_node_prop_constraint) { 'CREATE CONSTRAINT `not_null_node_prop_constraint` FOR (n:`Person`) REQUIRE (n.`name`) IS NOT NULL' }
+  let(:node_key_constraint) { "CREATE CONSTRAINT `node_key_constraint` FOR (n:`Person`) REQUIRE (n.`name`, n.`surname`) IS NODE KEY OPTIONS {indexConfig: {}, indexProvider: 'range-1.0'}" }
+
+  if ActiveGraph::Base.version?('<5')
+    let(:text_index) { "CREATE TEXT INDEX `text_index` FOR ()-[r:`KNOWS`]-() ON (r.`city`)" }
+  end
+
+  if ActiveGraph::Base.version?('<4.4')
+    let(:range_index) { "CREATE INDEX `range_index` FOR (n:`Person`) ON (n.`nickname`) OPTIONS {indexConfig: {`spatial.cartesian-3d.max`: [1000000.0, 1000000.0, 1000000.0],`spatial.cartesian-3d.min`: [-1000000.0, -1000000.0, -1000000.0],`spatial.cartesian.max`: [1000000.0, 1000000.0],`spatial.cartesian.min`: [-1000000.0, -1000000.0],`spatial.wgs-84-3d.max`: [180.0, 90.0, 1000000.0],`spatial.wgs-84-3d.min`: [-180.0, -90.0, -1000000.0],`spatial.wgs-84.max`: [180.0, 90.0],`spatial.wgs-84.min`: [-180.0, -90.0]}, indexProvider: 'native-btree-1.0'}" }
+    let(:point_index) {}
+    let(:text_index) {}
+
+    let(:unique_constraint) { "CREATE CONSTRAINT `unique_constraint` ON (n:`Person`) ASSERT (n.`name`) IS UNIQUE OPTIONS {indexConfig: {`spatial.cartesian-3d.max`: [1000000.0, 1000000.0, 1000000.0],`spatial.cartesian-3d.min`: [-1000000.0, -1000000.0, -1000000.0],`spatial.cartesian.max`: [1000000.0, 1000000.0],`spatial.cartesian.min`: [-1000000.0, -1000000.0],`spatial.wgs-84-3d.max`: [180.0, 90.0, 1000000.0],`spatial.wgs-84-3d.min`: [-180.0, -90.0, -1000000.0],`spatial.wgs-84.max`: [180.0, 90.0],`spatial.wgs-84.min`: [-180.0, -90.0]}, indexProvider: 'native-btree-1.0'}" }
+    let(:not_null_rel_prop_constraint) { 'CREATE CONSTRAINT `not_null_rel_prop_constraint` ON ()-[r:`LIKED`]-() ASSERT (r.`when`) IS NOT NULL' }
+    let(:not_null_node_prop_constraint) { 'CREATE CONSTRAINT `not_null_node_prop_constraint` ON (n:`Person`) ASSERT (n.`name`) IS NOT NULL' }
+    let(:node_key_constraint) { "CREATE CONSTRAINT `node_key_constraint` ON (n:`Person`) ASSERT (n.`name`, n.`surname`) IS NODE KEY OPTIONS {indexConfig: {`spatial.cartesian-3d.max`: [1000000.0, 1000000.0, 1000000.0],`spatial.cartesian-3d.min`: [-1000000.0, -1000000.0, -1000000.0],`spatial.cartesian.max`: [1000000.0, 1000000.0],`spatial.cartesian.min`: [-1000000.0, -1000000.0],`spatial.wgs-84-3d.max`: [180.0, 90.0, 1000000.0],`spatial.wgs-84-3d.min`: [-180.0, -90.0, -1000000.0],`spatial.wgs-84.max`: [180.0, 90.0],`spatial.wgs-84.min`: [-180.0, -90.0]}, indexProvider: 'native-btree-1.0'}" }
+  end
+
+  if ActiveGraph::Base.version?('<4.3')
+    let(:range_index) { "INDEX FOR (n:Person) ON (n.nickname)" }
+    let(:fulltext_index) {}
+
+    let(:unique_constraint) { "CONSTRAINT ON (n:Person) ASSERT (n.name) IS UNIQUE" }
+    let(:not_null_rel_prop_constraint) {}
+    let(:not_null_node_prop_constraint) {}
+    let(:node_key_constraint) {}
+  end
+
+  if ActiveGraph::Base.version?('<4')
+    let(:range_index) { "INDEX ON :Person(nickname)" }
+  end
+
+
+    context 'empty' do
+    it { is_expected.to eq schema_data }
+  end
+
+  context 'empty with removal' do
+    let(:remove_missing) { true }
+    it { is_expected.to eq schema_data }
+  end
+
+  context 'range index' do
+    let(:indexes) { [range_index].compact }
+    it { is_expected.to eq schema_data }
+  end
+
+  context 'point_index' do
+    let(:indexes) { [point_index].compact }
+    it { is_expected.to eq schema_data }
+  end
+
+  context 'fulltext_index' do
+    let(:indexes) { [fulltext_index].compact }
+    it { is_expected.to eq schema_data }
+  end
+
+  context 'text_index' do
+    let(:indexes) { [text_index].compact }
+    it { is_expected.to eq schema_data }
+  end
+
+  context 'unique_constraint' do
+    let(:constraints) { [unique_constraint].compact }
+    it { is_expected.to eq schema_data }
+  end
+
+  context 'not_null_rel_prop_constraint' do
+    let(:constraints) { [not_null_rel_prop_constraint].compact }
+    it { is_expected.to eq schema_data }
+  end
+
+  context 'not_null_node_prop_constraint' do
+    let(:constraints) { [not_null_node_prop_constraint].compact }
+    it { is_expected.to eq schema_data }
+  end
+
+  context 'node_key_constraint' do
+    let(:constraints) { [node_key_constraint].compact }
+    it { is_expected.to eq schema_data }
+  end
+
+  context 'indexes' do
+    let(:indexes) { all_indexes }
+    it { is_expected.to eq schema_data }
+  end
+
+  context 'constraint' do
+    let(:constraints) { all_constraints }
+    it { is_expected.to eq schema_data }
+  end
+
+  context 'drop missing' do
+    before do
+      described_class.synchronize_schema_data({ indexes: all_indexes, constraints: all_constraints }, false)
+    end
+    let(:indexes) { [range_index] }
+    let(:constraints) { [unique_constraint] }
+    let(:remove_missing) { true }
+    it { is_expected.to eq schema_data }
+  end
+end

spec/unit/relationship/rel_wrapper_spec.rb

@@ -5,7 +5,7 @@
   let(:start_node_id) { 1 }
   let(:end_node_id) { 2 }
 
-  let(:rel) { double(start_node_id: start_node_id, end_node_id: end_node_id, type: type, type: type, properties: properties) }
+  let(:rel) { double(start_node_id: start_node_id, end_node_id: end_node_id, type: type, properties: properties) }
   subject { ActiveGraph::RelWrapping.wrapper(rel) }
 
   it { should eq(rel) }

spec/unit/shared/attributes_spec.rb

@@ -124,7 +124,7 @@ def self.name
       end
 
       it 'returns false when compared to another type' do
-        is_expected.not_to eq Struct.new(:attributes).new('first_name' => 'Ben')
+        is_expected.not_to eq Struct.new(:attributes).new({'first_name' => 'Ben'})
       end
     end