THREESCALE-10880: Bump rails from 6.0.6.1 to 6.1.7.7 #3705

dependabot · 2024-02-28T00:35:31Z

Completed tasks:

upgrade Rails and dependent gems
Follow the steps listed in:
- FastRuby.io Upgrade Rails from 6.0 to 6.1
- Ruby on Rails Guides - Upgrading from Rails 6.0 to Rails 6.1
go through existing code comments related to "rails \d" and remove workarounds and monkey patches, where possible
regenerate database schema as Rails 6.1 updated format (only affected MySQL)
make sure oracle-enhanced works *
Set load_defaults to 6.1 at application.rb

The JIRA for this Rails 6.1 upgrade: https://issues.redhat.com/browse/THREESCALE-10880

*To fix in Oracle:

rspec './spec/acceptance/api/application_spec.rb[1:1:1:1:1]' # Cinstance application json format GET /admin/api/applications.:format List Application
rspec './spec/acceptance/api/application_spec.rb[1:1:2:1:1]' # Cinstance application xml format GET /admin/api/applications.:format List Application

Result of the paginated query returns an empty array, when before pagination there were 2 items.

The reason of this seems to be caused by the following change:
rails/rails@2ad2425#diff-f35ba011d4738058fa067289f7585a0224ef8ab22efe7a7ecdb07c2fed6940eaR419-R420

The result of the previous query (which is supposed to be calculating the list of IDs) used to return ActiveRecord::Result, from which the primary key column was plucked. Now it returns an array of pairs, in which the first value is the ID, and the second (aka last) value is the rownum (aliased RAW_RNUM_). So, the query actually tries to find something like AND "CINSTANCES"."ID" IN (1,2,3,4,5,6,7,8,9,10), which is WRONG (because it compares IDs with rownums).

Bumps rails from 6.0.6.1 to 6.1.7.7.

Release notes

Sourced from rails's releases.

6.1.7.7

Active Support

No changes.

Active Model

No changes.

Active Record

No changes.

Action View

No changes.

Action Pack

No changes.

Active Job

No changes.

Action Mailer

No changes.

Action Cable

No changes.

... (truncated)

Commits

ac87f58 Preparing for 6.1.7.7 release
fc2f1b8 update changelog
78fe149 Merge pull request #48869 from brunoprietog/disable-session-active-storage-pr...
56bcc0a Preparing for 6.1.7.6 release
1f03e9d Bumping version for new release
3a1b615 Preparing for 6.1.7.5 release
c2af578 bumping version / changelog
c85cc66 Use a temporary file for storing unencrypted files while editing
7d949d7 Preparing for 6.1.7.4 release
11d5de1 update changelog
Additional commits viewable in compare view

You can trigger a rebase of this PR by commenting @dependabot rebase.

Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

@dependabot rebase will rebase this PR
@dependabot recreate will recreate this PR, overwriting any edits that have been made to it
@dependabot merge will merge this PR after your CI passes on it
@dependabot squash and merge will squash and merge this PR after your CI passes on it
@dependabot cancel merge will cancel a previously requested merge and block automerging
@dependabot reopen will reopen this PR if it is closed
@dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
@dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency
@dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
@dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
@dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
You can disable automated security fix PRs for this repo from the Security Alerts page.

Note
Automatic rebases have been disabled on this pull request as it has been open for over 30 days.

mayorova · 2024-03-12T12:06:35Z

app/models/proxy_config.rb

@@ -104,7 +104,7 @@ def update_version

    # Read the value
    version = config.connection.select_value(config.select(:version)).to_i
-    write_attribute_without_type_cast :version, version
+    write_attribute_without_type_cast 'version', version


This behavior seems to have been changed here: rails/rails@7834363#diff-f029f28568c577e87cb998285cfdc00c05df84e95e9c28eb115c148e528c2b44L46

mayorova · 2024-03-12T12:10:07Z

app/validators/nested_association_validator.rb

