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 21, 2025
1 parent f2caecc commit 7d50e00
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 40 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
12 changes: 7 additions & 5 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.2].define(version: 2025_01_13_144018) do
ActiveRecord::Schema[7.2].define(version: 2025_01_14_180542) do
# These are extensions that must be enabled in order to support this database
enable_extension "btree_gist"
enable_extension "citext"
Expand Down Expand Up @@ -78,11 +78,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 @@ -313,9 +313,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 @@ -334,7 +334,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 @@ -685,8 +687,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 7d50e00

Please sign in to comment.