Clean up naming and usage of validation predicates

This clarifies the difference between validate! methods that
should raise, and the valid? method that does not raise.

Placates Rubocop.

Author: thatbudakguy

lib/geo_combine/bounding_box.rb

@@ -48,8 +48,6 @@ def valid?
     end
 
     def self.from_envelope(envelope)
-      return if envelope.nil?
-
       envelope = envelope[/.*ENVELOPE\(([^)]*)/, 1].split(',')
       new(
         west: envelope[0],
@@ -63,8 +61,6 @@ def self.from_envelope(envelope)
     # @param [String] spatial w,s,e,n or w s e n
     # @param [String] delimiter "," or " "
     def self.from_string_delimiter(spatial, delimiter: ',')
-      return if spatial.nil?
-
       spatial = spatial.split(delimiter)
       new(
         west: spatial[0],

lib/geo_combine/esri_open_data.rb

@@ -4,6 +4,7 @@ module GeoCombine
   # Data model for ESRI's open data portal metadata
   class EsriOpenData
     include GeoCombine::Formatting
+
     attr_reader :metadata
 
     ##

lib/geo_combine/geoblacklight.rb

@@ -55,20 +55,29 @@ def to_json(options = {})
       metadata.to_json(options)
     end
 
+    # True if the GeoBlacklight-Schema document is valid
+    # @return [Boolean]
+    def valid?
+      validate!
+      true
+    rescue StandardError
+      false
+    end
+
     ##
     # Validates a GeoBlacklight-Schema json document
     # @return [Boolean]
-    def valid?
-      JSON::Validator.validate!(schema, to_json, fragment: '#/definitions/layer') &&
-        dct_references_validate! &&
-        spatial_validate!
+    def validate!
+      JSON::Validator.validate!(schema, to_json, fragment: '#/definitions/layer')
+      validate_references!
+      validate_spatial!
     end
 
     ##
     # Validate dct_references_s
     # @return [Boolean]
-    def dct_references_validate!
-      return true unless metadata.key?('dct_references_s') # TODO: shouldn't we require this field?
+    def validate_references!
+      return unless metadata.key?('dct_references_s') # Not required
 
       begin
         ref = JSON.parse(metadata['dct_references_s'])
@@ -83,8 +92,8 @@ def dct_references_validate!
       end
     end
 
-    def spatial_validate!
-      GeoCombine::BoundingBox.from_envelope(metadata['solr_geom']).valid?
+    def validate_spatial!
+      raise GeoCombine::Exceptions::InvalidGeometry unless GeoCombine::BoundingBox.from_envelope(metadata['solr_geom']).valid?
     end
 
     private
@@ -163,7 +172,7 @@ def upgrade_to_v1
       metadata.except!(*DEPRECATED_KEYS_V1)
 
       # ensure we have a proper v1 record
-      valid?
+      validate!
     end
 
     def schema

lib/geo_combine/ogp.rb

@@ -8,6 +8,7 @@ module GeoCombine
   class OGP
     class InvalidMetadata < RuntimeError; end
     include GeoCombine::Formatting
+
     attr_reader :metadata
 
     ##

spec/features/fgdc2html_spec.rb

@@ -5,6 +5,7 @@
 # TODO: Provide additional expectations on html structure
 describe 'FGDC to html' do
   include XmlDocs
+
   let(:page) { GeoCombine::Fgdc.new(tufts_fgdc).to_html }
 
   describe 'Identification Information' do

spec/features/iso2html_spec.rb

@@ -5,6 +5,7 @@
 # TODO: Provide additional expectations on html structure
 describe 'ISO 19139 to html' do
   include XmlDocs
+
   let(:page) { GeoCombine::Iso19139.new(stanford_iso).to_html }
 
   describe 'Identification Information' do

spec/lib/geo_combine/ckan_metadata_spec.rb

@@ -4,6 +4,7 @@
 
 RSpec.describe GeoCombine::CkanMetadata do
   include JsonDocs
+
   let(:ckan_sample) { described_class.new(ckan_metadata) }
 
   describe '#to_geoblacklight' do

spec/lib/geo_combine/esri_open_data_spec.rb

@@ -4,6 +4,7 @@
 
 RSpec.describe GeoCombine::EsriOpenData do
   include JsonDocs
+
   let(:esri_sample) { described_class.new(esri_opendata_metadata) }
   let(:metadata) { esri_sample.instance_variable_get(:@metadata) }
 

spec/lib/geo_combine/fgdc_spec.rb

@@ -4,6 +4,7 @@
 
 RSpec.describe GeoCombine::Fgdc do
   include XmlDocs
+
   let(:fgdc_object) { described_class.new(tufts_fgdc) }
 
   describe '#initialize' do
@@ -32,7 +33,7 @@
     end
 
     it 'is not valid due to bad modification date but valid otherwise' do
-      expect { fgdc_geobl.valid? }.to raise_error(JSON::Schema::ValidationError, /layer_modified_dt/)
+      expect { fgdc_geobl.validate! }.to raise_error(JSON::Schema::ValidationError, /layer_modified_dt/)
       fgdc_geobl.metadata.delete 'layer_modified_dt'
       expect(fgdc_geobl).to be_valid
     end

spec/lib/geo_combine/geoblacklight_spec.rb

@@ -6,6 +6,7 @@
   include XmlDocs
   include JsonDocs
   include GeoCombine::Exceptions
+
   let(:full_geobl) { described_class.new(full_geoblacklight) }
   let(:enhanced_geobl) { described_class.new(basic_geoblacklight, 'layer_geom_type_s' => 'esriGeometryPolygon') }
   let(:basic_geobl) { described_class.new(basic_geoblacklight) }
@@ -62,9 +63,9 @@
     end
   end
 
-  describe '#valid?' do
+  describe '#validate!' do
     it 'a valid geoblacklight-schema document should be valid' do
-      expect(full_geobl.valid?).to be true
+      expect(full_geobl).to be_valid
     end
 
     context 'must have required fields' do
@@ -78,7 +79,7 @@
       ].each do |field|
         it field do
           full_geobl.metadata.delete field
-          expect { full_geobl.valid? }.to raise_error(JSON::Schema::ValidationError, /#{field}/)
+          expect { full_geobl.validate! }.to raise_error(JSON::Schema::ValidationError, /#{field}/)
         end
       end
     end
@@ -104,7 +105,7 @@
       ].each do |field|
         it field do
           full_geobl.metadata.delete field
-          expect { full_geobl.valid? }.not_to raise_error
+          expect { full_geobl.validate! }.not_to raise_error
         end
       end
     end
@@ -119,31 +120,25 @@
       ].each do |field|
         it field do
           full_geobl.metadata.delete field
-          expect { full_geobl.valid? }.not_to raise_error
+          expect { full_geobl.validate! }.not_to raise_error
         end
       end
     end
 
     it 'an invalid document' do
-      expect { basic_geobl.valid? }.to raise_error JSON::Schema::ValidationError
+      expect { basic_geobl.validate! }.to raise_error JSON::Schema::ValidationError
     end
 
     it 'calls the dct_references_s validator' do
-      expect(enhanced_geobl).to receive(:dct_references_validate!)
+      expect(enhanced_geobl).to receive(:validate_references!)
       enhanced_geobl.valid?
     end
-
-    it 'validates spatial bounding box' do
-      expect(JSON::Validator).to receive(:validate!).and_return true
-      expect { basic_geobl.valid? }
-        .to raise_error GeoCombine::Exceptions::InvalidGeometry
-    end
   end
 
-  describe '#dct_references_validate!' do
+  describe '#validate_references!' do
     context 'with valid document' do
       it 'is valid' do
-        expect(full_geobl.dct_references_validate!).to be true
+        expect { full_geobl.validate_references! }.not_to raise_error
       end
     end
 
@@ -173,22 +168,22 @@
       end
 
       it 'unparseable json' do
-        expect { bad_ref.dct_references_validate! }.to raise_error JSON::ParserError
+        expect { bad_ref.validate_references! }.to raise_error JSON::ParserError
       end
 
       it 'not a hash' do
-        expect { not_hash.dct_references_validate! }.to raise_error GeoCombine::Exceptions::InvalidDCTReferences
+        expect { not_hash.validate_references! }.to raise_error GeoCombine::Exceptions::InvalidDCTReferences
       end
     end
   end
 
-  describe 'spatial_validate!' do
+  describe 'validate_spatial!' do
     context 'when valid' do
-      it { expect { full_geobl.spatial_validate! }.not_to raise_error }
+      it { expect { full_geobl.validate_spatial! }.not_to raise_error }
     end
 
     context 'when invalid' do
-      it { expect { basic_geobl.spatial_validate! }.to raise_error GeoCombine::Exceptions::InvalidGeometry }
+      it { expect { basic_geobl.validate_spatial! }.to raise_error GeoCombine::Exceptions::InvalidGeometry }
     end
   end