From c16f40d171808a6dfab8e8c873b17036d7c2b97c Mon Sep 17 00:00:00 2001
From: "Aleksandar N. Kostadinov" <akostadi@redhat.com>
Date: Wed, 10 Jul 2024 23:29:24 +0300
Subject: [PATCH 01/13] THREESCALE-11139 simplify applications query

---
 .../admin/api/applications_controller.rb      |  8 +-----
 app/models/cinstance.rb                       |  7 +++---
 .../user-management-api/applications_test.rb  | 25 +++++++++++++++++++
 3 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/app/controllers/admin/api/applications_controller.rb b/app/controllers/admin/api/applications_controller.rb
index b9672bcafb..57679fa4ef 100644
--- a/app/controllers/admin/api/applications_controller.rb
+++ b/app/controllers/admin/api/applications_controller.rb
@@ -24,13 +24,7 @@ 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)
-      end
-      cinstances
-    end
+    @applications ||= Cinstance.provided_by(current_account, service_filter: -> { _1.merge accessible_services })
   end
 
   def application
diff --git a/app/models/cinstance.rb b/app/models/cinstance.rb
index 4d60f06861..c475e36676 100644
--- a/app/models/cinstance.rb
+++ b/app/models/cinstance.rb
@@ -147,8 +147,9 @@ def validate_plan_is_unique?
     where(['cinstances.created_at <= ?', period.end])
   }
 
-  def self.provided_by(account)
-    joins(:service).references(:service).merge(Service.of_account(account)).readonly(false)
+  def self.provided_by(account, service_filter: nil)
+    service_filter ||= -> { _1 }
+    where(plan_id: ApplicationPlan.unscoped.where(issuer_type: "Service", issuer_id: service_filter[Service.unscoped.select(:id).of_account(account)]))
   end
 
   scope :not_bought_by, ->(account) { where.has { user_account_id != account.id } }
@@ -200,7 +201,7 @@ def self.by_service(service)
   # maybe move both limit methods to their models?
 
   def self.serialization_preloading
