Merge pull request #655 from transitland/computed_attributes

Computed attributes

app/models/changeset.rb

@@ -154,6 +154,39 @@ def destroy_all_change_payloads
     change_payloads.destroy_all
   end
 
+  def update_computed_attributes
+    # This method updates the changeset's entity attributes that are computed/derived from the attribute data
+    # of multiple entity types. For example, here RouteStopPatterns will have to have their stop distances recomputed
+    # using both the RouteStopPattern and its stop_pattern Stops' geometries. Operators have their convex hulls
+    # recomputed from the Stops it serves.
+    #
+    # Ideally we would like to define methods at the model level (that would be the first place to put new
+    # recomputed attribute functionality if possible) but the need to avoid duplicate recomputation on entities of update
+    # changesets complicates this. E.g, We don't want to recompute the stop_distances of one RouteStopPattern
+    # multiple times if there are multiple Stops of that RouteStopPattern in the changeset.
+    rsps_to_update_distances = Set.new
+
+    if self.stops_created_or_updated
+      operators_to_update_convex_hull = Set.new
+      self.stops_created_or_updated.each do |stop|
+        rsps_to_update_distances.merge(RouteStopPattern.with_stops(stop.onestop_id))
+        operators_to_update_convex_hull.merge(OperatorServingStop.where(stop: stop).map(&:operator))
+      end
+
+      operators_to_update_convex_hull.each { |operator|
+        operator.geometry = operator.recompute_convex_hull_around_stops
+        operator.update_making_history(changeset: self)
+      }
+    end
+
+    rsps_to_update_distances.merge(self.route_stop_patterns_created_or_updated)
+    rsps_to_update_distances.each { |rsp|
+      rsp.update_making_history(changeset: self, new_attrs: { stop_distances: rsp.calculate_distances })
+    }
+    #mainly for testing
+    [rsps_to_update_distances.size, operators_to_update_convex_hull.size]
+  end
+
   def apply!
     fail Changeset::Error.new(changeset: self, message: 'has already been applied.') if applied
     Changeset.transaction do
@@ -176,6 +209,9 @@ def apply!
           end
           EntityImportedFromFeed.import eiff_batch
         end
+
+        # this will go before quality check once merged with issues branch. not called if import
+        update_computed_attributes unless self.imported_from_feed && self.imported_from_feed_version
       rescue => e
         logger.error "Error applying Changeset #{self.id}: #{e.message}"
         logger.error e.backtrace

spec/factories/changeset_factory.rb

@@ -40,5 +40,4 @@
       }
     }
   end
-
 end

spec/factories/route_factory.rb

@@ -40,4 +40,13 @@
     association :created_or_updated_in_changeset, factory: :changeset
     association :operator
   end
+
+  factory :route_bart, class: Route do
+    onestop_id { 'r-9q8y-richmond~dalycity~millbrae' }
+    name { 'Richmond - Daly City/Millbrae'  }
+    vehicle_type { 1 }
+    version 1
+    association :created_or_updated_in_changeset, factory: :changeset
+    association :operator
+  end
 end

spec/factories/stop_factory.rb

@@ -42,4 +42,31 @@
     version 1
     association :created_or_updated_in_changeset, factory: :changeset
   end
+
+  factory :stop_richmond, class: Stop do
+    onestop_id 's-9q8zzf1nks-richmond'
+    geometry { "POINT(-122.353165 37.936887)" }
+    timezone 'America/Los_Angeles'
+    name 'Richmond'
+    version 1
+    association :created_or_updated_in_changeset, factory: :changeset
+  end
+
+  factory :stop_richmond_offset, class: Stop do
+    onestop_id 's-9q8zzf1nks-richmond'
+    geometry { "POINT(-122.350721 37.952326)" }
+    timezone 'America/Los_Angeles'
+    name 'Richmond'
+    version 1
+    association :created_or_updated_in_changeset, factory: :changeset
+  end
+
+  factory :stop_millbrae, class: Stop do
+    onestop_id 's-9q8vzhbf8h-millbrae'
+    geometry { "POINT(-122.38666 37.599787)" }
+    timezone 'America/Los_Angeles'
+    name 'Millbrae'
+    version 1
+    association :created_or_updated_in_changeset, factory: :changeset
+  end
 end

spec/models/changeset_spec.rb

@@ -179,6 +179,109 @@
       expect(Stop.find_by_onestop_id('s-9q8yt4b-1AvHoS')).to be_nil
     end
 
