Merge pull request #4325 from 3scale/THREESCALE-11354-application-plan-name-sync

THREESCALE-11354: Sync application plan name to backend

Author: jlledom

app/lib/backend/model_extensions/application_plan.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+module Backend
+  module ModelExtensions
+    module ApplicationPlan
+      extend ActiveSupport::Concern
+
+      included do
+        after_commit :sync_backend_application_plan_name, if: :saved_change_to_name?, on: :update
+      end
+
+      private
+
+      def sync_backend_application_plan_name
+        BackendUpdateApplicationPlanJob.perform_async(id)
+      end
+    end
+  end
+end

app/lib/backend/model_extensions/cinstance.rb

@@ -20,20 +20,26 @@ def self.included(base)
 
       def update_backend_application
         if plan && service
-          state = self.state
-          state = :active if live?
-
-          ThreeScale::Core::Application.save( :service_id => service.backend_id,
-                                              :id         => application_id,
-                                              :state      => state,
-                                              :plan_id    => plan.id,
-                                              :plan_name  => plan.name,
-                                              :redirect_url => redirect_url )
+          ThreeScale::Core::Application.save(backend_application_attributes(service, plan))
         end
 
         true
       end
 
+      def backend_application_attributes(service, plan)
+        state = self.state
+        state = :active if live?
+
+        {
+          :service_id   => service.backend_id,
+          :id           => application_id,
+          :state        => state,
+          :plan_id      => plan.id,
+          :plan_name    => plan.name,
+          :redirect_url => redirect_url
+        }
+      end
+
       def delete_backend_application
         if service.present? && service.id.present? && application_id.present?
           ThreeScale::Core::Application.delete(service.backend_id, application_id)

app/models/application_plan.rb

@@ -1,5 +1,6 @@
 
 class ApplicationPlan < Plan
+  include Backend::ModelExtensions::ApplicationPlan
 
   #TODO: is the dependent :destroy working?
   has_many :cinstances, :foreign_key => :plan_id, :dependent => :destroy, :inverse_of => :plan

app/workers/backend_update_application_plan_job.rb

@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+class BackendUpdateApplicationPlanJob
+  include Sidekiq::IterableJob
+  include Sidekiq::Throttled::Job
+
+  sidekiq_options queue: :backend_sync
+
+  sidekiq_throttle concurrency: {
+    limit: 1,
+    key_suffix: ->(plan_id) { "plan:#{plan_id}" },
+    ttl: 1.hour.to_i
+  }
+
+  def build_enumerator(plan_id, cursor:)
+    plan = ApplicationPlan.find_by(id: plan_id)
+    return unless plan
+
+    active_record_batches_enumerator(plan.cinstances, cursor: cursor, batch_size: 10_000)
+  end
+
+  # :reek:UtilityFunction :reek:TooManyStatements
+  def each_iteration(batch, plan_id)
+    plan = ApplicationPlan.find_by(id: plan_id)
+    return unless plan
+
+    save_backend_applications(batch, plan)
+  end
+
+  private
+
+  def save_backend_applications(batch, plan)
+    service = plan.service
+    applications = batch.map { |app| app.backend_application_attributes(service, plan) }
+    ThreeScale::Core::Application.save_batch(service.backend_id, applications)
+  end
+end

test/integration/admin/api/application_plans_controller_test.rb

@@ -3,6 +3,7 @@
 require 'test_helper'
 
 class Admin::Api::ApplicationPlansControllerTest < ActionDispatch::IntegrationTest
+  include ActiveJob::TestHelper
 
   def setup
     Settings::Switch.any_instance.stubs(:allowed?).returns(true)
@@ -77,6 +78,20 @@ def test_index_json
       assert_equal 2, JSON.parse(response.body)['plans'].length
     end
 