-    includes(:application_keys, :plan, :user_account,
+    includes(:plan, :user_account,
              service: [:account, :default_application_plan])
   end
 
diff --git a/test/integration/user-management-api/applications_test.rb b/test/integration/user-management-api/applications_test.rb
index a29e653063..028bf8cabd 100644
--- a/test/integration/user-management-api/applications_test.rb
+++ b/test/integration/user-management-api/applications_test.rb
@@ -32,14 +32,39 @@ def setup
     User.any_instance.stubs(:has_access_to_all_services?).returns(false)
     user  = FactoryBot.create(:member, account: @provider, admin_sections: ['partners'])
     token = FactoryBot.create(:access_token, owner: user, scopes: 'account_management')
+    service_2 = FactoryBot.create(:service, account: @provider)
+    service_3 = FactoryBot.create(:service, account: @provider)
+    application_plan_2 = FactoryBot.create(:application_plan, issuer: service_2)
+    application_plan_3 = FactoryBot.create(:application_plan, issuer: service_3)
+    application_2 = FactoryBot.create(:cinstance, plan: application_plan_2, user_account: @buyer)
+    FactoryBot.create(:cinstance, plan: application_plan_3, user_account: @buyer)
 
     get(admin_api_applications_path)
     assert_response :forbidden
     get admin_api_applications_path, params: { access_token: token.value }
     assert_response :success
+    assert_select "applications/application", false
+
     User.any_instance.expects(:member_permission_service_ids).returns([@service.id]).at_least_once
+    get admin_api_applications_path, params: { access_token: token.value, service_id: @service.id + 1 }
+    assert_response :success
+    assert_select "applications/application", false
+
+    User.any_instance.expects(:member_permission_service_ids).returns([@service.id, service_2.id]).at_least_once
+    get admin_api_applications_path, params: { access_token: token.value }
+    assert_response :success
+    assert_select "applications/application", 2
+    assert_select "applications/application/id", @application.id.to_s
+    assert_select "applications/application/service_id", @service.id.to_s
+    assert_select "applications/application/id", application_2.id.to_s
+    assert_select "applications/application/service_id", service_2.id.to_s
+
+    User.any_instance.expects(:member_permission_service_ids).returns([@service.id, service_2.id]).at_least_once
     get admin_api_applications_path, params: { access_token: token.value, service_id: @service.id }
     assert_response :success
+    assert_select "applications/application", 1
+    assert_select "applications/application/id", @application.id.to_s
+    assert_select "applications/application/service_id", @service.id.to_s
   end
 
   # Provider key

From 8ed62c9134e95df6a4c789a3dc5767dc26890e8d Mon Sep 17 00:00:00 2001
From: "Aleksandar N. Kostadinov" <akostadi@redhat.com>
Date: Fri, 12 Jul 2024 00:19:52 +0300
Subject: [PATCH 02/13] optimize application queries further

---
 app/controllers/admin/api/applications_controller.rb | 8 ++++----
 app/models/cinstance.rb                              | 5 ++---
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/app/controllers/admin/api/applications_controller.rb b/app/controllers/admin/api/applications_controller.rb
index 57679fa4ef..c0a13b1b62 100644
--- a/app/controllers/admin/api/applications_controller.rb
+++ b/app/controllers/admin/api/applications_controller.rb
@@ -23,8 +23,8 @@ def application_filter_params
     params.permit(:active_since, :inactive_since)
   end
 
-  def applications
-    @applications ||= Cinstance.provided_by(current_account, service_filter: -> { _1.merge accessible_services })
+  def applications(services_association: Service.unscoped)
+    @applications ||= Cinstance.provided_by(current_account, services_association: services_association.merge(accessible_services))
   end
 
   def application
@@ -33,10 +33,10 @@ def application
                      when user_key = params[: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!
+      applications(services_association: Service.unscoped.where(backend_version: '1')).where(user_key: user_key).first!
 
                      when app_id = params[:app_id]
-      applications.joins(:service).where("(services.backend_version <> '1' AND cinstances.application_id = ?)", app_id).first!
+      applications(services_association: Service.unscoped.where.not(backend_version: '1')).where(application_id: app_id).first!
 
                      else
       applications.find(params[:application_id] || params[:id])
diff --git a/app/models/cinstance.rb b/app/models/cinstance.rb
index c475e36676..d9161036ec 100644
--- a/app/models/cinstance.rb
+++ b/app/models/cinstance.rb
@@ -147,9 +147,8 @@ def validate_plan_is_unique?
     where(['cinstances.created_at <= ?', period.end])
   }
 
-  def self.provided_by(account, service_filter: nil)
-    service_filter ||= -> { _1 }
-    where(plan_id: ApplicationPlan.unscoped.where(issuer_type: "Service", issuer_id: service_filter[Service.unscoped.select(:id).of_account(account)]))
+  def self.provided_by(account, services_association: Service.unscoped)
+    where(plan_id: ApplicationPlan.unscoped.where(issuer_type: "Service", issuer_id: Service.unscoped.select(:id).of_account(account).merge(services_association)))
   end
 
   scope :not_bought_by, ->(account) { where.has { user_account_id != account.id } }

From f788d8d4f73946ac0050d4baa58370a697d1502c Mon Sep 17 00:00:00 2001
From: "Aleksandar N. Kostadinov" <akostadi@redhat.com>
Date: Fri, 12 Jul 2024 00:20:32 +0300
Subject: [PATCH 03/13] Bullet optimization of
 Admin::Api::ApplicationsController

```
Bullet::Notification::UnoptimizedQueryError: user: localuser
GET http://admin.foo.3scale.localhost:39073/p/admin/applications/16
AVOID eager loading detected
  ApplicationPlan => [:plan_metrics]
  Remove from your query: .includes([:plan_metrics])
```
---
 app/lib/applications_controller_methods.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/lib/applications_controller_methods.rb b/app/lib/applications_controller_methods.rb
index 23115cdc2c..010cfb6655 100644
--- a/app/lib/applications_controller_methods.rb
+++ b/app/lib/applications_controller_methods.rb
@@ -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 original pricing_rules])
                                                  .find(params[:id])
   end
 

