Added unit tests for queue package

Author: pc9795

src/main/java/com/exceptionless/exceptionlessclient/ExceptionlessClient.java

@@ -43,7 +43,7 @@ private void init() {
           @Override
           public void run() {
             try {
-              configurationManager.getSettingsManager().updateSettingsThreadSafe();
+              configurationManager.getSettingsManager().updateSettings();
             } catch (Exception e) {
               LOG.error("Error in updating settings", e);
             }
@@ -53,12 +53,12 @@ public void run() {
         configurationManager.getConfiguration().getUpdateSettingsWhenIdleInterval());
 
     configurationManager.onChanged(
-        ignored -> configurationManager.getSettingsManager().updateSettingsThreadSafe());
+        ignored -> configurationManager.getSettingsManager().updateSettings());
     configurationManager
         .getQueue()
         .onEventsPosted(
             (ignored1, ignored2) ->
-                configurationManager.getSettingsManager().updateSettingsThreadSafe());
+                configurationManager.getSettingsManager().updateSettings());
   }
 
   public static ExceptionlessClient from(String apiKey, String serverUrl) {

src/main/java/com/exceptionless/exceptionlessclient/models/submission/SubmissionResponse.java

@@ -8,8 +8,8 @@
 @Value
 @NonFinal
 public class SubmissionResponse {
-  private int statusCode;
-  private String message;
+  int statusCode;
+  String message;
 
   public boolean isSuccess() {
     return statusCode >= 200 && statusCode <= 299;

src/main/java/com/exceptionless/exceptionlessclient/queue/DefaultEventQueue.java

@@ -7,6 +7,7 @@
 import com.exceptionless.exceptionlessclient.models.submission.SubmissionResponse;
 import com.exceptionless.exceptionlessclient.storage.StorageProviderIF;
 import com.exceptionless.exceptionlessclient.submission.SubmissionClientIF;
+import com.exceptionless.exceptionlessclient.utils.VisibleForTesting;
 import lombok.Builder;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -61,6 +62,7 @@ public void run() {
             }
           }
         },
+        processingIntervalInSecs * 1000,
         processingIntervalInSecs * 1000);
   }
 
@@ -84,17 +86,23 @@ private void process() {
     process(false);
   }
 
+  @VisibleForTesting
+  Boolean isProcessingCurrentlySuspended(){
+    return shouldSuspendProcessing();
+  }
+
   @Override
   public void enqueue(Event event) {
     if (shouldDiscard()) {
       LOG.info("Queue items are currently being discarded. This event will not be enqueued");
+      return;
     }
 
     long timestamp = storageProvider.getQueue().save(event);
     String logText =
         String.format("type: %s", event.getType())
             + (event.getReferenceId() != null
-                ? String.format("refId: %s", event.getReferenceId())
+                ? String.format(", refId: %s", event.getReferenceId())
                 : "");
     LOG.info(String.format("Enqueueing event: %s %s", timestamp, logText));
   }
