Add latest changes from gitlab-org/gitlab@master

5 files changed, 189 insertions, 13 deletions

diff --git a/lib/feature.rb b/lib/feature.rb
index 3115c97658d..7cf40b63fdf 100644
--- a/lib/feature.rb
+++ b/lib/feature.rb
@@ -54,12 +54,14 @@ class Feature
     # unless set explicitly.  The default is `disabled`
     # TODO: remove the `default_enabled:` and read it from the `defintion_yaml`
     # check: https://gitlab.com/gitlab-org/gitlab/-/issues/30228
-    def enabled?(key, thing = nil, default_enabled: false)
+    def enabled?(key, thing = nil, type: :development, default_enabled: false)
       if check_feature_flags_definition?
         if thing && !thing.respond_to?(:flipper_id)
           raise InvalidFeatureFlagError,
             "The thing '#{thing.class.name}' for feature flag '#{key}' needs to include `FeatureGate` or implement `flipper_id`"
         end
+
+        Feature::Definition.valid_usage!(key, type: type, default_enabled: default_enabled)
       end
 
       # During setup the database does not exist yet. So we haven't stored a value
@@ -75,9 +77,9 @@ class Feature
       !default_enabled || Feature.persisted_name?(feature.name) ? feature.enabled?(thing) : true
     end
 
-    def disabled?(key, thing = nil, default_enabled: false)
+    def disabled?(key, thing = nil, type: :development, default_enabled: false)
       # we need to make different method calls to make it easy to mock / define expectations in test mode
-      thing.nil? ? !enabled?(key, default_enabled: default_enabled) : !enabled?(key, thing, default_enabled: default_enabled)
+      thing.nil? ? !enabled?(key, type: type, default_enabled: default_enabled) : !enabled?(key, thing, type: type, default_enabled: default_enabled)
     end
 
     def enable(key, thing = true)
@@ -129,6 +131,12 @@ class Feature
     def register_feature_groups
     end
 
+    def register_definitions
+      return unless check_feature_flags_definition?
+
+      Feature::Definition.load_all!
+    end
+
     private
 
     def flipper
