api: added create and update methods to registries

Signed-off-by: Miquel Sabaté Solà <[email protected]>

app/controllers/admin/registries_controller.rb

@@ -26,18 +26,18 @@ def new
   # done. If this check is passed/skipped, then it will try to create the
   # registry.
   def create
-    @registry = Registry.new(create_params)
+    svc = ::Registries::CreateService.new(current_user, create_params)
+    svc.force = params[:force]
+    svc.execute
 
-    # Check the reachability of the registry.
-
-    check_reachability("new") unless params[:force]
-    return if @unreachable
-
-    if @registry.save
-      Namespace.update_all(registry_id: @registry.id)
+    if svc.valid?
       redirect_to admin_registries_path, notice: "Registry was successfully created."
+    elsif svc.reachable?
+      flash[:alert] = svc.messages
+      redirect_to new_admin_registry_path, alert: svc.messages
     else
-      redirect_to new_admin_registry_path, alert: @registry.errors.full_messages
+      flash[:alert] = svc.messages
+      render :new, status: :unprocessable_entity
     end
   end
 
@@ -55,40 +55,24 @@ def edit
   # Right now this just toggles the value of the "use_ssl" attribute for the
   # given registry. This might change in the future.
   def update
-    @registry = Registry.find(params[:id])
-    @registry.assign_attributes(update_params)
-    @can_change_hostname = !Repository.any?
-
-    # Check the reachability of the registry.
-    check_reachability("edit")
-    return if @unreachable
+    attrs = update_params.merge(id: params[:id])
+    svc = ::Registries::UpdateService.new(current_user, attrs)
+    @registry = svc.execute
 
-    if @registry.save
+    if svc.valid?
       # NOTE: if we decide to use rails-observers at some point,
       # we can remove this from here and use it in observers
       Rails.cache.delete "registry#{@registry.id}_status"
       redirect_to admin_registries_path, notice: "Registry updated successfully!"
     else
-      flash[:alert] = @registry.errors.full_messages
+      flash[:alert] = svc.messages
+      @can_change_hostname = !Repository.any?
       render "edit", status: :unprocessable_entity
     end
   end
 
   private
 
-  # Checks if registry is reachable and sends `unprocessable_entity`
-  # status if unreachable
-  def check_reachability(action)
-    msg = @registry.reachable?
-    return if msg.blank?
-
-    logger.info "\nRegistry not reachable:\n#{@registry.inspect}\n#{msg}\n"
-    flash[:alert] = "#{msg} You can skip this check by clicking on the
-      \"Skip remote checks\" checkbox."
-    render action, status: :unprocessable_entity
-    @unreachable = true
-  end
-
   # Raises a routing error if there is already a registry in place.
   # NOTE: (mssola) remove this once we support multiple registries.
   def registry_created

app/services/registries/base_service.rb

@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+module Registries
+  class BaseService < ::BaseService
+    attr_reader :messages
+    attr_accessor :force
+
+    def valid?
+      @valid
+    end
+
+    def reachable?
+      @reachable
+    end
+
+    protected
+
+    def check_reachability!
+      msg = @registry.reachable?
+      return if msg.blank?
+
+      Rails.logger.info "\nRegistry not reachable:\n#{@registry.inspect}\n#{msg}\n"
+      @valid = false
+      @reachable = false
+      @messages[:hostname] = (@messages[:hostname] || []).push(msg)
+    end
+  end
+end

app/services/registries/create_service.rb

@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+module Registries
+  class CreateService < ::Registries::BaseService
+    def execute
+      @registry  = Registry.new(params)
+      @valid     = @registry.valid?
+      @messages  = @registry.errors.messages
+      @reachable = true
+
+      check!
+      if @valid
+        if @registry.save
+          Namespace.update_all(registry_id: @registry.id)
+        else
+          @messages.merge(@registry.errors.messages)
+        end
+      end
+
+      @registry
+    end
+
+    protected
+
+    def check!
+      return unless @valid
+      check_uniqueness!
+      return unless @valid
+      check_reachability! unless @force
+    end
+
+    def check_uniqueness!
+      return unless Registry.any?
+      @valid = false
+      @messages[:status] = "You can only create one registry"
+    end
+  end
+end

