Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rails 7.x Compatibility #2384

Merged
merged 33 commits into from
Aug 13, 2024
Merged

Rails 7.x Compatibility #2384

merged 33 commits into from
Aug 13, 2024

Conversation

hss-mateus
Copy link
Contributor

Related to #2357

This PR just applies all the patches provided by @andynu since now, plus some temporary workarounds to get it working with a simple model.

I have tested this branch on a Rails 7.2.0.beta2 application, and it seems to be working for simple queries/models.

andynu and others added 30 commits March 25, 2024 09:29
Introduce the _connection method which returns whichever is defined
between @unconfigured_connection or @raw_connection.
verify! ensures that @raw_connection is defined.
Rails commit bd19d1baf1 [1] added a check of logger.level which was not
supported by the MockLogger.

This commit adds a mock level method. And limits the missing_method to
respond only to logging calls, so any other calls will raise a
NoMethodError to make future debugging easier.

[1]
rails/rails@bd19d1b
The SchemaMigration object no longer inherits from ActiveRecord::Base.

See "Move SchemaMigration to an independent object"
rails/rails@436277d

Several methods were renamed, and are no longer available as class
methods on SchemaMigration, but as an instance of SchemaMigration
per connection.

The object per connection seems to remove the need to `reset_table_name`
in the tests. That method is not available on the new object.
Rails commit e6da3ebd6c6 [1] introduces ColumnDefinition validations for
the keywords used in database commands. This commit adds the additional
keywords supported by this gem.

[1] rails/rails@e6da3eb
See rails/rails@07d2407

Both `table_name` and `column_name` were removed in favor of doing
o.left in the visitor pattern. This is just mimicing the adjustments
from upstream.
This extends the existing class level auto_retry functionality to
optionally allow an allow_retry boolean kwarg to allow retry of specific
queries.

This PR preserves the requirement that `autocommit?` be enabled which is the
safer choice.

```
  def with_retry(allow_retry: false) # :nodoc:
    should_retry = (allow_retry || self.class.auto_retry?) && autocommit?
```

See issue #2310
See rails/rails@3682aa1

So the shortening of index names that was added to rails itself
was almost immediately changed to use an 'idx_' prefix instead of an 'ix_'
prefix.

See rails/rails@59f2b06

However, now that rails does shortening. It is an opportunity to
simplify this library and just rely on the rails index name shortening.
    DEPRECATION WARNING: Passing the class as positional argument is deprecated and will be removed in Rails 7.2.

    Please pass the class as a keyword argument:

      serialize :serialized, type: Array
     (called from <class:TestSerializedColumn> at /home/andy/src/oracle-enhanced/spec/active_record/connection_adapters/oracle_enhanced_adapter_spec.rb:446)
Rails removed the table_name alias to Arel::Table.name
See rails/rails@1d98bc5
In rails commit rails/rails@942785f

A method `internal_exec_query` is introduced as the implementation for
exec_query. For now this just aliases one to the other.
This is to accomodate rails commit rails/rails@3421e89

The trouble is primary keys are not returned with this code. The
Column#auto_incremented_by_db? method would have to return true for any
column with an associated sequence, or perhaps marked as the primary
key.
…s nil

The super is not returning the id_value if it is available (as it used
to). Furthermore ActiveRecord::Persistence#_create_record expects an
array of values.
Rails now defaults to its own index name shortening (limited to 62 bytes).
This change overrides the rails max to exceed this gems shortening
method in order to preserve the historical behavior.
names

See rails/rails@3682aa1

So the shortening of index names that was added to rails itself
was almost immediately changed to use an 'idx_' prefix instead of an 'ix_'
prefix.

See rails/rails@59f2b06

However, now that rails does shortening. It is an opportunity to
consider simplifying this library and just using that.
Copy link
Collaborator

@yahonda yahonda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please ignore my previous update.

@yahonda yahonda marked this pull request as ready for review August 13, 2024 11:55
@yahonda yahonda merged commit f86f704 into rsim:master Aug 13, 2024
1 check failed
@yahonda
Copy link
Collaborator

yahonda commented Aug 13, 2024

Thanks for your contribution. Let me merge this pull request and address some TODO/known issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants