From 39eb8e112d1eb5467d483375aa24738324cb02f9 Mon Sep 17 00:00:00 2001 From: fumimowdan Date: Mon, 30 Oct 2023 15:05:22 +0000 Subject: [PATCH] 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: [], }