Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix shadowed vars pt6 #80899

Merged
merged 7 commits into from
Nov 23, 2021
Merged

Fix shadowed vars pt6 #80899

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4272a61
Fix more shadowed vars
pugnascotia Nov 19, 2021
9dc687e
Merge remote-tracking branch 'upstream/master' into checkstyle-shadow…
pugnascotia Nov 19, 2021
c9e8d24
Fix more shadowed vars
pugnascotia Nov 19, 2021
a347d9b
Fix more shadowed vars
pugnascotia Nov 19, 2021
413d730
Merge remote-tracking branch 'upstream/master' into checkstyle-shadow…
pugnascotia Nov 22, 2021
2017c23
Fix more shadowed vars
pugnascotia Nov 22, 2021
2e60de1
Formatting
pugnascotia Nov 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...ion/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/cli/InstallPluginAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,8 +467,8 @@ Path downloadZip(String urlString, Path tmpDir) throws IOException {
}

// for testing only
void setEnvironment(Environment env) {
this.env = env;
void setEnvironment(Environment environment) {
this.env = environment;
}

// for testing only
Expand Down
4 changes: 2 additions & 2 deletions ...ution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/cli/SyncPluginsAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public void execute() throws Exception {

// @VisibleForTesting
PluginChanges getPluginChanges(PluginsConfig pluginsConfig, Optional<PluginsConfig> cachedPluginsConfig) throws PluginSyncException {
final List<PluginInfo> existingPlugins = getExistingPlugins(this.env);
final List<PluginInfo> existingPlugins = getExistingPlugins();

final List<PluginDescriptor> pluginsThatShouldExist = pluginsConfig.getPlugins();
final List<PluginDescriptor> pluginsThatActuallyExist = existingPlugins.stream()
Expand Down Expand Up @@ -228,7 +228,7 @@ private List<PluginDescriptor> getPluginsToUpgrade(
}).collect(Collectors.toList());
}

private List<PluginInfo> getExistingPlugins(Environment env) throws PluginSyncException {
private List<PluginInfo> getExistingPlugins() throws PluginSyncException {
final List<PluginInfo> plugins = new ArrayList<>();

try {
Expand Down
49 changes: 25 additions & 24 deletions ...ools/plugin-cli/src/test/java/org/elasticsearch/plugins/cli/InstallPluginActionTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,15 +291,15 @@ void installPlugin(PluginDescriptor plugin, Path home, InstallPluginAction actio
}

void installPlugins(final List<PluginDescriptor> plugins, final Path home, final InstallPluginAction action) throws Exception {
final Environment env = TestEnvironment.newEnvironment(Settings.builder().put("path.home", home).build());
action.setEnvironment(env);
final Environment environment = TestEnvironment.newEnvironment(Settings.builder().put("path.home", home).build());
action.setEnvironment(environment);
action.execute(plugins);
}

void assertPlugin(String name, Path original, Environment env) throws IOException {
assertPluginInternal(name, env.pluginsFile(), original);
assertConfigAndBin(name, original, env);
assertInstallCleaned(env);
void assertPlugin(String name, Path original, Environment environment) throws IOException {
assertPluginInternal(name, environment.pluginsFile(), original);
assertConfigAndBin(name, original, environment);
assertInstallCleaned(environment);
}

void assertPluginInternal(String name, Path pluginsFile, Path originalPlugin) throws IOException {
Expand Down Expand Up @@ -331,9 +331,9 @@ void assertPluginInternal(String name, Path pluginsFile, Path originalPlugin) th
assertFalse("config was not copied", Files.exists(got.resolve("config")));
}

void assertConfigAndBin(String name, Path original, Environment env) throws IOException {
void assertConfigAndBin(String name, Path original, Environment environment) throws IOException {
if (Files.exists(original.resolve("bin"))) {
Path binDir = env.binFile().resolve(name);
Path binDir = environment.binFile().resolve(name);
assertTrue("bin dir exists", Files.exists(binDir));
assertTrue("bin is a dir", Files.isDirectory(binDir));
try (DirectoryStream<Path> stream = Files.newDirectoryStream(binDir)) {
Expand All @@ -347,15 +347,15 @@ void assertConfigAndBin(String name, Path original, Environment env) throws IOEx
}
}
if (Files.exists(original.resolve("config"))) {
Path configDir = env.configFile().resolve(name);
Path configDir = environment.configFile().resolve(name);
assertTrue("config dir exists", Files.exists(configDir));
assertTrue("config is a dir", Files.isDirectory(configDir));

UserPrincipal user = null;
GroupPrincipal group = null;

if (isPosix) {
PosixFileAttributes configAttributes = Files.getFileAttributeView(env.configFile(), PosixFileAttributeView.class)
PosixFileAttributes configAttributes = Files.getFileAttributeView(environment.configFile(), PosixFileAttributeView.class)
.readAttributes();
user = configAttributes.owner();
group = configAttributes.group();
Expand Down Expand Up @@ -383,8 +383,8 @@ void assertConfigAndBin(String name, Path original, Environment env) throws IOEx
}
}

void assertInstallCleaned(Environment env) throws IOException {
try (DirectoryStream<Path> stream = Files.newDirectoryStream(env.pluginsFile())) {
void assertInstallCleaned(Environment environment) throws IOException {
try (DirectoryStream<Path> stream = Files.newDirectoryStream(environment.pluginsFile())) {
for (Path file : stream) {
if (file.getFileName().toString().startsWith(".installing")) {
fail("Installation dir still exists, " + file);
Expand Down Expand Up @@ -598,22 +598,22 @@ public void testBinPermissions() throws Exception {
public void testPluginPermissions() throws Exception {
assumeTrue("posix filesystem", isPosix);

final Path pluginDir = createPluginDir(temp);
final Path resourcesDir = pluginDir.resolve("resources");
final Path platformDir = pluginDir.resolve("platform");
final Path tempPluginDir = createPluginDir(temp);
final Path resourcesDir = tempPluginDir.resolve("resources");
final Path platformDir = tempPluginDir.resolve("platform");
final Path platformNameDir = platformDir.resolve("linux-x86_64");
final Path platformBinDir = platformNameDir.resolve("bin");
Files.createDirectories(platformBinDir);

Files.createFile(pluginDir.resolve("fake-" + Version.CURRENT.toString() + ".jar"));
Files.createFile(tempPluginDir.resolve("fake-" + Version.CURRENT.toString() + ".jar"));
Files.createFile(platformBinDir.resolve("fake_executable"));
Files.createDirectory(resourcesDir);
Files.createFile(resourcesDir.resolve("resource"));

final PluginDescriptor pluginZip = createPluginZip("fake", pluginDir);
final PluginDescriptor pluginZip = createPluginZip("fake", tempPluginDir);

installPlugin(pluginZip);
assertPlugin("fake", pluginDir, env.v2());
assertPlugin("fake", tempPluginDir, env.v2());

final Path fake = env.v2().pluginsFile().resolve("fake");
final Path resources = fake.resolve("resources");
Expand Down Expand Up @@ -729,9 +729,9 @@ public void testZipRelativeOutsideEntryName() throws Exception {
}

public void testOfficialPluginsHelpSortedAndMissingObviouslyWrongPlugins() throws Exception {
MockTerminal terminal = new MockTerminal();
new MockInstallPluginCommand().main(new String[] { "--help" }, terminal);
try (BufferedReader reader = new BufferedReader(new StringReader(terminal.getOutput()))) {
MockTerminal mockTerminal = new MockTerminal();
new MockInstallPluginCommand().main(new String[] { "--help" }, mockTerminal);
try (BufferedReader reader = new BufferedReader(new StringReader(mockTerminal.getOutput()))) {
String line = reader.readLine();

// first find the beginning of our list of official plugins
Expand Down Expand Up @@ -1360,7 +1360,8 @@ private String signature(final byte[] bytes, final PGPSecretKey secretKey) {

// checks the plugin requires a policy confirmation, and does not install when that is rejected by the user
// the plugin is installed after this method completes
private void assertPolicyConfirmation(Tuple<Path, Environment> env, PluginDescriptor pluginZip, String... warnings) throws Exception {
private void assertPolicyConfirmation(Tuple<Path, Environment> pathEnvironmentTuple, PluginDescriptor pluginZip, String... warnings)
throws Exception {
for (int i = 0; i < warnings.length; ++i) {
String warning = warnings[i];
for (int j = 0; j < i; ++j) {
Expand All @@ -1372,7 +1373,7 @@ private void assertPolicyConfirmation(Tuple<Path, Environment> env, PluginDescri
assertThat(e.getMessage(), containsString("installation aborted by user"));

assertThat(terminal.getErrorOutput(), containsString("WARNING: " + warning));
try (Stream<Path> fileStream = Files.list(env.v2().pluginsFile())) {
try (Stream<Path> fileStream = Files.list(pathEnvironmentTuple.v2().pluginsFile())) {
assertThat(fileStream.collect(Collectors.toList()), empty());
}

Expand All @@ -1385,7 +1386,7 @@ private void assertPolicyConfirmation(Tuple<Path, Environment> env, PluginDescri
e = expectThrows(UserException.class, () -> installPlugin(pluginZip));
assertThat(e.getMessage(), containsString("installation aborted by user"));
assertThat(terminal.getErrorOutput(), containsString("WARNING: " + warning));
try (Stream<Path> fileStream = Files.list(env.v2().pluginsFile())) {
try (Stream<Path> fileStream = Files.list(pathEnvironmentTuple.v2().pluginsFile())) {
assertThat(fileStream.collect(Collectors.toList()), empty());
}
}
Expand Down
29 changes: 16 additions & 13 deletions qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/FullClusterRestartIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -782,7 +782,7 @@ public void testSingleDoc() throws IOException {
* Tests that a single empty shard index is correctly recovered. Empty shards are often an edge case.
*/
public void testEmptyShard() throws IOException {
final String index = "test_empty_shard";
final String indexName = "test_empty_shard";

if (isRunningAgainstOldCluster()) {
Settings.Builder settings = Settings.builder()
Expand All @@ -794,9 +794,9 @@ public void testEmptyShard() throws IOException {
// before timing out
.put(INDEX_DELAYED_NODE_LEFT_TIMEOUT_SETTING.getKey(), "100ms")
.put(SETTING_ALLOCATION_MAX_RETRY.getKey(), "0"); // fail faster
createIndex(index, settings.build());
createIndex(indexName, settings.build());
}
ensureGreen(index);
ensureGreen(indexName);
}

/**
Expand Down Expand Up @@ -1165,21 +1165,24 @@ public void testClosedIndices() throws Exception {
* that the index has started shards.
*/
@SuppressWarnings("unchecked")
private void assertClosedIndex(final String index, final boolean checkRoutingTable) throws IOException {
private void assertClosedIndex(final String indexName, final boolean checkRoutingTable) throws IOException {
final Map<String, ?> state = entityAsMap(client().performRequest(new Request("GET", "/_cluster/state")));

final Map<String, ?> metadata = (Map<String, Object>) XContentMapValues.extractValue("metadata.indices." + index, state);
final Map<String, ?> metadata = (Map<String, Object>) XContentMapValues.extractValue("metadata.indices." + indexName, state);
assertThat(metadata, notNullValue());
assertThat(metadata.get("state"), equalTo("close"));

final Map<String, ?> blocks = (Map<String, Object>) XContentMapValues.extractValue("blocks.indices." + index, state);
final Map<String, ?> blocks = (Map<String, Object>) XContentMapValues.extractValue("blocks.indices." + indexName, state);
assertThat(blocks, notNullValue());
assertThat(blocks.containsKey(String.valueOf(MetadataIndexStateService.INDEX_CLOSED_BLOCK_ID)), is(true));

final Map<String, ?> settings = (Map<String, Object>) XContentMapValues.extractValue("settings", metadata);
assertThat(settings, notNullValue());

final Map<String, ?> routingTable = (Map<String, Object>) XContentMapValues.extractValue("routing_table.indices." + index, state);
final Map<String, ?> routingTable = (Map<String, Object>) XContentMapValues.extractValue(
"routing_table.indices." + indexName,
state
);
if (checkRoutingTable) {
assertThat(routingTable, notNullValue());
assertThat(Booleans.parseBoolean((String) XContentMapValues.extractValue("index.verified_before_close", settings)), is(true));
Expand All @@ -1198,7 +1201,7 @@ private void assertClosedIndex(final String index, final boolean checkRoutingTab
for (Map<String, ?> shard : shards) {
assertThat(XContentMapValues.extractValue("shard", shard), equalTo(i));
assertThat(XContentMapValues.extractValue("state", shard), equalTo("STARTED"));
assertThat(XContentMapValues.extractValue("index", shard), equalTo(index));
assertThat(XContentMapValues.extractValue("index", shard), equalTo(indexName));
}
}
} else {
Expand Down Expand Up @@ -1353,12 +1356,12 @@ private String loadInfoDocument(String id) throws IOException {
return m.group(1);
}

private List<String> dataNodes(String index, RestClient client) throws IOException {
Request request = new Request("GET", index + "/_stats");
private List<String> dataNodes(String indexName, RestClient client) throws IOException {
Request request = new Request("GET", indexName + "/_stats");
request.addParameter("level", "shards");
Response response = client.performRequest(request);
List<String> nodes = new ArrayList<>();
List<Object> shardStats = ObjectPath.createFromResponse(response).evaluate("indices." + index + ".shards.0");
List<Object> shardStats = ObjectPath.createFromResponse(response).evaluate("indices." + indexName + ".shards.0");
for (Object shard : shardStats) {
final String nodeId = ObjectPath.evaluate(shard, "routing.node");
nodes.add(nodeId);
Expand All @@ -1370,8 +1373,8 @@ private List<String> dataNodes(String index, RestClient client) throws IOExcepti
* Wait for an index to have green health, waiting longer than
* {@link ESRestTestCase#ensureGreen}.
*/
protected void ensureGreenLongWait(String index) throws IOException {
Request request = new Request("GET", "/_cluster/health/" + index);
protected void ensureGreenLongWait(String indexName) throws IOException {
Request request = new Request("GET", "/_cluster/health/" + indexName);
request.addParameter("timeout", "2m");
request.addParameter("wait_for_status", "green");
request.addParameter("wait_for_no_relocating_shards", "true");
Expand Down
6 changes: 3 additions & 3 deletions qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,12 @@ public abstract class PackagingTestCase extends Assert {
// the java installation already installed on the system
protected static final String systemJavaHome;
static {
Shell sh = new Shell();
Shell initShell = new Shell();
if (Platforms.WINDOWS) {
systemJavaHome = sh.run("$Env:SYSTEM_JAVA_HOME").stdout.trim();
systemJavaHome = initShell.run("$Env:SYSTEM_JAVA_HOME").stdout.trim();
} else {
assert Platforms.LINUX || Platforms.DARWIN;
systemJavaHome = sh.run("echo $SYSTEM_JAVA_HOME").stdout.trim();
systemJavaHome = initShell.run("echo $SYSTEM_JAVA_HOME").stdout.trim();
}
}

Expand Down
8 changes: 2 additions & 6 deletions qa/os/src/test/java/org/elasticsearch/packaging/util/Distribution.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,8 @@ public Distribution(Path path) {

this.platform = filename.contains("windows") ? Platform.WINDOWS : Platform.LINUX;
this.hasJdk = filename.contains("no-jdk") == false;
String version = filename.split("-", 3)[1];
this.baseVersion = version;
if (filename.contains("-SNAPSHOT")) {
version += "-SNAPSHOT";
}
this.version = version;
this.baseVersion = filename.split("-", 3)[1];
this.version = filename.contains("-SNAPSHOT") ? this.baseVersion + "-SNAPSHOT" : this.baseVersion;
}

public boolean isArchive() {
Expand Down
14 changes: 7 additions & 7 deletions qa/os/src/test/java/org/elasticsearch/packaging/util/docker/DockerRun.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,18 +70,18 @@ public DockerRun volume(Path from, Path to) {
/**
* Sets the UID that the container is run with, and the GID too if specified.
*
* @param uid the UID to use, or {@code null} to use the image default
* @param gid the GID to use, or {@code null} to use the image default
* @param uidToUse the UID to use, or {@code null} to use the image default
* @param gidToUse the GID to use, or {@code null} to use the image default
* @return the current builder
*/
public DockerRun uid(Integer uid, Integer gid) {
if (uid == null) {
if (gid != null) {
public DockerRun uid(Integer uidToUse, Integer gidToUse) {
if (uidToUse == null) {
if (gidToUse != null) {
throw new IllegalArgumentException("Cannot override GID without also overriding UID");
}
}
this.uid = uid;
this.gid = gid;
this.uid = uidToUse;
this.gid = gidToUse;
return this;
}

Expand Down
8 changes: 4 additions & 4 deletions ...er/src/test/java/org/elasticsearch/index/replication/RetentionLeasesReplicationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ public void testOutOfOrderRetentionLeasesRequests() throws Exception {
IndexMetadata indexMetadata = buildIndexMetadata(numberOfReplicas, settings, indexMapping);
try (ReplicationGroup group = new ReplicationGroup(indexMetadata) {
@Override
protected void syncRetentionLeases(ShardId shardId, RetentionLeases leases, ActionListener<ReplicationResponse> listener) {
listener.onResponse(new SyncRetentionLeasesResponse(new RetentionLeaseSyncAction.Request(shardId, leases)));
protected void syncRetentionLeases(ShardId id, RetentionLeases leases, ActionListener<ReplicationResponse> listener) {
listener.onResponse(new SyncRetentionLeasesResponse(new RetentionLeaseSyncAction.Request(id, leases)));
}
}) {
group.startAll();
Expand All @@ -102,8 +102,8 @@ public void testSyncRetentionLeasesWithPrimaryPromotion() throws Exception {
IndexMetadata indexMetadata = buildIndexMetadata(numberOfReplicas, settings, indexMapping);
try (ReplicationGroup group = new ReplicationGroup(indexMetadata) {
@Override
protected void syncRetentionLeases(ShardId shardId, RetentionLeases leases, ActionListener<ReplicationResponse> listener) {
listener.onResponse(new SyncRetentionLeasesResponse(new RetentionLeaseSyncAction.Request(shardId, leases)));
protected void syncRetentionLeases(ShardId id, RetentionLeases leases, ActionListener<ReplicationResponse> listener) {
listener.onResponse(new SyncRetentionLeasesResponse(new RetentionLeaseSyncAction.Request(id, leases)));
}
}) {
group.startAll();
Expand Down
18 changes: 9 additions & 9 deletions test/framework/src/main/java/org/elasticsearch/cluster/DiskUsageIntegTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,11 @@ public String name() {

@Override
public long getTotalSpace() throws IOException {
final long totalSpace = this.totalSpace;
if (totalSpace == -1) {
final long totalSpaceCopy = this.totalSpace;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite see the point is creating this intermediate variable in the first place.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a volatile variable, I assume the copy is something to do with that.

if (totalSpaceCopy == -1) {
return super.getTotalSpace();
} else {
return totalSpace;
return totalSpaceCopy;
}
}

Expand All @@ -129,21 +129,21 @@ public void setTotalSpace(long totalSpace) {

@Override
public long getUsableSpace() throws IOException {
final long totalSpace = this.totalSpace;
if (totalSpace == -1) {
final long totalSpaceCopy = this.totalSpace;
if (totalSpaceCopy == -1) {
return super.getUsableSpace();
} else {
return Math.max(0L, totalSpace - getTotalFileSize(path));
return Math.max(0L, totalSpaceCopy - getTotalFileSize(path));
}
}

@Override
public long getUnallocatedSpace() throws IOException {
final long totalSpace = this.totalSpace;
if (totalSpace == -1) {
final long totalSpaceCopy = this.totalSpace;
if (totalSpaceCopy == -1) {
return super.getUnallocatedSpace();
} else {
return Math.max(0L, totalSpace - getTotalFileSize(path));
return Math.max(0L, totalSpaceCopy - getTotalFileSize(path));
}
}

Expand Down
Loading