Skip to content

Commit

Permalink
Merge pull request #1534 from weather-gov/mgwalker/slow-alerts
Browse files Browse the repository at this point in the history
Simplify how we put together polygons for alerts
  • Loading branch information
greg-does-weather committed Aug 5, 2024
2 parents 049c46e + 16e8c11 commit 737baef
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 179 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ abstract class EndToEndBase extends TestCase

$this->weatherData = new WeatherDataService(
$translation,
$cache,
$newRelic,
$dataLayer,
);
Expand Down
18 changes: 14 additions & 4 deletions web/modules/weather_data/src/Service/AlertTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -163,10 +163,20 @@ public function getAlerts($grid, $point, $self = false, $now = false)
$output->alertId = $index;
}

$output->geometry = AlertUtility::getGeometryAsJSON(
$alert,
$this->dataLayer,
);
$hit = $this->cache->get("alert-polygon-" . $alert->id);
if ($hit) {
$output->geometry = $hit->data;
} else {
$output->geometry = AlertUtility::getGeometryAsJSON(
$alert,
$this->dataLayer,
);
$this->cache->set(
"alert-polygon-" . $alert->id,
$output->geometry,
time() + 3600, // keep it for an hour
);
}

// See if there is place information from the alert description.
// This will be false if there is no special location information,
Expand Down
121 changes: 49 additions & 72 deletions web/modules/weather_data/src/Service/AlertUtility.php
Original file line number Diff line number Diff line change
Expand Up @@ -843,8 +843,6 @@ public static function getGeometryAsJSON($alert, $dataLayer)
return $alert->geometry;
}

$geometries = [];

// Otherwise, we need to derive the geometry from other metadata. Alerts
// without a geometry should have a list of affected zones, and we can
// build a geometry off that.
Expand All @@ -857,89 +855,68 @@ public static function getGeometryAsJSON($alert, $dataLayer)
}, $alert->properties->affectedZones);
$ids = implode(",", $ids);

$shapes = $dataLayer->databaseFetchAll(
"SELECT ST_AsWKT(shape) as shape
FROM weathergov_geo_zones
WHERE id IN ($ids)",
);

foreach ($shapes as $shape) {
$geometries[] = $shape;
}
}

if (count($geometries) == 0) {
// If an alert doesn't have a geometry or any zones, that's probably
// a bug. However, if the alert has SAME codes, we can use those to
// get geometries, too.
$counties = false;
if (
property_exists($alert->properties, "geocode") &&
property_exists($alert->properties->geocode, "SAME")
) {
$counties = count($alert->properties->geocode->SAME) > 0;
}
$sql = "SELECT ST_ASGEOJSON(
ST_SIMPLIFY(
ST_SRID(
ST_COLLECT(shape),
0
),
0.003
)
)
as shape
FROM weathergov_geo_zones
WHERE id IN ($ids)";

if ($counties) {
$fips = array_map(function ($same) {
return substr($same, 1);
}, $alert->properties->geocode->SAME);
$fips = implode(",", $fips);
$shape = $dataLayer->databaseFetch($sql);

$shapes = $dataLayer->databaseFetchAll(
"SELECT ST_AsWKT(shape) as shape
FROM weathergov_geo_counties
WHERE countyFips IN ($fips)",
);
if ($shape && property_exists($shape, "shape") && $shape->shape) {
$polygon = json_decode($shape->shape);

foreach ($shapes as $shape) {
$geometries[] = $shape;
}
return $polygon;
}
}

$polygon = "";

if (count($geometries) > 0) {
$polygon = array_pop($geometries)->shape;
// If an alert doesn't have a geometry or any zones, that's probably
// a bug. However, if the alert has SAME codes, we can use those to
// get geometries, too.
$counties = false;
if (
property_exists($alert->properties, "geocode") &&
property_exists($alert->properties->geocode, "SAME")
) {
$counties = count($alert->properties->geocode->SAME) > 0;
}

foreach ($geometries as $geometry) {
$union = $dataLayer->databaseFetch(
"SELECT ST_AsWKT(
ST_UNION(
ST_GEOMFROMTEXT('$polygon'),
ST_GEOMFROMTEXT('$geometry->shape')
)
) as shape",
);
$polygon = $union->shape;
}
if ($counties) {
$fips = array_map(function ($same) {
return substr($same, 1);
}, $alert->properties->geocode->SAME);
$fips = implode(",", $fips);

$polygon = $dataLayer->databaseFetch(
"SELECT ST_ASGEOJSON(
$sql = "SELECT ST_ASGEOJSON(
ST_SIMPLIFY(
ST_GEOMFROMTEXT('$polygon'),
ST_SRID(
ST_COLLECT(shape),
0
),
0.003
)
) as shape",
);