From 634e6ecc640c9ca9f43eb9262f0dc5cba3d9eca5 Mon Sep 17 00:00:00 2001
From: "Aleksandar N. Kostadinov" <akostadi@redhat.com>
Date: Sat, 3 Aug 2024 23:08:58 +0300
Subject: [PATCH 04/13] UnoptimizedQuery ApplicationPlan => [:original]

Fixes issue with keys.feature:Regenerate provider key

Bullet::Notification::UnoptimizedQueryError: user: avalon
PUT http://master-account.3scale.localhost:42285/p/admin/applications/4/change_user_key
AVOID eager loading detected
  ApplicationPlan => [:original]
  Remove from your query: .includes([:original])
---
 app/lib/applications_controller_methods.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/lib/applications_controller_methods.rb b/app/lib/applications_controller_methods.rb
index 010cfb6655..f2c5f45411 100644
--- a/app/lib/applications_controller_methods.rb
+++ b/app/lib/applications_controller_methods.rb
@@ -48,7 +48,7 @@ def find_states
   end
 
   def find_cinstance
-    @cinstance = accessible_not_bought_cinstances.includes(plan: %i[service original pricing_rules])
+    @cinstance = accessible_not_bought_cinstances.includes(plan: %i[service pricing_rules])
                                                  .find(params[:id])
   end
 

From 081a8c4eb27637773d9792e820a97c4f161135f6 Mon Sep 17 00:00:00 2001
From: "Aleksandar N. Kostadinov" <akostadi@redhat.com>
Date: Sun, 4 Aug 2024 15:09:47 +0300
Subject: [PATCH 05/13] debugging info on failure

---
 test/unit/finance/billing_strategy_test.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/unit/finance/billing_strategy_test.rb b/test/unit/finance/billing_strategy_test.rb
index 5fd5b8beb3..d0960c94b3 100644
--- a/test/unit/finance/billing_strategy_test.rb
+++ b/test/unit/finance/billing_strategy_test.rb
@@ -78,7 +78,7 @@ def setup
     canaries = FactoryBot.create_list(:provider_with_billing, 4).map(&:id)
     ThreeScale.config.payments.expects(:billing_canaries).at_least_once.returns(canaries)
 
-    Finance::BillingStrategy.expects(:daily_async).with { |scope| scope.pluck(:account_id) == canaries }.returns(true)
+    Finance::BillingStrategy.expects(:daily_async).with { |scope| assert_equal canaries, scope.pluck(:account_id) }.returns(true)
     assert Finance::BillingStrategy.daily_canaries
   end
 

From 37a6b21fecdcfa3f068060e49d1d96e314ab875d Mon Sep 17 00:00:00 2001
From: "Aleksandar N. Kostadinov" <akostadi@redhat.com>
Date: Sun, 4 Aug 2024 15:23:20 +0300
Subject: [PATCH 06/13] eager loading :default_application_plan of Service

```
Bullet::Notification::UnoptimizedQueryError:
  user: default
  GET /admin/api/applications.json?provider_key=***&inactive_since=2014-05-05
  AVOID eager loading detected
    Service => [:default_application_plan]
    Remove from your query: .includes([:default_application_plan])
```
---
 app/models/cinstance.rb | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/app/models/cinstance.rb b/app/models/cinstance.rb
index d9161036ec..20ac1460fd 100644
--- a/app/models/cinstance.rb
+++ b/app/models/cinstance.rb
@@ -200,8 +200,7 @@ def self.by_service(service)
   # maybe move both limit methods to their models?
 
   def self.serialization_preloading
-    includes(:plan, :user_account,
-             service: [:account, :default_application_plan])
+    includes(:plan, :user_account, service: [:account])
   end
 
 

From 4e955486fc4592e29b882f2424efdfcfb2bae802 Mon Sep 17 00:00:00 2001
From: "Aleksandar N. Kostadinov" <akostadi@redhat.com>
Date: Tue, 6 Aug 2024 12:00:30 +0300
Subject: [PATCH 07/13] 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.
---
 config/abilities/provider_member.rb         | 9 ++++++++-
 test/unit/abilities/provider_member_test.rb | 6 +++---
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/config/abilities/provider_member.rb b/config/abilities/provider_member.rb
index 499499dad0..abe62133ac 100644
--- a/config/abilities/provider_member.rb
+++ b/config/abilities/provider_member.rb
@@ -26,7 +26,14 @@
     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 commented out below
