refactoring and cleanup of OGP code and specs

lib/geo_combine/ogp.rb

@@ -1,5 +1,5 @@
 module GeoCombine
-  # Data model for ESRI's open data portal metadata
+  # Data model for OpenGeoPortal metadata
   class OGP
     include GeoCombine::Formatting
     attr_reader :metadata
@@ -26,11 +26,11 @@ def to_geoblacklight
     def geoblacklight_terms
       {
         # Required fields
-        dc_identifier_s: URI.encode(metadata['LayerId']),
-        layer_slug_s: sluggify(metadata['LayerId']),
+        dc_identifier_s: identifier,
+        layer_slug_s: slug,
         dc_title_s: metadata['LayerDisplayName'],
         solr_geom: envelope,
-        dct_provenance_s: metadata['Institution'],
+        dct_provenance_s: institution,
         dc_rights_s: metadata['Access'],
         geoblacklight_version: '1.0',
 
@@ -85,7 +85,7 @@ def ogp_formats
       when 'Polygon', 'Point', 'Line'
         return 'Shapefile'
       else
-        ''
+        raise ArgumentError
       end
     end
 
@@ -100,19 +100,23 @@ def references
     # Builds a Solr Envelope using CQL syntax
     # @return [String]
     def envelope
+      raise ArgumentError unless west >= -180 && west <= 180 &&
+                                 east >= -180 && east <= 180 &&
+                                 north >= -90 && north <= 90 &&
+                                 south >= -90 && south <= 90
       "ENVELOPE(#{west}, #{east}, #{north}, #{south})"
     end
 
     def subjects
-      fgdc.metadata.xpath('//themekey').map { |k| k.text }
+      fgdc.metadata.xpath('//themekey').map { |k| k.text } if fgdc
     end
 
     def placenames
-      fgdc.metadata.xpath('//placekey').map { |k| k.text }
+      fgdc.metadata.xpath('//placekey').map { |k| k.text } if fgdc
     end
 
     def fgdc
-      GeoCombine::Fgdc.new(metadata['FgdcText'])
+      GeoCombine::Fgdc.new(metadata['FgdcText']) if metadata['FgdcText']
     end
 
     private
@@ -137,19 +141,31 @@ def location
     end
 
     def north
-      @metadata['MaxY']
+      metadata['MaxY']
     end
 
     def south
-      @metadata['MinY']
+      metadata['MinY']
     end
 
     def east
-      @metadata['MaxX']
+      metadata['MaxX']
     end
 
     def west
-      @metadata['MinX']
+      metadata['MinX']
+    end
+
+    def institution
+      metadata['Institution']
+    end
+
+    def identifier
+      URI.encode(metadata['LayerId'])
+    end
+
+    def slug
+      sluggify(metadata['LayerId'])
     end
   end
 end

spec/lib/geo_combine/ogp_spec.rb

@@ -2,60 +2,60 @@
 
 RSpec.describe GeoCombine::OGP do
   include JsonDocs
-  let(:ogp_harvard) { GeoCombine::OGP.new(ogp_harvard_raster) }
-  let(:metadata) { ogp_harvard.instance_variable_get(:@metadata) }
+
+  subject(:ogp) { GeoCombine::OGP.new(ogp_harvard_raster) }
   let(:ogp_tufts) { GeoCombine::OGP.new(ogp_tufts_vector) }
-  # let(:metadata) { ogp_harvard.instance_variable_get(:@metadata) }
+  let(:metadata) { ogp.instance_variable_get(:@metadata) }
+
   describe '#initialize' do
     it 'parses JSON into metadata Hash' do
-      expect(ogp_harvard.instance_variable_get(:@metadata)).to be_an Hash
+      expect(metadata).to be_an Hash
     end
   end
+
   describe '#to_geoblacklight' do
     it 'calls geoblacklight_terms to create a GeoBlacklight object' do
-      expect(ogp_harvard).to receive(:geoblacklight_terms).and_return({})
-      expect(ogp_harvard.to_geoblacklight).to be_an GeoCombine::Geoblacklight
+      expect(ogp).to receive(:geoblacklight_terms).and_return({})
+      expect(ogp.to_geoblacklight).to be_an GeoCombine::Geoblacklight
     end
   end
+
   describe '#geoblacklight_terms' do
     describe 'builds a hash which maps metadata' do
       it 'with dc_identifier_s' do
-        expect(ogp_harvard.geoblacklight_terms).to include(dc_identifier_s: metadata['LayerId'])
+        expect(ogp.geoblacklight_terms).to include(dc_identifier_s: metadata['LayerId'])
       end
       it 'with dc_title_s' do
-        expect(ogp_harvard.geoblacklight_terms).to include(dc_title_s: metadata['LayerDisplayName'])
+        expect(ogp.geoblacklight_terms).to include(dc_title_s: metadata['LayerDisplayName'])
       end
       it 'with dc_description_s sanitized' do
-        expect(ogp_harvard.geoblacklight_terms).to include(dc_description_s: metadata['Abstract'])
+        expect(ogp.geoblacklight_terms).to include(dc_description_s: metadata['Abstract'])
       end
       it 'with dc_rights_s' do
-        expect(ogp_harvard.geoblacklight_terms).to include(dc_rights_s: 'Public')
+        expect(ogp.geoblacklight_terms).to include(dc_rights_s: 'Public')
       end
       it 'with dct_provenance_s' do
-        expect(ogp_harvard.geoblacklight_terms).to include(dct_provenance_s: metadata['Institution'])
+        expect(ogp.geoblacklight_terms).to include(dct_provenance_s: metadata['Institution'])
       end
       it 'with dct_references_s' do
-        expect(ogp_harvard).to receive(:references).and_return ''
-        expect(ogp_harvard.geoblacklight_terms).to include(:dct_references_s)
+        expect(ogp.geoblacklight_terms).to include(:dct_references_s)
       end
       it 'with layer_id_s that is blank' do
-        expect(ogp_harvard.geoblacklight_terms)
+        expect(ogp.geoblacklight_terms)
           .to include(layer_id_s: "#{metadata['WorkspaceName']}:#{metadata['Name']}")
       end
       it 'with layer_geom_type_s' do
-        expect(ogp_harvard).to receive(:ogp_geom).and_return ''
-        expect(ogp_harvard.geoblacklight_terms).to include(:layer_geom_type_s)
+        expect(ogp.geoblacklight_terms).to include(:layer_geom_type_s)
       end
       it 'with layer_slug_s' do
-        expect(ogp_harvard.geoblacklight_terms)
+        expect(ogp.geoblacklight_terms)
           .to include(layer_slug_s: 'harvard-sde2-g1059-w57-1654-pf-sh1')
       end
       it 'with solr_geom' do
-        expect(ogp_harvard).to receive(:envelope).and_return ''
-        expect(ogp_harvard.geoblacklight_terms).to include(solr_geom: '')
+        expect(ogp.geoblacklight_terms).to include(:solr_geom)
       end
       it 'with dc_subject_sm' do
-        expect(ogp_harvard.geoblacklight_terms).to include(
+        expect(ogp.geoblacklight_terms).to include(
           dc_subject_sm: [
             'Maps', 'Human settlements', 'Cities and towns', 'Villages',
             'Bodies of water', 'Landforms', 'Transportation',
@@ -64,7 +64,7 @@
         )
       end
       it 'with dct_spatial_sm' do
-        expect(ogp_harvard.geoblacklight_terms).to include(
+        expect(ogp.geoblacklight_terms).to include(
           dct_spatial_sm: [
             'Earth', 'Northern Hemisphere', 'Southern Hemisphere',
             'Eastern Hemisphere', 'Western Hemisphere', 'Africa', 'Asia',
@@ -75,60 +75,74 @@
       end
     end
   end
+
   describe '#ogp_geom' do
     it 'when Paper Map use Raster' do
-      expect(ogp_harvard.ogp_geom).to eq 'Raster'
+      expect(ogp.ogp_geom).to eq 'Raster'
     end
     it 'anything else, return it' do
       expect(ogp_tufts.ogp_geom).to eq 'Polygon'
     end
   end
+
   describe '#ogp_formats' do
     context 'when Paper Map or Raster' do
       it 'returns GeoTIFF' do
-        expect(ogp_harvard).to receive(:metadata).and_return('DataType' => 'Raster')
-        expect(ogp_harvard.ogp_formats).to eq 'GeoTIFF'
-        expect(ogp_harvard).to receive(:metadata).and_return('DataType' => 'Paper Map')
-        expect(ogp_harvard.ogp_formats).to eq 'GeoTIFF'
+        %w[Raster Paper\ Map].each do |datatype|
+          expect(ogp).to receive(:metadata).and_return('DataType' => datatype)
+          expect(ogp.ogp_formats).to eq 'GeoTIFF'
+        end
+
       end
     end
     context 'when Polygon, Line, or Point' do
       it 'returns Shapefile' do
-        expect(ogp_harvard).to receive(:metadata).and_return('DataType' => 'Polygon')
-        expect(ogp_harvard.ogp_formats).to eq 'Shapefile'
-        expect(ogp_harvard).to receive(:metadata).and_return('DataType' => 'Line')
-        expect(ogp_harvard.ogp_formats).to eq 'Shapefile'
-        expect(ogp_harvard).to receive(:metadata).and_return('DataType' => 'Point')
-        expect(ogp_harvard.ogp_formats).to eq 'Shapefile'
+        %w[Polygon Line Point].each do |datatype|
+          expect(ogp).to receive(:metadata).and_return('DataType' => datatype)
+          expect(ogp.ogp_formats).to eq 'Shapefile'
+        end
+      end
+    end
+    context 'unknown data types' do
+      it 'throws exception' do
+        expect(ogp).to receive(:metadata).and_return('DataType' => 'Unknown')
+        expect { ogp.ogp_formats }.to raise_error(ArgumentError)
       end
     end
   end
+
   describe '#envelope' do
     it 'properly formatted envelope' do
-      expect(ogp_harvard.envelope).to eq 'ENVELOPE(-180, 180, 90, -90)'
+      expect(ogp.envelope).to eq 'ENVELOPE(-180, 180, 90, -90)'
+    end
+    it 'fails on out-of-bounds envelopes' do
+      expect(ogp).to receive(:west).and_return(-181)
+      expect { ogp.envelope }.to raise_error(ArgumentError)
     end
   end
+
   describe '#references' do
     context 'harvard raster' do
-      it do
-        expect(JSON.parse(ogp_harvard.references)).to include(
-          'http://www.opengis.net/def/serviceType/ogc/wms' => 'http://pelham.lib.harvard.edu:8090/geoserver/wms',
+      it 'has wms and download services' do
+        expect(JSON.parse(ogp.references)).to include(
+          'http://www.opengis.net/def/serviceType/ogc/wms' => 'http://pelham.lib.harvard.edu:8080/geoserver/wms',
           'http://schema.org/DownloadAction' => 'http://pelham.lib.harvard.edu:8080/HGL/HGLOpenDelivery'
         )
       end
     end
     context 'tufts vector' do
-      it do
+      it 'has wms wfs services' do
         expect(JSON.parse(ogp_tufts.references)).to include(
           'http://www.opengis.net/def/serviceType/ogc/wms' => 'http://geoserver01.uit.tufts.edu/wms',
           'http://www.opengis.net/def/serviceType/ogc/wfs' => 'http://geoserver01.uit.tufts.edu/wfs'
         )
       end
     end
   end
+
   describe 'valid geoblacklight schema' do
     context 'harvard' do
-      it { expect { ogp_harvard.to_geoblacklight.valid? }.to_not raise_error }
+      it { expect { ogp.to_geoblacklight.valid? }.to_not raise_error }
     end
     context 'tufts' do
       it { expect { ogp_tufts.to_geoblacklight.valid? }.to_not raise_error }