Fixes option passing in the watcher command

lib/gingr/cli.rb

@@ -23,7 +23,7 @@ class Cli < Thor
     option :geoserver_secure_url
     def watch(root_dir = nil)
       root_dir ||= ENV['GINGR_WATCH_DIRECTORY'] || '/opt/app/data/gingr'
-      watcher = Gingr::Watcher.new(root_dir, *options)
+      watcher = Gingr::Watcher.new(root_dir, options)
       watcher.start!
     end
 

lib/gingr/watcher.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 require 'listen'
 require 'open3'
+require 'thor'
 require_relative 'logging'
 
 module Gingr
@@ -23,7 +24,7 @@ class Watcher
     attr_reader :options
     attr_reader :root_dir
 
-    def initialize(root_dir, *options)
+    def initialize(root_dir, options = {})
       # This is the Gingr root directory, not the directory to be watched.
       # Watcher watches the ./ready directory under this one.
       @root_dir = root_dir
@@ -35,6 +36,13 @@ def initialize(root_dir, *options)
       validate_directories!
     end
 
+    # We receive parsed options from Thor but then have to pass them back to the CLI
+    # un-parsed, so we essentially reverse option-parsing here. Returns an Array so
+    # it's easier to pass to Open3.capture3.
+    def arguments
+      options.to_h { |k, v| [dasherize(k.to_s), v.to_s] }.to_a.flatten
+    end
+
     def start!
       start
       sleep
@@ -63,7 +71,7 @@ def listener
 
     def exec_gingr_all!(zipfile)
       begin
-        command = ['gingr', 'all', zipfile, *options]
+        command = ['gingr', 'all', zipfile, *arguments]
         logger.debug("Running command: #{command}")
 
         stdout, stderr, status = Open3.capture3(*command)
@@ -94,6 +102,10 @@ def failed_dir
 
     private
 
+    def dasherize(str)
+      (str.length > 1 ? "--" : "-") + str.tr("_", "-")
+    end
+
     def collate_logs(stdout, stderr)
       "#{stdout}\n#{stderr}\n"
     end

spec/watcher_spec.rb

@@ -27,32 +27,49 @@ def success?
 RSpec.describe Gingr::Watcher do
   before(:each) { FileUtils.rm_rf Dir.glob('/opt/app/data/gingr/*/*') }
 
-  context 'a valid watcher' do
-    subject :watcher do
-      Gingr::Watcher.new(
-        '/opt/app/data/gingr',
-        '--solr-url=http://solr:8983/solr/geodata-test',
-        '--geoserver-url=http://admin:geoserver@geoserver:8080/geoserver/rest/',
-        '--geoserver-root=/opt/app/data/geoserver',
-        '--geoserver-secure-root=/opt/app/data/geoserver',
-        '--spatial-root=/opt/app/data/spatial',
-        '--update-reference-field',
-      )
-    end
+  subject(:watcher) { Gingr::Watcher.new('/opt/app/data/gingr', options) }
+
+  let(:options) do
+    {
+      geoserver_root: '/opt/app/data/geoserver',
+      geoserver_secure_url: 'http://admin:geoserver@geoserver-secure:8080/geoserver/rest/',
+      geoserver_url: 'http://admin:geoserver@geoserver:8080/geoserver/rest/',
+      solr_url: 'http://solr:8983/solr/geodata-test',
+      spatial_root: '/opt/app/data/spatial',
+      update_reference_field: true,
+    }
+  end
 
-    it 'passes arguments to `gingr all`' do
-      expect(Open3).to receive(:capture3).with('gingr', 'all',
-        '/opt/app/data/gingr/ready/vector.zip',
-        '--solr-url=http://solr:8983/solr/geodata-test',
-        '--geoserver-url=http://admin:geoserver@geoserver:8080/geoserver/rest/',
-        '--geoserver-root=/opt/app/data/geoserver',
-        '--geoserver-secure-root=/opt/app/data/geoserver',
-        '--spatial-root=/opt/app/data/spatial',
-        '--update-reference-field',
-      ).and_return(['', '', MockStatus.successful])
+  context 'a valid watcher' do
+    describe 'option handling' do
+      it 'parses a hash of options into a list of CLI arguments' do
+        expect(watcher.arguments).to eq %w(
+          --geoserver-root /opt/app/data/geoserver
+          --geoserver-secure-url http://admin:geoserver@geoserver-secure:8080/geoserver/rest/
+          --geoserver-url http://admin:geoserver@geoserver:8080/geoserver/rest/
+          --solr-url http://solr:8983/solr/geodata-test
+          --spatial-root /opt/app/data/spatial
+          --update-reference-field true
+        )
+
+        watcher.options[:update_reference_field] = false
+        expect(watcher.arguments.last(2)).to eq %w(--update-reference-field false)
+      end
 