+    # The resulting hash presently is something like {"type"=>"Cinstance", "service_id"=>[ids..]}
+    # Seems like canonically though that we are moving towards accessing service through plan
+    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
 
     # abilities for buyer users
     can %i[read update update_role destroy suspend unsuspend], User, account: { provider_account_id: user.account_id }
diff --git a/test/unit/abilities/provider_member_test.rb b/test/unit/abilities/provider_member_test.rb
index 96b3abd65a..7576ef6f39 100644
--- a/test/unit/abilities/provider_member_test.rb
+++ b/test/unit/abilities/provider_member_test.rb
@@ -121,9 +121,9 @@ def test_cinstances
       service_2 = FactoryBot.create(:simple_service, account: @account)
       service_3 = FactoryBot.create(:simple_service, account: @account)
 
-      plan_1 = FactoryBot.create(:simple_application_plan, service: service_1)
-      plan_2 = FactoryBot.create(:simple_application_plan, service: service_2)
-      plan_3 = FactoryBot.create(:simple_application_plan, service: service_3)
+      plan_1 = FactoryBot.create(:simple_application_plan, issuer: service_1)
+      plan_2 = FactoryBot.create(:simple_application_plan, issuer: service_2)
+      plan_3 = FactoryBot.create(:simple_application_plan, issuer: service_3)
 
       app_1 = FactoryBot.create(:simple_cinstance, plan: plan_1)
       app_2 = FactoryBot.create(:simple_cinstance, plan: plan_2)

From ef6760bc0cc2586d010d96798edda7bfcf201c8b Mon Sep 17 00:00:00 2001
From: "Aleksandar N. Kostadinov" <akostadi@redhat.com>
Date: Thu, 12 Sep 2024 00:13:53 +0300
Subject: [PATCH 08/13] access cinstance service directly not through plan

Historically it was a conscious decision to optimize access to cintance
-> service by denormalizing the database. So we can access service
directly and not through the plan.

We also keep these two in sync with model callbacks.

So this commit further simplifies the queries to fully rely on this
fact.
---
 app/controllers/admin/api/applications_controller.rb | 10 +++++-----
 app/models/cinstance.rb                              |  6 ++++--
 app/models/service.rb                                |  6 +++++-
 config/abilities/provider_member.rb                  |  3 +--
 4 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/app/controllers/admin/api/applications_controller.rb b/app/controllers/admin/api/applications_controller.rb
index c0a13b1b62..62cf6cafbb 100644
--- a/app/controllers/admin/api/applications_controller.rb
+++ b/app/controllers/admin/api/applications_controller.rb
@@ -23,20 +23,20 @@ def application_filter_params
     params.permit(:active_since, :inactive_since)
   end
 
-  def applications(services_association: Service.unscoped)
-    @applications ||= Cinstance.provided_by(current_account, services_association: services_association.merge(accessible_services))
+  def applications
+    @applications ||= current_account.provided_cinstances.merge(accessible_services)
   end
 
   def application
     @application ||= case
 
-                     when user_key = params[:user_key]
+                     when param_key = params[:user_key]
       # TODO: these scopes should be in model layer
       # but there is scope named by_user_key already
-      applications(services_association: Service.unscoped.where(backend_version: '1')).where(user_key: user_key).first!
+      applications.where.has { (service.backend_version == '1') & (user_key == param_key) }.first!
 
                      when app_id = params[:app_id]
-      applications(services_association: Service.unscoped.where.not(backend_version: '1')).where(application_id: app_id).first!
+      applications.where.has { (service.backend_version != '1') & (application_id == app_id) }.first!
 
                      else
       applications.find(params[:application_id] || params[:id])