+    def test_update_syncs_plan_name_to_backend
+      application_plan = FactoryBot.create(:application_plan, name: 'old name', issuer: service)
+      cinstances = FactoryBot.create_list(:simple_cinstance, 2, plan: application_plan)
+
+      ThreeScale::Core::Application.expects(:save_batch)
+        .with(service.backend_id, expected_backend_applications(cinstances, application_plan, 'new name'))
+
+      Sidekiq::Testing.inline! do
+        put admin_api_service_application_plan_path(application_plan, service_id: service.id, format: :json, access_token: @token,
+                                                    application_plan: { name: 'new name' })
+        assert_response :success
+      end
+    end
+
     def test_approval_required
       assert_difference service.application_plans.method(:count) do
         post admin_api_service_application_plans_path(application_plan_params(approval_required: true))
@@ -160,6 +175,15 @@ def current_account
 
   attr_reader :service
 
+  def expected_backend_applications(cinstances, plan, plan_name)
+    cinstances.map do |cinstance|
+      state = cinstance.state
+      state = :active if cinstance.live?
+      { service_id: plan.service.backend_id, id: cinstance.application_id, state: state,
+        plan_id: plan.id, plan_name: plan_name, redirect_url: cinstance.redirect_url }
+    end
+  end
+
   def application_plan_params(state_event: 'publish', approval_required: 0)
     @application_plan_params ||= {
       service_id: service.id,

test/unit/backend/model_extensions/application_plan_test.rb

@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class Backend::ModelExtensions::ApplicationPlanTest < ActiveSupport::TestCase
+  def setup
+    @plan = FactoryBot.create(:application_plan)
+  end
+
+  attr_reader :plan
+
+  test 'enqueues worker when plan name is updated' do
+    BackendUpdateApplicationPlanJob.jobs.clear
+    plan.update!(name: 'New Plan Name')
+    assert_equal 1, BackendUpdateApplicationPlanJob.jobs.size
+    assert_equal [plan.id], BackendUpdateApplicationPlanJob.jobs.first['args']
+  end
+
+  test 'does not enqueue worker when other attributes are updated' do
+    BackendUpdateApplicationPlanJob.jobs.clear
+    plan.update!(description: 'Updated description')
+    assert_equal 0, BackendUpdateApplicationPlanJob.jobs.size
+  end
+
+  test 'does not enqueue worker when plan is created' do
+    BackendUpdateApplicationPlanJob.jobs.clear
+    FactoryBot.create(:application_plan)
+    assert_equal 0, BackendUpdateApplicationPlanJob.jobs.size
+  end
+end

test/workers/backend_update_application_plan_job_test.rb

@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class BackendUpdateApplicationPlanJobTest < ActiveSupport::TestCase
+  include NPlusOneControl::MinitestHelper
+
+  def setup
+    @plan = FactoryBot.create(:application_plan)
+  end
+
+  attr_reader :plan
+
+  test 'batch syncs plan name to backend for all cinstances' do
+    cinstances = FactoryBot.create_list(:simple_cinstance, 2, plan: plan)
+
+    expected_applications = cinstances.map do |cinstance|
+      state = cinstance.state
+      state = :active if cinstance.live?
+
+      {
+        service_id: plan.service.backend_id,
+        id: cinstance.application_id,
+        state: state,
+        plan_id: plan.id,
+        plan_name: plan.name,
+        redirect_url: cinstance.redirect_url
+      }
+    end
+
+    ThreeScale::Core::Application.expects(:save_batch).with(plan.service.backend_id, expected_applications)
+
+    BackendUpdateApplicationPlanJob.new.perform(plan.id)
+  end
+
+  test 'no n+1 queries' do
+    ThreeScale::Core::Application.stubs(:save_batch)
+
+    populate = ->(count) { FactoryBot.create_list(:simple_cinstance, count, plan: plan) }
+
+    assert_perform_constant_number_of_queries(populate: populate) do
+      BackendUpdateApplicationPlanJob.new.perform(plan.id)
+    end
+  end
+
+  test 'does nothing when plan does not exist' do
+    ThreeScale::Core::Application.expects(:save_batch).never
+
+    BackendUpdateApplicationPlanJob.new.perform(0)
+  end
+
+  test 'does nothing when plan has no cinstances' do
+    ThreeScale::Core::Application.expects(:save_batch).never
+
+    BackendUpdateApplicationPlanJob.new.perform(plan.id)
+  end
+end