@@ -9,8 +9,11 @@ def initialize(options)
  def validate_each(record, _attribute, value)
    return if !value || value.valid? || value.marked_for_destruction?
    options[:report].each do |alternate, attr|
-      errors = value.errors.messages[alternate]
-      record.errors[attr || alternate].concat(errors) if errors.present?
+      errors = value.errors.where(alternate)


This block fixes a FrozenError (described here, and uses a new Error interface, see rails/rails#32313

mayorova · 2024-03-12T12:32:30Z

config/application.rb

@@ -240,7 +240,7 @@ def cache_store_config
    config.three_scale.cors.enabled = false
    config.three_scale.cors.merge!(try_config_for(:cors) || {})

-    three_scale = config_for(:settings).symbolize_keys
+    three_scale = config_for(:settings).deep_symbolize_keys


I don't quite get this one to be honest 🙃

I think some things broke because of this change: https://guides.rubyonrails.org/upgrading_ruby_on_rails.html#rails-application-config-for-return-value-no-longer-supports-access-with-string-keys

However, it says that what stopped being supported is access with string keys, but what I observe is that the difference is that now the hash that is produced by config_for on the contrary has string keys, and not symbols like it used to be. So, maybe I'm missing something...

But this specific change fixes several issues with the impersonation_admin config.

Is maybe related to this?

porta/config/application.rb

Lines 88 to 93 in d2fc12b

# The old config_for gem returns HashWithIndifferentAccess

# https://github.com/3scale/config_for/blob/master/lib/config_for/config.rb#L16

def config_for(*args)

config = super

config.is_a?(Hash) ? config.with_indifferent_access : config

end

Yeah, but we are using the Rails config_for, it's just the description of the change is not 100% aligned with the actual behavior. But to be honest, I already don't remember how it used to behave, and how it behaves now. If needed, I can recheck :)

But this works.

Yeah, but we are using the Rails config_for

If I place a breakpoint on our config_for it stops many times, so we use it. Anyway, if it works, fine for me.

@jlledom Yes yes, we DO use this method, what I meant is we then use Rails' (via super) config_for.

I've been looking into it, and indeed, the Rails' description is correct. Rails' config_for now returns a config that is deep-symbolized already. But when we apply .with_indifferent_access, and then .symbolize_keys that was lost, and accessing the nested values by symbols didn't work: Rails.application.config.three_scale.impersonation_admin[:domain] was nil.

Applying .deep_symbolize_keys fixed this. But also, I think we transform the configs too much, and they are not consistent (deep_symbolize_keys is applied for some, symbolize_keys applied for others etc.).

So, I thought we could take this opportunity to align all, and to use the default format that Rails provides - ActiveSupport::OrderedOptions with deep-symbolized keys.
We just need to make sure we use symbols throughout the app.

This is applied here: dc7bf1a

mayorova · 2024-03-12T12:46:22Z

lib/system/database.rb

    end

    def adapter
      adapter_method.match(/^(oracle|postgres|mysql).*/).to_a.last
    end

    def adapter_method
-      ActiveSupport::StringInquirer.new(configuration_specification.adapter_method)
+      @adapter_method ||= ActiveSupport::StringInquirer.new(database_config.adapter)


database_config.adapter_method also exists, but it basically is a contatenation of _connection to the adapter value:
https://github.com/rails/rails/blob/v6.1.7.7/activerecord/lib/active_record/database_configurations/database_config.rb#L27-L29

mayorova · 2024-03-12T12:50:18Z

spec/acceptance/api/cms_file_spec.rb

@@ -21,7 +21,7 @@
      parameter :attachment, 'The Attachment'

      let(:section_id) { resource.section_id }
-      let(:attachment) { fixture_file_upload('/wide.jpg',' image/jpeg') }
+      let(:attachment) { fixture_file_upload('wide.jpg',' image/jpeg') }


This changed from File.join(self.class.fixture_path, path) to Pathname.new(self.class.fixture_path).join(path) and the latter treats /wide.jpg as an absolute path (rightfully, IMO).

