:fix: Name conflicts between columns and nosql attributes not detected

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 All notable changes to this project made by Monade Team are documented in this file. For info refer to [email protected]
 
+## [0.1.1]
+### Fixed
+- Name conflicts between columns and nosql attributes where not detected
+- Added more specs
+
 ## [0.1.0]
 First release with basic querying and nested objects.

lib/acts_as_nosql/attributes.rb

@@ -4,10 +4,10 @@ module Attributes
     extend ActiveSupport::Concern
 
     included do
-      after_initialize :_acts_as_nosql_init
+      after_initialize :_acts_as_nosql_init_defaults
     end
 
-    def _acts_as_nosql_init
+    def _acts_as_nosql_init_defaults
       self.class.nosql_attributes.each do |name, attribute|
         public_send("#{name}=", attribute.default.dup) if public_send(name).nil? && !attribute.default.nil?
       end
@@ -22,7 +22,7 @@ def nosql_attr(*names, type: nil, default: nil, path: nil)
         attribute = self._acts_as_nosql_options[:field_name]
 
         names.each do |name|
-          raise "Attribute #{name} already defined" if instance_methods.include?(name.to_sym) || name.to_sym == attribute.to_sym
+          raise "Attribute #{name} already defined" if _acts_as_nosql_attribute_defined?(name)
 
           nosql_attributes[name] = ActsAsNosql::Attribute.new(name, type: type, default: default, path: path)
           _acts_as_nosql_define_attribute(nosql_attributes[name])
@@ -33,12 +33,36 @@ def nosql_attributes
         self._acts_as_nosql_options[:attributes] ||= {}
       end
 
+      def connection
+        unless acts_as_nosql_conflicts_checked?
+          @acts_as_nosql_conflicts_checked = true
+          acts_as_nosql_check_conflicts!
+        end
+        super
+      end
+
+      def acts_as_nosql_conflicts_checked?
+        @acts_as_nosql_conflicts_checked ||= false
+      end
+
+      def acts_as_nosql_check_conflicts!
+        columns_map = columns.index_by(&:name)
+        nosql_attributes.each do |name, attribute|
+          raise "Attribute #{name} already defined" if columns_map[name.to_s]
+        end
+      end
+
       private
 
       def nosql_data_attribute
         @nosql_data_attribute ||= self._acts_as_nosql_options[:field_name]
       end
 
+      def _acts_as_nosql_attribute_defined?(name)
+        instance_methods.include?(name.to_sym) ||
+          name.to_sym == nosql_data_attribute.to_sym
+      end
+
       def _acts_as_nosql_define_attribute(nosql_attribute)
         attribute = nosql_data_attribute
 

lib/acts_as_nosql/version.rb

@@ -1,3 +1,3 @@
 module ActsAsNosql
-  VERSION = '0.1.0'.freeze
+  VERSION = '0.1.1'.freeze
 end

spec/acts_as_nosql/model_spec.rb

@@ -34,11 +34,45 @@
     expect { Article.nosql_attr :data }.to raise_error("Attribute data already defined")
   end
 
+  it 'raises error if there\'s a name conflict an actual column' do
+    expect { Article.nosql_attr :title }.to raise_error("Attribute title already defined")
+  end
+
+  it 'raises error if there\'s a name conflict an existing nosql attribute' do
+    expect { Article.nosql_attr :body }.to raise_error("Attribute body already defined")
+  end
+
   it 'raises error if there\'s a name conflict' do
     expect { Article.nosql_attr :some_column }.to raise_error("Attribute some_column already defined")
   end
 
-  it 'fileds are saved as string' do
+  it 'raises error if there\'s a name conflict when calling #acts_as_nosql_check_conflicts!' do
+    expect do
+      class ConflictingArticle < ActiveRecord::Base
+        self.table_name = 'articles'
+        acts_as_nosql field_name: :data
+
+        nosql_attrs :title, :editor, type: String
+      end
+    end.not_to raise_error("Attribute title already defined")
+
+    expect { ConflictingArticle.acts_as_nosql_check_conflicts! }.to raise_error("Attribute title already defined")
+  end
+
+  it 'raises error if there\'s a name conflict when the model columns are loaded' do
+    expect do
+      class ConflictingArticle2 < ActiveRecord::Base
+        self.table_name = 'articles'
+        acts_as_nosql field_name: :data
+
+        nosql_attrs :title, :editor, type: String
+      end
+    end.not_to raise_error("Attribute title already defined")
+
+    expect { ConflictingArticle2.new }.to raise_error("Attribute title already defined")
+  end
+
+  it 'fields are saved as string' do
     subject.editor = 'John Doe'
     subject.save!
     subject.reload

spec/acts_as_nosql/querying_spec.rb

@@ -16,9 +16,16 @@
 
     it 'can be queried' do
       query = Article.where(body: 'body')
-      # expect(query.to_sql).to eq(
-      #   "SELECT \"articles\".* FROM \"articles\" WHERE (\"articles\".\"data\"->>\"body\" = 'body')"
-      # )
+      expect(query.to_sql).to include(
+        case ENV['ACTIVE_RECORD_ADAPTER']
+        when 'postgresql'
+          "SELECT \"articles\".* FROM \"articles\" WHERE (\"articles\".\"data\"->>'body' = 'body')"
+        when 'mysql'
+          "SELECT `articles`.* FROM `articles` WHERE (`articles`.`data`->>'$.body' = 'body')"
+        else
+          "SELECT \"articles\".* FROM \"articles\" WHERE (\"articles\".\"data\"->>'$.body' = 'body')"
+        end
+      )
       expect(query.to_a).to contain_exactly(article)
     end
   end
@@ -36,9 +43,16 @@
 
     it 'can query nested attributes' do
       query = Setting.where(user_auth_token: '123123')
-      # expect(query.to_sql).to eq(
-      #   "SELECT \"settings\".* FROM \"settings\" WHERE (\"settings\".\"config\"->>\"user\"->>\"auth\"->>\"token\" = '123123')"
-      # )
+      expect(query.to_sql).to eq(
+        case ENV['ACTIVE_RECORD_ADAPTER']
+        when 'postgresql'
+          "SELECT \"settings\".* FROM \"settings\" WHERE (\"settings\".\"config\"->'user'->'auth'->>'token' = '123123')"
+        when 'mysql'
+          "SELECT `settings`.* FROM `settings` WHERE (`settings`.`config`->>'$.user.auth.token' = '123123')"
+        else
+          "SELECT \"settings\".* FROM \"settings\" WHERE (\"settings\".\"config\"->>'$.user.auth.token' = '123123')"
+        end
+      )
       expect(query.to_a).to contain_exactly(setting)
     end
   end