From 131a3911eb7488cc667e771bc90a047c9433f022 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Wed, 6 May 2020 09:59:12 +0200 Subject: [PATCH] Replace BlobContainerWrapper by FilterBlobContainer (#56200) A FilterBlobContainer class was introduced in #55952 and it delegates its behavior to a given BlobContainer while allowing to override only necessary methods. This commit replaces the existing BlobContainerWrapper class from the test framework with the new FilterBlobContainer from core. --- .../snapshots/BlobStoreFormatTests.java | 9 +- .../mockstore/BlobContainerWrapper.java | 93 ------------------- .../snapshots/mockstore/MockRepository.java | 8 +- .../store/cache/CachePreWarmingTests.java | 21 ++++- .../CachedBlobContainerIndexInputTests.java | 9 +- 5 files changed, 37 insertions(+), 103 deletions(-) delete mode 100644 test/framework/src/main/java/org/elasticsearch/snapshots/mockstore/BlobContainerWrapper.java diff --git a/server/src/test/java/org/elasticsearch/snapshots/BlobStoreFormatTests.java b/server/src/test/java/org/elasticsearch/snapshots/BlobStoreFormatTests.java index d8a1a1def87..13f0fc14fcc 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/BlobStoreFormatTests.java +++ b/server/src/test/java/org/elasticsearch/snapshots/BlobStoreFormatTests.java @@ -26,6 +26,7 @@ import org.elasticsearch.common.blobstore.BlobMetadata; import org.elasticsearch.common.blobstore.BlobPath; import org.elasticsearch.common.blobstore.BlobStore; import org.elasticsearch.common.blobstore.fs.FsBlobStore; +import org.elasticsearch.common.blobstore.support.FilterBlobContainer; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.io.Streams; import org.elasticsearch.common.io.stream.BytesStreamOutput; @@ -38,7 +39,6 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.translog.BufferedChecksumStreamOutput; import org.elasticsearch.repositories.blobstore.BlobStoreTestUtil; import org.elasticsearch.repositories.blobstore.ChecksumBlobStoreFormat; -import org.elasticsearch.snapshots.mockstore.BlobContainerWrapper; import org.elasticsearch.test.ESTestCase; import java.io.EOFException; @@ -217,12 +217,17 @@ public class BlobStoreFormatTests extends ESTestCase { { IOException writeBlobException = expectThrows(IOException.class, () -> { - BlobContainer wrapper = new BlobContainerWrapper(blobContainer) { + BlobContainer wrapper = new FilterBlobContainer(blobContainer) { @Override public void writeBlobAtomic(String blobName, InputStream inputStream, long blobSize, boolean failIfAlreadyExists) throws IOException { throw new IOException("Exception thrown in writeBlobAtomic() for " + blobName); } + + @Override + protected BlobContainer wrapChild(BlobContainer child) { + return child; + } }; checksumFormat.writeAtomic(blobObj, wrapper, name); }); diff --git a/test/framework/src/main/java/org/elasticsearch/snapshots/mockstore/BlobContainerWrapper.java b/test/framework/src/main/java/org/elasticsearch/snapshots/mockstore/BlobContainerWrapper.java deleted file mode 100644 index 1e1484ca9a3..00000000000 --- a/test/framework/src/main/java/org/elasticsearch/snapshots/mockstore/BlobContainerWrapper.java +++ /dev/null @@ -1,93 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.snapshots.mockstore; - -import org.elasticsearch.common.blobstore.BlobContainer; -import org.elasticsearch.common.blobstore.BlobMetadata; -import org.elasticsearch.common.blobstore.BlobPath; -import org.elasticsearch.common.blobstore.DeleteResult; - -import java.io.IOException; -import java.io.InputStream; -import java.util.List; -import java.util.Map; - -public class BlobContainerWrapper implements BlobContainer { - private BlobContainer delegate; - - public BlobContainerWrapper(BlobContainer delegate) { - this.delegate = delegate; - } - - @Override - public BlobPath path() { - return delegate.path(); - } - - @Override - public InputStream readBlob(String name) throws IOException { - return delegate.readBlob(name); - } - - @Override - public InputStream readBlob(String blobName, long position, long length) throws IOException { - return delegate.readBlob(blobName, position, length); - } - - @Override - public long readBlobPreferredLength() { - return delegate.readBlobPreferredLength(); - } - - @Override - public void writeBlob(String blobName, InputStream inputStream, long blobSize, boolean failIfAlreadyExists) throws IOException { - delegate.writeBlob(blobName, inputStream, blobSize, failIfAlreadyExists); - } - - @Override - public void writeBlobAtomic(final String blobName, final InputStream inputStream, final long blobSize, - boolean failIfAlreadyExists) throws IOException { - delegate.writeBlobAtomic(blobName, inputStream, blobSize, failIfAlreadyExists); - } - - @Override - public DeleteResult delete() throws IOException { - return delegate.delete(); - } - - @Override - public void deleteBlobsIgnoringIfNotExists(List blobNames) throws IOException { - delegate.deleteBlobsIgnoringIfNotExists(blobNames); - } - - @Override - public Map listBlobs() throws IOException { - return delegate.listBlobs(); - } - - @Override - public Map children() throws IOException { - return delegate.children(); - } - - @Override - public Map listBlobsByPrefix(String blobNamePrefix) throws IOException { - return delegate.listBlobsByPrefix(blobNamePrefix); - } -} diff --git a/test/framework/src/main/java/org/elasticsearch/snapshots/mockstore/MockRepository.java b/test/framework/src/main/java/org/elasticsearch/snapshots/mockstore/MockRepository.java index fd1b486b0b5..51fa9877e52 100644 --- a/test/framework/src/main/java/org/elasticsearch/snapshots/mockstore/MockRepository.java +++ b/test/framework/src/main/java/org/elasticsearch/snapshots/mockstore/MockRepository.java @@ -32,6 +32,7 @@ import org.elasticsearch.common.blobstore.BlobPath; import org.elasticsearch.common.blobstore.BlobStore; import org.elasticsearch.common.blobstore.DeleteResult; import org.elasticsearch.common.blobstore.fs.FsBlobContainer; +import org.elasticsearch.common.blobstore.support.FilterBlobContainer; import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; @@ -230,7 +231,7 @@ public class MockRepository extends FsRepository { return new MockBlobContainer(super.blobContainer(path)); } - private class MockBlobContainer extends BlobContainerWrapper { + private class MockBlobContainer extends FilterBlobContainer { private MessageDigest digest; private boolean shouldFail(String blobName, double probability) { @@ -306,6 +307,11 @@ public class MockRepository extends FsRepository { super(delegate); } + @Override + protected BlobContainer wrapChild(BlobContainer child) { + return new MockBlobContainer(child); + } + @Override public InputStream readBlob(String name) throws IOException { maybeIOExceptionOrBlock(name); diff --git a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/index/store/cache/CachePreWarmingTests.java b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/index/store/cache/CachePreWarmingTests.java index 3d39e3d2c0f..000f20bd52a 100644 --- a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/index/store/cache/CachePreWarmingTests.java +++ b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/index/store/cache/CachePreWarmingTests.java @@ -24,6 +24,7 @@ import org.elasticsearch.cluster.metadata.RepositoryMetadata; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.blobstore.BlobContainer; +import org.elasticsearch.common.blobstore.support.FilterBlobContainer; import org.elasticsearch.common.lease.Releasable; import org.elasticsearch.common.lease.Releasables; import org.elasticsearch.common.lucene.Lucene; @@ -45,7 +46,6 @@ import org.elasticsearch.repositories.blobstore.BlobStoreRepository; import org.elasticsearch.repositories.blobstore.BlobStoreTestUtil; import org.elasticsearch.repositories.fs.FsRepository; import org.elasticsearch.snapshots.SnapshotId; -import org.elasticsearch.snapshots.mockstore.BlobContainerWrapper; import org.elasticsearch.test.DummyShardLock; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.IndexSettingsModule; @@ -63,6 +63,7 @@ import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; @@ -217,7 +218,7 @@ public class CachePreWarmingTests extends ESTestCase { .filter(fileInfo -> excludedFromCache.contains(IndexFileNames.getExtension(fileInfo.physicalName())) == false) .collect(Collectors.toList()); - final FilterBlobContainer filterBlobContainer = new FilterBlobContainer(blobContainer); + final TrackingFilesBlobContainer filterBlobContainer = new TrackingFilesBlobContainer(blobContainer); try ( SearchableSnapshotDirectory snapshotDirectory = new SearchableSnapshotDirectory( () -> filterBlobContainer, @@ -264,12 +265,17 @@ public class CachePreWarmingTests extends ESTestCase { } } - private static class FilterBlobContainer extends BlobContainerWrapper { + private static class TrackingFilesBlobContainer extends FilterBlobContainer { - private final Map files = new ConcurrentHashMap<>(); + private final ConcurrentHashMap files; - FilterBlobContainer(BlobContainer delegate) { + TrackingFilesBlobContainer(BlobContainer delegate) { + this(delegate, new ConcurrentHashMap<>()); + } + + TrackingFilesBlobContainer(BlobContainer delegate, ConcurrentHashMap files) { super(delegate); + this.files = Objects.requireNonNull(files); } public long totalFilesRead() { @@ -317,5 +323,10 @@ public class CachePreWarmingTests extends ESTestCase { } }; } + + @Override + protected BlobContainer wrapChild(BlobContainer child) { + return new TrackingFilesBlobContainer(child, this.files); + } } } diff --git a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/index/store/cache/CachedBlobContainerIndexInputTests.java b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/index/store/cache/CachedBlobContainerIndexInputTests.java index aaf3240c64e..9c141b713d3 100644 --- a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/index/store/cache/CachedBlobContainerIndexInputTests.java +++ b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/index/store/cache/CachedBlobContainerIndexInputTests.java @@ -8,6 +8,7 @@ package org.elasticsearch.index.store.cache; import org.apache.lucene.store.IndexInput; import org.elasticsearch.Version; import org.elasticsearch.common.blobstore.BlobContainer; +import org.elasticsearch.common.blobstore.support.FilterBlobContainer; import org.elasticsearch.common.lucene.store.ESIndexInputTestCase; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeValue; @@ -17,7 +18,6 @@ import org.elasticsearch.index.store.SearchableSnapshotDirectory; import org.elasticsearch.index.store.StoreFileMetadata; import org.elasticsearch.repositories.IndexId; import org.elasticsearch.snapshots.SnapshotId; -import org.elasticsearch.snapshots.mockstore.BlobContainerWrapper; import org.elasticsearch.threadpool.TestThreadPool; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots; @@ -208,7 +208,7 @@ public class CachedBlobContainerIndexInputTests extends ESIndexInputTestCase { * BlobContainer that counts the number of {@link java.io.InputStream} it opens, as well as the * total number of bytes read from them. */ - private static class CountingBlobContainer extends BlobContainerWrapper { + private static class CountingBlobContainer extends FilterBlobContainer { private final LongAdder totalBytes = new LongAdder(); private final LongAdder totalOpens = new LongAdder(); @@ -225,6 +225,11 @@ public class CachedBlobContainerIndexInputTests extends ESIndexInputTestCase { return new CountingInputStream(this, super.readBlob(blobName, position, length), length, rangeSize); } + @Override + protected BlobContainer wrapChild(BlobContainer child) { + return new CountingBlobContainer(child, this.rangeSize); + } + @Override public InputStream readBlob(String name) { assert false : "this method should never be called";