Skip to content

Commit

Permalink
Begin moving XContent to a separate lib/artifact (#29300)
Browse files Browse the repository at this point in the history
* Begin moving XContent to a separate lib/artifact

This commit moves a large portion of the XContent code from the `server` project
to the `libs/xcontent` project. For the pieces that have been moved, some
helpers have been duplicated to allow them to be decoupled from ES helper
classes. In addition, `Booleans` and `CheckedFunction` have been moved to the
`elasticsearch-core`  project.

This decoupling is a move so that we can eventually make things like the
high-level REST client not rely on the entire ES jar, only the parts it needs.

There are some pieces that are still not decoupled, in particular some of the
XContent tests still remain in the server project, this is because they test a
large portion of the pluggable xcontent pieces through
`XContentElasticsearchException`. They may be decoupled in future work.
Additionally, there may be more piecese that we want to move to the xcontent lib
in the future that are not part of this PR, this is a starting point.

Relates to #28504
  • Loading branch information
dakrone authored Apr 2, 2018
1 parent 1172b3b commit 6b2167f
Show file tree
Hide file tree
Showing 56 changed files with 395 additions and 176 deletions.
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ subprojects {
"org.elasticsearch:elasticsearch-cli:${version}": ':server:cli',
"org.elasticsearch:elasticsearch-core:${version}": ':libs:elasticsearch-core',
"org.elasticsearch:elasticsearch-nio:${version}": ':libs:elasticsearch-nio',
"org.elasticsearch:elasticsearch-x-content:${version}": ':libs:x-content',
"org.elasticsearch:elasticsearch-secure-sm:${version}": ':libs:secure-sm',
"org.elasticsearch.client:elasticsearch-rest-client:${version}": ':client:rest',
"org.elasticsearch.client:elasticsearch-rest-client-sniffer:${version}": ':client:sniffer',
Expand Down
2 changes: 0 additions & 2 deletions buildSrc/src/main/resources/checkstyle_suppressions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -248,9 +248,7 @@
<suppress files="server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]util[/\\]concurrent[/\\]EsExecutors.java" checks="LineLength" />
<suppress files="server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]util[/\\]concurrent[/\\]ThreadBarrier.java" checks="LineLength" />
<suppress files="server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]util[/\\]concurrent[/\\]ThreadContext.java" checks="LineLength" />
<suppress files="server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]xcontent[/\\]XContentFactory.java" checks="LineLength" />
<suppress files="server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]xcontent[/\\]XContentHelper.java" checks="LineLength" />
<suppress files="server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]xcontent[/\\]smile[/\\]SmileXContent.java" checks="LineLength" />
<suppress files="server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]discovery[/\\]Discovery.java" checks="LineLength" />
<suppress files="server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]discovery[/\\]DiscoverySettings.java" checks="LineLength" />
<suppress files="server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]discovery[/\\]zen[/\\]ZenDiscovery.java" checks="LineLength" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,21 +73,34 @@ public static boolean parseBoolean(String value) {
throw new IllegalArgumentException("Failed to parse value [" + value + "] as only [true] or [false] are allowed.");
}

private static boolean hasText(CharSequence str) {
if (str == null || str.length() == 0) {
return false;
}
int strLen = str.length();
for (int i = 0; i < strLen; i++) {
if (!Character.isWhitespace(str.charAt(i))) {
return true;
}
}
return false;
}

/**
*
* @param value text to parse.
* @param defaultValue The default value to return if the provided value is <code>null</code>.
* @return see {@link #parseBoolean(String)}
*/
public static boolean parseBoolean(String value, boolean defaultValue) {
if (Strings.hasText(value)) {
if (hasText(value)) {
return parseBoolean(value);
}
return defaultValue;
}

public static Boolean parseBoolean(String value, Boolean defaultValue) {
if (Strings.hasText(value)) {
if (hasText(value)) {
return parseBoolean(value);
}
return defaultValue;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
* 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;

/**
* Utility class for glob-like matching
*/
public class Glob {

/**
* Match a String against the given pattern, supporting the following simple
* pattern styles: "xxx*", "*xxx", "*xxx*" and "xxx*yyy" matches (with an
* arbitrary number of pattern parts), as well as direct equality.
*
* @param pattern the pattern to match against
* @param str the String to match
* @return whether the String matches the given pattern
*/
public static boolean globMatch(String pattern, String str) {
if (pattern == null || str == null) {
return false;
}
int firstIndex = pattern.indexOf('*');
if (firstIndex == -1) {
return pattern.equals(str);
}
if (firstIndex == 0) {
if (pattern.length() == 1) {
return true;
}
int nextIndex = pattern.indexOf('*', firstIndex + 1);
if (nextIndex == -1) {
return str.endsWith(pattern.substring(1));
} else if (nextIndex == 1) {
// Double wildcard "**" - skipping the first "*"
return globMatch(pattern.substring(1), str);
}
String part = pattern.substring(1, nextIndex);
int partIndex = str.indexOf(part);
while (partIndex != -1) {
if (globMatch(pattern.substring(nextIndex), str.substring(partIndex + part.length()))) {
return true;
}
partIndex = str.indexOf(part, partIndex + 1);
}
return false;
}
return (str.length() >= firstIndex &&
pattern.substring(0, firstIndex).equals(str.substring(0, firstIndex)) &&
globMatch(pattern.substring(firstIndex), str.substring(firstIndex)));
}

}
85 changes: 85 additions & 0 deletions libs/x-content/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import org.elasticsearch.gradle.precommit.PrecommitTasks

/*
* 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.
*/

apply plugin: 'elasticsearch.build'
apply plugin: 'nebula.maven-base-publish'
apply plugin: 'nebula.maven-scm'

archivesBaseName = 'elasticsearch-x-content'

publishing {
publications {
nebula {
artifactId = archivesBaseName
}
}
}

dependencies {
compile "org.elasticsearch:elasticsearch-core:${version}"

compile "org.yaml:snakeyaml:${versions.snakeyaml}"
compile "com.fasterxml.jackson.core:jackson-core:${versions.jackson}"
compile "com.fasterxml.jackson.dataformat:jackson-dataformat-smile:${versions.jackson}"
compile "com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:${versions.jackson}"
compile "com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:${versions.jackson}"

testCompile "com.carrotsearch.randomizedtesting:randomizedtesting-runner:${versions.randomizedrunner}"
testCompile "junit:junit:${versions.junit}"
testCompile "org.hamcrest:hamcrest-all:${versions.hamcrest}"

if (isEclipse == false || project.path == ":libs:x-content-tests") {
testCompile("org.elasticsearch.test:framework:${version}") {
exclude group: 'org.elasticsearch', module: 'elasticsearch-x-content'
}
}

}

forbiddenApisMain {
// x-content does not depend on server
// TODO: Need to decide how we want to handle for forbidden signatures with the changes to core
signaturesURLs = [PrecommitTasks.getResource('/forbidden/jdk-signatures.txt')]
}

if (isEclipse) {
// in eclipse the project is under a fake root, we need to change around the source sets
sourceSets {
if (project.path == ":libs:x-content") {
main.java.srcDirs = ['java']
main.resources.srcDirs = ['resources']
} else {
test.java.srcDirs = ['java']
test.resources.srcDirs = ['resources']
}
}
}

thirdPartyAudit.excludes = [
// from com.fasterxml.jackson.dataformat.yaml.YAMLMapper (jackson-dataformat-yaml)
'com.fasterxml.jackson.databind.ObjectMapper',
]

dependencyLicenses {
mapping from: /jackson-.*/, to: 'jackson'
}

jarHell.enabled = false
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ public class ParseField {
private String allReplacedWith = null;
private final String[] allNames;

private static final String[] EMPTY = new String[0];

/**
* @param name
* the primary name for this field. This will be returned by
Expand All @@ -46,7 +48,7 @@ public class ParseField {
public ParseField(String name, String... deprecatedNames) {
this.name = name;
if (deprecatedNames == null || deprecatedNames.length == 0) {
this.deprecatedNames = Strings.EMPTY_ARRAY;
this.deprecatedNames = EMPTY;
} else {
final HashSet<String> set = new HashSet<>();
Collections.addAll(set, deprecatedNames);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

package org.elasticsearch.common.xcontent;

import org.elasticsearch.common.Booleans;

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

package org.elasticsearch.common.xcontent;

import org.elasticsearch.common.Booleans;

import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@

package org.elasticsearch.common.xcontent;

import org.elasticsearch.common.util.CollectionUtils;

import java.io.ByteArrayOutputStream;
import java.io.Closeable;
import java.io.Flushable;
Expand All @@ -35,6 +33,7 @@
import java.util.Date;
import java.util.GregorianCalendar;
import java.util.HashMap;
import java.util.IdentityHashMap;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -740,7 +739,9 @@ private void unknownValue(Object value, boolean ensureNoSelfReferences) throws I
//Path implements Iterable<Path> and causes endless recursion and a StackOverFlow if treated as an Iterable here
value((Path) value);
} else if (value instanceof Map) {
map((Map<String,?>) value, ensureNoSelfReferences);
@SuppressWarnings("unchecked")
final Map<String, ?> valueMap = (Map<String, ?>) value;
map(valueMap, ensureNoSelfReferences);
} else if (value instanceof Iterable) {
value((Iterable<?>) value, ensureNoSelfReferences);
} else if (value instanceof Object[]) {
Expand Down Expand Up @@ -799,7 +800,7 @@ private XContentBuilder map(Map<String, ?> values, boolean ensureNoSelfReference
// checks that the map does not contain references to itself because
// iterating over map entries will cause a stackoverflow error
if (ensureNoSelfReferences) {
CollectionUtils.ensureNoSelfReferences(values);
ensureNoSelfReferences(values);
}

startObject();
Expand Down Expand Up @@ -828,7 +829,7 @@ private XContentBuilder value(Iterable<?> values, boolean ensureNoSelfReferences
// checks that the iterable does not contain references to itself because
// iterating over entries will cause a stackoverflow error
if (ensureNoSelfReferences) {
CollectionUtils.ensureNoSelfReferences(values);
ensureNoSelfReferences(values);
}
startArray();
for (Object value : values) {
Expand Down Expand Up @@ -937,4 +938,39 @@ static void ensureNotNull(Object value, String message) {
throw new IllegalArgumentException(message);
}
}

private static void ensureNoSelfReferences(Object value) {
Iterable<?> it = convert(value);
if (it != null) {
ensureNoSelfReferences(it, value, Collections.newSetFromMap(new IdentityHashMap<>()));
}
}

private static Iterable<?> convert(Object value) {
if (value == null) {
return null;
}
if (value instanceof Map) {
return ((Map<?,?>) value).values();
} else if ((value instanceof Iterable) && (value instanceof Path == false)) {
return (Iterable<?>) value;
} else if (value instanceof Object[]) {
return Arrays.asList((Object[]) value);
} else {
return null;
}
}

private static void ensureNoSelfReferences(final Iterable<?> value, Object originalReference, final Set<Object> ancestors) {
if (value != null) {
if (ancestors.add(originalReference) == false) {
throw new IllegalArgumentException("Iterable object is self-referencing itself");
}
for (Object o : value) {
ensureNoSelfReferences(convert(o), o, ancestors);
}
ancestors.remove(originalReference);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

import com.fasterxml.jackson.dataformat.cbor.CBORConstants;
import com.fasterxml.jackson.dataformat.smile.SmileConstants;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.xcontent.cbor.CborXContent;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.common.xcontent.smile.SmileXContent;
Expand Down Expand Up @@ -154,7 +153,8 @@ public static XContentType xContentType(CharSequence content) {
return XContentType.JSON;
}
// Should we throw a failure here? Smile idea is to use it in bytes....
if (length > 2 && first == SmileConstants.HEADER_BYTE_1 && content.charAt(1) == SmileConstants.HEADER_BYTE_2 && content.charAt(2) == SmileConstants.HEADER_BYTE_3) {
if (length > 2 && first == SmileConstants.HEADER_BYTE_1 && content.charAt(1) == SmileConstants.HEADER_BYTE_2 &&
content.charAt(2) == SmileConstants.HEADER_BYTE_3) {
return XContentType.SMILE;
}
if (length > 2 && first == '-' && content.charAt(1) == '-' && content.charAt(2) == '-') {
Expand Down Expand Up @@ -186,7 +186,7 @@ public static XContentType xContentType(CharSequence content) {
public static XContent xContent(CharSequence content) {
XContentType type = xContentType(content);
if (type == null) {
throw new ElasticsearchParseException("Failed to derive xcontent");
throw new XContentParseException("Failed to derive xcontent");
}
return xContent(type);
}
Expand All @@ -213,7 +213,7 @@ public static XContent xContent(byte[] data) {
public static XContent xContent(byte[] data, int offset, int length) {
XContentType type = xContentType(data, offset, length);
if (type == null) {
throw new ElasticsearchParseException("Failed to derive xcontent");
throw new XContentParseException("Failed to derive xcontent");
}
return xContent(type);
}
Expand Down Expand Up @@ -278,7 +278,8 @@ public static XContentType xContentType(byte[] bytes, int offset, int length) {
if (first == '{') {
return XContentType.JSON;
}
if (length > 2 && first == SmileConstants.HEADER_BYTE_1 && bytes[offset + 1] == SmileConstants.HEADER_BYTE_2 && bytes[offset + 2] == SmileConstants.HEADER_BYTE_3) {
if (length > 2 && first == SmileConstants.HEADER_BYTE_1 && bytes[offset + 1] == SmileConstants.HEADER_BYTE_2 &&
bytes[offset + 2] == SmileConstants.HEADER_BYTE_3) {
return XContentType.SMILE;
}
if (length > 2 && first == '-' && bytes[offset + 1] == '-' && bytes[offset + 2] == '-') {
Expand Down
Loading

0 comments on commit 6b2167f

Please sign in to comment.