From 6e9281dca2bb00fcb7d3cf233b8e959b9667350c Mon Sep 17 00:00:00 2001 From: fumimowdan Date: Mon, 30 Oct 2023 15:01:00 +0000 Subject: [PATCH 1/3] Add helper queries class for application urn FindApplicationIndexQuery will be used to retrieve the index of an created application and to later match it with a random urn --- app/queries/applications_index_query.rb | 33 +++++++++++++++++ app/queries/find_application_index_query.rb | 37 +++++++++++++++++++ spec/queries/applications_index_query_spec.rb | 12 ++++++ .../find_application_index_query_spec.rb | 14 +++++++ 4 files changed, 96 insertions(+) create mode 100644 app/queries/applications_index_query.rb create mode 100644 app/queries/find_application_index_query.rb create mode 100644 spec/queries/applications_index_query_spec.rb create mode 100644 spec/queries/find_application_index_query_spec.rb diff --git a/app/queries/applications_index_query.rb b/app/queries/applications_index_query.rb new file mode 100644 index 00000000..bb2638ff --- /dev/null +++ b/app/queries/applications_index_query.rb @@ -0,0 +1,33 @@ +class ApplicationsIndexQuery + delegate :to_sql, to: :query + + def execute + ApplicationRecord.connection.execute(to_sql) + end + + def query + @query ||= applications + .project(projection) + .order(applications[:created_at].asc) + end + + def projection + [ + applications[:id], + row_number.as("application_index"), + ] + end + + def row_number + Arel::Nodes::Over.new( + Arel::Nodes::NamedFunction.new("ROW_NUMBER", []), + Arel.sql("(ORDER BY created_at ASC)"), + ) + end + +private + + def applications + Application.arel_table + end +end diff --git a/app/queries/find_application_index_query.rb b/app/queries/find_application_index_query.rb new file mode 100644 index 00000000..551c6f16 --- /dev/null +++ b/app/queries/find_application_index_query.rb @@ -0,0 +1,37 @@ +class FindApplicationIndexQuery + delegate :to_sql, to: :query + attr_reader :application_id + + def initialize(application_id:) + @application_id = application_id + end + + def execute + ApplicationRecord.connection.execute(to_sql) + end + + def query + @query ||= manager + .project(projection) + .from(from_clause) + .where(where_clause) + end + + def projection + [Arel.star] + end + + def from_clause + ApplicationsIndexQuery.new.query.as("list") + end + + def where_clause + Arel.sql("list.id").eq(application_id) + end + +private + + def manager + @manager ||= Arel::SelectManager.new(Arel::Table.engine) + end +end diff --git a/spec/queries/applications_index_query_spec.rb b/spec/queries/applications_index_query_spec.rb new file mode 100644 index 00000000..dda743be --- /dev/null +++ b/spec/queries/applications_index_query_spec.rb @@ -0,0 +1,12 @@ +require "rails_helper" + +RSpec.describe ApplicationsIndexQuery, type: :model do + subject(:sql) { described_class.new.to_sql } + + describe "#to_sql" do + it { expect(sql).to include("SELECT") } + it { expect(sql).to include('"applications"."id"') } + it { expect(sql).to include("ROW_NUMBER() OVER (ORDER BY created_at ASC) AS application_index") } + it { expect(sql).to include('FROM "applications"') } + end +end diff --git a/spec/queries/find_application_index_query_spec.rb b/spec/queries/find_application_index_query_spec.rb new file mode 100644 index 00000000..7de40a1c --- /dev/null +++ b/spec/queries/find_application_index_query_spec.rb @@ -0,0 +1,14 @@ +require "rails_helper" + +RSpec.describe FindApplicationIndexQuery, type: :model do + subject(:sql) { described_class.new(application_id: application.id).to_sql } + + let(:application) { create(:application) } + let(:subquery) { ApplicationsIndexQuery.new.to_sql } + + describe "#to_sql" do + it { expect(sql).to include("SELECT *") } + it { expect(sql).to include("FROM (#{subquery}) list") } + it { expect(sql).to include("WHERE list.id = #{application.id}") } + end +end From 1d0601096d8bc18671b2adc0371b7dc55c5859d9 Mon Sep 17 00:00:00 2001 From: fumimowdan Date: Mon, 30 Oct 2023 15:03:22 +0000 Subject: [PATCH 2/3] Add Application::Urn This service will retreive the associated urn for an application that exists in the database. When the availabe urn set is exhausted it will increase its size automatically. --- app/models/application/urn.rb | 81 +++++++++++++++++++++++++++++ spec/models/application/urn_spec.rb | 35 +++++++++++++ 2 files changed, 116 insertions(+) create mode 100644 app/models/application/urn.rb create mode 100644 spec/models/application/urn_spec.rb diff --git a/app/models/application/urn.rb b/app/models/application/urn.rb new file mode 100644 index 00000000..3bf3ae4b --- /dev/null +++ b/app/models/application/urn.rb @@ -0,0 +1,81 @@ +# +# build a pseudo random urns list with the existing urns removed from that list +# and we calculate the global index of the current application based on list sorted by created_at +# then we get the urn located at the determined application index +# + +class Application::Urn + class << self + def reset_urns + const_set(:URNS, build_urns) + nil + end + + def build_urns + { + "teacher" => build_list("TE"), + "salaried_trainee" => build_list("ST"), + }.freeze + end + + def build_list(code) + su_size = LENGTH.to_s.size + Array + .new(LENGTH) { [PREFIX, code, sprintf("%0##{su_size}d", _1)].join } + .drop(1) + .shuffle! + end + end + + LENGTH = 99_999 + PREFIX = "IRP".freeze + URNS = build_urns + + def initialize(application) + @application = application + end + + def urn + MUTEX.synchronize do + increase_suffix if urns_exhausted? + available_urns[application_index] + end + end + +private + + def increase_suffix + self.class.const_set(:LENGTH, "#{self.class::LENGTH}9".to_i) + self.class.reset_urns + end + + def urns_exhausted? + application_index > available_urns.size + end + + def application_index + return @application_index if @application_index + + @application_index = FindApplicationIndexQuery + .new(application_id: @application.id) + .execute + .to_a + .dig(0, "application_index") + + raise(ArgumentError, "application not found") unless @application_index + + @application_index -= 1 # FindApplicationIndexQuery returns a index starting at 1 + @application_index -= used_urns.size # removes the existing applications to have a correct index + @application_index + end + + def available_urns + @available_urns ||= URNS.fetch(@application.application_route) - used_urns + end + + def used_urns + @used_urns ||= Application.where(application_route: @application.application_route).pluck(:urn) + end + + MUTEX = Mutex.new +end diff --git a/spec/models/application/urn_spec.rb b/spec/models/application/urn_spec.rb new file mode 100644 index 00000000..6f2ef92f --- /dev/null +++ b/spec/models/application/urn_spec.rb @@ -0,0 +1,35 @@ +require "rails_helper" + +RSpec.describe Application::Urn do + describe "constants" do + it { expect(described_class::LENGTH).to eq(99_999) } + it { expect(described_class::PREFIX).to eq("IRP") } + it { expect(described_class::URNS.keys).to contain_exactly("teacher", "salaried_trainee") } + it { expect(described_class::URNS.dig("teacher", 1)).to include("IRPTE") } + it { expect(described_class::URNS.fetch("teacher").size).to eq(99_998) } + it { expect(described_class::URNS.dig("salaried_trainee", 1)).to include("IRPST") } + it { expect(described_class::URNS.fetch("salaried_trainee").size).to eq(99_998) } + end + + describe "urn" do + subject(:service) { described_class.new(application) } + + context "teacher" do + let(:application) { create(:teacher_application) } + + it { expect(service.urn).to include("IRPTE") } + end + + context "salaried_trainee" do + let(:application) { create(:salaried_trainee_application) } + + it { expect(service.urn).to include("IRPST") } + end + + context "missing application" do + let(:application) { build(:salaried_trainee_application) } + + it { expect { service.urn }.to raise_error(ArgumentError, "application not found") } + end + end +end From 8509f36cc47c82a7753704006780af290621cbf0 Mon Sep 17 00:00:00 2001 From: fumimowdan Date: Mon, 30 Oct 2023 15:05:22 +0000 Subject: [PATCH 3/3] Swap Urn class for Application::Urn This is to avoid the FiberError caused by a multithreaded app server --- app/models/application.rb | 10 ++- app/models/urn.rb | 115 ------------------------------ app/services/submit_form.rb | 1 - config/initializers/urn.rb | 7 -- spec/factories/applications.rb | 1 - spec/models/application_spec.rb | 9 ++- spec/models/urn_spec.rb | 39 ---------- spec/services/submit_form_spec.rb | 8 +-- 8 files changed, 17 insertions(+), 173 deletions(-) delete mode 100644 app/models/urn.rb delete mode 100644 config/initializers/urn.rb delete mode 100644 spec/models/urn_spec.rb diff --git a/app/models/application.rb b/app/models/application.rb index 05be63be..c2b91312 100644 --- a/app/models/application.rb +++ b/app/models/application.rb @@ -60,6 +60,8 @@ def mark_as_qa! unique_by: %i[application_id status]) end + after_create :set_urn + validates(:application_date, presence: true) validates(:application_route, presence: true) validates(:date_of_entry, presence: true) @@ -67,5 +69,11 @@ def mark_as_qa! validates(:subject, presence: true) validates(:visa_type, presence: true) validates(:applicant, presence: true) - validates(:urn, uniqueness: true) + validates(:urn, uniqueness: true, allow_blank: true) + +private + + def set_urn + update(urn: Urn.new(self).urn) if urn.blank? + end end diff --git a/app/models/urn.rb b/app/models/urn.rb deleted file mode 100644 index 9a2a31ad..00000000 --- a/app/models/urn.rb +++ /dev/null @@ -1,115 +0,0 @@ -# frozen_string_literal: true - -# Urn represents a pseudo random Uniform Resource Name (URN) generator. -# Invoking the method `next` returns a unique URN with a fixed prefix -# and a random alphanumeric suffix. -# -# Urn.configure do |c| -# c.max_suffix = 11 -# c.seeds = { teacher: ENV['TEACHER_URN_SEED'] } -# c.urns = ->(route) { Application.where(application_route: route).pluck(:urn) } -# end -# -# Example: -# -# Urn.next('teacher') # => "IRPTE12345" -# Urn.next('teacher') # => "IRPTE12345" -# Urn.next('salaried_trainee') # => "IRPST12345" -# -class Urn - class NoUrnAvailableError < StandardError; end - - class Config - def initialize - @default_prefix = "IRP" - @default_max_suffix = 99_999 - @default_codes = { - teacher: "TE", - salaried_trainee: "ST", - }.with_indifferent_access - @default_urns = ->(_) { [] } - end - - attr_writer :prefix, :codes, :max_suffix, :seeds, :urns, :padding_size - - def prefix - @prefix || @default_prefix - end - - def codes - (@codes || @default_codes).with_indifferent_access - end - - def max_suffix - @max_suffix || @default_max_suffix - end - - def padding_size - @padding_size || max_suffix.to_s.size - end - - def seeds - (@seeds || {}).with_indifferent_access - end - - def urns - @urns || @default_urns - end - end - - class << self - def configure - yield(config) - end - - def config - return @config if @config.present? - - @config = Config.new - end - - def next(route) - routes[route].next - rescue KeyError - raise(ArgumentError, "Invalid route: #{route}, must be one of #{config.codes.keys}") - end - - private - - def routes - @routes ||= Concurrent::Hash.new do |hash, route| - hash[route] = urn_enumerator( - config.codes.fetch(route), - config.seeds.fetch(route, Random.new_seed), - config.urns.call(route), - ) - end - end - - def urns(code, seed) - Array - .new(config.max_suffix) { formatter(code, _1) } - .drop(1) - .shuffle!(random: Random.new(seed)) - end - - def formatter(code, suffix) - [config.prefix, code, sprintf("%0#{config.padding_size}d", suffix)].join - end - - def available_urns(code, seed, used_urns) - urns(code, seed) - used_urns - end - - def urn_enumerator(code, seed, used_urns) - list = Concurrent::Array.new(available_urns(code, seed, used_urns)) - error_msg = "you have exhausted urn for code #{code} you need to increase the size of the suffix" - - Enumerator.new do |yielder| - list.each { yielder << _1 } - - raise(NoUrnAvailableError, error_msg) - end - end - end -end diff --git a/app/services/submit_form.rb b/app/services/submit_form.rb index a6ccb4d5..3b3eb902 100644 --- a/app/services/submit_form.rb +++ b/app/services/submit_form.rb @@ -88,7 +88,6 @@ def create_application date_of_entry: form.date_of_entry, start_date: form.start_date, subject: SubjectStep.new(form).answer.formatted_value, - urn: Urn.next(form.application_route), visa_type: form.visa_type, ) end diff --git a/config/initializers/urn.rb b/config/initializers/urn.rb deleted file mode 100644 index f9658c35..00000000 --- a/config/initializers/urn.rb +++ /dev/null @@ -1,7 +0,0 @@ -require Rails.root.join("app/models/urn") - -Urn.configure do |c| - c.prefix = "IRP" - c.max_suffix = 99_999 - c.urns = ->(route) { Application.where(application_route: route).pluck(:urn) } -end diff --git a/spec/factories/applications.rb b/spec/factories/applications.rb index 712bf90b..170bc160 100644 --- a/spec/factories/applications.rb +++ b/spec/factories/applications.rb @@ -31,7 +31,6 @@ visa_type { VisaStep::VALID_ANSWERS_OPTIONS.reject { _1 == "Other" }.sample } date_of_entry { Time.zone.today } start_date { 1.month.from_now.to_date } - urn { Urn.next(application_route) } factory :teacher_application do application_route { "teacher" } diff --git a/spec/models/application_spec.rb b/spec/models/application_spec.rb index 5ed916b0..6eb28d61 100644 --- a/spec/models/application_spec.rb +++ b/spec/models/application_spec.rb @@ -67,11 +67,16 @@ end describe "#urn" do - it "is blank before creation" do - application = described_class.new + let(:application) { build(:application) } + it "is blank before creation" do expect(application.urn).to be_blank end + + it "is set after creation" do + application.save + expect(application.urn).not_to be_blank + end end describe ".search" do diff --git a/spec/models/urn_spec.rb b/spec/models/urn_spec.rb deleted file mode 100644 index 366389b0..00000000 --- a/spec/models/urn_spec.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true - -require "rails_helper" - -RSpec.describe Urn do - subject(:urn) { described_class.next(applicant_type) } - - describe ".next" do - context 'when applicant type is "teacher"' do - let(:applicant_type) { "teacher" } - - it "generates a URN with the correct prefix and suffix" do - expect(urn).to match(/^IRPTE[0-9]{5}$/) - end - - it "generates a Urn with a suffix of only characters in the CHARSET" do - charset = %w[0 1 2 3 4 5 6 7 8 9] - - expect(urn[5..9].chars).to all(be_in(charset)) - end - end - - context 'when applicant type is "salaried_trainee"' do - let(:applicant_type) { "salaried_trainee" } - - it "generates a URN with the correct prefix and suffix" do - expect(urn).to match(/^IRPST[0-9]{5}$/) - end - end - - context "when an invalid applicant type is provided" do - let(:applicant_type) { "invalid_type" } - - it "raises an ArgumentError" do - expect { urn }.to raise_error(ArgumentError, 'Invalid route: invalid_type, must be one of ["teacher", "salaried_trainee"]') - end - end - end -end diff --git a/spec/services/submit_form_spec.rb b/spec/services/submit_form_spec.rb index c7faac40..7a30ca2d 100644 --- a/spec/services/submit_form_spec.rb +++ b/spec/services/submit_form_spec.rb @@ -126,7 +126,6 @@ date_of_entry: form.date_of_entry, start_date: form.start_date, subject: SubjectStep.new(form).answer.formatted_value, - urn: kind_of(String), visa_type: form.visa_type, } end @@ -135,16 +134,11 @@ end context "applicant email" do - before do - allow(Urn).to receive(:next).and_return(urn) - end - - let(:urn) { "SOMEURN" } let(:expected_email_params) do { params: { email: form.email_address, - urn: urn, + urn: kind_of(String), }, args: [], }