Skip to content

Commit

Permalink
fix: rename email subscription user IDs to lowercase
Browse files Browse the repository at this point in the history
We had a customer report where they complained that they were still receiving
Errata notifications even after unsubscribing from them.

After investigating, we found out that we had multiple and even overlapping
email subscriptions for almost the same username. By "almost", I mean that the
username was essentially the same, but with different case. For example: UserA
and usera.

Notifications takes all the email subscriptions' usernames and makes them
lowercase to make a case insensitive comparison. Also, when user preferences
are saved, the email subscriptions that are generated are generated with
lowercase user IDs.

When we migrated the Errata email subscriptions, some of them contained
mixed case user IDs, and we did not realize about that.

What ended up happening was:

- A customer was initially subscribed to errata Notifications, and therefore
we migrated their email subscription from Errata with a mixed case.
- The customer unsubscribed, which generated a lowercase email unsubscription.
- The recipients resolver took both subscriptions and since it still found the
mixed case subscription, it was sending the email.

RHCLOUD-37310
  • Loading branch information
MikelAlejoBR committed Jan 14, 2025
1 parent 0a6d105 commit b583052
Show file tree
Hide file tree
Showing 6 changed files with 453 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,14 @@
import jakarta.inject.Inject;
import jakarta.persistence.EntityManager;
import jakarta.transaction.Transactional;
import org.hibernate.StatelessSession;
import org.hibernate.Transaction;
import org.hibernate.exception.ConstraintViolationException;
import org.postgresql.util.PSQLState;

import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.UUID;

import static com.redhat.cloud.notifications.models.SubscriptionType.DAILY;
Expand All @@ -26,6 +32,9 @@ public class SubscriptionRepository {
@Inject
BackendConfig backendConfig;

@Inject
StatelessSession statelessSession;

@Inject
TemplateRepository templateRepository;

Expand Down Expand Up @@ -127,4 +136,93 @@ private List<SubscriptionType> getAvailableTypes() {
return List.of(INSTANT, DAILY);
}
}


/**
* Fetches user IDs from email subscriptions whose case is mixed.
* @return a set of email subscription user IDs in mixed case.
*/
public Set<String> findMixedCaseUserIds() {
final String fetchDistinctMixedCaseUserIds =
"SELECT " +
"DISTINCT(user_id) " +
"FROM " +
"email_subscriptions " +
"WHERE " +
"LOWER(user_id) <> user_id ";

return new HashSet<>(
this.statelessSession
.createNativeQuery(fetchDistinctMixedCaseUserIds, String.class)
.getResultList()
);
}

/**
* Fetches the email subscriptions of the given user.
* @param userId the user ID to fetch the subscriptions for.
* @return the email subscriptions related to the given user ID.
*/
public List<EventTypeEmailSubscription> findEmailSubscriptionsByUserId(final String userId) {
final String fetchSql =
"FROM " +
"EventTypeEmailSubscription " +
"WHERE " +
"id.userId = :userId";

return this.statelessSession
.createQuery(fetchSql, EventTypeEmailSubscription.class)
.setParameter("userId", userId)
.getResultList();
}

