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.

Sign up for GitHub

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

THREESCALE-11139 simplify application queries #3845

Merged
merged 14 commits into from
Oct 11, 2024
Merged

THREESCALE-11139 simplify application queries #3845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
61e2c26
THREESCALE-11139 simplify applications query
akostadinov Jul 10, 2024
16f19f9
optimize application queries further
akostadinov Jul 11, 2024
cef5079
Bullet optimization of Admin::Api::ApplicationsController
akostadinov Jul 11, 2024
9eba9d6
UnoptimizedQuery ApplicationPlan => [:original]
akostadinov Aug 3, 2024
77ea9e7
debugging info on failure
akostadinov Aug 4, 2024
63d063c
eager loading :default_application_plan of Service
akostadinov Aug 4, 2024
0f93b94
fix user can access Cinstance
akostadinov Aug 6, 2024
dcdcd9a
access cinstance service directly not through plan
akostadinov Sep 11, 2024
8377ecf
remove some n+1 issues
akostadinov Sep 11, 2024
f368ddf
allow tracing SQL queries
akostadinov Sep 12, 2024
dc71b57
fix N+1s for NginxesController
akostadinov Sep 12, 2024
cd9acce
N+1 in signup controller
akostadinov Sep 12, 2024
4073aec
more meaningful method name
akostadinov Sep 18, 2024
c2ed9f1
finding single applications depends on service_id
akostadinov Oct 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,8 @@ group :test do
end

group :development, :test do
gem 'active_record_query_trace'

gem 'bootsnap', '~> 1.16'
gem 'bullet', '~> 6.1.5'
gem 'colorize'
Expand Down
3 changes: 3 additions & 0 deletions Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ GEM
erubi (~> 1.4)
rails-dom-testing (~> 2.0)
rails-html-sanitizer (~> 1.1, >= 1.2.0)
active_record_query_trace (1.8.2)
activerecord (>= 6.0.0)
activejob (6.1.7.8)
activesupport (= 6.1.7.8)
globalid (>= 0.3.6)
Expand Down Expand Up @@ -958,6 +960,7 @@ DEPENDENCIES
3scale_time_range (= 0.0.6)
RedCloth (~> 4.3)
active-docs!
active_record_query_trace
activejob-uniqueness
activemerchant (~> 1.107.4)
activemodel-serializers-xml
Expand Down
2 changes: 1 addition & 1 deletion app/concerns/new_application_form.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def buyers
end

def products
paginated_products.map { |p| ServicePresenter.new(p).new_application_data.as_json }
paginated_products.includes(:default_application_plan).map { |p| ServicePresenter.new(p).new_application_data.as_json }
end

def application_defined_fields_data(provider)
Expand Down
15 changes: 13 additions & 2 deletions app/controllers/admin/api/accounts_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,24 @@ def index
def find
buyer_account = find_buyer_account
authorize! :read, buyer_account
respond_with(buyer_account)
respond_with(preload_to_present(buyer_account))
end

# Account Read
# GET /admin/api/accounts/{id}.xml
def show
authorize! :read, buyer_account

respond_with(buyer_account)
respond_with(preload_to_present(buyer_account))
end

# Account Update
# PUT /admin/api/accounts/{id}.xml
def update
authorize! :update, buyer_account

preload_to_present(buyer_account)

buyer_account.vat_rate = params[:vat_rate].to_f if params[:vat_rate]
buyer_account.settings.attributes = billing_params
buyer_account.update_with_flattened_attributes(flat_params)
Expand Down Expand Up @@ -71,6 +73,7 @@ def change_plan
def approve
authorize! :approve, buyer_account

preload_to_present buyer_account
buyer_account.approve

respond_with(buyer_account)
Expand All @@ -81,6 +84,7 @@ def approve
def reject
authorize! :reject, buyer_account

preload_to_present buyer_account
buyer_account.reject

respond_with(buyer_account)
Expand All @@ -91,6 +95,7 @@ def reject
def make_pending
authorize! :update, buyer_account

preload_to_present buyer_account
buyer_account.make_pending

respond_with(buyer_account)
Expand All @@ -110,6 +115,12 @@ def buyer_account
@buyer_account ||= buyer_accounts.find(params[:id])
end

