From 4321270e0cc9f9802206c04d5df776d6b5e5b71f Mon Sep 17 00:00:00 2001 From: Jym Dyer Date: Tue, 16 May 2023 18:32:16 -0700 Subject: [PATCH 01/20] Support multiple notification channels. --- .../controllers/api/OtpUserController.java | 2 +- .../middleware/models/OtpUser.java | 45 ++++++++++++++++--- .../tripmonitor/jobs/CheckMonitoredTrip.java | 27 +++++------ .../testutils/PersistenceTestUtils.java | 2 +- 4 files changed, 52 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java b/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java index 15eb9a9d3..6bb536f8f 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java @@ -112,7 +112,7 @@ public VerificationResult sendVerificationText(Request req, Response res) { if (verification.getStatus().equals("pending")) { otpUser.phoneNumber = phoneNumber; otpUser.isPhoneNumberVerified = false; - otpUser.notificationChannel = "sms"; + otpUser.notificationChannel.add(OtpUser.Notification.SMS); Persistence.otpUsers.replace(otpUser.id, otpUser); } diff --git a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java index 5d793896e..c5006f59c 100644 --- a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java @@ -1,14 +1,17 @@ package org.opentripplanner.middleware.models; +import com.fasterxml.jackson.annotation.JsonGetter; import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonSetter; import org.opentripplanner.middleware.auth.Auth0Users; import org.opentripplanner.middleware.auth.RequestingUser; import org.opentripplanner.middleware.persistence.Persistence; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.ArrayList; -import java.util.List; +import java.util.*; +import java.util.stream.Collectors; +import java.util.stream.Stream; /** * This represents a user of an OpenTripPlanner instance (typically of the standard OTP UI/otp-react-redux). @@ -16,6 +19,10 @@ * can also opt-in to storing their trip planning requests/responses. */ public class OtpUser extends AbstractUser { + public enum Notification { + EMAIL, PUSH, SMS + } + public static final String AUTH0_SCOPE = "otp-user"; private static final long serialVersionUID = 1L; private static final Logger LOG = LoggerFactory.getLogger(OtpUser.class); @@ -30,11 +37,10 @@ public class OtpUser extends AbstractUser { public boolean isPhoneNumberVerified; /** - * Notification preference for this user - * ("email", "sms", or "none"). - * TODO: Convert to enum. See http://mongodb.github.io/mongo-java-driver/3.7/bson/pojos/ for guidance. + * Notification preferences for this user + * (EMAIL and/or SMS and/or PUSH). */ - public String notificationChannel; + public EnumSet notificationChannel = EnumSet.noneOf(OtpUser.Notification.class); /** * Verified phone number for SMS notifications, in +15551234 format (E.164 format, includes country code, no spaces). @@ -105,4 +111,31 @@ public boolean canBeManagedBy(RequestingUser requestingUser) { return super.canBeManagedBy(requestingUser); } + /** + * Get notification channels as comma-separated list in one string + */ + @JsonGetter(value = "notificationChannel") + public String getNotificationChannel() { + return notificationChannel.stream() + .map(channel -> channel.name().toLowerCase()) + .collect(Collectors.joining(",")); + } + + /** + * Set notification channels based on comma-separated list in one string + */ + @JsonSetter(value = "notificationChannel") + public void setNotificationChannel(String channels) { + Stream.of(channels.split(",")) + .filter(Objects::nonNull) + .map(str -> str.trim().toUpperCase()) + .filter(str -> !str.isEmpty()) + .forEach(channel -> { + try { + notificationChannel.add(Enum.valueOf(OtpUser.Notification.class, channel)); + } catch (Exception e) { + LOG.error("Notification channel \"{}\" is not valid", channel); + } + }); + } } diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java index 8ba7d47ac..2c0a661d6 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java @@ -403,23 +403,18 @@ private void sendNotifications() { ); // FIXME: Change log level LOG.info("Sending notification to user {}", trip.userId); - boolean success = false; - // FIXME: This needs to be an enum. - switch (otpUser.notificationChannel.toLowerCase()) { - case "sms": - success = sendSMS(otpUser, templateData); - break; - case "email": - success = sendEmail(otpUser, templateData); - break; - case "all": - // TODO better handle below when one of the following fails - success = sendSMS(otpUser, templateData) && sendEmail(otpUser, templateData); - break; - default: - break; + boolean successEmail = false; + boolean successSms = false; + + if (otpUser.notificationChannel.contains(OtpUser.Notification.EMAIL)) { + successEmail = sendEmail(otpUser, templateData); + } + if (otpUser.notificationChannel.contains(OtpUser.Notification.SMS)) { + successSms = sendSMS(otpUser, templateData); } - if (success) { + + // TODO: better handle below when one of the following fails + if (successEmail || successSms) { notificationTimestampMillis = DateTimeUtils.currentTimeMillis(); } } diff --git a/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java b/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java index 34c595d43..98c4897a4 100644 --- a/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java +++ b/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java @@ -41,7 +41,7 @@ public static OtpUser createUser(String email, String phoneNumber) { OtpUser user = new OtpUser(); user.email = email; user.phoneNumber = phoneNumber; - user.notificationChannel = "email"; + user.notificationChannel.add(OtpUser.Notification.EMAIL); user.hasConsentedToTerms = true; user.storeTripHistory = true; Persistence.otpUsers.create(user); From 0644054edb180789fd7ecf13745837f0c4674ad9 Mon Sep 17 00:00:00 2001 From: Jym Dyer Date: Wed, 17 May 2023 14:16:17 -0700 Subject: [PATCH 02/20] Handle "none" value in notification channel. --- .../middleware/models/OtpUser.java | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java index c5006f59c..d42558dd2 100644 --- a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java @@ -126,16 +126,20 @@ public String getNotificationChannel() { */ @JsonSetter(value = "notificationChannel") public void setNotificationChannel(String channels) { - Stream.of(channels.split(",")) - .filter(Objects::nonNull) - .map(str -> str.trim().toUpperCase()) - .filter(str -> !str.isEmpty()) - .forEach(channel -> { - try { - notificationChannel.add(Enum.valueOf(OtpUser.Notification.class, channel)); - } catch (Exception e) { - LOG.error("Notification channel \"{}\" is not valid", channel); - } - }); + if (channels.isEmpty() || "none".equals(channels)) { + notificationChannel.clear(); + } else { + Stream.of(channels.split(",")) + .filter(Objects::nonNull) + .map(str -> str.trim().toUpperCase()) + .filter(str -> !str.isEmpty()) + .forEach(channel -> { + try { + notificationChannel.add(Enum.valueOf(OtpUser.Notification.class, channel)); + } catch (Exception e) { + LOG.error("Notification channel \"{}\" is not valid", channel); + } + }); + } } } From 2715c0028bfec5a2a3a83acdb0ebc51ab78c9d20 Mon Sep 17 00:00:00 2001 From: Jym Dyer Date: Thu, 8 Jun 2023 12:20:51 -0700 Subject: [PATCH 03/20] Push notifications. --- .../tripmonitor/jobs/CheckMonitoredTrip.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java index 2c0a661d6..2ae84ee8f 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java @@ -404,17 +404,21 @@ private void sendNotifications() { // FIXME: Change log level LOG.info("Sending notification to user {}", trip.userId); boolean successEmail = false; + boolean successPush = false; boolean successSms = false; if (otpUser.notificationChannel.contains(OtpUser.Notification.EMAIL)) { successEmail = sendEmail(otpUser, templateData); } + if (otpUser.notificationChannel.contains(OtpUser.Notification.PUSH)) { + successPush = sendPush(otpUser, templateData); + } if (otpUser.notificationChannel.contains(OtpUser.Notification.SMS)) { successSms = sendSMS(otpUser, templateData); } // TODO: better handle below when one of the following fails - if (successEmail || successSms) { + if (successEmail || successPush || successSms) { notificationTimestampMillis = DateTimeUtils.currentTimeMillis(); } } @@ -426,6 +430,13 @@ private boolean sendSMS(OtpUser otpUser, Map data) { return NotificationUtils.sendSMS(otpUser, "MonitoredTripSms.ftl", data) != null; } + /** + * Send push notification. + */ + private boolean sendPush(OtpUser otpUser, Map data) { + return NotificationUtils.sendPush(otpUser, "MonitoredTripText.ftl", data) != null; + } + /** * Send notification email in MonitoredTrip template. */ From c3a96c725aac3594cb8f601845f84ce2cee75456 Mon Sep 17 00:00:00 2001 From: Jym Dyer Date: Thu, 10 Aug 2023 13:53:20 -0700 Subject: [PATCH 04/20] Add pushDevices (number of push devices last we looked) --- .../java/org/opentripplanner/middleware/models/OtpUser.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java index d42558dd2..7fd236861 100644 --- a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java @@ -53,6 +53,11 @@ public enum Notification { */ public String preferredLocale; + /** + * Number of push devices associated with user email + */ + public int pushDevices; + /** Locations that the user has saved. */ public List savedLocations = new ArrayList<>(); From 5f285018219f42e5db87655de92bd763cd96379b Mon Sep 17 00:00:00 2001 From: Jym Dyer Date: Thu, 10 Aug 2023 13:54:55 -0700 Subject: [PATCH 05/20] Add pushDevices (number of push devices) to test --- .../middleware/testutils/PersistenceTestUtils.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java b/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java index 98c4897a4..aa43ba3be 100644 --- a/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java +++ b/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java @@ -44,6 +44,7 @@ public static OtpUser createUser(String email, String phoneNumber) { user.notificationChannel.add(OtpUser.Notification.EMAIL); user.hasConsentedToTerms = true; user.storeTripHistory = true; + user.pushDevices = 0; Persistence.otpUsers.create(user); return user; } From eb2d20742c61a4deed6b16b556318a49ed1433fa Mon Sep 17 00:00:00 2001 From: Jym Dyer Date: Thu, 10 Aug 2023 14:00:16 -0700 Subject: [PATCH 06/20] Add sendPush() method for push notifications. --- .../middleware/utils/NotificationUtils.java | 54 ++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index 7791bc11c..4ef10246a 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -9,18 +9,24 @@ import com.twilio.rest.verify.v2.service.VerificationCreator; import com.twilio.type.PhoneNumber; import freemarker.template.TemplateException; +import org.eclipse.jetty.http.HttpMethod; import org.opentripplanner.middleware.bugsnag.BugsnagReporter; import org.opentripplanner.middleware.models.AdminUser; import org.opentripplanner.middleware.models.OtpUser; +import org.opentripplanner.middleware.utils.HttpResponseValues; +import org.opentripplanner.middleware.utils.HttpUtils; +import org.opentripplanner.middleware.utils.JsonUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.IOException; +import java.net.URI; +import java.util.Map; import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsText; /** - * This class contains utils for sending SMS and email notifications. + * This class contains utils for sending SMS, email, and push notifications. * * TODO: It may be better to initialize all of these notification clients in a static block? This may not really be * necessary though -- needs some more research. @@ -36,6 +42,52 @@ public class NotificationUtils { private static final String SPARKPOST_KEY = getConfigPropertyAsText("SPARKPOST_KEY"); private static final String FROM_EMAIL = getConfigPropertyAsText("NOTIFICATION_FROM_EMAIL"); public static final String OTP_ADMIN_DASHBOARD_FROM_EMAIL = getConfigPropertyAsText("OTP_ADMIN_DASHBOARD_FROM_EMAIL"); + private static final String PUSH_API_KEY = getConfigPropertyAsText("PUSH_API_KEY"); + private static final String PUSH_API_URL = "https://demo.ibigroupmobile.com/ext_api"; + + /** + * @param otpUser target user + * @param textTemplate template to use for email in text format + * @param templateData template data + */ + public static String sendPush(OtpUser otpUser, String textTemplate, Object templateData) { + try { + String body = TemplateUtils.renderTemplate(textTemplate, templateData); + return sendPush(otpUser.email, body); + } catch (TemplateException | IOException e) { + // This catch indicates there was an error rendering the template. Note: TemplateUtils#renderTemplate + // handles Bugsnag reporting/error logging, so that is not needed here. + return null; + } + } + + /** + * Send a push notification message to the provided user + * @param toUser user account ID (email address) + * @param body message body + * @return "OK" if message was successful (null otherwise) + */ + public static String sendPush(String toUser, String body) { + try { + var jsonBody = "{\"user\":\"" + toUser + "\",\"message\":\"" + body + "\"}"; + Map headers = Map.of("Accept", "application/json"); + var httpResponse = HttpUtils.httpRequestRawResponse( + URI.create(PUSH_API_URL + "/devices/get?api_key=" + PUSH_API_KEY + "&user=" + toUser), + 1000, + HttpMethod.POST, + headers, + jsonBody + ); + if (httpResponse.status == 200) { + return "OK"; + } else { + LOG.error("Error {} while trying to initiate push notification", httpResponse.status); + } + } catch (Exception e) { + LOG.error("Could not initiate push notification", e); + } + return null; + } /** * Send templated SMS to {@link OtpUser}'s verified phone number. From 722143aaa32129b7654d1548b041282b5d63a7d3 Mon Sep 17 00:00:00 2001 From: Jym Dyer Date: Thu, 10 Aug 2023 14:05:07 -0700 Subject: [PATCH 07/20] Rely on PUSH_API_URL as config property for entire push API URL. --- .../org/opentripplanner/middleware/utils/NotificationUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index 4ef10246a..64cc16c21 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -43,7 +43,7 @@ public class NotificationUtils { private static final String FROM_EMAIL = getConfigPropertyAsText("NOTIFICATION_FROM_EMAIL"); public static final String OTP_ADMIN_DASHBOARD_FROM_EMAIL = getConfigPropertyAsText("OTP_ADMIN_DASHBOARD_FROM_EMAIL"); private static final String PUSH_API_KEY = getConfigPropertyAsText("PUSH_API_KEY"); - private static final String PUSH_API_URL = "https://demo.ibigroupmobile.com/ext_api"; + private static final String PUSH_API_URL = getConfigPropertyAsText("PUSH_API_URL"); /** * @param otpUser target user From a109dc6a47d9c0fb7b61cc3e5dea8eb7bd2c8f1a Mon Sep 17 00:00:00 2001 From: Jym Dyer Date: Thu, 10 Aug 2023 14:13:51 -0700 Subject: [PATCH 08/20] Add canSendPushNotification() test. --- .../utils/NotificationUtilsTest.java | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java b/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java index 5745e56e2..261adced1 100644 --- a/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java +++ b/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java @@ -20,9 +20,9 @@ import static org.opentripplanner.middleware.utils.NotificationUtils.OTP_ADMIN_DASHBOARD_FROM_EMAIL; /** - * Contains tests for the various notification utilities to send SMS and email messages. Note: these tests require the - * environment variables RUN_E2E=true and valid values for TEST_TO_EMAIL and TEST_TO_PHONE. Furthermore, TEST_TO_PHONE - * must be a verified phone number in a valid Twilio account. + * Contains tests for the various notification utilities to send SMS, email messages, and push notifications. + * Note: these tests require the environment variables RUN_E2E=true and valid values for TEST_TO_EMAIL, TEST_TO_PHONE, + * and TEST_TO_PUSH. Furthermore, TEST_TO_PHONE must be a verified phone number in a valid Twilio account. */ public class NotificationUtilsTest extends OtpMiddlewareTestEnvironment { private static final Logger LOG = LoggerFactory.getLogger(NotificationUtilsTest.class); @@ -35,10 +35,13 @@ public class NotificationUtilsTest extends OtpMiddlewareTestEnvironment { private static final String email = System.getenv("TEST_TO_EMAIL"); /** Phone must be in the form "+15551234" and must be verified first in order to send notifications */ private static final String phone = System.getenv("TEST_TO_PHONE"); + /** Push notification is conventionally a user.email value and must be known to the mobile team's push API */ + private static final String push = System.getenv("TEST_TO_PUSH"); /** * Currently, since these tests require target email/SMS values, these tests should not run on CI. */ - private static final boolean shouldTestsRun = !isRunningCi && IS_END_TO_END && email != null && phone != null; + private static final boolean shouldTestsRun = + !isRunningCi && IS_END_TO_END && email != null && phone != null && push != null; @BeforeAll public static void setup() throws IOException { @@ -51,6 +54,17 @@ public static void tearDown() { if (user != null) Persistence.otpUsers.removeById(user.id); } + @Test + public void canSendPushNotification() { + String ret = NotificationUtils.sendPush( + // Conventionally user.email + push, + "Tough little ship!" + ); + LOG.info("Push notification (ret={}) sent to {}", ret, push); + Assertions.assertNotNull(ret); + } + @Test public void canSendSparkpostEmailNotification() { boolean success = NotificationUtils.sendEmailViaSparkpost( From 24e825eab18515e0b4a58fa61ff125c708236c01 Mon Sep 17 00:00:00 2001 From: Jym Dyer Date: Thu, 10 Aug 2023 17:23:59 -0700 Subject: [PATCH 09/20] Generated with VersionControlledSwaggerUpdater --- src/main/resources/latest-spark-swagger-output.yaml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/main/resources/latest-spark-swagger-output.yaml b/src/main/resources/latest-spark-swagger-output.yaml index 2689cf6f4..5ea445db3 100644 --- a/src/main/resources/latest-spark-swagger-output.yaml +++ b/src/main/resources/latest-spark-swagger-output.yaml @@ -2548,11 +2548,20 @@ definitions: isPhoneNumberVerified: type: "boolean" notificationChannel: - type: "string" + type: "array" + items: + type: "string" + enum: + - "EMAIL" + - "PUSH" + - "SMS" phoneNumber: type: "string" preferredLocale: type: "string" + pushDevices: + type: "integer" + format: "int32" savedLocations: type: "array" items: From 1fa77f36c8dda4a3f6584f3a4d1e868da5eedc0b Mon Sep 17 00:00:00 2001 From: Jym Dyer Date: Thu, 10 Aug 2023 17:56:53 -0700 Subject: [PATCH 10/20] Update with PUSH_API_* properties. --- src/main/resources/env.schema.json | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/main/resources/env.schema.json b/src/main/resources/env.schema.json index 134b54883..88d186756 100644 --- a/src/main/resources/env.schema.json +++ b/src/main/resources/env.schema.json @@ -202,6 +202,16 @@ "examples": ["https://plan.example.com"], "description": "Config setting for linking to the OTP UI (trip planner)." }, + "PUSH_API_KEY": { + "type": "string", + "examples": ["your-api-key"], + "description": "Key for Mobile Team push notifications internal API." + }, + "PUSH_API_URL": { + "type": "string", + "examples": ["https://example.com/api/otp_push/sound_transit"], + "description": "URL for Mobile Team push notifications internal API." + }, "SERVICE_DAY_START_HOUR": { "type": "integer", "examples": ["3"], From 13893ddc27812adabaecddce77c85aa6d6f90e1d Mon Sep 17 00:00:00 2001 From: Jym Dyer Date: Thu, 17 Aug 2023 14:00:53 -0700 Subject: [PATCH 11/20] Generated with ReadMeEnvSchemaValuesUpdater --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 16fa3dced..2cb6a8baa 100644 --- a/README.md +++ b/README.md @@ -258,6 +258,8 @@ The special E2E client settings should be defined in `env.yml`: | OTP_TIMEZONE | string | Required | America/Los_Angeles | The timezone identifier that OTP is using to parse dates and times. OTP will use the timezone identifier that it finds in the first available agency to parse dates and times. | | OTP_UI_NAME | string | Optional | Trip Planner | Config setting for linking to the OTP UI (trip planner). | | OTP_UI_URL | string | Optional | https://plan.example.com | Config setting for linking to the OTP UI (trip planner). | +| PUSH_API_KEY | string | Optional | your-api-key | Key for Mobile Team push notifications internal API. | +| PUSH_API_URL | string | Optional | https://example.com/api/otp_push/sound_transit | URL for Mobile Team push notifications internal API. | | SERVICE_DAY_START_HOUR | integer | Optional | 3 | Optional parameter for the hour (local time, 24-hr format) at which a service day starts. To make the service day change at 2am, enter 2. The default is 3am. | | SPARKPOST_KEY | string | Optional | your-api-key | Get Sparkpost key at: https://app.sparkpost.com/account/api-keys | | TWILIO_ACCOUNT_SID | string | Optional | your-account-sid | Twilio settings available at: https://twilio.com/user/account | From 3503ee25f07ef61c95883957087bab3356d399cc Mon Sep 17 00:00:00 2001 From: Jym Dyer Date: Thu, 17 Aug 2023 14:35:05 -0700 Subject: [PATCH 12/20] Space formatting. --- .../org/opentripplanner/middleware/utils/NotificationUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index 64cc16c21..340d93bfc 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -48,7 +48,7 @@ public class NotificationUtils { /** * @param otpUser target user * @param textTemplate template to use for email in text format - * @param templateData template data + * @param templateData template data */ public static String sendPush(OtpUser otpUser, String textTemplate, Object templateData) { try { From d6c484fe35bc459aa212c90ab0cf9f8ad6683227 Mon Sep 17 00:00:00 2001 From: Jym Dyer Date: Mon, 21 Aug 2023 12:42:54 -0700 Subject: [PATCH 13/20] Add in `get` endpoint to maintain number of push notification devices. --- .../middleware/utils/NotificationUtils.java | 37 +++++++++++++++++-- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index 340d93bfc..757c7928a 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -13,7 +13,6 @@ import org.opentripplanner.middleware.bugsnag.BugsnagReporter; import org.opentripplanner.middleware.models.AdminUser; import org.opentripplanner.middleware.models.OtpUser; -import org.opentripplanner.middleware.utils.HttpResponseValues; import org.opentripplanner.middleware.utils.HttpUtils; import org.opentripplanner.middleware.utils.JsonUtils; import org.slf4j.Logger; @@ -53,7 +52,11 @@ public class NotificationUtils { public static String sendPush(OtpUser otpUser, String textTemplate, Object templateData) { try { String body = TemplateUtils.renderTemplate(textTemplate, templateData); - return sendPush(otpUser.email, body); + String toUser = otpUser.email; + // Calls the `get` endpoint, the only reliable way to get number of push notification devices, + // as the `publish` endpoint returns success for zero devices and/or if devices turn them off. + otpUser.pushDevices = getPushInfo(toUser); + return otpUser.pushDevices > 0 ? sendPush(toUser, body) : "OK"; } catch (TemplateException | IOException e) { // This catch indicates there was an error rendering the template. Note: TemplateUtils#renderTemplate // handles Bugsnag reporting/error logging, so that is not needed here. @@ -72,7 +75,7 @@ public static String sendPush(String toUser, String body) { var jsonBody = "{\"user\":\"" + toUser + "\",\"message\":\"" + body + "\"}"; Map headers = Map.of("Accept", "application/json"); var httpResponse = HttpUtils.httpRequestRawResponse( - URI.create(PUSH_API_URL + "/devices/get?api_key=" + PUSH_API_KEY + "&user=" + toUser), + URI.create(PUSH_API_URL + "/notification/publish?api_key=" + PUSH_API_KEY), 1000, HttpMethod.POST, headers, @@ -274,5 +277,31 @@ public static boolean sendEmailViaSparkpost( return false; } } -} + /** + * @param toUser email address of user that devices are indexed by + */ + public static int getPushInfo(String toUser) { + try { + Map headers = Map.of("Accept", "application/json"); + var httpResponse = HttpUtils.httpRequestRawResponse( + URI.create(PUSH_API_URL + "/devices/get?api_key=" + PUSH_API_KEY + "&user=" + toUser), + 1000, + HttpMethod.GET, + headers, + null + ); + if (httpResponse.status == 200) { + // We don't use any of this information, we only care how many devices are registered. + var devices = JsonUtils.getPOJOFromHttpBodyAsList(httpResponse, Object.class); + return devices.size(); + } else { + LOG.error("Error {} while getting info on push notification devices", httpResponse.status); + } + } catch (Exception e) { + LOG.error("No info on push notification devices", e); + } + return 0; + } + +} From 759d21a4cfab9bc4c4f38a9d3a3a0abfd6d80adf Mon Sep 17 00:00:00 2001 From: Jym Dyer Date: Wed, 23 Aug 2023 15:51:52 -0700 Subject: [PATCH 14/20] Persistence of pushDevices --- .../org/opentripplanner/middleware/utils/NotificationUtils.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index 757c7928a..0cc8debd3 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -56,6 +56,7 @@ public static String sendPush(OtpUser otpUser, String textTemplate, Object templ // Calls the `get` endpoint, the only reliable way to get number of push notification devices, // as the `publish` endpoint returns success for zero devices and/or if devices turn them off. otpUser.pushDevices = getPushInfo(toUser); + Persistence.otpUsers.replace(otpUser.id, otpUser); return otpUser.pushDevices > 0 ? sendPush(toUser, body) : "OK"; } catch (TemplateException | IOException e) { // This catch indicates there was an error rendering the template. Note: TemplateUtils#renderTemplate From ce21e89c0173acf49eab1938ea010216f2aa2b12 Mon Sep 17 00:00:00 2001 From: Jym Dyer Date: Wed, 23 Aug 2023 15:53:30 -0700 Subject: [PATCH 15/20] Persistence of pushDevices --- .../org/opentripplanner/middleware/utils/NotificationUtils.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index 0cc8debd3..aa70a2981 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -13,6 +13,7 @@ import org.opentripplanner.middleware.bugsnag.BugsnagReporter; import org.opentripplanner.middleware.models.AdminUser; import org.opentripplanner.middleware.models.OtpUser; +import org.opentripplanner.middleware.persistence.Persistence; import org.opentripplanner.middleware.utils.HttpUtils; import org.opentripplanner.middleware.utils.JsonUtils; import org.slf4j.Logger; From a3e08ab5a912c9a8a2458ecf58803136f850c9f7 Mon Sep 17 00:00:00 2001 From: Jym Dyer Date: Thu, 24 Aug 2023 14:21:48 -0700 Subject: [PATCH 16/20] Move NotificationUtils#getPushInfo() call into CheckMonitoredTrip class. --- .../middleware/tripmonitor/jobs/CheckMonitoredTrip.java | 7 +++++++ .../middleware/utils/NotificationUtils.java | 8 +++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java index 2ae84ee8f..f44ee17d8 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java @@ -407,6 +407,13 @@ private void sendNotifications() { boolean successPush = false; boolean successSms = false; + // Update push notification devices count, which may change asynchronously + int numPushDevices = NotificationUtils.getPushInfo(otpUser.email); + if (numPushDevices != otpUser.pushDevices) { + otpUser.pushDevices = numPushDevices; + Persistence.otpUsers.replace(otpUser.id, otpUser); + } + if (otpUser.notificationChannel.contains(OtpUser.Notification.EMAIL)) { successEmail = sendEmail(otpUser, templateData); } diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index aa70a2981..ae0299a37 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -13,7 +13,6 @@ import org.opentripplanner.middleware.bugsnag.BugsnagReporter; import org.opentripplanner.middleware.models.AdminUser; import org.opentripplanner.middleware.models.OtpUser; -import org.opentripplanner.middleware.persistence.Persistence; import org.opentripplanner.middleware.utils.HttpUtils; import org.opentripplanner.middleware.utils.JsonUtils; import org.slf4j.Logger; @@ -54,10 +53,6 @@ public static String sendPush(OtpUser otpUser, String textTemplate, Object templ try { String body = TemplateUtils.renderTemplate(textTemplate, templateData); String toUser = otpUser.email; - // Calls the `get` endpoint, the only reliable way to get number of push notification devices, - // as the `publish` endpoint returns success for zero devices and/or if devices turn them off. - otpUser.pushDevices = getPushInfo(toUser); - Persistence.otpUsers.replace(otpUser.id, otpUser); return otpUser.pushDevices > 0 ? sendPush(toUser, body) : "OK"; } catch (TemplateException | IOException e) { // This catch indicates there was an error rendering the template. Note: TemplateUtils#renderTemplate @@ -281,6 +276,9 @@ public static boolean sendEmailViaSparkpost( } /** + * Get number of push notification devices. Calls Push API's get endpoint, the only reliable way + * to obtain this value, as the publish endpoint returns success even for zero devices. + * * @param toUser email address of user that devices are indexed by */ public static int getPushInfo(String toUser) { From fb0df0092f61a2c21e09dbc74821f2a5989c3c5d Mon Sep 17 00:00:00 2001 From: Jym Dyer Date: Mon, 28 Aug 2023 10:57:08 -0700 Subject: [PATCH 17/20] Update number of push devices sooner (per binh's PR comment). --- .../tripmonitor/jobs/CheckMonitoredTrip.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java index f44ee17d8..d591f4fc0 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java @@ -389,6 +389,12 @@ private void sendNotifications() { // TODO: Bugsnag / delete monitored trip? return; } + // Update push notification devices count, which may change asynchronously + int numPushDevices = NotificationUtils.getPushInfo(otpUser.email); + if (numPushDevices != otpUser.pushDevices) { + otpUser.pushDevices = numPushDevices; + Persistence.otpUsers.replace(otpUser.id, otpUser); + } // If the same notifications were just sent, there is no need to send the same notification. // TODO: Should there be some time threshold check here based on lastNotificationTime? if (previousJourneyState.lastNotifications.containsAll(notifications)) { @@ -407,13 +413,6 @@ private void sendNotifications() { boolean successPush = false; boolean successSms = false; - // Update push notification devices count, which may change asynchronously - int numPushDevices = NotificationUtils.getPushInfo(otpUser.email); - if (numPushDevices != otpUser.pushDevices) { - otpUser.pushDevices = numPushDevices; - Persistence.otpUsers.replace(otpUser.id, otpUser); - } - if (otpUser.notificationChannel.contains(OtpUser.Notification.EMAIL)) { successEmail = sendEmail(otpUser, templateData); } From be241ed8e284281eb6420adb419a7a030c23d297 Mon Sep 17 00:00:00 2001 From: Jym Dyer Date: Mon, 28 Aug 2023 10:58:31 -0700 Subject: [PATCH 18/20] Actually, put it very near the very top, after getting `otpUser`. --- .../tripmonitor/jobs/CheckMonitoredTrip.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java index d591f4fc0..5c3f15f68 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java @@ -378,11 +378,6 @@ public TripMonitorNotification checkTripForDelay(NotificationType delayType) { * preferences. */ private void sendNotifications() { - if (notifications.size() == 0) { - // FIXME: Change log level - LOG.info("No notifications queued for trip. Skipping notify."); - return; - } OtpUser otpUser = Persistence.otpUsers.getById(trip.userId); if (otpUser == null) { LOG.error("Cannot find user for id {}", trip.userId); @@ -395,6 +390,11 @@ private void sendNotifications() { otpUser.pushDevices = numPushDevices; Persistence.otpUsers.replace(otpUser.id, otpUser); } + if (notifications.size() == 0) { + // FIXME: Change log level + LOG.info("No notifications queued for trip. Skipping notify."); + return; + } // If the same notifications were just sent, there is no need to send the same notification. // TODO: Should there be some time threshold check here based on lastNotificationTime? if (previousJourneyState.lastNotifications.containsAll(notifications)) { From 38660e634dcf3a7949d9ead4c3f6f19342099e4b Mon Sep 17 00:00:00 2001 From: Jym Dyer Date: Thu, 31 Aug 2023 14:23:13 -0700 Subject: [PATCH 19/20] Make public static methods contingent on setting PUSH_API_KEY/PUSH_API_URL. Make inner sendPush() method package scope. --- .../middleware/utils/NotificationUtils.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index ae0299a37..c84387da3 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -50,6 +50,8 @@ public class NotificationUtils { * @param templateData template data */ public static String sendPush(OtpUser otpUser, String textTemplate, Object templateData) { + // If Push API config properties aren't set, do nothing. + if (PUSH_API_KEY.equals(null) || PUSH_API_URL.equals(null)) return null; try { String body = TemplateUtils.renderTemplate(textTemplate, templateData); String toUser = otpUser.email; @@ -67,7 +69,7 @@ public static String sendPush(OtpUser otpUser, String textTemplate, Object templ * @param body message body * @return "OK" if message was successful (null otherwise) */ - public static String sendPush(String toUser, String body) { + static String sendPush(String toUser, String body) { try { var jsonBody = "{\"user\":\"" + toUser + "\",\"message\":\"" + body + "\"}"; Map headers = Map.of("Accept", "application/json"); @@ -280,8 +282,11 @@ public static boolean sendEmailViaSparkpost( * to obtain this value, as the publish endpoint returns success even for zero devices. * * @param toUser email address of user that devices are indexed by + * @return number of devices registered, 0 can mean zero devices or an error obtaining the number */ public static int getPushInfo(String toUser) { + // If Push API config properties aren't set, no info can be obtained. + if (PUSH_API_KEY.equals(null) || PUSH_API_URL.equals(null)) return 0; try { Map headers = Map.of("Accept", "application/json"); var httpResponse = HttpUtils.httpRequestRawResponse( From 17ba7fb24293a725b2e76833b8ea5943226c331d Mon Sep 17 00:00:00 2001 From: Jym Dyer Date: Thu, 31 Aug 2023 14:35:19 -0700 Subject: [PATCH 20/20] Make public static methods contingent on setting PUSH_API_KEY/PUSH_API_URL. Make inner sendPush() method package scope. --- .../opentripplanner/middleware/utils/NotificationUtils.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index c84387da3..4b42d9efd 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -51,7 +51,7 @@ public class NotificationUtils { */ public static String sendPush(OtpUser otpUser, String textTemplate, Object templateData) { // If Push API config properties aren't set, do nothing. - if (PUSH_API_KEY.equals(null) || PUSH_API_URL.equals(null)) return null; + if (PUSH_API_KEY == null || PUSH_API_URL == null) return null; try { String body = TemplateUtils.renderTemplate(textTemplate, templateData); String toUser = otpUser.email; @@ -286,7 +286,7 @@ public static boolean sendEmailViaSparkpost( */ public static int getPushInfo(String toUser) { // If Push API config properties aren't set, no info can be obtained. - if (PUSH_API_KEY.equals(null) || PUSH_API_URL.equals(null)) return 0; + if (PUSH_API_KEY == null || PUSH_API_URL == null) return 0; try { Map headers = Map.of("Accept", "application/json"); var httpResponse = HttpUtils.httpRequestRawResponse(