From a4f15c6f298119ffe6812feaa5e9d2975b1f02fd Mon Sep 17 00:00:00 2001
From: Denis Talakevich <senid231@gmail.com>
Date: Fri, 12 Jul 2024 18:01:32 +0300
Subject: [PATCH] provisioning object validation

---
 app/models/billing/provisioning/base.rb       |  29 +++
 app/models/billing/provisioning/errors.rb     |  49 +++++
 .../billing/provisioning/free_minutes.rb      |  55 +++++-
 app/models/billing/provisioning/logging.rb    |  20 ++
 app/models/billing/service.rb                 |  29 ++-
 app/models/billing/service_type.rb            |  17 +-
 config/initializers/dry_schema.rb             |  11 ++
 spec/models/billing/service_spec.rb           | 173 ++++++++++++++++++
 spec/models/billing/service_type_spec.rb      | 128 ++++++++++++-
 9 files changed, 499 insertions(+), 12 deletions(-)
 create mode 100644 app/models/billing/provisioning/errors.rb
 create mode 100644 config/initializers/dry_schema.rb
 create mode 100644 spec/models/billing/service_spec.rb

diff --git a/app/models/billing/provisioning/base.rb b/app/models/billing/provisioning/base.rb
index e6c8ab734..f410e55b8 100644
--- a/app/models/billing/provisioning/base.rb
+++ b/app/models/billing/provisioning/base.rb
@@ -3,6 +3,15 @@
 module Billing
   module Provisioning
     class Base
+      class << self
+        # @param service_type [Billing::ServiceType]
+        # @raise [Billing::Provisioning::Errors::InvalidVariablesError]
+        # @return [Hash,nil] verified service_type variables
+        def verify_service_type_variables!(service_type)
+          service_type.variables
+        end
+      end
+
       attr_reader :service
       delegate :account, to: :service
 
@@ -11,6 +20,12 @@ def initialize(service)
         @service = service
       end
 
+      # @raise [Billing::Provisioning::Errors::InvalidVariablesError]
+      # @return [Hash,nil] verified service variables
+      def verify_service_variables!
+        service.variables
+      end
+
       def after_create
         nil
       end
@@ -34,6 +49,20 @@ def after_success_renew
       def after_failed_renew
         nil
       end
+
+      # Called before destroying the service and before destroying dependant package_counters
+      # To prevent service destruction:
+      #   service.errors.add(:base, ...)
+      #   throw(:abort)
+      #
+      def before_destroy
+        nil
+      end
+
+      # Called after destroying the service but before transaction COMMIT
+      def after_destroy
+        nil
+      end
     end
   end
 end
diff --git a/app/models/billing/provisioning/errors.rb b/app/models/billing/provisioning/errors.rb
new file mode 100644
index 000000000..6a63f15a5
--- /dev/null
+++ b/app/models/billing/provisioning/errors.rb
@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+module Billing
+  module Provisioning
+    module Errors
+      class Error < StandardError
+      end
+
+      class InvalidVariablesError < Error
+        attr_reader :errors
+
+        # @param errors [Hash,String]
+        # @param msg [String,nil]
+        def initialize(errors, msg = nil)
+          if errors.is_a?(String)
+            msg = errors
+            errors = { base: [msg] }
+          end
+          @errors = errors
+          super(msg || "Validation error: #{full_error_messages.join(', ')}")
+        end
+
+        def full_error_messages(errors = @errors, prefix = nil)
+          full_messages = []
+          errors.each do |key, values|
+            if values.is_a?(Array)
+              full_messages.concat parse_errors_array(key, values, prefix)
+            elsif values.is_a?(Hash)
+              full_messages.concat full_error_messages(values, [prefix, key].compact.join('.'))
+            else
+              raise ArgumentError, "Invalid error format: #{values.inspect}\nerrors: #{errors.inspect}"
+            end
+          end
+          full_messages
+        end
+
+        def parse_errors_array(key, values, prefix)
+          values.map do |value|
+            if prefix.nil?
+              key == :base || key.nil? ? value : "#{prefix}.#{key} - #{value}"
+            else
+              key == :base || key.nil? ? "#{prefix} - #{value}" : "#{prefix}.#{key} - #{value}"
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/app/models/billing/provisioning/free_minutes.rb b/app/models/billing/provisioning/free_minutes.rb
index 9c6f63067..f85097c44 100644
--- a/app/models/billing/provisioning/free_minutes.rb
+++ b/app/models/billing/provisioning/free_minutes.rb
@@ -29,23 +29,60 @@ module Provisioning
     # "ignore_prefixes" is optional and defaults to []
     #
     class FreeMinutes < Base
