You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by cs...@apache.org on 2019/12/30 10:14:45 UTC

[sling-org-apache-sling-distribution-journal] branch master updated: Fixes from sonar report (#22)

This is an automated email from the ASF dual-hosted git repository.

cschneider pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-distribution-journal.git


The following commit(s) were added to refs/heads/master by this push:
     new 29e2a39  Fixes from sonar report (#22)
29e2a39 is described below

commit 29e2a39c533de7c10fca3f18c0d094688b913e21
Author: Christian Schneider <ch...@die-schneider.net>
AuthorDate: Mon Dec 30 11:14:36 2019 +0100

    Fixes from sonar report (#22)
    
    * SLING-8932 - Fix issues from sonar report
    
    * SLING-8932 - Some more sonar fixes. Improved logging performance
    
    * SLING-8932 - Fix error in log string
---
 .../journal/impl/event/DistributionEvent.java             |  3 +++
 .../distribution/journal/impl/publisher/PackageRepo.java  |  6 +++---
 .../journal/impl/shared/DefaultDistributionLog.java       | 15 ++++++++-------
 .../journal/impl/subscriber/DistributionSubscriber.java   | 11 ++++++-----
 .../journal/impl/subscriber/PackageHandler.java           |  2 +-
 5 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/src/main/java/org/apache/sling/distribution/journal/impl/event/DistributionEvent.java b/src/main/java/org/apache/sling/distribution/journal/impl/event/DistributionEvent.java
index 5811357..6c61274 100644
--- a/src/main/java/org/apache/sling/distribution/journal/impl/event/DistributionEvent.java
+++ b/src/main/java/org/apache/sling/distribution/journal/impl/event/DistributionEvent.java
@@ -48,6 +48,9 @@ public class DistributionEvent {
     private static final String KIND_AGENT = "agent";
     private static final String KIND_IMPORTER = "importer";
 
+    private DistributionEvent() {
+    }
+    
     public static Event eventImporterImported(Messages.PackageMessage pkgMsg, String agentName) {
         return buildEvent(IMPORTER_PACKAGE_IMPORTED, KIND_IMPORTER, agentName, pkgMsg);
     }
diff --git a/src/main/java/org/apache/sling/distribution/journal/impl/publisher/PackageRepo.java b/src/main/java/org/apache/sling/distribution/journal/impl/publisher/PackageRepo.java
index e87a960..aee0c52 100644
--- a/src/main/java/org/apache/sling/distribution/journal/impl/publisher/PackageRepo.java
+++ b/src/main/java/org/apache/sling/distribution/journal/impl/publisher/PackageRepo.java
@@ -94,8 +94,8 @@ public class PackageRepo {
             cntNode.setProperty(Property.JCR_DATA, binary);
             resolver.commit();
             String blobRef = ((ReferenceBinary) binary).getReference();
-            LOG.info(String.format("Stored content package %s under path %s with blobRef %s",
-                    disPkg.getId(), pkgPath, blobRef));
+            LOG.info("Stored content package {} under path {} with blobRef {}",
+                    disPkg.getId(), pkgPath, blobRef);
             return blobRef;
         } catch (Exception e) {
             throw new DistributionException(e.getMessage(), e);
@@ -142,7 +142,7 @@ public class PackageRepo {
 
     private void cleanup(ResourceResolver resolver, long headOffset, long tailOffset)
             throws PersistenceException {
-        LOG.info(String.format("Cleanup headOffset %s tailOffset %s", headOffset, tailOffset));
+        LOG.info("Cleanup headOffset {} tailOffset {}", headOffset, tailOffset);
         Resource root = getRoot(resolver);
         int removedCount = 0;
         for (Resource type : root.getChildren()) {
diff --git a/src/main/java/org/apache/sling/distribution/journal/impl/shared/DefaultDistributionLog.java b/src/main/java/org/apache/sling/distribution/journal/impl/shared/DefaultDistributionLog.java
index d8894c2..bb49514 100644
--- a/src/main/java/org/apache/sling/distribution/journal/impl/shared/DefaultDistributionLog.java
+++ b/src/main/java/org/apache/sling/distribution/journal/impl/shared/DefaultDistributionLog.java
@@ -41,9 +41,11 @@ public class DefaultDistributionLog implements DistributionLog {
     private final LinkedList<String> lines = new LinkedList<>();
     private final Logger logger;
     private final LogLevel logLevel;
+    
+    private final DateFormat dateFormat = new SimpleDateFormat("yyyy/MM/dd HH:mm:ss:SSS");
+    private final Calendar cal = Calendar.getInstance();
 
     public DefaultDistributionLog(String name, Class<?> clazz, LogLevel logLevel) {
-
         this.name = name;
         this.logLevel = logLevel;
         this.logger = LoggerFactory.getLogger(clazz);
@@ -63,7 +65,7 @@ public class DefaultDistributionLog implements DistributionLog {
         try {
             FormattingTuple fmtp = MessageFormatter.arrayFormat(fmt, objects);
             internalLog(level, fmtp.getMessage());
-        } catch (Throwable e) {
+        } catch (Exception e) {
             logger.error("cannot add entry log", e);
         }
     }
@@ -72,16 +74,15 @@ public class DefaultDistributionLog implements DistributionLog {
         if (level.cardinal < logLevel.cardinal) {
             return;
         }
-
-        DateFormat dateFormat = new SimpleDateFormat("yyyy/MM/dd HH:mm:ss:SSS");
-        Calendar cal = Calendar.getInstance();
-
-
         String log = dateFormat.format(cal.getTime()) +
                 " - " +
                 level.name() +
                 " - " +
                 message;
+        addLine(log);
+    }
+
+    private void addLine(String log) {
         synchronized (lines) {
             lines.add(log);
             int maxLines = 1000;
diff --git a/src/main/java/org/apache/sling/distribution/journal/impl/subscriber/DistributionSubscriber.java b/src/main/java/org/apache/sling/distribution/journal/impl/subscriber/DistributionSubscriber.java
index 6f6d676..1b91fcb 100644
--- a/src/main/java/org/apache/sling/distribution/journal/impl/subscriber/DistributionSubscriber.java
+++ b/src/main/java/org/apache/sling/distribution/journal/impl/subscriber/DistributionSubscriber.java
@@ -58,6 +58,7 @@ import org.apache.sling.distribution.DistributionRequestType;
 import org.apache.sling.distribution.DistributionResponse;
 import org.apache.sling.distribution.agent.DistributionAgentState;
 import org.apache.sling.distribution.agent.spi.DistributionAgent;
+import org.apache.sling.distribution.common.DistributionException;
 import org.apache.sling.distribution.journal.JournalAvailable;
 import org.apache.sling.distribution.journal.MessageInfo;
 import org.apache.sling.distribution.journal.MessageSender;
@@ -316,11 +317,11 @@ public class DistributionSubscriber implements DistributionAgent {
 
     private boolean shouldEnqueue(PackageMessage message) {
         if (!queueNames.contains(message.getPubAgentName())) {
-            LOG.info(String.format("Skipping package for Publisher agent %s (not subscribed)", message.getPubAgentName()));
+            LOG.info("Skipping package for Publisher agent {} (not subscribed)", message.getPubAgentName());
             return false;
         }
         if (!pkgType.equals(message.getPkgType())) {
-            LOG.warn(String.format("Skipping package with type %s", message.getPkgType()));
+            LOG.warn("Skipping package with type {}", message.getPkgType());
             return false;
         }
         return true;
@@ -379,9 +380,9 @@ public class DistributionSubscriber implements DistributionAgent {
             Thread.sleep(RETRY_DELAY);
         } catch (InterruptedException e) {
             throw e;
-        } catch (Throwable t) {
+        } catch (Exception e) {
             // Catch all to prevent processing from stopping
-            LOG.error("Error processing queue item", t);
+            LOG.error("Error processing queue item", e);
             Thread.sleep(RETRY_DELAY);
         }
     }
@@ -397,7 +398,7 @@ public class DistributionSubscriber implements DistributionAgent {
         }
     }
 
-    private void processQueueItem(DistributionQueueItem queueItem) throws Exception {
+    private void processQueueItem(DistributionQueueItem queueItem) throws PersistenceException, LoginException, DistributionException {
         long offset = queueItem.get(RECORD_OFFSET, Long.class);
         PackageMessage pkgMsg = queueItem.get(PACKAGE_MSG, PackageMessage.class);
         boolean skip = shouldSkip(offset);
diff --git a/src/main/java/org/apache/sling/distribution/journal/impl/subscriber/PackageHandler.java b/src/main/java/org/apache/sling/distribution/journal/impl/subscriber/PackageHandler.java
index ae355c9..a7148c4 100644
--- a/src/main/java/org/apache/sling/distribution/journal/impl/subscriber/PackageHandler.java
+++ b/src/main/java/org/apache/sling/distribution/journal/impl/subscriber/PackageHandler.java
@@ -77,7 +77,7 @@ public class PackageHandler {
 
     private void installDeletePackage(ResourceResolver resolver, PackageMessage pkgMsg)
             throws PersistenceException {
-        LOG.info("Deleting paths " + pkgMsg.getPathsList());
+        LOG.info("Deleting paths {}",pkgMsg.getPathsList());
         for (String path : pkgMsg.getPathsList()) {
             Resource resource = resolver.getResource(path);
             if (resource != null) {