Fix branch protection API.

1. Previously, we were not removing existing access levels before
   creating new ones. This is not a problem for EE, but _is_ for CE,
   since we restrict the number of access levels in CE to 1.

2. The correct approach is:

    CE -> delete all access levels before updating a protected branch
    EE -> delete developer access levels if "developers_can_{merge,push}" is switched off

3. The dispatch is performed by checking if a "length: 1" validation is
   present on the access levels or not.

4. Another source of problems was that we didn't put multiple queries in
   a transaction. If the `destroy_all` passes, but the `update` fails,
   we should have a rollback.

5. Modifying the API to provide users direct access to CRUD access
   levels will make things a lot simpler.

6. Create `create/update` services separately for this API, which
   perform the necessary data translation, before calling the regular
   `create/update` services. The translation code was getting too large
   for the API endpoint itself, so this move makes sense.

2 files changed, 109 insertions, 0 deletions

diff --git a/app/services/protected_branches/api_create_service.rb b/app/services/protected_branches/api_create_service.rb
new file mode 100644
index 00000000000..a28056035b8
--- /dev/null
+++ b/app/services/protected_branches/api_create_service.rb
@@ -0,0 +1,30 @@
+# The protected branches API still uses the `developers_can_push` and `developers_can_merge`
+# flags for backward compatibility, and so performs translation between that format and the
+# internal data model (separate access levels). The translation code is non-trivial, and so
+# lives in this service.
+module ProtectedBranches
+  class ApiCreateService < BaseService
+    def initialize(project, user, params, developers_can_push:, developers_can_merge:)
+      super(project, user, params)
+      @developers_can_merge = developers_can_merge
+      @developers_can_push = developers_can_push
+    end
+
+    def execute
+      if @developers_can_push
+        @params.merge!(push_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }])
+      else
+        @params.merge!(push_access_levels_attributes: [{ access_level: Gitlab::Access::MASTER }])
+      end
+
+      if @developers_can_merge
+        @params.merge!(merge_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }])
+      else
+        @params.merge!(merge_access_levels_attributes: [{ access_level: Gitlab::Access::MASTER }])
+      end
+
+      service = ProtectedBranches::CreateService.new(@project, @current_user, @params)
+      service.execute
+    end
+  end
+end
diff --git a/app/services/protected_branches/api_update_service.rb b/app/services/protected_branches/api_update_service.rb
new file mode 100644
index 00000000000..41d3d413caa
--- /dev/null
+++ b/app/services/protected_branches/api_update_service.rb
@@ -0,0 +1,79 @@
+# The protected branches API still uses the `developers_can_push` and `developers_can_merge`
+# flags for backward compatibility, and so performs translation between that format and the
+# internal data model (separate access levels). The translation code is non-trivial, and so
+# lives in this service.
+module ProtectedBranches
+  class ApiUpdateService < BaseService
+    def initialize(project, user, params, developers_can_push:, developers_can_merge:)
+      super(project, user, params)
+      @developers_can_merge = developers_can_merge
+      @developers_can_push = developers_can_push
+    end
+
+    def execute(protected_branch)
+      @protected_branch = protected_branch
+
+      protected_branch.transaction do
+        # If a protected branch can have only a single access level (CE), delete it to
+        # make room for the new access level. If a protected branch can have more than
+        # one access level (EE), only remove the relevant access levels. If we don't do this,
+        # we'll have a failed validation.
+        if protected_branch_restricted_to_single_access_level?
+          delete_redundant_ce_access_levels
+        else
+          delete_redundant_ee_access_levels
+        end
+
+        if @developers_can_push
+          params.merge!(push_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }])
+        elsif @developers_can_push == false
+          params.merge!(push_access_levels_attributes: [{ access_level: Gitlab::Access::MASTER }])
+        end
+
+        if @developers_can_merge
+          params.merge!(merge_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }])
+        elsif @developers_can_merge == false
+          params.merge!(merge_access_levels_attributes: [{ access_level: Gitlab::Access::MASTER }])
+        end
+
+        service = ProtectedBranches::UpdateService.new(@project, @current_user, @params)
+        service.execute(protected_branch)
+      end
+    end
+
+    private
+
+    def delete_redundant_ce_access_levels
+      if @developers_can_merge || @developers_can_merge == false
+        @protected_branch.merge_access_levels.destroy_all
+      end
+
+      if @developers_can_push || @developers_can_push == false
+        @protected_branch.push_access_levels.destroy_all
+      end
+    end
+
+    def delete_redundant_ee_access_levels
+      if @developers_can_merge
+        @protected_branch.merge_access_levels.developer.destroy_all
+      elsif @developers_can_merge == false
+        @protected_branch.merge_access_levels.developer.destroy_all
+        @protected_branch.merge_access_levels.master.destroy_all
+      end
+
+      if @developers_can_push
+        @protected_branch.push_access_levels.developer.destroy_all
+      elsif @developers_can_push == false
+        @protected_branch.push_access_levels.developer.destroy_all
+        @protected_branch.push_access_levels.master.destroy_all
+      end
+    end
+
+    def protected_branch_restricted_to_single_access_level?
+      length_validator = ProtectedBranch.validators_on(:push_access_levels).find do |validator|
+        validator.is_a? ActiveModel::Validations::LengthValidator
+      end
+      length_validator.options[:is] == 1 if length_validator
+    end
+  end
+end