/**
* Sets the email subscription's user ID in lowercase. In the case that
* there is a "primary key constraint violation", which signals that the
* email subscription already exists, the given email subscription is
* deleted instead.
* @param eventTypeEmailSubscription the email subscription to which the
* user ID must be set in lowercase.
*/
public void setEmailSubscriptionUserIdLowercase(final EventTypeEmailSubscription eventTypeEmailSubscription) {
final String updateSql =
"UPDATE " +
"email_subscriptions " +
"SET " +
"user_id = LOWER(user_id) " +
"WHERE " +
"user_id = :userId AND " +
"org_id = :orgId AND " +
"event_type_id = :eventTypeId AND " +
"subscription_type = :subscriptionType AND " +
"subscribed = :isSubscribed";

final Transaction updateTransaction = this.statelessSession.beginTransaction();
try {
this.statelessSession
.createNativeQuery(updateSql)
.setParameter("userId", eventTypeEmailSubscription.getUserId())
.setParameter("orgId", eventTypeEmailSubscription.getOrgId())
.setParameter("eventTypeId", eventTypeEmailSubscription.getEventType().getId())
.setParameter("subscriptionType", eventTypeEmailSubscription.getSubscriptionType().name())
.setParameter("isSubscribed", eventTypeEmailSubscription.isSubscribed())
.executeUpdate();

updateTransaction.commit();
Log.debugf("[user_id: %s][org_id: %s][event_type_id: %s] Email subscription's user ID set to lowercase", eventTypeEmailSubscription.getUserId(), eventTypeEmailSubscription.getOrgId(), eventTypeEmailSubscription.getEventType().getId());
} catch (final ConstraintViolationException e) {
updateTransaction.rollback();

if (PSQLState.UNIQUE_VIOLATION.getState().equals(e.getSQLState()) && "pk_email_subscriptions".equals(e.getConstraintName())) {
final Transaction deleteTransaction = this.statelessSession.beginTransaction();

this.statelessSession.delete(eventTypeEmailSubscription);

deleteTransaction.commit();
Log.debugf("[user_id: %s][org_id: %s][event_type_id: %s] Email subscription deleted", eventTypeEmailSubscription.getUserId(), eventTypeEmailSubscription.getOrgId(), eventTypeEmailSubscription.getEventType().getId());
} else {
throw e;
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package com.redhat.cloud.notifications.routers.internal.errata;

import com.redhat.cloud.notifications.Constants;
import com.redhat.cloud.notifications.db.repositories.SubscriptionRepository;
import com.redhat.cloud.notifications.models.EventTypeEmailSubscription;
import io.quarkus.logging.Log;
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.inject.Inject;
import jakarta.ws.rs.POST;
import jakarta.ws.rs.Path;

import java.util.List;
import java.util.Set;

@ApplicationScoped
@Path(Constants.API_INTERNAL + "/team-nado")
public class ErrataEmailPreferencesUserIdFixService {
@Inject
SubscriptionRepository subscriptionRepository;

@Path("/migrate/rename-lowercase")
@POST
public void migrateMixedCaseEmailSubscriptions() {
final Set<String> mixedCaseUserIds = this.subscriptionRepository.findMixedCaseUserIds();

Log.debugf("Fetched the following email subscription user IDs: %s", mixedCaseUserIds);

for (final String mixedCaseUserId : mixedCaseUserIds) {
Log.debugf("[user_id: %s] Processing mixed case user id", mixedCaseUserId);

final List<EventTypeEmailSubscription> mixedCaseEmailSubs = this.subscriptionRepository.findEmailSubscriptionsByUserId(mixedCaseUserId);

Log.debugf("[user_id: %s] Fetched the following email subscriptions for user: %s", mixedCaseUserId, mixedCaseEmailSubs);

for (final EventTypeEmailSubscription subscription : mixedCaseEmailSubs) {
this.subscriptionRepository.setEmailSubscriptionUserIdLowercase(subscription);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,205 @@
package com.redhat.cloud.notifications.db.repositories;

import com.redhat.cloud.notifications.config.BackendConfig;
import com.redhat.cloud.notifications.db.DbIsolatedTest;
import com.redhat.cloud.notifications.db.ResourceHelpers;
import com.redhat.cloud.notifications.models.Application;
import com.redhat.cloud.notifications.models.Bundle;
import com.redhat.cloud.notifications.models.EventType;
import com.redhat.cloud.notifications.models.EventTypeEmailSubscription;
import com.redhat.cloud.notifications.models.SubscriptionType;
import io.quarkus.test.InjectMock;
import io.quarkus.test.junit.QuarkusTest;
import jakarta.inject.Inject;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

import java.util.HashSet;
import java.util.List;
import java.util.Random;
import java.util.Set;

import static com.redhat.cloud.notifications.TestConstants.DEFAULT_ORG_ID;

@QuarkusTest
public class SubscriptionRepositoryTest extends DbIsolatedTest {
@InjectMock
BackendConfig backendConfig;

@Inject
ResourceHelpers resourceHelpers;

@Inject
SubscriptionRepository subscriptionRepository;

/**
* Tests that the function under test only finds mixed cased user IDs from
* email subscriptions.
*/
@Test
void testFindMixedCaseUserIds() {
// Simulate that the default template is enabled so that we can create
// instant subscriptions without facing any "missing template" errors.
Mockito.when(this.backendConfig.isDefaultTemplateEnabled()).thenReturn(true);

// Create a single event type we want to create the subscriptions for.
final Bundle randomBundle = this.resourceHelpers.createBundle("random-bundle");
final Application randomApplication = this.resourceHelpers.createApplication(randomBundle.getId(), "random-application");
final EventType randomEventType = this.resourceHelpers.createEventType(randomApplication.getId(), "random-event-type");

// Create five subscriptions with mixed case user IDs.
final Random random = new Random();
final Set<String> expectedUsernames = new HashSet<>(5);
for (int i = 0; i < 5; i++) {
final String userId;
if (random.nextBoolean()) {
userId = String.format("MixedCaseUSERid%d", i);
} else {
userId = String.format("UPPERCASEUSERID%d", i);
}
final String orgId = String.format("%d", i);

// Store the created user ID to verify it afterward.
expectedUsernames.add(userId);

this.subscriptionRepository.updateSubscription(orgId, userId, randomEventType.getId(), SubscriptionType.INSTANT, random.nextBoolean());
}

// Create five subscriptions with lowercase usernames.
for (int i = 0; i < 5; i++) {
final String userId = String.format("lowercaseusername%d", i);
final String orgId = String.format("%d", i);

this.subscriptionRepository.updateSubscription(orgId, userId, randomEventType.getId(), SubscriptionType.INSTANT, random.nextBoolean());
}

// Call the function under test.
final Set<String> userIds = this.subscriptionRepository.findMixedCaseUserIds();

// Assert that the fetched user IDs are just the ones with mixed case.
for (final String userId : userIds) {
Assertions.assertTrue(
expectedUsernames.contains(userId),
String.format("a non-mixed-case user ID \"%s\" was fetched from the database", userId)
);
}
}

/**
* Tests that the function under test only finds the email subscriptions
* related to the given user ID.
*/
@Test
void testFindEmailSubscriptionsByUserId() {
// Simulate that the default template is enabled so that we can create
// instant subscriptions without facing any "missing template" errors.
Mockito.when(this.backendConfig.isDefaultTemplateEnabled()).thenReturn(true);

// Create a single event type we want to create the subscriptions for.
final Bundle randomBundle = this.resourceHelpers.createBundle("random-bundle");
final Application randomApplication = this.resourceHelpers.createApplication(randomBundle.getId(), "random-application");
final EventType randomEventType = this.resourceHelpers.createEventType(randomApplication.getId(), "random-event-type");
final EventType randomEventTypeTwo = this.resourceHelpers.createEventType(randomApplication.getId(), "random-event-type-two");
final EventType randomEventTypeThree = this.resourceHelpers.createEventType(randomApplication.getId(), "random-event-type-three");

// Create some subscriptions for two different users.
final String userIdOne = "userIdOne";
final String userIdTwo = "userIdTwo";

final Random random = new Random();
this.subscriptionRepository.updateSubscription(DEFAULT_ORG_ID, userIdOne, randomEventType.getId(), SubscriptionType.INSTANT, random.nextBoolean());
this.subscriptionRepository.updateSubscription(DEFAULT_ORG_ID, userIdOne, randomEventTypeTwo.getId(), SubscriptionType.INSTANT, random.nextBoolean());
this.subscriptionRepository.updateSubscription(DEFAULT_ORG_ID, userIdOne, randomEventTypeThree.getId(), SubscriptionType.INSTANT, random.nextBoolean());

this.subscriptionRepository.updateSubscription(DEFAULT_ORG_ID, userIdTwo, randomEventType.getId(), SubscriptionType.INSTANT, random.nextBoolean());
this.subscriptionRepository.updateSubscription(DEFAULT_ORG_ID, userIdTwo, randomEventTypeTwo.getId(), SubscriptionType.INSTANT, random.nextBoolean());
this.subscriptionRepository.updateSubscription(DEFAULT_ORG_ID, userIdTwo, randomEventTypeThree.getId(), SubscriptionType.INSTANT, random.nextBoolean());

// Call the function under test.
final List<EventTypeEmailSubscription> subscriptions = this.subscriptionRepository.findEmailSubscriptionsByUserId(userIdOne);

// Assert that all the subscriptions belong to the target user ID.
for (final EventTypeEmailSubscription subscription : subscriptions) {
Assertions.assertEquals(userIdOne, subscription.getUserId(), "fetched a subscription from a different user");
}
}

/**
* Tests that an email subscription's user ID can be set to lowercase, and
* that when it is not possible due to an already existing email
* subscription, the email subscription gets deleted.
*/
@Test
void testSetSubscriptionUserIdLowercase() {
// Simulate that the default template is enabled so that we can create
// instant subscriptions without facing any "missing template" errors.
Mockito.when(this.backendConfig.isDefaultTemplateEnabled()).thenReturn(true);

// Create a single event type we want to create the subscriptions for.
final Bundle randomBundle = this.resourceHelpers.createBundle("random-bundle");
final Application randomApplication = this.resourceHelpers.createApplication(randomBundle.getId(), "random-application");
final EventType randomEventType = this.resourceHelpers.createEventType(randomApplication.getId(), "random-event-type");
final EventType randomEventTypeTwo = this.resourceHelpers.createEventType(randomApplication.getId(), "random-event-type-two");
final EventType randomEventTypeThree = this.resourceHelpers.createEventType(randomApplication.getId(), "random-event-type-three");

// Create some subscriptions for a user that should simply be
// renamed to lowercase.
final String userId = "userIdOne";
this.subscriptionRepository.updateSubscription(DEFAULT_ORG_ID, userId, randomEventType.getId(), SubscriptionType.INSTANT, true);
this.subscriptionRepository.updateSubscription(DEFAULT_ORG_ID, userId, randomEventTypeTwo.getId(), SubscriptionType.INSTANT, false);
this.subscriptionRepository.updateSubscription(DEFAULT_ORG_ID, userId, randomEventTypeThree.getId(), SubscriptionType.INSTANT, true);

// Find the email subscriptions for the user.
final List<EventTypeEmailSubscription> originalSubscriptions = this.subscriptionRepository.findEmailSubscriptionsByUserId(userId);

// Set the user ID to lowercase.
for (final EventTypeEmailSubscription subscription : originalSubscriptions) {
this.subscriptionRepository.setEmailSubscriptionUserIdLowercase(subscription);
}

// Attempt finding the email subscriptions again for the original user
// ID.
Assertions.assertEquals(
0,
this.subscriptionRepository.findEmailSubscriptionsByUserId(userId).size(),
String.format("fetched email subscriptions for \"%s\", when their user ID should have been renamed to lowercase", userId)
);

// Attempt finding the email subscriptions but for the lowercase user
// ID.
Assertions.assertEquals(
3,
this.subscriptionRepository.findEmailSubscriptionsByUserId(userId.toLowerCase()).size(),
String.format("user ID \"%s\" should have had all the subscriptions renamed to lowercase", userId.toLowerCase())
);

// Create the same subscriptions again for the previous user ID.
this.subscriptionRepository.updateSubscription(DEFAULT_ORG_ID, userId, randomEventType.getId(), SubscriptionType.INSTANT, true);
this.subscriptionRepository.updateSubscription(DEFAULT_ORG_ID, userId, randomEventTypeTwo.getId(), SubscriptionType.INSTANT, false);
this.subscriptionRepository.updateSubscription(DEFAULT_ORG_ID, userId, randomEventTypeThree.getId(), SubscriptionType.INSTANT, true);

// Find the email subscriptions for the user again.
final List<EventTypeEmailSubscription> duplicatedMixedCaseSubscriptions = this.subscriptionRepository.findEmailSubscriptionsByUserId(userId);

// Attempt setting the user ID to lowercase.
for (final EventTypeEmailSubscription subscription : duplicatedMixedCaseSubscriptions) {
this.subscriptionRepository.setEmailSubscriptionUserIdLowercase(subscription);
}

// Assert that the mixed case subscriptions were deleted.
Assertions.assertEquals(
0,
this.subscriptionRepository.findEmailSubscriptionsByUserId(userId).size(),
String.format("fetched email subscriptions for \"%s\", when the subscriptions themselves should have been deleted", userId)
);

// Assert that the email subscriptions with the lowercase user ID still
// exist.
Assertions.assertEquals(
3,
this.subscriptionRepository.findEmailSubscriptionsByUserId(userId.toLowerCase()).size(),
String.format("user ID \"%s\" should still have all the lowercase email subscriptions after deleting the mixed case ones", userId.toLowerCase())
);
}
}
Loading

0 comments on commit b583052

Please sign in to comment.