This is an automated email from the ASF dual-hosted git repository.
diru 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 76c3034 SLING-12784: call DistributionPackageBuilder for DELETE (#173)
76c3034 is described below
commit 76c30345bea2b78579abec2a22c347ee635592ee
Author: Dirk Rudolph <[email protected]>
AuthorDate: Mon May 19 10:11:52 2025 +0200
SLING-12784: call DistributionPackageBuilder for DELETE (#173)
* feat: call packageBuilder for delete operations as well
* chore: restore journal-messages release version
* feat: allow package builders to return null
* fix: sonar issues
* fix: sonar issues
* update deps, fix potential NPE
---
.../impl/publisher/DistributionPublisher.java | 9 ++++
.../impl/publisher/PackageMessageFactory.java | 63 ++++++++++------------
.../shared/JournalDistributionPackageBuilder.java | 16 +++---
.../publisher/DistributionPackageFactoryTest.java | 40 ++++++++++----
.../impl/publisher/DistributionPublisherTest.java | 10 ++++
.../impl/publisher/PackageMessageFactoryTest.java | 4 +-
6 files changed, 90 insertions(+), 52 deletions(-)
diff --git
a/src/main/java/org/apache/sling/distribution/journal/impl/publisher/DistributionPublisher.java
b/src/main/java/org/apache/sling/distribution/journal/impl/publisher/DistributionPublisher.java
index 7133ae4..5829126 100644
---
a/src/main/java/org/apache/sling/distribution/journal/impl/publisher/DistributionPublisher.java
+++
b/src/main/java/org/apache/sling/distribution/journal/impl/publisher/DistributionPublisher.java
@@ -22,6 +22,7 @@ package org.apache.sling.distribution.journal.impl.publisher;
import static java.lang.String.format;
import static java.util.Objects.requireNonNull;
import static org.apache.sling.distribution.DistributionRequestState.ACCEPTED;
+import static org.apache.sling.distribution.DistributionRequestState.DROPPED;
import static org.apache.sling.distribution.DistributionRequestType.*;
import static
org.apache.sling.distribution.journal.shared.Strings.requireNotBlank;
import static
org.osgi.service.component.annotations.ReferenceCardinality.OPTIONAL;
@@ -33,6 +34,7 @@ import java.util.concurrent.TimeUnit;
import java.util.function.Consumer;
import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
import javax.annotation.ParametersAreNonnullByDefault;
import org.apache.commons.io.IOUtils;
@@ -214,6 +216,12 @@ public class DistributionPublisher implements
DistributionAgent {
int sleepMs = getSleepTime(queueSize);
sleep(sleepMs);
final PackageMessage pkg = buildPackage(resourceResolver, request);
+
+ if (pkg == null) {
+ distLog.debug("Empty request: {}", request);
+ return new SimpleDistributionResponse(DROPPED, "Empty request");
+ }
+
return send(pkg, queueSize, sleepMs);
}
@@ -239,6 +247,7 @@ public class DistributionPublisher implements
DistributionAgent {
}
}
+ @Nullable
private PackageMessage buildPackage(ResourceResolver resourceResolver,
DistributionRequest request)
throws DistributionException {
try {
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 f787f38..209a491 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
@@ -19,7 +19,6 @@
package org.apache.sling.distribution.journal.impl.publisher;
import static java.lang.String.format;
-import static java.util.Objects.requireNonNull;
import static
org.apache.sling.distribution.packaging.DistributionPackageInfo.PROPERTY_REQUEST_DEEP_PATHS;
import java.io.IOException;
@@ -28,6 +27,7 @@ import java.util.List;
import java.util.UUID;
import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
import javax.annotation.ParametersAreNonnullByDefault;
import org.apache.commons.io.IOUtils;
@@ -82,6 +82,7 @@ public class PackageMessageFactory {
LOG.info("Stopped package message factory for pubSlingId={}",
pubSlingId);
}
+ @Nullable
public PackageMessage create(
DistributionPackageBuilder packageBuilder,
ResourceResolver resourceResolver,
@@ -89,46 +90,54 @@ public class PackageMessageFactory {
DistributionRequest request)
throws DistributionException {
switch (request.getRequestType()) {
- case ADD: return createAdd(packageBuilder, resourceResolver,
pubAgentName, request);
- case DELETE: return createDelete(packageBuilder, resourceResolver,
request, pubAgentName);
+ case ADD: return create(ReqType.ADD, packageBuilder,
resourceResolver, pubAgentName, request);
+ case DELETE: return create(ReqType.DELETE, packageBuilder,
resourceResolver, pubAgentName, request);
case INVALIDATE: return createInvalidate(packageBuilder,
resourceResolver, request, pubAgentName);
case TEST: return createTest(packageBuilder, resourceResolver,
request, pubAgentName);
default: throw new IllegalArgumentException(format("Unsupported
request with requestType=%s", request.getRequestType()));
}
}
- @Nonnull
- private PackageMessage createAdd(DistributionPackageBuilder
packageBuilder, ResourceResolver resourceResolver, String pubAgentName,
DistributionRequest request) throws DistributionException {
- final DistributionPackage disPkg =
requireNonNull(packageBuilder.createPackage(resourceResolver, request));
- final byte[] pkgBinary = pkgBinary(disPkg);
- long pkgLength = assertPkgLength(pkgBinary.length);
+ @Nullable
+ private PackageMessage create(ReqType type, DistributionPackageBuilder
packageBuilder, ResourceResolver resourceResolver, String pubAgentName,
DistributionRequest request) throws DistributionException {
+ final DistributionPackage disPkg =
packageBuilder.createPackage(resourceResolver, request);
+
+ if (disPkg == null) {
+ return null;
+ }
+
+ long pkgLength = assertPkgLength(disPkg.getSize());
final DistributionPackageInfo pkgInfo = disPkg.getInfo();
final List<String> paths = Arrays.asList(pkgInfo.getPaths());
- final List<String> deepPaths =
Arrays.asList(pkgInfo.get(PROPERTY_REQUEST_DEEP_PATHS, String[].class));
+ final List<String> deepPaths =
Arrays.asList(pkgInfo.get(PROPERTY_REQUEST_DEEP_PATHS, new String[0]));
final String pkgId = disPkg.getId();
PackageMessageBuilder pkgBuilder = PackageMessage.builder()
.pubSlingId(pubSlingId)
.pkgId(pkgId)
.pubAgentName(pubAgentName)
.paths(paths)
- .reqType(ReqType.ADD)
+ .reqType(type)
.deepPaths(deepPaths)
.pkgLength(pkgLength)
.userId(resourceResolver.getUserID())
.pkgType(packageBuilder.getType());
- String storeRef;
- try {
- storeRef = binaryStore.put(pkgId, disPkg.createInputStream(),
pkgLength);
- } catch (IOException e) {
- throw new DistributionException(e.getMessage(), e);
+ if (pkgLength > 0) {
+ // a delete package may not contain any data
+ String storeRef;
+ try {
+ storeRef = binaryStore.put(pkgId, disPkg.createInputStream(),
pkgLength);
+ } catch (IOException e) {
+ throw new DistributionException(e.getMessage(), e);
+ }
+
+ if (StringUtils.isNotEmpty(storeRef)) {
+ pkgBuilder.pkgBinaryRef(storeRef);
+ } else {
+ pkgBuilder.pkgBinary(pkgBinary(disPkg));
+ }
}
- if (StringUtils.isNotEmpty(storeRef)) {
- pkgBuilder.pkgBinaryRef(storeRef);
- } else {
- pkgBuilder.pkgBinary(pkgBinary);
- }
PackageMessage pipePackage = pkgBuilder.build();
disPkg.delete();
@@ -149,20 +158,6 @@ public class PackageMessageFactory {
.build();
}
- @Nonnull
- private PackageMessage createDelete(DistributionPackageBuilder
packageBuilder, ResourceResolver resourceResolver, DistributionRequest request,
String pubAgentName) {
- String pkgId = UUID.randomUUID().toString();
- return PackageMessage.builder()
- .pubSlingId(pubSlingId)
- .pkgId(pkgId)
- .pubAgentName(pubAgentName)
- .paths(Arrays.asList(request.getPaths()))
- .reqType(ReqType.DELETE)
- .pkgType(packageBuilder.getType())
- .userId(resourceResolver.getUserID())
- .build();
- }
-
@Nonnull
public PackageMessage createTest(DistributionPackageBuilder
packageBuilder, ResourceResolver resourceResolver, DistributionRequest request,
String pubAgentName) {
String pkgId = UUID.randomUUID().toString();
diff --git
a/src/main/java/org/apache/sling/distribution/journal/shared/JournalDistributionPackageBuilder.java
b/src/main/java/org/apache/sling/distribution/journal/shared/JournalDistributionPackageBuilder.java
index b9c0c03..518f1ed 100644
---
a/src/main/java/org/apache/sling/distribution/journal/shared/JournalDistributionPackageBuilder.java
+++
b/src/main/java/org/apache/sling/distribution/journal/shared/JournalDistributionPackageBuilder.java
@@ -108,12 +108,16 @@ public class JournalDistributionPackageBuilder implements
DistributionPackageBui
String packageId = format("dstrpck-%s-%s", currentTimeMillis(),
randomUUID());
final byte[] data;
- try (ByteArrayOutputStream outputStream = new ByteArrayOutputStream())
{
- DistributionExportOptions distributionExportOptions = new
DistributionExportOptions(distributionRequest, null /* Filters set on the
serializer */);
- contentSerializer.exportToStream(resourceResolver,
distributionExportOptions, outputStream);
- data = outputStream.toByteArray();
- } catch (IOException e) {
- throw new DistributionException("Failed to create package for
paths: " + Arrays.toString(distributionRequest.getPaths()), e);
+ if (distributionRequest.getRequestType() ==
DistributionRequestType.ADD) {
+ try (ByteArrayOutputStream outputStream = new
ByteArrayOutputStream()) {
+ DistributionExportOptions distributionExportOptions = new
DistributionExportOptions(distributionRequest, null /* Filters set on the
serializer */);
+ contentSerializer.exportToStream(resourceResolver,
distributionExportOptions, outputStream);
+ data = outputStream.toByteArray();
+ } catch (IOException e) {
+ throw new DistributionException("Failed to create package for
paths: " + Arrays.toString(distributionRequest.getPaths()), e);
+ }
+ } else {
+ data = new byte[0];
}
DistributionPackageInfo distributionPackageInfo = new
DistributionPackageInfo(getType());
diff --git
a/src/test/java/org/apache/sling/distribution/journal/impl/publisher/DistributionPackageFactoryTest.java
b/src/test/java/org/apache/sling/distribution/journal/impl/publisher/DistributionPackageFactoryTest.java
index bc675e1..ec6465c 100644
---
a/src/test/java/org/apache/sling/distribution/journal/impl/publisher/DistributionPackageFactoryTest.java
+++
b/src/test/java/org/apache/sling/distribution/journal/impl/publisher/DistributionPackageFactoryTest.java
@@ -24,9 +24,8 @@ import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.notNullValue;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyLong;
-import static org.mockito.ArgumentMatchers.anyString;
+import static org.junit.Assert.assertNull;
+import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.osgi.util.converter.Converters.standardConverter;
@@ -52,7 +51,6 @@ import org.junit.Before;
import org.junit.Test;
import org.mockito.InjectMocks;
import org.mockito.Mock;
-import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
public class DistributionPackageFactoryTest {
@@ -85,6 +83,17 @@ public class DistributionPackageFactoryTest {
when(resourceResolver.getUserID()).thenReturn("testUser");
}
+
+ @Test
+ public void testEmpty() throws DistributionException {
+ DistributionRequest add = new
SimpleDistributionRequest(DistributionRequestType.ADD, "/test");
+ DistributionRequest delete = new
SimpleDistributionRequest(DistributionRequestType.DELETE, "/test");
+
+ when(packageBuilder.createPackage(eq(resourceResolver),
any())).thenReturn(null);
+
+ assertNull(publisher.create(packageBuilder, resourceResolver,
"pub1agent1", add));
+ assertNull(publisher.create(packageBuilder, resourceResolver,
"pub1agent1", delete));
+ }
@Test
public void testAdd() throws DistributionException, IOException {
@@ -93,7 +102,8 @@ public class DistributionPackageFactoryTest {
DistributionPackage pkg = mock(DistributionPackage.class);
when(binaryStore.put(anyString(), any(), anyLong())).thenReturn(null);
- when(pkg.createInputStream()).thenReturn(new ByteArrayInputStream(new
byte[] {}));
+ when(pkg.createInputStream()).thenReturn(new ByteArrayInputStream(new
byte[] { 0x00 }));
+ when(pkg.getSize()).thenReturn(1L);
when(pkg.getId()).thenReturn("myid");
Map<String, Object> props = new HashMap<>();
props.put(DistributionPackageInfo.PROPERTY_REQUEST_PATHS,
request.getPaths());
@@ -101,12 +111,12 @@ public class DistributionPackageFactoryTest {
DistributionPackageInfo info = new DistributionPackageInfo("journal",
props);
when(pkg.getInfo()).thenReturn(info);
- when(packageBuilder.createPackage(Mockito.eq(resourceResolver),
Mockito.eq(request))).thenReturn(pkg);
+ when(packageBuilder.createPackage(resourceResolver,
request)).thenReturn(pkg);
PackageMessage sent = publisher.create(packageBuilder,
resourceResolver, "pub1agent1", request);
assertThat(sent.getPkgBinary(), notNullValue());
- assertThat(sent.getPkgLength(), equalTo(0L));
+ assertThat(sent.getPkgLength(), equalTo(1L));
assertThat(sent.getReqType(), equalTo(ReqType.ADD));
assertThat(sent.getPkgType(), equalTo("journal"));
assertThat(sent.getPaths(), contains("/test"));
@@ -121,14 +131,14 @@ public class DistributionPackageFactoryTest {
when(binaryStore.put(anyString(), any(),
anyLong())).thenReturn("emptyId");
when(pkg.createInputStream()).thenReturn(new ByteArrayInputStream(new
byte[819200]));
+ when(pkg.getSize()).thenReturn(819200L);
when(pkg.getId()).thenReturn("myid");
Map<String, Object> props = new HashMap<>();
props.put(DistributionPackageInfo.PROPERTY_REQUEST_PATHS,
request.getPaths());
props.put(DistributionPackageInfo.PROPERTY_REQUEST_DEEP_PATHS,
"/test2");
- DistributionPackageInfo info = new DistributionPackageInfo("journal",
- props);
+ DistributionPackageInfo info = new DistributionPackageInfo("journal",
props);
when(pkg.getInfo()).thenReturn(info);
- when(packageBuilder.createPackage(Mockito.eq(resourceResolver),
Mockito.eq(request))).thenReturn(pkg);
+ when(packageBuilder.createPackage(resourceResolver,
request)).thenReturn(pkg);
PackageMessage sent = publisher.create(packageBuilder,
resourceResolver, "pub1agent1", request);
@@ -144,6 +154,16 @@ public class DistributionPackageFactoryTest {
public void testDelete() throws DistributionException, IOException {
DistributionRequest request = new
SimpleDistributionRequest(DistributionRequestType.DELETE, "/test");
+ DistributionPackage pkg = mock(DistributionPackage.class);
+ when(pkg.getSize()).thenReturn(0L);
+ when(pkg.getId()).thenReturn("myid");
+ Map<String, Object> props = new HashMap<>();
+ props.put(DistributionPackageInfo.PROPERTY_REQUEST_PATHS,
request.getPaths());
+ props.put(DistributionPackageInfo.PROPERTY_REQUEST_DEEP_PATHS,
"/test");
+ DistributionPackageInfo info = new DistributionPackageInfo("journal",
props);
+ when(pkg.getInfo()).thenReturn(info);
+ when(packageBuilder.createPackage(resourceResolver,
request)).thenReturn(pkg);
+
PackageMessage sent = publisher.create(packageBuilder,
resourceResolver, "pub1agent1", request);
assertThat(sent.getReqType(), equalTo(ReqType.DELETE));
assertThat(sent.getPkgBinary(), nullValue());
diff --git
a/src/test/java/org/apache/sling/distribution/journal/impl/publisher/DistributionPublisherTest.java
b/src/test/java/org/apache/sling/distribution/journal/impl/publisher/DistributionPublisherTest.java
index fe61a07..200d564 100644
---
a/src/test/java/org/apache/sling/distribution/journal/impl/publisher/DistributionPublisherTest.java
+++
b/src/test/java/org/apache/sling/distribution/journal/impl/publisher/DistributionPublisherTest.java
@@ -271,6 +271,16 @@ public class DistributionPublisherTest {
publisher.execute(resourceResolver, request);
}
+ @Test
+ public void testEmptyRequest() throws DistributionException {
+ DistributionRequest request = new
SimpleDistributionRequest(DistributionRequestType.ADD, new String[] { "/" });
+ when(factory.create(any(), any(), anyString(),
any())).thenReturn(null);
+
+ DistributionResponse response = publisher.execute(resourceResolver,
request);
+ assertEquals(DistributionRequestState.DROPPED, response.getState());
+ assertEquals("Empty request", response.getMessage());
+ }
+
private long distribute(DistributionRequest request) throws IOException,
DistributionException {
StopWatch stopwatch = new StopWatch();
stopwatch.start();
diff --git
a/src/test/java/org/apache/sling/distribution/journal/impl/publisher/PackageMessageFactoryTest.java
b/src/test/java/org/apache/sling/distribution/journal/impl/publisher/PackageMessageFactoryTest.java
index 9b86b5b..f7b54ac 100644
---
a/src/test/java/org/apache/sling/distribution/journal/impl/publisher/PackageMessageFactoryTest.java
+++
b/src/test/java/org/apache/sling/distribution/journal/impl/publisher/PackageMessageFactoryTest.java
@@ -89,8 +89,7 @@ public class PackageMessageFactoryTest {
.convert(singletonMap("maxPackageSize", 1000))
.to(PackageFactoryConfiguration.class);
factory.activate(config);
- when(distributionPackage.createInputStream())
- .thenReturn(new ByteArrayInputStream(new
byte[(int)config.maxPackageSize() + 1]));
+ when(distributionPackage.getSize()).thenReturn(config.maxPackageSize()
+ 1);
DistributionRequest request = new SimpleDistributionRequest(ADD,
"/some/path");
factory.create(packageBuilder,
resolverFactory.getServiceResourceResolver(null), PUB_AGENT_NAME, request);
}
@@ -103,6 +102,7 @@ public class PackageMessageFactoryTest {
factory.activate(config);
when(distributionPackage.createInputStream())
.thenReturn(new ByteArrayInputStream(new byte[10_000_000]));
+ when(distributionPackage.getSize()).thenReturn(10_000_000L);
DistributionRequest request = new SimpleDistributionRequest(ADD,
"/some/path");
assertNotNull(factory.create(packageBuilder,
resolverFactory.getServiceResourceResolver(null), PUB_AGENT_NAME, request));
}
