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 2020/07/21 14:12:05 UTC

[sling-org-apache-sling-distribution-journal] branch SLING-9593-repo created (now cc41668)

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

cschneider pushed a change to branch SLING-9593-repo
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-distribution-journal.git.


      at cc41668  SLING-9593 - Do not use ResourceResolver and DistributionPackage in PackageRepo

This branch includes the following new commits:

     new cc41668  SLING-9593 - Do not use ResourceResolver and DistributionPackage in PackageRepo

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[sling-org-apache-sling-distribution-journal] 01/01: SLING-9593 - Do not use ResourceResolver and DistributionPackage in PackageRepo

Posted by cs...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit cc416686b7667bd884f3478cec25357d92049f5a
Author: Christian Schneider <cs...@adobe.com>
AuthorDate: Tue Jul 21 16:11:47 2020 +0200

    SLING-9593 - Do not use ResourceResolver and DistributionPackage in PackageRepo
---
 .../journal/impl/publisher/PackageCleaner.java     |  9 ++----
 .../impl/publisher/PackageMessageFactory.java      | 10 +++++--
 .../journal/impl/publisher/PackageRepo.java        | 22 ++++++++-------
 .../journal/impl/publisher/PackageRepoTest.java    | 33 +++++-----------------
 4 files changed, 29 insertions(+), 45 deletions(-)