-      def after_create
-        raise ArgumentError, 'No prefixes configured' if prefixes.empty?
+      PrefixSchema = Dry::Schema.JSON do
+        required(:prefix).filled(:string)
+        required(:duration).value(:integer)
+        optional(:exclude).maybe(:bool)
+      end
+
+      class ServiceTypeVariablesContract < Dry::Validation::Contract
+        json do
+          optional(:prefixes).array(PrefixSchema).default([])
+        end
+      end
 
+      class ServiceVariablesContract < Dry::Validation::Contract
+        json do
+          optional(:prefixes).array(PrefixSchema).default([])
+          optional(:ignore_prefixes).array(:string).default([])
+        end
+      end
+
+      class << self
+        def verify_service_type_variables!(service_type)
+          contract = ServiceTypeVariablesContract.new
+          result = contract.call(service_type.variables || {})
+          raise Billing::Provisioning::Errors::InvalidVariablesError, result.errors.to_h unless result.success?
+
+          result.to_h
+        end
+      end
+
+      def verify_service_variables!
+        contract = ServiceVariablesContract.new
+        result = contract.call(service.variables || {})
+        raise Billing::Provisioning::Errors::InvalidVariablesError, result.errors.to_h unless result.success?
+
+        result.to_h
+      end
+
+      def after_create
         prefixes_data.each do |data|
           create_or_reset_counter(data)
         end
       end
 
       def after_success_renew
-        raise ArgumentError, 'No prefixes configured' if prefixes.empty?
-
         prefixes_data.each do |data|
           create_or_reset_counter(data)
         end
         destroy_obsolete_counters
       end
 
+      def before_destroy
+        nil
+      end
+
       private
 
       # if counter for prefix does not exist - will created it.
@@ -73,17 +110,19 @@ def prefixes_data
 
       def build_prefixes_data
         # service may override some service.type prefixes
-        ignore_type_prefixes = service.variables['prefixes'].map { |data| data['prefix'] }
+        service_prefixes = service.variables['prefixes']
+        service_type_prefixes = service.type.variables['prefixes']
+        ignore_type_prefixes = service_prefixes.map { |data| data['prefix'] }
         # service may ignore some service.type prefixes
-        ignore_type_prefixes += service.variables['ignore_prefixes'] || []
+        ignore_type_prefixes += service.variables['ignore_prefixes']
 
         data_list = []
-        service.type.variables['prefixes'].each do |data|
+        service_type_prefixes.each do |data|
           next if ignore_type_prefixes.include? data['prefix']
 
           data_list << data
         end
-        service.variables['prefixes'].each do |data|
+        service_prefixes.each do |data|
           data_list << data
         end
         data_list.map do |data|
diff --git a/app/models/billing/provisioning/logging.rb b/app/models/billing/provisioning/logging.rb
index 69593fa83..19598eff4 100644
--- a/app/models/billing/provisioning/logging.rb
+++ b/app/models/billing/provisioning/logging.rb
@@ -3,6 +3,18 @@
 module Billing
   module Provisioning
     class Logging < Base
