From f0ff80bad3da2068d050d06c405a5024a5260bc7 Mon Sep 17 00:00:00 2001
From: Andrey Deryabin <deriabin@gmail.com>
Date: Thu, 15 Oct 2015 17:59:50 +0300
Subject: [PATCH] refactor check attribute name

---
 lib/lotus/entity.rb | 19 ++++++++-----------
 test/entity_test.rb | 20 ++++++++++++++++++++
 2 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/lib/lotus/entity.rb b/lib/lotus/entity.rb
index 2b2f4486..860131e2 100644
--- a/lib/lotus/entity.rb
+++ b/lib/lotus/entity.rb
@@ -140,15 +140,13 @@ module ClassMethods
       #   DeletedUser.attributes => #<Set: {:id, :name, :deleted_at}>
       #
       def attributes(*attrs)
-        if attrs.any?
-          attrs = Lotus::Utils::Kernel.Array(attrs)
-          self.attributes.merge attrs
+        return @attributes ||= Set.new unless attrs.any?
 
-          attrs.each do |attr|
-            define_attr_accessor(attr) if defined_attribute?(attr)
+        Lotus::Utils::Kernel.Array(attrs).each do |attr|
+          if allowed_attribute_name?(attr)
+            define_attr_accessor(attr)
+            self.attributes << attr
           end
-        else
-          @attributes ||= Set.new
         end
       end
 
@@ -164,11 +162,10 @@ def define_attr_accessor(attr)
 
       # Check if attr_reader define the given attribute
       #
-      # @since 0.3.1
+      # @since 0.5.1
       # @api private
-      def defined_attribute?(name)
-        name == :id ||
-          !instance_methods.include?(name)
+      def allowed_attribute_name?(name)
+        !instance_methods.include?(name)
       end
 
       protected
diff --git a/test/entity_test.rb b/test/entity_test.rb
index aaf1167c..5452e18a 100644
--- a/test/entity_test.rb
+++ b/test/entity_test.rb
@@ -38,6 +38,11 @@ class Camera
       Car.attributes.must_equal Set.new([:id, :model])
     end
 
+    it 'rejects existed instance methods' do
+      Car.attributes :object_id
+      Car.attributes.must_equal Set.new([:id])
+    end
+
     describe 'params is array' do
       it 'defines attributes' do
         Car.attributes [:model]
@@ -46,6 +51,21 @@ class Camera
     end
   end
 
+  describe '.allowed_attribute_name?' do
+    it 'returns true if attrubute not defined' do
+      Car.allowed_attribute_name?(:model).must_equal true
+    end
+
+    it 'returns false if attribute defined' do
+      Car.attributes :model
+      Car.allowed_attribute_name?(:model).must_equal false
+    end
+
+    it 'returns false for reserved word' do
+      Car.allowed_attribute_name?(:to_json).must_equal false
+    end
+  end
+
   describe '#initialize' do
     describe 'with defined attributes' do
       it 'accepts given attributes' do