def preload_to_present(accounts)
# ActiveRecord::Associations::Preloader.new(records: Array(accounts), associations: [:annotations, {bought_plans: %i[original]}]).call # Rails 7.x
ActiveRecord::Associations::Preloader.new.preload(Array(accounts), [:annotations, {bought_plans: %i[original]}])
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this method ever used for arrays? if not, maybe we should rename the argument as account to make it clear it's accepts and returns a single object?

Otherwise (to make it more generic), it can be left as it is, but then I guess it should be Array(accounts).flatten in case accounts is actually an array?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't need flatten.

[1] pry(main)> Array([1,2,3])
=> [1, 2, 3]

Copy link
Contributor

Choose a reason for hiding this comment

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

oh OK, nice

Copy link
Contributor

Choose a reason for hiding this comment

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

It's the first time I see Preloader, what's the advantage over .includes? Why is this method supposed to receive "accounts" but instead it's called with buyer_account?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there are no advantages over #includes. Issue is that #includes only operates with Associations/queries that are not already performed. If your method receives a bunch of active record objects, then you can't call #includes on them.

accounts
end

def buyer_users
@buyer_users ||= current_account.buyer_users
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/admin/api/application_plans_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class Admin::Api::ApplicationPlansController < Admin::Api::ServiceBaseController
# Application Plan List
# GET /admin/api/services/{service_id}/application_plans.xml
def index
respond_with(application_plans)
respond_with(application_plans.includes(:original, :issuer))
end

# Application Plan Create
Expand Down
26 changes: 12 additions & 14 deletions app/controllers/admin/api/applications_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class Admin::Api::ApplicationsController < Admin::Api::BaseController
# GET /admin/api/applications.xml
def index
apps = applications.scope_search(search)
.serialization_preloading.paginate(:page => current_page, :per_page => per_page)
.serialization_preloading(request.format).paginate(:page => current_page, :per_page => per_page)
respond_with(apps)
end

Expand All @@ -24,28 +24,26 @@ def application_filter_params
end

def applications
@applications ||= begin
cinstances = current_account.provided_cinstances.where(service: accessible_services)
if (service_id = params[:service_id])
cinstances = cinstances.where(service_id: service_id)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is already being done by scope_search in #index. I was thinking whether this scope_serach call should be moved here inside #applications because #application also calls #applications.

But I think it doesn't make sense for #application because there a single app is selected by other parameters.

Copy link
Contributor

@mayorova mayorova Oct 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This is the actual fix for the query in JIRA, right? The rest are just N+1 improvements?
UPDATE: together with the provided_by refactoring...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

provided_by is the main thing. This is just further optimization.

Copy link
Contributor

Choose a reason for hiding this comment

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

But I think it doesn't make sense for #application because there a single app is selected by other parameters.

I actually don't agree with this.

We do have service_id as a filtering parameter in the API docs. And we actually need to conserve it, because we allow applications with the same app ID under different services. So, we should be able to find the application that belongs to the specified service.

application-find

I tried these three tests (in test/integration/user-management-api/applications_test.rb). They all pass in master but the 2nd and the 3rd fail in the branch.

  test 'find by user_key with correct service_id' do
    @service.backend_version = '1'
    @service.save!

    get find_admin_api_applications_path(format: :xml), params: { user_key: @application.user_key,
                                                                   provider_key: @provider.api_key,
                                                                   service_id: @service.id }

    assert_response :success
    assert_application(@response.body,
                       { id: @application.id,
                         user_key: @application.user_key })
  end

  test 'find by user_key with incorrect service_id' do
    @service.backend_version = '1'
    @service.save!

    another_service = FactoryBot.create(:service, account: @provider)

    get find_admin_api_applications_path(format: :xml), params: { user_key: @application.user_key,
                                                                   provider_key: @provider.api_key,
                                                                   service_id: another_service.id }

    assert_response :not_found
  end

  test 'find by user_key when there are two apps with the same user key' do
    @service.backend_version = '1'
    @service.save!

    another_service = FactoryBot.create(:service, account: @provider, backend_version: '1')
    another_app = FactoryBot.create(:cinstance,
                                    service: another_service,
                                    plan: FactoryBot.create(:application_plan, issuer: another_service),
                                    user_key: @application.user_key)

    get find_admin_api_applications_path(format: :xml), params: { user_key: @application.user_key,
                                                                  provider_key: @provider.api_key,
                                                                  service_id: another_service.id }

    assert_application(@response.body,
                       { id: another_app.id,
                         service_id: another_service.id,
                         user_key: another_app.user_key })
  end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a lot for catching this!

