Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
17 changes: 15 additions & 2 deletions ios/NotificationService/IosNotifications.g.swift
Original file line number Diff line number Diff line change
Expand Up @@ -140,24 +140,37 @@ struct NotificationContent: Hashable {
struct ImprovedNotificationContent: Hashable {
/// The new title to use for the notification.
var title: String
/// The new subtitle to use for the notification.
var subtitle: String
/// The new body to use for the notification.
var body: String
/// The internal data to attach with the new notification.
///
/// This replaces the raw APNs payload that was initially set from
/// the remote push notification.
var userInfo: [String: Any?]


// swift-format-ignore: AlwaysUseLowerCamelCase
static func fromList(_ pigeonVar_list: [Any?]) -> ImprovedNotificationContent? {
let title = pigeonVar_list[0] as! String
let body = pigeonVar_list[1] as! String
let subtitle = pigeonVar_list[1] as! String
let body = pigeonVar_list[2] as! String
let userInfo = pigeonVar_list[3] as! [String: Any?]

return ImprovedNotificationContent(
title: title,
body: body
subtitle: subtitle,
body: body,
userInfo: userInfo
)
}
func toList() -> [Any?] {
return [
title,
subtitle,
body,
userInfo,
]
}
static func == (lhs: ImprovedNotificationContent, rhs: ImprovedNotificationContent) -> Bool {
Expand Down
5 changes: 5 additions & 0 deletions ios/NotificationService/NotificationService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ class NotificationService: UNNotificationServiceExtension {
return
}

IosNativeHostApiSetup.setUp(
binaryMessenger: headlessEngine.binaryMessenger, api: IosNativeHostApiImpl())

// Register Flutter plugins with the headless engine.
GeneratedPluginRegistrant.register(with: headlessEngine)

Expand All @@ -60,7 +63,9 @@ class NotificationService: UNNotificationServiceExtension {
switch result {
case .success(let improvedNotificationContent):
bestAttemptContent.title = improvedNotificationContent.title
bestAttemptContent.subtitle = improvedNotificationContent.subtitle
bestAttemptContent.body = improvedNotificationContent.body
bestAttemptContent.userInfo = improvedNotificationContent.userInfo as [AnyHashable : Any]
contentHandler(bestAttemptContent)

case .failure(let error): // TODO(log)
Expand Down
8 changes: 8 additions & 0 deletions ios/Runner.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,13 @@
74858FAF1ED2DC5600515810 /* AppDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 74858FAE1ED2DC5600515810 /* AppDelegate.swift */; };
97C146FC1CF9000F007C117D /* Main.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 97C146FA1CF9000F007C117D /* Main.storyboard */; };
97C146FE1CF9000F007C117D /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 97C146FD1CF9000F007C117D /* Assets.xcassets */; };
B32717692F6C49E5007682B1 /* IosNativeHostApi.swift in Sources */ = {isa = PBXBuildFile; fileRef = B32717682F6C49E5007682B1 /* IosNativeHostApi.swift */; };
B340EB382F5B092B007AD309 /* IosNative.g.swift in Sources */ = {isa = PBXBuildFile; fileRef = B340EB372F5B092B007AD309 /* IosNative.g.swift */; };
B34E9F092D776BEB0009AED2 /* Notifications.g.swift in Sources */ = {isa = PBXBuildFile; fileRef = B34E9F082D776BEB0009AED2 /* Notifications.g.swift */; };
B35E11A62F484E6800DE4085 /* GeneratedPluginRegistrant.m in Sources */ = {isa = PBXBuildFile; fileRef = 1498D2331E8E89220040F4C2 /* GeneratedPluginRegistrant.m */; };
B378A5012F45B08F0031EFA1 /* NotificationService.appex in Embed Foundation Extensions */ = {isa = PBXBuildFile; fileRef = B378A4FA2F45B08F0031EFA1 /* NotificationService.appex */; settings = {ATTRIBUTES = (RemoveHeadersOnCopy, ); }; };
B3D425322F6D40C200F9AE69 /* IosNative.g.swift in Sources */ = {isa = PBXBuildFile; fileRef = B340EB372F5B092B007AD309 /* IosNative.g.swift */; };
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm, this line is added in this commit:
02704311e notif: Setup IosNativeHostApi Pigeon API for NotificationService headless Flutter engine

But that file isn't new (or even touched) in that commit; in fact it's already present in main, added originally in 1d03e57 (#2195).

What caused this line to appear in this particular commit?

Should this line have been present all along? What's been the effect of it not being present?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The changes in `project.pbxproj` are result of adding
`IosNative.g.swift` and `IosNativeHostApi.swift` to the compile
sources of the NotificationService target, by following the below
steps:
  - Open the project navigator (View > Navigators > Project).
  - In the main window under TARGETS, select NotificationService.
  - Open the Build Phases tab.
  - Expand Compile Sources.
  - Click +.
  - Select the desired file.
  - Click Add.

Great, thanks for adding this information.

FWIW, the list of steps has more detail than I think is strictly needed here. That's fine, though; some detail is helpful.

And the crucial part is the sentence up to the comma (plus "in Xcode", which the steps imply). That's critical particularly because it lets the reader infer an answer to these questions I asked above:

Should this line have been present all along? What's been the effect of it not being present?

(Namely: it meant those files weren't included in that service; and that was fine until we start using them there in this commit.)

B3D425332F6D40C200F9AE69 /* IosNativeHostApi.swift in Sources */ = {isa = PBXBuildFile; fileRef = B32717682F6C49E5007682B1 /* IosNativeHostApi.swift */; };
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This line is added in this commit (same as the comment above):
02704311e notif: Setup IosNativeHostApi Pigeon API for NotificationService headless Flutter engine

But that file isn't new in that commit, or touched there; it was introduced in the previous commit:
9882e18e2 ios [nfc]: Move IosNativeHostApiImpl to its own Swift file

Should this line have been added in that commit? What caused it to appear in this file?

It also looks quite a lot like another line a few lines above:

               B32717692F6C49E5007682B1 /* IosNativeHostApi.swift in Sources */ = {isa = PBXBuildFile; fileRef = B32717682F6C49E5007682B1 /* IosNativeHostApi.swift */; };

which does appear in that earlier commit. What's the effect of having both lines, instead of just the line that was added later? Should we have only one of them?

F311C174AF9C005CE4AADD72 /* Pods_Runner.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 3EAE3F3F518B95B7BFEB4FE7 /* Pods_Runner.framework */; };
/* End PBXBuildFile section */

Expand Down Expand Up @@ -84,6 +87,7 @@
97C146FB1CF9000F007C117D /* Base */ = {isa = PBXFileReference; lastKnownFileType = file.storyboard; name = Base; path = Base.lproj/Main.storyboard; sourceTree = "<group>"; };
97C146FD1CF9000F007C117D /* Assets.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = Assets.xcassets; sourceTree = "<group>"; };
97C147021CF9000F007C117D /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
B32717682F6C49E5007682B1 /* IosNativeHostApi.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IosNativeHostApi.swift; sourceTree = "<group>"; };
B340EB372F5B092B007AD309 /* IosNative.g.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IosNative.g.swift; sourceTree = "<group>"; };
B34E9F082D776BEB0009AED2 /* Notifications.g.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Notifications.g.swift; sourceTree = "<group>"; };
B378A4FA2F45B08F0031EFA1 /* NotificationService.appex */ = {isa = PBXFileReference; explicitFileType = "wrapper.app-extension"; includeInIndex = 0; path = NotificationService.appex; sourceTree = BUILT_PRODUCTS_DIR; };
Expand Down Expand Up @@ -212,6 +216,7 @@
1498D2321E8E86230040F4C2 /* GeneratedPluginRegistrant.h */,
1498D2331E8E89220040F4C2 /* GeneratedPluginRegistrant.m */,
74858FAE1ED2DC5600515810 /* AppDelegate.swift */,
B32717682F6C49E5007682B1 /* IosNativeHostApi.swift */,
B34E9F082D776BEB0009AED2 /* Notifications.g.swift */,
74858FAD1ED2DC5600515810 /* Runner-Bridging-Header.h */,
);
Expand Down Expand Up @@ -569,13 +574,16 @@
B34E9F092D776BEB0009AED2 /* Notifications.g.swift in Sources */,
1498D2341E8E89220040F4C2 /* GeneratedPluginRegistrant.m in Sources */,
B340EB382F5B092B007AD309 /* IosNative.g.swift in Sources */,
B32717692F6C49E5007682B1 /* IosNativeHostApi.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
B378A4F62F45B08F0031EFA1 /* Sources */ = {
isa = PBXSourcesBuildPhase;
buildActionMask = 2147483647;
files = (
B3D425322F6D40C200F9AE69 /* IosNative.g.swift in Sources */,
B3D425332F6D40C200F9AE69 /* IosNativeHostApi.swift in Sources */,
B35E11A62F484E6800DE4085 /* GeneratedPluginRegistrant.m in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
Expand Down
10 changes: 0 additions & 10 deletions ios/Runner/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,3 @@ class NotificationTapEventListener: NotificationTapEventsStreamHandler {
eventSink?.success(IosNotificationTapEvent(payload: payload))
}
}

private class IosNativeHostApiImpl: IosNativeHostApi {
func setExcludedFromBackup(filePath: String) throws {
var resourceValues = URLResourceValues()
resourceValues.isExcludedFromBackup = true

var url = URL(fileURLWithPath: filePath, isDirectory: false)
try url.setResourceValues(resourceValues)
}
}
12 changes: 12 additions & 0 deletions ios/Runner/IosNativeHostApi.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import Flutter
import UIKit

public class IosNativeHostApiImpl: IosNativeHostApi {
func setExcludedFromBackup(filePath: String) throws {
var resourceValues = URLResourceValues()
resourceValues.isExcludedFromBackup = true

var url = URL(fileURLWithPath: filePath, isDirectory: false)
try url.setResourceValues(resourceValues)
}
}
23 changes: 23 additions & 0 deletions lib/api/notifications.dart
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,29 @@ class EncryptedFcmMessage {
Map<String, dynamic> toJson() => _$EncryptedFcmMessageToJson(this);
}

/// An APNs payload whose contents are encrypted end-to-end from the Zulip server.
///
/// Once decrypted, the contents will become a [NotifPayload].
///
/// API docs:
/// https://zulip.com/api/mobile-notifications#data-sent-to-apns
@JsonSerializable(fieldRename: FieldRename.snake)
class EncryptedApnsPayload {
final int pushKeyId;

@JsonKey(fromJson: base64Decode, toJson: base64Encode)
final Uint8List encryptedData;

// final Map<String, dynamic> aps; // ignore; never used

EncryptedApnsPayload({required this.pushKeyId, required this.encryptedData});

factory EncryptedApnsPayload.fromJson(Map<String, dynamic> json) =>
_$EncryptedApnsPayloadFromJson(json);

Map<String, dynamic> toJson() => _$EncryptedApnsPayloadToJson(this);
}

//|//////////////////////////////////////////////////////////////
// Types for parsing E2EE notification payloads.
//
Expand Down
14 changes: 14 additions & 0 deletions lib/api/notifications.g.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion lib/api/route/notifications.dart
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,15 @@ class PushRegistration {
final PushTokenKind tokenKind;
final String token;
final int timestamp;
// final String? iosAppId; // TODO(#1764)

@JsonKey(includeIfNull: false)
final String? iosAppId;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This can be easily split out into a prep commit.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Almost NFC, because E2EE notifications are only enabled on Android.
But the PushRegistration request will now have `ios_app_id: null`
pair in the request.

Hmm, good to flag this. (Part of the benefit of a separate commit! There's now a separate commit message, making a natural space for this sort of note.)

I think the ideal implementation here would avoid sending that. Not important, though, as long as the server currently does accept this.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed this by adding a @JsonKey(includeIfNull: false) over iosAppId.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great, glad that was easy to fix.


PushRegistration({
required this.tokenKind,
required this.token,
required this.timestamp,
required this.iosAppId,
});

Map<String, dynamic> toJson() => _$PushRegistrationToJson(this);
Expand Down
1 change: 1 addition & 0 deletions lib/api/route/notifications.g.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 16 additions & 1 deletion lib/host/ios_notifications.g.dart
Original file line number Diff line number Diff line change
Expand Up @@ -79,19 +79,32 @@ class NotificationContent {
class ImprovedNotificationContent {
ImprovedNotificationContent({
required this.title,
required this.subtitle,
required this.body,
required this.userInfo,
});

/// The new title to use for the notification.
String title;

/// The new subtitle to use for the notification.
String subtitle;

/// The new body to use for the notification.
String body;

/// The internal data to attach with the new notification.
///
/// This replaces the raw APNs payload that was initially set from
/// the remote push notification.
Map<String, Object?> userInfo;

List<Object?> _toList() {
return <Object?>[
title,
subtitle,
body,
userInfo,
];
}

Expand All @@ -102,7 +115,9 @@ class ImprovedNotificationContent {
result as List<Object?>;
return ImprovedNotificationContent(
title: result[0]! as String,
body: result[1]! as String,
subtitle: result[1]! as String,
body: result[2]! as String,
userInfo: (result[3] as Map<Object?, Object?>?)!.cast<String, Object?>(),
);
}

Expand Down
14 changes: 10 additions & 4 deletions lib/model/push_device.dart
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ class PushDeviceManager extends PerAccountStoreBase {
/// This is an entry in [devices].
ClientDevice? get thisDevice => _devices[account.deviceId];

bool get _e2eeAvailable => zulipFeatureLevel >= 468 // TODO(server-12)
&& defaultTargetPlatform == TargetPlatform.android; // TODO(#1764)
bool get _e2eeAvailable => zulipFeatureLevel >= 468; // TODO(server-12)

void handleDeviceEvent(DeviceEvent event) {
switch (event) {
Expand Down Expand Up @@ -237,8 +236,15 @@ class PushDeviceManager extends PerAccountStoreBase {
_ => throw StateError('unexpected platform: $defaultTargetPlatform'),
};

final pushRegistration = PushRegistration( // TODO(#1764) also iosAppId
tokenKind: tokenKind, token: token,
String? iosAppId;
if (defaultTargetPlatform == TargetPlatform.iOS) {
final packageInfo = await ZulipBinding.instance.packageInfo;
iosAppId = packageInfo!.packageName;
}
final pushRegistration = PushRegistration(
iosAppId: iosAppId,
tokenKind: tokenKind,
token: token,
timestamp: timestamp);
final encryptedPushRegistration = await _encryptToBouncer(
bouncerPublicKey, jsonEncode(pushRegistration));
Expand Down
63 changes: 42 additions & 21 deletions lib/notifications/display.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'package:http/http.dart' as http;

import '../api/model/model.dart';
import '../api/notifications.dart';
import '../generated/l10n/zulip_localizations.dart';
import '../host/android_notifications.dart';
import '../log.dart';
import '../model/binding.dart';
Expand Down Expand Up @@ -259,17 +260,7 @@ class NotificationDisplayManager {
// changed, which is a rare edge case but probably good. The main effect is that
// group-DM threads (pending #794) get titled with the latest sender, rather than
// the first.
messagingStyle.conversationTitle = switch (data.recipient) {
NotifPayloadChannelRecipient(:var channelName?, :var topic) =>
'#$channelName > ${topic.displayName}',
NotifPayloadChannelRecipient(:var topic) =>
'#${zulipLocalizations.unknownChannelName} > ${topic.displayName}', // TODO get stream name from data
NotifPayloadDmRecipient(:var allRecipientIds) when allRecipientIds.length > 2 =>
zulipLocalizations.notifGroupDmConversationLabel(
data.senderFullName, allRecipientIds.length - 2), // TODO use others' names, from data
NotifPayloadDmRecipient() =>
data.senderFullName,
};
messagingStyle.conversationTitle = titleForNotifPayload(data, zulipLocalizations);

messagingStyle.messages.add(MessagingStyleMessage(
text: data.content,
Expand All @@ -279,15 +270,7 @@ class NotificationDisplayManager {
name: data.senderFullName,
iconBitmap: await _fetchBitmap(data.senderAvatarUrl))));

final intentDataUrl = NotificationOpenPayload(
realmUrl: data.realmUrl,
userId: data.userId,
narrow: switch (data.recipient) {
NotifPayloadChannelRecipient(:var channelId, :var topic) =>
TopicNarrow(channelId, topic),
NotifPayloadDmRecipient(:var allRecipientIds) =>
DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId),
}).buildAndroidNotificationUrl();
final intentDataUrl = notificationUrlForNotifPayload(data);

await _androidHost.notify(
id: kNotificationId,
Expand Down Expand Up @@ -416,11 +399,49 @@ class NotificationDisplayManager {
//
// Even though we enable the `autoCancel` flag for summary notification
// during creation, the summary notification doesn't get auto canceled if
// child notifications are canceled programatically as done above.
// child notifications are canceled programmatically as done above.
await _androidHost.cancel(tag: groupKey, id: kNotificationId);
}
}

static String titleForNotifPayload(NotifPayloadNewMessage data, ZulipLocalizations zulipLocalizations) {
return switch (data.recipient) {
NotifPayloadChannelRecipient(:var channelName?, :var topic) =>
'#$channelName > ${topic.displayName}',
NotifPayloadChannelRecipient(:var topic) =>
'#${zulipLocalizations.unknownChannelName} > ${topic.displayName}', // TODO get stream name from data
NotifPayloadDmRecipient(:var allRecipientIds) when allRecipientIds.length > 2 =>
zulipLocalizations.notifGroupDmConversationLabel(
data.senderFullName, allRecipientIds.length - 2), // TODO use others' names, from data
NotifPayloadDmRecipient() =>
data.senderFullName,
};
}

static String subtitleForNotifPayloadOnIos(NotifPayloadNewMessage data) {
// Adapted from server implementation:
// https://github.com/zulip/zulip/blob/11bf985d1/zerver/lib/push_notifications.py#L1087-L1124
// TODO handle subtitle for user/group/wildcard mentions
return switch (data.recipient) {
NotifPayloadChannelRecipient() => '${data.senderFullName}:',

// The title indicates the sender's name in both 1-1 and group DMs.
NotifPayloadDmRecipient() => '',
};
}

static Uri notificationUrlForNotifPayload(NotifPayloadNewMessage data) {
return NotificationOpenPayload(
realmUrl: data.realmUrl,
userId: data.userId,
narrow: switch (data.recipient) {
NotifPayloadChannelRecipient(:var channelId, :var topic) =>
TopicNarrow(channelId, topic),
NotifPayloadDmRecipient(:var allRecipientIds) =>
DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId),
}).buildNotificationUrl();
}

static Future<void> removeNotificationsForAccount(Uri realmUrl, int userId) async {
assert(defaultTargetPlatform == TargetPlatform.android);

Expand Down
Loading