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

Communication: Improve push notifications for the iOS mobile app #9787

Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -242,16 +242,18 @@ public static GroupNotification createAnnouncementNotification(Post post, User a
GroupNotification notification;
title = NotificationConstants.NEW_ANNOUNCEMENT_POST_TITLE;
text = NotificationConstants.NEW_ANNOUNCEMENT_POST_TEXT;
var imageUrl = post.getAuthor().getImageUrl() == null ? "" : post.getAuthor().getImageUrl();
placeholderValues = createPlaceholdersNewAnnouncementPost(course.getTitle(), post.getTitle(), Jsoup.parse(post.getContent()).text(), post.getCreationDate().toString(),
post.getAuthor().getName());
post.getAuthor().getName(), imageUrl, post.getAuthor().getId().toString(), post.getId().toString());
notification = new GroupNotification(course, title, text, true, placeholderValues, author, groupNotificationType);
notification.setTransientAndStringTarget(createCoursePostTarget(post, course));
return notification;
}

@NotificationPlaceholderCreator(values = { NEW_ANNOUNCEMENT_POST })
public static String[] createPlaceholdersNewAnnouncementPost(String courseTitle, String postTitle, String postContent, String postCreationDate, String postAuthorName) {
return new String[] { courseTitle, postTitle, postContent, postCreationDate, postAuthorName };
public static String[] createPlaceholdersNewAnnouncementPost(String courseTitle, String postTitle, String postContent, String postCreationDate, String postAuthorName,
String imageUrl, String authorId, String postId) {
return new String[] { courseTitle, postTitle, postContent, postCreationDate, postAuthorName, imageUrl, authorId, postId };
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -321,9 +321,11 @@ public static SingleUserNotification createNotification(AnswerPost answerPost, N
}

Conversation conversation = answerPost.getPost().getConversation();
var imageUrl = answerPost.getAuthor().getImageUrl() != null ? answerPost.getAuthor().getImageUrl() : "";
var placeholders = createPlaceholdersNewReply(conversation.getCourse().getTitle(), answerPost.getPost().getContent(), answerPost.getPost().getCreationDate().toString(),
answerPost.getPost().getAuthor().getName(), answerPost.getContent(), answerPost.getCreationDate().toString(), answerPost.getAuthor().getName(),
conversation.getHumanReadableNameForReceiver(answerPost.getAuthor()));
conversation.getHumanReadableNameForReceiver(answerPost.getAuthor()), imageUrl, answerPost.getAuthor().getId().toString(), answerPost.getId().toString(),
answerPost.getPost().getId().toString());
PaRangger marked this conversation as resolved.
Show resolved Hide resolved
PaRangger marked this conversation as resolved.
Show resolved Hide resolved

String messageReplyTextType = MESSAGE_REPLY_IN_CONVERSATION_TEXT;

Expand All @@ -340,8 +342,9 @@ public static SingleUserNotification createNotification(AnswerPost answerPost, N
@NotificationPlaceholderCreator(values = { NEW_REPLY_FOR_EXERCISE_POST, NEW_REPLY_FOR_LECTURE_POST, NEW_REPLY_FOR_COURSE_POST, NEW_REPLY_FOR_EXAM_POST,
CONVERSATION_NEW_REPLY_MESSAGE, CONVERSATION_USER_MENTIONED })
public static String[] createPlaceholdersNewReply(String courseTitle, String postContent, String postCreationData, String postAuthorName, String answerPostContent,
String answerPostCreationDate, String authorName, String conversationName) {
return new String[] { courseTitle, postContent, postCreationData, postAuthorName, answerPostContent, answerPostCreationDate, authorName, conversationName };
String answerPostCreationDate, String authorName, String conversationName, String imageUrl, String userId, String postingId, String parentPostId) {
return new String[] { courseTitle, postContent, postCreationData, postAuthorName, answerPostContent, answerPostCreationDate, authorName, conversationName, imageUrl, userId,
postingId, parentPostId };
PaRangger marked this conversation as resolved.
Show resolved Hide resolved
PaRangger marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package de.tum.cit.aet.artemis.communication.domain.push_notification;

import java.util.Arrays;

public enum PushNotificationApiType {

DEFAULT((short) 0), IOS_V2((short) 1);

private final short databaseKey;

PushNotificationApiType(short databaseKey) {
this.databaseKey = databaseKey;
}

public short getDatabaseKey() {
return databaseKey;
}

public static PushNotificationApiType fromDatabaseKey(short databaseKey) {
return Arrays.stream(PushNotificationApiType.values()).filter(type -> type.getDatabaseKey() == databaseKey).findFirst()
.orElseThrow(() -> new IllegalArgumentException("Unknown database key: " + databaseKey));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.Enumerated;
import jakarta.persistence.Id;
import jakarta.persistence.IdClass;
import jakarta.persistence.JoinColumn;
Expand Down Expand Up @@ -34,6 +35,10 @@ public class PushNotificationDeviceConfiguration {
@Column(name = "device_type")
private PushNotificationDeviceType deviceType;

@Enumerated
@Column(name = "api_type")
private PushNotificationApiType apiType;

@Column(name = "expiration_date")
private Date expirationDate;

Expand All @@ -53,6 +58,16 @@ public PushNotificationDeviceConfiguration(String token, PushNotificationDeviceT
this.owner = owner;
}

public PushNotificationDeviceConfiguration(String token, PushNotificationDeviceType deviceType, Date expirationDate, byte[] secretKey, User owner,
PushNotificationApiType apiType) {
this.token = token;
this.deviceType = deviceType;
this.expirationDate = expirationDate;
this.secretKey = secretKey;
this.owner = owner;
this.apiType = apiType;
}

public PushNotificationDeviceConfiguration() {
// needed for JPA
}
Expand Down Expand Up @@ -97,6 +112,10 @@ public void setOwner(User owner) {
this.owner = owner;
}

public PushNotificationApiType getApiType() {
return apiType;
}
PaRangger marked this conversation as resolved.
Show resolved Hide resolved

@Override
public boolean equals(Object object) {
if (this == object) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
package de.tum.cit.aet.artemis.communication.dto;

import de.tum.cit.aet.artemis.communication.domain.push_notification.PushNotificationApiType;
import de.tum.cit.aet.artemis.communication.domain.push_notification.PushNotificationDeviceType;

public record PushNotificationRegisterBody(String token, PushNotificationDeviceType deviceType) {
public record PushNotificationRegisterBody(String token, PushNotificationDeviceType deviceType, PushNotificationApiType apiType) {

public PushNotificationRegisterBody(String token, PushNotificationDeviceType deviceType) {
this(token, deviceType, PushNotificationApiType.DEFAULT);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,17 +84,19 @@ public ConversationNotification createNotification(Post createdMessage, Conversa
}
default -> throw new IllegalStateException("Unexpected value: " + conversation);
}
var imageUrl = createdMessage.getAuthor().getImageUrl() == null ? "" : createdMessage.getAuthor().getImageUrl();
String[] placeholders = createPlaceholdersNewMessageChannelText(course.getTitle(), createdMessage.getContent(), createdMessage.getCreationDate().toString(),
conversationName, createdMessage.getAuthor().getName(), conversationType);
conversationName, createdMessage.getAuthor().getName(), conversationType, imageUrl, createdMessage.getAuthor().getId().toString(),
createdMessage.getId().toString());
PaRangger marked this conversation as resolved.
Show resolved Hide resolved
ConversationNotification notification = createConversationMessageNotification(course.getId(), createdMessage, notificationType, notificationText, true, placeholders);
save(notification, mentionedUsers, placeholders, createdMessage);
return notification;
}

@NotificationPlaceholderCreator(values = { CONVERSATION_NEW_MESSAGE })
public static String[] createPlaceholdersNewMessageChannelText(String courseTitle, String messageContent, String messageCreationDate, String conversationName,
String authorName, String conversationType) {
return new String[] { courseTitle, messageContent, messageCreationDate, conversationName, authorName, conversationType };
String authorName, String conversationType, String imageUrl, String userId, String postId) {
return new String[] { courseTitle, messageContent, messageCreationDate, conversationName, authorName, conversationType, imageUrl, userId, postId };
}

private void save(ConversationNotification notification, Set<User> mentionedUsers, String[] placeHolders, Post createdMessage) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ private void notifyGroupsWithNotificationType(GroupNotificationType[] groups, No
* @param author is the user who initiated the process of the notifications. Can be null if not specified
* @param onlySave whether the notification should only be saved and not sent to users
*/
@SuppressWarnings("unchecked")
private void notifyGroupsWithNotificationType(GroupNotificationType[] groups, NotificationType notificationType, Object notificationSubject, Object typeSpecificInformation,
User author, boolean onlySave) {
for (GroupNotificationType group : groups) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,11 +155,11 @@ public void sendNotification(Notification notification, Set<User> users, Object
}

final String date = Instant.now().toString();
var notificationData = new PushNotificationData(notification.getTransientPlaceholderValuesAsArray(), notification.getTarget(), type.name(), date,
Constants.PUSH_NOTIFICATION_VERSION);

try {
final String payload = mapper.writeValueAsString(notificationData);
var notificationData = new PushNotificationData(notification.getTransientPlaceholderValuesAsArray(), notification.getTarget(), type.name(), date,
Constants.PUSH_NOTIFICATION_VERSION);
var payload = mapper.writeValueAsString(notificationData);
final byte[] initializationVector = new byte[16];

List<RelayNotificationRequest> notificationRequests = userDeviceConfigurations.stream().flatMap(deviceConfiguration -> {
Expand All @@ -170,7 +170,8 @@ public void sendNotification(Notification notification, Set<User> users, Object
String ivAsString = Base64.getEncoder().encodeToString(initializationVector);
Optional<String> payloadCiphertext = encrypt(payload, key, initializationVector);

return payloadCiphertext.stream().map(s -> new RelayNotificationRequest(ivAsString, s, deviceConfiguration.getToken()));
return payloadCiphertext.stream()
.map(s -> new RelayNotificationRequest(ivAsString, s, deviceConfiguration.getToken(), deviceConfiguration.getApiType().getDatabaseKey()));
PaRangger marked this conversation as resolved.
Show resolved Hide resolved
}).toList();

sendNotificationRequestsToEndpoint(notificationRequests, relayServerBaseUrl.get());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package de.tum.cit.aet.artemis.communication.service.notifications.push_notifications;

public record RelayNotificationRequest(String initializationVector, String payloadCipherText, String token) {
public record RelayNotificationRequest(String initializationVector, String payloadCipherText, String token, short apiType) {
PaRangger marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;

import de.tum.cit.aet.artemis.communication.domain.push_notification.PushNotificationApiType;
import de.tum.cit.aet.artemis.communication.domain.push_notification.PushNotificationDeviceConfiguration;
import de.tum.cit.aet.artemis.communication.domain.push_notification.PushNotificationDeviceConfigurationId;
import de.tum.cit.aet.artemis.communication.dto.PushNotificationRegisterBody;
Expand Down Expand Up @@ -100,10 +101,12 @@ public ResponseEntity<PushNotificationRegisterDTO> register(@Valid @RequestBody
return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
}

PushNotificationApiType apiType = pushNotificationRegisterBody.apiType() != null ? pushNotificationRegisterBody.apiType() : PushNotificationApiType.DEFAULT;

User user = userRepository.getUser();

PushNotificationDeviceConfiguration deviceConfiguration = new PushNotificationDeviceConfiguration(pushNotificationRegisterBody.token(),
pushNotificationRegisterBody.deviceType(), expirationDate, newKey.getEncoded(), user);
pushNotificationRegisterBody.deviceType(), expirationDate, newKey.getEncoded(), user, apiType);
pushNotificationDeviceConfigurationRepository.save(deviceConfiguration);

var encodedKey = Base64.getEncoder().encodeToString(newKey.getEncoded());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,11 @@ public final class Constants {
*/
public static final int PUSH_NOTIFICATION_VERSION = 1;

/**
* The value of the version field we send with each push notification to the native clients (Android & iOS).
*/
public static final int PUSH_NOTIFICATION_MINOR_VERSION = 2;

/**
* The directory in the docker container in which the build script is executed
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?xml version="1.0" encoding="UTF-8"?>
<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog https://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-latest.xsd">
<changeSet id="20241114122713" author="rangger">
<preConditions onFail="MARK_RAN">
<not>
<columnExists tableName="push_notification_device_configuration" columnName="api_type"/>
</not>
</preConditions>

<addColumn tableName="push_notification_device_configuration">
<column name="api_type" type="tinyint" defaultValue="0">
<constraints nullable="false"/>
</column>
</addColumn>
</changeSet>
</databaseChangeLog>
1 change: 1 addition & 0 deletions src/main/resources/config/liquibase/master.xml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
<include file="classpath:config/liquibase/changelog/20241101121000_changelog.xml" relativeToChangelogFile="false"/>
<include file="classpath:config/liquibase/changelog/20241105150000_changelog.xml" relativeToChangelogFile="false"/>
<include file="classpath:config/liquibase/changelog/20241112123600_changelog.xml" relativeToChangelogFile="false"/>
<include file="classpath:config/liquibase/changelog/20241114122713_changelog.xml" relativeToChangelogFile="false"/>
<include file="classpath:config/liquibase/changelog/20241125000900_changelog.xml" relativeToChangelogFile="false"/>

<!-- NOTE: please use the format "YYYYMMDDhhmmss_changelog.xml", i.e. year month day hour minutes seconds and not something else! -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,16 @@ class GroupNotificationFactoryTest {

private static final String POST_TITLE = "post title";

private static final Long POST_ID = 1L;

private static final String POST_CONTENT = "post content";

private static final String ANSWER_POST_CONTENT = "answer post content";

private static User user = new User();

private static final Long USER_ID = 1L;

private String expectedTitle;

private String expectedText;
Expand Down Expand Up @@ -196,8 +200,10 @@ static void setUp() {
user = new User();
user.setFirstName("John");
user.setLastName("Smith");
user.setId(USER_ID);

post = new Post();
post.setId(POST_ID);
post.setConversation(new Channel());
post.setAuthor(user);
post.setTitle(POST_TITLE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ class NotificationPlaceholderSignatureTest extends AbstractSpringIntegrationInde
* 2. The database becomes inconsistent. The placeholders are stored as JSON in the database.
* You must now do the following:
* 1. Check if you really need to change these placeholders. If not, revert your changes.
* 2. Write a database migration for the old placeholder JSON strings, such that they match your new signature.
* 3. Increment the {{@link Constants#PUSH_NOTIFICATION_VERSION}}. This ensures that old versions of the native apps discard your new
* 2a. In case of breaking changes, increment the {{@link Constants#PUSH_NOTIFICATION_VERSION}}. This ensures that old versions of the native apps discard your new
* notifications.
* 4. Update both the Android and iOS app. Only merge this server PR after they have been updated and released to the stores. Otherwise, notifications no longer work for
* 2b. In case of non-breaking changes, increment the {{@link Constants#PUSH_NOTIFICATION_MINOR_VERSION}} for better traceability
* 3. Update both the Android and iOS app. Only merge this server PR after they have been updated and released to the stores. Otherwise, notifications no longer work for
* end users.
* 5. Execute the Gradle task generatePlaceholderRevisionSignatures to make this test pass again.
* 4. Execute the Gradle task generatePlaceholderRevisionSignatures to make this test pass again.
PaRangger marked this conversation as resolved.
Show resolved Hide resolved
*/
@Test
void testSignatureHasNotChanged() throws URISyntaxException, IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ void setUp() {
channel.setCreationDate(ZonedDateTime.now());

post = new Post();
post.setId(1L);
post.setAuthor(userTwo);
post.setConversation(channel);
post.setTitle(POST_TITLE);
Expand All @@ -223,7 +224,9 @@ void setUp() {
Post answerPostPost = new Post();
answerPostPost.setConversation(channel);
answerPostPost.setAuthor(userTwo);
answerPostPost.setId(2L);
answerPost = new AnswerPost();
answerPost.setId(1L);
answerPost.setPost(answerPostPost);
answerPost.setAuthor(userThree);
answerPost.setContent(ANSWER_POST_CONTENT);
Expand Down Expand Up @@ -464,11 +467,13 @@ void testConversationNotificationsChannel(NotificationType notificationType, Str
@Test
void testConversationNotificationsNewMessageReply() {
Post post = new Post();
post.setId(1L);
PaRangger marked this conversation as resolved.
Show resolved Hide resolved
post.setAuthor(user);
post.setCreationDate(ZonedDateTime.now());
post.setConversation(groupChat);

AnswerPost answerPost = new AnswerPost();
answerPost.setId(1L);
answerPost.setAuthor(userTwo);
answerPost.setCreationDate(ZonedDateTime.now().plusSeconds(5));
answerPost.setPost(post);
Expand Down
Loading
Loading