+      class << self
+        def verify_service_type_variables!(service_type)
+          Rails.logger.info "Verify service type variables variables=#{service_type.variables}"
+          super
+        end
+      end
+
+      def verify_service_variables!
+        Rails.logger.info "Verify service variables variables=#{service.variables}"
+        super
+      end
+
       def after_create
         Rails.logger.info "Service created service_id=#{service.id}"
       end
@@ -22,6 +34,14 @@ def after_success_renew
       def after_failed_renew
         Rails.logger.info "Service failed to renew service_id=#{service.id}, account_balance=#{account.balance}"
       end
+
+      def before_destroy
+        Rails.logger.info "Before destroy service service_id=#{service.id}"
+      end
+
+      def after_destroy
+        Rails.logger.info "After destroy service service_id=#{service.id}"
+      end
     end
   end
 end
diff --git a/app/models/billing/service.rb b/app/models/billing/service.rb
index 1a018364d..51075ad04 100644
--- a/app/models/billing/service.rb
+++ b/app/models/billing/service.rb
@@ -53,19 +53,29 @@ class Billing::Service < ApplicationRecord
   belongs_to :type, class_name: 'Billing::ServiceType'
   belongs_to :account, class_name: 'Account'
   has_many :transactions, class_name: 'Billing::Transaction'
+
+  # callback defined before association because it should be called before it's `dependent: :destroy` callback
+  before_destroy :provisioning_object_before_destroy
   has_many :package_counters, class_name: 'Billing::PackageCounter', dependent: :destroy
 
   attr_readonly :account_id, :type_id
 
+  before_validation { self.variables = nil if variables.blank? }
+
   validates :initial_price, :renew_price, presence: true
   validates :initial_price, :renew_price, numericality: true, allow_nil: true
   validates :state_id, inclusion: { in: STATES.keys }
   validates :renew_period_id, inclusion: { in: RENEW_PERIODS.keys }, allow_nil: true
   validate :validate_variables
 
+  before_create :verify_provisioning_variables
+  before_update :verify_provisioning_variables, if: :variables_changed?
+
   after_create :create_initial_transaction
   after_create :provisioning_object_after_create
 
+  after_destroy :provisioning_object_after_destroy
+
   scope :ready_for_renew, lambda {
     where('renew_period_id is not null AND renew_at <= ? ', Time.current)
   }
@@ -110,8 +120,25 @@ def provisioning_object_after_create
     build_provisioning_object.after_create
   end
 
+  def provisioning_object_before_destroy
+    build_provisioning_object.before_destroy
+  end
+
+  def provisioning_object_after_destroy
+    build_provisioning_object.after_destroy
+  end
+
   def validate_variables
-    errors.add(:variables, 'must be a JSON object or empty') if !variables.nil? && !variables.is_a?(Hash)
+    if !variables.nil? && !variables.is_a?(Hash)
+      errors.add(:variables, 'must be a JSON object or empty')
+    end
+  end
+
+  def verify_provisioning_variables
+    self.variables = build_provisioning_object.verify_service_variables!
+  rescue Billing::Provisioning::Errors::InvalidVariablesError => e
+    e.full_error_messages.each { |msg| errors.add(:variables, msg) }
+    throw(:abort)
   end
 
   def create_initial_transaction
diff --git a/app/models/billing/service_type.rb b/app/models/billing/service_type.rb
index 1becf50db..d2ec2fc0c 100644
--- a/app/models/billing/service_type.rb
+++ b/app/models/billing/service_type.rb
@@ -21,6 +21,8 @@ class Billing::ServiceType < ApplicationRecord
 
   has_many :services, class_name: 'Billing::Service', foreign_key: :type_id, dependent: :restrict_with_error
 
+  before_validation { self.variables = nil if variables.blank? }
+
   validates :name, presence: true
   validates :name, uniqueness: true, allow_blank: true
   validates :provisioning_class, presence: true
@@ -28,6 +30,9 @@ class Billing::ServiceType < ApplicationRecord
   validates :force_renew, inclusion: { in: [true, false] }
   validate :validate_variables
 
