Skip to content

Commit

Permalink
Precompute Centroid for Location Polygons
Browse files Browse the repository at this point in the history
The centroid is used for calculating the distances on the vacancy
searches.

The centroid for a polygon area doesn't change unless the area changes
too.

As we dinamically calculate it for every location polygon area
against every vacancy during the searches, if we pre-compute it we
save the DB to execute again the centroid calculation on every search.
  • Loading branch information
scruti committed Jan 23, 2025
1 parent f9e4342 commit 42e08e9
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 39 deletions.
21 changes: 15 additions & 6 deletions app/services/ons_data_import/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,27 @@ def call
geometry = feature["geometry"].to_json

Rails.logger.info("Persisting new area data for '#{name}' (#{type})")
ActiveRecord::Base.connection.exec_update("
UPDATE location_polygons
SET area=ST_GeomFromGeoJSON(#{ActiveRecord::Base.connection.quote(geometry)}),
location_type=#{ActiveRecord::Base.connection.quote(type)}
WHERE id='#{location_polygon.id}'
")
set_area_data(location_polygon, geometry, type)
end
end
end

private

def set_area_data(location_polygon, geometry, type)
ActiveRecord::Base.connection.exec_update("
WITH geom AS (
SELECT ST_GeomFromGeoJSON(#{ActiveRecord::Base.connection.quote(geometry)}) AS geo
)
UPDATE location_polygons
SET area=geom.geo,
location_type=#{ActiveRecord::Base.connection.quote(type)},
centroid=ST_Centroid(geom.geo)
FROM geom
WHERE id='#{location_polygon.id}'
")
end

def arcgis_features(offset)
params = [
"where=1%3D1",
Expand Down
15 changes: 9 additions & 6 deletions app/services/ons_data_import/create_composites.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,16 @@ def call
quoted_constituents = constituents.map { |c| ActiveRecord::Base.connection.quote(c.downcase) }

ActiveRecord::Base.connection.exec_update("
WITH composite_area AS (
SELECT ST_Union(area::geometry)::geography AS geo
FROM location_polygons
WHERE name IN (#{quoted_constituents.join(', ')})
)
UPDATE location_polygons
SET area=(
SELECT ST_Union(area::geometry)::geography
FROM location_polygons
WHERE name IN (#{quoted_constituents.join(', ')})
),
location_type='composite'
SET area=composite_area.geo,
location_type='composite',
centroid=ST_Centroid(composite_area.geo)
FROM composite_area
WHERE id='#{composite.id}'
")
end
Expand Down
1 change: 1 addition & 0 deletions config/analytics.yml
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ shared:
- name
- location_type
- area
- centroid
- created_at
- updated_at
notifications:
Expand Down
46 changes: 23 additions & 23 deletions config/brakeman.ignore
Original file line number Diff line number Diff line change
Expand Up @@ -115,29 +115,6 @@
],
"note": ""
},
{
"warning_type": "SQL Injection",
"warning_code": 0,
"fingerprint": "549d2779be79702f98156c6ec1a388b35ff19d87b0978910d4fad9d59a424cb9",
"check_name": "SQL",
"message": "Possible SQL injection",
"file": "app/services/ons_data_import/create_composites.rb",
"line": 14,
"link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
"code": "ActiveRecord::Base.connection.exec_update(\"\\n UPDATE location_polygons\\n SET area=(\\n SELECT ST_Union(area::geometry)::geography\\n FROM location_polygons\\n WHERE name IN (#{constituents.map do\n ActiveRecord::Base.connection.quote(c.downcase)\n end.join(\", \")})\\n ),\\n location_type='composite'\\n WHERE id='#{LocationPolygon.find_or_create_by(:name => name).id}'\\n \")",
"render_path": null,
"location": {
"type": "method",
"class": "OnsDataImport::CreateComposites",
"method": "call"
},
"user_input": "constituents.map do\n ActiveRecord::Base.connection.quote(c.downcase)\n end.join(\", \")",
"confidence": "Medium",
"cwe_id": [
89
],
"note": "This is our own static data with no user input provided in the query"
},
{
"warning_type": "Unscoped Find",
"warning_code": 82,
Expand Down Expand Up @@ -218,6 +195,29 @@
],
"note": ""
},
{
"warning_type": "SQL Injection",
"warning_code": 0,
"fingerprint": "b2035069089eae3a5d8ec3a3a615aee253ec3cf2cd68c5535af152b8de779f46",
"check_name": "SQL",
"message": "Possible SQL injection",
"file": "app/services/ons_data_import/create_composites.rb",
"line": 13,
"link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
"code": "ActiveRecord::Base.connection.exec_update(\"\\n WITH composite_area AS (\\n SELECT ST_Union(area::geometry)::geography AS geo\\n FROM location_polygons\\n WHERE name IN (#{constituents.map do\n ActiveRecord::Base.connection.quote(c.downcase)\n end.join(\", \")})\\n )\\n UPDATE location_polygons\\n SET area=composite_area.geo,\\n location_type='composite',\\n centroid=ST_Centroid(composite_area.geo)\\n FROM composite_area\\n WHERE id='#{LocationPolygon.find_or_create_by(:name => name).id}'\\n \")",
"render_path": null,
"location": {
"type": "method",
"class": "OnsDataImport::CreateComposites",
"method": "call"
},
"user_input": "constituents.map do\n ActiveRecord::Base.connection.quote(c.downcase)\n end.join(\", \")",
"confidence": "Medium",
"cwe_id": [
89
],
"note": ""
},
{
"warning_type": "SQL Injection",
"warning_code": 0,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class AddCentroidToLocationPolygons < ActiveRecord::Migration[7.2]
disable_ddl_transaction!

def change
add_column :location_polygons, :centroid, :st_point, geographic: true
add_index :location_polygons, :centroid, using: :gist, algorithm: :concurrently
end
end
10 changes: 6 additions & 4 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,11 @@
t.uuid "job_application_id"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.integer "employment_type", default: 0
t.text "reason_for_break", default: ""
t.text "organisation_ciphertext"
t.text "job_title_ciphertext"
t.text "main_duties_ciphertext"
t.integer "employment_type", default: 0
t.text "reason_for_break", default: ""
t.uuid "jobseeker_profile_id"
t.text "reason_for_leaving"
t.index ["job_application_id"], name: "index_employments_on_job_application_id"
Expand Down Expand Up @@ -312,9 +312,9 @@
t.date "account_closed_on"
t.text "current_sign_in_ip_ciphertext"
t.text "last_sign_in_ip_ciphertext"
t.string "govuk_one_login_id"
t.string "account_merge_confirmation_code"
t.datetime "account_merge_confirmation_code_generated_at"
t.string "govuk_one_login_id"
t.index ["email"], name: "index_jobseekers_on_email", unique: true
t.index ["govuk_one_login_id"], name: "index_jobseekers_on_govuk_one_login_id", unique: true
end
Expand All @@ -333,7 +333,9 @@
t.datetime "created_at", precision: nil, null: false
t.datetime "updated_at", precision: nil, null: false
t.geography "area", limit: {:srid=>4326, :type=>"geometry", :geographic=>true}
t.geography "centroid", limit: {:srid=>4326, :type=>"st_point", :geographic=>true}
t.index ["area"], name: "index_location_polygons_on_area", using: :gist
t.index ["centroid"], name: "index_location_polygons_on_centroid", using: :gist
t.index ["name"], name: "index_location_polygons_on_name"
end

Expand Down Expand Up @@ -697,8 +699,8 @@
t.boolean "include_additional_documents"
t.boolean "visa_sponsorship_available"
t.boolean "is_parental_leave_cover"
t.boolean "is_job_share"
t.string "hourly_rate"
t.boolean "is_job_share"
t.string "flexi_working"
t.integer "extension_reason"
t.string "other_extension_reason_details"
Expand Down

0 comments on commit 42e08e9

Please sign in to comment.