Skip to content

Commit

Permalink
Follow up - ca56de3 - Maintenance: Updated to Rails 6.0.4 and the new…
Browse files Browse the repository at this point in the history
… Zeitwerk autoloader: Fixed deprecation of autoloading in initializers causing namespace lookup errors.
  • Loading branch information
thorsteneckel committed Jul 21, 2021
1 parent 3d946cf commit 887b779
Show file tree
Hide file tree
Showing 17 changed files with 194 additions and 68 deletions.
13 changes: 13 additions & 0 deletions app/models/session.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,17 @@

class Session < ActiveRecord::SessionStore::Session
include Session::SetsPersistentFlag

def self.secure_flag?
# enable runtime change support in test/develop environments
return https? if !Rails.env.production?

@secure_flag ||= https?
rescue ActiveRecord::NoDatabaseError, ActiveRecord::StatementInvalid
false
end

def self.https?
Setting.get('http_type') == 'https'
end
end
2 changes: 1 addition & 1 deletion config/initializers/db_preflight_check.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/

# Rails' constant auto-loading resolves to 'rails/initializable' instead
require_dependency 'zammad/application/initializer/db_preflight_check'
require 'zammad/application/initializer/db_preflight_check'

Zammad::Application::Initializer::DbPreflightCheck.perform
13 changes: 8 additions & 5 deletions config/initializers/logo.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/

return if !ActiveRecord::Base.connected?
Rails.application.reloader.to_prepare do

# sync logo to fs / only if settings already exists
return if ActiveRecord::Base.connection.tables.exclude?('settings')
return if Setting.column_names.exclude?('state_current')
next if !ActiveRecord::Base.connected?

StaticAssets.sync
# sync logo to fs / only if settings already exists
next if ActiveRecord::Base.connection.tables.exclude?('settings')
next if Setting.column_names.exclude?('state_current')

StaticAssets.sync
end
12 changes: 7 additions & 5 deletions config/initializers/models_searchable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@

# update settings for searchable models

begin
return if !Setting.exists?(name: 'models_searchable')
Rails.application.reloader.to_prepare do
begin
next if !Setting.exists?(name: 'models_searchable')

Setting.set('models_searchable', Models.searchable.map(&:to_s))
rescue ActiveRecord::StatementInvalid
nil
Setting.set('models_searchable', Models.searchable.map(&:to_s))
rescue ActiveRecord::StatementInvalid
nil
end
end
2 changes: 1 addition & 1 deletion config/initializers/session_store.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/

# Rails' constant auto-loading resolves to 'rails/initializable' instead
require_dependency 'zammad/application/initializer/session_store'
require 'zammad/application/initializer/session_store'

Zammad::Application::Initializer::SessionStore.perform
17 changes: 17 additions & 0 deletions lib/core_ext/action_dispatch/middleware/cookies.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/

require 'action_dispatch/middleware/cookies'

module ActionDispatch
class Cookies
class CookieJar

alias original_write_cookie? write_cookie?

# https://github.com/rails/rails/blob/v6.0.4/actionpack/lib/action_dispatch/middleware/cookies.rb#L447-L449
def write_cookie?(cookie)
original_write_cookie?(cookie.merge(secure: ::Session.secure_flag?))
end
end
end
end
20 changes: 20 additions & 0 deletions lib/core_ext/rack/session/abstract/id.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/

require 'rack/session/abstract/id'

module Rack
module Session
module Abstract
class Persisted

alias original_security_matches? security_matches?

# https://github.com/rack/rack/blob/2.2.3/lib/rack/session/abstract/id.rb#L363-L366
def security_matches?(request, options)
options[:secure] = ::Session.secure_flag?
original_security_matches?(request, options)
end
end
end
end
end
22 changes: 22 additions & 0 deletions lib/core_ext/rack/utils.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/

require 'rack/utils'

module Rack
module Utils

module_function

singleton_class.alias_method :original_add_cookie_to_header, :add_cookie_to_header

# https://github.com/rack/rack/blob/2.2.3/lib/rack/session/utils.rb#L223-L262
def add_cookie_to_header(header, key, value)

if value.is_a?(Hash)
value[:secure] = ::Session.secure_flag?
end

original_add_cookie_to_header(header, key, value)
end
end
end
2 changes: 1 addition & 1 deletion lib/zammad/application/initializer/db_preflight_check.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

module Zammad
class Application
class Initializer
module Initializer
module DbPreflightCheck
def self.perform
adapter.perform
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

module Zammad
class Application
class Initializer
module Initializer
module DbPreflightCheck
module Base
def check_version_compatibility
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

module Zammad
class Application
class Initializer
module Initializer
module DbPreflightCheck
module Mysql2
extend Base
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

module Zammad
class Application
class Initializer
module Initializer
module DbPreflightCheck
module Nulldb
# no-op
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

module Zammad
class Application
class Initializer
module Initializer
module DbPreflightCheck
module Postgresql
extend Base
Expand Down
24 changes: 14 additions & 10 deletions lib/zammad/application/initializer/session_store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,28 @@

