Skip to content

Commit

Permalink
Replace more uses of immutable map builder (elastic#41823)
Browse files Browse the repository at this point in the history
This commit replaces more uses of a custom immutable map builder in
favor of convenience collection factory methods available since JDK 9.
  • Loading branch information
jasontedor authored and Gurkan Kaymak committed May 27, 2019
1 parent 041f4d6 commit 07789ba
Show file tree
Hide file tree
Showing 22 changed files with 378 additions and 116 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,24 +34,23 @@
import org.elasticsearch.client.license.StartTrialRequest;
import org.elasticsearch.client.license.StartTrialResponse;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.MapBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.junit.After;
import org.junit.BeforeClass;

import java.io.IOException;
import java.util.Arrays;
import java.util.List;
import java.util.Map;

import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.isEmptyOrNullString;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.empty;

public class LicenseIT extends ESRestHighLevelClientTestCase {

Expand Down Expand Up @@ -108,26 +107,26 @@ public static void putTrialLicense() throws IOException {
Build.CURRENT.isSnapshot());

// use a hard-coded trial license for 20 yrs to be able to roll back from another licenses
final String signature =
"AAAABAAAAA3FXON9kGmNqmH+ASDWAAAAIAo5/x6hrsGh1GqqrJmy4qgmEC7gK0U4zQ6q5ZEMhm4jAAABAAcdKHL0BfM2uqTgT7BDuFxX5lb"
+ "t/bHDVJ421Wwgm5p3IMbw/W13iiAHz0hhDziF7acJbc/y65L+BKGtVC1gSSHeLDHaAD66VrjKxfc7VbGyJIAYBOdujf0rheurmaD3IcNo"
+ "/tWDjCdtTwrNziFkorsGcPadBP5Yc6csk3/Q74DlfiYweMBxLUfkBERwxwd5OQS6ujGvl/4bb8p5zXvOw8vMSaAXSXXnExP6lam+0934W"
+ "0kHvU7IGk+fCUjOaiSWKSoE4TEcAtVNYj/oRoRtfQ1KQGpdCHxTHs1BimdZaG0nBHDsvhYlVVLSvHN6QzqsHWgFDG6JJxhtU872oTRSUHA=";
final String licenseDefinition = Strings.toString(jsonBuilder()
.startObject()
.field("licenses", Arrays.asList(
MapBuilder.<String, Object>newMapBuilder()
.put("uid", "96fc37c6-6fc9-43e2-a40d-73143850cd72")
.put("type", "trial")
.field("licenses", List.of(
Map.of(
"uid", "96fc37c6-6fc9-43e2-a40d-73143850cd72",
"type", "trial",
// 2018-10-16 07:02:48 UTC
.put("issue_date_in_millis", "1539673368158")
"issue_date_in_millis", "1539673368158",
// 2038-10-11 07:02:48 UTC, 20 yrs later
.put("expiry_date_in_millis", "2170393368158")
.put("max_nodes", "5")
.put("issued_to", "client_rest-high-level_integTestCluster")
.put("issuer", "elasticsearch")
.put("start_date_in_millis", "-1")
.put("signature",
"AAAABAAAAA3FXON9kGmNqmH+ASDWAAAAIAo5/x6hrsGh1GqqrJmy4qgmEC7gK0U4zQ6q5ZEMhm4jAAABAAcdKHL0BfM2uqTgT7BDuFxX5lb"
+ "t/bHDVJ421Wwgm5p3IMbw/W13iiAHz0hhDziF7acJbc/y65L+BKGtVC1gSSHeLDHaAD66VrjKxfc7VbGyJIAYBOdujf0rheurmaD3IcNo"
+ "/tWDjCdtTwrNziFkorsGcPadBP5Yc6csk3/Q74DlfiYweMBxLUfkBERwxwd5OQS6ujGvl/4bb8p5zXvOw8vMSaAXSXXnExP6lam+0934W"
+ "0kHvU7IGk+fCUjOaiSWKSoE4TEcAtVNYj/oRoRtfQ1KQGpdCHxTHs1BimdZaG0nBHDsvhYlVVLSvHN6QzqsHWgFDG6JJxhtU872oTRSUHA=")
.immutableMap()))
"expiry_date_in_millis", "2170393368158",
"max_nodes", "5",
"issued_to", "client_rest-high-level_integTestCluster",
"issuer", "elasticsearch",
"start_date_in_millis", "-1",
"signature", signature)))
.endObject());

final PutLicenseRequest request = new PutLicenseRequest();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import com.google.cloud.http.HttpTransportOptions;
import com.google.cloud.storage.Storage;
import com.google.cloud.storage.StorageOptions;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
Expand All @@ -43,6 +42,7 @@
import java.net.URL;
import java.util.Map;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Collectors;

import static java.util.Collections.emptyMap;

Expand All @@ -65,13 +65,15 @@ public class GoogleCloudStorageService {
*/
public synchronized void refreshAndClearCache(Map<String, GoogleCloudStorageClientSettings> clientsSettings) {
// build the new lazy clients
final MapBuilder<String, LazyInitializable<Storage, IOException>> newClientsCache = MapBuilder.newMapBuilder();
for (final Map.Entry<String, GoogleCloudStorageClientSettings> entry : clientsSettings.entrySet()) {
newClientsCache.put(entry.getKey(),
new LazyInitializable<Storage, IOException>(() -> createClient(entry.getKey(), entry.getValue())));
}
final Map<String, LazyInitializable<Storage, IOException>> newClientsCache =
clientsSettings.entrySet()
.stream()
.collect(Collectors.toUnmodifiableMap(
Map.Entry::getKey,
entry -> new LazyInitializable<>(() -> createClient(entry.getKey(), entry.getValue()))));

// make the new clients available
final Map<String, LazyInitializable<Storage, IOException>> oldClientCache = clientsCache.getAndSet(newClientsCache.immutableMap());
final Map<String, LazyInitializable<Storage, IOException>> oldClientCache = clientsCache.getAndSet(newClientsCache);
// release old clients
oldClientCache.values().forEach(LazyInitializable::reset);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@
import org.elasticsearch.common.blobstore.BlobStoreException;
import org.elasticsearch.common.blobstore.support.AbstractBlobContainer;
import org.elasticsearch.common.blobstore.support.PlainBlobMetaData;
import org.elasticsearch.common.collect.MapBuilder;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.util.Maps;

import java.io.IOException;
import java.io.InputStream;
Expand All @@ -55,6 +55,7 @@
import java.util.Set;
import java.util.stream.Collectors;

import static java.util.Map.entry;
import static org.elasticsearch.repositories.s3.S3Repository.MAX_FILE_SIZE;
import static org.elasticsearch.repositories.s3.S3Repository.MAX_FILE_SIZE_USING_MULTIPART;
import static org.elasticsearch.repositories.s3.S3Repository.MIN_PART_SIZE_USING_MULTIPART;
Expand Down Expand Up @@ -193,7 +194,7 @@ public void deleteBlobIgnoringIfNotExists(String blobName) throws IOException {

@Override
public Map<String, BlobMetaData> listBlobsByPrefix(@Nullable String blobNamePrefix) throws IOException {
final MapBuilder<String, BlobMetaData> blobsBuilder = MapBuilder.newMapBuilder();
final var entries = new ArrayList<Map.Entry<String, BlobMetaData>>();
try (AmazonS3Reference clientReference = blobStore.clientReference()) {
ObjectListing prevListing = null;
while (true) {
Expand All @@ -211,15 +212,15 @@ public Map<String, BlobMetaData> listBlobsByPrefix(@Nullable String blobNamePref
}
for (final S3ObjectSummary summary : list.getObjectSummaries()) {
final String name = summary.getKey().substring(keyPath.length());
blobsBuilder.put(name, new PlainBlobMetaData(name, summary.getSize()));
entries.add(entry(name, new PlainBlobMetaData(name, summary.getSize())));
}
if (list.isTruncated()) {
prevListing = list;
} else {
break;
}
}
return blobsBuilder.immutableMap();
return Maps.ofEntries(entries);
} catch (final AmazonClientException e) {
throw new IOException("Exception when listing blobs by prefix [" + blobNamePrefix + "]", e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,12 @@
import com.amazonaws.services.s3.AmazonS3;
import com.amazonaws.services.s3.AmazonS3ClientBuilder;
import com.amazonaws.services.s3.internal.Constants;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.cluster.metadata.RepositoryMetaData;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.MapBuilder;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.Maps;

import java.io.Closeable;
import java.io.IOException;
Expand All @@ -52,8 +51,8 @@ class S3Service implements Closeable {
/**
* Client settings calculated from static configuration and settings in the keystore.
*/
private volatile Map<String, S3ClientSettings> staticClientSettings = MapBuilder.<String, S3ClientSettings>newMapBuilder()
.put("default", S3ClientSettings.getClientSettings(Settings.EMPTY, "default")).immutableMap();
private volatile Map<String, S3ClientSettings> staticClientSettings =
Map.of("default", S3ClientSettings.getClientSettings(Settings.EMPTY, "default"));

/**
* Client settings derived from those in {@link #staticClientSettings} by combining them with settings
Expand All @@ -71,7 +70,7 @@ public synchronized void refreshAndClearCache(Map<String, S3ClientSettings> clie
// shutdown all unused clients
// others will shutdown on their respective release
releaseCachedClients();
this.staticClientSettings = MapBuilder.newMapBuilder(clientsSettings).immutableMap();
this.staticClientSettings = Maps.ofEntries(clientsSettings.entrySet());
derivedClientSettings = emptyMap();
assert this.staticClientSettings.containsKey("default") : "always at least have 'default'";
// clients are built lazily by {@link client}
Expand All @@ -96,7 +95,7 @@ public AmazonS3Reference client(RepositoryMetaData repositoryMetaData) {
}
final AmazonS3Reference clientReference = new AmazonS3Reference(buildClient(clientSettings));
clientReference.incRef();
clientsCache = MapBuilder.newMapBuilder(clientsCache).put(clientSettings, clientReference).immutableMap();
clientsCache = Maps.copyMapWithAddedEntry(clientsCache, clientSettings, clientReference);
return clientReference;
}
}
Expand Down Expand Up @@ -125,9 +124,11 @@ private S3ClientSettings settings(RepositoryMetaData repositoryMetaData) {
return existing;
}
final S3ClientSettings newSettings = staticSettings.refine(repositoryMetaData);
derivedClientSettings = MapBuilder.newMapBuilder(derivedClientSettings).put(
staticSettings, MapBuilder.newMapBuilder(derivedSettings).put(repositoryMetaData, newSettings).immutableMap()
).immutableMap();
derivedClientSettings =
Maps.copyMayWithAddedOrReplacedEntry(
derivedClientSettings,
staticSettings,
Maps.copyMapWithAddedEntry(derivedSettings, repositoryMetaData, newSettings));
return newSettings;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
import org.elasticsearch.cluster.node.DiscoveryNodes;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Randomness;
import org.elasticsearch.common.collect.MapBuilder;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.util.Maps;
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.shard.ShardId;
Expand Down Expand Up @@ -571,7 +571,7 @@ private AttributesRoutings getActiveAttribute(AttributesKey key, DiscoveryNodes
List<ShardRouting> to = collectAttributeShards(key, nodes, from);

shardRoutings = new AttributesRoutings(to, Collections.unmodifiableList(from));
activeShardsByAttributes = MapBuilder.newMapBuilder(activeShardsByAttributes).put(key, shardRoutings).immutableMap();
activeShardsByAttributes = Maps.copyMapWithAddedEntry(activeShardsByAttributes, key, shardRoutings);
}
}
return shardRoutings;
Expand All @@ -585,7 +585,7 @@ private AttributesRoutings getInitializingAttribute(AttributesKey key, Discovery
List<ShardRouting> to = collectAttributeShards(key, nodes, from);
shardRoutings = new AttributesRoutings(to, Collections.unmodifiableList(from));
initializingShardsByAttributes =
MapBuilder.newMapBuilder(initializingShardsByAttributes).put(key, shardRoutings).immutableMap();
Maps.copyMapWithAddedEntry(initializingShardsByAttributes, key, shardRoutings);
}
}
return shardRoutings;
Expand Down
102 changes: 102 additions & 0 deletions server/src/main/java/org/elasticsearch/common/util/Maps.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* 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.common.util;

import org.elasticsearch.Assertions;

import java.util.Collection;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static java.util.Map.entry;

public class Maps {

/**
* Adds an entry to an immutable map by copying the underlying map and adding the new entry. This method expects there is not already a
* mapping for the specified key in the map.
*
* @param map the immutable map to concatenate the entry to
* @param key the key of the new entry
* @param value the value of the entry
* @param <K> the type of the keys in the map
* @param <V> the type of the values in the map
* @return an immutable map that contains the items from the specified map and the concatenated entry
*/
public static <K, V> Map<K, V> copyMapWithAddedEntry(final Map<K, V> map, final K key, final V value) {
Objects.requireNonNull(map);
Objects.requireNonNull(key);
Objects.requireNonNull(value);
assertImmutableMap(map, key, value);
assert map.containsKey(key) == false : "expected entry [" + key + "] to not already be present in map";
return Stream.concat(map.entrySet().stream(), Stream.of(entry(key, value)))
.collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue));
}

/**
* Adds a new entry to or replaces an existing entry in an immutable map by copying the underlying map and adding the new or replacing
* the existing entry.
*
* @param map the immutable map to add to or replace in
* @param key the key of the new entry
* @param value the value of the new entry
* @param <K> the type of the keys in the map
* @param <V> the type of the values in the map
* @return an immutable map that contains the items from the specified map and a mapping from the specified key to the specified value
*/
public static <K, V> Map<K, V> copyMayWithAddedOrReplacedEntry(final Map<K, V> map, final K key, final V value) {
Objects.requireNonNull(map);
Objects.requireNonNull(key);
Objects.requireNonNull(value);
assertImmutableMap(map, key, value);
return Stream.concat(map.entrySet().stream().filter(k -> key.equals(k.getKey()) == false), Stream.of(entry(key, value)))
.collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue));
}

private static <K, V> void assertImmutableMap(final Map<K, V> map, final K key, final V value) {
if (Assertions.ENABLED) {
boolean immutable;
try {
map.put(key, value);
immutable = false;
} catch (final UnsupportedOperationException e) {
immutable = true;
}
assert immutable : "expected an immutable map but was [" + map.getClass() + "]";
}
}

/**
* A convenience method to convert a collection of map entries to a map. The primary reason this method exists is to have a single
* source file with an unchecked suppression rather than scattered at the various call sites.
*
* @param entries the entries to convert to a map
* @param <K> the type of the keys
* @param <V> the type of the values
* @return an immutable map containing the specified entries
*/
public static <K, V> Map<K, V> ofEntries(final Collection<Map.Entry<K, V>> entries) {
@SuppressWarnings("unchecked") final Map<K, V> map = Map.ofEntries(entries.toArray(Map.Entry[]::new));
return map;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.util.BigArrays;
import org.elasticsearch.common.util.Maps;
import org.elasticsearch.common.util.concurrent.AbstractAsyncTask;
import org.elasticsearch.common.util.concurrent.AbstractRunnable;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
Expand Down Expand Up @@ -91,12 +92,9 @@
import java.util.function.Consumer;
import java.util.function.LongSupplier;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static java.util.Collections.emptyMap;
import static java.util.Collections.unmodifiableMap;
import static java.util.Map.entry;

public class IndexService extends AbstractIndexComponent implements IndicesClusterStateService.AllocatedIndex<IndexShard> {

Expand Down Expand Up @@ -430,8 +428,7 @@ public synchronized IndexShard createShard(
circuitBreakerService);
eventListener.indexShardStateChanged(indexShard, null, indexShard.state(), "shard created");
eventListener.afterIndexShardCreated(indexShard);
shards = Stream.concat(shards.entrySet().stream(), Stream.of(entry(shardId.id(), indexShard)))
.collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue));
shards = Maps.copyMapWithAddedEntry(shards, shardId.id(), indexShard);
success = true;
return indexShard;
} catch (ShardLockObtainFailedException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Streamable;
import org.elasticsearch.common.lucene.Lucene;
import org.elasticsearch.common.util.Maps;
import org.elasticsearch.common.xcontent.ToXContentFragment;
import org.elasticsearch.common.xcontent.XContentBuilder;

Expand Down Expand Up @@ -99,8 +100,7 @@ public void readFrom(StreamInput in) throws IOException {
for (int i = length; i > 0; i--) {
entries.add(entry(in.readString(), in.readString()));
}
// noinspection unchecked
userData = Map.ofEntries(entries.toArray((Map.Entry<String, String>[])new Map.Entry[0]));
userData = Maps.ofEntries(entries);
generation = in.readLong();
id = in.readOptionalString();
numDocs = in.readInt();
Expand Down
Loading

0 comments on commit 07789ba

Please sign in to comment.