Merge pull request #49 from guanglunw/object_creation_performance

Fix object instantiation performance issue

README.md

@@ -702,7 +702,7 @@ would output the following url
 
 ### Automatic Conversion of Fields to Date/DateTime
 
-By default Flexirest will attempt to convert all fields to a Date or DateTime object if it's a string and the value matches a pair of regular expressions. However, on large responses this can be computationally expensive.  So, you can either disable this automatic conversion completely with:
+By default Flexirest will attempt to convert all fields to a Date or DateTime object if it's a string and the value matches a pair of regular expressions. However, on large responses this can be computationally expensive.  You can disable this automatic conversion completely with:
 
 ```ruby
 Flexirest::Base.disable_automatic_date_parsing = true
@@ -716,6 +716,21 @@ class Person < Flexirest::Base
 end
 ```
 
+It is also possible to whitelist fields should be parsed in your models, which is useful if you are instantiating these objects directly. The specified fields also apply automatically to request mapping.
+
+```ruby
+class Person < Flexirest::Base
+  parse_date :updated_at, :created_at
+end
+
+# to disable all mapping
+class Disabled < Flexirest::Base
+  parse_date :none
+end
+```
+
+This system respects `disable_automatic_date_parsing`, and will default to mapping everything - unless a `parse_date` whitelist is specified, or automatic parsing is globally disabled.
+
 ### Raw Requests
 
 Sometimes you have have a URL that you just want to force through, but have the response handled in the same way as normal objects or you want to have the callbacks run (say for authentication).  The easiest way to do that is to call `_request` on the class:

lib/flexirest/associations.rb

@@ -50,6 +50,19 @@ def has_one(key, klass = nil)
           _attributes[key]
         end
       end
+
+      def parse_date(*keys)
+        keys.each { |key| @_date_fields << key }
+      end
+
+      def _date_fields
+        @_date_fields.uniq
+      end
+
+      def inherited(subclass)
+        subclass.instance_variable_set(:@_date_fields, [])
+        super
+      end
     end
 
     def self.included(base)

lib/flexirest/base.rb

@@ -26,7 +26,7 @@ def initialize(attrs={})
 
       attrs.each do |attribute_name, attribute_value|
         attribute_name = attribute_name.to_sym
-        @attributes[attribute_name] = parse_attribute_value(attribute_value)
+        @attributes[attribute_name] = parse_date?(attribute_name) ? parse_attribute_value(attribute_value) : attribute_value
         @dirty_attributes << attribute_name
       end
     end
@@ -175,6 +175,12 @@ def value_for_inspect(value)
       end
     end
 
+    def parse_date?(name)
+      return true if self.class._date_fields.include?(name)
+      return true if !Flexirest::Base.disable_automatic_date_parsing && self.class._date_fields.empty?
+      false
+    end
+
   end
 
   class NoAttributeException < StandardError ; end

lib/flexirest/request.rb

@@ -479,9 +479,11 @@ def new_object(attributes, name = nil)
             end
           end
         else
-          if @method[:options][:parse_fields] && @method[:options][:parse_fields].include?(k)
+          parse_fields = [ @method[:options][:parse_fields], @object._date_fields ].compact.reduce([], :|)
+          parse_fields = nil if parse_fields.empty?
+          if (parse_fields && parse_fields.include?(k))
             object._attributes[k] = parse_attribute_value(v)
-          elsif @method[:options][:parse_fields]
+          elsif parse_fields
             object._attributes[k] = v
           elsif Flexirest::Base.disable_automatic_date_parsing
             object._attributes[k] = v

spec/lib/associations_spec.rb

@@ -29,6 +29,16 @@ class DeepNestedHasManyExample < Flexirest::Base
   get :find, "/iterate", fake: hash.to_json
 end
 
+class WhitelistedDateExample < Flexirest::Base
+  parse_date :updated_at
+end
+
+class WhitelistedDateMultipleExample < Flexirest::Base
+  parse_date :updated_at, :created_at
+  parse_date :generated_at
+end
+
+
 describe "Has Many Associations" do
   let(:subject) {AssociationExampleBase.new}
 
@@ -108,3 +118,29 @@ class DeepNestedHasManyExample < Flexirest::Base
     expect(subject.child.nested_child.test).to eq("foo")
   end
 end
+
+describe "whitelisted date fields" do
+  context "no whitelist specified" do
+    let(:subject) {AssociationExampleNested}
+
+    it "should show whitelist as empty array" do
+      expect(subject._date_fields).to eq([])
+    end
+  end
+
+  context "whitelist specified" do
+    let(:subject) {WhitelistedDateExample}
+
+    it "should contain whitelisted field" do
+      expect(subject._date_fields).to eq([:updated_at])
+    end
+  end
+
+  context "multiple attributes whitelisted" do
+    let(:subject) {WhitelistedDateMultipleExample}
+
+    it "should contain all fields" do
+      expect(subject._date_fields).to match_array([:updated_at, :created_at, :generated_at])
+    end
+  end
+end

spec/lib/base_spec.rb

@@ -42,6 +42,11 @@ class InstanceMethodExample < Flexirest::Base
   get :all, "/all"
 end
 
+class WhitelistedDateExample < Flexirest::Base
+  parse_date :updated_at
+end
+
+
 describe Flexirest::Base do
   it 'should instantiate a new descendant' do
     expect{EmptyExample.new}.to_not raise_error
@@ -431,4 +436,22 @@ class DummyExample < Flexirest::Base
     end
   end
 
+  describe "instantiating object" do
+    context "no whitelist specified" do
+      it "should convert dates automatically" do
+        client = EmptyExample.new(test: Time.now.iso8601)
+        expect(client["test"]).to be_an_instance_of(DateTime)
+      end
+    end
+
+    context "whitelist specified" do
+      it "should only convert specified dates" do
+        time = Time.now.iso8601
+        client = WhitelistedDateExample.new(updated_at: time, created_at: time)
+        expect(client["updated_at"]).to be_an_instance_of(DateTime)
+        expect(client["created_at"]).to be_an_instance_of(String)
+      end
+    end
+  end
+
 end

spec/lib/request_spec.rb

@@ -99,6 +99,12 @@ class IgnoredRootExampleClient < ExampleClient
       }
     end
 
+    class WhitelistedDateClient < Flexirest::Base
+      base_url "http://www.example.com"
+      put :conversion, "/put/:id"
+      parse_date :converted
+    end
+
     allow_any_instance_of(Flexirest::Request).to receive(:read_cached_response)
   end
 
@@ -287,6 +293,13 @@ class IgnoredRootExampleClient < ExampleClient
     expect(object.not_converted).to be_an_instance_of(String)
   end
 
+  it "should convert date times in JSON if whitelisted" do
+    expect_any_instance_of(Flexirest::Connection).to receive(:put).with("/put/1234", "debug=true", an_instance_of(Hash)).and_return(::FaradayResponseMock.new(OpenStruct.new(body:"{\"converted\":\"2012-03-04T01:02:03Z\", \"not_converted\":\"2012-03-04T01:02:03Z\"}", response_headers:{})))
+    object = WhitelistedDateClient.conversion id:1234, debug:true
+    expect(object.converted).to be_an_instance_of(DateTime)
+    expect(object.not_converted).to be_an_instance_of(String)
+  end
+
   it "should parse JSON and return a nice object for faked responses" do
     object = ExampleClient.fake id:1234, debug:true
     expect(object.result).to eq(true)