-      copy_zipfile_to_ready('vector.zip')
-      watcher.exec_gingr_all!('/opt/app/data/gingr/ready/vector.zip')
+      it 'passes arguments to `gingr all`' do
+        expect(Open3).to receive(:capture3).with(*%w(
+          gingr all /opt/app/data/gingr/ready/vector.zip
+            --geoserver-root /opt/app/data/geoserver
+            --geoserver-secure-url http://admin:geoserver@geoserver-secure:8080/geoserver/rest/
+            --geoserver-url http://admin:geoserver@geoserver:8080/geoserver/rest/
+            --solr-url http://solr:8983/solr/geodata-test
+            --spatial-root /opt/app/data/spatial
+            --update-reference-field true
+        )).and_return(['', '', MockStatus.successful])
+
+        copy_zipfile_to_ready('vector.zip')
+        watcher.exec_gingr_all!('/opt/app/data/gingr/ready/vector.zip')
+      end
     end
 
     it 'moves successfully processed files to the processed directory' do
@@ -66,43 +83,45 @@ def success?
     it 'processes newly added files and keeps processing on error' do
       watcher.start
 
-      (1..5).each do |i|
-        expection = expect(watcher)
-          .to receive(:exec_gingr_all!)
-          .with "/opt/app/data/gingr/ready/vector#{i}.zip"
-        expection.and_raise if i.odd?
+      (1..4).each do |i|
+        exp = expect(Open3).to receive(:capture3).with(*%W(
+          gingr all /opt/app/data/gingr/ready/vector#{i}.zip
+            --geoserver-root /opt/app/data/geoserver
+            --geoserver-secure-url http://admin:geoserver@geoserver-secure:8080/geoserver/rest/
+            --geoserver-url http://admin:geoserver@geoserver:8080/geoserver/rest/
+            --solr-url http://solr:8983/solr/geodata-test
+            --spatial-root /opt/app/data/spatial
+            --update-reference-field true
+        ))
+
+        if i.odd?
+          exp.and_raise(Gingr::Watcher::SubprocessError)
+        else
+          exp.and_return(['', '', MockStatus.successful])
+        end
 
         copy_zipfile_to_ready('vector.zip', "vector#{i}.zip")
-        sleep 3
+        sleep 2
       end
     end
   end
 
-  context 'a watcher with invalid arguments' do
-    subject :watcher do
-      Gingr::Watcher.new(
-        '/opt/app/data/gingr',
-        '--solr-url=http://solr:8983/solr/geodata-test',
-        '--geoserver-url=http://admin:geoserver@geoserver:8081/geoserver/rest/',
-        '--invalid-argument'
-      )
-    end
+  context 'a watcher with invalid options' do
+    let(:options) { { unexpected_argument: true } }
 
     it 'moves failed files and the logs to the failed directory' do
+      expect(Open3).to receive(:capture3).and_return(['', 'Unknown switches', MockStatus.failed])
       copy_zipfile_to_ready('vector.zip')
 
-      expect(Open3).to receive(:capture3).and_return(['', 'Unknown switches', MockStatus.failed])
-      expect { watcher.exec_gingr_all!('/opt/app/data/gingr/ready/vector.zip') }.to raise_error Gingr::Watcher::SubprocessError
+      expect { watcher.exec_gingr_all!('/opt/app/data/gingr/ready/vector.zip') }.to raise_error(Gingr::Watcher::SubprocessError)
       expect(File).to exist('/opt/app/data/gingr/failed/vector.zip')
       expect(File).to exist('/opt/app/data/gingr/failed/vector.log')
       expect(File.read('/opt/app/data/gingr/failed/vector.log')).to match(/Unknown switches/)
     end
   end
 
-  context 'a watcher that cannot write to its watch dirs' do
-    subject :watcher do
-      Gingr::Watcher.new('/opt/app/data/gingr-does-not-exist')
-    end
+  context 'a watcher with an invalid root directory' do
+    subject(:watcher) { Gingr::Watcher.new('/path/does-not-exist') }
 
     it 'fails to initialize' do
       expect { watcher }.to raise_error(Gingr::Watcher::DirectoryError)