Pass filter_identifiers to keep

gems/gitlab-housekeeper/lib/gitlab/housekeeper/change.rb

@@ -68,14 +68,6 @@ def commit_message
         MARKDOWN
       end
 
-      def matches_filters?(filters)
-        filters.all? do |filter|
-          identifiers.any? do |identifier|
-            identifier.match?(filter)
-          end
-        end
-      end
-
       def update_required?(category)
         !category.in?(non_housekeeper_changes)
       end

gems/gitlab-housekeeper/lib/gitlab/housekeeper/filter_identifiers.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Housekeeper
+    class FilterIdentifiers
+      def initialize(filters)
+        @filters = filters
+      end
+
+      def matches_filters?(identifiers)
+        @filters.all? do |filter|
+          identifiers.any? do |identifier|
+            identifier.match?(filter)
+          end
+        end
+      end
+    end
+  end
+end

gems/gitlab-housekeeper/lib/gitlab/housekeeper/keep.rb

@@ -15,8 +15,31 @@ module Housekeeper
     #   )
     #   ```
     class Keep
-      def initialize(logger: nil)
+      attr_reader :filter_identifiers
+
+      def initialize(logger: nil, filter_identifiers: nil)
         @logger = logger || Logger.new(nil)
+        @filter_identifiers = filter_identifiers
+      end
+
+      # This method is used to filter for each change at more granular level. For example:
+      # - When we pass --filter-identifiers my_feature_flag,
+      #   it should only create an MR specifically for my_feature_flag because
+      #   we are having the feature flag name in the change.identifiers
+      # Since the filtering logic is also implemented in the `Runner` class,
+      # it is not required to use this in a keep, but it is recommended for
+      # keeps which have expensive computation to perform. This is because
+      # the Runner only gets the change object after the Keep has finished
+      # computing the change.
+      # This can be done in the `each_change` method like:
+      # ```
+      # next unless matches_filter_identifiers?(change.identifiers)
+      # ... do the computation heavy work and yield change object.
+      # ```
+      def matches_filter_identifiers?(identifiers)
+        return true unless filter_identifiers
+
+        filter_identifiers.matches_filters?(identifiers)
       end
 
       # The each_change method must update local working copy files and yield a Change object which describes the

gems/gitlab-housekeeper/lib/gitlab/housekeeper/runner.rb

@@ -36,7 +36,7 @@ def initialize(
                    all_keeps
                  end
 
-        @filter_identifiers = filter_identifiers
+        @filter_identifiers = ::Gitlab::Housekeeper::FilterIdentifiers.new(filter_identifiers)
       end
 
       def run
@@ -45,7 +45,7 @@ def run
         git.with_clean_state do
           @keeps.each do |keep_class|
             @logger.puts "Running keep #{keep_class}"
-            keep = keep_class.new(logger: @logger)
+            keep = keep_class.new(logger: @logger, filter_identifiers: @filter_identifiers)
             keep.each_change do |change|
               unless change.valid?
                 @logger.warn "Ignoring invalid change from: #{keep_class}"
@@ -57,7 +57,7 @@ def run
               branch_name = git.create_branch(change)
               add_standard_change_data(change)
 
-              if change.aborted? || !change.matches_filters?(@filter_identifiers)
+              if change.aborted? || !@filter_identifiers.matches_filters?(change.identifiers)
                 # At this point the keep has already run and edited files so we need to
                 # restore the local working copy. We could simply checkout all
                 # changed_files but this is very risky as it could mean losing work that

gems/gitlab-housekeeper/spec/gitlab/housekeeper/change_spec.rb

@@ -168,25 +168,4 @@
       end
     end
   end
-
-  describe '#matches_filters?' do
-    let(:identifiers) { %w[this-is a-list of IdentifierS] }
-    let(:change) { create_change(identifiers: identifiers) }
-
-    it 'matches when all regexes match at least one identifier' do
-      expect(change.matches_filters?([/list/, /Ide.*fier/])).to eq(true)
-    end
-
-    it 'does not match when none of the regexes match' do
-      expect(change.matches_filters?([/nomatch/, /Ide.*fffffier/])).to eq(false)
-    end
-
-    it 'does not match when only some of the regexes match' do
-      expect(change.matches_filters?([/nomatch/, /Ide.*fier/])).to eq(false)
-    end
-
-    it 'matches an empty list of filters' do
-      expect(change.matches_filters?([])).to eq(true)
-    end
-  end
 end

gems/gitlab-housekeeper/spec/gitlab/housekeeper/filter_identifiers_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'gitlab/housekeeper/filter_identifiers'
+
+RSpec.describe ::Gitlab::Housekeeper::FilterIdentifiers do
+  describe '.matches_filters?' do
+    let(:identifiers) { %w[this-is a-list of IdentifierS] }
+
+    it 'matches when all regexes match at least one identifier' do
+      filter_identifiers = described_class.new([/list/, /Ide.*fier/])
+      expect(filter_identifiers.matches_filters?(identifiers)).to eq(true)
+    end
+
+    it 'does not match when none of the regexes match' do
+      filter_identifiers = described_class.new([/nomatch/, /Ide.*fffffier/])
+      expect(filter_identifiers.matches_filters?(identifiers)).to eq(false)
+    end
+
+    it 'does not match when only some of the regexes match' do
+      filter_identifiers = described_class.new([/nomatch/, /Ide.*fier/])
+      expect(filter_identifiers.matches_filters?(identifiers)).to eq(false)
+    end
+
+    it 'matches an empty list of filters' do
+      filter_identifiers = described_class.new([])
+      expect(filter_identifiers.matches_filters?(identifiers)).to eq(true)
+    end
+  end
+end