diff --git a/app/models/cinstance.rb b/app/models/cinstance.rb
index 20ac1460fd..ad949041ec 100644
--- a/app/models/cinstance.rb
+++ b/app/models/cinstance.rb
@@ -147,8 +147,10 @@ def validate_plan_is_unique?
     where(['cinstances.created_at <= ?', period.end])
   }
 
-  def self.provided_by(account, services_association: Service.unscoped)
-    where(plan_id: ApplicationPlan.unscoped.where(issuer_type: "Service", issuer_id: Service.unscoped.select(:id).of_account(account).merge(services_association)))
+  def self.provided_by(account)
+    # 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) }
   end
 
   scope :not_bought_by, ->(account) { where.has { user_account_id != account.id } }
diff --git a/app/models/service.rb b/app/models/service.rb
index 7b9a415a14..d65494d2a8 100644
--- a/app/models/service.rb
+++ b/app/models/service.rb
@@ -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) } }
 
   has_one :proxy, dependent: :destroy, inverse_of: :service, autosave: true
 
diff --git a/config/abilities/provider_member.rb b/config/abilities/provider_member.rb
index abe62133ac..486ccf57da 100644
--- a/config/abilities/provider_member.rb
+++ b/config/abilities/provider_member.rb
@@ -26,9 +26,8 @@
     can :create, Account
     can :update, Account if account.provider_can_use?(:service_permissions)
 
-    # Using historical optimized way and leave canonical way commented out below
+    # 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..]}
-    # Seems like canonically though that we are moving towards accessing service through plan
     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 &&

From aaea00d315f7b5caf301a7320c814fc6f8d75b41 Mon Sep 17 00:00:00 2001
From: "Aleksandar N. Kostadinov" <akostadi@redhat.com>
Date: Thu, 12 Sep 2024 01:06:41 +0300
Subject: [PATCH 09/13] remove some n+1 issues

---
 app/concerns/new_application_form.rb              |  2 +-
 app/controllers/admin/api/accounts_controller.rb  | 15 +++++++++++++--
 .../admin/api/application_plans_controller.rb     |  2 +-
 .../admin/api/applications_controller.rb          |  2 +-
 app/models/cinstance.rb                           | 13 +++++++++++--
 app/presenters/applications_index_presenter.rb    |  2 +-
 config/environments/test.rb                       |  4 ----
 .../admin/plans_widget_controller.rb              |  3 +--
 8 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/app/concerns/new_application_form.rb b/app/concerns/new_application_form.rb
index b9cc585d38..735dc3f14b 100644
--- a/app/concerns/new_application_form.rb
+++ b/app/concerns/new_application_form.rb
@@ -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)
diff --git a/app/controllers/admin/api/accounts_controller.rb b/app/controllers/admin/api/accounts_controller.rb
index e2422e1ebc..efb95fcd9a 100644
--- a/app/controllers/admin/api/accounts_controller.rb
+++ b/app/controllers/admin/api/accounts_controller.rb
@@ -22,7 +22,7 @@ def index
   def find
     buyer_account = find_buyer_account
     authorize! :read, buyer_account
-    respond_with(buyer_account)
+    respond_with(to_present(buyer_account))
   end
 
   # Account Read
@@ -30,7 +30,7 @@ def find
   def show
     authorize! :read, buyer_account
 
-    respond_with(buyer_account)
+    respond_with(to_present(buyer_account))
   end
 
   # Account Update
@@ -38,6 +38,8 @@ def show
   def update
     authorize! :update, buyer_account
 
+    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)
@@ -71,6 +73,7 @@ def change_plan
   def approve
     authorize! :approve, buyer_account
 
+    to_present buyer_account
     buyer_account.approve
 
     respond_with(buyer_account)
@@ -81,6 +84,7 @@ def approve
   def reject
     authorize! :reject, buyer_account
 
+    to_present buyer_account
     buyer_account.reject
 
     respond_with(buyer_account)
@@ -91,6 +95,7 @@ def reject
   def make_pending
     authorize! :update, buyer_account
 
+    to_present buyer_account
     buyer_account.make_pending
 
     respond_with(buyer_account)
@@ -110,6 +115,12 @@ def buyer_account
     @buyer_account ||= buyer_accounts.find(params[:id])
   end
 
