[MODEL] [STORE] Fixed the problem where document_type configuration was not propagated to mapping

This PR tries to fix the bug described in elastic#270

The problem was the following:

* Once a model extends Persistence::Model it sets up two attributes. As a consequence, the attribute macro instantiates a Elasticsearch::Model::Indexing::Mappings in the model. This instance contains an attribute @type that defaults to the model name in lowercase.

* Any other definition of document_type will override the document_type in the gateway delegate, but the mappings instance originally created in the previous step remains untouched, and hence in an inconsistent state.

What I did here is: Instead of simply forward document_type to the gateway, also change the mappings' document_type accordingly, leaving it in a correct state.

An integration test was modified, in order to have a regression of the fix.

Unit tests were added.

Fixes elastic#270
Closes elastic#366

Author: miguelff

elasticsearch-model/lib/elasticsearch/model/indexing.rb

@@ -34,7 +34,7 @@ def as_json(options={})
       # Wraps the [index mappings](http://www.elasticsearch.org/guide/en/elasticsearch/reference/current/mapping.html)
       #
       class Mappings
-        attr_accessor :options
+        attr_accessor :options, :type
 
         def initialize(type, options={})
           raise ArgumentError, "`type` is missing" if type.nil?

elasticsearch-persistence/lib/elasticsearch/persistence/model.rb

@@ -71,7 +71,6 @@ def gateway(&block)
             delegate :settings,
                      :mappings,
                      :mapping,
-                     :document_type,
                      :document_type=,
                      :index_name,
                      :index_name=,
@@ -80,6 +79,13 @@ def gateway(&block)
                      :create_index!,
                      :refresh_index!,
               to: :gateway
+
+            # forward document type to mappings when set
+            def document_type(type = nil)
+              return gateway.document_type unless type
+              gateway.document_type type
+              mapping.type = type
+            end
           end
 
           # Configure the repository based on the model (set up index_name, etc)

elasticsearch-persistence/test/integration/model/model_basic_test.rb

@@ -12,6 +12,7 @@ class ::Person
         include Elasticsearch::Persistence::Model::Rails
 
         settings index: { number_of_shards: 1 }
+        document_type 'human_being'
 
         attribute :name, String,
                   mapping: { fields: {
@@ -60,7 +61,7 @@ class ::Person
           assert_equal 'John Smith', document.name
           assert_equal 'John Smith', Person.find(person.id).name
 
-          assert_not_nil Elasticsearch::Persistence.client.get index: 'people', type: 'person', id: person.id
+          assert_not_nil Elasticsearch::Persistence.client.get index: 'people', type: 'human_being', id: person.id
         end
 
         should "not save an invalid object" do

elasticsearch-persistence/test/unit/model_base_test.rb

@@ -20,29 +20,53 @@ class DummyBaseModel
     end
 
     should "set the ID from attributes during initialization" do
-      m = DummyBaseModel.new id: 1
-      assert_equal 1, m.id
+      model = DummyBaseModel.new id: 1
+      assert_equal 1, model.id
 
-      m = DummyBaseModel.new 'id' => 2
-      assert_equal 2, m.id
+      model = DummyBaseModel.new 'id' => 2
+      assert_equal 2, model.id
     end
 
     should "set the ID using setter method" do
-      m = DummyBaseModel.new id: 1
-      assert_equal 1, m.id
+      model = DummyBaseModel.new id: 1
+      assert_equal 1, model.id
 
-      m.id = 2
-      assert_equal 2, m.id
+      model.id = 2
+      assert_equal 2, model.id
     end
 
     should "have ID in attributes" do
-      m = DummyBaseModel.new id: 1, name: 'Test'
-      assert_equal 1, m.attributes[:id]
+      model = DummyBaseModel.new id: 1, name: 'Test'
+      assert_equal 1, model.attributes[:id]
     end
 
     should "have the customized inspect method" do
-      m = DummyBaseModel.new name: 'Test'
-      assert_match /name\: "Test"/, m.inspect
+      model = DummyBaseModel.new name: 'Test'
+      assert_match /name\: "Test"/, model.inspect
+    end
+
+    context "with custom document_type" do
+      setup do
+        @model = DummyBaseModel
+        @gateway = mock()
+        @mapping = mock()
+        @model.stubs(:gateway).returns(@gateway)
+        @gateway.stubs(:mapping).returns(@mapping)
+        @document_type = 'dummybase'
+      end
+
+      should "forward the argument to mapping" do
+        @gateway.expects(:document_type).with(@document_type).once
+        @mapping.expects(:type=).with(@document_type).once
+        @model.document_type @document_type
+      end
+
+      should "return the value from the gateway" do
+        @gateway.expects(:document_type).once.returns(@document_type)
+        @mapping.expects(:type=).never
+        returned_type = @model.document_type
+        assert_equal @document_type, returned_type
+      end
     end
   end
 end