You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by ct...@apache.org on 2022/04/18 16:50:45 UTC
[thrift] branch master updated: THRIFT-5563: fix deprecation and enable xlint for java library (#2575)
This is an automated email from the ASF dual-hosted git repository.
ctubbsii pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/thrift.git
The following commit(s) were added to refs/heads/master by this push:
new 6bdefc47c THRIFT-5563: fix deprecation and enable xlint for java library (#2575)
6bdefc47c is described below
commit 6bdefc47c3408dc4f9b6eefb6d3449c596109bb3
Author: Jiayu Liu <Ji...@users.noreply.github.com>
AuthorDate: Tue Apr 19 00:50:35 2022 +0800
THRIFT-5563: fix deprecation and enable xlint for java library (#2575)
---
lib/java/gradle/sourceConfiguration.gradle | 11 +-
.../src/org/apache/thrift/partial/TFieldData.java | 2 +-
.../org/apache/thrift/partial/ThriftMetadata.java | 30 ++--
.../apache/thrift/protocol/TCompactProtocol.java | 13 +-
.../org/apache/thrift/transport/THttpClient.java | 163 ++++++++++-----------
.../apache/thrift/protocol/ProtocolTestBase.java | 2 +-
.../test/org/apache/thrift/test/JavaBeansTest.java | 8 +-
7 files changed, 115 insertions(+), 114 deletions(-)
diff --git a/lib/java/gradle/sourceConfiguration.gradle b/lib/java/gradle/sourceConfiguration.gradle
index c510a40d5..b45fdc803 100644
--- a/lib/java/gradle/sourceConfiguration.gradle
+++ b/lib/java/gradle/sourceConfiguration.gradle
@@ -60,7 +60,16 @@ tasks.withType(JavaCompile) {
if (JavaVersion.current() > JavaVersion.VERSION_1_8) {
options.compilerArgs.addAll(['--release', '8'])
}
- // options.compilerArgs.addAll('-Xlint:unchecked')
+ options.compilerArgs.addAll([
+ '-Werror',
+ '-Xlint:deprecation',
+ '-Xlint:cast',
+ '-Xlint:empty',
+ '-Xlint:fallthrough',
+ '-Xlint:finally',
+ '-Xlint:overrides',
+ // we can't enable -Xlint:unchecked just yet
+ ])
}
// ----------------------------------------------------------------------------
diff --git a/lib/java/src/org/apache/thrift/partial/TFieldData.java b/lib/java/src/org/apache/thrift/partial/TFieldData.java
index 9ba1a17ce..d77302e48 100644
--- a/lib/java/src/org/apache/thrift/partial/TFieldData.java
+++ b/lib/java/src/org/apache/thrift/partial/TFieldData.java
@@ -27,7 +27,7 @@ package org.apache.thrift.partial;
*/
public class TFieldData {
public static int encode(byte type) {
- return (int) (type & 0xff);
+ return type & 0xff;
}
public static int encode(byte type, short id) {
diff --git a/lib/java/src/org/apache/thrift/partial/ThriftMetadata.java b/lib/java/src/org/apache/thrift/partial/ThriftMetadata.java
index 1146720c2..46a37f2d9 100644
--- a/lib/java/src/org/apache/thrift/partial/ThriftMetadata.java
+++ b/lib/java/src/org/apache/thrift/partial/ThriftMetadata.java
@@ -25,23 +25,20 @@ import org.apache.thrift.TFieldIdEnum;
import org.apache.thrift.TFieldRequirementType;
import org.apache.thrift.TUnion;
import org.apache.thrift.meta_data.FieldMetaData;
-import org.apache.thrift.meta_data.FieldValueMetaData;
import org.apache.thrift.meta_data.ListMetaData;
import org.apache.thrift.meta_data.MapMetaData;
import org.apache.thrift.meta_data.SetMetaData;
import org.apache.thrift.meta_data.StructMetaData;
-import org.apache.thrift.partial.Validate;
import org.apache.thrift.protocol.TType;
import java.io.Serializable;
-import java.lang.reflect.Field;
+import java.lang.reflect.Constructor;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import java.util.Set;
/**
* Container for Thrift metadata classes such as {@link ThriftPrimitive},
@@ -59,8 +56,8 @@ public class ThriftMetadata {
MAP_VALUE((short) 4, "mapValue"),
SET_ELEMENT((short) 5, "setElement");
- private short id;
- private String name;
+ private final short id;
+ private final String name;
FieldTypeEnum(short id, String name) {
this.id = id;
@@ -415,9 +412,9 @@ public class ThriftMetadata {
Iterable<ThriftField> fieldsData) {
if (isUnion(data)) {
- return new ThriftUnion(parent, fieldId, data, fieldsData);
+ return new ThriftUnion<>(parent, fieldId, data, fieldsData);
} else {
- return new ThriftStruct(parent, fieldId, data, fieldsData);
+ return new ThriftStruct<>(parent, fieldId, data, fieldsData);
}
}
}
@@ -463,18 +460,13 @@ public class ThriftMetadata {
}
public <T extends TBase> T createNewStruct() {
- T instance = null;
-
try {
Class<T> structClass = getStructClass(this.data);
- instance = (T) structClass.newInstance();
- } catch (InstantiationException e) {
- throw new RuntimeException(e);
- } catch (IllegalAccessException e) {
+ Constructor<T> declaredConstructor = structClass.getDeclaredConstructor();
+ return declaredConstructor.newInstance();
+ } catch (ReflectiveOperationException e) {
throw new RuntimeException(e);
}
-
- return instance;
}
public static <T extends TBase> ThriftStruct of(Class<T> clasz) {
@@ -494,7 +486,7 @@ public class ThriftMetadata {
Validate.checkNotNull(clasz, "clasz");
Validate.checkNotNull(fields, "fields");
- return new ThriftStruct(
+ return new ThriftStruct<>(
null,
FieldTypeEnum.ROOT,
new FieldMetaData(
@@ -519,7 +511,7 @@ public class ThriftMetadata {
if (this.fields.size() == 0) {
this.append(sb, "%s*;", indent2);
} else {
- List<Integer> ids = new ArrayList(this.fields.keySet());
+ List<Integer> ids = new ArrayList<>(this.fields.keySet());
Collections.sort(ids);
for (Integer id : ids) {
this.fields.get(id).toPrettyString(sb, level + 1);
@@ -535,7 +527,7 @@ public class ThriftMetadata {
Map<? extends TFieldIdEnum, FieldMetaData> fieldsMetaData =
FieldMetaData.getStructMetaDataMap(clasz);
- Map<Integer, ThriftObject> fields = new HashMap();
+ Map<Integer, ThriftObject> fields = new HashMap<>();
boolean getAllFields = !fieldsData.iterator().hasNext();
if (getAllFields) {
diff --git a/lib/java/src/org/apache/thrift/protocol/TCompactProtocol.java b/lib/java/src/org/apache/thrift/protocol/TCompactProtocol.java
index 832e197dd..3bace8e90 100644
--- a/lib/java/src/org/apache/thrift/protocol/TCompactProtocol.java
+++ b/lib/java/src/org/apache/thrift/protocol/TCompactProtocol.java
@@ -117,7 +117,7 @@ public class TCompactProtocol extends TProtocol {
* Used to keep track of the last field for the current and previous structs,
* so we can do the delta stuff.
*/
- private ShortStack lastField_ = new ShortStack(15);
+ private final ShortStack lastField_ = new ShortStack(15);
private short lastFieldId_ = 0;
@@ -617,7 +617,7 @@ public class TCompactProtocol extends TProtocol {
*/
public boolean readBool() throws TException {
if (boolValue_ != null) {
- boolean result = boolValue_.booleanValue();
+ boolean result = boolValue_;
boolValue_ = null;
return result;
}
@@ -750,10 +750,15 @@ public class TCompactProtocol extends TProtocol {
// These methods are here for the struct to call, but don't have any wire
// encoding.
//
+ @Override
public void readMessageEnd() throws TException {}
+ @Override
public void readFieldEnd() throws TException {}
+ @Override
public void readMapEnd() throws TException {}
+ @Override
public void readListEnd() throws TException {}
+ @Override
public void readSetEnd() throws TException {}
//
@@ -773,7 +778,7 @@ public class TCompactProtocol extends TProtocol {
int off = 0;
while (true) {
byte b = buf[pos+off];
- result |= (int) (b & 0x7f) << shift;
+ result |= (b & 0x7f) << shift;
if ((b & 0x80) != 0x80) break;
shift += 7;
off++;
@@ -782,7 +787,7 @@ public class TCompactProtocol extends TProtocol {
} else {
while (true) {
byte b = readByte();
- result |= (int) (b & 0x7f) << shift;
+ result |= (b & 0x7f) << shift;
if ((b & 0x80) != 0x80) break;
shift += 7;
}
diff --git a/lib/java/src/org/apache/thrift/transport/THttpClient.java b/lib/java/src/org/apache/thrift/transport/THttpClient.java
index 7d61b5c8e..574682248 100644
--- a/lib/java/src/org/apache/thrift/transport/THttpClient.java
+++ b/lib/java/src/org/apache/thrift/transport/THttpClient.java
@@ -21,11 +21,11 @@ package org.apache.thrift.transport;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
-import java.io.InputStream;
import java.io.IOException;
-
-import java.net.URL;
+import java.io.InputStream;
import java.net.HttpURLConnection;
+import java.net.URL;
+import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
@@ -34,9 +34,9 @@ import org.apache.http.HttpHost;
import org.apache.http.HttpResponse;
import org.apache.http.HttpStatus;
import org.apache.http.client.HttpClient;
+import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.entity.ByteArrayEntity;
-import org.apache.http.params.CoreConnectionPNames;
import org.apache.thrift.TConfiguration;
/**
@@ -68,7 +68,7 @@ import org.apache.thrift.TConfiguration;
public class THttpClient extends TEndpointTransport {
- private URL url_ = null;
+ private final URL url_;
private final ByteArrayOutputStream requestBuffer_ = new ByteArrayOutputStream();
@@ -84,6 +84,8 @@ public class THttpClient extends TEndpointTransport {
private final HttpClient client;
+ private static final Map<String, String> DEFAULT_HEADERS = Collections.unmodifiableMap(getDefaultHeaders());
+
public static class Factory extends TTransportFactory {
private final String url;
@@ -159,35 +161,27 @@ public class THttpClient extends TEndpointTransport {
public void setConnectTimeout(int timeout) {
connectTimeout_ = timeout;
- if (null != this.client) {
- // WARNING, this modifies the HttpClient params, this might have an impact elsewhere if the
- // same HttpClient is used for something else.
- client.getParams().setParameter(CoreConnectionPNames.CONNECTION_TIMEOUT, connectTimeout_);
- }
}
public void setReadTimeout(int timeout) {
readTimeout_ = timeout;
- if (null != this.client) {
- // WARNING, this modifies the HttpClient params, this might have an impact elsewhere if the
- // same HttpClient is used for something else.
- client.getParams().setParameter(CoreConnectionPNames.SO_TIMEOUT, readTimeout_);
- }
}
public void setCustomHeaders(Map<String,String> headers) {
- customHeaders_ = headers;
+ customHeaders_ = new HashMap<>(headers);
}
public void setCustomHeader(String key, String value) {
if (customHeaders_ == null) {
- customHeaders_ = new HashMap<String, String>();
+ customHeaders_ = new HashMap<>();
}
customHeaders_.put(key, value);
}
+ @Override
public void open() {}
+ @Override
public void close() {
if (null != inputStream_) {
try {
@@ -198,10 +192,12 @@ public class THttpClient extends TEndpointTransport {
}
}
+ @Override
public boolean isOpen() {
return true;
}
+ @Override
public int read(byte[] buf, int off, int len) throws TTransportException {
if (inputStream_ == null) {
throw new TTransportException("Response buffer is empty, no request.");
@@ -222,10 +218,30 @@ public class THttpClient extends TEndpointTransport {
}
}
+ @Override
public void write(byte[] buf, int off, int len) {
requestBuffer_.write(buf, off, len);
}
+ private RequestConfig getRequestConfig() {
+ RequestConfig requestConfig = RequestConfig.DEFAULT;
+ if (connectTimeout_ > 0) {
+ requestConfig = RequestConfig.copy(requestConfig).setConnectionRequestTimeout(connectTimeout_).build();
+ }
+ if (readTimeout_ > 0) {
+ requestConfig = RequestConfig.copy(requestConfig).setSocketTimeout(readTimeout_).build();
+ }
+ return requestConfig;
+ }
+
+ private static Map<String, String> getDefaultHeaders() {
+ Map<String, String> headers = new HashMap<>();
+ headers.put("Content-Type", "application/x-thrift");
+ headers.put("Accept", "application/x-thrift");
+ headers.put("User-Agent", "Java/THttpClient/HC");
+ return headers;
+ }
+
/**
* copy from org.apache.http.util.EntityUtils#consume. Android has it's own httpcore
* that doesn't have a consume.
@@ -243,7 +259,6 @@ public class THttpClient extends TEndpointTransport {
}
private void flushUsingHttpClient() throws TTransportException {
-
if (null == this.client) {
throw new TTransportException("Null HttpClient, aborting.");
}
@@ -252,63 +267,36 @@ public class THttpClient extends TEndpointTransport {
byte[] data = requestBuffer_.toByteArray();
requestBuffer_.reset();
- HttpPost post = null;
-
- InputStream is = null;
-
+ HttpPost post = new HttpPost(this.url_.getFile());
try {
// Set request to path + query string
- post = new HttpPost(this.url_.getFile());
-
- //
- // Headers are added to the HttpPost instance, not
- // to HttpClient.
- //
-
- post.setHeader("Content-Type", "application/x-thrift");
- post.setHeader("Accept", "application/x-thrift");
- post.setHeader("User-Agent", "Java/THttpClient/HC");
-
+ post.setConfig(getRequestConfig());
+ DEFAULT_HEADERS.forEach(post::addHeader);
if (null != customHeaders_) {
- for (Map.Entry<String, String> header : customHeaders_.entrySet()) {
- post.setHeader(header.getKey(), header.getValue());
- }
+ customHeaders_.forEach(post::addHeader);
}
-
post.setEntity(new ByteArrayEntity(data));
-
HttpResponse response = this.client.execute(this.host, post);
- int responseCode = response.getStatusLine().getStatusCode();
-
- //
- // Retrieve the inputstream BEFORE checking the status code so
- // resources get freed in the finally clause.
- //
-
- is = response.getEntity().getContent();
+ handleResponse(response);
+ } catch (IOException ioe) {
+ // Abort method so the connection gets released back to the connection manager
+ post.abort();
+ throw new TTransportException(ioe);
+ } finally {
+ resetConsumedMessageSize(-1);
+ post.releaseConnection();
+ }
+ }
+ private void handleResponse(HttpResponse response) throws TTransportException {
+ // Retrieve the InputStream BEFORE checking the status code so
+ // resources get freed in the with clause.
+ try (InputStream is = response.getEntity().getContent()) {
+ int responseCode = response.getStatusLine().getStatusCode();
if (responseCode != HttpStatus.SC_OK) {
throw new TTransportException("HTTP Response code: " + responseCode);
}
-
- // Read the responses into a byte array so we can release the connection
- // early. This implies that the whole content will have to be read in
- // memory, and that momentarily we might use up twice the memory (while the
- // thrift struct is being read up the chain).
- // Proceeding differently might lead to exhaustion of connections and thus
- // to app failure.
-
- byte[] buf = new byte[1024];
- ByteArrayOutputStream baos = new ByteArrayOutputStream();
-
- int len = 0;
- do {
- len = is.read(buf);
- if (len > 0) {
- baos.write(buf, 0, len);
- }
- } while (-1 != len);
-
+ byte[] readByteArray = readIntoByteArray(is);
try {
// Indicate we're done with the content.
consume(response.getEntity());
@@ -316,30 +304,37 @@ public class THttpClient extends TEndpointTransport {
// We ignore this exception, it might only mean the server has no
// keep-alive capability.
}
-
- inputStream_ = new ByteArrayInputStream(baos.toByteArray());
+ inputStream_ = new ByteArrayInputStream(readByteArray);
} catch (IOException ioe) {
- // Abort method so the connection gets released back to the connection manager
- if (null != post) {
- post.abort();
- }
throw new TTransportException(ioe);
- } finally {
- resetConsumedMessageSize(-1);
- if (null != is) {
- // Close the entity's input stream, this will release the underlying connection
- try {
- is.close();
- } catch (IOException ioe) {
- throw new TTransportException(ioe);
- }
- }
- if (post != null) {
- post.releaseConnection();
- }
}
}
+ /**
+ * Read the responses into a byte array so we can release the connection
+ * early. This implies that the whole content will have to be read in
+ * memory, and that momentarily we might use up twice the memory (while the
+ * thrift struct is being read up the chain).
+ * Proceeding differently might lead to exhaustion of connections and thus
+ * to app failure.
+ *
+ * @param is input stream
+ * @return read bytes
+ * @throws IOException when exception during read
+ */
+ private static byte[] readIntoByteArray(InputStream is) throws IOException {
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ byte[] buf = new byte[1024];
+ int len;
+ do {
+ len = is.read(buf);
+ if (len > 0) {
+ baos.write(buf, 0, len);
+ }
+ } while (-1 != len);
+ return baos.toByteArray();
+ }
+
public void flush() throws TTransportException {
if (null != this.client) {
diff --git a/lib/java/test/org/apache/thrift/protocol/ProtocolTestBase.java b/lib/java/test/org/apache/thrift/protocol/ProtocolTestBase.java
index 52b107441..a87327121 100644
--- a/lib/java/test/org/apache/thrift/protocol/ProtocolTestBase.java
+++ b/lib/java/test/org/apache/thrift/protocol/ProtocolTestBase.java
@@ -167,7 +167,7 @@ public abstract class ProtocolTestBase extends TestCase {
}
public void readMethod(TProtocol proto) throws TException {
- assertEquals((byte)b, proto.readByte());
+ assertEquals(b, proto.readByte());
}
});
}
diff --git a/lib/java/test/org/apache/thrift/test/JavaBeansTest.java b/lib/java/test/org/apache/thrift/test/JavaBeansTest.java
index 6a2a0ed0c..0bfcefec0 100644
--- a/lib/java/test/org/apache/thrift/test/JavaBeansTest.java
+++ b/lib/java/test/org/apache/thrift/test/JavaBeansTest.java
@@ -61,10 +61,10 @@ public class JavaBeansTest {
// Everything is set
ooe.set_a_bite((byte) 1);
ooe.set_base64(ByteBuffer.wrap("bytes".getBytes()));
- ooe.set_byte_list(new LinkedList<Byte>());
+ ooe.set_byte_list(new LinkedList<>());
ooe.set_double_precision(1);
- ooe.set_i16_list(new LinkedList<Short>());
- ooe.set_i64_list(new LinkedList<Long>());
+ ooe.set_i16_list(new LinkedList<>());
+ ooe.set_i64_list(new LinkedList<>());
ooe.set_boolean_field(true);
ooe.set_integer16((short) 1);
ooe.set_integer32(1);
@@ -102,7 +102,7 @@ public class JavaBeansTest {
// Should throw exception when field doesn't exist
boolean exceptionThrown = false;
try{
- if (ooe.isSet(ooe.fieldForId(100)));
+ ooe.isSet(ooe.fieldForId(100));
} catch (IllegalArgumentException e){
exceptionThrown = true;
}