Fixed String property to auto-migrate the property length column

[#1106 state:resolved]

lib/dm-core/migrations.rb

@@ -243,7 +243,7 @@ def create_unique_index_statements(model)
         def property_schema_hash(property)
           schema = (self.class.type_map[property.type] || self.class.type_map[property.primitive]).merge(:name => property.field)
 
-          if property.primitive == String && schema[:primitive] != 'TEXT' && schema[:primitive] != 'CLOB' && schema[:primitive != 'NVARCHAR']
+          if property.primitive == String && schema[:primitive] != 'TEXT' && schema[:primitive] != 'CLOB' && schema[:primitive] != 'NVARCHAR'
             schema[:length] = property.length
           elsif property.primitive == BigDecimal || property.primitive == Float
             schema[:precision] = property.precision
@@ -257,10 +257,10 @@ def property_schema_hash(property)
             # remove the default if the property is not nullable
             schema.delete(:default) unless property.nullable?
           else
-            if property.type.respond_to?(:dump)
-              schema[:default] = property.type.dump(property.default, property)
+            schema[:default] = if property.type.respond_to?(:dump)
+              property.type.dump(property.default, property)
             else
-              schema[:default] = property.default
+              property.default
             end
           end
 
@@ -275,12 +275,10 @@ def property_schema_statement(connection, schema)
 
           if schema[:precision] && schema[:scale]
             statement << "(#{[ :precision, :scale ].map { |key| connection.quote_value(schema[key]) }.join(', ')})"
+          elsif schema[:length] == 'max'
+            statement << '(max)'
           elsif schema[:length]
-            unless schema[:length] == 'max'
-              statement << "(#{connection.quote_value(schema[:length])})"
-            else
-              statement << "(max)"
-            end
+            statement << "(#{connection.quote_value(schema[:length])})"
           end
 
           statement << " DEFAULT #{connection.quote_value(schema[:default])}" if schema.key?(:default)

spec/public/migrations_spec.rb

@@ -243,6 +243,38 @@ class Article
           end
         end
       end
+
+      describe 'String property' do
+        before :all do
+          @model.property(:id, DataMapper::Types::Serial)
+        end
+
+        [
+          [ 1,          'VARCHAR(1)'   ],
+          [ 50,         'VARCHAR(50)'  ],
+          [ 255,        'VARCHAR(255)' ],
+          [ nil,        'VARCHAR(50)'  ],
+        ].each do |length, statement|
+          options = {}
+          options[:length] = length if length
+
+          describe "with a length of #{length}" do
+            before :all do
+              @property = @model.property(:title, String, options)
+
+              @response = capture_log(DataObjects::Mysql) { @model.auto_migrate! }
+            end
+
+            it 'should return true' do
+              @response.should be_true
+            end
+
+            it "should create a #{statement} column" do
+              @output.last.should =~ %r{\ACREATE TABLE `blog_articles` \(`id` INT\(10\) UNSIGNED NOT NULL AUTO_INCREMENT, `title` #{Regexp.escape(statement)}, PRIMARY KEY\(`id`\)\) ENGINE = InnoDB CHARACTER SET [a-z\d]+ COLLATE (?:[a-z\d](?:_?[a-z\d]+)*)\z}
+            end
+          end
+        end
+      end
     end
   end
 
@@ -354,6 +386,38 @@ class Article
           end
         end
       end
+
+      describe 'String property' do
+        before :all do
+          @model.property(:id, DataMapper::Types::Serial)
+        end
+
+        [
+          [ 1,          'VARCHAR(1)'   ],
+          [ 50,         'VARCHAR(50)'  ],
+          [ 255,        'VARCHAR(255)' ],
+          [ nil,        'VARCHAR(50)'  ],
+        ].each do |length, statement|
+          options = {}
+          options[:length] = length if length
+
+          describe "with a length of #{length}" do
+            before :all do
+              @property = @model.property(:title, String, options)
+
+              @response = capture_log(DataObjects::Postgres) { @model.auto_migrate! }
+            end
+
+            it 'should return true' do
+              @response.should be_true
+            end
+
+            it "should create a #{statement} column" do
+              @output[-2].should == "CREATE TABLE \"blog_articles\" (\"id\" SERIAL NOT NULL, \"title\" #{statement}, PRIMARY KEY(\"id\"))"
+            end
+          end
+        end
+      end
     end
   end
 
@@ -430,6 +494,10 @@ class Article
           end
         end
       end
+
+      describe 'String property' do
+        it 'needs specs'
+      end
     end
   end
 end