test/unit/cache_store_test.rb

test/unit/event_store/event_test.rb

test/unit/authentication_provider_test.rb

app/lib/cms/database_file_system.rb

test/integration/application_controller_test.rb

app/controllers/master/providers/switches_controller.rb

mayorova · 2024-03-13T14:40:37Z

Gemfile

@@ -265,4 +265,4 @@ end

 gem 'kubeclient'

-gem 'pg', '~> 0.21.0'
+gem 'pg', '~> 1.3'


It seems that in the latest Rails 6.1 the required version is ">= 1.3.0.rc1", but the error I was getting:

Gem::LoadError: can't activate pg (~> 1.1), already activated pg-0.21.0. Make sure all dependencies are added to Gemfile.

such error I usually see when I forget to call bundle exec ... but it's fine to get it updated either way

Forgetting bundle exec was not the case, but indeed there was something weird 😬

I sometimes get this error for the default gems (like json).

mayorova · 2024-03-14T13:27:06Z

test/unit/three_scale/api/responder_test.rb

@@ -18,7 +18,7 @@ module Representer

  test '#to_format PATCH' do
    @controller.formats = [:json]
-    @controller.request = ActionDispatch::Request.new('REQUEST_METHOD' => 'PATCH')
+    @controller.request = ActionDispatch::Request.new('REQUEST_METHOD' => 'PATCH', 'rack.input' => StringIO.new)


This one made me scratch my head quite a bit...

The error was:

Error: ThreeScale::Api::ResponderTest#test_#to_format_PATCH: RuntimeError: Missing rack.input test/unit/three_scale/api/responder_test.rb:25:in `block in <class:ResponderTest>'

Debugging showed that it was caused by https://github.com/rack/rack/blob/v2.2.8.1/lib/rack/request.rb#L441-L442

And the reason why this called in the first place was this addition:

added here: rails/rails@5745a3c

By the way, rack.input was later made optional in rack: rack/rack#2018

But this version is not released yet.

I think this really happens just because we are creating this fake request, and in real-life scenario this would not cause issues, so I think adding rack.input to the env in Request initializer should do the trick. But I don't understand this test enough to be sure 😬

maybe put a comment that this can be removed after we have a rack version with rack/rack#2018

mayorova · 2024-03-15T13:19:40Z

Gemfile

@@ -124,7 +124,8 @@ gem 'rack-x_served_by', '~> 0.1.1'
 gem 'rack-cors'
 gem 'roar-rails'

-gem 'reform', '~> 2.0.3', require: false
+gem 'reform', '~> 2.3.0', require: false
+gem 'reform-rails', '~> 0.2.2', require: false


This upgrade is extracted to #3725 and this PR will be rebased once it is merged.

mayorova · 2024-03-15T13:24:04Z

app/lib/three_scale/reform.rb

@@ -24,17 +24,14 @@ def save!(*)
    protected

    def merge_errors
-      definitions = property_definitions.values
-      definitions.each do |definition|
+      schema.each do |value|


I found this change pretty tricky, because I don't really understand the documentation of the reform gem very well (not sure if it's mine, or the documentation's fault).

This seems to do the trick, although:

I am not sure if the behavior is 100% equivalent

The code says The #errors method will be removed in Reform 3.0 core. def errors(*args) https://github.com/trailblazer/reform/blob/v2.6.2/lib/reform/contract/validate.rb#L14

Also, there are just three forms, it seems, that use reform,and honestly, I think it might be possible to get rid of it at some point and simplify the implementation.

I think for the current purposes this rework should be enough, but probably we can refactor/remove this later.

mayorova · 2024-03-15T13:51:10Z

config.ru

 run Rails.application
+Rails.application.load_server


Not sure it's a good idea to add this... but this is the new default content of config.ru:
rails/rails@868866c

I think it's fine to have it. It won't make any difference unless we manually add servers anyway.

Not sure what you mean by manually adding servers.

@jlledom ☝️ ?

