search

rsim / oracle-enhanced

Oracle enhaced adapter for ActiveRecord
MIT License
545 stars 308 forks source link

Support https://github.com/rails/rails/pull/44591 #2329

Open yahonda opened 1 year ago

yahonda commented 1 year ago

Oracle enhanced adapter needs to support https://github.com/rails/rails/pull/44591 which has eight commits. https://github.com/rails/rails/commit/deec3004d8d85443dc4f3f5fd22ab86b10adb58b triggers unit test failures.

Steps to reproduce

bundle exec rspec ./spec/active_record/connection_adapters/emulation/oracle_adapter_spec.rb:12

Expected behavior

It should pass.

Actual behavior

$ bundle exec rspec ./spec/active_record/connection_adapters/emulation/oracle_adapter_spec.rb:12
Fetching https://github.com/kubo/ruby-oci8.git
Fetching https://github.com/rsim/ruby-plsql.git
Fetching gem metadata from https://rubygems.org/.........
Resolving dependencies...
Using rake 13.0.6
Using concurrent-ruby 1.2.2
Using connection_pool 2.4.0
Using minitest 5.18.0
Using ast 2.4.2
Using bundler 2.4.10
Using io-console 0.6.0
Using diff-lcs 1.5.0
Using json 2.6.3
Using parallel 1.23.0
Using stringio 3.0.6
Using rack 3.0.7
Using rainbow 3.1.1
Using regexp_parser 2.8.0
Using rexml 3.2.5
Using rspec-support 3.12.0
Using ruby-progressbar 1.13.0
Using i18n 1.13.0
Using tzinfo 2.0.6
Using reline 0.3.3
Using parser 3.2.2.1
Using psych 5.1.0
Using rspec-core 3.12.2
Using rspec-expectations 3.12.3
Using rspec-mocks 3.12.5
Using ruby-plsql 0.8.0 from https://github.com/rsim/ruby-plsql.git (at master@89a68f9)
Using activesupport 7.1.0.alpha from https://github.com/rails/rails.git (at /home/yahonda/src/github.com/rails/rails@50f8b85)
Using rspec 3.12.0
Using ruby-oci8 2.2.12 from https://github.com/kubo/ruby-oci8.git (at master@29290ee)
Using irb 1.6.4
Using rdoc 6.5.0
Using rubocop-ast 1.28.1
Using debug 1.7.2
Using unicode-display_width 2.4.2
Using rubocop 1.50.2
Using rubocop-capybara 2.18.0
Using rubocop-factory_bot 2.22.0
Using rubocop-performance 1.17.1
Using rubocop-rspec 2.22.0
Using activemodel 7.1.0.alpha from https://github.com/rails/rails.git (at /home/yahonda/src/github.com/rails/rails@50f8b85)
Using rubocop-rails 2.19.1
Using activerecord 7.1.0.alpha from https://github.com/rails/rails.git (at /home/yahonda/src/github.com/rails/rails@50f8b85)
Bundle complete! 13 Gemfile dependencies, 42 gems now installed.
Use `bundle info [gemname]` to see where a bundled gem is installed.
==> Loading config from ENV or use default
==> Running specs with ruby version 3.2.2
==> Effective ActiveRecord version 7.1.0.alpha
Run options: include {:locations=>{"./spec/active_record/connection_adapters/emulation/oracle_adapter_spec.rb"=>[12]}}
F

