Skip to content

Commit

Permalink
Support response content-type with versioned media type (#65500)
Browse files Browse the repository at this point in the history
This commit allows returning a correct requested response content-type - it did not work for versioned media types.
It is done by adding new vendor specific instances to XContent and TextFormat enums. These instances can then "format" the response content type string when provided with parameters. This is similar to what SQL plugin does with its media types.

#51816
  • Loading branch information
pgomulka authored Jan 5, 2021
1 parent f24665f commit 5e74f79
Show file tree
Hide file tree
Showing 61 changed files with 446 additions and 122 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1177,14 +1177,14 @@ Params withWaitForEvents(Priority waitForEvents) {
*/
static XContentType enforceSameContentType(IndexRequest indexRequest, @Nullable XContentType xContentType) {
XContentType requestContentType = indexRequest.getContentType();
if (requestContentType != XContentType.JSON && requestContentType != XContentType.SMILE) {
if (requestContentType.canonical() != XContentType.JSON && requestContentType.canonical() != XContentType.SMILE) {
throw new IllegalArgumentException("Unsupported content-type found for request with content-type [" + requestContentType
+ "], only JSON and SMILE are supported");
}
if (xContentType == null) {
return requestContentType;
}
if (requestContentType != xContentType) {
if (requestContentType.canonical() != xContentType.canonical()) {
throw new IllegalArgumentException("Mismatching content-type found for request with content-type [" + requestContentType
+ "], previous requests have content-type [" + xContentType + "]");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1821,9 +1821,11 @@ public void testCreateContentType() {
}

public void testEnforceSameContentType() {
XContentType xContentType = randomFrom(XContentType.JSON, XContentType.SMILE);
XContentType xContentType = randomFrom(XContentType.JSON, XContentType.SMILE, XContentType.VND_JSON, XContentType.VND_SMILE);
IndexRequest indexRequest = new IndexRequest().source(singletonMap("field", "value"), xContentType);
assertEquals(xContentType, enforceSameContentType(indexRequest, null));
// indexRequest content type is made canonical because IndexRequest's content-type is
// from XContentBuilder.getXContentType (hardcoded in JsonXContentGEnerator)
assertEquals(xContentType.canonical(), enforceSameContentType(indexRequest, null));
assertEquals(xContentType, enforceSameContentType(indexRequest, xContentType));

XContentType bulkContentType = randomBoolean() ? xContentType : null;
Expand All @@ -1840,7 +1842,7 @@ public void testEnforceSameContentType() {
assertEquals("Unsupported content-type found for request with content-type [YAML], only JSON and SMILE are supported",
exception.getMessage());

XContentType requestContentType = xContentType == XContentType.JSON ? XContentType.SMILE : XContentType.JSON;
XContentType requestContentType = xContentType.canonical() == XContentType.JSON ? XContentType.SMILE : XContentType.JSON;

exception = expectThrows(IllegalArgumentException.class,
() -> enforceSameContentType(new IndexRequest().source(singletonMap("field", "value"), requestContentType), xContentType));
Expand Down
2 changes: 1 addition & 1 deletion docs/reference/search/search-template.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ The API returns the following result:
"lang" : "mustache",
"source" : """{"query":{"match":{"title":"{{query_string}}"}}}""",
"options": {
"content_type" : "application/json; charset=UTF-8"
"content_type" : "application/json;charset=utf-8"
}
},
"_id": "<templateid>",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@
* I.e. txt used in path _sql?format=txt will return TextFormat.PLAIN_TEXT
*
* Multiple header representations may map to a single {@link MediaType} for example, "application/json"
* and "application/vnd.elasticsearch+json" both represent a JSON MediaType.
* and "application/x-ndjson" both represent a JSON MediaType.
* A MediaType can have only one query parameter representation.
* For example "json" (case insensitive) maps back to a JSON media type.
*
* Additionally, a http header may optionally have parameters. For example "application/json; charset=utf-8".
* Additionally, a http header may optionally have parameters. For example "application/vnd.elasticsearch+json; compatible-with=7".
* This class also allows to define a regular expression for valid values of charset.
*/
public class MediaTypeRegistry<T extends MediaType> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.Locale;
import java.util.Map;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

/**
* A raw result of parsing media types from Accept or Content-Type headers.
Expand All @@ -44,7 +45,7 @@ private ParsedMediaType(String originalHeaderValue, String type, String subType,
this.originalHeaderValue = originalHeaderValue;
this.type = type;
this.subType = subType;
this.parameters = Collections.unmodifiableMap(parameters);
this.parameters = parameters;
}

/**
Expand All @@ -55,7 +56,7 @@ public String mediaTypeWithoutParameters() {
}

public Map<String, String> getParameters() {
return parameters;
return Collections.unmodifiableMap(parameters);
}

/**
Expand All @@ -81,7 +82,7 @@ public static ParsedMediaType parseMediaType(String headerValue) {
throw new IllegalArgumentException("invalid media-type [" + headerValue + "]");
}
if (elements.length == 1) {
return new ParsedMediaType(headerValue, splitMediaType[0].trim(), splitMediaType[1].trim(), Collections.emptyMap());
return new ParsedMediaType(headerValue, splitMediaType[0].trim(), splitMediaType[1].trim(), new HashMap<>());
} else {
Map<String, String> parameters = new HashMap<>();
for (int i = 1; i < elements.length; i++) {
Expand All @@ -105,6 +106,12 @@ public static ParsedMediaType parseMediaType(String headerValue) {
return null;
}

public static ParsedMediaType parseMediaType(XContentType requestContentType, Map<String, String> parameters) {
ParsedMediaType parsedMediaType = parseMediaType(requestContentType.mediaTypeWithoutParameters());
parsedMediaType.parameters.putAll(parameters);
return parsedMediaType;
}

// simplistic check for media ranges. do not validate if this is a correct header
private static boolean isMediaRange(String headerValue) {
return headerValue.contains(",");
Expand Down Expand Up @@ -151,4 +158,16 @@ private boolean isValidParameter(String paramName, String value, Map<String, Pat
public String toString() {
return originalHeaderValue;
}

public String responseContentTypeHeader(Map<String,String> parameters) {
return this.mediaTypeWithoutParameters() + formatParameters(parameters);
}

private String formatParameters(Map<String, String> parameters) {
String joined = parameters.entrySet().stream()
.map(e -> e.getKey() + "=" + e.getValue())
.collect(Collectors.joining(";"));
return joined.isEmpty() ? "" : ";" + joined;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@
*/
public final class XContentBuilder implements Closeable, Flushable {

private byte compatibleMajorVersion;

/**
* Create a new {@link XContentBuilder} using the given {@link XContent} content.
* <p>
Expand All @@ -65,20 +63,21 @@ public static XContentBuilder builder(XContent xContent) throws IOException {
}

/**
* Create a new {@link XContentBuilder} using the given {@link XContent} content and some inclusive and/or exclusive filters.
* Create a new {@link XContentBuilder} using the given {@link XContentType} xContentType and some inclusive and/or exclusive filters.
* <p>
* The builder uses an internal {@link ByteArrayOutputStream} output stream to build the content. When both exclusive and
* inclusive filters are provided, the underlying builder will first use exclusion filters to remove fields and then will check the
* remaining fields against the inclusive filters.
* <p>
*
* @param xContent the {@link XContent}
* @param xContentType the {@link XContentType}
* @param includes the inclusive filters: only fields and objects that match the inclusive filters will be written to the output.
* @param excludes the exclusive filters: only fields and objects that don't match the exclusive filters will be written to the output.
* @throws IOException if an {@link IOException} occurs while building the content
*/
public static XContentBuilder builder(XContent xContent, Set<String> includes, Set<String> excludes) throws IOException {
return new XContentBuilder(xContent, new ByteArrayOutputStream(), includes, excludes);
public static XContentBuilder builder(XContentType xContentType, Set<String> includes, Set<String> excludes) throws IOException {
return new XContentBuilder(xContentType.xContent(), new ByteArrayOutputStream(), includes, excludes,
ParsedMediaType.parseMediaType(xContentType.mediaType()));
}

private static final Map<Class<?>, Writer> WRITERS;
Expand Down Expand Up @@ -167,12 +166,16 @@ public interface HumanReadableTransformer {
*/
private boolean humanReadable = false;

private byte compatibleMajorVersion;

private ParsedMediaType responseContentType;

/**
* Constructs a new builder using the provided XContent and an OutputStream. Make sure
* to call {@link #close()} when the builder is done with.
*/
public XContentBuilder(XContent xContent, OutputStream bos) throws IOException {
this(xContent, bos, Collections.emptySet(), Collections.emptySet());
this(xContent, bos, Collections.emptySet(), Collections.emptySet(), ParsedMediaType.parseMediaType(xContent.type().mediaType()));
}

/**
Expand All @@ -181,8 +184,8 @@ public XContentBuilder(XContent xContent, OutputStream bos) throws IOException {
* filter will be written to the output stream. Make sure to call
* {@link #close()} when the builder is done with.
*/
public XContentBuilder(XContent xContent, OutputStream bos, Set<String> includes) throws IOException {
this(xContent, bos, includes, Collections.emptySet());
public XContentBuilder(XContentType xContentType, OutputStream bos, Set<String> includes) throws IOException {
this(xContentType.xContent(), bos, includes, Collections.emptySet(), ParsedMediaType.parseMediaType(xContentType.mediaType()));
}

/**
Expand All @@ -191,16 +194,25 @@ public XContentBuilder(XContent xContent, OutputStream bos, Set<String> includes
* remaining fields against the inclusive filters.
* <p>
* Make sure to call {@link #close()} when the builder is done with.
*
* @param os the output stream
* @param includes the inclusive filters: only fields and objects that match the inclusive filters will be written to the output.
* @param excludes the exclusive filters: only fields and objects that don't match the exclusive filters will be written to the output.
* @param responseContentType a content-type header value to be send back on a response
*/
public XContentBuilder(XContent xContent, OutputStream os, Set<String> includes, Set<String> excludes) throws IOException {
public XContentBuilder(XContent xContent, OutputStream os, Set<String> includes, Set<String> excludes,
ParsedMediaType responseContentType) throws IOException {
this.bos = os;
assert responseContentType != null : "generated response cannot be null";
this.responseContentType = responseContentType;
this.generator = xContent.createGenerator(bos, includes, excludes);
}

public String getResponseContentTypeString() {
Map<String, String> parameters = responseContentType != null ?
responseContentType.getParameters() : Collections.emptyMap();
return responseContentType.responseContentTypeHeader(parameters);
}

public XContentType contentType() {
return generator.contentType();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,14 @@ public static XContentBuilder cborBuilder(OutputStream os) throws IOException {
* Constructs a xcontent builder that will output the result into the provided output stream.
*/
public static XContentBuilder contentBuilder(XContentType type, OutputStream outputStream) throws IOException {
if (type == XContentType.JSON) {
XContentType canonical = type.canonical();
if (canonical == XContentType.JSON) {
return jsonBuilder(outputStream);
} else if (type == XContentType.SMILE) {
} else if (canonical == XContentType.SMILE) {
return smileBuilder(outputStream);
} else if (type == XContentType.YAML) {
} else if (canonical == XContentType.YAML) {
return yamlBuilder(outputStream);
} else if (type == XContentType.CBOR) {
} else if (canonical == XContentType.CBOR) {
return cborBuilder(outputStream);
}
throw new IllegalArgumentException("No matching content type for " + type);
Expand All @@ -113,13 +114,15 @@ public static XContentBuilder contentBuilder(XContentType type, OutputStream out
* Returns a binary content builder for the provided content type.
*/
public static XContentBuilder contentBuilder(XContentType type) throws IOException {
if (type == XContentType.JSON) {
XContentType canonical = type.canonical();

if (canonical == XContentType.JSON) {
return JsonXContent.contentBuilder();
} else if (type == XContentType.SMILE) {
} else if (canonical == XContentType.SMILE) {
return SmileXContent.contentBuilder();
} else if (type == XContentType.YAML) {
} else if (canonical == XContentType.YAML) {
return YamlXContent.contentBuilder();
} else if (type == XContentType.CBOR) {
} else if (canonical == XContentType.CBOR) {
return CborXContent.contentBuilder();
}
throw new IllegalArgumentException("No matching content type for " + type);
Expand Down
Loading

0 comments on commit 5e74f79

Please sign in to comment.