module Zammad
class Application
class Initializer
module Initializer
module SessionStore
STORE_TYPE = :active_record_store # default: :cookie_store
SESSION_KEY = "_zammad_session_#{Digest::MD5.hexdigest(Rails.root.to_s)[5..15]}".freeze # default: '_zammad_session'

def self.perform
ActionDispatch::Session::ActiveRecordStore.session_class = Session
# it's important to register the session store at initialization time
# otherwise the store won't be used
# ATTENTION: Rails/Rack Cookie handling was customized to call `Session.secure_flag?`
# instead of accessing the `:secure` key (default Rack/Rails behavior).
# See: lib/core_ext/action_dispatch/middleware/cookies.rb
# See: lib/core_ext/rack/session/abstract/id.rb
# See: lib/core_ext/rack/session/utils.rb
Rails.application.config.session_store STORE_TYPE,
key: SESSION_KEY,
secure: secure?
end
key: SESSION_KEY

def self.secure?
Setting.get('http_type') == 'https'
rescue ActiveRecord::StatementInvalid
false
# once the application is initialized and we can access the models
# we need to update the session_class
Rails.application.reloader.to_prepare do
ActionDispatch::Session::ActiveRecordStore.session_class = Session
end
end
private_class_method :secure?
end
end
end
Expand Down
31 changes: 0 additions & 31 deletions spec/lib/zammad/application/initializer/session_store_spec.rb

This file was deleted.

34 changes: 24 additions & 10 deletions spec/requests/external_credentials_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@

shared_examples 'for failure cases' do
it 'responds with the appropriate status and error message' do
send(*endpoint, as: :json, params: try(:params) || {})
send(*endpoint, as: :json, params: try(:params) || {}, headers: headers)

expect(response).to have_http_status(status)
expect(json_response).to include('error' => error_message)
Expand Down Expand Up @@ -415,7 +415,14 @@

let!(:twitter_credential) { create(:twitter_credential) }

before { get '/api/v1/external_credentials/twitter/link_account', as: :json }
# Rails / Rack needs to detect that the request comes via HTTPS as well
let(:headers) do
{
'X-Forwarded-Proto' => 'https'
}
end

before { get '/api/v1/external_credentials/twitter/link_account', as: :json, headers: headers }

include_examples 'for failure cases' do
let(:status) { :unprocessable_entity }
Expand Down Expand Up @@ -458,7 +465,14 @@
let(:oauth_token) { 'DyhnyQAAAAAA9CNXAAABcSxAexs' }
let(:oauth_verifier) { '15DOeRkjP4JkOSVqULkTKA1SCuIPP105' }

before { get '/api/v1/external_credentials/twitter/link_account', as: :json }
# Rails / Rack needs to detect that the request comes via HTTPS as well
let(:headers) do
{
'X-Forwarded-Proto' => 'https'
}
end

before { get '/api/v1/external_credentials/twitter/link_account', as: :json, headers: headers }

context 'if Twitter account has already been added' do
let!(:channel) { create(:twitter_channel, custom_options: channel_options) }
Expand All @@ -472,20 +486,20 @@
end

it 'uses the existing channel' do
expect { send(*endpoint, as: :json, params: params) }
expect { send(*endpoint, as: :json, params: params, headers: headers) }
.not_to change(Channel, :count)
end

it 'updates channel properties' do
expect { send(*endpoint, as: :json, params: params) }
expect { send(*endpoint, as: :json, params: params, headers: headers) }
.to change { channel.reload.options[:user][:name] }
.and change { channel.reload.options[:auth][:external_credential_id] }
.and change { channel.reload.options[:auth][:oauth_token] }
.and change { channel.reload.options[:auth][:oauth_token_secret] }
end

it 'subscribes to webhooks' do
send(*endpoint, as: :json, params: params)
send(*endpoint, as: :json, params: params, headers: headers)

expect(WebMock)
.to have_requested(:post, "https://api.twitter.com/1.1/account_activity/all/#{twitter_credential.credentials[:env]}/subscriptions.json")
Expand All @@ -496,7 +510,7 @@
end

it 'creates a new channel' do
expect { send(*endpoint, as: :json, params: params) }
expect { send(*endpoint, as: :json, params: params, headers: headers) }
.to change(Channel, :count).by(1)

expect(Channel.last.options)
Expand All @@ -506,19 +520,19 @@
end

it 'redirects to the newly created channel' do
send(*endpoint, as: :json, params: params)
send(*endpoint, as: :json, params: params, headers: headers)

expect(response).to redirect_to(%r{/#channels/twitter/#{Channel.last.id}$})
end

it 'clears the :request_token session variable' do
send(*endpoint, as: :json, params: params)
send(*endpoint, as: :json, params: params, headers: headers)

expect(session[:request_token]).to be(nil)
end

it 'subscribes to webhooks' do
send(*endpoint, as: :json, params: params)
send(*endpoint, as: :json, params: params, headers: headers)

expect(WebMock)
.to have_requested(:post, "https://api.twitter.com/1.1/account_activity/all/#{twitter_credential.credentials[:env]}/subscriptions.json")
Expand Down
Loading

0 comments on commit 887b779

Please sign in to comment.