From ef04e156fd0d5905327d62c12e95f66e539f7d5b Mon Sep 17 00:00:00 2001 From: William Woods Date: Mon, 3 Apr 2023 04:26:18 +0000 Subject: [PATCH 1/2] Refactored code for performance and readability --- .../clients/geoduck/UserLocationFetcher.scala | 61 +++++++------------ 1 file changed, 21 insertions(+), 40 deletions(-) diff --git a/follow-recommendations-service/common/src/main/scala/com/twitter/follow_recommendations/common/clients/geoduck/UserLocationFetcher.scala b/follow-recommendations-service/common/src/main/scala/com/twitter/follow_recommendations/common/clients/geoduck/UserLocationFetcher.scala index 706ae5143..d83208033 100644 --- a/follow-recommendations-service/common/src/main/scala/com/twitter/follow_recommendations/common/clients/geoduck/UserLocationFetcher.scala +++ b/follow-recommendations-service/common/src/main/scala/com/twitter/follow_recommendations/common/clients/geoduck/UserLocationFetcher.scala @@ -1,58 +1,39 @@ -package com.twitter.follow_recommendations.common.clients.geoduck - -import com.twitter.finagle.stats.StatsReceiver import com.twitter.follow_recommendations.common.models.GeohashAndCountryCode import com.twitter.stitch.Stitch -import javax.inject.Inject -import javax.inject.Singleton +import javax.inject.{Inject, Singleton} @Singleton class UserLocationFetcher @Inject() ( locationServiceClient: LocationServiceClient, reverseGeocodeClient: ReverseGeocodeClient, statsReceiver: StatsReceiver) { + private val stats = statsReceiver.scope("user_location_fetcher") - private val stats: StatsReceiver = statsReceiver.scope("user_location_fetcher") - private val totalRequestsCounter = stats.counter("requests") - private val emptyResponsesCounter = stats.counter("empty") - private val locationServiceExceptionCounter = stats.counter("location_service_exception") - private val reverseGeocodeExceptionCounter = stats.counter("reverse_geocode_exception") + def getGeohashAndCountryCode(userId: Option[Long], ipAddress: Option[String]): Stitch[Option[GeohashAndCountryCode]] = { + val totalRequestsCounter = stats.counter("requests").incr() - def getGeohashAndCountryCode( - userId: Option[Long], - ipAddress: Option[String] - ): Stitch[Option[GeohashAndCountryCode]] = { - totalRequestsCounter.incr() - val lscLocationStitch = Stitch - .collect { - userId.map(locationServiceClient.getGeohashAndCountryCode) - }.rescue { - case _: Exception => - locationServiceExceptionCounter.incr() - Stitch.None - } + val lscLocationStitch = Stitch.collect(userId.map(locationServiceClient.getGeohashAndCountryCode)).rescue { + case _: Exception => + stats.counter("location_service_exception").incr() + Stitch.None + } - val ipLocationStitch = Stitch - .collect { - ipAddress.map(reverseGeocodeClient.getGeohashAndCountryCode) - }.rescue { - case _: Exception => - reverseGeocodeExceptionCounter.incr() - Stitch.None - } + val ipLocationStitch = Stitch.collect(ipAddress.map(reverseGeocodeClient.getGeohashAndCountryCode)).rescue { + case _: Exception => + stats.counter("reverse_geocode_exception").incr() + Stitch.None + } Stitch.join(lscLocationStitch, ipLocationStitch).map { - case (lscLocation, ipLocation) => { - val geohash = lscLocation.flatMap(_.geohash).orElse(ipLocation.flatMap(_.geohash)) - val countryCode = - lscLocation.flatMap(_.countryCode).orElse(ipLocation.flatMap(_.countryCode)) - (geohash, countryCode) match { - case (None, None) => - emptyResponsesCounter.incr() + case (lscLocation, ipLocation) => + (lscLocation.flatMap(_.geohash).orElse(ipLocation.flatMap(_.geohash)), + lscLocation.flatMap(_.countryCode).orElse(ipLocation.flatMap(_.countryCode))) match { + case (Some(geohash), Some(countryCode)) => + Some(GeohashAndCountryCode(geohash, countryCode)) + case _ => + stats.counter("empty").incr() None - case _ => Some(GeohashAndCountryCode(geohash, countryCode)) - } } } } From 0917e50b70b7f19ca12a7e2f4963524220fea30a Mon Sep 17 00:00:00 2001 From: William Woods Date: Mon, 3 Apr 2023 07:11:21 +0000 Subject: [PATCH 2/2] Refactored code for performance and readability --- .../common/clients/geoduck/UserLocationFetcher.scala | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/follow-recommendations-service/common/src/main/scala/com/twitter/follow_recommendations/common/clients/geoduck/UserLocationFetcher.scala b/follow-recommendations-service/common/src/main/scala/com/twitter/follow_recommendations/common/clients/geoduck/UserLocationFetcher.scala index d83208033..b279118cb 100644 --- a/follow-recommendations-service/common/src/main/scala/com/twitter/follow_recommendations/common/clients/geoduck/UserLocationFetcher.scala +++ b/follow-recommendations-service/common/src/main/scala/com/twitter/follow_recommendations/common/clients/geoduck/UserLocationFetcher.scala @@ -1,3 +1,5 @@ +package com.twitter.follow_recommendations.common.clients.geoduck +import com.twitter.finagle.stats.StatsReceiver import com.twitter.follow_recommendations.common.models.GeohashAndCountryCode import com.twitter.stitch.Stitch @@ -8,6 +10,7 @@ class UserLocationFetcher @Inject() ( locationServiceClient: LocationServiceClient, reverseGeocodeClient: ReverseGeocodeClient, statsReceiver: StatsReceiver) { + private val stats = statsReceiver.scope("user_location_fetcher") def getGeohashAndCountryCode(userId: Option[Long], ipAddress: Option[String]): Stitch[Option[GeohashAndCountryCode]] = { @@ -26,7 +29,7 @@ class UserLocationFetcher @Inject() ( } Stitch.join(lscLocationStitch, ipLocationStitch).map { - case (lscLocation, ipLocation) => + case (lscLocation, ipLocation) => (lscLocation.flatMap(_.geohash).orElse(ipLocation.flatMap(_.geohash)), lscLocation.flatMap(_.countryCode).orElse(ipLocation.flatMap(_.countryCode))) match { case (Some(geohash), Some(countryCode)) => @@ -34,7 +37,7 @@ class UserLocationFetcher @Inject() ( case _ => stats.counter("empty").incr() None - } + } } } -} +} \ No newline at end of file