Prevent PublicBody#set_api_key call if unrequired

Don't call the method unless we need to. Adds #set_api_key! to set a new
API key even if there's an existing one.

2 files changed, 43 insertions, 3 deletions

diff --git a/app/models/public_body.rb b/app/models/public_body.rb
index 9b2a20d82..d11ed683a 100644
--- a/app/models/public_body.rb
+++ b/app/models/public_body.rb
@@ -68,7 +68,7 @@ class PublicBody < ActiveRecord::Base
 
     validate :request_email_if_requestable
 
-    before_save :set_api_key
+    before_save :set_api_key!, :unless => :api_key
     before_save :set_default_publication_scheme
     after_save :purge_in_cache
     after_update :reindex_requested_from
@@ -187,7 +187,11 @@ class PublicBody < ActiveRecord::Base
     end
 
     def set_api_key
-      self.api_key = SecureRandom.base64(33) if self.api_key.nil?
+        set_api_key! if api_key.nil?
+    end
+
+    def set_api_key!
+        self.api_key = SecureRandom.base64(33)
     end
 
     # like find_by_url_name but also search historic url_name if none found
diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb
index ca94c59a8..3d14127f4 100644
--- a/spec/models/public_body_spec.rb
+++ b/spec/models/public_body_spec.rb
@@ -102,8 +102,44 @@ describe PublicBody do
           end
       end
   end
-end
 
+  describe :set_api_key do
+
+    it 'generates and sets an API key' do
+      SecureRandom.stub(:base64).and_return('APIKEY')
+      body = PublicBody.new
+      body.set_api_key
+      expect(body.api_key).to eq('APIKEY')
+    end
+
+    it 'does not overwrite an existing API key' do
+      SecureRandom.stub(:base64).and_return('APIKEY')
+      body = PublicBody.new(:api_key => 'EXISTING')
+      body.set_api_key
+      expect(body.api_key).to eq('EXISTING')
+    end
+
+  end
+
+  describe :set_api_key! do
+
+    it 'generates and sets an API key' do
+      SecureRandom.stub(:base64).and_return('APIKEY')
+      body = PublicBody.new
+      body.set_api_key!
+      expect(body.api_key).to eq('APIKEY')
+    end
+
+    it 'overwrites an existing API key' do
+      SecureRandom.stub(:base64).and_return('APIKEY')
+      body = PublicBody.new(:api_key => 'EXISTING')
+      body.set_api_key!
+      expect(body.api_key).to eq('APIKEY')
+    end
+
+  end
+
+end
 
 describe PublicBody, " using tags" do
     before do