Few clean ups in ESBlobStoreRepositoryIntegTestCase (#46068)

plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStoreRepositoryTests.java

@@ -25,7 +25,6 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.ByteSizeUnit;
 import org.elasticsearch.common.unit.ByteSizeValue;
 import org.elasticsearch.plugins.Plugin;
-import org.elasticsearch.repositories.Repository;
 import org.elasticsearch.repositories.blobstore.ESBlobStoreRepositoryIntegTestCase;
 import org.junit.After;
 
@@ -34,9 +33,6 @@ import java.util.Collections;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 
-import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
-import static org.hamcrest.Matchers.instanceOf;
-
 public class GoogleCloudStorageBlobStoreRepositoryTests extends ESBlobStoreRepositoryIntegTestCase {
 
     private static final String BUCKET = "gcs-repository-test";
@@ -45,28 +41,25 @@ public class GoogleCloudStorageBlobStoreRepositoryTests extends ESBlobStoreRepos
     // all nodes must see the same content
     private static final ConcurrentMap<String, byte[]> blobs = new ConcurrentHashMap<>();
 
+    @Override
+    protected String repositoryType() {
+        return GoogleCloudStorageRepository.TYPE;
+    }
+
+    @Override
+    protected Settings repositorySettings() {
+        return Settings.builder()
+            .put(super.repositorySettings())
+            .put("bucket", BUCKET)
+            .put("base_path", GoogleCloudStorageBlobStoreRepositoryTests.class.getSimpleName())
+            .build();
+    }
+
     @Override
     protected Collection<Class<? extends Plugin>> nodePlugins() {
         return Collections.singletonList(MockGoogleCloudStoragePlugin.class);
     }
 
-    @Override
-    protected void createTestRepository(String name, boolean verify) {
-        assertAcked(client().admin().cluster().preparePutRepository(name)
-                .setType(GoogleCloudStorageRepository.TYPE)
-                .setVerify(verify)
-                .setSettings(Settings.builder()
-                        .put("bucket", BUCKET)
-                        .put("base_path", GoogleCloudStorageBlobStoreRepositoryTests.class.getSimpleName())
-                        .put("compress", randomBoolean())
-                        .put("chunk_size", randomIntBetween(100, 1000), ByteSizeUnit.BYTES)));
-    }
-
-    @Override
-    protected void afterCreationCheck(Repository repository) {
-        assertThat(repository, instanceOf(GoogleCloudStorageRepository.class));
-    }
-
     @After
     public void wipeRepository() {
         blobs.clear();

plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java

@@ -40,9 +40,6 @@ import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 
-import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
-import static org.hamcrest.Matchers.instanceOf;
-
 public class S3BlobStoreRepositoryTests extends ESBlobStoreRepositoryIntegTestCase {
 
     private static final ConcurrentMap<String, byte[]> blobs = new ConcurrentHashMap<>();
@@ -71,21 +68,19 @@ public class S3BlobStoreRepositoryTests extends ESBlobStoreRepositoryIntegTestCa
     }
 
     @Override
-    protected void createTestRepository(final String name, boolean verify) {
+    protected String repositoryType() {
-        assertAcked(client().admin().cluster().preparePutRepository(name)
+        return S3Repository.TYPE;
-            .setType(S3Repository.TYPE)
-            .setVerify(verify)
-            .setSettings(Settings.builder()
-                .put(S3Repository.BUCKET_SETTING.getKey(), bucket)
-                .put(S3Repository.BUFFER_SIZE_SETTING.getKey(), bufferSize)
-                .put(S3Repository.SERVER_SIDE_ENCRYPTION_SETTING.getKey(), serverSideEncryption)
-                .put(S3Repository.CANNED_ACL_SETTING.getKey(), cannedACL)
-                .put(S3Repository.STORAGE_CLASS_SETTING.getKey(), storageClass)));
     }
 
     @Override
-    protected void afterCreationCheck(Repository repository) {
+    protected Settings repositorySettings() {
-        assertThat(repository, instanceOf(S3Repository.class));
+        return Settings.builder()
+            .put(S3Repository.BUCKET_SETTING.getKey(), bucket)
+            .put(S3Repository.BUFFER_SIZE_SETTING.getKey(), bufferSize)
+            .put(S3Repository.SERVER_SIDE_ENCRYPTION_SETTING.getKey(), serverSideEncryption)
+            .put(S3Repository.CANNED_ACL_SETTING.getKey(), cannedACL)
+            .put(S3Repository.STORAGE_CLASS_SETTING.getKey(), storageClass)
+            .build();
     }
 
     @Override

server/src/test/java/org/elasticsearch/repositories/fs/FsBlobStoreRepositoryIT.java

@@ -22,7 +22,6 @@ import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.ByteSizeUnit;
 import org.elasticsearch.core.internal.io.IOUtils;
-import org.elasticsearch.repositories.Repository;
 import org.elasticsearch.repositories.blobstore.ESBlobStoreRepositoryIntegTestCase;
 
 import java.io.IOException;
@@ -37,23 +36,22 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitC
 import static org.hamcrest.Matchers.instanceOf;
 
 public class FsBlobStoreRepositoryIT extends ESBlobStoreRepositoryIntegTestCase {
+
     @Override
-    protected void createTestRepository(String name, boolean verify) {
+    protected String repositoryType() {
-        assertAcked(client().admin().cluster().preparePutRepository(name)
+        return FsRepository.TYPE;
-            .setVerify(verify)
-            .setType("fs").setSettings(Settings.builder()
-                .put("location", randomRepoPath())
-                .put("compress", randomBoolean())
-                .put("chunk_size", randomIntBetween(100, 1000), ByteSizeUnit.BYTES)));
     }
 
     @Override
-    protected void afterCreationCheck(Repository repository) {
+    protected Settings repositorySettings() {
-        assertThat(repository, instanceOf(FsRepository.class));
+        return Settings.builder()
+            .put(super.repositorySettings())
+            .put("location", randomRepoPath())
+            .build();
     }
 
     public void testMissingDirectoriesNotCreatedInReadonlyRepository() throws IOException, ExecutionException, InterruptedException {
-        final String repoName = randomAsciiName();
+        final String repoName = randomName();
         final Path repoPath = randomRepoPath();
 
         logger.info("--> creating repository {} at {}", repoName, repoPath);
@@ -63,13 +61,13 @@ public class FsBlobStoreRepositoryIT extends ESBlobStoreRepositoryIntegTestCase
             .put("compress", randomBoolean())
             .put("chunk_size", randomIntBetween(100, 1000), ByteSizeUnit.BYTES)));
 
-        String indexName = randomAsciiName();
+        final String indexName = randomName();
         int docCount = iterations(10, 1000);
         logger.info("-->  create random index {} with {} records", indexName, docCount);
         addRandomDocuments(indexName, docCount);
         assertHitCount(client().prepareSearch(indexName).setSize(0).get(), docCount);
 
-        final String snapshotName = randomAsciiName();
+        final String snapshotName = randomName();
         logger.info("-->  create snapshot {}:{}", repoName, snapshotName);
         assertSuccessfulSnapshot(client().admin().cluster().prepareCreateSnapshot(repoName, snapshotName)
             .setWaitForCompletion(true).setIndices(indexName));

test/framework/src/main/java/org/elasticsearch/repositories/blobstore/ESBlobStoreRepositoryIntegTestCase.java

@@ -26,9 +26,12 @@ import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotR
 import org.elasticsearch.action.index.IndexRequestBuilder;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.common.blobstore.BlobContainer;
+import org.elasticsearch.common.blobstore.BlobStore;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.unit.ByteSizeUnit;
+import org.elasticsearch.common.unit.ByteSizeValue;
 import org.elasticsearch.repositories.IndexId;
 import org.elasticsearch.repositories.RepositoriesService;
-import org.elasticsearch.repositories.Repository;
 import org.elasticsearch.repositories.RepositoryData;
 import org.elasticsearch.snapshots.SnapshotMissingException;
 import org.elasticsearch.snapshots.SnapshotRestoreException;
@@ -47,42 +50,50 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcke
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThan;
+import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
 
 /**
- * Basic integration tests for blob-based repository validation.
+ * Integration tests for {@link BlobStoreRepository} implementations.
  */
 public abstract class ESBlobStoreRepositoryIntegTestCase extends ESIntegTestCase {
 
-    protected abstract void createTestRepository(String name, boolean verify);
+    protected abstract String repositoryType();
-
-    protected void afterCreationCheck(Repository repository) {
 
+    protected Settings repositorySettings() {
+        final Settings.Builder settings = Settings.builder();
+        settings.put("compress", randomBoolean());
+        if (randomBoolean()) {
+            long size = 1 << randomIntBetween(7, 10);
+            settings.put("chunk_size", new ByteSizeValue(size, randomFrom(ByteSizeUnit.BYTES, ByteSizeUnit.KB)));
+        }
+        return settings.build();
     }
 
-    protected void createAndCheckTestRepository(String name) {
+    protected final String createRepository(final String name) {
         final boolean verify = randomBoolean();
-        createTestRepository(name, verify);
 
-        final Iterable<RepositoriesService> repositoriesServices =
+        logger.debug("-->  creating repository [name: {}, verify: {}]", name, verify);
-            internalCluster().getDataOrMasterNodeInstances(RepositoriesService.class);
+        assertAcked(client().admin().cluster().preparePutRepository(name)
+            .setType(repositoryType())
+            .setVerify(verify)
+            .setSettings(repositorySettings()));
 
-        for (RepositoriesService repositoriesService : repositoriesServices) {
+        internalCluster().getDataOrMasterNodeInstances(RepositoriesService.class).forEach(repositories -> {
-            final BlobStoreRepository repository = (BlobStoreRepository) repositoriesService.repository(name);
+            assertThat(repositories.repository(name), notNullValue());
-
+            assertThat(repositories.repository(name), instanceOf(BlobStoreRepository.class));
-            afterCreationCheck(repository);
+            assertThat(repositories.repository(name).isReadOnly(), is(false));
-            assertThat("blob store has to be lazy initialized",
+            BlobStore blobStore = ((BlobStoreRepository) repositories.repository(name)).getBlobStore();
-                repository.getBlobStore(), verify ? is(notNullValue()) : is(nullValue()));
+            assertThat("blob store has to be lazy initialized", blobStore, verify ? is(notNullValue()) : is(nullValue()));
-        }
+        });
 
+        return name;
     }
 
     public void testSnapshotAndRestore() throws Exception {
-        final String repoName = randomAsciiName();
+        final String repoName = createRepository(randomName());
-        logger.info("-->  creating repository {}", repoName);
-        createAndCheckTestRepository(repoName);
         int indexCount = randomIntBetween(1, 5);
         int[] docCounts = new int[indexCount];
         String[] indexNames = generateRandomNames(indexCount);
@@ -93,7 +104,7 @@ public abstract class ESBlobStoreRepositoryIntegTestCase extends ESIntegTestCase
             assertHitCount(client().prepareSearch(indexNames[i]).setSize(0).get(), docCounts[i]);
         }
 
-        final String snapshotName = randomAsciiName();
+        final String snapshotName = randomName();
         logger.info("-->  create snapshot {}:{}", repoName, snapshotName);
         assertSuccessfulSnapshot(client().admin().cluster().prepareCreateSnapshot(repoName, snapshotName)
                 .setWaitForCompletion(true).setIndices(indexNames));
@@ -153,13 +164,11 @@ public abstract class ESBlobStoreRepositoryIntegTestCase extends ESIntegTestCase
     }
 
     public void testMultipleSnapshotAndRollback() throws Exception {
-        String repoName = randomAsciiName();
+        final String repoName = createRepository(randomName());
-        logger.info("-->  creating repository {}", repoName);
-        createAndCheckTestRepository(repoName);
         int iterationCount = randomIntBetween(2, 5);
         int[] docCounts = new int[iterationCount];
-        String indexName = randomAsciiName();
+        String indexName = randomName();
-        String snapshotName = randomAsciiName();
+        String snapshotName = randomName();
         assertAcked(client().admin().indices().prepareCreate(indexName).get());
         for (int i = 0; i < iterationCount; i++) {
             if (randomBoolean() && i > 0) { // don't delete on the first iteration
@@ -210,12 +219,8 @@ public abstract class ESBlobStoreRepositoryIntegTestCase extends ESIntegTestCase
     }
 
     public void testIndicesDeletedFromRepository() throws Exception {
+        final String repoName = createRepository("test-repo");
         Client client = client();
-
-        logger.info("-->  creating repository");
-        final String repoName = "test-repo";
-        createAndCheckTestRepository(repoName);
-
         createIndex("test-idx-1", "test-idx-2", "test-idx-3");
         ensureGreen();
 
@@ -280,41 +285,39 @@ public abstract class ESBlobStoreRepositoryIntegTestCase extends ESIntegTestCase
         indexRandom(true, indexRequestBuilders);
     }
 
-    protected String[] generateRandomNames(int num) {
+    private String[] generateRandomNames(int num) {
         Set<String> names = new HashSet<>();
         for (int i = 0; i < num; i++) {
             String name;
             do {
-                name = randomAsciiName();
+                name = randomName();
             } while (names.contains(name));
             names.add(name);
         }
         return names.toArray(new String[num]);
     }
 
-    public static CreateSnapshotResponse assertSuccessfulSnapshot(CreateSnapshotRequestBuilder requestBuilder) {
+    protected static void assertSuccessfulSnapshot(CreateSnapshotRequestBuilder requestBuilder) {
         CreateSnapshotResponse response = requestBuilder.get();
         assertSuccessfulSnapshot(response);
-        return response;
     }
 
-    public static void assertSuccessfulSnapshot(CreateSnapshotResponse response) {
+    private static void assertSuccessfulSnapshot(CreateSnapshotResponse response) {
         assertThat(response.getSnapshotInfo().successfulShards(), greaterThan(0));
         assertThat(response.getSnapshotInfo().successfulShards(), equalTo(response.getSnapshotInfo().totalShards()));
     }
 
-    public static RestoreSnapshotResponse assertSuccessfulRestore(RestoreSnapshotRequestBuilder requestBuilder) {
+    private static void assertSuccessfulRestore(RestoreSnapshotRequestBuilder requestBuilder) {
         RestoreSnapshotResponse response = requestBuilder.get();
         assertSuccessfulRestore(response);
-        return response;
     }
 
-    public static void assertSuccessfulRestore(RestoreSnapshotResponse response) {
+    private static void assertSuccessfulRestore(RestoreSnapshotResponse response) {
         assertThat(response.getRestoreInfo().successfulShards(), greaterThan(0));
         assertThat(response.getRestoreInfo().successfulShards(), equalTo(response.getRestoreInfo().totalShards()));
     }
 
-    public static String randomAsciiName() {
+    protected static String randomName() {
         return randomAlphaOfLength(randomIntBetween(1, 10)).toLowerCase(Locale.ROOT);
     }
 }