From 151f651ba09f9b404af0218971747ef1f646da60 Mon Sep 17 00:00:00 2001
From: "Patrick J. Cherry" <patch@dominoid.net>
Date: Thu, 9 Mar 2023 18:13:28 +0000
Subject: [PATCH 1/2] Ensure PhraseIdentifier fails in consistent ways

* Remove unlimimted retries, raising an error after 10
* Raises an error if no words are found in the list
* refactor `#unique?` to be more specific SQL
---
 lib/phrase_identifier.rb           | 18 ++++++++--
 spec/factories/word.rb             |  7 ++++
 spec/lib/phrase_identifier_spec.rb | 53 ++++++++++++++++++++++++++++++
 3 files changed, 75 insertions(+), 3 deletions(-)
 create mode 100644 spec/factories/word.rb
 create mode 100644 spec/lib/phrase_identifier_spec.rb

diff --git a/lib/phrase_identifier.rb b/lib/phrase_identifier.rb
index 5c90f5a86..421a3c1ac 100644
--- a/lib/phrase_identifier.rb
+++ b/lib/phrase_identifier.rb
@@ -1,12 +1,24 @@
 # frozen_string_literal: true
 
 class PhraseIdentifier
+  class Error < RuntimeError
+  end
+
   def self.generate
-    phrase = Word.order(Arel.sql('RANDOM()')).take(3).pluck(:word).join('-') until unique?(phrase)
-    phrase
+    10.times do
+      phrase = Word.order(Arel.sql('RANDOM()')).take(3).pluck(:word).join('-')
+
+      # Uh-oh, no words found?
+      raise PhraseIdentifier::Error, 'Unable to generate a random phrase identifier' if phrase.blank?
+
+      return phrase if unique?(phrase)
+    end
+
+    # Hmmm we've tried 10 times, so raise an exception.
+    raise PhraseIdentifier::Error, 'Unable to generate a unique phrase identifier'
   end
 
   def self.unique?(phrase)
-    !phrase.nil? && Project.find_by(identifier: phrase).nil?
+    phrase.present? && Project.where(identifier: phrase).none?
   end
 end
diff --git a/spec/factories/word.rb b/spec/factories/word.rb
new file mode 100644
index 000000000..f5bc702bb
--- /dev/null
+++ b/spec/factories/word.rb
@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :word do
+    word { Faker::Verb.base }
+  end
+end
diff --git a/spec/lib/phrase_identifier_spec.rb b/spec/lib/phrase_identifier_spec.rb
new file mode 100644
index 000000000..ab0de8bc6
--- /dev/null
+++ b/spec/lib/phrase_identifier_spec.rb
@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+require 'phrase_identifier'
+
+RSpec.describe PhraseIdentifier do
+  describe '#generate' do
+    subject(:generate) { described_class.generate }
+
+    context 'when there are words in the database' do
+      let(:words) { %w[a b c] }
+      let(:phrase_regex) { /^(#{Regexp.union(words)})-(#{Regexp.union(words)})-(#{Regexp.union(words)})$/ }
+
+      before do
+        Word.delete_all
+        words.each { |word| Word.new(word:).save! }
+      end
+
+      it { is_expected.to match phrase_regex }
+    end
+
+    context 'when there are no available combinations' do
+      let(:identifier) { create(:word).word }
+
+      before do
+        Word.delete_all
+        create(:project, identifier:)
+      end
+
+      it { expect { generate }.to raise_exception(PhraseIdentifier::Error) }
+    end
+
+    context 'when no words are in the database' do
+      before { Word.delete_all }
+
+      it { expect { generate }.to raise_exception(PhraseIdentifier::Error) }
+    end
+  end
+
+  describe '#unique?' do
+    subject { described_class.unique?(phrase) }
+
+    let(:phrase) { 'Hello? Is it me you\'re looking for?' }
+
+    it { is_expected.to be_truthy }
+
+    context 'when a project exists with the phrase as its identifier' do
+      before { create(:project, identifier: phrase) }
+
+      it { is_expected.to be_falsey }
+    end
+  end
+end

From 662d4c4190462e64338e04c0480554c29289b264 Mon Sep 17 00:00:00 2001
From: "Patrick J. Cherry" <patch@dominoid.net>
Date: Fri, 10 Mar 2023 09:29:17 +0000
Subject: [PATCH 2/2] Simplify

---
 spec/lib/phrase_identifier_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/lib/phrase_identifier_spec.rb b/spec/lib/phrase_identifier_spec.rb
index ab0de8bc6..6aa1b0ca0 100644
--- a/spec/lib/phrase_identifier_spec.rb
+++ b/spec/lib/phrase_identifier_spec.rb
@@ -9,7 +9,7 @@
 
     context 'when there are words in the database' do
       let(:words) { %w[a b c] }
-      let(:phrase_regex) { /^(#{Regexp.union(words)})-(#{Regexp.union(words)})-(#{Regexp.union(words)})$/ }
+      let(:phrase_regex) { /^[abc]-[abc]-[abc]$/ }
 
       before do
         Word.delete_all