I don't know what this line does. It's just coming in the default templates now.

@mayorova @akostadinov this line runs whatever blocks you added through Rails::Railtie.server. Check https://api.rubyonrails.org/v6.1.7.7/classes/Rails/Railtie.html, at the section Run another program when the Rails server starts.

AFAIK we don't use this feature so there's no difference having this line or not.

There might be gems that use, now or in the future?

No idea, but it seems to me the best decision is to leave this line. If servers are added, this line is required; if no servers are added, this line does nothing. So it will work in all scenarios, and it's only executed once during startup, so it has no impact on performance.

config/environments/development.rb

app/lib/event_store/event.rb

akostadinov

One comment suggestion, other than that ready to go IMO!

akostadinov · 2024-05-13T13:02:33Z

config/application.rb

+
+    # Support for inversing belongs_to -> has_many Active Record associations.
+    # Overriding Rails 6.1 default, because it causes various issues
+    config.active_record.has_many_inversing = false


Suggested change

config.active_record.has_many_inversing = false

# likely need to keep it forever as we can't override it for individual use cases

# also the feature has outstanding bugs: rails/rails#47559 rails/rails#50258

config.active_record.has_many_inversing = false

app/lib/event_store/event.rb

jlledom · 2024-05-14T07:27:26Z

app/models/cinstance.rb

-      where{ plan_id.in( my{Plan.issued_by(service).select(:id)} ) }
+      # NOTE: `reorder(nil)` resets the order set in the Plan's default scope,
+      # because ORDER BY in subqueries fails in Oracle with 'ORA-00907: missing right parenthesis'.
+      where { plan_id.in( my { Plan.issued_by(service).select(:id).reorder(nil)} ) }


If I understood this correctly, you want to remove the ordered default scope because that's not compatible with Oracle, is that right? in that case I think its a good idea to remove it.

app/models/contract.rb

test/unit/event_store/event_test.rb

mayorova · 2024-05-14T13:30:24Z

One comment suggestion, other than that ready to go IMO!

f1e2ad9 @akostadinov

akostadinov · 2024-05-14T15:18:22Z

app/models/cinstance.rb

-      # NOTE: `reorder(nil)` resets the order set in the Plan's default scope,
-      # because ORDER BY in subqueries fails in Oracle with 'ORA-00907: missing right parenthesis'.
-      where { plan_id.in( my { Plan.issued_by(service).select(:id).reorder(nil)} ) }
+      where { plan_id.in( my { Plan.issued_by(service).select(:id)} ) }


I wouldn't remove the reorder(nil) from these queries. IMO keep it. It is solid to stay. But also fine to remove if you really want.

After removing the default scope I don't think it's needed. And if someone ever wants to add the default scope, or add some order in subqueries, the test should fail.

I reverted the reorder(nil), but the spaces changes have left 😬

This is caused by the following change: https://guides.rubyonrails.org/upgrading_ruby_on_rails.html#rails-application-config-for-return-value-no-longer-supports-access-with-string-keys Now all configs coming from YAML files are deep-symbolized by default

See https://guides.rubyonrails.org/upgrading_ruby_on_rails.html#new-activemodel-error-class and rails/rails#32313

Rails 6.1 cuts out the `no-cache` value from the header, due to this normalization [1], see commit [2]. According to the docs [3] `no-store` is "stronger" than `no-cache`, which indeed makes `no-cache` redundant in presence of `no-store`, so I changed the headers we set in `ApplicationController#disable_client_cache`. [1] https://github.com/rails/rails/blob/v6.1.7.7/actionpack/lib/action_dispatch/http/cache.rb#L185-L222 [2] rails/rails@e06f7b3#diff-d77084e8407ab72b3068ae0268553de59c351ba520adc9c248e24e887b3fd5bf [3] https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control

…n rollback

Old Arel visitor was causing issues with ROWNUM, so we removed it. The new Arel visitor forms some queries that incude subqueries with ORDER BY, which throws an error in Oracle: ORA-00907: missing right parenthesis So, order has been removed from some scopes to avoid this issue.

