From 21d2d73d644ae7ad26daf19d540269a0e1955c1f Mon Sep 17 00:00:00 2001 From: fumimowdan Date: Tue, 19 Sep 2023 11:50:07 +0100 Subject: [PATCH] Add uniqueness constraint on applications.urn Urn.call returns a unique urn for the application route. URN suffix is generated from a UUID and guaranties uniqueness and randomness --- .rubocop.yml | 4 + app/models/application.rb | 1 + app/models/urn.rb | 92 +++++++++++++------ app/services/submit_form.rb | 16 +++- ...03917_add_uniqueness_to_application_urn.rb | 9 ++ db/schema.rb | 3 +- spec/factories/applications.rb | 2 +- spec/models/application_spec.rb | 2 + spec/models/urn_spec.rb | 20 ++-- spec/services/submit_form_spec.rb | 2 +- 10 files changed, 107 insertions(+), 44 deletions(-) create mode 100644 db/migrate/20230919103917_add_uniqueness_to_application_urn.rb diff --git a/.rubocop.yml b/.rubocop.yml index 9e23adcb..10e624cf 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -55,6 +55,10 @@ Rails/HasManyOrHasOneDependent: Exclude: - 'app/models/application.rb' +Rails/UniqueValidationWithoutIndex: + Exclude: + - 'app/models/application.rb' + Performance/MethodObjectAsBlock: Exclude: - 'app/models/summary.rb' diff --git a/app/models/application.rb b/app/models/application.rb index 79e54f6e..01ef7e2c 100644 --- a/app/models/application.rb +++ b/app/models/application.rb @@ -67,4 +67,5 @@ def mark_as_qa! validates(:subject, presence: true) validates(:visa_type, presence: true) validates(:applicant, presence: true) + validates(:urn, presence: true, uniqueness: true) end diff --git a/app/models/urn.rb b/app/models/urn.rb index 56330c1c..a5c4b12c 100644 --- a/app/models/urn.rb +++ b/app/models/urn.rb @@ -1,38 +1,78 @@ # frozen_string_literal: true # Urn represents a Uniform Resource Name (URN) generator. -# It generates a URN with a fixed prefix and a random alphanumeric suffix. +# It generates a URN with a fixed prefix and suffix created from UUID +# converted to based 8 to only contain digits. # +# Suffix Algo: +# generate a UUID +# remove any '-' chars +# convert to a binary string +# split in groups of 3 bits, each group is a one char in base 8; from "000" to "111" +# convert each group to a char in base 10; from "0" to "7" => OCTAL_CHARSET +# map each char to the OCTAL_CHARSET # -# Example: +# original idea: https://www.fastruby.io/blog/ruby/uuid/friendlier-uuid-urls-in-ruby.html # -# Urn.generate('teacher') # => "IRPTE12345" -# Urn.generate('teacher') # => "IRPTE12345" -# Urn.generate('salaried_trainee') # => "IRPST12345" +# Example: # +# Urn.call("salaried_trainee") => "IRPST010016643431256522320603357512034131753313" +# Urn.call("teacher") => "IRPTE103726104165343422047203770401347034264072" +# Urn.call("teacher") => "IRPTE447417140555766622065232356514736154173132" + class Urn - attr_reader :value - attr_writer :suffix + OCTAL_CHARSET = %w[0 1 2 3 4 5 6 7].freeze - def self.generate(applicant_type) - code = applicant_type_code(applicant_type) - PREFIX + code + Array.new(LENGTH) { CHARSET.sample }.join + PREFIX = "IRP" + TEACHER_ROUTE = "teacher" + TRAINEE_ROUTE = "salaried_trainee" + ROUTE_MAPPING = { + TEACHER_ROUTE => "TE", + TRAINEE_ROUTE => "ST", + }.freeze + + def self.call(...) + service = new(...) + service.generate_suffix + service.urn end - CHARSET = %w[0 1 2 3 4 5 6 7 8 9].freeze - PREFIX = "IRP" - LENGTH = 5 - private_constant :CHARSET, :PREFIX, :LENGTH - - def self.applicant_type_code(applicant_type) - case applicant_type - when "teacher" - "TE" - when "salaried_trainee" - "ST" - else - raise(ArgumentError, "Invalid applicant type: #{applicant_type}") - end - end - private_methods :applicant_type_code + def initialize(route, prefix: PREFIX, route_mapping: ROUTE_MAPPING) + @prefix = prefix + @code = route_mapping.fetch(route) + end + + def urn + [prefix, code, suffix].join + end + + def generate_suffix + @suffix = ( + method(:binary_coded_decimal) >> + method(:to_base_eight) >> + method(:charset_encode) + ).call(compact_uuid) + end + +private + + attr_reader :prefix, :code, :suffix + + def compact_uuid + SecureRandom.uuid.tr("-", "") + end + + def binary_coded_decimal(uuid) + uuid.chars.map { |c| c.hex.to_s(2).rjust(4, "0") }.join + end + + def to_base_eight(str) + str + .scan(/.{3}/) + .map { |x| x.to_i(2) } + end + + def charset_encode(str) + str.map { |x| OCTAL_CHARSET.fetch(x) } + end end diff --git a/app/services/submit_form.rb b/app/services/submit_form.rb index 939f2756..849d6a4b 100644 --- a/app/services/submit_form.rb +++ b/app/services/submit_form.rb @@ -11,8 +11,9 @@ def initialize(form, ip_address) @form = form @ip_address = ip_address @success = false + @counter = 0 end - attr_reader :form, :ip_address, :application + attr_reader :form, :ip_address, :application, :counter delegate :errors, to: :form @@ -30,6 +31,9 @@ def submit_form! create_application_records send_applicant_email @success = true + rescue StandardError => e + Sentry.capture_exception(e) + errors.add(:base, :technical_error, message: "We are unable to submit your application at the moment. Please try again later") end private @@ -75,20 +79,20 @@ def create_applicant city: form.city, postcode: form.postcode, }, - school: @school, + school: school, ) end def create_application @application = Application.create!( - applicant: @applicant, + applicant: applicant, application_date: Date.current.to_s, application_route: form.application_route, application_progress: ApplicationProgress.new, date_of_entry: form.date_of_entry, start_date: form.start_date, subject: SubjectStep.new(form).answer.formatted_value, - urn: Urn.generate(form.application_route), + urn: Urn.call(form.application_route), visa_type: form.visa_type, ) end @@ -100,10 +104,12 @@ def delete_form def send_applicant_email GovukNotifyMailer .with( - email: @applicant.email_address, + email: applicant.email_address, urn: application.urn, ) .application_submission .deliver_later end + + attr_reader :school, :applicant end diff --git a/db/migrate/20230919103917_add_uniqueness_to_application_urn.rb b/db/migrate/20230919103917_add_uniqueness_to_application_urn.rb new file mode 100644 index 00000000..7a308142 --- /dev/null +++ b/db/migrate/20230919103917_add_uniqueness_to_application_urn.rb @@ -0,0 +1,9 @@ +class AddUniquenessToApplicationUrn < ActiveRecord::Migration[7.0] + def up + drop_view :duplicate_applications + add_index(:applications, :urn) + create_view :duplicate_applications, version: 3 + end + + def down; end +end diff --git a/db/schema.rb b/db/schema.rb index 2873ecf2..b9037b41 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_09_15_100841) do +ActiveRecord::Schema[7.0].define(version: 2023_09_19_103917) do # These are extensions that must be enabled in order to support this database enable_extension "citext" enable_extension "plpgsql" @@ -84,6 +84,7 @@ t.datetime "standing_data_csv_downloaded_at" t.datetime "payroll_csv_downloaded_at" t.index ["applicant_id"], name: "index_applications_on_applicant_id" + t.index ["urn"], name: "index_applications_on_urn" end create_table "audits", force: :cascade do |t| diff --git a/spec/factories/applications.rb b/spec/factories/applications.rb index 49d86172..a2b60ded 100644 --- a/spec/factories/applications.rb +++ b/spec/factories/applications.rb @@ -31,7 +31,7 @@ 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.generate(application_route) } + urn { Urn.call(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..032333a4 100644 --- a/spec/models/application_spec.rb +++ b/spec/models/application_spec.rb @@ -35,6 +35,8 @@ it { expect(application).to validate_presence_of(:subject) } it { expect(application).to validate_presence_of(:visa_type) } it { expect(application).to validate_presence_of(:applicant) } + it { expect(application).to validate_presence_of(:urn) } + it { expect(application).to validate_uniqueness_of(:urn) } end end diff --git a/spec/models/urn_spec.rb b/spec/models/urn_spec.rb index 4df75ded..aa32199f 100644 --- a/spec/models/urn_spec.rb +++ b/spec/models/urn_spec.rb @@ -3,36 +3,36 @@ require "rails_helper" RSpec.describe Urn do - subject(:urn) { described_class.generate(applicant_type) } + describe ".call" do + subject(:urn) { described_class.call(application_route) } - describe ".generate" do context 'when applicant type is "teacher"' do - let(:applicant_type) { "teacher" } + let(:application_route) { "teacher" } it "generates a URN with the correct prefix and suffix" do - expect(urn).to match(/^IRPTE[0-9]{5}$/) + expect(urn).to match(/^IRPTE[0-7]{42}$/) 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] + charset = %w[0 1 2 3 4 5 6 7] - expect(urn[5..9].chars).to all(be_in(charset)) + expect(urn[5..47].chars).to all(be_in(charset)) end end context 'when applicant type is "salaried_trainee"' do - let(:applicant_type) { "salaried_trainee" } + let(:application_route) { "salaried_trainee" } it "generates a URN with the correct prefix and suffix" do - expect(urn).to match(/^IRPST[0-9]{5}$/) + expect(urn).to match(/^IRPST[0-7]{42}$/) end end context "when an invalid applicant type is provided" do - let(:applicant_type) { "invalid_type" } + let(:application_route) { "invalid_type" } it "raises an ArgumentError" do - expect { urn }.to raise_error(ArgumentError, "Invalid applicant type: invalid_type") + expect { urn }.to raise_error(KeyError, 'key not found: "invalid_type"') end end end diff --git a/spec/services/submit_form_spec.rb b/spec/services/submit_form_spec.rb index 3c110afe..92ea6af2 100644 --- a/spec/services/submit_form_spec.rb +++ b/spec/services/submit_form_spec.rb @@ -133,7 +133,7 @@ context "applicant email" do before do - allow(Urn).to receive(:generate).and_return(urn) + allow(Urn).to receive(:call).and_return(urn) end let(:urn) { "SOMEURN" }