Failures:

  1) OracleEnhancedAdapter emulate OracleAdapter should be an OracleAdapter
     Failure/Error: @raw_connection.database_version

     NoMethodError:
       undefined method `database_version' for nil:NilClass
     # ./lib/active_record/connection_adapters/oracle_enhanced_adapter.rb:699:in `get_database_version'
     # /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/schema_cache.rb:145:in `database_version'
     # /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:780:in `database_version'
     # ./lib/active_record/connection_adapters/oracle_enhanced_adapter.rb:307:in `supports_fetch_first_n_rows_and_offset?'
     # ./lib/active_record/connection_adapters/oracle_enhanced_adapter.rb:271:in `arel_visitor'
     # /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:123:in `initialize'
     # ./lib/active_record/connection_adapters/oracle_enhanced_adapter.rb:250:in `initialize'
     # ./lib/active_record/connection_adapters/oracle_enhanced_adapter.rb:73:in `new'
     # ./lib/active_record/connection_adapters/oracle_enhanced_adapter.rb:73:in `oracle_enhanced_connection'
     # /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:659:in `public_send'
     # /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:659:in `new_connection'
     # /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:703:in `checkout_new_connection'
     # /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:682:in `try_to_checkout_new_connection'
     # /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:643:in `acquire_connection'
     # /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:344:in `checkout'
     # /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:178:in `connection'
     # /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/abstract/connection_handler.rb:193:in `retrieve_connection'
     # /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_handling.rb:287:in `retrieve_connection'
     # /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_handling.rb:254:in `connection'
     # ./spec/active_record/connection_adapters/emulation/oracle_adapter_spec.rb:14:in `block (2 levels) in <top (required)>'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/example.rb:263:in `instance_exec'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/example.rb:263:in `block in run'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/example.rb:511:in `block in with_around_and_singleton_context_hooks'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/example.rb:468:in `block in with_around_example_hooks'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/hooks.rb:486:in `block in run'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/hooks.rb:624:in `run_around_example_hooks_for'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/hooks.rb:486:in `run'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/example.rb:468:in `with_around_example_hooks'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/example.rb:511:in `with_around_and_singleton_context_hooks'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/example.rb:259:in `run'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/example_group.rb:646:in `block in run_examples'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/example_group.rb:642:in `map'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/example_group.rb:642:in `run_examples'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/example_group.rb:607:in `run'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/runner.rb:121:in `block (3 levels) in run_specs'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/runner.rb:121:in `map'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/runner.rb:121:in `block (2 levels) in run_specs'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/configuration.rb:2070:in `with_suite_hooks'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/runner.rb:116:in `block in run_specs'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/reporter.rb:74:in `report'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/runner.rb:115:in `run_specs'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/runner.rb:89:in `run'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/runner.rb:71:in `run'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/lib/rspec/core/runner.rb:45:in `invoke'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.2/exe/rspec:4:in `<top (required)>'
     # /home/yahonda/.rbenv/versions/3.2.2/bin/rspec:25:in `load'
     # /home/yahonda/.rbenv/versions/3.2.2/bin/rspec:25:in `<top (required)>'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/3.2.0/bundler/cli/exec.rb:58:in `load'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/3.2.0/bundler/cli/exec.rb:58:in `kernel_load'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/3.2.0/bundler/cli/exec.rb:23:in `run'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/3.2.0/bundler/cli.rb:492:in `exec'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/3.2.0/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/3.2.0/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/3.2.0/bundler/vendor/thor/lib/thor.rb:392:in `dispatch'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/3.2.0/bundler/cli.rb:34:in `dispatch'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/3.2.0/bundler/vendor/thor/lib/thor/base.rb:485:in `start'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/3.2.0/bundler/cli.rb:28:in `start'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/bundler-2.4.10/libexec/bundle:45:in `block in <top (required)>'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/3.2.0/bundler/friendly_errors.rb:117:in `with_friendly_errors'
     # /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/bundler-2.4.10/libexec/bundle:33:in `<top (required)>'
     # /home/yahonda/.rbenv/versions/3.2.2/bin/bundle:25:in `load'
     # /home/yahonda/.rbenv/versions/3.2.2/bin/bundle:25:in `<main>'

Finished in 0.04103 seconds (files took 0.43629 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/active_record/connection_adapters/emulation/oracle_adapter_spec.rb:12 # OracleEnhancedAdapter emulate OracleAdapter should be an OracleAdapter

$

System configuration

Rails version: main branch after https://github.com/rails/rails/commit/deec3004d8d85443dc4f3f5fd22ab86b10adb58b

Oracle enhanced adapter version: master

Ruby version: Ruby 2.7 or higher

Oracle Database version: Oracle Database 21c Express Edition

andynu commented 4 months ago

So the introduction of @unconfigured_connection breaks all the references to @raw_connection prior to connecting. After the connection, @raw_connection gets set. The timing of all the references to @raw_connection in the oracle_enhanced_adapter.rb are unclear to me. It seems like a connection lookup along the lines of

def _connection
  @unconfigured_connection || @raw_connection
end

Might work. It would certainly fix the specific database_version call in this immediate test failure. But this rails change cascades down through all the other references to @raw_connection, and I'm unclear if casually grabbing whichever ivar is set is appropriate.

Or if, in some number of cases, the library needs to be forcing the verify! call to ensure @raw_connection is defined and active.

I've added a PR with this dual lookup approach: "Add _connection method" #2367 mostly as a basis for making other progress on rails 7.1 differences.

I'm still orienting here and would humbly welcome any guidance or thoughts you might have on tackling this particular upstream change @yahonda . Thanks!