This got broken in Rails 6.1, specifically for PostgreSQL, reported in rails/rails#41651 It was fixed by rails/rails@587522e but I believe that the fix in this commit will work both with or without the Rails fix, so we don't need to worry about it when we upgrade again (it's just that the .cast method will be called more times after all column types are back again in the `column_types` hash in Rails 7, which was the behavior until Rails 6.1)

jlledom

Good job! 🥳

dependabot bot added dependencies Pull requests that update a dependency file ruby Pull requests that update Ruby code labels Feb 28, 2024

dependabot bot force-pushed the dependabot/bundler/rails-6.1.7.7 branch 3 times, most recently from e3e230c to 27d3cdd Compare March 6, 2024 13:34

mayorova force-pushed the dependabot/bundler/rails-6.1.7.7 branch from 6cb2c9c to 71f725c Compare March 8, 2024 23:09

mayorova changed the title ~~Bump rails from 6.0.6.1 to 6.1.7.7~~ THREESCALE-10880: Bump rails from 6.0.6.1 to 6.1.7.7 Mar 8, 2024

mayorova reviewed Mar 12, 2024

View reviewed changes

test/unit/cache_store_test.rb Show resolved Hide resolved

mayorova reviewed Mar 12, 2024

View reviewed changes

test/unit/event_store/event_test.rb Show resolved Hide resolved

mayorova reviewed Mar 12, 2024

View reviewed changes

test/unit/authentication_provider_test.rb Outdated Show resolved Hide resolved

mayorova reviewed Mar 12, 2024

View reviewed changes

app/lib/cms/database_file_system.rb Show resolved Hide resolved

mayorova reviewed Mar 13, 2024

View reviewed changes

test/integration/application_controller_test.rb Show resolved Hide resolved

mayorova reviewed Mar 13, 2024

View reviewed changes

app/controllers/master/providers/switches_controller.rb Show resolved Hide resolved

mayorova reviewed Mar 13, 2024

View reviewed changes

mayorova force-pushed the dependabot/bundler/rails-6.1.7.7 branch from d1111f6 to 6f16f2a Compare March 13, 2024 15:20

mayorova reviewed Mar 14, 2024

View reviewed changes

mayorova force-pushed the dependabot/bundler/rails-6.1.7.7 branch 2 times, most recently from c80f107 to 55320cd Compare March 14, 2024 14:35

mayorova reviewed Mar 15, 2024

View reviewed changes

mayorova force-pushed the dependabot/bundler/rails-6.1.7.7 branch from 1410363 to 96bab09 Compare March 15, 2024 14:21

mayorova reviewed Mar 15, 2024

View reviewed changes

config/environments/development.rb Show resolved Hide resolved

jlledom mentioned this pull request Mar 20, 2024

Upgrade dependencies (reform, roar, representable and others) #3725

Merged

mayorova force-pushed the dependabot/bundler/rails-6.1.7.7 branch from 2aa1981 to 1ea06c6 Compare April 5, 2024 09:02

mayorova reviewed May 13, 2024

View reviewed changes

app/lib/event_store/event.rb Show resolved Hide resolved

akostadinov previously approved these changes May 13, 2024

View reviewed changes

jlledom reviewed May 14, 2024

View reviewed changes

mayorova dismissed akostadinov’s stale review via f1e2ad9 May 14, 2024 13:29

akostadinov previously approved these changes May 14, 2024

View reviewed changes

akostadinov reviewed May 14, 2024

View reviewed changes

mayorova force-pushed the dependabot/bundler/rails-6.1.7.7 branch 2 times, most recently from 1425637 to e51c72d Compare May 15, 2024 08:51

mayorova added 4 commits May 15, 2024 12:04

Upgrade rails to 6.1 and other dependencies

2c09360

Adapt database configuration to ActiveRecord 6.1

20c9b8a

Update Oracle permissions using the new interface

