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/23 10:04:59 UTC
[sling-org-apache-sling-distribution-journal] branch master
updated: SLING-9593 - Do not use ResourceResolver and DistributionPackage
in PackageRepo (#55)
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 136128c SLING-9593 - Do not use ResourceResolver and DistributionPackage in PackageRepo (#55)
136128c is described below
commit 136128cfdbe203da335636d4f5c09d1fb4e02c7d
Author: Christian Schneider <ch...@die-schneider.net>
AuthorDate: Thu Jul 23 12:04:53 2020 +0200
SLING-9593 - Do not use ResourceResolver and DistributionPackage in PackageRepo (#55)
* SLING-9593 - Do not use ResourceResolver and DistributionPackage in PackageRepo
* SLING-9593 - Change root path to avoid clashes
* SLING-9593 - Check for null
---
.../journal/impl/publisher/PackageCleaner.java | 9 ++----
.../impl/publisher/PackageMessageFactory.java | 10 +++++--
.../journal/impl/publisher/PackageRepo.java | 27 ++++++++++--------
.../journal/impl/publisher/PackageRepoTest.java | 33 +++++-----------------
4 files changed, 32 insertions(+), 47 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..6f2791b 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,9 @@ import org.slf4j.LoggerFactory;
import static java.util.Collections.singletonMap;
import static org.apache.sling.api.resource.ResourceResolverFactory.SUBSERVICE;
+import java.io.InputStream;
+import java.util.Objects;
+
/**
* 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.
@@ -66,26 +68,23 @@ public class PackageRepo {
private DistributionMetricsService distributionMetricsService;
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
-
+ static final String PACKAGES_ROOT_PATH = "/var/sling/distribution/journal/packagebinaries";
@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 pkgNode = Objects.requireNonNull(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 +98,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 +111,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;
- }
-
}