From fcd1fc097076c19ae6d0855d73813d7ed5cea911 Mon Sep 17 00:00:00 2001 From: vempo Date: Tue, 27 Nov 2018 15:12:27 +0200 Subject: Improved notification on item archive/restore Addressed code review comments, added unit-tests. Change-Id: I2a74e9969540cb636658aa012f82a81e0fd2eac2 Issue-ID: SDC-1667 Signed-off-by: vempo --- .../sdcrests/item/rest/services/ItemsImpl.java | 12 +++--- .../catalog/notification/AsyncNotifier.java | 49 +++++++++++++++++----- .../catalog/notification/NotifierFactory.java | 4 ++ .../notification/http/HttpTaskProducer.java | 33 +++++++++------ 4 files changed, 70 insertions(+), 28 deletions(-) (limited to 'openecomp-be/api/openecomp-sdc-rest-webapp/item-rest/item-rest-services/src/main/java/org/openecomp/sdcrests') diff --git a/openecomp-be/api/openecomp-sdc-rest-webapp/item-rest/item-rest-services/src/main/java/org/openecomp/sdcrests/item/rest/services/ItemsImpl.java b/openecomp-be/api/openecomp-sdc-rest-webapp/item-rest/item-rest-services/src/main/java/org/openecomp/sdcrests/item/rest/services/ItemsImpl.java index a93c063220..265570af4e 100644 --- a/openecomp-be/api/openecomp-sdc-rest-webapp/item-rest/item-rest-services/src/main/java/org/openecomp/sdcrests/item/rest/services/ItemsImpl.java +++ b/openecomp-be/api/openecomp-sdc-rest-webapp/item-rest/item-rest-services/src/main/java/org/openecomp/sdcrests/item/rest/services/ItemsImpl.java @@ -67,6 +67,8 @@ import static org.openecomp.sdc.versioning.VersioningNotificationConstansts.ITEM @Validated public class ItemsImpl implements Items { + private static final String ONBOARDING_METHOD = "onboardingMethod"; + private ItemManager itemManager = ItemManagerFactory.getInstance().createInterface(); private static ActivityLogManager activityLogManager = ActivityLogManagerFactory.getInstance().createInterface(); @@ -92,8 +94,6 @@ public class ItemsImpl implements Items { .put(ItemAction.RESTORE, new ActionSideAffects(ActivityType.Restore, NotificationEventTypes.RESTORE)); } - private static final String ONBOARDING_METHOD = "onboardingMethod"; - @Override public Response actOn(ItemActionRequestDto request, String itemId, String user) { @@ -115,10 +115,10 @@ public class ItemsImpl implements Items { actionSideAffectsMap.get(request.getAction()).execute(item, user); try { - Notifier notifier = NotifierFactory.getInstance(); - notifier.execute(Collections.singleton(itemId), request.getAction()); + Notifier catalogNotifier = NotifierFactory.getInstance(); + catalogNotifier.execute(Collections.singleton(itemId), request.getAction()); } catch (Exception e) { - LOGGER.error("Failed to send catalog notification on item " + itemId + " Error: " + e.getMessage()); + LOGGER.error("Failed to send catalog notification on item {}", itemId, e); } return Response.ok().build(); @@ -222,7 +222,7 @@ public class ItemsImpl implements Items { try { notifier.notifySubscribers(syncEvent, userName); } catch (Exception e) { - LOGGER.error("Failed to send sync notification to users subscribed to item '" + itemId); + LOGGER.error("Failed to send sync notification to users subscribed to item '{}'", itemId, e); } } } diff --git a/openecomp-be/api/openecomp-sdc-rest-webapp/item-rest/item-rest-services/src/main/java/org/openecomp/sdcrests/item/rest/services/catalog/notification/AsyncNotifier.java b/openecomp-be/api/openecomp-sdc-rest-webapp/item-rest/item-rest-services/src/main/java/org/openecomp/sdcrests/item/rest/services/catalog/notification/AsyncNotifier.java index 82880106d7..872c61e480 100644 --- a/openecomp-be/api/openecomp-sdc-rest-webapp/item-rest/item-rest-services/src/main/java/org/openecomp/sdcrests/item/rest/services/catalog/notification/AsyncNotifier.java +++ b/openecomp-be/api/openecomp-sdc-rest-webapp/item-rest/item-rest-services/src/main/java/org/openecomp/sdcrests/item/rest/services/catalog/notification/AsyncNotifier.java @@ -17,10 +17,12 @@ package org.openecomp.sdcrests.item.rest.services.catalog.notification; import java.util.Collection; +import java.util.Objects; import java.util.concurrent.Callable; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.BiFunction; import org.openecomp.sdc.logging.api.Logger; import org.openecomp.sdc.logging.api.LoggerFactory; @@ -41,9 +43,19 @@ public class AsyncNotifier implements Notifier { private static final long DEFAULT_INTERVAL = 5000; private final BiFunction, ItemAction, Callable> taskProducer; + private final int numberOfRetries; + private final long retryInterval; + AsyncNotifier(BiFunction, ItemAction, Callable> taskProducer) { + this(taskProducer, DEFAULT_NUM_OF_RETRIES, DEFAULT_INTERVAL); + } + + AsyncNotifier(BiFunction, ItemAction, Callable> taskProducer, int numOfRetries, + long retryInterval) { this.taskProducer = taskProducer; + this.numberOfRetries = numOfRetries; + this.retryInterval = retryInterval; } @Override @@ -51,8 +63,7 @@ public class AsyncNotifier implements Notifier { Callable worker = taskProducer.apply(itemIds, action); - RetryingTask retryingTask = - new RetryingTask(worker, DEFAULT_NUM_OF_RETRIES, DEFAULT_INTERVAL, EXECUTOR_SERVICE); + RetryingTask retryingTask = new RetryingTask(worker, numberOfRetries, retryInterval, EXECUTOR_SERVICE); EXECUTOR_SERVICE.submit(LoggingContext.copyToCallable(retryingTask)); } @@ -68,19 +79,37 @@ public class AsyncNotifier implements Notifier { private final Callable worker; private final long delay; private final ScheduledExecutorService scheduler; - private volatile int retries; + private final AtomicInteger retries; RetryingTask(Callable worker, int numOfRetries, long delay, ScheduledExecutorService scheduler) { - this.worker = worker; - this.retries = numOfRetries; - this.delay = delay; - this.scheduler = scheduler; + this.worker = Objects.requireNonNull(worker); + this.retries = new AtomicInteger(requirePositiveRetries(numOfRetries)); + this.delay = requirePositiveDelay(delay); + this.scheduler = Objects.requireNonNull(scheduler); + } + + private int requirePositiveRetries(int number) { + + if (number < 1) { + throw new IllegalArgumentException("Number of retries must be positive"); + } + + return number; + } + + private long requirePositiveDelay(long number) { + + if (number < 1) { + throw new IllegalArgumentException("Delay must be positive"); + } + + return number; } @Override - public synchronized Void call() throws Exception { + public Void call() throws Exception { NextAction next = worker.call(); if (next == NextAction.DONE) { @@ -88,8 +117,8 @@ public class AsyncNotifier implements Notifier { return null; } - retries--; - if (retries == 0) { + int attempts = retries.decrementAndGet(); + if (attempts < 1) { LOGGER.warn("Exhausted number of retries for task {}, exiting", worker); return null; } diff --git a/openecomp-be/api/openecomp-sdc-rest-webapp/item-rest/item-rest-services/src/main/java/org/openecomp/sdcrests/item/rest/services/catalog/notification/NotifierFactory.java b/openecomp-be/api/openecomp-sdc-rest-webapp/item-rest/item-rest-services/src/main/java/org/openecomp/sdcrests/item/rest/services/catalog/notification/NotifierFactory.java index a7f1e9c7fb..462ffdf00c 100644 --- a/openecomp-be/api/openecomp-sdc-rest-webapp/item-rest/item-rest-services/src/main/java/org/openecomp/sdcrests/item/rest/services/catalog/notification/NotifierFactory.java +++ b/openecomp-be/api/openecomp-sdc-rest-webapp/item-rest/item-rest-services/src/main/java/org/openecomp/sdcrests/item/rest/services/catalog/notification/NotifierFactory.java @@ -43,6 +43,10 @@ public class NotifierFactory { private static final Notifier INSTANCE; + private NotifierFactory() { + // prevent instantiation + } + static { INSTANCE = createInstance(); } diff --git a/openecomp-be/api/openecomp-sdc-rest-webapp/item-rest/item-rest-services/src/main/java/org/openecomp/sdcrests/item/rest/services/catalog/notification/http/HttpTaskProducer.java b/openecomp-be/api/openecomp-sdc-rest-webapp/item-rest/item-rest-services/src/main/java/org/openecomp/sdcrests/item/rest/services/catalog/notification/http/HttpTaskProducer.java index c6abd346ff..d210dc21af 100644 --- a/openecomp-be/api/openecomp-sdc-rest-webapp/item-rest/item-rest-services/src/main/java/org/openecomp/sdcrests/item/rest/services/catalog/notification/http/HttpTaskProducer.java +++ b/openecomp-be/api/openecomp-sdc-rest-webapp/item-rest/item-rest-services/src/main/java/org/openecomp/sdcrests/item/rest/services/catalog/notification/http/HttpTaskProducer.java @@ -17,6 +17,8 @@ package org.openecomp.sdcrests.item.rest.services.catalog.notification.http; import java.util.Collection; +import java.util.EnumMap; +import java.util.Map; import java.util.concurrent.Callable; import java.util.function.BiFunction; import org.openecomp.sdc.common.session.SessionContextProviderFactory; @@ -40,6 +42,13 @@ public class HttpTaskProducer private static final String CATALOG_HTTP_PROTOCOL = "HTTP"; private static final String CATALOG_HTTPS_PROTOCOL = "HTTPS"; + private static final Map ACTION_PATHS; + + static { + ACTION_PATHS = new EnumMap<>(ItemAction.class); + ACTION_PATHS.put(ItemAction.ARCHIVE, "archived"); + ACTION_PATHS.put(ItemAction.RESTORE, "restored"); + } private final String notifyCatalogUrl; @@ -81,15 +90,21 @@ public class HttpTaskProducer return createNotificationTask(itemIds, action); } - private static String getEndpoint(ItemAction action) { + private HttpNotificationTask createNotificationTask(Collection itemIds, ItemAction action) { + String userId = SessionContextProviderFactory.getInstance().createInterface().get().getUser().getUserId(); + String notificationEndpoint = notifyCatalogUrl + getApiPath(action); + LOGGER.debug("Catalog notification URL: {}", notificationEndpoint); + return new HttpNotificationTask(notificationEndpoint, userId, itemIds); + } - if (action == ItemAction.ARCHIVE) { - return "archived"; - } else if (action == ItemAction.RESTORE) { - return "restored"; - } else { + static String getApiPath(ItemAction action) { + + String path = ACTION_PATHS.get(action); + if (path == null) { throw new IllegalArgumentException("Unsupported action: " + action.name()); } + + return path; } @Override @@ -98,10 +113,4 @@ public class HttpTaskProducer task.call(); } - private HttpNotificationTask createNotificationTask(Collection itemIds, ItemAction action) { - String userId = SessionContextProviderFactory.getInstance().createInterface().get().getUser().getUserId(); - String notificationEndpoint = notifyCatalogUrl + getEndpoint(action); - LOGGER.debug("Catalog notification URL: " + notificationEndpoint); - return new HttpNotificationTask(notificationEndpoint, userId, itemIds); - } } -- cgit 1.2.3-korg