eac75ab

mayorova dismissed akostadinov’s stale review via 3f442b5 May 15, 2024 10:04

mayorova force-pushed the dependabot/bundler/rails-6.1.7.7 branch from e51c72d to 3f442b5 Compare May 15, 2024 10:04

mayorova added 10 commits May 15, 2024 12:09

Update Rails default files and configs with 'rails app:update' task

628f1e4

Remove monkey patches that are not needed after Rails 6.1 upgrade

67bcad0

Fix issues related to errors API deprecation

cb70e6a

See https://guides.rubyonrails.org/upgrading_ruby_on_rails.html#new-activemodel-error-class and rails/rails#32313

Update DB schemas after Rails 6.1. upgrade

07878b7

Fix different issues after Rails 6.1 upgrade

aada331

Prevent exceptions when deserializing EventStore::Event on transactio…

9959364

…n rollback

Avoid return within a transaction

e62185e

mayorova force-pushed the dependabot/bundler/rails-6.1.7.7 branch from 3f442b5 to e62185e Compare May 15, 2024 10:10

Fix small issue in cucumber steps

6485c37

mayorova force-pushed the dependabot/bundler/rails-6.1.7.7 branch from 568945c to 6485c37 Compare May 15, 2024 12:10

akostadinov approved these changes May 15, 2024

View reviewed changes

jlledom approved these changes May 16, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

THREESCALE-10880: Bump rails from 6.0.6.1 to 6.1.7.7 #3705

THREESCALE-10880: Bump rails from 6.0.6.1 to 6.1.7.7 #3705

dependabot bot commented on behalf of github Feb 28, 2024 •

edited by mayorova

mayorova Mar 12, 2024

mayorova Mar 12, 2024

mayorova Mar 12, 2024

jlledom Apr 18, 2024

mayorova Apr 18, 2024

jlledom Apr 19, 2024

mayorova Apr 19, 2024 •

edited

mayorova Mar 12, 2024

mayorova Mar 12, 2024

mayorova Mar 13, 2024 •

edited

akostadinov May 10, 2024

mayorova May 12, 2024

mayorova Mar 14, 2024

akostadinov May 9, 2024

mayorova Mar 15, 2024 •

edited

mayorova Mar 15, 2024

mayorova Mar 15, 2024

jlledom Apr 18, 2024

akostadinov May 11, 2024

mayorova May 13, 2024

jlledom May 14, 2024

akostadinov May 14, 2024

jlledom May 14, 2024 •

edited

akostadinov left a comment

akostadinov May 13, 2024

jlledom May 14, 2024

mayorova commented May 14, 2024

akostadinov May 14, 2024

mayorova May 14, 2024

mayorova May 15, 2024

jlledom left a comment

	# The old config_for gem returns HashWithIndifferentAccess
	# https://github.com/3scale/config_for/blob/master/lib/config_for/config.rb#L16
	def config_for(*args)
	config = super
	config.is_a?(Hash) ? config.with_indifferent_access : config
	end

THREESCALE-10880: Bump rails from 6.0.6.1 to 6.1.7.7 #3705

Are you sure you want to change the base?

THREESCALE-10880: Bump rails from 6.0.6.1 to 6.1.7.7 #3705

Conversation

dependabot bot commented on behalf of github Feb 28, 2024 • edited by mayorova

6.1.7.7

Active Support

Active Model

Active Record

Action View

Action Pack

Active Job

Action Mailer

Action Cable

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mayorova Apr 19, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mayorova Mar 13, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mayorova Mar 15, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlledom May 14, 2024 • edited

Choose a reason for hiding this comment

akostadinov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mayorova commented May 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlledom left a comment

Choose a reason for hiding this comment

dependabot bot commented on behalf of github Feb 28, 2024 •

edited by mayorova

mayorova Apr 19, 2024 •

edited

mayorova Mar 13, 2024 •

edited

mayorova Mar 15, 2024 •

edited

jlledom May 14, 2024 •

edited