app/services/registries/update_service.rb

@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+module Registries
+  class UpdateService < ::Registries::BaseService
+    def execute
+      @registry = Registry.find(params[:id])
+      @messages = @registry.errors.messages
+      @registry.assign_attributes(params)
+      @valid = @registry.valid?
+
+      check!
+      if @valid
+        @messages.merge(@registry.errors.messages) unless @registry.save
+      end
+
+      @registry
+    end
+
+    protected
+
+    def check!
+      return unless @valid
+      check_hostname!
+      return unless @valid
+      check_reachability! unless @force
+    end
+
+    def check_hostname!
+      return if !Repository.any? || params[:hostname].blank?
+      @valid = false
+      @messages[:hostname] = "Registry is not empty, cannot change hostname"
+    end
+  end
+end

lib/api/v1/registries.rb

@@ -25,6 +25,77 @@ class Registries < Grape::API
           present Registry.all, with: API::Entities::Registries
         end
 
+        desc "Create a registry",
+             tags:     ["registries"],
+             detail:   "Allow users to create a registry. " \
+                       "This will only work if no registry works yet",
+             entity:   API::Entities::Registries,
+             consumes: ["application/x-www-form-urlencoded", "application/json"],
+             failure:  [
+               [400, "Bad request"],
+               [401, "Authentication fails"],
+               [403, "Authorization fails"]
+             ]
+
+        params do
+          requires :registry, type: Hash do
+            requires :all,
+                     only:  %i[name hostname use_ssl],
+                     using: API::Entities::Registries.documentation.slice(
+                       :name, :hostname, :use_ssl
+                     )
+            optional :all,
+                     only:  %i[use_ssl external_hostname],
+                     using: API::Entities::Registries.documentation.slice(:external_hostname)
+          end
+        end
+
+        post do
+          svc = ::Registries::CreateService.new(current_user, permitted_params[:registry])
+          obj = svc.execute
+
+          if svc.valid?
+            present obj, with: API::Entities::Registries
+          else
+            status 400
+            { errors: svc.messages }
+          end
+        end
+
+        desc "Update registry",
+             params:   API::Entities::Registries.documentation.slice(:id),
+             failure:  [
+               [400, "Bad request", API::Entities::ApiErrors],
+               [401, "Authentication fails"],
+               [403, "Authorization fails"],
+               [404, "Not found"]
+             ],
+             entity:   API::Entities::Registries,
+             consumes: ["application/x-www-form-urlencoded", "application/json"]
+
+        params do
+          requires :registry, type: Hash do
+            optional :all,
+                     only:  %i[name hostname use_ssl external_hostname],
+                     using: API::Entities::Registries.documentation.slice(
+                       :name, :hostname, :use_ssl, :external_hostname
+                     )
+          end
+        end
+
+        put ":id" do
+          attrs = declared(params, include_missing: false)[:registry].merge(id: params[:id])
+          svc = ::Registries::UpdateService.new(current_user, attrs)
+          obj = svc.execute
+
+          if svc.valid?
+            present obj, with: API::Entities::Registries
+          else
+            status 400
+            { errors: svc.messages }
+          end
+        end
+
         desc "Validates the given registry",
              tags:    ["registries"],
              detail:  "Besides containing the usual Status object, it adds the reachable " \

spec/api/grape_api/v1/registries_spec.rb

@@ -9,6 +9,116 @@
     @header = build_token_header(token)
   end
 