diff --git a/src/main/java/org/apache/sling/distribution/journal/impl/publisher/PackageCleaner.java b/src/main/java/org/apache/sling/distribution/journal/impl/publisher/PackageCleaner.java
index 9326e3b..d272904 100644
--- a/src/main/java/org/apache/sling/distribution/journal/impl/publisher/PackageCleaner.java
+++ b/src/main/java/org/apache/sling/distribution/journal/impl/publisher/PackageCleaner.java
@@ -43,13 +43,8 @@ public class PackageCleaner {
     public int cleanup(Resource root)
             throws PersistenceException {
         int removedCount = 0;
-        for (Resource type : root.getChildren()) {
-            Resource data = type.getChild("data");
-            if (data != null) {
-                for (Resource pkgNode : data.getChildren()) {
-                    removedCount += cleanNode(pkgNode);
-                }
-            }
+        for (Resource pkgNode : root.getChildren()) {
+            removedCount += cleanNode(pkgNode);
         }
         if (resolver.hasChanges()) {
             resolver.commit();
diff --git a/src/main/java/org/apache/sling/distribution/journal/impl/publisher/PackageMessageFactory.java b/src/main/java/org/apache/sling/distribution/journal/impl/publisher/PackageMessageFactory.java
index 28bf70a..5072640 100644
--- a/src/main/java/org/apache/sling/distribution/journal/impl/publisher/PackageMessageFactory.java
+++ b/src/main/java/org/apache/sling/distribution/journal/impl/publisher/PackageMessageFactory.java
@@ -22,6 +22,7 @@ import static java.util.Objects.requireNonNull;
 import static org.apache.sling.distribution.packaging.DistributionPackageInfo.PROPERTY_REQUEST_DEEP_PATHS;
 
 import java.io.IOException;
+import java.io.InputStream;
 import java.util.Arrays;
 import java.util.List;
 import java.util.UUID;
@@ -127,9 +128,14 @@ public class PackageMessageFactory {
              * size issues when sending messages, which is not
              * always the case.
              */
-
+            InputStream binaryStream;
+            try {
+                binaryStream = disPkg.createInputStream();
+            } catch (IOException e) {
+                throw new DistributionException("Error creating stream for package " + disPkg.getId(), e);
+            }
             LOG.info("Package {} too large ({}B) to be sent inline", disPkg.getId(), pkgLength);
-            String pkgBinRef = packageRepo.store(resourceResolver, disPkg);
+            String pkgBinRef = packageRepo.store(disPkg.getId(), binaryStream);
             pkgBuilder.pkgBinaryRef(pkgBinRef);
         } else {
             pkgBuilder.pkgBinary(pkgBinary);
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 8e192a9..9e03f58 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
@@ -36,7 +36,6 @@ import org.apache.sling.api.resource.ResourceUtil;
 import org.apache.sling.commons.metrics.Timer;
 import org.apache.sling.distribution.common.DistributionException;
 import org.apache.sling.distribution.journal.shared.DistributionMetricsService;
-import org.apache.sling.distribution.packaging.DistributionPackage;
 import org.apache.sling.serviceusermapping.ServiceUserMapped;
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Reference;
@@ -46,6 +45,8 @@ import org.slf4j.LoggerFactory;
 import static java.util.Collections.singletonMap;
 import static org.apache.sling.api.resource.ResourceResolverFactory.SUBSERVICE;
 
+import java.io.InputStream;
+
 /**
  * Manages the binary content of DistributionPackages. If they are too big to fit in a journal message then they
  * are written to the blob store. It also offers cleanup functionality to remove the data when it is not needed anymore.
@@ -67,25 +68,22 @@ public class PackageRepo {
 
     private static final Logger LOG = LoggerFactory.getLogger(PackageRepo.class);
     static final String PACKAGES_ROOT_PATH = "/var/sling/distribution/journal/packages";
-    private static final String PACKAGE_PATH_PATTERN = PACKAGES_ROOT_PATH + "/%s/data/%s"; // packageType x packageId
-
 
     @Nonnull
-    public String store(ResourceResolver resolver, DistributionPackage disPkg)
-            throws DistributionException {
-        try {
-            String pkgPath = String.format(PACKAGE_PATH_PATTERN, disPkg.getType(), disPkg.getId());
+    public String store(String id, InputStream binaryStream)throws DistributionException {
+        try (ResourceResolver resolver = createResourceResolver()) {
+            String pkgPath = PACKAGES_ROOT_PATH + "/" + id;
             Resource pkgResource = ResourceUtil.getOrCreateResource(resolver,
                     pkgPath, SLING_FOLDER, SLING_FOLDER, false);
             Node pkgNode = pkgResource.adaptTo(Node.class);
             Node binNode = JcrUtils.getOrAddNode(pkgNode, "bin", NodeType.NT_FILE);
             Node cntNode = JcrUtils.getOrAddNode(binNode, Node.JCR_CONTENT, NodeType.NT_RESOURCE);
-            Binary binary = pkgNode.getSession().getValueFactory().createBinary(disPkg.createInputStream());
+            Binary binary = pkgNode.getSession().getValueFactory().createBinary(binaryStream);
             cntNode.setProperty(Property.JCR_DATA, binary);
             resolver.commit();
             String blobRef = ((ReferenceBinary) binary).getReference();
             LOG.info("Stored content package {} under path {} with blobRef {}",
-                    disPkg.getId(), pkgPath, blobRef);
+                    id, pkgPath, blobRef);
             return blobRef;
         } catch (Exception e) {
             throw new DistributionException(e.getMessage(), e);
@@ -99,7 +97,7 @@ public class PackageRepo {
     public void cleanup(long deleteOlderThanTime) {
         Timer.Context context = distributionMetricsService.getCleanupPackageDuration().time();
         // Auto-refresh policy is disabled for service resource resolver
-        try (ResourceResolver resolver = resolverFactory.getServiceResourceResolver(singletonMap(SUBSERVICE, "bookkeeper"))) {
+        try (ResourceResolver resolver = createResourceResolver()) {
             
             PackageCleaner packageCleaner = new PackageCleaner(resolver, deleteOlderThanTime);
             Resource root = getRoot(resolver);
@@ -112,6 +110,10 @@ public class PackageRepo {
         }
     }
 
+    private ResourceResolver createResourceResolver() throws LoginException {
+        return resolverFactory.getServiceResourceResolver(singletonMap(SUBSERVICE, "bookkeeper"));
+    }
+
     @Nonnull
     private Resource getRoot(ResourceResolver resolver)
             throws PersistenceException {
diff --git a/src/test/java/org/apache/sling/distribution/journal/impl/publisher/PackageRepoTest.java b/src/test/java/org/apache/sling/distribution/journal/impl/publisher/PackageRepoTest.java
index 975ebbe..c6200c8 100644
--- a/src/test/java/org/apache/sling/distribution/journal/impl/publisher/PackageRepoTest.java
+++ b/src/test/java/org/apache/sling/distribution/journal/impl/publisher/PackageRepoTest.java
@@ -20,11 +20,11 @@ package org.apache.sling.distribution.journal.impl.publisher;
 
 import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertThat;
-import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.UUID;
@@ -40,7 +40,6 @@ import org.apache.sling.distribution.journal.MessagingProvider;
 import org.apache.sling.distribution.journal.messages.PackageMessage;
 import org.apache.sling.distribution.journal.shared.DistributionMetricsService;
 import org.apache.sling.distribution.journal.shared.Topics;
-import org.apache.sling.distribution.packaging.DistributionPackage;
 import org.apache.sling.testing.mock.osgi.MockOsgi;
 import org.apache.sling.testing.mock.sling.MockSling;
 import org.apache.sling.testing.mock.sling.ResourceResolverType;
@@ -107,7 +106,10 @@ public class PackageRepoTest {
                 .thenReturn(counter);
 
         long createTime = System.currentTimeMillis();
-        store(mockPackage());
+        String id = UUID.randomUUID().toString();
+        byte[] content = new byte[] {};
+        InputStream binaryStream = new ByteArrayInputStream(content);
+        packageRepo.store(id, binaryStream);
         assertNumNodes(1);
         packageRepo.cleanup(createTime - 1000);
         assertNumNodes(1);
@@ -115,12 +117,6 @@ public class PackageRepoTest {
         assertNumNodes(0);
     }
 
-    private void store(DistributionPackage pkg) throws DistributionException, IOException, LoginException {
-        try (ResourceResolver resolver = resolverFactory.getServiceResourceResolver(null)) {
-            packageRepo.store(resolver, pkg);
-        }
-    }
-
     private void assertNumNodes(int num) throws LoginException {
         try (ResourceResolver resolver = resolverFactory.getServiceResourceResolver(null)) {
             assertThat(getPackageNodes(resolver).size(), equalTo(num));
@@ -130,25 +126,10 @@ public class PackageRepoTest {
     private List<Resource> getPackageNodes(ResourceResolver resolver) throws LoginException {
         List<Resource> result = new ArrayList<>();
         Resource root = resolver.getResource(PackageRepo.PACKAGES_ROOT_PATH);
-        for (Resource type : root.getChildren()) {
-            Resource data = type.getChild("data");
-            if (data != null) {
-                for (Resource pkg : data.getChildren()) {
-                    result.add(pkg);
-                }
-            }
+        for (Resource pkg : root.getChildren()) {
+            result.add(pkg);
         }
         return result;
     }
     
-    private DistributionPackage mockPackage() throws IOException {
-        DistributionPackage pkg = mock(DistributionPackage.class);
-        when(pkg.getId()).thenReturn(UUID.randomUUID().toString());
-        when(pkg.getType()).thenReturn("journal");
-        byte[] content = new byte[] {};
-        ByteArrayInputStream stream = new ByteArrayInputStream(content);
-        when(pkg.createInputStream()).thenReturn(stream);
-        return pkg;
-    }
-    
 }