From f51af496769f2fe181d4633f810b85103efd181e Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Wed, 15 Jun 2016 11:15:01 +0530 Subject: Support wildcard matches for protected branches at the model level. 1. The main implementation is in the `ProtectedBranch` model. The wildcard is converted to a Regex and compared. This has been tested thoroughly. - While `Project#protected_branch?` is the main entry point, `project#open_branches` and `project#developers_can_push_to_protected_branch?` have also been modified to work with wildcard protected branches. - The regex is memoized (within the `ProtectedBranch` instance) 2. Improve the performance of `Project#protected_branch?` - This method is called from `Project#open_branches` once _per branch_ in the project, to check if that branch is protected or not. - Before, `#protected_branch?` was making a database call every time it was invoked (in the above case, that amounts to once per branch), which is expensive. - This commit caches the list of protected branches in memory, which reduces the number of database calls down to 1. - A downside to this approach is that `#protected_branch?` _could_ return a stale value (due to the caching), but this is an acceptable tradeoff. 3. Remove the (now) unused `Project#protected_branch_names` method. - This was previously used to check for protected branch status. --- app/models/protected_branch.rb | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) (limited to 'app/models/protected_branch.rb') diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb index 33cf046fa75..3db1ab0e5f9 100644 --- a/app/models/protected_branch.rb +++ b/app/models/protected_branch.rb @@ -8,4 +8,40 @@ class ProtectedBranch < ActiveRecord::Base def commit project.commit(self.name) end + + # Returns all protected branches that match the given branch name. + # This realizes all records from the scope built up so far, and does + # _not_ return a relation. + # + # This method optionally takes in a list of `protected_branches` to search + # through, to avoid calling out to the database. + def self.matching(branch_name, protected_branches: nil) + (protected_branches || all).select { |protected_branch| protected_branch.matches?(branch_name) } + end + + # Checks if the protected branch matches the given branch name. + def matches?(branch_name) + return false if self.name.blank? + + exact_match?(branch_name) || wildcard_match?(branch_name) + end + + protected + + def exact_match?(branch_name) + self.name == branch_name + end + + def wildcard_match?(branch_name) + wildcard_regex === branch_name + end + + def wildcard_regex + @wildcard_regex ||= begin + name = self.name.gsub('*', 'STAR_DONT_ESCAPE') + quoted_name = Regexp.quote(name) + regex_string = quoted_name.gsub('STAR_DONT_ESCAPE', '.*?') + /\A#{regex_string}\z/ + end + end end -- cgit v1.2.1 From 2a5cb7ec5259123cbbecb0577b9b4afacaf7546a Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Thu, 16 Jun 2016 13:03:30 +0530 Subject: Modify the frontend for wildcard protected branches. 1. Allow entering any branch name for a protected branch. - Either pick from a list of options, or enter it manually - You can enter wildcards. 2. Display branches matching a protected branch. - Add a `ProtectedBranches#show` page that displays the branches matching the given protected branch, or a message if there are no matches. - On the `index` page, display the last commit for an exact match, or the number of matching branches for a wildcard match. - Add an `iid` column to `protected_branches` - this is what we use for the `show` page URL. - On the off chance that this feature is unnecessary, this commit encapsulates it neatly, so it can be removed without affecting anything else. 3. Remove the "Last Commit" column from the list of protected branches. - There's no way to pull these for wildcard protected branches, so it's best left for the `show` page. - Rename the `@branches` instance variable to `@protected_branches` - Minor styling changes with the "Unprotect" button - floated right like the "Revoke" button for personal access tokens 4. Paginate the list of protected branches. 5. Move the instructions to the left side of the page. --- app/models/protected_branch.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'app/models/protected_branch.rb') diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb index 3db1ab0e5f9..d3d5e1d98b2 100644 --- a/app/models/protected_branch.rb +++ b/app/models/protected_branch.rb @@ -19,6 +19,12 @@ class ProtectedBranch < ActiveRecord::Base (protected_branches || all).select { |protected_branch| protected_branch.matches?(branch_name) } end + # Returns all branches (among the given list of branches [`Gitlab::Git::Branch`]) + # that match the current protected branch. + def matching(branches) + branches.select { |branch| self.matches?(branch.name) } + end + # Checks if the protected branch matches the given branch name. def matches?(branch_name) return false if self.name.blank? @@ -26,6 +32,11 @@ class ProtectedBranch < ActiveRecord::Base exact_match?(branch_name) || wildcard_match?(branch_name) end + # Checks if this protected branch contains a wildcard + def wildcard? + self.name.include?('*') + end + protected def exact_match?(branch_name) -- cgit v1.2.1 From b1c81f849e5e5b03f56e89cdcefba029ed5c0543 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Wed, 6 Jul 2016 15:07:30 +0530 Subject: Have `Project#open_branches` return branches that are matched by a wildcard protected branch. 1. The `open_branches` method is used to provide a list of branches while creating a protected branch. 2. It makes sense to include branches which are matched by one or more wildcard protected branches, since the user might want to make exact protected branches from these as well. 3. This also provides a large performance improvement. On my machine, in a project with 5000 branches and 2000 protected branches, the `ProtectedBranches#index` page went from a 40 seconds load time to 4 seconds (10x speedup). --- app/models/protected_branch.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/models/protected_branch.rb') diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb index d3d5e1d98b2..b7011d7afdf 100644 --- a/app/models/protected_branch.rb +++ b/app/models/protected_branch.rb @@ -34,7 +34,7 @@ class ProtectedBranch < ActiveRecord::Base # Checks if this protected branch contains a wildcard def wildcard? - self.name.include?('*') + self.name && self.name.include?('*') end protected -- cgit v1.2.1