Merge pull request #148 from OpenGeoMetadata/prevent-nested-clones

Avoid nesting cloned OGM repos inside themselves

lib/geo_combine/harvester.rb

@@ -53,27 +53,35 @@ def docs_to_index
       end
     end
 
-    # Update a repository via git, or all repositories if none specified.
-    # If the repository doesn't exist, clone it. Return the count of repositories updated.
-    def pull(repo = nil)
-      return repositories.map(&method(:pull)).reduce(:+) unless repo
-
+    # Update a repository via git
+    # If the repository doesn't exist, clone it.
+    def pull(repo)
       repo_path = File.join(@ogm_path, repo)
       clone(repo) unless File.directory? repo_path
 
       Git.open(repo_path).pull && 1
     end
 
-    # Clone a repository via git, or all repositories if none specified.
-    # If the repository already exists, skip it. Return the count of repositories cloned.
-    def clone(repo = nil)
-      return repositories.map(&method(:clone)).reduce(:+) unless repo
+    # Update all repositories
+    # Return the count of repositories updated
+    def pull_all
+      repositories.map(&method(:pull)).reduce(:+)
+    end
 
+    # Clone a repository via git
+    # If the repository already exists, skip it.
+    def clone(repo)
       repo_path = File.join(@ogm_path, repo)
       return 0 if File.directory? repo_path
 
       repo_url = "https://github.com/OpenGeoMetadata/#{repo}.git"
-      Git.clone(repo_url, repo, path: repo_path, depth: 1) && 1
+      Git.clone(repo_url, nil, path: ogm_path, depth: 1) && 1
+    end
+
+    # Clone all repositories via git
+    # Return the count of repositories cloned.
+    def clone_all
+      repositories.map(&method(:clone)).reduce(:+)
     end
 
     private

lib/tasks/geo_combine.rake

@@ -12,14 +12,14 @@ namespace :geocombine do
   desc 'Clone OpenGeoMetadata repositories'
   task :clone, [:repo] do |_t, args|
     harvester = GeoCombine::Harvester.new
-    total = harvester.clone(args.repo)
+    total = args[:repo] ? harvester.clone(args.repo) : harvester.clone_all
     puts "Cloned #{total} repositories"
   end
 
   desc '"git pull" OpenGeoMetadata repositories'
   task :pull, [:repo] do |_t, args|
     harvester = GeoCombine::Harvester.new
-    total = harvester.pull(args.repo)
+    total = args[:repo] ? harvester.pull(args.repo) : harvester.pull_all
     puts "Updated #{total} repositories"
   end
 

spec/lib/geo_combine/harvester_spec.rb

@@ -49,24 +49,26 @@
       expect(stub_repo).to have_received(:pull)
     end
 
+    it 'clones a repo before pulling if it does not exist' do
+      harvester.pull(repo_name)
+      expect(Git).to have_received(:clone)
+    end
+  end
+
+  describe '#pull_all' do
     it 'can pull all repositories' do
-      harvester.pull
+      harvester.pull_all
       expect(Git).to have_received(:open).exactly(2).times
       expect(stub_repo).to have_received(:pull).exactly(2).times
     end
 
-    it 'skips repositories in the denylist' do
-      harvester.pull
-      expect(Git).not_to have_received(:open).with('https://github.com/OpenGeoMetadata/aardvark.git')
-    end
-
-    it 'clones a repo before pulling if it does not exist' do
-      harvester.pull(repo_name)
-      expect(Git).to have_received(:clone)
+    it 'returns the count of repositories pulled' do
+      expect(harvester.pull_all).to eq(2)
     end
 
-    it 'returns the count of repositories pulled' do
-      expect(harvester.pull).to eq(2)
+    it 'skips repositories in the denylist' do
+      harvester.pull_all
+      expect(Git).not_to have_received(:open).with('https://github.com/OpenGeoMetadata/aardvark.git')
     end
   end
 
@@ -75,31 +77,33 @@
       harvester.clone(repo_name)
       expect(Git).to have_received(:clone).with(
         repo_url,
-        repo_name, {
+        nil, {
           depth: 1, # shallow clone
-          path: repo_path
+          path: harvester.ogm_path
         }
       )
     end
 
+    it 'skips repositories that already exist' do
+      allow(File).to receive(:directory?).with(repo_path).and_return(true)
+      harvester.clone(repo_name)
+      expect(Git).not_to have_received(:clone)
+    end
+  end
+
+  describe '#clone_all' do
     it 'can clone all repositories' do
-      harvester.clone
+      harvester.clone_all
       expect(Git).to have_received(:clone).exactly(2).times
     end
 
     it 'skips repositories in the denylist' do
-      harvester.pull
+      harvester.clone_all
       expect(Git).not_to have_received(:clone).with('https://github.com/OpenGeoMetadata/aardvark.git')
     end
 
-    it 'skips repositories that already exist' do
-      allow(File).to receive(:directory?).with(repo_path).and_return(true)
-      harvester.clone(repo_name)
-      expect(Git).not_to have_received(:clone)
-    end
-
     it 'returns the count of repositories cloned' do
-      expect(harvester.clone).to eq(2)
+      expect(harvester.clone_all).to eq(2)
     end
   end
 end