@@ -109,17 +117,24 @@ private boolean shouldDiscard() {
 
   @Override
   public void process(boolean isAppExiting) {
-    if (processingQueue) {
-      return;
+    synchronized (this){
+      if (processingQueue) {
+        LOG.trace("Currently processing queue; Returning...");
+        return;
+      }
+      processingQueue = true;
     }
 
-    processingQueue = true;
     try {
       List<StorageItem<Event>> storedEvents =
           storageProvider.getQueue().get(currentSubmissionBatchSize);
+      if (storedEvents.isEmpty()) {
+        LOG.trace("No events found to submit; Returning...");
+        return;
+      }
+
       List<Event> events =
           storedEvents.stream().map(StorageItem::getValue).collect(Collectors.toList());
-
       LOG.info(
           String.format("Sending %s events to %s", events.size(), configuration.getServerUrl()));
       SubmissionResponse response = submissionClient.postEvents(events, isAppExiting);
@@ -129,7 +144,9 @@ public void process(boolean isAppExiting) {
       LOG.error("Error processing queue", e);
       suspendProcessing();
     } finally {
-      processingQueue = false;
+      synchronized (this){
+        processingQueue = false;
+      }
     }
   }
 

src/main/java/com/exceptionless/exceptionlessclient/queue/EventDataFilter.java

@@ -39,7 +39,7 @@ private Object filter(Object data, int currDepth) {
 
     if (data instanceof List) {
       List<Object> dataList = (List<Object>) data;
-      return dataList.stream().map(val -> filter(val, currDepth + 1)).collect(Collectors.toList());
+      return dataList.stream().map(val -> filter(val, currDepth)).collect(Collectors.toList());
     }
 
     return ((Map<String, Object>) data)

src/main/java/com/exceptionless/exceptionlessclient/queue/EventValidator.java

@@ -1,6 +1,5 @@
 package com.exceptionless.exceptionlessclient.queue;
 
-
 import com.exceptionless.exceptionlessclient.exceptions.BadEventDataException;
 
 public final class EventValidator {
@@ -12,12 +11,15 @@ private EventValidator() {}
   public static void validateIdentifier(String value) {
     if (value.length() < VALID_IDENTIFIER_MIN_LENGTH
         || value.length() > VALID_IDENTIFIER_MAX_LENGTH) {
-      throw new BadEventDataException("Value must contain between 8 and 100 characters");
+      throw new BadEventDataException(
+          String.format("Value must contain between 8 and 100 characters, found: [%s]", value));
     }
 
     if (!value.chars().allMatch(ch -> Character.isLetterOrDigit(ch) || ch == '-')) {
       throw new BadEventDataException(
-          "Value should contain only alphanumeric characters and minus sign");
+          String.format(
+              "Value should contain only alphanumeric characters and hyphen sign, found: [%s]",
+              value));
     }
   }
 

src/main/java/com/exceptionless/exceptionlessclient/settings/SettingsManager.java

@@ -40,7 +40,7 @@ public void checkVersion(long version) {
     }
 
     LOG.info(String.format("Updating settings from v%s to v%s", currentVersion, version));
-    updateSettingsThreadSafe();
+    updateSettings();
   }
 
   private long getVersion() {
@@ -56,32 +56,31 @@ public ServerSettings getSavedServerSettings() {
     return storageItem.getValue();
   }
 
-  // This method is thread safe as settings are updated both by the users and by the client at
-  // regular intervals
-  public synchronized void updateSettingsThreadSafe() {
-    if (updatingSettings) {
-      return;
+  public void updateSettings() {
+    synchronized (this) {
+      if (updatingSettings) {
+        LOG.trace("Already updating settings; Returning...");
+        return;
+      }
+      updatingSettings = true;
     }
 
-    updatingSettings = true;
     try {
-      updateSettings();
-    } finally {
-      updatingSettings = false;
-    }
-  }
+      long currentVersion = getVersion();
+      LOG.info(String.format("Checking for updated settings  from: v%s", currentVersion));
 
-  private void updateSettings() {
-    long currentVersion = getVersion();
-    LOG.info(String.format("Checking for updated settings  from: v%s", currentVersion));
-
-    SettingsResponse response = settingsClient.getSettings(currentVersion);
-    if (!response.isSuccess()) {
-      LOG.warn(String.format("Unable to update settings: %s:", response.getMessage()));
-      return;
+      SettingsResponse response = settingsClient.getSettings(currentVersion);
+      if (!response.isSuccess()) {
+        LOG.warn(String.format("Unable to update settings: %s:", response.getMessage()));
+        return;
+      }
+      ServerSettings prevValue = storageProvider.getSettings().peek().getValue();
+      storageProvider.getSettings().save(response.getSettings());
+      propertyChangeSupport.firePropertyChange("settings", prevValue, response.getSettings());
+    } finally {
+      synchronized (this) {
+        updatingSettings = false;
+      }
     }
-    ServerSettings prevValue = storageProvider.getSettings().peek().getValue();
-    storageProvider.getSettings().save(response.getSettings());
-    propertyChangeSupport.firePropertyChange("settings", prevValue, response.getSettings());
   }
 }

src/main/java/com/exceptionless/exceptionlessclient/storage/InMemoryStorage.java

@@ -37,7 +37,7 @@ public StorageItem<X> peek() {
 
   @Override
   public List<StorageItem<X>> get(int limit) {
-    return items.subList(0, Math.min(limit, items.size()));
+    return new ArrayList<>(items.subList(0, Math.min(limit, items.size())));
   }
 
   @Override

src/main/java/com/exceptionless/exceptionlessclient/utils/VisibleForTesting.java

@@ -0,0 +1,3 @@
+package com.exceptionless.exceptionlessclient.utils;
+
+public @interface VisibleForTesting {}