Skip to content

Switch XContentBuilder from BytesStreamOutput to ByteArrayOutputStream #28945

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.

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 1 commit into from
Mar 8, 2018
Merged
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@
package org.elasticsearch.common.xcontent;

import org.apache.lucene.util.BytesRef;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.geo.GeoPoint;
import org.elasticsearch.common.io.stream.BytesStream;
import org.elasticsearch.common.io.stream.BytesStreamOutput;
import org.elasticsearch.common.lease.Releasable;
import org.elasticsearch.common.text.Text;
import org.elasticsearch.common.unit.ByteSizeValue;
Expand All @@ -34,6 +34,7 @@
import org.joda.time.format.DateTimeFormatter;
import org.joda.time.format.ISODateTimeFormat;

import java.io.ByteArrayOutputStream;
import java.io.Flushable;
import java.io.IOException;
import java.io.InputStream;
Expand All @@ -58,21 +59,21 @@ public final class XContentBuilder implements Releasable, Flushable {
/**
* Create a new {@link XContentBuilder} using the given {@link XContent} content.
* <p>
* The builder uses an internal {@link BytesStreamOutput} output stream to build the content.
* The builder uses an internal {@link ByteArrayOutputStream} output stream to build the content.
* </p>
*
* @param xContent the {@link XContent}
* @return a new {@link XContentBuilder}
* @throws IOException if an {@link IOException} occurs while building the content
*/
public static XContentBuilder builder(XContent xContent) throws IOException {
return new XContentBuilder(xContent, new BytesStreamOutput());
return new XContentBuilder(xContent, new ByteArrayOutputStream());
}

/**
* Create a new {@link XContentBuilder} using the given {@link XContent} content and some inclusive and/or exclusive filters.
* <p>
* The builder uses an internal {@link BytesStreamOutput} output stream to build the content. When both exclusive and
* 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>
Expand All @@ -83,7 +84,7 @@ public static XContentBuilder builder(XContent xContent) throws IOException {
* @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 BytesStreamOutput(), includes, excludes);
return new XContentBuilder(xContent, new ByteArrayOutputStream(), includes, excludes);
}

public static final DateTimeFormatter DEFAULT_DATE_PRINTER = ISODateTimeFormat.dateTime().withZone(DateTimeZone.UTC);
Expand Down Expand Up @@ -1036,7 +1037,11 @@ public XContentGenerator generator() {

public BytesReference bytes() {
close();
return ((BytesStream) bos).bytes();
if (bos instanceof ByteArrayOutputStream) {
return new BytesArray(((ByteArrayOutputStream) bos).toByteArray());
} else {
return ((BytesStream) bos).bytes();
Copy link
Member

Choose a reason for hiding this comment

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

Oh boy!

Copy link
Member

Choose a reason for hiding this comment

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

A problem for another PR, I assume.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, unfortunately

}
}

/**
Expand Down