+    context 'computed attributes' do
+      it 'recomputes rsp stop distances from rsp update changeset' do
+        create(:stop_richmond_offset)
+        create(:stop_millbrae)
+        create(:route_stop_pattern_bart)
+
+        # now, a minor tweak to the first rsp geometry endpoint to demonstrate a change in stop distance for the second stop
+        changeset = create(:changeset, payload: {
+          changes: [
+            {
+              action: 'createUpdate',
+              routeStopPattern: {
+                onestopId: 'r-9q8y-richmond~dalycity~millbrae-45cad3-46d384',
+                stopPattern: ['s-9q8zzf1nks-richmond', 's-9q8vzhbf8h-millbrae'],
+                geometry: { type: "LineString", coordinates: [[-122.351529, 37.937750], [-122.38666, 37.599787]] }
+              }
+            }
+          ]
+        })
+        changeset.apply!
+        expect(RouteStopPattern.find_by_onestop_id!('r-9q8y-richmond~dalycity~millbrae-45cad3-46d384').stop_distances).to eq [0.0, 37748.7]
+      end
+
+      it 'recomputes rsp stop distances from stop update changeset' do
+        create(:stop_richmond_offset)
+        create(:stop_millbrae)
+        create(:route_stop_pattern_bart)
+        changeset = create(:changeset, payload: {
+          changes: [
+            {
+              action: 'createUpdate',
+              stop: {
+                onestopId: 's-9q8zzf1nks-richmond',
+                timezone: 'America/Los_Angeles',
+                name: 'Richmond',
+                geometry: { type: "Point", coordinates: [-122.353165, 37.936887] }
+              }
+            }
+          ]
+        })
+        changeset.apply!
+        expect(RouteStopPattern.find_by_onestop_id!('r-9q8y-richmond~dalycity~millbrae-45cad3-46d384').stop_distances).to eq [0.0, 37641.4]
+      end
+
+      it 'avoids duplication of rsp distance calculation' do
+        create(:stop_richmond_offset)
+        create(:stop_millbrae)
+        create(:route_stop_pattern_bart)
+        changeset = create(:changeset, payload: {
+          changes: [
+            {
+              action: 'createUpdate',
+              stop: {
+                onestopId: 's-9q8zzf1nks-richmond',
+                timezone: 'America/Los_Angeles',
+                name: 'Richmond',
+                geometry: { type: "Point", coordinates: [-122.353165, 37.936887] }
+              },
+              stop: {
+                onestopId: 's-9q8vzhbf8h-millbrae',
+                timezone: 'America/Los_Angeles',
+                name: 'Millbrae',
+                geometry: { type: "Point", coordinates: [-122.38266, 37.599487] }
+              }
+            },
+            {
+              action: 'createUpdate',
+              routeStopPattern: {
+                onestopId: 'r-9q8y-richmond~dalycity~millbrae-45cad3-46d384',
+                stopPattern: ['s-9q8zzf1nks-richmond', 's-9q8vzhbf8h-millbrae'],
+                geometry: { type: "LineString", coordinates: [[-122.351529, 37.937750], [-122.38666, 37.599787]] }
+              }
+            }
+          ]
+        })
+        changeset.change_payloads.each do |change_payload|
+          change_payload.apply!
+        end
+        expect(changeset.update_computed_attributes).to eq [1,0]
+      end
+
+      it 'recomputes operator convex hull on stop update changeset' do
+        stop = create(:stop_richmond)
+        operator = create(:operator, geometry: { type: "Point", coordinates: stop.geometry[:coordinates] } )
+        OperatorServingStop.new(operator: operator, stop: stop).save!
+        changeset = create(:changeset, payload: {
+          changes: [
+            {
+              action: 'createUpdate',
+              stop: {
+                onestopId: 's-9q8zzf1nks-richmond',
+                timezone: 'America/Los_Angeles',
+                name: 'Richmond',
+                geometry: { type: "Point", coordinates: [-122.5, 37.9] }
+              }
+            }
+          ]
+        })
+        changeset.apply!
+        expect(Operator.find_by_onestop_id!(operator.onestop_id).geometry[:coordinates]).to match_array([a_value_within(0.001).of(-122.5), a_value_within(0.001).of(37.9)])
+      end
+    end
+
     it 'to create and remove a relationship' do
       @changeset1.apply!
       @changeset2.apply!

spec/models/operator_serving_stop_spec.rb

@@ -33,7 +33,8 @@
             stop: {
               onestopId: 's-9q8yt4b-19Hollway',
               name: '19th Ave & Holloway St',
-              timezone: 'America/Los_Angeles'
+              timezone: 'America/Los_Angeles',
+              geometry: { type: "Point", coordinates: [-122.475075, 37.721323] }
             }
           },
           {

spec/models/route_serving_stop_spec.rb

@@ -33,7 +33,8 @@
             stop: {
               onestopId: 's-9q8yt4b-19Hollway',
               name: '19th Ave & Holloway St',
-              timezone: 'America/Los_Angeles'
+              timezone: 'America/Los_Angeles',
+              geometry: { type: "Point", coordinates: [-122.475075, 37.721323] }
             }
           },
           {