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

Implement message-type Android push notifications #4270

Merged
merged 18 commits into from
Oct 18, 2021
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
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
3 changes: 3 additions & 0 deletions android/app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,8 @@
</intent-filter>
</activity>
<activity android:name="com.facebook.react.devsupport.DevSettingsActivity" />

<meta-data android:name="com.urbanairship.reactnative.AIRSHIP_EXTENDER"
android:value="com.expensify.chat.CustomAirshipExtender" />
</application>
</manifest>
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package com.expensify.chat;

import android.content.Context;
import androidx.annotation.NonNull;
import com.urbanairship.UAirship;
import com.urbanairship.push.NotificationListener;
import com.urbanairship.push.PushManager;
import com.urbanairship.reactnative.AirshipExtender;

public class CustomAirshipExtender implements AirshipExtender {
Julesssss marked this conversation as resolved.
Show resolved Hide resolved
@Override
public void onAirshipReady(@NonNull Context context, @NonNull UAirship airship) {
PushManager pushManager = airship.getPushManager();

CustomNotificationProvider notificationProvider = new CustomNotificationProvider(context, airship.getAirshipConfigOptions());
pushManager.setNotificationProvider(notificationProvider);

NotificationListener notificationListener = airship.getPushManager().getNotificationListener();
pushManager.setNotificationListener(new CustomNotificationListener(notificationListener, notificationProvider));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package com.expensify.chat;

import androidx.annotation.NonNull;
import com.urbanairship.push.NotificationActionButtonInfo;
import com.urbanairship.push.NotificationInfo;
import com.urbanairship.push.NotificationListener;
import com.urbanairship.push.PushMessage;
import org.jetbrains.annotations.NotNull;

/**
* Allows us to clear the notification cache when the user dismisses a notification.
*/
public class CustomNotificationListener implements NotificationListener {
private final NotificationListener parent;
private final CustomNotificationProvider provider;

CustomNotificationListener(NotificationListener parent, CustomNotificationProvider provider) {
this.parent = parent;
this.provider = provider;
}

@Override
public void onNotificationPosted(@NonNull @NotNull NotificationInfo notificationInfo) {
parent.onNotificationPosted(notificationInfo);
}

@Override
public boolean onNotificationOpened(@NonNull @NotNull NotificationInfo notificationInfo) {
return parent.onNotificationOpened(notificationInfo);
}

@Override
public boolean onNotificationForegroundAction(@NonNull @NotNull NotificationInfo notificationInfo, @NonNull @NotNull NotificationActionButtonInfo actionButtonInfo) {
return parent.onNotificationForegroundAction(notificationInfo, actionButtonInfo);
}

@Override
public void onNotificationBackgroundAction(@NonNull @NotNull NotificationInfo notificationInfo, @NonNull @NotNull NotificationActionButtonInfo actionButtonInfo) {
parent.onNotificationBackgroundAction(notificationInfo, actionButtonInfo);
}

@Override
public void onNotificationDismissed(@NonNull @NotNull NotificationInfo notificationInfo) {
parent.onNotificationDismissed(notificationInfo);

PushMessage message = notificationInfo.getMessage();
provider.onDismissNotification(message);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,242 @@
package com.expensify.chat;

import android.content.Context;
import android.graphics.Bitmap;
import android.util.DisplayMetrics;
import android.util.Log;
import android.util.TypedValue;
import android.view.WindowManager;
import androidx.annotation.NonNull;
import androidx.core.app.NotificationCompat;
import androidx.core.app.NotificationManagerCompat;
import androidx.core.app.Person;
import androidx.core.graphics.drawable.IconCompat;
import com.urbanairship.AirshipConfigOptions;
import com.urbanairship.json.JsonMap;
import com.urbanairship.json.JsonValue;
import com.urbanairship.push.PushMessage;
import com.urbanairship.push.notifications.NotificationArguments;
import com.urbanairship.reactnative.ReactNotificationProvider;
import com.urbanairship.util.ImageUtils;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;

public class CustomNotificationProvider extends ReactNotificationProvider {

// Resize icons to 100 dp x 100 dp
private static final int MAX_ICON_SIZE_DPS = 100;

// Max wait time to resolve an icon. We have ~10 seconds to a little less
// to ensure the notification builds.
Comment on lines +36 to +37
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to ensure the notification builds

What does this mean? Is there a hard limit of 10 seconds?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's related to this: https://developer.android.com/about/versions/12/behavior-changes-all#foreground-service-notification-delay

To provide a streamlined experience for short-running foreground services on Android 12, the system can delay the display of foreground service notifications by 10 seconds for certain foreground services. This change gives short-lived tasks a chance to complete before their notifications appear.

Copy link
Contributor

@Julesssss Julesssss Aug 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Android 12 change only applies to foreground services. Regular notifications have no such limit and should always display -- unless the UA implementation is doing something very weird.

Copy link
Contributor

@Julesssss Julesssss Aug 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Android is happy for us to do stuff when the app is backgrounded, provided we display a long-running notification (like FB and WhatsApp 'Checking for messages...' notification). But that is very different from what we're trying to do here, which is just display a notification.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this then; we should ask the UA team. Still, delaying notifications +10 seconds because we're fetching an avatar doesn't seem like a good idea even in bad network conditions.

private static final int MAX_ICON_FETCH_WAIT_TIME_SECONDS = 8;

// Logging
private static final String TAG = "NotificationProvider";

// Conversation JSON keys
private static final String PAYLOAD_KEY = "payload";
private static final String TYPE_KEY = "type";
private static final String REPORT_COMMENT_TYPE = "reportComment";

private final ExecutorService executorService = Executors.newCachedThreadPool();
public final HashMap<Integer, NotificationCache> cache = new HashMap<>();

public CustomNotificationProvider(@NonNull Context context, @NonNull AirshipConfigOptions configOptions) {
super(context, configOptions);
}

@NonNull
@Override
protected NotificationCompat.Builder onExtendBuilder(@NonNull Context context, @NonNull NotificationCompat.Builder builder, @NonNull NotificationArguments arguments) {
super.onExtendBuilder(context, builder, arguments);
PushMessage message = arguments.getMessage();

if (message.containsKey(PAYLOAD_KEY)) {
try {
JsonMap payload = JsonValue.parseString(message.getExtra(PAYLOAD_KEY)).optMap();

// Apply message style only for report comments
if (REPORT_COMMENT_TYPE.equals(payload.get(TYPE_KEY).getString())) {
applyMessageStyle(builder, payload, arguments.getNotificationId());
}
} catch (Exception e) {
Log.e(TAG, "Failed to parse conversation. SendID=" + message.getSendId(), e);
}
}

return builder;
}

/**
* Applies the message style to the notification builder. It also takes advantage of the
* notification cache to build conversations.
*
* @param builder Notification builder that will receive the message style
* @param payload Notification payload, which contains all the data we need to build the notifications.
* @param notificationID Current notification ID
*/
private void applyMessageStyle(NotificationCompat.Builder builder, JsonMap payload, int notificationID) {
int reportID = payload.get("reportID").getInt(-1);
Julesssss marked this conversation as resolved.
Show resolved Hide resolved
if (reportID == -1) {
return;
}

NotificationCache notificationCache = findOrCreateNotificationCache(reportID);
JsonMap reportAction = payload.get("reportAction").getMap();
String name = reportAction.get("person").getList().get(0).getMap().get("text").getString();
String avatar = reportAction.get("avatar").getString();
String accountID = Integer.toString(reportAction.get("actorAccountID").getInt(-1));
String message = reportAction.get("message").getList().get(0).getMap().get("text").getString();
long time = reportAction.get("timestamp").getLong(0);
String roomName = payload.get("roomName") == null ? "" : payload.get("roomName").getString("");
String conversationTitle = "Chat with " + name;
Julesssss marked this conversation as resolved.
Show resolved Hide resolved
if (!roomName.isEmpty()) {
conversationTitle = "#" + roomName;
}

// Retrieve or create the Person object who sent the latest report comment
Person person = notificationCache.people.get(accountID);
if (person == null) {
IconCompat iconCompat = fetchIcon(avatar);
person = new Person.Builder()
.setIcon(iconCompat)
.setKey(accountID)
.setName(name)
.build();

notificationCache.people.put(accountID, person);
}

// Store the latest report comment in the local conversation history
notificationCache.messages.add(new NotificationCache.Message(person, message, time));

// Create the messaging style notification builder for this notification.
// Associate the notification with the person who sent the report comment.
// If this conversation has 2 participants or more and there's no room name, we should mark
// it as a group conversation.
// Also set the conversation title.
NotificationCompat.MessagingStyle messagingStyle = new NotificationCompat.MessagingStyle(person)
.setGroupConversation(notificationCache.people.size() > 2 || !roomName.isEmpty())
.setConversationTitle(conversationTitle);

// Add all conversation messages to the notification, including the last one we just received.
for (NotificationCache.Message cachedMessage : notificationCache.messages) {
messagingStyle.addMessage(cachedMessage.text, cachedMessage.time, cachedMessage.person);
}

// Clear the previous notification associated to this conversation so it looks like we are
// replacing them with this new one we just built.
if (notificationCache.prevNotificationID != -1) {
NotificationManagerCompat.from(context).cancel(notificationCache.prevNotificationID);
}

// Apply the messaging style to the notification builder
builder.setStyle(messagingStyle);

// Store the new notification ID so we can replace the notification if this conversation
// receives more messages
notificationCache.prevNotificationID = notificationID;
}

/**
* Check if we are showing a notification related to a reportID.
* If not, create a new NotificationCache so we can build a conversation notification
* as the messages come.
*
* @param reportID Report ID.
* @return Notification Cache.
*/
private NotificationCache findOrCreateNotificationCache(int reportID) {
NotificationCache notificationCache = cache.get(reportID);

if (notificationCache == null) {
notificationCache = new NotificationCache();
cache.put(reportID, notificationCache);
}

return notificationCache;
}

/**
* Remove the notification data from the cache when the user dismisses the notification.
*
* @param message Push notification's message
*/
public void onDismissNotification(PushMessage message) {
try {
JsonMap payload = JsonValue.parseString(message.getExtra(PAYLOAD_KEY)).optMap();
int reportID = payload.get("reportID").getInt(-1);

if (reportID == -1) {
return;
}

cache.remove(reportID);
} catch (Exception e) {
Log.e(TAG, "Failed to delete conversation cache. SendID=" + message.getSendId(), e);
}
}

private IconCompat fetchIcon(String urlString) {
// TODO: Add disk LRU cache
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't leave TODO's. Does this still need to be done?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure, and this was one of the things that I didn't touch at all. It would be interesting to store avatar data in the disk to avoid fetching from the network every time we receive a new notification that belongs to a different person.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove the TODO in that case, as it sounds like it is not a priority of ours

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!


URL parsedUrl = null;
try {
parsedUrl = urlString == null ? null : new URL(urlString);
Julesssss marked this conversation as resolved.
Show resolved Hide resolved
} catch (MalformedURLException e) {
Log.e(TAG, "Failed to resolve URL " + urlString, e);
}

if (parsedUrl == null) {
return null;
}

WindowManager window = (WindowManager) context.getSystemService(Context.WINDOW_SERVICE);
DisplayMetrics dm = new DisplayMetrics();
window.getDefaultDisplay().getMetrics(dm);

final int reqWidth = (int) TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, MAX_ICON_SIZE_DPS, dm);
final int reqHeight = (int) TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, MAX_ICON_SIZE_DPS, dm);

final URL url = parsedUrl;
Future<Bitmap> future = executorService.submit(() -> ImageUtils.fetchScaledBitmap(context, url, reqWidth, reqHeight));

try {
Bitmap bitmap = future.get(MAX_ICON_FETCH_WAIT_TIME_SECONDS, TimeUnit.SECONDS);
return IconCompat.createWithBitmap(bitmap);
} catch (InterruptedException e) {
Log.e(TAG,"Failed to fetch icon", e);
Thread.currentThread().interrupt();
} catch (Exception e) {
Log.e(TAG,"Failed to fetch icon", e);
future.cancel(true);
}

return null;
}

private static class NotificationCache {
Comment on lines +222 to +223
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, why does this need to be static? Is this to ensure only one instance of NotificationCache exists?

Even as a static class, I would have thought that the cache would be lost when the app was killed? Unless CustomNotificationProvider has a lifecycle that extends beyond the life of the app? 😕

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are multiple instances of NotificationCache, one for each conversation that gets shown to the user in a notification. We only remove the cache after the user dismisses or opens a notification.

From my understanding, the CustomNotificationProvider lives after the app gets killed as a background service. I don't know how reliable it is, though, as I have no experience with Android dev. If we shouldn't rely on it, maybe it would be interesting to store the notification cache data in SQLite or something similar?

Copy link
Contributor

@Julesssss Julesssss Aug 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are multiple instances of NotificationCache, one for each conversation that gets shown to the user in a notification.

In which case, I don't think the class should be static.

If we shouldn't rely on it, maybe it would be interesting to store the notification cache data in SQLite or something similar?

That still leaves a dependency on a DB that will need to be run in the background. Ideally the data should be sent in the push notification payload, so we would send all unread messages.

CustomNotificationProvider lives after the app gets killed as a background service.

This would be odd if true. Depending on how they implemented this there is no guarantee that it will remain alive. Would you mind sharing the documentation for this? Or any links you have to the code they gave us.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't allow me to remove the static modifier of the class, and if I do it, we won't be able to do new NotificationCache.Message(...);


Yeah, I didn't explain correctly. This service gets launched when a notification arrives, but it will die after a while; as you say, we have no guarantee.

We can't send all data in the notification payload because of the payload size limits (~10KB). That's why it's better to cache the notification conversations on the device. I guess using the shared preferences could be enough for this use case instead of using a DB.

Also, having the conversation stored on the device will make it easier for us when we want to implement the quick reply from the notification.

Copy link
Contributor

@Julesssss Julesssss Aug 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that they are making the cache class static so that it's lifecycle extends that of each individual ReactNotificationProvider instance. It works I guess, but it's a bit hacky and not guaranteed to be persisted.

Copy link
Contributor

@Julesssss Julesssss Aug 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't send all data in the notification payload because of the payload size limits (~10KB)

But surely 10KB is fine for some simple objects? We don't need the full profile image but simply the URL, no?

public Map<String, Person> people = new HashMap<>();
public ArrayList<Message> messages = new ArrayList<>();
public int prevNotificationID = -1;

public static class Message {
public Person person;
public String text;
public long time;

Message(Person person, String text, long time) {
this.person = person;
this.text = text;
this.time = time;
}
}
}
}