+  def 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]}])
+    accounts
+  end
+
   def buyer_users
     @buyer_users ||= current_account.buyer_users
   end
diff --git a/app/controllers/admin/api/application_plans_controller.rb b/app/controllers/admin/api/application_plans_controller.rb
index 1f15b278d4..2781cbbe6d 100644
--- a/app/controllers/admin/api/application_plans_controller.rb
+++ b/app/controllers/admin/api/application_plans_controller.rb
@@ -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
diff --git a/app/controllers/admin/api/applications_controller.rb b/app/controllers/admin/api/applications_controller.rb
index 62cf6cafbb..6ce56d0e6c 100644
--- a/app/controllers/admin/api/applications_controller.rb
+++ b/app/controllers/admin/api/applications_controller.rb
@@ -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
 
diff --git a/app/models/cinstance.rb b/app/models/cinstance.rb
index ad949041ec..dd0e3f9958 100644
--- a/app/models/cinstance.rb
+++ b/app/models/cinstance.rb
@@ -201,8 +201,17 @@ def self.by_service(service)
 
   # maybe move both limit methods to their models?
 
-  def self.serialization_preloading
-    includes(:plan, :user_account, service: [:account])
+  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
+    includes(:user_account, service: service_includes, plan: plan_includes)
   end
 
 
diff --git a/app/presenters/applications_index_presenter.rb b/app/presenters/applications_index_presenter.rb
index fc4226f169..4c838dc9e3 100644
--- a/app/presenters/applications_index_presenter.rb
+++ b/app/presenters/applications_index_presenter.rb
@@ -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])
                                       .paginate(pagination_params)
                                       .decorate
   end
diff --git a/config/environments/test.rb b/config/environments/test.rb
index 24a7f3677f..14fe179bec 100644
--- a/config/environments/test.rb
+++ b/config/environments/test.rb
@@ -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
@@ -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
diff --git a/lib/developer_portal/app/controllers/developer_portal/admin/plans_widget_controller.rb b/lib/developer_portal/app/controllers/developer_portal/admin/plans_widget_controller.rb
index 186f472b0f..152d8966ac 100644
--- a/lib/developer_portal/app/controllers/developer_portal/admin/plans_widget_controller.rb
+++ b/lib/developer_portal/app/controllers/developer_portal/admin/plans_widget_controller.rb
@@ -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

From 82e5492fd89987093cd48d46c291f533b2405b20 Mon Sep 17 00:00:00 2001
From: "Aleksandar N. Kostadinov" <akostadi@redhat.com>
Date: Thu, 12 Sep 2024 16:43:26 +0300
Subject: [PATCH 10/13] allow tracing SQL queries

---
 Gemfile                                          | 2 ++
 Gemfile.lock                                     | 3 +++
 config/initializers/active-record-query-trace.rb | 5 +++++
 3 files changed, 10 insertions(+)
 create mode 100644 config/initializers/active-record-query-trace.rb

diff --git a/Gemfile b/Gemfile
index 3711240101..6943425f09 100644
--- a/Gemfile
+++ b/Gemfile
@@ -233,6 +233,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'
diff --git a/Gemfile.lock b/Gemfile.lock
index bc83feefae..9888bb7130 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -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)
@@ -942,6 +944,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
diff --git a/config/initializers/active-record-query-trace.rb b/config/initializers/active-record-query-trace.rb
new file mode 100644
index 0000000000..c7a0cf1b4b
--- /dev/null
+++ b/config/initializers/active-record-query-trace.rb
@@ -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

From 01d602cfc7b4e27a4f68ae5569e844f3b8e44283 Mon Sep 17 00:00:00 2001
From: "Aleksandar N. Kostadinov" <akostadi@redhat.com>
Date: Thu, 12 Sep 2024 16:43:56 +0300
Subject: [PATCH 11/13] fix N+1s for NginxesController

---
 app/lib/apicast/provider_source.rb          | 1 +
 app/lib/backend_api_logic/routing_policy.rb | 2 +-
 app/models/account.rb                       | 2 +-
 app/models/backend_api_config.rb            | 4 ++++
 4 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/app/lib/apicast/provider_source.rb b/app/lib/apicast/provider_source.rb