diff --git a/lib/feature/definition.rb b/lib/feature/definition.rb
new file mode 100644
index 00000000000..b0ea55c5805
--- /dev/null
+++ b/lib/feature/definition.rb
@@ -0,0 +1,137 @@
+# frozen_string_literal: true
+
+class Feature
+  class Definition
+    include ::Feature::Shared
+
+    attr_reader :path
+    attr_reader :attributes
+
+    PARAMS.each do |param|
+      define_method(param) do
+        attributes[param]
+      end
+    end
+
+    def initialize(path, opts = {})
+      @path = path
+      @attributes = {}
+
+      # assign nil, for all unknown opts
+      PARAMS.each do |param|
+        @attributes[param] = opts[param]
+      end
+    end
+
+    def key
+      name.to_sym
+    end
+
+    def validate!
+      unless name.present?
+        raise Feature::InvalidFeatureFlagError, "Feature flag is missing name"
+      end
+
+      unless path.present?
+        raise Feature::InvalidFeatureFlagError, "Feature flag '#{name}' is missing path"
+      end
+
+      unless type.present?
+        raise Feature::InvalidFeatureFlagError, "Feature flag '#{name}' is missing type. Ensure to update #{path}"
+      end
+
+      unless Definition::TYPES.include?(type.to_sym)
+        raise Feature::InvalidFeatureFlagError, "Feature flag '#{name}' type '#{type}' is invalid. Ensure to update #{path}"
+      end
+
+      unless File.basename(path, ".yml") == name
+        raise Feature::InvalidFeatureFlagError, "Feature flag '#{name}' has an invalid path: '#{path}'. Ensure to update #{path}"
+      end
+
+      unless File.basename(File.dirname(path)) == type
+        raise Feature::InvalidFeatureFlagError, "Feature flag '#{name}' has an invalid type: '#{path}'. Ensure to update #{path}"
+      end
+
+      if default_enabled.nil?
+        raise Feature::InvalidFeatureFlagError, "Feature flag '#{name}' is missing default_enabled. Ensure to update #{path}"
+      end
+    end
+
+    def valid_usage!(type_in_code:, default_enabled_in_code:)
+      unless Array(type).include?(type_in_code.to_s)
+        # Raise exception in test and dev
+        raise Feature::InvalidFeatureFlagError, "The `type:` of `#{key}` is not equal to config: " \
+          "#{type_in_code} vs #{type}. Ensure to use valid type in #{path} or ensure that you use " \
+          "a valid syntax: #{TYPES.dig(type, :example)}"
+      end
+
+      # We accept an array of defaults as some features are undefined
+      # and have `default_enabled: true/false`
+      unless Array(default_enabled).include?(default_enabled_in_code)
+        # Raise exception in test and dev
+        raise Feature::InvalidFeatureFlagError, "The `default_enabled:` of `#{key}` is not equal to config: " \
+          "#{default_enabled_in_code} vs #{default_enabled}. Ensure to update #{path}"
+      end
+    end
+
+    def to_h
+      attributes
+    end
+
+    class << self
+      def paths
+        @paths ||= [Rails.root.join('config', 'feature_flags', '**', '*.yml')]
+      end
+
+      def definitions
+        @definitions ||= {}
+      end
+
+      def load_all!
+        definitions.clear
+
+        paths.each do |glob_path|
+          load_all_from_path!(glob_path)
+        end
+
+        definitions
+      end
+
+      def valid_usage!(key, type:, default_enabled:)
+        if definition = definitions[key.to_sym]
+          definition.valid_usage!(type_in_code: type, default_enabled_in_code: default_enabled)
+        elsif type_definition = self::TYPES[type]
+          raise InvalidFeatureFlagError, "Missing feature definition for `#{key}`" unless type_definition[:optional]
+        else
+          raise InvalidFeatureFlagError, "Unknown feature flag type used: `#{type}`"
+        end
+      end
+
+      private
+
+      def load_from_file(path)
+        definition = File.read(path)
+        definition = YAML.safe_load(definition)
+        definition.deep_symbolize_keys!
+
+        self.new(path, definition).tap(&:validate!)
+      rescue => e
+        raise Feature::InvalidFeatureFlagError, "Invalid definition for `#{path}`: #{e.message}"
+      end
+
+      def load_all_from_path!(glob_path)
+        Dir.glob(glob_path).each do |path|
+          definition = load_from_file(path)
+
+          if previous = definitions[definition.key]
+            raise InvalidFeatureFlagError, "Feature flag '#{definition.key}' is already defined in '#{previous.path}'"
+          end
+
+          definitions[definition.key] = definition
+        end
+      end
+    end
+  end
+end
+
+Feature::Definition.prepend_if_ee('EE::Feature::Definition')
diff --git a/lib/feature/shared.rb b/lib/feature/shared.rb
new file mode 100644
index 00000000000..14efbb07100
--- /dev/null
+++ b/lib/feature/shared.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+# This file can contain only simple constructs as it is shared between:
+# 1. `Pure Ruby`: `bin/feature-flag`
+# 2. `GitLab Rails`: `lib/feature/definition.rb`
+
+class Feature
+  module Shared
+    # optional: defines if a on-disk definition is required for this feature flag type
+    # rollout_issue: defines if `bin/feature-flag` asks for rollout issue
+    # example: usage being shown when exception is raised
+    TYPES = {
+      development: {
+        description: 'Short lived, used to enable unfinished code to be deployed',
+        optional: true,
+        rollout_issue: true,
+        example: <<-EOS
+          Feature.enabled?(:my_feature_flag)
+          Feature.enabled?(:my_feature_flag, type: :development)
+        EOS
+      }
+    }.freeze
+
+    PARAMS = %i[
+      name
+      default_enabled
+      type
+      introduced_by_url
+      rollout_issue_url
+      group
+    ].freeze
+  end
+end
diff --git a/lib/gitlab/import_export/snippet_repo_restorer.rb b/lib/gitlab/import_export/snippet_repo_restorer.rb
index 31b1a37bbe1..2d0aa05fc3c 100644
--- a/lib/gitlab/import_export/snippet_repo_restorer.rb
+++ b/lib/gitlab/import_export/snippet_repo_restorer.rb
@@ -42,6 +42,8 @@ module Gitlab
           snippet.repository.expire_exists_cache
 
           raise SnippetRepositoryError, _("Invalid repository bundle for snippet with id %{snippet_id}") % { snippet_id: snippet.id }
+        else
+          Snippets::UpdateStatisticsService.new(snippet).execute
         end
       end
 
diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb
index 6cbf4e978fa..b1953aab4fd 100644
--- a/lib/gitlab/usage_data.rb
+++ b/lib/gitlab/usage_data.rb
@@ -159,8 +159,7 @@ module Gitlab
             usage_counters,
             user_preferences_usage,
             ingress_modsecurity_usage,
-            container_expiration_policies_usage,
-            merge_requests_usage(last_28_days_time_period)
+            container_expiration_policies_usage
           ).tap do |data|
             data[:snippets] = data[:personal_snippets] + data[:project_snippets]
           end
@@ -405,23 +404,19 @@ module Gitlab
       end
 
       # rubocop: disable CodeReuse/ActiveRecord
-      def merge_requests_usage(time_period)
+      def merge_requests_users(time_period)
         query =
           Event
             .where(target_type: Event::TARGET_TYPES[:merge_request].to_s)
             .where(time_period)
 
-        merge_request_users = distinct_count(
+        distinct_count(
           query,
           :author_id,
           batch_size: 5_000, # Based on query performance, this is the optimal batch size.
           start: User.minimum(:id),
           finish: User.maximum(:id)
         )
-
-        {
-          merge_requests_users: merge_request_users
-        }
       end
       # rubocop: enable CodeReuse/ActiveRecord
 
@@ -477,9 +472,10 @@ module Gitlab
       end
       # rubocop: enable CodeReuse/ActiveRecord
 
-      # Omitted because no user, creator or author associated: `lfs_objects`, `pool_repositories`, `web_hooks`
       def usage_activity_by_stage_create(time_period)
-        {}
+        {}.tap do |h|
+          h[:merge_requests_users] = merge_requests_users(time_period) if time_period.present?
+        end
       end
 
       # Omitted because no user, creator or author associated: `campaigns_imported_from_github`, `ldap_group_links`