From 0bf4c0c18e6777113e9170b79cea24662b740286 Mon Sep 17 00:00:00 2001 From: Maciej Moscicki Date: Tue, 6 Aug 2024 15:36:56 +0200 Subject: [PATCH] do not remove subscription if it already existed before subscription creation (#1887) * do not remove subscription if it already existed before subscription creation * fix MultiDatacenterRepositoryCommandExecutorTest --- .../AddTopicToBlacklistRepositoryCommand.java | 2 +- ...veTopicFromBlacklistRepositoryCommand.java | 2 +- .../UpdateCredentialsRepositoryCommand.java | 2 +- ...tiDatacenterRepositoryCommandExecutor.java | 8 +-- .../domain/dc/RepositoryCommand.java | 2 +- .../CreateGroupRepositoryCommand.java | 2 +- .../RemoveGroupRepositoryCommand.java | 2 +- .../UpdateGroupRepositoryCommand.java | 2 +- .../CreateOAuthProviderRepositoryCommand.java | 2 +- .../RemoveOAuthProviderRepositoryCommand.java | 2 +- .../UpdateOAuthProviderRepositoryCommand.java | 2 +- .../domain/readiness/SetReadinessCommand.java | 2 +- ...reateOfflineRetransmissionTaskCommand.java | 2 +- ...eleteOfflineRetransmissionTaskCommand.java | 2 +- .../domain/retransmit/RetransmitCommand.java | 2 +- .../CreateSubscriptionRepositoryCommand.java | 7 ++- .../RemoveSubscriptionRepositoryCommand.java | 2 +- .../UpdateSubscriptionRepositoryCommand.java | 2 +- .../CreateTopicRepositoryCommand.java | 2 +- .../RemoveTopicRepositoryCommand.java | 2 +- .../commands/TouchTopicRepositoryCommand.java | 2 +- .../UpdateTopicRepositoryCommand.java | 2 +- ...scriptionConstraintsRepositoryCommand.java | 2 +- ...eateTopicConstraintsRepositoryCommand.java | 2 +- ...scriptionConstraintsRepositoryCommand.java | 2 +- ...leteTopicConstraintsRepositoryCommand.java | 2 +- ...scriptionConstraintsRepositoryCommand.java | 2 +- ...dateTopicConstraintsRepositoryCommand.java | 2 +- ...centerRepositoryCommandExecutorTest.groovy | 6 +-- ...teSubscriptionRepositoryCommandTest.groovy | 53 +++++++++++++++++++ 30 files changed, 92 insertions(+), 34 deletions(-) create mode 100644 hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/subscription/commands/CreateSubscriptionRepositoryCommandTest.groovy diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/blacklist/commands/AddTopicToBlacklistRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/blacklist/commands/AddTopicToBlacklistRepositoryCommand.java index 2bd07569ce..d4259545d8 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/blacklist/commands/AddTopicToBlacklistRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/blacklist/commands/AddTopicToBlacklistRepositoryCommand.java @@ -21,7 +21,7 @@ public void execute(DatacenterBoundRepositoryHolder ho } @Override - public void rollback(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { holder.getRepository().remove(qualifiedTopicName); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/blacklist/commands/RemoveTopicFromBlacklistRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/blacklist/commands/RemoveTopicFromBlacklistRepositoryCommand.java index 8515befcc1..1ca0ef3367 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/blacklist/commands/RemoveTopicFromBlacklistRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/blacklist/commands/RemoveTopicFromBlacklistRepositoryCommand.java @@ -23,7 +23,7 @@ public void execute(DatacenterBoundRepositoryHolder ho } @Override - public void rollback(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { if (exists) { holder.getRepository().add(qualifiedTopicName); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/credentials/commands/UpdateCredentialsRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/credentials/commands/UpdateCredentialsRepositoryCommand.java index 44ea88c3c6..f8c30cc7fc 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/credentials/commands/UpdateCredentialsRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/credentials/commands/UpdateCredentialsRepositoryCommand.java @@ -24,7 +24,7 @@ public void execute(DatacenterBoundRepositoryHolder holde } @Override - public void rollback(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { } @Override diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/dc/MultiDatacenterRepositoryCommandExecutor.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/dc/MultiDatacenterRepositoryCommandExecutor.java index e4baa9efec..2648407fa4 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/dc/MultiDatacenterRepositoryCommandExecutor.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/dc/MultiDatacenterRepositoryCommandExecutor.java @@ -55,7 +55,7 @@ private void execute(RepositoryCommand command, boolean isRollbackEnabled } catch (RepositoryNotAvailableException e) { logger.warn("Execute failed with an RepositoryNotAvailableException error", e); if (isRollbackEnabled) { - rollback(executedRepoHolders, command); + rollback(executedRepoHolders, command, e); } if (shouldStopExecutionOnFailure) { throw ExceptionWrapper.wrapInInternalProcessingExceptionIfNeeded(e, command.toString(), repoHolder.getDatacenterName()); @@ -63,19 +63,19 @@ private void execute(RepositoryCommand command, boolean isRollbackEnabled } catch (Exception e) { logger.warn("Failed to execute repository command: {} in ZK dc: {} in: {} ms", command, repoHolder.getDatacenterName(), System.currentTimeMillis() - start, e); if (isRollbackEnabled) { - rollback(executedRepoHolders, command); + rollback(executedRepoHolders, command, e); } throw ExceptionWrapper.wrapInInternalProcessingExceptionIfNeeded(e, command.toString(), repoHolder.getDatacenterName()); } } } - private void rollback(List> repoHolders, RepositoryCommand command) { + private void rollback(List> repoHolders, RepositoryCommand command, Exception exception) { long start = System.currentTimeMillis(); for (DatacenterBoundRepositoryHolder repoHolder : repoHolders) { logger.info("Executing rollback of repository command: {} in ZK dc: {}", command, repoHolder.getDatacenterName()); try { - command.rollback(repoHolder); + command.rollback(repoHolder, exception); logger.info("Successfully executed rollback of repository command: {} in ZK dc: {} in: {} ms", command, repoHolder.getDatacenterName(), System.currentTimeMillis() - start); } catch (Exception e) { logger.error("Rollback procedure failed for command {} on DC {}", command, repoHolder.getDatacenterName(), e); diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/dc/RepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/dc/RepositoryCommand.java index 246e0e3964..1b2d899137 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/dc/RepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/dc/RepositoryCommand.java @@ -6,7 +6,7 @@ public abstract class RepositoryCommand { public abstract void execute(DatacenterBoundRepositoryHolder holder); - public abstract void rollback(DatacenterBoundRepositoryHolder holder); + public abstract void rollback(DatacenterBoundRepositoryHolder holder, Exception exception); public abstract Class getRepositoryType(); diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/group/commands/CreateGroupRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/group/commands/CreateGroupRepositoryCommand.java index 3b67269d1b..cfea032d3d 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/group/commands/CreateGroupRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/group/commands/CreateGroupRepositoryCommand.java @@ -25,7 +25,7 @@ public void execute(DatacenterBoundRepositoryHolder holder) { } @Override - public void rollback(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { if (!exists) { holder.getRepository().removeGroup(group.getGroupName()); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/group/commands/RemoveGroupRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/group/commands/RemoveGroupRepositoryCommand.java index f9b41d8e6b..c4714744db 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/group/commands/RemoveGroupRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/group/commands/RemoveGroupRepositoryCommand.java @@ -26,7 +26,7 @@ public void execute(DatacenterBoundRepositoryHolder holder) { } @Override - public void rollback(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { holder.getRepository().createGroup(backup); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/group/commands/UpdateGroupRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/group/commands/UpdateGroupRepositoryCommand.java index c25dcd73e3..62fa38600c 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/group/commands/UpdateGroupRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/group/commands/UpdateGroupRepositoryCommand.java @@ -26,7 +26,7 @@ public void execute(DatacenterBoundRepositoryHolder holder) { } @Override - public void rollback(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { holder.getRepository().updateGroup(backup); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/oauth/commands/CreateOAuthProviderRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/oauth/commands/CreateOAuthProviderRepositoryCommand.java index d5b641b998..be440319d3 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/oauth/commands/CreateOAuthProviderRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/oauth/commands/CreateOAuthProviderRepositoryCommand.java @@ -22,7 +22,7 @@ public void execute(DatacenterBoundRepositoryHolder hol } @Override - public void rollback(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { holder.getRepository().removeOAuthProvider(provider.getName()); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/oauth/commands/RemoveOAuthProviderRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/oauth/commands/RemoveOAuthProviderRepositoryCommand.java index 4ff7f08f3d..a17a725779 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/oauth/commands/RemoveOAuthProviderRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/oauth/commands/RemoveOAuthProviderRepositoryCommand.java @@ -26,7 +26,7 @@ public void execute(DatacenterBoundRepositoryHolder hol } @Override - public void rollback(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { holder.getRepository().createOAuthProvider(backup); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/oauth/commands/UpdateOAuthProviderRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/oauth/commands/UpdateOAuthProviderRepositoryCommand.java index cf6ba8bdca..beeed66941 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/oauth/commands/UpdateOAuthProviderRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/oauth/commands/UpdateOAuthProviderRepositoryCommand.java @@ -26,7 +26,7 @@ public void execute(DatacenterBoundRepositoryHolder hol } @Override - public void rollback(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { holder.getRepository().updateOAuthProvider(backup); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/readiness/SetReadinessCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/readiness/SetReadinessCommand.java index 5cc0777f49..5dac6e9174 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/readiness/SetReadinessCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/readiness/SetReadinessCommand.java @@ -22,7 +22,7 @@ public void execute(DatacenterBoundRepositoryHolder holder) { } + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { } @Override public Class getRepositoryType() { diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/retransmit/CreateOfflineRetransmissionTaskCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/retransmit/CreateOfflineRetransmissionTaskCommand.java index b6d070a7b7..cfd2e40c61 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/retransmit/CreateOfflineRetransmissionTaskCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/retransmit/CreateOfflineRetransmissionTaskCommand.java @@ -21,7 +21,7 @@ public void execute(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { holder.getRepository().deleteTask(task.getTaskId()); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/retransmit/DeleteOfflineRetransmissionTaskCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/retransmit/DeleteOfflineRetransmissionTaskCommand.java index 8697d50505..14d0f2029c 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/retransmit/DeleteOfflineRetransmissionTaskCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/retransmit/DeleteOfflineRetransmissionTaskCommand.java @@ -21,7 +21,7 @@ public void execute(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/retransmit/RetransmitCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/retransmit/RetransmitCommand.java index eccd673d2b..669f45aca0 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/retransmit/RetransmitCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/retransmit/RetransmitCommand.java @@ -23,7 +23,7 @@ public void execute(DatacenterBoundRepositoryHolder holder) { } @Override - public void rollback(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/subscription/commands/CreateSubscriptionRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/subscription/commands/CreateSubscriptionRepositoryCommand.java index 7004608ff8..728dda8d7c 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/subscription/commands/CreateSubscriptionRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/subscription/commands/CreateSubscriptionRepositoryCommand.java @@ -1,6 +1,7 @@ package pl.allegro.tech.hermes.management.domain.subscription.commands; import pl.allegro.tech.hermes.api.Subscription; +import pl.allegro.tech.hermes.domain.subscription.SubscriptionAlreadyExistsException; import pl.allegro.tech.hermes.domain.subscription.SubscriptionRepository; import pl.allegro.tech.hermes.management.domain.dc.DatacenterBoundRepositoryHolder; import pl.allegro.tech.hermes.management.domain.dc.RepositoryCommand; @@ -22,7 +23,11 @@ public void execute(DatacenterBoundRepositoryHolder hold } @Override - public void rollback(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { + if (exception instanceof SubscriptionAlreadyExistsException) { + // prevents removal of already existing subscription + return; + } holder.getRepository().removeSubscription(subscription.getTopicName(), subscription.getName()); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/subscription/commands/RemoveSubscriptionRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/subscription/commands/RemoveSubscriptionRepositoryCommand.java index 9d0c2d838d..d1b346b3dc 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/subscription/commands/RemoveSubscriptionRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/subscription/commands/RemoveSubscriptionRepositoryCommand.java @@ -30,7 +30,7 @@ public void execute(DatacenterBoundRepositoryHolder hold } @Override - public void rollback(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { holder.getRepository().createSubscription(backup); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/subscription/commands/UpdateSubscriptionRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/subscription/commands/UpdateSubscriptionRepositoryCommand.java index 3d0684f2f1..c0ac52c3c2 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/subscription/commands/UpdateSubscriptionRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/subscription/commands/UpdateSubscriptionRepositoryCommand.java @@ -25,7 +25,7 @@ public void execute(DatacenterBoundRepositoryHolder hold } @Override - public void rollback(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { holder.getRepository().updateSubscription(backup); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/topic/commands/CreateTopicRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/topic/commands/CreateTopicRepositoryCommand.java index 1666c50bf8..2842b7e988 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/topic/commands/CreateTopicRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/topic/commands/CreateTopicRepositoryCommand.java @@ -22,7 +22,7 @@ public void execute(DatacenterBoundRepositoryHolder holder) { } @Override - public void rollback(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { /* We don't want to do a rollback due to possible race conditions with creating a topic on Kafka. It increases the probability of discrepancies between Kafka and Zookeeper: topic exists in Kafka, diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/topic/commands/RemoveTopicRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/topic/commands/RemoveTopicRepositoryCommand.java index d378548603..397c60c295 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/topic/commands/RemoveTopicRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/topic/commands/RemoveTopicRepositoryCommand.java @@ -22,7 +22,7 @@ public void execute(DatacenterBoundRepositoryHolder holder) { } @Override - public void rollback(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { /* We don't want to do a rollback due to possible race conditions with creating a topic on Kafka. It increases the probability of discrepancies between Kafka and Zookeeper: topic exists in Kafka, diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/topic/commands/TouchTopicRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/topic/commands/TouchTopicRepositoryCommand.java index b210658ca7..d4a850f681 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/topic/commands/TouchTopicRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/topic/commands/TouchTopicRepositoryCommand.java @@ -22,7 +22,7 @@ public void execute(DatacenterBoundRepositoryHolder holder) { } @Override - public void rollback(DatacenterBoundRepositoryHolder holder) {} + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) {} @Override public Class getRepositoryType() { diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/topic/commands/UpdateTopicRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/topic/commands/UpdateTopicRepositoryCommand.java index 85fdea7c6d..0f007f5fe9 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/topic/commands/UpdateTopicRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/topic/commands/UpdateTopicRepositoryCommand.java @@ -26,7 +26,7 @@ public void execute(DatacenterBoundRepositoryHolder holder) { } @Override - public void rollback(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { holder.getRepository().updateTopic(backup); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/CreateSubscriptionConstraintsRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/CreateSubscriptionConstraintsRepositoryCommand.java index 9f064531a2..bf38652177 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/CreateSubscriptionConstraintsRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/CreateSubscriptionConstraintsRepositoryCommand.java @@ -28,7 +28,7 @@ public void execute(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { if (!exists) { holder.getRepository().deleteConstraints(subscriptionName); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/CreateTopicConstraintsRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/CreateTopicConstraintsRepositoryCommand.java index 33e924e63e..e96ba4875d 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/CreateTopicConstraintsRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/CreateTopicConstraintsRepositoryCommand.java @@ -28,7 +28,7 @@ public void execute(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { if (!exist) { holder.getRepository().deleteConstraints(topicName); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/DeleteSubscriptionConstraintsRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/DeleteSubscriptionConstraintsRepositoryCommand.java index 7f04122339..3f0f713367 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/DeleteSubscriptionConstraintsRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/DeleteSubscriptionConstraintsRepositoryCommand.java @@ -26,7 +26,7 @@ public void execute(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { if (backup != null) { holder.getRepository().createConstraints(subscriptionName, backup); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/DeleteTopicConstraintsRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/DeleteTopicConstraintsRepositoryCommand.java index 4f19ea376e..981a198968 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/DeleteTopicConstraintsRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/DeleteTopicConstraintsRepositoryCommand.java @@ -26,7 +26,7 @@ public void execute(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { if (backup != null) { holder.getRepository().createConstraints(topicName, backup); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/UpdateSubscriptionConstraintsRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/UpdateSubscriptionConstraintsRepositoryCommand.java index bb237f9e2f..ade0564ee3 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/UpdateSubscriptionConstraintsRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/UpdateSubscriptionConstraintsRepositoryCommand.java @@ -28,7 +28,7 @@ public void execute(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { if (backup != null) { holder.getRepository().updateConstraints(subscriptionName, backup); } diff --git a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/UpdateTopicConstraintsRepositoryCommand.java b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/UpdateTopicConstraintsRepositoryCommand.java index 6bf4f8ac8c..e69d08642b 100644 --- a/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/UpdateTopicConstraintsRepositoryCommand.java +++ b/hermes-management/src/main/java/pl/allegro/tech/hermes/management/domain/workload/constraints/command/UpdateTopicConstraintsRepositoryCommand.java @@ -28,7 +28,7 @@ public void execute(DatacenterBoundRepositoryHolder holder) { + public void rollback(DatacenterBoundRepositoryHolder holder, Exception exception) { if (backup != null) { holder.getRepository().updateConstraints(topicName, backup); } diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/dc/MultiDatacenterRepositoryCommandExecutorTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/dc/MultiDatacenterRepositoryCommandExecutorTest.groovy index b6f8ec4807..d6e621932f 100644 --- a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/dc/MultiDatacenterRepositoryCommandExecutorTest.groovy +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/dc/MultiDatacenterRepositoryCommandExecutorTest.groovy @@ -66,7 +66,7 @@ class MultiDatacenterRepositoryCommandExecutorTest extends Specification { executor.execute(command) then: - 1 * command.rollback(holder1) + 1 * command.rollback(holder1, _) thrown InternalProcessingException } @@ -104,7 +104,7 @@ class MultiDatacenterRepositoryCommandExecutorTest extends Specification { executor.executeByUser(command, ADMIN) then: - 1 * command.rollback(holder1) + 1 * command.rollback(holder1, _) thrown InternalProcessingException } @@ -160,7 +160,7 @@ class MultiDatacenterRepositoryCommandExecutorTest extends Specification { executor.executeByUser(command, NON_ADMIN) then: - 1 * command.rollback(holder1) + 1 * command.rollback(holder1, _) thrown InternalProcessingException } diff --git a/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/subscription/commands/CreateSubscriptionRepositoryCommandTest.groovy b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/subscription/commands/CreateSubscriptionRepositoryCommandTest.groovy new file mode 100644 index 0000000000..47d59dfddb --- /dev/null +++ b/hermes-management/src/test/groovy/pl/allegro/tech/hermes/management/domain/subscription/commands/CreateSubscriptionRepositoryCommandTest.groovy @@ -0,0 +1,53 @@ +package pl.allegro.tech.hermes.management.domain.subscription.commands + +import pl.allegro.tech.hermes.api.Subscription +import pl.allegro.tech.hermes.api.TopicName +import pl.allegro.tech.hermes.domain.subscription.SubscriptionAlreadyExistsException +import pl.allegro.tech.hermes.domain.subscription.SubscriptionRepository +import pl.allegro.tech.hermes.management.domain.dc.DatacenterBoundRepositoryHolder +import spock.lang.Shared +import spock.lang.Specification + +import static pl.allegro.tech.hermes.test.helper.builder.SubscriptionBuilder.subscription + +class CreateSubscriptionRepositoryCommandTest extends Specification { + + @Shared + def topicName = new TopicName("group", "topic") + + @Shared + def subscriptionName = "subscription" + + @Shared + Subscription subscription = subscription(topicName, subscriptionName).build() + + def "should not remove subscription if subscription already exists during rollback"() { + given: + SubscriptionRepository subscriptionRepository = Mock(SubscriptionRepository) + DatacenterBoundRepositoryHolder repository = Mock(DatacenterBoundRepositoryHolder) { + getRepository() >> subscriptionRepository + } + def command = new CreateSubscriptionRepositoryCommand(subscription) + + when: + command.rollback(repository, new SubscriptionAlreadyExistsException(subscription)) + + then: + 0 * subscriptionRepository.removeSubscription(topicName, subscriptionName) + } + + def "should remove subscription during rollback"() { + given: + SubscriptionRepository subscriptionRepository = Mock(SubscriptionRepository) + DatacenterBoundRepositoryHolder repository = Mock(DatacenterBoundRepositoryHolder) { + getRepository() >> subscriptionRepository + } + def command = new CreateSubscriptionRepositoryCommand(subscription) + + when: + command.rollback(repository, new RuntimeException()) + + then: + 1 * subscriptionRepository.removeSubscription(topicName, subscriptionName) + } +}