$polygon = json_decode($polygon->shape);

if ($polygon->type == "GeometryCollection") {
foreach ($polygon->geometries as $innerPolygon) {
$innerPolygon->coordinates = SpatialUtility::swapLatLon(
$innerPolygon->coordinates,
);
}
} else {
$polygon->coordinates = SpatialUtility::swapLatLon(
$polygon->coordinates,
);
)
as shape
FROM weathergov_geo_counties
WHERE countyFips IN ($fips)";

$shape = $dataLayer->databaseFetch($sql);
if ($shape && property_exists($shape, "shape") && $shape->shape) {
$polygon = json_decode($shape->shape);
return $polygon;
}
}

return $polygon;
// If we're here, then we didn't find a polygon, zone, or county for
// the alert. That's almost certainly a bug.
return false;
}

public static function getPlacesFromAlertDescription($alert)
Expand All @@ -954,7 +931,7 @@ public static function getPlacesFromAlertDescription($alert)
//
// So if we have a line that matches, we should try parsing it.
preg_match(
"/IN \S+ THIS \S+ INCLUDES \d+ COUNTIES$/sim",
"/IN \S+ (THIS|THE NEW) \S+ INCLUDES \d+ COUNTIES$/sim",
$description,
$matches,
);
Expand Down
129 changes: 27 additions & 102 deletions web/modules/weather_data/src/Service/Test/AlertUtilityGeometry.php.test
Original file line number Diff line number Diff line change
Expand Up @@ -46,70 +46,31 @@ final class AlertUtilityGeometryTest extends TestCase
],
];

$zoneShapes = [
(object) ["shape" => "shape 1"],
(object) ["shape" => "shape 2"],
(object) ["shape" => "shape 3"],
];

$this->dataLayer->method("databaseFetchAll")->will(
$this->returnValueMap([
// Return the shapes for the selected zones
[
"SELECT ST_AsWKT(shape) as shape
FROM weathergov_geo_zones
WHERE id IN ('zone 1','zone 2','zone 3')",
$zoneShapes,
],
]),
);
$zoneShapes = (object) ["shape" => '{"combined":"zones"}'];

$this->dataLayer->method("databaseFetch")->will(
$this->returnValueMap([
// Return the unioned shape of 3 and 1
[
"SELECT ST_AsWKT(
ST_UNION(
ST_GEOMFROMTEXT('shape 3'),
ST_GEOMFROMTEXT('shape 1')
)
) as shape",
(object) ["shape" => "union 3,1"],
],

// Return the unioned shape of 3,1 and 2
[
"SELECT ST_AsWKT(
ST_UNION(
ST_GEOMFROMTEXT('union 3,1'),
ST_GEOMFROMTEXT('shape 2')
)
) as shape",
(object) ["shape" => "union 1,2,3"],
],

// Return the simplified shape as a geometry collection, so we
// can also test that those points are flipped properly.
// Return the shapes for the selected zones
[
"SELECT ST_ASGEOJSON(
ST_SIMPLIFY(
ST_GEOMFROMTEXT('union 1,2,3'),
0.003
)
) as shape",
(object) [
"shape" =>
'{"type":"GeometryCollection","geometries":[{"coordinates":[[0,1],[2,3],[4,5]]}]}',
],
ST_SIMPLIFY(
ST_SRID(
ST_COLLECT(shape),
0
),
0.003
)
)
as shape
FROM weathergov_geo_zones
WHERE id IN ('zone 1','zone 2','zone 3')",
$zoneShapes,
],
]),
);

