Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple notification channels in middleware #185

Merged
merged 20 commits into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
4321270
Support multiple notification channels.
May 17, 2023
0644054
Handle "none" value in notification channel.
JymDyerIBI May 17, 2023
2715c00
Push notifications.
JymDyerIBI Jun 8, 2023
c3a96c7
Add pushDevices (number of push devices last we looked)
JymDyerIBI Aug 10, 2023
5f28501
Add pushDevices (number of push devices) to test
JymDyerIBI Aug 10, 2023
eb2d207
Add sendPush() method for push notifications.
JymDyerIBI Aug 10, 2023
722143a
Rely on PUSH_API_URL as config property for entire push API URL.
JymDyerIBI Aug 10, 2023
a109dc6
Add canSendPushNotification() test.
JymDyerIBI Aug 10, 2023
24e825e
Generated with VersionControlledSwaggerUpdater
JymDyerIBI Aug 11, 2023
1fa77f3
Update with PUSH_API_* properties.
JymDyerIBI Aug 11, 2023
13893dd
Generated with ReadMeEnvSchemaValuesUpdater
JymDyerIBI Aug 17, 2023
3503ee2
Space formatting.
JymDyerIBI Aug 17, 2023
d6c484f
Add in `get` endpoint to maintain number of push notification devices.
JymDyerIBI Aug 21, 2023
759d21a
Persistence of pushDevices
JymDyerIBI Aug 23, 2023
ce21e89
Persistence of pushDevices
JymDyerIBI Aug 23, 2023
a3e08ab
Move NotificationUtils#getPushInfo() call into CheckMonitoredTrip class.
JymDyerIBI Aug 24, 2023
fb0df00
Update number of push devices sooner (per binh's PR comment).
JymDyerIBI Aug 28, 2023
be241ed
Actually, put it very near the very top, after getting `otpUser`.
JymDyerIBI Aug 28, 2023
38660e6
Make public static methods contingent on setting PUSH_API_KEY/PUSH_AP…
JymDyerIBI Aug 31, 2023
17ba7fb
Make public static methods contingent on setting PUSH_API_KEY/PUSH_AP…
JymDyerIBI Aug 31, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
54 changes: 48 additions & 6 deletions src/main/java/org/opentripplanner/middleware/models/OtpUser.java
Original file line number Diff line number Diff line change
@@ -1,21 +1,28 @@
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).
* otp-middleware stores these users and associated information (e.g., home/work locations and other favorites). Users
* 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);
Expand All @@ -30,11 +37,10 @@ public class OtpUser extends AbstractUser {
public boolean isPhoneNumberVerified;
JymDyerIBI marked this conversation as resolved.
Show resolved Hide resolved

/**
* 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<OtpUser.Notification> notificationChannel = EnumSet.noneOf(OtpUser.Notification.class);

/**
* Verified phone number for SMS notifications, in +15551234 format (E.164 format, includes country code, no spaces).
Expand All @@ -47,6 +53,11 @@ public class OtpUser extends AbstractUser {
*/
public String preferredLocale;

/**
* Number of push devices associated with user email
*/
public int pushDevices;

/** Locations that the user has saved. */
public List<UserLocation> savedLocations = new ArrayList<>();

Expand Down Expand Up @@ -105,4 +116,35 @@ 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) {
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);
}
});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -378,17 +378,23 @@ 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);
// TODO: Bugsnag / delete monitored trip?
return;
}
// Update push notification devices count, which may change asynchronously
int numPushDevices = NotificationUtils.getPushInfo(otpUser.email);
JymDyerIBI marked this conversation as resolved.
Show resolved Hide resolved
if (numPushDevices != otpUser.pushDevices) {
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)) {
Expand All @@ -403,23 +409,22 @@ 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 successPush = false;
boolean successSms = false;

JymDyerIBI marked this conversation as resolved.
Show resolved Hide resolved
if (otpUser.notificationChannel.contains(OtpUser.Notification.EMAIL)) {
successEmail = sendEmail(otpUser, templateData);
}
if (otpUser.notificationChannel.contains(OtpUser.Notification.PUSH)) {
successPush = sendPush(otpUser, templateData);
}
if (success) {
if (otpUser.notificationChannel.contains(OtpUser.Notification.SMS)) {
successSms = sendSMS(otpUser, templateData);
}

// TODO: better handle below when one of the following fails
if (successEmail || successPush || successSms) {
notificationTimestampMillis = DateTimeUtils.currentTimeMillis();
}
}
Expand All @@ -431,6 +436,13 @@ private boolean sendSMS(OtpUser otpUser, Map<String, Object> data) {
return NotificationUtils.sendSMS(otpUser, "MonitoredTripSms.ftl", data) != null;
}

/**
* Send push notification.
*/
private boolean sendPush(OtpUser otpUser, Map<String, Object> data) {
return NotificationUtils.sendPush(otpUser, "MonitoredTripText.ftl", data) != null;
}

/**
* Send notification email in MonitoredTrip template.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,23 @@
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.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.
Expand All @@ -36,6 +41,55 @@ 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 = getConfigPropertyAsText("PUSH_API_URL");

/**
* @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) {
// If Push API config properties aren't set, do nothing.
if (PUSH_API_KEY == null || PUSH_API_URL == null) return null;
try {
String body = TemplateUtils.renderTemplate(textTemplate, templateData);
String toUser = otpUser.email;
return otpUser.pushDevices > 0 ? sendPush(toUser, body) : "OK";
binh-dam-ibigroup marked this conversation as resolved.
Show resolved Hide resolved
} 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)
*/
static String sendPush(String toUser, String body) {
try {
var jsonBody = "{\"user\":\"" + toUser + "\",\"message\":\"" + body + "\"}";
Map<String, String> headers = Map.of("Accept", "application/json");
var httpResponse = HttpUtils.httpRequestRawResponse(
URI.create(PUSH_API_URL + "/notification/publish?api_key=" + PUSH_API_KEY),
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.
Expand Down Expand Up @@ -222,5 +276,37 @@ public static boolean sendEmailViaSparkpost(
return false;
}
}
}

/**
* Get number of push notification devices. Calls Push API's <code>get</code> endpoint, the only reliable way
* to obtain this value, as the <code>publish</code> endpoint returns success even for zero devices.
*
* @param toUser email address of user that devices are indexed by
* @return number of devices registered, <code>0</code> 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 == null || PUSH_API_URL == null) return 0;
try {
Map<String, String> 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;
}

}
10 changes: 10 additions & 0 deletions src/main/resources/env.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Expand Down
11 changes: 10 additions & 1 deletion src/main/resources/latest-spark-swagger-output.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,10 @@ 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;
user.pushDevices = 0;
Persistence.otpUsers.create(user);
return user;
}
Expand Down
Loading
Loading