From d1be4c21c9b4d495951c53cb7249354aadb07ab3 Mon Sep 17 00:00:00 2001
From: Siddharth Dungarwal <sdungarwal@gitlab.com>
Date: Fri, 14 Feb 2025 04:27:43 +0000
Subject: [PATCH] Pass filter_identifiers to keep
---
.../lib/gitlab/housekeeper/change.rb | 8 -----
.../gitlab/housekeeper/filter_identifiers.rb | 19 ++++++++++++
.../lib/gitlab/housekeeper/keep.rb | 25 +++++++++++++++-
.../lib/gitlab/housekeeper/runner.rb | 6 ++--
.../spec/gitlab/housekeeper/change_spec.rb | 21 -------------
.../housekeeper/filter_identifiers_spec.rb | 30 +++++++++++++++++++
6 files changed, 76 insertions(+), 33 deletions(-)
create mode 100644 gems/gitlab-housekeeper/lib/gitlab/housekeeper/filter_identifiers.rb
create mode 100644 gems/gitlab-housekeeper/spec/gitlab/housekeeper/filter_identifiers_spec.rb
diff --git a/gems/gitlab-housekeeper/lib/gitlab/housekeeper/change.rb b/gems/gitlab-housekeeper/lib/gitlab/housekeeper/change.rb
index ce43bd6e0c6ac..586f64cd76c19 100644
--- a/gems/gitlab-housekeeper/lib/gitlab/housekeeper/change.rb
+++ b/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
diff --git a/gems/gitlab-housekeeper/lib/gitlab/housekeeper/filter_identifiers.rb b/gems/gitlab-housekeeper/lib/gitlab/housekeeper/filter_identifiers.rb
new file mode 100644
index 0000000000000..8db407eb33593
--- /dev/null
+++ b/gems/gitlab-housekeeper/lib/gitlab/housekeeper/filter_identifiers.rb
@@ -0,0 +1,19 @@
+# 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
diff --git a/gems/gitlab-housekeeper/lib/gitlab/housekeeper/keep.rb b/gems/gitlab-housekeeper/lib/gitlab/housekeeper/keep.rb
index d982e3ba9bbe4..e0609cb8767f7 100644
--- a/gems/gitlab-housekeeper/lib/gitlab/housekeeper/keep.rb
+++ b/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
diff --git a/gems/gitlab-housekeeper/lib/gitlab/housekeeper/runner.rb b/gems/gitlab-housekeeper/lib/gitlab/housekeeper/runner.rb
index 8b587518bc7c0..040842c6b39ab 100644
--- a/gems/gitlab-housekeeper/lib/gitlab/housekeeper/runner.rb
+++ b/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
diff --git a/gems/gitlab-housekeeper/spec/gitlab/housekeeper/change_spec.rb b/gems/gitlab-housekeeper/spec/gitlab/housekeeper/change_spec.rb
index c50b6ab60664d..dbe515e089042 100644
--- a/gems/gitlab-housekeeper/spec/gitlab/housekeeper/change_spec.rb
+++ b/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
diff --git a/gems/gitlab-housekeeper/spec/gitlab/housekeeper/filter_identifiers_spec.rb b/gems/gitlab-housekeeper/spec/gitlab/housekeeper/filter_identifiers_spec.rb
new file mode 100644
index 0000000000000..0c3774f7ae711
--- /dev/null
+++ b/gems/gitlab-housekeeper/spec/gitlab/housekeeper/filter_identifiers_spec.rb
@@ -0,0 +1,30 @@
+# 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
--
GitLab