$expected = (object) [
"type" => "GeometryCollection",
"geometries" => [
(object) ["coordinates" => [[1, 0], [3, 2], [5, 4]]],
],
"combined" => "zones",
];

$actual = AlertUtility::getGeometryAsJSON($alert, $this->dataLayer);
Expand All @@ -134,68 +95,32 @@ final class AlertUtilityGeometryTest extends TestCase
],
];

$countyShapes = [
(object) ["shape" => "shape 1"],
(object) ["shape" => "shape 2"],
(object) ["shape" => "shape 3"],
];
$countyShapes = (object) ["shape" => '{"combined":"county"}'];

$this->dataLayer->method("databaseFetchAll")->will(
$this->dataLayer->method("databaseFetch")->will(
$this->returnValueMap([
// Return the shapes for the selected zones. Note that county
// FIPS codes are numeric, so they are not quoted in the query.
[
"SELECT ST_AsWKT(shape) as shape
FROM weathergov_geo_counties
WHERE countyFips IN (county 1,county 2,county 3)",
$countyShapes,
],
]),
);

$this->dataLayer->method("databaseFetch")->will(
$this->returnValueMap([
// Return the unioned shape of 3 and 1
[
"SELECT ST_AsWKT(
ST_UNION(
ST_GEOMFROMTEXT('shape 3'),
ST_GEOMFROMTEXT('shape 1')
)
) as shape",
(object) ["shape" => "union 3,1"],
],

// Return the unioned shape of 3,1 and 2
[
"SELECT ST_AsWKT(
ST_UNION(
ST_GEOMFROMTEXT('union 3,1'),
ST_GEOMFROMTEXT('shape 2')
)
) as shape",
(object) ["shape" => "union 1,2,3"],
],

// Return the simplified shape
[
"SELECT ST_ASGEOJSON(
ST_SIMPLIFY(
ST_GEOMFROMTEXT('union 1,2,3'),
ST_SRID(
ST_COLLECT(shape),
0
),
0.003
)
) as shape",
(object) [
"shape" =>
'{"type":"Polygon","coordinates":[[0,1],[2,3],[4,5]]}',
],
)
as shape
FROM weathergov_geo_counties
WHERE countyFips IN (county 1,county 2,county 3)",
$countyShapes,
],
]),
);

$expected = (object) [
"type" => "Polygon",
"coordinates" => [[1, 0], [3, 2], [5, 4]],
"combined" => "county",
];

$actual = AlertUtility::getGeometryAsJSON($alert, $this->dataLayer);
Expand Down
10 changes: 10 additions & 0 deletions web/modules/weather_data/src/Service/WeatherDataService.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Drupal\weather_data\Service;

use Drupal\weather_data\Service\HourlyTableTrait;
use Drupal\Core\Cache\CacheBackendInterface;
use Drupal\Core\StringTranslation\TranslationInterface;
use Symfony\Component\HttpFoundation\RequestStack;

Expand All @@ -26,6 +27,13 @@ class WeatherDataService
*/
private $dataLayer;

/**
* Cache of API calls for this request.
*
* @var cache
*/
private $cache;

/**
* Mapping of legacy API icon paths to new icons and conditions text.
*
Expand Down Expand Up @@ -78,9 +86,11 @@ class WeatherDataService
*/
public function __construct(
TranslationInterface $t,
CacheBackendInterface $cache,
NewRelicMetrics $newRelic,
DataLayer $dataLayer,
) {
$this->cache = $cache;
$this->dataLayer = $dataLayer;
$this->t = $t;
$this->newRelic = $newRelic;
Expand Down
2 changes: 1 addition & 1 deletion web/modules/weather_data/weather_data.services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ services:
arguments: ['@http_client','@cache.default','@database', '@current_route_match']
weather_data:
class: Drupal\weather_data\Service\WeatherDataService
arguments: ['@string_translation', '@newrelic_metrics', '@weather_data_layer']
arguments: ['@string_translation', '@cache.default', '@newrelic_metrics', '@weather_data_layer']
newrelic_metrics:
class: Drupal\weather_data\Service\NewRelicMetrics
arguments: ['@http_client']

0 comments on commit 737baef

Please sign in to comment.