From 003a51d17aea6af92953f7207e2b39a5cb6db8de Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 30 Mar 2017 13:45:54 +0200
Subject: [PATCH] Check container repository exists for a given path

---
 lib/container_registry/path.rb                | 11 +++++--
 spec/lib/container_registry/path_spec.rb      | 30 +++++++++++++++++-
 .../create_repository_service_spec.rb         | 31 ++++++++++---------
 3 files changed, 54 insertions(+), 18 deletions(-)

diff --git a/lib/container_registry/path.rb b/lib/container_registry/path.rb
index d3f8bf74e14..3a6fde08e8f 100644
--- a/lib/container_registry/path.rb
+++ b/lib/container_registry/path.rb
@@ -33,8 +33,15 @@ module ContainerRegistry
       end
     end
 
+    def has_project?
+      repository_project.present?
+    end
+
     def has_repository?
-      # ContainerRepository.find_by_full_path(@path).present?
+      return false unless has_project?
+
+      repository_project.container_repositories
+        .where(name: repository_name).any?
     end
 
     def repository_project
@@ -42,7 +49,7 @@ module ContainerRegistry
     end
 
     def repository_name
-      return unless repository_project
+      return unless has_project?
 
       @path.remove(%r(^?#{Regexp.escape(repository_project.full_path)}/?))
     end
diff --git a/spec/lib/container_registry/path_spec.rb b/spec/lib/container_registry/path_spec.rb
index 6384850eb19..906dd920031 100644
--- a/spec/lib/container_registry/path_spec.rb
+++ b/spec/lib/container_registry/path_spec.rb
@@ -39,7 +39,7 @@ describe ContainerRegistry::Path do
       it 'is not valid' do
         expect(subject).not_to be_valid
       end
-    end
+   end
 
     context 'when path has more than allowed number of components' do
       let(:path) { 'a/b/c/d/e/f/g/h/i/j/k/l/m/n/o/p/r/s/t/u/w/y/z' }
@@ -58,6 +58,34 @@ describe ContainerRegistry::Path do
     end
   end
 
+  describe '#has_repository?' do
+    context 'when project exists' do
+      let(:project) { create(:empty_project) }
+      let(:path) { "#{project.full_path}/my/image" }
+
+      context 'when path already has matching repository' do
+        before do
+          create(:container_repository, project: project, name: 'my/image')
+        end
+
+        it { is_expected.to have_repository }
+        it { is_expected.to have_project }
+      end
+
+      context 'when path does not have matching repository' do
+        it { is_expected.not_to have_repository }
+        it { is_expected.to have_project }
+      end
+    end
+
+    context 'when project does not exist' do
+      let(:path) { 'some/project/my/image' }
+
+      it { is_expected.not_to have_repository }
+      it { is_expected.not_to have_project }
+    end
+  end
+
   describe '#repository_project' do
     let(:group) { create(:group, path: 'some_group') }
 
diff --git a/spec/services/container_registry/create_repository_service_spec.rb b/spec/services/container_registry/create_repository_service_spec.rb
index dfd07c8cc02..ac7b147f92e 100644
--- a/spec/services/container_registry/create_repository_service_spec.rb
+++ b/spec/services/container_registry/create_repository_service_spec.rb
@@ -14,18 +14,6 @@ describe ContainerRegistry::CreateRepositoryService, '#execute' do
     stub_container_registry_config(enabled: true)
   end
 
-  context 'when container repository already exists' do
-    before do
-      create(:container_repository, project: project, name: 'my/image')
-    end
-
-    it 'does not create container repository again' do
-      expect { service.execute(path) }
-        .to raise_error(Gitlab::Access::AccessDeniedError)
-        .and change { ContainerRepository.count }.by(0)
-    end
-  end
-
   context 'when repository is created by an user' do
     context 'when user has no ability to create a repository' do
       it 'does not create a new container repository' do
@@ -40,9 +28,22 @@ describe ContainerRegistry::CreateRepositoryService, '#execute' do
         project.add_developer(user)
       end
 
-      it 'creates a new container repository' do
-        expect { service.execute(path) }
-          .to change { project.container_repositories.count }.by(1)
+      context 'when repository already exists' do
+        before do
+          create(:container_repository, project: project, name: 'my/image')
+        end
+
+        it 'does not create container repository again' do
+          expect { service.execute(path) }
+            .to_not change { ContainerRepository.count }
+        end
+      end
+
+      context 'when repository does not exist yet' do
+        it 'creates a new container repository' do
+          expect { service.execute(path) }
+            .to change { project.container_repositories.count }.by(1)
+        end
       end
     end
   end