+  before_create :verify_provisioning_variables
+  before_update :verify_provisioning_variables, if: proc { variables_changed? || provisioning_class_changed? }
+
   def display_name
     name
   end
@@ -64,6 +69,16 @@ def validate_provisioning_class
   end
 
   def validate_variables
-    errors.add(:variables, 'must be a JSON object or empty') if !variables.nil? && !variables.is_a?(Hash)
+    if !variables.nil? && !variables.is_a?(Hash)
+      errors.add(:variables, 'must be a JSON object or empty')
+    end
+  end
+
+  def verify_provisioning_variables
+    klass = provisioning_class.constantize
+    self.variables = klass.verify_service_type_variables!(self)
+  rescue Billing::Provisioning::Errors::InvalidVariablesError => e
+    e.full_error_messages.each { |msg| errors.add(:variables, msg) }
+    throw(:abort)
   end
 end
diff --git a/config/initializers/dry_schema.rb b/config/initializers/dry_schema.rb
new file mode 100644
index 000000000..b47b407ff
--- /dev/null
+++ b/config/initializers/dry_schema.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+require 'dry/schema'
+
+class Dry::Schema::Macros::DSL
+  def default(value)
+    schema_dsl.before(:rule_applier) do |result|
+      result.update(name => value) unless result[name]
+    end
+  end
+end
diff --git a/spec/models/billing/service_spec.rb b/spec/models/billing/service_spec.rb
new file mode 100644
index 000000000..97f954a73
--- /dev/null
+++ b/spec/models/billing/service_spec.rb
@@ -0,0 +1,173 @@
+# frozen_string_literal: true
+
+# == Schema Information
+#
+# Table name: billing.services
+#
+#  id              :bigint(8)        not null, primary key
+#  initial_price   :decimal(, )      not null
+#  name            :string
+#  renew_at        :timestamptz
+#  renew_price     :decimal(, )      not null
+#  uuid            :uuid             not null
+#  variables       :jsonb
+#  created_at      :timestamptz      not null
+#  account_id      :integer(4)       not null
+#  renew_period_id :integer(2)
+#  state_id        :integer(2)       default(10), not null
+#  type_id         :integer(2)       not null
+#
+# Indexes
+#
+#  services_account_id_idx  (account_id)
+#  services_renew_at_idx    (renew_at)
+#  services_type_id_idx     (type_id)
+#  services_uuid_idx        (uuid)
+#
+# Foreign Keys
+#
+#  services_account_id_fkey  (account_id => accounts.id)
+#  services_type_id_fkey     (type_id => service_types.id)
+#
+RSpec.describe Billing::Service do
+  describe '.create' do
+    subject do
+      described_class.create(create_params)
+    end
+
+    let!(:account) { create(:account) }
+    let!(:service_type) { create(:service_type, service_type_attrs) }
+    let(:service_type_attrs) do
+      { provisioning_class: 'Billing::Provisioning::Logging' }
+    end
+    let(:create_params) do
+      {
+        name: 'test',
+        account:,
+        type: service_type,
+        initial_price: 10,
+        renew_price: 20
+      }
+    end
+    let(:default_attributes) do
+      {
+        renew_at: nil,
+        renew_period_id: nil,
+        state_id: described_class::STATE_ID_ACTIVE
+      }
+    end
+    let(:expected_attrs) do
+      default_attributes.merge(create_params)
+    end
+
+    include_examples :creates_record do
+      let(:expected_record_attrs) { expected_attrs }
+    end
+
+    context 'with variables' do
+      let(:create_params) do
+        super().merge(variables: { 'foo' => 'bar' })
+      end
+
+      include_examples :creates_record do
+        let(:expected_record_attrs) { expected_attrs }
+      end
+    end
+
+    context 'with provisioning_class=Billing::Provisioning::FreeMinutes' do
+      let(:service_type_attrs) do
+        super().merge(provisioning_class: 'Billing::Provisioning::FreeMinutes')
+      end
+      let(:expected_attrs) do
+        super().merge(
+          variables: { 'prefixes' => [], 'ignore_prefixes' => [] }
+        )
+      end
+
+      include_examples :creates_record do
+        let(:expected_record_attrs) { expected_attrs }
+      end
+
+      context 'with valid prefixes' do
+        let(:create_params) do
+          super().merge(
+            variables: {
+              'prefixes' => [
+                { 'prefix' => '123', 'duration' => 60, 'exclude' => false },
+                { 'prefix' => '124', 'duration' => 30, 'exclude' => true }
+              ],
+              'ignore_prefixes' => []
+            }
+          )
+        end
+        let(:expected_attrs) do
+          default_attributes.merge(create_params)
+        end
+
+        include_examples :creates_record do
+          let(:expected_record_attrs) { expected_attrs }
+        end
+      end
+
+      context 'with invalid prefixes' do
+        let(:create_params) do
+          super().merge(
+            variables: {
+              'prefixes' => [
+                { 'prefix' => '', 'duration' => 60 },
+                { 'prefix' => nil, 'duration' => 60 },
+                { 'prefix' => 112, 'duration' => 60 },
+                { 'duration' => 60 },
+                { 'prefix' => '114', 'duration' => '60' },
+                { 'prefix' => '115', 'duration' => 'test' },
+                { 'prefix' => '116', 'duration' => nil },
+                { 'prefix' => '117' },
+                { 'prefix' => '118', 'duration' => 60, 'exclude' => 'test' },
+                { 'prefix' => '119', 'duration' => 60, 'exclude' => 123 },
+                {}
+              ],
+              'ignore_prefixes' => [
+                123,
+                nil
+              ]
+            }
+          )
+        end
+
+        include_examples :does_not_create_record, errors: {
+          variables: [
+            'prefixes.0.prefix - must be filled',
+            'prefixes.1.prefix - must be a string',
+            'prefixes.2.prefix - must be a string',
+            'prefixes.3.prefix - is missing',
+            'prefixes.4.duration - must be an integer',
+            'prefixes.5.duration - must be an integer',
+            'prefixes.6.duration - must be an integer',
+            'prefixes.7.duration - is missing',
+            'prefixes.8.exclude - must be boolean',
+            'prefixes.9.exclude - must be boolean',
+            'prefixes.10.prefix - is missing',
+            'prefixes.10.duration - is missing',
+            'ignore_prefixes.0 - must be a string',
+            'ignore_prefixes.1 - must be a string'
+          ]
+        }
+      end
+
+      context 'with extra variables' do
+        let(:create_params) do
+          super().merge(variables: { 'foo' => 'bar' })
+        end
+        let(:expected_attrs) do
+          super().merge(
+            variables: { 'prefixes' => [], 'ignore_prefixes' => [] }
+          )
+        end
+
+        include_examples :creates_record do
+          let(:expected_record_attrs) { expected_attrs }
+        end
+      end
+    end
+  end
+end
diff --git a/spec/models/billing/service_type_spec.rb b/spec/models/billing/service_type_spec.rb
index abd67fde9..779824c82 100644
--- a/spec/models/billing/service_type_spec.rb
+++ b/spec/models/billing/service_type_spec.rb
@@ -22,11 +22,135 @@
     it { is_expected.not_to allow_value('Billing::Provisioning::Base').for(:provisioning_class) }
     it { is_expected.not_to allow_value('Billing::Service').for(:provisioning_class) }
     it { is_expected.not_to allow_value('NotExistingConst').for(:provisioning_class) }
