From 38e2ebd6fa91cd74699f7fd1f0989b89ce9fa034 Mon Sep 17 00:00:00 2001 From: Mike Bryant Date: Tue, 5 Mar 2024 16:34:34 +0000 Subject: [PATCH] Implement the repository geocoder using AWS Location service The old Bing one seems sadly defunct --- build.sbt | 3 + conf/application.conf | 5 ++ conf/logback-play-dev.xml | 1 + conf/test.conf | 5 ++ .../core/src/main/scala/models/Address.scala | 20 +++++- modules/portal/app/guice/AppModule.scala | 8 ++- .../geocoding/AwsGeocodingService.scala | 68 +++++++++++++++++++ .../services/geocoding/GeocodingService.scala | 2 - test/helpers/TestConfiguration.scala | 3 +- .../geocoding/NoopGeocodingService.scala | 16 +++++ 10 files changed, 126 insertions(+), 5 deletions(-) create mode 100644 modules/portal/app/services/geocoding/AwsGeocodingService.scala create mode 100644 test/services/geocoding/NoopGeocodingService.scala diff --git a/build.sbt b/build.sbt index 149b9849aa..7df0f1b6cb 100644 --- a/build.sbt +++ b/build.sbt @@ -108,6 +108,9 @@ val portalDependencies = Seq( // S3 sdk "software.amazon.awssdk" % "s3" % "2.15.63", + + // AWS Location sdk + "software.amazon.awssdk" % "location" % "2.15.63", ) val adminDependencies = Seq( diff --git a/conf/application.conf b/conf/application.conf index d2be2c9130..d3035ebc4b 100644 --- a/conf/application.conf +++ b/conf/application.conf @@ -124,6 +124,11 @@ services { geocoding { parallelism: 3 + + config = { + # This should be configured for the individual Geocoding service instance + # e.g. for AWS Location or Bing + } } } diff --git a/conf/logback-play-dev.xml b/conf/logback-play-dev.xml index ace124cef9..4df539db15 100644 --- a/conf/logback-play-dev.xml +++ b/conf/logback-play-dev.xml @@ -60,6 +60,7 @@ + diff --git a/conf/test.conf b/conf/test.conf index c59b69795b..7d159db737 100644 --- a/conf/test.conf +++ b/conf/test.conf @@ -72,3 +72,8 @@ storage.portal.classifier = portal storage.test.config = ${minio} storage.test.classifier = test + +# The full AWS configuration is missing but this +# is enough to run tests that instantiate the client +services.geocoding.config = ${minio} +services.geocoding.config.aws.index-name = ehri diff --git a/modules/core/src/main/scala/models/Address.scala b/modules/core/src/main/scala/models/Address.scala index 6cb7e16448..96bd751994 100644 --- a/modules/core/src/main/scala/models/Address.scala +++ b/modules/core/src/main/scala/models/Address.scala @@ -3,6 +3,8 @@ package models import models.json._ import play.api.libs.json._ +import java.util.Locale + object AddressF { val UNNAMED_ADDRESS = "Unnamed Address" @@ -48,9 +50,25 @@ case class AddressF( fax: Seq[String] = Nil, url: Seq[String] = Nil ) extends ModelData { + + /** + * Returns a concise representation of the address, suitable for display in a list. + */ def concise: String = Seq(streetAddress, city, region).flatten.filterNot(_.trim.isEmpty).mkString(", ") + /** + * Returns a postal address, suitable for display in a single block. + */ + def postalAddress: String = + Seq(streetAddress, city, region, postalCode, countryCode).flatten.filterNot(_.trim.isEmpty).mkString("\n") + + /** + * Returns the ISO 3166-1 alpha-3 code for the country, if available. + */ + def countryCode3: Option[String] = + countryCode.flatMap(c => Locale.getAvailableLocales.find(_.getCountry == c).map(_.getISO3Country)) + override def toString: String = Seq(name, contactPerson, streetAddress, city, region, countryCode).filter(_.isDefined).mkString(", ") } @@ -60,7 +78,7 @@ object Address { import play.api.data.Form import play.api.data.Forms._ - val form = Form( + val form: Form[AddressF] = Form( mapping( Entity.ISA -> ignored(EntityType.Address), Entity.ID -> optional(nonEmptyText), diff --git a/modules/portal/app/guice/AppModule.scala b/modules/portal/app/guice/AppModule.scala index 3e865be18f..826dd15ba4 100644 --- a/modules/portal/app/guice/AppModule.scala +++ b/modules/portal/app/guice/AppModule.scala @@ -12,6 +12,7 @@ import services.RateLimitChecker import services.cypher.{CypherQueryService, CypherService, SqlCypherQueryService, WsCypherService} import services.data._ import services.feedback.{FeedbackService, SqlFeedbackService} +import services.geocoding.{AwsGeocodingService, GeocodingService} import services.htmlpages.{GoogleDocsHtmlPages, HtmlPages} import services.redirects.{MovedPageLookup, SqlMovedPageLookup} import services.search._ @@ -31,12 +32,15 @@ private class DamStorageProvider @Inject()(config: play.api.Configuration)(impli S3CompatibleFileStorage(config.get[com.typesafe.config.Config]("storage.dam")) } +private class AwsGeocodingServiceProvider @Inject()(config: play.api.Configuration, ec: ExecutionContext) extends Provider[GeocodingService] { + override def get(): GeocodingService = AwsGeocodingService(config.get[com.typesafe.config.Config]("services.geocoding"))(ec) +} + class AppModule extends AbstractModule with AkkaGuiceSupport { override def configure(): Unit = { bind(classOf[AppConfig]) bind(classOf[RateLimitChecker]) bind(classOf[EventHandler]).to(classOf[IndexingEventHandler]) - bind(classOf[ItemLifecycle]).to(classOf[GeocodingItemLifecycle]) bind(classOf[DataServiceBuilder]).to(classOf[WsDataServiceBuilder]) bind(classOf[FeedbackService]).to(classOf[SqlFeedbackService]) bind(classOf[CypherQueryService]).to(classOf[SqlCypherQueryService]) @@ -48,6 +52,8 @@ class AppModule extends AbstractModule with AkkaGuiceSupport { bind(classOf[RawMarkdownRenderer]).to(classOf[CommonmarkMarkdownRenderer]) bind(classOf[MarkdownRenderer]).to(classOf[SanitisingMarkdownRenderer]) bind(classOf[CypherService]).to(classOf[WsCypherService]) + bind(classOf[ItemLifecycle]).to(classOf[GeocodingItemLifecycle]) + bind(classOf[GeocodingService]).toProvider(classOf[AwsGeocodingServiceProvider]) bindActor[EventForwarder]("event-forwarder") } } diff --git a/modules/portal/app/services/geocoding/AwsGeocodingService.scala b/modules/portal/app/services/geocoding/AwsGeocodingService.scala new file mode 100644 index 0000000000..d24d6c5b03 --- /dev/null +++ b/modules/portal/app/services/geocoding/AwsGeocodingService.scala @@ -0,0 +1,68 @@ +package services.geocoding + +import models.AddressF +import software.amazon.awssdk.auth.credentials.{AwsCredentials, AwsCredentialsProvider, StaticCredentialsProvider} +import software.amazon.awssdk.regions.Region +import software.amazon.awssdk.regions.providers.AwsRegionProvider +import software.amazon.awssdk.services.location.LocationClient +import software.amazon.awssdk.services.location.model.SearchPlaceIndexForTextRequest + +import javax.inject.Inject +import scala.collection.JavaConverters._ +import scala.concurrent.{ExecutionContext, Future} + + +object AwsGeocodingService { + def apply(config: com.typesafe.config.Config)(implicit ec: ExecutionContext): AwsGeocodingService = { + + val credentials = StaticCredentialsProvider.create(new AwsCredentials { + override def accessKeyId(): String = config.getString("config.aws.credentials.access-key-id") + + override def secretAccessKey(): String = config.getString("config.aws.credentials.secret-access-key") + }) + val region: AwsRegionProvider = new AwsRegionProvider { + override def getRegion: Region = Region.of(config.getString("config.aws.region.default-region")) + } + + new AwsGeocodingService( + credentials, + region, + config.getString("config.aws.index-name") + ) + } +} + +case class AwsGeocodingService @Inject()(credentials: AwsCredentialsProvider, region: AwsRegionProvider, indexName: String)(implicit executionContext: ExecutionContext) extends GeocodingService { + + private val logger = play.api.Logger(classOf[AwsGeocodingService]) + + override def geocode(address: AddressF): Future[Option[Point]] = { + val client: LocationClient = LocationClient + .builder() + .credentialsProvider(credentials) + .region(region.getRegion) + .build() + + val request = SearchPlaceIndexForTextRequest.builder() + .indexName(indexName) + .text(address.postalAddress) + .filterCountries(address.countryCode3.toList: _*) + .maxResults(1) + .build() + + Future { + client.searchPlaceIndexForText(request) + .results() + .asScala + .headOption + .map { result => + logger.debug(s"Geocoding result: $result, point: ${result.place().geometry().point()}") + val lonLat = result.place().geometry().point() + Point( + BigDecimal(lonLat.get(1)), + BigDecimal(lonLat.get(0)) + ) + } + } + } +} diff --git a/modules/portal/app/services/geocoding/GeocodingService.scala b/modules/portal/app/services/geocoding/GeocodingService.scala index eabacc29bb..389446798e 100644 --- a/modules/portal/app/services/geocoding/GeocodingService.scala +++ b/modules/portal/app/services/geocoding/GeocodingService.scala @@ -1,13 +1,11 @@ package services.geocoding -import com.google.inject.ImplementedBy import models.AddressF import scala.concurrent.Future case class Point(latitude: BigDecimal, longitude: BigDecimal) -@ImplementedBy(classOf[BingGeocodingService]) trait GeocodingService { def geocode(address: AddressF): Future[Option[Point]] } diff --git a/test/helpers/TestConfiguration.scala b/test/helpers/TestConfiguration.scala index 2907f99448..701b8a063a 100644 --- a/test/helpers/TestConfiguration.scala +++ b/test/helpers/TestConfiguration.scala @@ -29,6 +29,7 @@ import services.accounts.{AccountManager, MockAccountManager} import services.cypher.{CypherQueryService, MockCypherQueryService} import services.data._ import services.feedback.{FeedbackService, MockFeedbackService} +import services.geocoding.{GeocodingService, NoopGeocodingService} import services.harvesting.{MockResourceSyncClient, ResourceSyncClient} import services.htmlpages.{HtmlPages, MockHtmlPages} import services.ingest.{EadValidator, MockEadValidatorService} @@ -118,7 +119,7 @@ trait TestConfiguration { bind[EventHandler].toInstance(testEventHandler), bind[HtmlPages].toInstance(mockHtmlPages), - bind[ItemLifecycle].to[NoopItemLifecycle], + bind[GeocodingService].to[NoopGeocodingService], bind[EadValidator].to[MockEadValidatorService], bind[ResourceSyncClient].to[MockResourceSyncClient], // NB: Graph IDs are not stable during testing due to diff --git a/test/services/geocoding/NoopGeocodingService.scala b/test/services/geocoding/NoopGeocodingService.scala new file mode 100644 index 0000000000..45a180de9d --- /dev/null +++ b/test/services/geocoding/NoopGeocodingService.scala @@ -0,0 +1,16 @@ +package services.geocoding + +import models.AddressF +import play.api.Logger + +import javax.inject.{Inject, Singleton} +import scala.concurrent.Future + +@Singleton +case class NoopGeocodingService @Inject()() extends GeocodingService { + private val logger = Logger(classOf[NoopGeocodingService]) + override def geocode(address: AddressF): Future[Option[Point]] = { + logger.info(s"Geocoding disabled: $address") + Future.successful(None) + } +}