From 75dbb7380892ab5428f247b021d249cf19473bd8 Mon Sep 17 00:00:00 2001 From: Wai Sing Yiu Date: Tue, 16 Apr 2024 11:28:45 +0100 Subject: [PATCH 1/3] Extract FcmClient constructor arguments into a case class --- .../com/gu/notifications/worker/AndroidSender.scala | 8 ++++++-- .../worker/delivery/fcm/FcmClient.scala | 13 ++++++++++--- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/AndroidSender.scala b/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/AndroidSender.scala index b128caa11..555387f3f 100644 --- a/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/AndroidSender.scala +++ b/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/AndroidSender.scala @@ -16,6 +16,8 @@ import java.time.Instant import java.util.UUID import java.util.concurrent.Executors import scala.concurrent.{ExecutionContext, ExecutionContextExecutor} +import com.gu.notifications.worker.delivery.fcm.FcmFirebase +import scala.util.Try class AndroidSender(val config: FcmWorkerConfiguration, val firebaseAppName: Option[String], val metricNs: String) extends SenderRequestHandler[FcmClient] { @@ -37,8 +39,10 @@ class AndroidSender(val config: FcmWorkerConfiguration, val firebaseAppName: Opt override implicit val ioContextShift: ContextShift[IO] = IO.contextShift(ec) override implicit val timer: Timer[IO] = IO.timer(ec) - override val deliveryService: IO[Fcm[IO]] = - FcmClient(config.fcmConfig, firebaseAppName).fold(e => IO.raiseError(e), c => IO.delay(new Fcm(c))) + val fcmFirebase: Try[FcmFirebase] = FcmFirebase(config.fcmConfig, firebaseAppName) + override val deliveryService: IO[Fcm[IO]] = + fcmFirebase.fold(e => IO.raiseError(e), c => IO.delay(new Fcm(FcmClient(c)))) + override val maxConcurrency = config.concurrencyForIndividualSend override val batchConcurrency = 100 diff --git a/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/delivery/fcm/FcmClient.scala b/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/delivery/fcm/FcmClient.scala index 4da807cf8..2fd3df29d 100644 --- a/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/delivery/fcm/FcmClient.scala +++ b/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/delivery/fcm/FcmClient.scala @@ -176,8 +176,10 @@ class FcmClient (firebaseMessaging: FirebaseMessaging, firebaseApp: FirebaseApp, } -object FcmClient { - def apply(config: FcmConfig, firebaseAppName: Option[String]): Try[FcmClient] = +case class FcmFirebase(firebaseMessaging: FirebaseMessaging, firebaseApp: FirebaseApp, config: FcmConfig, projectId: String, credential: GoogleCredentials, jsonFactory: JsonFactory) + +object FcmFirebase { + def apply(config: FcmConfig, firebaseAppName: Option[String]): Try[FcmFirebase] = Try { val credential = GoogleCredentials.fromStream(new ByteArrayInputStream(config.serviceAccountKey.getBytes)) val firebaseOptions: FirebaseOptions = FirebaseOptions.builder() @@ -193,10 +195,15 @@ object FcmClient { case s: ServiceAccountCredentials => s.getProjectId() case _ => "" } - new FcmClient(FirebaseMessaging.getInstance(firebaseApp), firebaseApp, config, projectId, credential, firebaseOptions.getJsonFactory()) + new FcmFirebase(FirebaseMessaging.getInstance(firebaseApp), firebaseApp, config, projectId, credential, firebaseOptions.getJsonFactory()) } } +object FcmClient { + def apply(firebase: FcmFirebase): FcmClient = + new FcmClient(firebase.firebaseMessaging, firebase.firebaseApp, firebase.config, firebase.projectId, firebase.credential, firebase.jsonFactory) +} + object FirebaseHelpers { implicit class RichApiFuture[T](val af: ApiFuture[T]) extends AnyVal { From 0b7dae169121ecb718d757b68c5cca4b6bcb937d Mon Sep 17 00:00:00 2001 From: Wai Sing Yiu Date: Tue, 16 Apr 2024 11:37:36 +0100 Subject: [PATCH 2/3] Add log message --- .../worker/delivery/fcm/FcmTransportJdkImpl.scala | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/delivery/fcm/FcmTransportJdkImpl.scala b/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/delivery/fcm/FcmTransportJdkImpl.scala index a91b86159..3a4cc6b37 100644 --- a/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/delivery/fcm/FcmTransportJdkImpl.scala +++ b/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/delivery/fcm/FcmTransportJdkImpl.scala @@ -20,6 +20,7 @@ import java.net.http.HttpRequest import java.net.URI import java.net.http.HttpClient import java.net.http.HttpResponse +import org.slf4j.{Logger, LoggerFactory} trait FcmTransport { def sendAsync(token: String, payload: FcmPayload, dryRun: Boolean): Future[String] @@ -27,8 +28,12 @@ trait FcmTransport { class FcmTransportJdkImpl(credential: GoogleCredentials, url: String, jsonFactory: JsonFactory) extends FcmTransport { + implicit val logger: Logger = LoggerFactory.getLogger(this.getClass) + private val httpClient: HttpClient = HttpClient.newHttpClient() + logger.info("HttpClient is instantiated") + private val charSet = StandardCharsets.UTF_8 private val mediaType = "application/json; charset=UTF-8" From b5eb60e4adb2eb6e1f69cf8d5a207086fb0b87d8 Mon Sep 17 00:00:00 2001 From: Wai Sing Yiu Date: Tue, 16 Apr 2024 16:59:44 +0100 Subject: [PATCH 3/3] Remove the close method as it is not used anywhere --- .../com/gu/notifications/worker/delivery/DeliveryClient.scala | 1 - .../com/gu/notifications/worker/delivery/apns/ApnsClient.scala | 2 -- .../com/gu/notifications/worker/delivery/fcm/FcmClient.scala | 2 -- 3 files changed, 5 deletions(-) diff --git a/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/delivery/DeliveryClient.scala b/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/delivery/DeliveryClient.scala index f8dc7633a..d8d592317 100644 --- a/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/delivery/DeliveryClient.scala +++ b/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/delivery/DeliveryClient.scala @@ -12,7 +12,6 @@ trait DeliveryClient { type Payload <: DeliveryPayload type BatchSuccess <: BatchDeliverySuccess - def close(): Unit def sendNotification(notificationId: UUID, token: String, payload: Payload, dryRun: Boolean) (onComplete: Either[DeliveryException, Success] => Unit) (implicit ece: ExecutionContextExecutor): Unit diff --git a/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/delivery/apns/ApnsClient.scala b/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/delivery/apns/ApnsClient.scala index 053ee7491..c8db286a8 100644 --- a/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/delivery/apns/ApnsClient.scala +++ b/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/delivery/apns/ApnsClient.scala @@ -39,8 +39,6 @@ class ApnsClient(private val underlying: PushyApnsClient, val config: ApnsConfig "DeviceTokenNotForTopic" ) - def close(): Unit = underlying.close().get - def payloadBuilder: Notification => Option[ApnsPayload] = apnsPayloadBuilder.apply _ def sendNotification(notificationId: UUID, token: String, payload: Payload, dryRun: Boolean) diff --git a/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/delivery/fcm/FcmClient.scala b/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/delivery/fcm/FcmClient.scala index 2fd3df29d..3e94817b5 100644 --- a/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/delivery/fcm/FcmClient.scala +++ b/notificationworkerlambda/src/main/scala/com/gu/notifications/worker/delivery/fcm/FcmClient.scala @@ -42,8 +42,6 @@ class FcmClient (firebaseMessaging: FirebaseMessaging, firebaseApp: FirebaseApp, ErrorCode.PERMISSION_DENIED ) - def close(): Unit = firebaseApp.delete() - private final val FCM_URL: String = s"https://fcm.googleapis.com/v1/projects/${projectId}/messages:send"; private val fcmTransport: FcmTransport = new FcmTransportJdkImpl(credential, FCM_URL, jsonFactory)