I added your tests and updated controller to add the service condition when searching by user_key. See fc5f1e7 . Is this enough? app_id should always be unique as far as I understand so it doesn't need such restriction. Correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nope, it's actually the same for app_id :) we allow having the same ID (and the app key too) as long as the apps are under different services.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😵‍💫 ok

      scope.where.has { (service.backend_version == '1') & (user_key == param_key) }.first!

                   when app_id = params[:app_id]
      scope.where.has { (service.backend_version != '1') & (application_id == app_id) }.first!

                     else
      scope.find(params[:application_id] || params[:id])

So it sounds like application_id and app_id are not the same. How about application_id, do we need it to be scope.find(params[:application_id] || params[:id]) or applications.find(params[:application_id] || params[:id]) is sufficient (in case this finally is the primary key and can't be anything else)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, right, application_id seems to the be id of the Cinstance (aka primary key). It seems a bit weird using it as a search query parameter... But well. To answer your question I'd say we need to check the current behavior. To me it seems logical to apply find to the already filtered scope. So, if you have the following:

service: 1
  application: 
    id: 11
    app_id: a1
    
service: 2
  application: 
    id: 22
    app_id: a2

searching by application_id=11&app_id=a1&service_id=1 should return the first application, while any other combination should return a 404, e.g.:
application_id=22&app_id=a2&service_id=1
application_id=22&app_id=a1&service_id=2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you have app_id then application_id appears to be ignored. Historically using the wrong service_id would still return nothing though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, PTAL

end
cinstances
end
@applications ||= current_account.provided_cinstances.merge(accessible_services)
end

def application
@application ||= case
return @application if defined?(@application)

scope = params[:service_id] ? applications.where(service_id: params[:service_id]) : applications

@application = case

when user_key = params[:user_key]
when param_key = params[:user_key]
Copy link
Contributor

Choose a reason for hiding this comment

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

why rename user_key to param_key ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because underneath there is a baby_squeel expression (user_key == param_key). I thought that even if it worked, it would be highly confusing to read (user_key == user_key)

# TODO: these scopes should be in model layer
# but there is scope named by_user_key already
applications.joins(:service).where("(services.backend_version = '1' AND cinstances.user_key = ?)", user_key).first!
scope.where.has { (service.backend_version == '1') & (user_key == param_key) }.first!

when app_id = params[:app_id]
applications.joins(:service).where("(services.backend_version <> '1' AND cinstances.application_id = ?)", app_id).first!
when app_id = params[:app_id]
scope.where.has { (service.backend_version != '1') & (application_id == app_id) }.first!

else
applications.find(params[:application_id] || params[:id])
scope.find(params[:application_id] || params[:id])
end
end

Expand Down
1 change: 1 addition & 0 deletions app/lib/apicast/provider_source.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ def attributes_for_proxy
]
}

ActiveRecord::Associations::Preloader.new.preload(provider, {services: [:service_tokens, {backend_api_configs: :backend_api, proxy: [:gateway_configuration, {proxy_rules: :metric}]}]})
provider.as_json(hash).merge(timestamp: Time.now.utc.iso8601)
end

Expand Down
2 changes: 1 addition & 1 deletion app/lib/applications_controller_methods.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def find_states
end

def find_cinstance
@cinstance = accessible_not_bought_cinstances.includes(plan: %i[service original plan_metrics pricing_rules])
@cinstance = accessible_not_bought_cinstances.includes(plan: %i[service pricing_rules])
.find(params[:id])
end

Expand Down
2 changes: 1 addition & 1 deletion app/lib/backend_api_logic/routing_policy.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def policy_chain
end

def with_subpaths?
backend_api_configs.with_subpath.any?
backend_api_configs.any?(&:with_subpath?)
end

class Builder
Expand Down
1 change: 1 addition & 0 deletions app/lib/signup/plans_with_defaults.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ def service_plan_errors
end

def any_plan_for?(issuer:, plan_type:)
ActiveRecord::Associations::Preloader.new.preload(plans[plan_type], [:issuer])
plans[plan_type].any? { |plan| plan.issuer == issuer }
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/account.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ def on_trial?
# Grabs the support_email if defined, otherwise falls back to the email of first admin. Dog.
def support_email
se = self[:support_email]
se.presence || admins.first&.email
se.presence || first_admin&.email
Copy link
Contributor