-    it { is_expected.to allow_value(nil, { foo: 'bar' }).for(:variables) }
-    it { is_expected.not_to allow_value('').for(:variables) }
+    it { is_expected.to allow_values('', nil, {}, { foo: 'bar' }).for(:variables) }
     it { is_expected.not_to allow_value('test').for(:variables) }
     it { is_expected.not_to allow_value([{ foo: 'bar' }]).for(:variables) }
     it { is_expected.not_to allow_value(123).for(:variables) }
     it { is_expected.not_to allow_value(true).for(:variables) }
   end
+
+  describe '.create' do
+    subject do
+      described_class.create(create_params)
+    end
+
+    let(:create_params) do
+      {
+        name: 'test',
+        provisioning_class: 'Billing::Provisioning::Logging'
+      }
+    end
+    let(:default_attributes) do
+      {
+        variables: nil,
+        force_renew: false
+      }
+    end
+    let(:expected_attrs) do
+      default_attributes.merge(create_params)
+    end
+
+    include_examples :creates_record do
+      let(:expected_record_attrs) { expected_attrs }
+    end
+
+    context 'with variables' do
+      let(:create_params) do
+        super().merge(variables: { 'foo' => 'bar' })
+      end
+
+      include_examples :creates_record do
+        let(:expected_record_attrs) { expected_attrs }
+      end
+    end
+
+    context 'with provisioning_class=Billing::Provisioning::FreeMinutes' do
+      let(:create_params) do
+        super().merge(provisioning_class: 'Billing::Provisioning::FreeMinutes')
+      end
+      let(:expected_attrs) do
+        super().merge(
+          variables: { 'prefixes' => [] }
+        )
+      end
+
+      include_examples :creates_record do
+        let(:expected_record_attrs) { expected_attrs }
+      end
+
+      context 'with valid prefixes' do
+        let(:create_params) do
+          super().merge(
+            variables: {
+              'prefixes' => [
+                { 'prefix' => '123', 'duration' => 60 },
+                { 'prefix' => '124', 'duration' => 30, 'exclude' => true }
+              ]
+            }
+          )
+        end
+        let(:expected_attrs) do
+          default_attributes.merge(create_params)
+        end
+
+        include_examples :creates_record do
+          let(:expected_record_attrs) { expected_attrs }
+        end
+      end
+
+      context 'with invalid prefixes' do
+        let(:create_params) do
+          super().merge(
+            variables: {
+              'prefixes' => [
+                { 'prefix' => '', 'duration' => 60 },
+                { 'prefix' => nil, 'duration' => 60 },
+                { 'prefix' => 112, 'duration' => 60 },
+                { 'duration' => 60 },
+                { 'prefix' => '114', 'duration' => '60' },
+                { 'prefix' => '115', 'duration' => 'test' },
+                { 'prefix' => '116', 'duration' => nil },
+                { 'prefix' => '117' },
+                { 'prefix' => '118', 'duration' => 60, 'exclude' => 'test' },
+                { 'prefix' => '119', 'duration' => 60, 'exclude' => 123 },
+                {}
+              ]
+            }
+          )
+        end
+
+        include_examples :does_not_create_record, errors: {
+          variables: [
+            'prefixes.0.prefix - must be filled',
+            'prefixes.1.prefix - must be a string',
+            'prefixes.2.prefix - must be a string',
+            'prefixes.3.prefix - is missing',
+            'prefixes.4.duration - must be an integer',
+            'prefixes.5.duration - must be an integer',
+            'prefixes.6.duration - must be an integer',
+            'prefixes.7.duration - is missing',
+            'prefixes.8.exclude - must be boolean',
+            'prefixes.9.exclude - must be boolean',
+            'prefixes.10.prefix - is missing',
+            'prefixes.10.duration - is missing'
+          ]
+        }
+      end
+
+      context 'with extra variables' do
+        let(:create_params) do
+          super().merge(variables: { 'foo' => 'bar' })
+        end
+        let(:expected_attrs) do
+          super().merge(
+            variables: { 'prefixes' => [] }
+          )
+        end
+
+        include_examples :creates_record do
+          let(:expected_record_attrs) { expected_attrs }
+        end
+      end
+    end
+  end
 end