bug6017: added locking also for replicated subscription operations and in all handlers

This commit is contained in:
Axel Uhl
2024-06-14 16:21:20 +02:00
parent 8220d36f5d
commit 53ba61d131
6 changed files with 236 additions and 182 deletions
@@ -809,4 +809,26 @@ public interface SecurityService extends ReplicableWithObjectInputStream<Replica
*/
Role getOrThrowRoleFromIDsAndCheckMetaPermissions(UUID roleDefinitionId, UUID qualifyingGroupId, String userQualifierName,
boolean transitive) throws UserManagementException;
/**
* When updating a user's {@link User#getSubscriptions() subscriptions}, use this method to obtain a write lock. For
* complex operations, such as first reading the current subscriptions, then manipulating them and writing them back
* to the user object, ensure the lock is held throughout the complex operation sequence.
* <p>
*
* Unlock again using {@ink #unlockSubscriptionsForUser(User)}, always in a {@code finally} clause, to avoid hanging
* locks.
* <p>
*
* The lock will also be obtained upon processing a replication operation (see, e.g.,
* {@link ReplicableSecurityService#internalUpdateSubscription(String, Subscription)}) internally. This way,
* compound operations are also protected against concurrent manipulation through replicated transactions.
*/
void lockSubscriptionsForUser(User user);
/**
* Releases the lock obtained with {@link #lockSubscriptionsForUser(User)}. Always call this in a {@code finally} clause
* that follows the {@link #lockSubscriptionsForUser(User)} call, so a lock can never hang.
*/
void unlockSubscriptionsForUser(User user);
}
@@ -85,6 +85,8 @@ import com.sap.sse.ServerInfo;
import com.sap.sse.common.Util;
import com.sap.sse.common.Util.Pair;
import com.sap.sse.common.mail.MailException;
import com.sap.sse.concurrent.LockUtil;
import com.sap.sse.concurrent.NamedReentrantReadWriteLock;
import com.sap.sse.i18n.impl.ResourceBundleStringMessagesImpl;
import com.sap.sse.mail.MailService;
import com.sap.sse.replication.interfaces.impl.AbstractReplicableWithObjectInputStream;
@@ -234,11 +236,19 @@ implements ReplicableSecurityService, ClearStateTestSupport {
private final PermissionChangeListeners permissionChangeListeners;
private final ClassLoaderRegistry initialLoadClassLoaderRegistry = ClassLoaderRegistry.createInstance();
/**
* When working with a user's subscriptions, such as first reading, then changing and updating a user's subscription
* based on what was read, a user-specific write lock must be obtained to ensure that no writes can cut in between.
* See also {@link #lockSubscriptionsForUser} and {@link #unlockSubscriptionsForUser}.
*/
private final static ConcurrentMap<User, NamedReentrantReadWriteLock> subscriptionLocksForUsers;
static {
shiroConfiguration = new Ini();
shiroConfiguration.loadFromPath("classpath:shiro.ini");
shiroEnvironment = new BasicIniEnvironment("classpath:shiro.ini");
subscriptionLocksForUsers = new ConcurrentHashMap<>();
}
/**
@@ -2611,40 +2621,45 @@ implements ReplicableSecurityService, ClearStateTestSupport {
throws UserManagementException {
final User user = getUserByName(username);
if (user != null) {
final String newSubscriptionPlanId = newSubscription.getPlanId();
final Subscription currentSubscription = user.getSubscriptionByPlan(newSubscriptionPlanId);
if (shouldProcessNewSubscription(currentSubscription, newSubscription)) {
logger.info(() -> "Update user subscription for plan " + newSubscriptionPlanId);
logger.info(() -> "Current user plan subscription: "
+ (currentSubscription != null ? currentSubscription.toString() : "null"));
logger.info(() -> "New plan subscription: "
+ (newSubscription != null ? newSubscription.toString() : "null"));
// In some cases there is no invoice or transaction information. E.g. if the subscription has
// been cancelled.
// To ensure information about previous payments is preserved, the subscription is patched.
if (currentSubscription != null && newSubscription != null
&& newSubscription.getSubscriptionId() != null
&& newSubscription.getSubscriptionId().equals(currentSubscription.getSubscriptionId())) {
if (currentSubscription.getTransactionStatus() != null
&& newSubscription.getTransactionStatus() == null) {
newSubscription.patchTransactionData(currentSubscription);
lockSubscriptionsForUser(user);
try {
final String newSubscriptionPlanId = newSubscription.getPlanId();
final Subscription currentSubscription = user.getSubscriptionByPlan(newSubscriptionPlanId);
if (shouldProcessNewSubscription(currentSubscription, newSubscription)) {
logger.info(() -> "Update user subscription for plan " + newSubscriptionPlanId);
logger.info(() -> "Current user plan subscription: "
+ (currentSubscription != null ? currentSubscription.toString() : "null"));
logger.info(() -> "New plan subscription: "
+ (newSubscription != null ? newSubscription.toString() : "null"));
// In some cases there is no invoice or transaction information. E.g. if the subscription has
// been cancelled.
// To ensure information about previous payments is preserved, the subscription is patched.
if (currentSubscription != null && newSubscription != null
&& newSubscription.getSubscriptionId() != null
&& newSubscription.getSubscriptionId().equals(currentSubscription.getSubscriptionId())) {
if (currentSubscription.getTransactionStatus() != null
&& newSubscription.getTransactionStatus() == null) {
newSubscription.patchTransactionData(currentSubscription);
}
if (currentSubscription.getInvoiceId() != null && newSubscription.getInvoiceId() == null) {
newSubscription.patchInvoiceData(currentSubscription);
}
}
if (currentSubscription.getInvoiceId() != null && newSubscription.getInvoiceId() == null) {
newSubscription.patchInvoiceData(currentSubscription);
if (shouldUpdateUserRolesForSubscription(user, currentSubscription, newSubscription)) {
updateUserRolesOnSubscriptionChange(user, currentSubscription, newSubscription);
}
final Subscription[] newSubscriptions = buildNewUserSubscriptions(user, newSubscription);
if (newSubscriptions != null) {
user.setSubscriptions(newSubscriptions);
}
store.updateUser(user);
} else {
logger.info(() -> "New subscription has been ignored: " + newSubscription);
}
if (shouldUpdateUserRolesForSubscription(user, currentSubscription, newSubscription)) {
updateUserRolesOnSubscriptionChange(user, currentSubscription, newSubscription);
}
final Subscription[] newSubscriptions = buildNewUserSubscriptions(user, newSubscription);
if (newSubscriptions != null) {
user.setSubscriptions(newSubscriptions);
}
store.updateUser(user);
} else {
logger.info(() -> "New subscription has been ignored: " + newSubscription);
return null;
} finally {
unlockSubscriptionsForUser(user);
}
return null;
} else {
throw new UserManagementException(UserManagementException.USER_DOES_NOT_EXIST);
}
@@ -3032,4 +3047,14 @@ implements ReplicableSecurityService, ClearStateTestSupport {
}
return role;
}
@Override
public void lockSubscriptionsForUser(final User user) {
LockUtil.lockForWrite(subscriptionLocksForUsers.computeIfAbsent(user, u->new NamedReentrantReadWriteLock("Subscriptions lock for user "+user.getName(), /* fair */ false)));
}
@Override
public void unlockSubscriptionsForUser(final User user) {
LockUtil.unlockAfterWrite(subscriptionLocksForUsers.computeIfAbsent(user, u->new NamedReentrantReadWriteLock("Subscriptions lock for user "+user.getName(), /* fair */ false)));
}
}
@@ -62,43 +62,48 @@ public class ProviderSubscriptionUpdateTask implements SubscriptionApiService.On
SubscriptionApiService provider) throws UserManagementException {
logger.info(() -> "Subscriptions from provider " + provider.getProviderName() + " for user " + user.getName()
+ ": " + (userSubscriptions == null ? "empty" : userSubscriptions));
Iterable<Subscription> currentSubscriptions = user.getSubscriptions();
if ((userSubscriptions == null || !userSubscriptions.iterator().hasNext())
&& (currentSubscriptions != null && currentSubscriptions.iterator().hasNext())) {
// No subscriptions so we need to remove all current subscriptions of user for the provider
Subscription emptySubscription = createEmptySubscription(provider, null);
getSecurityService().updateUserSubscription(user.getName(), emptySubscription);
} else if (userSubscriptions != null) {
if (currentSubscriptions != null) {
Map<String, Boolean> existingPlans = getExistingPlans(userSubscriptions);
for (Subscription subscription : currentSubscriptions) {
if (subscription.hasPlan() && !existingPlans.containsKey(subscription.getPlanId())) {
// Current subscription plan doesn't exist in subscription list from provider, that means
// subscription for the plan has been deleted, then we need to remove the subscription from
// database
Subscription emptySubscription = createEmptySubscription(provider, subscription.getPlanId());
getSecurityService().updateUserSubscription(user.getName(), emptySubscription);
getSecurityService().lockSubscriptionsForUser(user);
try {
Iterable<Subscription> currentSubscriptions = user.getSubscriptions();
if ((userSubscriptions == null || !userSubscriptions.iterator().hasNext())
&& (currentSubscriptions != null && currentSubscriptions.iterator().hasNext())) {
// No subscriptions so we need to remove all current subscriptions of user for the provider
Subscription emptySubscription = createEmptySubscription(provider, null);
getSecurityService().updateUserSubscription(user.getName(), emptySubscription);
} else if (userSubscriptions != null) {
if (currentSubscriptions != null) {
Map<String, Boolean> existingPlans = getExistingPlans(userSubscriptions);
for (Subscription subscription : currentSubscriptions) {
if (subscription.hasPlan() && !existingPlans.containsKey(subscription.getPlanId())) {
// Current subscription plan doesn't exist in subscription list from provider, that means
// subscription for the plan has been deleted, then we need to remove the subscription from
// database
Subscription emptySubscription = createEmptySubscription(provider, subscription.getPlanId());
getSecurityService().updateUserSubscription(user.getName(), emptySubscription);
}
}
}
}
// Only one subscription per plan has to be processed.
final Set<Subscription> skimmedSubscriptions = new HashSet<>();
for (SubscriptionPlan subscriptionPlan : getSecurityService().getAllSubscriptionPlans().values()) {
Subscription planSubscription = null;
for(Subscription userSubscription : userSubscriptions) {
final boolean isSamePlan = subscriptionPlan.getId().equals(userSubscription.getPlanId());
if (isSamePlan && planSubscription == null ||
isSamePlan && userSubscription.isUpdatedMoreRecently(planSubscription)) {
planSubscription = userSubscription;
// Only one subscription per plan has to be processed.
final Set<Subscription> skimmedSubscriptions = new HashSet<>();
for (SubscriptionPlan subscriptionPlan : getSecurityService().getAllSubscriptionPlans().values()) {
Subscription planSubscription = null;
for(Subscription userSubscription : userSubscriptions) {
final boolean isSamePlan = subscriptionPlan.getId().equals(userSubscription.getPlanId());
if (isSamePlan && planSubscription == null ||
isSamePlan && userSubscription.isUpdatedMoreRecently(planSubscription)) {
planSubscription = userSubscription;
}
}
if (planSubscription != null) {
skimmedSubscriptions.add(planSubscription);
}
}
if (planSubscription != null) {
skimmedSubscriptions.add(planSubscription);
for (Subscription subscription : skimmedSubscriptions) {
getSecurityService().updateUserSubscription(user.getName(), subscription);
}
}
for (Subscription subscription : skimmedSubscriptions) {
getSecurityService().updateUserSubscription(user.getName(), subscription);
}
} finally {
getSecurityService().unlockSubscriptionsForUser(user);
}
}