Choose a reason for hiding this comment

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

There are many occurrences of admins.first, should we change all of them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needs to be evaluated. Maybe but I'm not sure it can be a simple sed command because it involves caching.

I would rather leave this for further PRs. This is big enough already.

end

def finance_support_email
Expand Down
4 changes: 4 additions & 0 deletions app/models/backend_api_config.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ class BackendApiConfig < ApplicationRecord

delegate :proxy, to: :service, allow_nil: true

def with_subpath?
path != ConfigPath::EMPTY_PATH
end

def path=(value)
super(ConfigPath.new(value).path)
end
Expand Down
18 changes: 14 additions & 4 deletions app/models/cinstance.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,9 @@ def validate_plan_is_unique?
}

def self.provided_by(account)
joins(:service).references(:service).merge(Service.of_account(account)).readonly(false)
# we can access service through plan but also keep service.id in sync with plan.service.id
# this is a simpler way to do the query used historically
joins(:service).where.has { service.sift(:of_account, account) }
Comment on lines +151 to +153
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the main fix point (UPDATED). But there are further optimizations in other files.

Copy link
Contributor

Choose a reason for hiding this comment

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

@akostadinov Do you have any idea why the ogiginal query had readonly(false)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no idea, probably itdidn't work without it at some point. I think it shouldn't be needed anymore

https://stackoverflow.com/a/33360519/520567

end

scope :not_bought_by, ->(account) { where.has { user_account_id != account.id } }
Expand Down Expand Up @@ -199,9 +201,17 @@ def self.by_service(service)

# maybe move both limit methods to their models?

def self.serialization_preloading
includes(:application_keys, :plan, :user_account,
service: [:account, :default_application_plan])
def self.serialization_preloading(format = nil)
# With Rails 6.1 trying to include plan->issuer without service results in
# > Cannot eagerly load the polymorphic association :issuer
# When both have the same sub-includes, cache takes care of the duplicate queries.
service_includes = %i[proxy account]
plan_includes = [{issuer: service_includes}]
if format == "xml"
service_includes << :default_application_plan
plan_includes << :original
end
Comment on lines +210 to +213
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this only for XML responses?... JSON/UI don't need these fields?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For JSON it complained that we are eager loading stuff that is not later accessed. UI uses another controller.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like this too much... but OK, if that makes the queries more efficient - let's keep it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Otherwise these are queried once per entry displayed in the XML. So we should have it. But if you have a better idea how to do, that would be cool.

includes(:user_account, service: service_includes, plan: plan_includes)
end


Expand Down
6 changes: 5 additions & 1 deletion app/models/service.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,11 @@ class Service < ApplicationRecord # rubocop:disable Metrics/ClassLength
service.has_many :api_docs_services, class_name: 'ApiDocs::Service'
end

scope :of_account, ->(account) { where.has { account_id == account.id } }
sifter :of_account do |account|
account_id == account.id
end

scope :of_account, ->(account) { where.has { sift(:of_account, account) } }
Copy link
Contributor

Choose a reason for hiding this comment

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

Is using sifter any better than a plain scope?...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can use the sifter in a join query while I don't think you can use a scope like that.


has_one :proxy, dependent: :destroy, inverse_of: :service, autosave: true

Expand Down
2 changes: 1 addition & 1 deletion app/presenters/applications_index_presenter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ def raw_applications
def applications
@applications ||= raw_applications.scope_search(search)
.order_by(*sorting_params)
.preload(:service, user_account: %i[admin_user], plan: %i[pricing_rules])
.includes(plan: %i[pricing_rules])
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for my laziness - what is the main difference between preload and includes ? 😬

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIU #preload simply preloads what was requested with separate queries. While #includes is supposed to be smarter and extract objects from joins as well. Also it accumulates with other includes. There is a caveat that if in the join some associated objects are filtered, they are not seen by #includes.

In this case I don't want to mix them because IIRC there was some includes used in a chain and probably that will play better together with it. I'm not sure whether preload plays together well with includes. But also see no reason to experiment. includes seems to be the primary documented way. Although some people are concerned about possible surprises with it due to filtering of related objects in joins.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks a lot for the explanation!

.paginate(pagination_params)
.decorate
end
Expand Down
8 changes: 7 additions & 1 deletion config/abilities/provider_member.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,13 @@
can :create, Account
can :update, Account if account.provider_can_use?(:service_permissions)

