JAMES-3763 Restores test on deduplicated hashes being encoded in a safe way for all implementations
This is especially important for the S3 implementation which requires blobid
to be valid url path values.
Co-Authored-By: Matthieu Baechler <matthieu@apache.org>
diff --git a/server/blob/blob-api/src/test/java/org/apache/james/blob/api/DeduplicationBlobStoreContract.java b/server/blob/blob-api/src/test/java/org/apache/james/blob/api/DeduplicationBlobStoreContract.java
index a133833..07434b5 100644
--- a/server/blob/blob-api/src/test/java/org/apache/james/blob/api/DeduplicationBlobStoreContract.java
+++ b/server/blob/blob-api/src/test/java/org/apache/james/blob/api/DeduplicationBlobStoreContract.java
@@ -24,10 +24,13 @@
import static org.apache.james.blob.api.BlobStore.StoragePolicy.SIZE_BASED;
import static org.apache.james.blob.api.BlobStoreContract.SHORT_BYTEARRAY;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
import java.io.ByteArrayInputStream;
import java.util.stream.Stream;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
@@ -48,6 +51,22 @@
BlobId.Factory blobIdFactory();
+ BlobStore createBlobStore();
+
+ @BeforeEach
+ default void beforeEach() {
+ System.clearProperty("james.blob.id.hash.encoding");
+ }
+
+ @Test
+ default void deduplicationBlobstoreCreationShouldFailOnInvalidProperty() {
+ System.setProperty("james.blob.id.hash.encoding", "invalid");
+
+ assertThatThrownBy(this::createBlobStore)
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Unknown encoding type: invalid");
+ }
+
@ParameterizedTest
@MethodSource("storagePolicies")
default void saveShouldReturnBlobIdOfString(BlobStore.StoragePolicy storagePolicy) {
diff --git a/server/blob/blob-cassandra/src/test/java/org/apache/james/blob/cassandra/CassandraBlobStoreClOneTest.java b/server/blob/blob-cassandra/src/test/java/org/apache/james/blob/cassandra/CassandraBlobStoreClOneTest.java
index 772529a..fe49d43 100644
--- a/server/blob/blob-cassandra/src/test/java/org/apache/james/blob/cassandra/CassandraBlobStoreClOneTest.java
+++ b/server/blob/blob-cassandra/src/test/java/org/apache/james/blob/cassandra/CassandraBlobStoreClOneTest.java
@@ -53,12 +53,13 @@
import reactor.core.publisher.Mono;
-class CassandraBlobStoreClOneTest implements CassandraBlobStoreContract {
+class CassandraBlobStoreClOneTest implements CassandraBlobStoreContract, DeduplicationBlobStoreContract {
@RegisterExtension
static CassandraClusterExtension cassandraCluster = new CassandraClusterExtension(CassandraBlobModule.MODULE);
private BlobStore testee;
private CassandraDefaultBucketDAO defaultBucketDAO;
+ private CassandraCluster cassandra;
@BeforeEach
void setUp(CassandraCluster cassandra) {
@@ -72,17 +73,17 @@
CassandraBucketDAO bucketDAO = new CassandraBucketDAO(blobIdFactory, this.cassandra.getConf());
defaultBucketDAO = spy(new CassandraDefaultBucketDAO(this.cassandra.getConf(), blobIdFactory));
CassandraConfiguration cassandraConfiguration = CassandraConfiguration.builder()
- .blobPartSize(CHUNK_SIZE)
- .optimisticConsistencyLevel(true)
- .build();
+ .blobPartSize(CHUNK_SIZE)
+ .optimisticConsistencyLevel(true)
+ .build();
MetricFactory metricFactory = metricsTestExtension.getMetricFactory();
- testee = new MetricableBlobStore(
- metricFactory,
- BlobStoreFactory.builder()
- .blobStoreDAO(new CassandraBlobStoreDAO(defaultBucketDAO, bucketDAO, cassandraConfiguration, BucketName.DEFAULT, metricFactory))
- .blobIdFactory(blobIdFactory)
- .defaultBucketName()
- .deduplication());
+ return new MetricableBlobStore(
+ metricFactory,
+ BlobStoreFactory.builder()
+ .blobStoreDAO(new CassandraBlobStoreDAO(defaultBucketDAO, bucketDAO, cassandraConfiguration, BucketName.DEFAULT, metricFactory))
+ .blobIdFactory(blobIdFactory)
+ .defaultBucketName()
+ .deduplication());
}
@Override
diff --git a/server/blob/blob-cassandra/src/test/java/org/apache/james/blob/cassandra/CassandraBlobStoreTest.java b/server/blob/blob-cassandra/src/test/java/org/apache/james/blob/cassandra/CassandraBlobStoreTest.java
index 2ca4489..c49bfa9 100644
--- a/server/blob/blob-cassandra/src/test/java/org/apache/james/blob/cassandra/CassandraBlobStoreTest.java
+++ b/server/blob/blob-cassandra/src/test/java/org/apache/james/blob/cassandra/CassandraBlobStoreTest.java
@@ -41,6 +41,7 @@
private BlobStore testee;
private CassandraDefaultBucketDAO defaultBucketDAO;
+ private CassandraCluster cassandra;
@BeforeEach
void setUp(CassandraCluster cassandra) {
@@ -54,16 +55,16 @@
CassandraBucketDAO bucketDAO = new CassandraBucketDAO(blobIdFactory, this.cassandra.getConf());
defaultBucketDAO = spy(new CassandraDefaultBucketDAO(this.cassandra.getConf(), blobIdFactory));
CassandraConfiguration cassandraConfiguration = CassandraConfiguration.builder()
- .blobPartSize(CHUNK_SIZE)
- .build();
+ .blobPartSize(CHUNK_SIZE)
+ .build();
MetricFactory metricFactory = metricsTestExtension.getMetricFactory();
- testee = new MetricableBlobStore(
- metricFactory,
- BlobStoreFactory.builder()
- .blobStoreDAO(new CassandraBlobStoreDAO(defaultBucketDAO, bucketDAO, cassandraConfiguration, BucketName.DEFAULT, metricFactory))
- .blobIdFactory(blobIdFactory)
- .defaultBucketName()
- .deduplication());
+ return new MetricableBlobStore(
+ metricFactory,
+ BlobStoreFactory.builder()
+ .blobStoreDAO(new CassandraBlobStoreDAO(defaultBucketDAO, bucketDAO, cassandraConfiguration, BucketName.DEFAULT, metricFactory))
+ .blobIdFactory(blobIdFactory)
+ .defaultBucketName()
+ .deduplication());
}
@Override
diff --git a/server/blob/blob-file/src/test/java/org/apache/james/blob/file/FileBlobStoreTest.java b/server/blob/blob-file/src/test/java/org/apache/james/blob/file/FileBlobStoreTest.java
index 2854c7b..ca3d30a 100644
--- a/server/blob/blob-file/src/test/java/org/apache/james/blob/file/FileBlobStoreTest.java
+++ b/server/blob/blob-file/src/test/java/org/apache/james/blob/file/FileBlobStoreTest.java
@@ -34,11 +34,16 @@
@BeforeEach
void setUp() {
+ blobStore = createBlobStore();
+ }
+
+ @Override
+ public MetricableBlobStore createBlobStore() {
FileSystemImpl fileSystem = FileSystemImpl.forTesting();
- blobStore = new MetricableBlobStore(metricsTestExtension.getMetricFactory(), new FileBlobStoreFactory(fileSystem).builder()
- .blobIdFactory(BLOB_ID_FACTORY)
- .defaultBucketName()
- .deduplication());
+ return new MetricableBlobStore(metricsTestExtension.getMetricFactory(), new FileBlobStoreFactory(fileSystem).builder()
+ .blobIdFactory(BLOB_ID_FACTORY)
+ .defaultBucketName()
+ .deduplication());
}
@Override
diff --git a/server/blob/blob-memory/src/test/java/org/apache/james/blob/memory/MemoryBlobStoreTest.java b/server/blob/blob-memory/src/test/java/org/apache/james/blob/memory/MemoryBlobStoreTest.java
index f678066..7310f0d 100644
--- a/server/blob/blob-memory/src/test/java/org/apache/james/blob/memory/MemoryBlobStoreTest.java
+++ b/server/blob/blob-memory/src/test/java/org/apache/james/blob/memory/MemoryBlobStoreTest.java
@@ -33,10 +33,15 @@
@BeforeEach
void setUp() {
- blobStore = new MetricableBlobStore(metricsTestExtension.getMetricFactory(), MemoryBlobStoreFactory.builder()
- .blobIdFactory(BLOB_ID_FACTORY)
- .defaultBucketName()
- .deduplication());
+ blobStore = createBlobStore();
+ }
+
+ @Override
+ public MetricableBlobStore createBlobStore() {
+ return new MetricableBlobStore(metricsTestExtension.getMetricFactory(), MemoryBlobStoreFactory.builder()
+ .blobIdFactory(BLOB_ID_FACTORY)
+ .defaultBucketName()
+ .deduplication());
}
@Override
diff --git a/server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3DeDuplicationBlobStoreTest.java b/server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3DeDuplicationBlobStoreTest.java
index 1d4c5d1..728a25d 100644
--- a/server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3DeDuplicationBlobStoreTest.java
+++ b/server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3DeDuplicationBlobStoreTest.java
@@ -19,6 +19,11 @@
package org.apache.james.blob.objectstorage.aws;
+import static org.apache.james.blob.api.BlobStoreDAOFixture.TEST_BUCKET_NAME;
+
+import java.nio.charset.StandardCharsets;
+
+import org.apache.commons.lang3.RandomStringUtils;
import org.apache.james.blob.api.BlobId;
import org.apache.james.blob.api.BlobStore;
import org.apache.james.blob.api.BlobStoreContract;
@@ -27,49 +32,65 @@
import org.apache.james.metrics.api.NoopGaugeRegistry;
import org.apache.james.metrics.tests.RecordingMetricFactory;
import org.apache.james.server.blob.deduplication.BlobStoreFactory;
-import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
+import reactor.core.publisher.Flux;
+
@ExtendWith(DockerAwsS3Extension.class)
-class S3DeDuplicationBlobStoreTest implements BlobStoreContract {
+class S3DeDuplicationBlobStoreTest implements BlobStoreContract, DeduplicationBlobStoreContract {
- private static BlobStore testee;
- private static S3BlobStoreDAO s3BlobStoreDAO;
- private static S3ClientFactory s3ClientFactory;
+ private BlobStore testee;
+ private DockerAwsS3Container dockerAwsS3;
+ private S3BlobStoreDAO s3BlobStoreDAO;
+ private S3ClientFactory s3ClientFactory;
- @BeforeAll
- static void setUpClass(DockerAwsS3Container dockerAwsS3) {
+ @BeforeEach
+ void setUpClass(DockerAwsS3Container dockerAwsS3) {
+ this.dockerAwsS3 = dockerAwsS3;
+ testee = createBlobStore();
+ }
+
+ @Override
+ public BlobStore createBlobStore() {
AwsS3AuthConfiguration authConfiguration = AwsS3AuthConfiguration.builder()
- .endpoint(dockerAwsS3.getEndpoint())
- .accessKeyId(DockerAwsS3Container.ACCESS_KEY_ID)
- .secretKey(DockerAwsS3Container.SECRET_ACCESS_KEY)
- .build();
+ .endpoint(dockerAwsS3.getEndpoint())
+ .accessKeyId(DockerAwsS3Container.ACCESS_KEY_ID)
+ .secretKey(DockerAwsS3Container.SECRET_ACCESS_KEY)
+ .build();
S3BlobStoreConfiguration s3Configuration = S3BlobStoreConfiguration.builder()
- .authConfiguration(authConfiguration)
- .region(dockerAwsS3.dockerAwsS3().region())
- .build();
+ .authConfiguration(authConfiguration)
+ .region(dockerAwsS3.dockerAwsS3().region())
+ .build();
HashBlobId.Factory blobIdFactory = new HashBlobId.Factory();
s3ClientFactory = new S3ClientFactory(s3Configuration, new RecordingMetricFactory(), new NoopGaugeRegistry());
s3BlobStoreDAO = new S3BlobStoreDAO(s3ClientFactory, s3Configuration, blobIdFactory);
- testee = BlobStoreFactory.builder()
- .blobStoreDAO(s3BlobStoreDAO)
- .blobIdFactory(blobIdFactory)
- .defaultBucketName()
- .deduplication();
+ return BlobStoreFactory.builder()
+ .blobStoreDAO(s3BlobStoreDAO)
+ .blobIdFactory(blobIdFactory)
+ .defaultBucketName()
+ .deduplication();
+ }
+
+ @Test
+ void saveShouldNotGenerateBlobIdValuesThatAreNotValidS3ObjectKeys() {
+ Flux.range(0, 1000)
+ .concatMap(i -> {
+ byte[] payload = RandomStringUtils.random(128).getBytes(StandardCharsets.UTF_8);
+ return testee.save(TEST_BUCKET_NAME, payload, BlobStore.StoragePolicy.LOW_COST);
+ })
+ .then()
+ .block();
}
@AfterEach
void tearDown() {
s3BlobStoreDAO.deleteAllBuckets().block();
- }
-
- @AfterAll
- static void tearDownClass() {
s3ClientFactory.close();
}
diff --git a/server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3MinioTest.java b/server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3MinioTest.java
index 62f9f0f..44eafb6 100644
--- a/server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3MinioTest.java
+++ b/server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3MinioTest.java
@@ -29,14 +29,11 @@
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import java.net.URI;
-import java.nio.charset.StandardCharsets;
import java.util.Optional;
-import org.apache.commons.lang3.RandomStringUtils;
import org.apache.james.blob.api.BlobId;
import org.apache.james.blob.api.BlobStoreDAO;
import org.apache.james.blob.api.BlobStoreDAOContract;
-import org.apache.james.blob.api.HashBlobId;
import org.apache.james.blob.api.TestBlobId;
import org.apache.james.metrics.api.NoopGaugeRegistry;
import org.apache.james.metrics.tests.RecordingMetricFactory;
@@ -48,7 +45,6 @@
import org.testcontainers.junit.jupiter.Container;
import org.testcontainers.junit.jupiter.Testcontainers;
-import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import reactor.util.retry.Retry;
import software.amazon.awssdk.services.s3.model.S3Exception;
@@ -120,16 +116,4 @@
Mono.from(testee.save(TEST_BUCKET_NAME, TEST_BLOB_ID, SHORT_BYTEARRAY)).block();
assertThat(Mono.from(testee.readBytes(TEST_BUCKET_NAME, TEST_BLOB_ID)).block()).isEqualTo(SHORT_BYTEARRAY);
}
-
- @Test
- void saveShouldWorkForAllPayloadsWithHash() {
- Flux.range(0, 1000)
- .concatMap(i -> {
- byte[] payload = RandomStringUtils.random(128).getBytes(StandardCharsets.UTF_8);
- BlobId blobId = new HashBlobId.Factory().forPayload(payload);
- return testee.save(TEST_BUCKET_NAME, blobId, payload);
- })
- .then()
- .block();
- }
}
diff --git a/server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3PrefixAndNamespaceTest.java b/server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3PrefixAndNamespaceTest.java
index 0928b64..10d39c2 100644
--- a/server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3PrefixAndNamespaceTest.java
+++ b/server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3PrefixAndNamespaceTest.java
@@ -28,51 +28,53 @@
import org.apache.james.metrics.api.NoopGaugeRegistry;
import org.apache.james.metrics.tests.RecordingMetricFactory;
import org.apache.james.server.blob.deduplication.BlobStoreFactory;
-import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.extension.ExtendWith;
@ExtendWith(DockerAwsS3Extension.class)
-class S3PrefixAndNamespaceTest implements BlobStoreContract {
- private static BlobStore testee;
- private static S3BlobStoreDAO s3BlobStoreDAO;
+class S3PrefixAndNamespaceTest implements BlobStoreContract, DeduplicationBlobStoreContract {
+ private BlobStore testee;
+ private S3BlobStoreDAO s3BlobStoreDAO;
- private static S3ClientFactory s3ClientFactory;
+ private S3ClientFactory s3ClientFactory;
+ private DockerAwsS3Container dockerAwsS3;
- @BeforeAll
- static void setUpClass(DockerAwsS3Container dockerAwsS3) {
+ @BeforeEach
+ void setUpClass(DockerAwsS3Container dockerAwsS3) {
+ this.dockerAwsS3 = dockerAwsS3;
+ this.testee = createBlobStore();
+ }
+
+ @Override
+ public BlobStore createBlobStore() {
AwsS3AuthConfiguration authConfiguration = AwsS3AuthConfiguration.builder()
- .endpoint(dockerAwsS3.getEndpoint())
- .accessKeyId(DockerAwsS3Container.ACCESS_KEY_ID)
- .secretKey(DockerAwsS3Container.SECRET_ACCESS_KEY)
- .build();
+ .endpoint(dockerAwsS3.getEndpoint())
+ .accessKeyId(DockerAwsS3Container.ACCESS_KEY_ID)
+ .secretKey(DockerAwsS3Container.SECRET_ACCESS_KEY)
+ .build();
S3BlobStoreConfiguration s3Configuration = S3BlobStoreConfiguration.builder()
- .authConfiguration(authConfiguration)
- .region(dockerAwsS3.dockerAwsS3().region())
- .defaultBucketName(BucketName.of("namespace"))
- .bucketPrefix("prefix")
- .build();
+ .authConfiguration(authConfiguration)
+ .region(dockerAwsS3.dockerAwsS3().region())
+ .defaultBucketName(BucketName.of("namespace"))
+ .bucketPrefix("prefix")
+ .build();
HashBlobId.Factory blobIdFactory = new HashBlobId.Factory();
s3ClientFactory = new S3ClientFactory(s3Configuration, new RecordingMetricFactory(), new NoopGaugeRegistry());
s3BlobStoreDAO = new S3BlobStoreDAO(s3ClientFactory, s3Configuration, blobIdFactory);
- testee = BlobStoreFactory.builder()
- .blobStoreDAO(s3BlobStoreDAO)
- .blobIdFactory(blobIdFactory)
- .bucket(BucketName.of("namespace"))
- .deduplication();
+ return BlobStoreFactory.builder()
+ .blobStoreDAO(s3BlobStoreDAO)
+ .blobIdFactory(blobIdFactory)
+ .bucket(BucketName.of("namespace"))
+ .deduplication();
}
@AfterEach
void tearDown() {
s3BlobStoreDAO.deleteAllBuckets().block();
- }
-
- @AfterAll
- static void tearDownClass() {
s3ClientFactory.close();
}
