Skip to content

Internal: Remove Strings.cleanPath #25209

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

Merged
merged 3 commits into from
Jun 14, 2017
Merged

Internal: Remove Strings.cleanPath #25209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
de61645
Internal: Remove Strings.cleanPath
rjernst Jun 13, 2017
a5dcbe2
Merge branch 'master' into clean_paths
rjernst Jun 14, 2017
98c11ba
Remove unused vars
rjernst Jun 14, 2017
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
68 changes: 0 additions & 68 deletions core/src/main/java/org/elasticsearch/common/Strings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,6 @@ public class Strings {

public static final String[] EMPTY_ARRAY = new String[0];

private static final String FOLDER_SEPARATOR = "/";

private static final String WINDOWS_FOLDER_SEPARATOR = "\\";

private static final String TOP_PATH = "src/test";

private static final String CURRENT_PATH = ".";

public static void spaceify(int spaces, String from, StringBuilder to) throws Exception {
try (BufferedReader reader = new BufferedReader(new FastStringReader(from))) {
String line;
Expand Down Expand Up @@ -403,66 +395,6 @@ public static boolean validFileNameExcludingAstrix(String fileName) {
return true;
}

/**
* Normalize the path by suppressing sequences like "path/.." and
* inner simple dots.
* <p>The result is convenient for path comparison. For other uses,
* notice that Windows separators ("\") are replaced by simple slashes.
*
* @param path the original path
* @return the normalized path
*/
public static String cleanPath(String path) {
if (path == null) {
return null;
}
String pathToUse = replace(path, WINDOWS_FOLDER_SEPARATOR, FOLDER_SEPARATOR);

// Strip prefix from path to analyze, to not treat it as part of the
// first path element. This is necessary to correctly parse paths like
// "file:core/../core/io/Resource.class", where the ".." should just
// strip the first "core" directory while keeping the "file:" prefix.
int prefixIndex = pathToUse.indexOf(":");
String prefix = "";
if (prefixIndex != -1) {
prefix = pathToUse.substring(0, prefixIndex + 1);
pathToUse = pathToUse.substring(prefixIndex + 1);
}
if (pathToUse.startsWith(FOLDER_SEPARATOR)) {
prefix = prefix + FOLDER_SEPARATOR;
pathToUse = pathToUse.substring(1);
}

String[] pathArray = delimitedListToStringArray(pathToUse, FOLDER_SEPARATOR);
List<String> pathElements = new LinkedList<>();
int tops = 0;

for (int i = pathArray.length - 1; i >= 0; i--) {
String element = pathArray[i];
if (CURRENT_PATH.equals(element)) {
// Points to current directory - drop it.
} else if (TOP_PATH.equals(element)) {
// Registering top path found.
tops++;
} else {
if (tops > 0) {
// Merging path element with element corresponding to top path.
tops--;
} else {
// Normal path element found.
pathElements.add(0, element);
}
}
}

// Remaining top paths need to be retained.
for (int i = 0; i < tops; i++) {
pathElements.add(0, TOP_PATH);
}

return prefix + collectionToDelimitedString(pathElements, FOLDER_SEPARATOR);
Copy link
Member

@jasontedor jasontedor Jun 14, 2017

Choose a reason for hiding this comment

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

I think that FOLDER_SEPARATOR and WINDOWS_FOLDER_SEPARATOR are dead fields now?

Copy link
Member Author

@rjernst rjernst Jun 14, 2017

Choose a reason for hiding this comment

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

Removed.

}

/**
* Copy the given Collection into a String array.
* The Collection must contain String elements only.
Expand Down
16 changes: 5 additions & 11 deletions core/src/main/java/org/elasticsearch/env/Environment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,17 @@
import org.elasticsearch.common.settings.Settings;

import java.io.IOException;
import java.io.UncheckedIOException;
import java.net.MalformedURLException;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.FileStore;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.function.Function;

import static org.elasticsearch.common.Strings.cleanPath;

/**
* The environment of where things exists.
*/
Expand Down Expand Up @@ -100,14 +94,14 @@ public class Environment {
public Environment(Settings settings) {
final Path homeFile;
if (PATH_HOME_SETTING.exists(settings)) {
homeFile = PathUtils.get(cleanPath(PATH_HOME_SETTING.get(settings)));
homeFile = PathUtils.get(PATH_HOME_SETTING.get(settings)).normalize();
} else {
throw new IllegalStateException(PATH_HOME_SETTING.getKey() + " is not configured");
}

// this is trappy, Setting#get(Settings) will get a fallback setting yet return false for Settings#exists(Settings)
if (PATH_CONF_SETTING.exists(settings) || DEFAULT_PATH_CONF_SETTING.exists(settings)) {
configFile = PathUtils.get(cleanPath(PATH_CONF_SETTING.get(settings)));
configFile = PathUtils.get(PATH_CONF_SETTING.get(settings)).normalize();
} else {
configFile = homeFile.resolve("config");
}
Expand All @@ -128,7 +122,7 @@ public Environment(Settings settings) {
dataWithClusterFiles = new Path[]{homeFile.resolve("data").resolve(clusterName.value())};
}
if (PATH_SHARED_DATA_SETTING.exists(settings)) {
sharedDataFile = PathUtils.get(cleanPath(PATH_SHARED_DATA_SETTING.get(settings)));
sharedDataFile = PathUtils.get(PATH_SHARED_DATA_SETTING.get(settings)).normalize();
} else {
sharedDataFile = null;
}
Expand All @@ -144,13 +138,13 @@ public Environment(Settings settings) {

// this is trappy, Setting#get(Settings) will get a fallback setting yet return false for Settings#exists(Settings)
if (PATH_LOGS_SETTING.exists(settings) || DEFAULT_PATH_LOGS_SETTING.exists(settings)) {
logsFile = PathUtils.get(cleanPath(PATH_LOGS_SETTING.get(settings)));
logsFile = PathUtils.get(PATH_LOGS_SETTING.get(settings)).normalize();
} else {
logsFile = homeFile.resolve("logs");
}

if (PIDFILE_SETTING.exists(settings)) {
pidFile = PathUtils.get(cleanPath(PIDFILE_SETTING.get(settings)));
pidFile = PathUtils.get(PIDFILE_SETTING.get(settings)).normalize();
} else {
pidFile = null;
}
Expand Down
6 changes: 1 addition & 5 deletions core/src/main/java/org/elasticsearch/node/InternalSettingsPreparer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,8 @@
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;

import org.elasticsearch.cli.Terminal;
Expand All @@ -38,8 +36,6 @@
import org.elasticsearch.common.settings.SettingsException;
import org.elasticsearch.env.Environment;

import static org.elasticsearch.common.Strings.cleanPath;

public class InternalSettingsPreparer {

private static final String[] ALLOWED_SUFFIXES = {".yml", ".yaml", ".json"};
Expand Down Expand Up @@ -111,7 +107,7 @@ public static Environment prepareEnvironment(Settings input, Terminal terminal,
environment = new Environment(output.build());

// we put back the path.logs so we can use it in the logging configuration file
output.put(Environment.PATH_LOGS_SETTING.getKey(), cleanPath(environment.logsFile().toAbsolutePath().toString()));
output.put(Environment.PATH_LOGS_SETTING.getKey(), environment.logsFile().toAbsolutePath().normalize().toString());
return new Environment(output.build());
}

Expand Down