index 0a6a9952c7..611729c978 100644
--- a/app/lib/apicast/provider_source.rb
+++ b/app/lib/apicast/provider_source.rb
@@ -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
 
diff --git a/app/lib/backend_api_logic/routing_policy.rb b/app/lib/backend_api_logic/routing_policy.rb
index 7c744eb1f6..a3ce54bbb7 100644
--- a/app/lib/backend_api_logic/routing_policy.rb
+++ b/app/lib/backend_api_logic/routing_policy.rb
@@ -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
diff --git a/app/models/account.rb b/app/models/account.rb
index ff951ec1bf..d2bc9547bf 100644
--- a/app/models/account.rb
+++ b/app/models/account.rb
@@ -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
   end
 
   def finance_support_email
diff --git a/app/models/backend_api_config.rb b/app/models/backend_api_config.rb
index bf0c3662dd..fa82b9d573 100644
--- a/app/models/backend_api_config.rb
+++ b/app/models/backend_api_config.rb
@@ -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

From 53d022240068a0610d17f6281a6915616b05f0c3 Mon Sep 17 00:00:00 2001
From: "Aleksandar N. Kostadinov" <akostadi@redhat.com>
Date: Thu, 12 Sep 2024 17:13:41 +0300
Subject: [PATCH 12/13] N+1 in signup controller

---
 app/lib/signup/plans_with_defaults.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/lib/signup/plans_with_defaults.rb b/app/lib/signup/plans_with_defaults.rb
index 95e60088d3..55d0a442d2 100644
--- a/app/lib/signup/plans_with_defaults.rb
+++ b/app/lib/signup/plans_with_defaults.rb
@@ -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

From 62b07d952ba5aaec55387f5d1e75b3aab5ab0753 Mon Sep 17 00:00:00 2001
From: "Aleksandar N. Kostadinov" <akostadi@redhat.com>
Date: Sat, 14 Sep 2024 01:07:18 +0300
Subject: [PATCH 13/13] fix cinstances N+1 bullets

---
 config/environments/test.rb | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/config/environments/test.rb b/config/environments/test.rb
index 14fe179bec..f6cf533964 100644
--- a/config/environments/test.rb
+++ b/config/environments/test.rb
@@ -131,17 +131,6 @@
       Bullet.add_safelist class_name: "CMS::File", type: :n_plus_one_query, association: :provider
       Bullet.add_safelist class_name: "CMS::Page", type: :n_plus_one_query, association: :provider
       Bullet.add_safelist class_name: "CMS::Page", type: :n_plus_one_query, association: :section
-      Bullet.add_safelist class_name: "Cinstance", type: :n_plus_one_query, association: :plan
-      Bullet.add_safelist class_name: "Cinstance", type: :n_plus_one_query, association: :service
-      Bullet.add_safelist class_name: "Cinstance", type: :n_plus_one_query, association: :user_account
-      Bullet.add_safelist class_name: "Cinstance", type: :unused_eager_loading, association: :plan
-      Bullet.add_safelist class_name: "Cinstance", type: :unused_eager_loading, association: :service
-      Bullet.add_safelist class_name: "Cinstance", type: :unused_eager_loading, association: :service
-      Bullet.add_safelist class_name: "Cinstance", type: :unused_eager_loading, association: :service
-      Bullet.add_safelist class_name: "Cinstance", type: :unused_eager_loading, association: :plan
-      Bullet.add_safelist class_name: "Cinstance", type: :unused_eager_loading, association: :user_account
-      Bullet.add_safelist class_name: "Cinstance", type: :unused_eager_loading, association: :user_account
-      Bullet.add_safelist class_name: "Cinstance", type: :unused_eager_loading, association: :service
       Bullet.add_safelist class_name: "Invoice", type: :counter_cache, association: :payment_transactions
       Bullet.add_safelist class_name: "Invoice", type: :n_plus_one_query, association: :buyer_account
       Bullet.add_safelist class_name: "Invoice", type: :n_plus_one_query, association: :provider_account