Improved notification on item archive/restore

Addressed code review comments, added unit-tests.

Change-Id: I2a74e9969540cb636658aa012f82a81e0fd2eac2
Issue-ID: SDC-1667
Signed-off-by: vempo <vitaliy.emporopulo@amdocs.com>

4 files changed, 70 insertions, 28 deletions

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<Collection<String>, ItemAction, Callable<NextAction>> taskProducer;
+    private final int numberOfRetries;
+    private final long retryInterval;
+
 
     AsyncNotifier(BiFunction<Collection<String>, ItemAction, Callable<NextAction>> taskProducer) {
+        this(taskProducer, DEFAULT_NUM_OF_RETRIES, DEFAULT_INTERVAL);
+    }
+
+    AsyncNotifier(BiFunction<Collection<String>, ItemAction, Callable<NextAction>> 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<AsyncNotifier.NextAction> 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<AsyncNotifier.NextAction> worker;
         private final long delay;
         private final ScheduledExecutorService scheduler;
-        private volatile int retries;
+        private final AtomicInteger retries;
 
         RetryingTask(Callable<AsyncNotifier.NextAction> 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<ItemAction, String> 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<String> 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<String> 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);
-    }
 }