Added better validations for the weather agent.

app/models/agents/weather_agent.rb

@@ -88,6 +88,16 @@ def weather_provider
       interpolated["service"].presence || "wunderground"
     end
 
+    # a check to see if the weather provider is wunderground.
+    def wunderground?
+      weather_provider.downcase == "wunderground"
+    end
+
+    # a check to see if the weather provider is one of the valid aliases for Dark Sky.
+    def dark_sky?
+      ["dark_sky", "darksky", "forecast_io", "forecastio"].include? weather_provider.downcase
+    end
+
     def which_day
       (interpolated["which_day"].presence || 1).to_i
     end
@@ -96,13 +106,37 @@ def location
       interpolated["location"].presence || interpolated["zipcode"]
     end
 
+    def coordinates
+      location.split(',').map { |e| e.to_f }
+    end
+
     def language
       interpolated['language'].presence || 'EN'
     end
 
-    def validate_options
-      errors.add(:base, "service must be set to 'darksky' or 'wunderground'") unless %w[darksky forecastio wunderground].include?(weather_provider)
+    VALID_COORDS_REGEX = /^\s*-?\d{1,3}\.\d+\s*,\s*-?\d{1,3}\.\d+\s*$/
+
+    def validate_location
       errors.add(:base, "location is required") unless location.present?
+      return if wunderground?
+      if location.match? VALID_COORDS_REGEX
+        lat, lon = coordinates
+        errors.add :base, "too low of a latitude" unless lat > -90
+        errors.add :base, "too big of a latitude" unless lat < 90
+        errors.add :base, "too low of a longitude" unless lon > -180
+        errors.add :base, "too high of a longitude" unless lon < 180
+      else
+        errors.add(
+          :base,
+          "Location #{location} is malformed. Location for " +
+          'Dark Sky must be in the format "-00.000,-00.00000". The ' +
+          "number of decimal places does not matter.")
+      end
+    end
+
+    def validate_options
+      errors.add(:base, "service must be set to 'darksky' or 'wunderground'") unless wunderground? || dark_sky?
+      validate_location
       errors.add(:base, "api_key is required") unless interpolated['api_key'].present?
       errors.add(:base, "which_day selection is required") unless which_day.present?
     end
@@ -120,15 +154,15 @@ def wunderground
     def dark_sky
       if key_setup?
         ForecastIO.api_key = interpolated['api_key']
-        lat, lng = location.split(',')
+        lat, lng = coordinates
         ForecastIO.forecast(lat, lng, params: {lang: language.downcase})['daily']['data']
       end
     end
 
     def model(weather_provider,which_day)
-      if weather_provider == "wunderground"
+      if wunderground?
         wunderground[which_day]
-      elsif weather_provider == "darksky" || weather_provider == "forecastio"
+      elsif dark_sky?
         dark_sky.each do |value|
           timestamp = Time.at(value.time)
           if (timestamp.to_date - Time.now.to_date).to_i == which_day

spec/models/agents/weather_agent_spec.rb

@@ -4,18 +4,32 @@
   let(:agent) do
     Agents::WeatherAgent.create(
       name: 'weather',
-      options: { 
-        :location => 94103, 
-        :lat => 37.779329, 
-        :lng => -122.41915, 
-        :api_key => 'test' 
+      options: {
+        :location => 94103,
+        :api_key => 'test',
+        :which_day => 1,
       }
     ).tap do |agent|
-      agent.user = users(:bob)  
+      agent.user = users(:bob)
       agent.save!
     end
   end
-
+
+  let :dark_sky_agent do
+    Agents::WeatherAgent.create(
+      name: "weather from dark sky",
+      options: {
+          :location => "37.779329,-122.41915",
+          :service => "darksky",
+          :which_day => 1,
+          :api_key => "test"
+      }
+    ).tap do |agent|
+      agent.user = users(:bob)
+      agent.save!
+    end
+  end
+
   it "creates a valid agent" do
     expect(agent).to be_valid
   end
@@ -29,10 +43,58 @@
     expect(agent).to be_valid
     expect(agent.working?).to be_falsey
   end
-
+
+  context "wunderground" do
+    it "validates the location properly" do
+      expect(agent.options["location"]).to eq 94103
+      agent.options["location"] = "meh, really whatever, this is on the way to deprecation anyway"
+      expect(agent).to be_valid
+      agent.options["location"] = nil
+      expect(agent).to_not be_valid
+      agent.options["location"] = 94103
+    end
+  end
+
+  context "dark sky" do
+    it "validates the location properly" do
+      expect(dark_sky_agent.options["location"]).to eq "37.779329,-122.41915"
+      expect(dark_sky_agent).to be_valid
+      dark_sky_agent.options["location"] = "37.779329, -122.41915" # with a space
+      expect(dark_sky_agent).to be_valid
+      dark_sky_agent.options["location"] = "94103" # a zip code
+      expect(dark_sky_agent).to_not be_valid
+      dark_sky_agent.options["location"] = "37.779329,-122.41915"
+      expect(dark_sky_agent.options["location"]).to eq "37.779329,-122.41915"
+      expect(dark_sky_agent).to be_valid
+    end
+    it "fails cases that pass the first test but are invalid" do
+      dark_sky_agent.options["location"] = "137.779329, -122.41915" # too high latitude
+      expect(dark_sky_agent).to_not be_valid
+      dark_sky_agent.options["location"] = "37.779329, -522.41915" # too low longitude
+      expect(dark_sky_agent).to_not be_valid
+    end
+  end
+
   describe "#service" do
     it "doesn't have a Service object attached" do
       expect(agent.service).to be_nil
     end
   end
+
+  describe "Agents::WeatherAgent::VALID_COORDS_REGEX" do
+    it "matches 37.779329,-122.41915" do
+      expect(
+        "37.779329,-122.41915".match? Agents::WeatherAgent::VALID_COORDS_REGEX
+      ).to be_truthy
+    end
+    it "matches a dozen random valid values" do
+      valid_longitude_range = -180.0..180.0
+      valid_latitude_range = -90.0..90.0
+      12.times do
+        expect(
+          "#{rand valid_latitude_range},#{rand valid_longitude_range}".match? Agents::WeatherAgent::VALID_COORDS_REGEX
+        ).to be true
+      end
+    end
+  end
 end