Support ActiveRecord 7.0 #14
Conversation
In ActiveRecord 7, the `initialize_type_map` method has been moved to class method by rails/rails#42773 and armg no longer works correctly. Therefore, I have added a monkey patch to the `type_map` method. Because ActiveRecord 7 defines the type map statically when loaded, monkey patching class method does not make sense. https://github.com/rails/rails/blob/v7.0.3/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb#L616-L619 The `type_map` method is defined in the base class `ActiveRecord::ConnectionAdapters::AbstractAdapter`. It is the same for all versions of ActiveRecord supported by armg. And this fix works in any version.
There was a problem hiding this comment.
In old Rails (< 7.0), it seems like the geometry type is registered twice. The first time is when the old initialize_type_map has been called, and the second time is when the new type_map has been called. Is that right?
I think this behavior does not occur any problems because ActiveRecord::Type::TypeMap#register_type is idempotent.
https://github.com/rails/rails/blob/6-0-stable/activerecord/lib/active_record/type/type_map.rb#L32
But the monkey-patched initialized_type_map is no longer necessary and should be removed.
spec/mysql_helper.rb
Outdated
| @@ -28,7 +28,7 @@ def reset | |||
| def dump | |||
| buf = StringIO.new | |||
| ActiveRecord::SchemaDumper.dump(ActiveRecord::Base.connection, buf) | |||
| buf = buf.string.sub(/\A.*\bActiveRecord::Schema\.define\(version: \d+\) do/m, '').sub(/end\s*\z/, '') | |||
| buf = buf.string.sub(/\A.*\bActiveRecord::Schema(?:\[[\d.]+?\])?\.define\(version: \d+\) do/m, '').sub(/end\s*\z/, '') | |||
There was a problem hiding this comment.
Can we write this regular expression strict and simpler?
- Replace
.(any character) with\.(dot itself) - Remove the first
?because the second?matches the case of[7.0]missing
| buf = buf.string.sub(/\A.*\bActiveRecord::Schema(?:\[[\d.]+?\])?\.define\(version: \d+\) do/m, '').sub(/end\s*\z/, '') | |
| buf = buf.string.sub(/\A.*\bActiveRecord::Schema(?:\[[\d\.]+\])?\.define\(version: \d+\) do/m, '').sub(/end\s*\z/, '') |
There was a problem hiding this comment.
- Replace
.(any character) with\.(dot itself)
Escaping the dot is redundant, and this will match only the dot as expected.
- Remove the first
?because the second?matches the case of[7.0]missing
Added a commit to remove it.
No longer necessary because geometry type is registered in monkey-patched `type_map` method.
Because it is unnecessary.
You are right. |
In ActiveRecord 7, the
initialize_type_mapmethod has been moved to class method by rails/rails#42773 and armg no longer works correctly.Therefore, I have added a monkey patch to the
type_mapmethod. Because ActiveRecord 7 defines the type map statically when loaded, monkey patching class method does not make sense.https://github.com/rails/rails/blob/v7.0.3/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb#L616-L619
The
type_mapmethod is defined in the base classActiveRecord::ConnectionAdapters::AbstractAdapter. It is the same for all versions of ActiveRecord supported by armg. And this fix works in any version.