can %i[read show edit update], Cinstance, user.accessible_cinstances.where_values_hash
# Using historical optimized way and leave canonical way (through plan) commented out below
# The resulting hash presently is something like {"type"=>"Cinstance", "service_id"=>[ids..]}
can %i[read show edit update], Cinstance, Cinstance.permitted_for(user).where_values_hash
# can %i[read show edit update], Cinstance, user.accessible_cinstances do |cinstance|
# cinstance.plan&.issuer_type == "Service" && cinstance.plan.issuer.account == user.account &&
# (!user.forbidden_some_services? || user.member_permission_service_ids.include?(cinstance.plan.issuer.id))
# end
Comment on lines +31 to +35
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the difference between the new one, the old one and the commented one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the commented out uses a block which is not recommended. I believe more description is in the commit messages. But for the time being, I'm looking at other things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

 fix user can access Cinstance

The `#where_values_hash` method does not support joins and sub-queries.

Originally the `account.provided_cinstances` part was ignored because it
was JOINs. With the update to sub-queries, it turned into `plan_id: nil`
which is incorrect and broke the logic.

So now we keep logic as previously by resorting only to the
`Cinstance.permitted_for` part of the query.

This relies on the fact that `Cinstance.plan.issuer` is set as
`Cinstance.service` when that issuer is a service.

Also relies on the fact that `User.member_permission_service_ids` will
not set to ids of services that don't belong to the account.

Which may not be ideal but allows for permission checking without extra
database queries.


# abilities for buyer users
can %i[read update update_role destroy suspend unsuspend], User, account: { provider_account_id: user.account_id }
Expand Down
4 changes: 0 additions & 4 deletions config/environments/test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,6 @@
Bullet.add_safelist class_name: "Alert", type: :n_plus_one_query, association: :cinstance
Bullet.add_safelist class_name: "ApiDocs::Service", type: :unused_eager_loading, association: :service
Bullet.add_safelist class_name: "ApplicationPlan", type: :n_plus_one_query, association: :customizations
Bullet.add_safelist class_name: "ApplicationPlan", type: :n_plus_one_query, association: :issuer
Bullet.add_safelist class_name: "ApplicationPlan", type: :n_plus_one_query, association: :original
Bullet.add_safelist class_name: "ApplicationPlan", type: :n_plus_one_query, association: :pricing_rules
Bullet.add_safelist class_name: "ApplicationPlan", type: :n_plus_one_query, association: :usage_limits
Bullet.add_safelist class_name: "ApplicationPlan", type: :unused_eager_loading, association: :issuer
Expand Down Expand Up @@ -170,10 +168,8 @@
Bullet.add_safelist class_name: "Service", type: :counter_cache, association: :backend_api_configs
Bullet.add_safelist class_name: "Service", type: :counter_cache, association: :cinstances
Bullet.add_safelist class_name: "Service", type: :n_plus_one_query, association: :account
Bullet.add_safelist class_name: "Service", type: :n_plus_one_query, association: :default_application_plan
Bullet.add_safelist class_name: "Service", type: :n_plus_one_query, association: :default_service_plan
Bullet.add_safelist class_name: "Service", type: :n_plus_one_query, association: :metrics
Bullet.add_safelist class_name: "Service", type: :n_plus_one_query, association: :proxy
Bullet.add_safelist class_name: "Service", type: :unused_eager_loading, association: :application_plans
Bullet.add_safelist class_name: "ServiceContract", type: :n_plus_one_query, association: :plan
Bullet.add_safelist class_name: "ServiceContract", type: :n_plus_one_query, association: :user_account
Expand Down
5 changes: 5 additions & 0 deletions config/initializers/active-record-query-trace.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
if (Rails.env.development? || Rails.env.test?) && ENV["TRACE_SQL"] == "1"
ActiveRecordQueryTrace.enabled = true
ActiveRecordQueryTrace.level = :app
Rails.application.config.log_level = :debug
end
3 changes: 1 addition & 2 deletions lib/developer_portal/app/controllers/developer_portal/admin/plans_widget_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ def index
end

@wizard = params[:wizard].to_s == 'true'
@plans = @service.application_plans.not_custom.published.to_a
@plans = @service.application_plans.not_custom.published.includes(:issuer).to_a
@plans.delete @plan
end

end
Loading