Merge branch 'zj-validation-initializer' into 'master'

config/initializers/6_validations.rb

@@ -26,17 +26,6 @@ def validate_storages_config
   Gitlab.config.repositories.storages.each do |name, repository_storage|
     storage_validation_error("\"#{name}\" is not a valid storage name") unless storage_name_valid?(name)
 
-    if repository_storage.is_a?(String)
-      raise "#{name} is not a valid storage, because it has no `path` key. " \
-        "It may be configured as:\n\n#{name}:\n  path: #{repository_storage}\n\n" \
-        "For source installations, update your config/gitlab.yml Refer to gitlab.yml.example for an updated example.\n\n" \
-        "If you're using the Gitlab Development Kit, you can update your configuration running `gdk reconfigure`.\n"
-    end
-
-    if !repository_storage.is_a?(Gitlab::GitalyClient::StorageSettings) || repository_storage.legacy_disk_path.nil?
-      storage_validation_error("#{name} is not a valid storage, because it has no `path` key. Refer to gitlab.yml.example for an updated example")
-    end
-
     %w(failure_count_threshold failure_reset_time storage_timeout).each do |setting|
       # Falling back to the defaults is fine!
       next if repository_storage[setting].nil?

lib/gitlab/gitaly_client/storage_settings.rb

@@ -5,6 +5,14 @@ module Gitlab
     # directly.
     class StorageSettings
       DirectPathAccessError = Class.new(StandardError)
+      InvalidConfigurationError = Class.new(StandardError)
+
+      INVALID_STORAGE_MESSAGE = <<~MSG.freeze
+        Storage is invalid because it has no `path` key.
+
+        For source installations, update your config/gitlab.yml Refer to gitlab.yml.example for an updated example.
+        If you're using the Gitlab Development Kit, you can update your configuration running `gdk reconfigure`.
+      MSG
 
       # This class will give easily recognizable NoMethodErrors
       Deprecated = Class.new
@@ -12,7 +20,8 @@ module Gitlab
       attr_reader :legacy_disk_path
 
       def initialize(storage)
-        raise "expected a Hash, got a #{storage.class.name}" unless storage.is_a?(Hash)
+        raise InvalidConfigurationError, "expected a Hash, got a #{storage.class.name}" unless storage.is_a?(Hash)
+        raise InvalidConfigurationError, INVALID_STORAGE_MESSAGE unless storage.has_key?('path')
 
         # Support a nil 'path' field because some of the circuit breaker tests use it.
         @legacy_disk_path = File.expand_path(storage['path'], Rails.root) if storage['path']

spec/initializers/6_validations_spec.rb

@@ -42,26 +42,6 @@ describe '6_validations' do
         expect { validate_storages_config }.to raise_error('"name with spaces" is not a valid storage name. Please fix this in your gitlab.yml before starting GitLab.')
       end
     end
-
-    context 'with incomplete settings' do
-      before do
-        mock_storages('foo' => {})
-      end
-
-      it 'throws an error suggesting the user to update its settings' do
-        expect { validate_storages_config }.to raise_error('foo is not a valid storage, because it has no `path` key. Refer to gitlab.yml.example for an updated example. Please fix this in your gitlab.yml before starting GitLab.')
-      end
-    end
-
-    context 'with deprecated settings structure' do
-      before do
-        mock_storages('foo' => 'tmp/tests/paths/a/b/c')
-      end
-
-      it 'throws an error suggesting the user to update its settings' do
-        expect { validate_storages_config }.to raise_error("foo is not a valid storage, because it has no `path` key. It may be configured as:\n\nfoo:\n  path: tmp/tests/paths/a/b/c\n\nFor source installations, update your config/gitlab.yml Refer to gitlab.yml.example for an updated example.\n\nIf you're using the Gitlab Development Kit, you can update your configuration running `gdk reconfigure`.\n")
-      end
-    end
   end
 
   describe 'validate_storages_paths' do

spec/lib/gitlab/gitaly_client/storage_settings_spec.rb

+require 'spec_helper'
+
+describe Gitlab::GitalyClient::StorageSettings do
+  describe "#initialize" do
+    context 'when the storage contains no path' do
+      it 'raises an error' do
+        expect do
+          described_class.new("foo" => {})
+        end.to raise_error(described_class::InvalidConfigurationError)
+      end
+    end
+
+    context "when the argument isn't a hash" do
+      it 'raises an error' do
+        expect do
+          described_class.new("test")
+        end.to raise_error("expected a Hash, got a String")
+      end
+    end
+
+    context 'when the storage is valid' do
+      it 'raises no error' do
+        expect do
+          described_class.new("path" => Rails.root)
+        end.not_to raise_error
+      end
+    end
+  end
+end