+  context "POST /api/v1/registries" do
+    let(:data) do
+      {
+        registry: {
+          name:     "registry",
+          hostname: "my.registry.cat",
+          use_ssl:  true
+        }
+      }
+    end
+
+    it "creates a registry" do
+      allow_any_instance_of(Registry).to receive(:reachable?).and_return(nil)
+      post "/api/v1/registries", data, @header
+      expect(response).to have_http_status(:success)
+
+      resp = JSON.parse(response.body)
+      expect(resp["name"]).to eq(data[:registry][:name])
+    end
+
+    it "does not allow to create multiple registries" do
+      create :registry
+
+      allow_any_instance_of(Registry).to receive(:reachable?).and_return(nil)
+      post "/api/v1/registries", data, @header
+      expect(response).to have_http_status(:bad_request)
+
+      resp = JSON.parse(response.body)
+      expect(resp["errors"]["status"]).to eq("You can only create one registry")
+    end
+
+    it "returns an error un unreachable registry" do
+      allow_any_instance_of(Registry).to receive(:reachable?).and_return("Not reachable")
+      post "/api/v1/registries", data, @header
+      expect(response).to have_http_status(:bad_request)
+
+      resp = JSON.parse(response.body)
+      expect(resp["errors"]["hostname"].first).to eq("Not reachable")
+    end
+  end
+
+  context "PUT /api/v1/registries/:id" do
+    let(:data) do
+      {
+        registry: {
+          name:     "registry",
+          hostname: "my.registry.cat",
+          use_ssl:  true
+        }
+      }
+    end
+
+    let(:just_name) do
+      { registry: { name: "newname" } }
+    end
+
+    it "updates a registry" do
+      r = create :registry, hostname: "lala"
+
+      allow_any_instance_of(Registry).to receive(:reachable?).and_return(nil)
+      put "/api/v1/registries/#{r.id}", data, @header
+      expect(response).to have_http_status(:success)
+
+      reg  = Registry.first
+      resp = JSON.parse(response.body)
+
+      expect(resp["hostname"]).to eq(data[:registry][:hostname])
+      expect(reg.hostname).to eq(data[:registry][:hostname])
+    end
+
+    it "does not allow to update the hostname if there are repositories" do
+      r = create :registry
+      namespace = create(:namespace, registry: r, team: create(:team))
+      create(:repository, namespace: namespace)
+
+      allow_any_instance_of(Registry).to receive(:reachable?).and_return(nil)
+      put "/api/v1/registries/#{r.id}", data, @header
+      expect(response).to have_http_status(:bad_request)
+
+      resp = JSON.parse(response.body)
+      expect(resp["errors"]["hostname"]).to eq("Registry is not empty, cannot change hostname")
+    end
+
+    it "allows to update if there are repositories and you don't speak about the hostname" do
+      r = create :registry
+      namespace = create(:namespace, registry: r, team: create(:team))
+      create(:repository, namespace: namespace)
+
+      allow_any_instance_of(Registry).to receive(:reachable?).and_return(nil)
+      put "/api/v1/registries/#{r.id}", just_name, @header
+      expect(response).to have_http_status(:success)
+
+      reg  = Registry.first
+      resp = JSON.parse(response.body)
+      expect(resp["name"]).to eq(just_name[:registry][:name])
+      expect(reg.name).to eq(just_name[:registry][:name])
+    end
+
+    it "returns an error un unreachable registry" do
+      r = create :registry, hostname: "lala"
+
+      allow_any_instance_of(Registry).to receive(:reachable?).and_return("Not reachable")
+      put "/api/v1/registries/#{r.id}", data, @header
+      expect(response).to have_http_status(:bad_request)
+
+      resp = JSON.parse(response.body)
+      expect(resp["errors"]["hostname"].first).to eq("Not reachable")
+    end
+  end
+
   context "GET /api/v1/registries" do
     it "returns list of registries" do
       r = create :registry

spec/controllers/admin/registries_controller_spec.rb

@@ -117,8 +117,7 @@
       expect(response).to have_http_status(:unprocessable_entity)
     end
 
-    it "renders 'edit' with unprocessable entity status (422)
-        when registry is invalid" do
+    it "renders 'edit' with unprocessable entity status (422) when registry is invalid" do
       allow_any_instance_of(Registry).to receive(:reachable?).and_return(nil)
       expect do
         put :update, id: registry.id, registry: { name: "" }