You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by br...@apache.org on 2010/03/23 04:50:16 UTC
svn commit: r926460 - in /incubator/thrift/trunk/lib/java:
src/org/apache/thrift/ src/org/apache/thrift/protocol/
test/org/apache/thrift/test/
Author: bryanduxbury
Date: Tue Mar 23 03:50:16 2010
New Revision: 926460
URL: http://svn.apache.org/viewvc?rev=926460&view=rev
Log:
THRIFT-739. java: TCompactProtocol isn't suitable for reuse in partialDeserialize
This patch changes TProtocol to support a reset() method that should clear any internal state. Stateless protocols can ignore it; stateful ones should implement it. TDeserializer has been updated to take advantage of this method.
Modified:
incubator/thrift/trunk/lib/java/src/org/apache/thrift/TDeserializer.java
incubator/thrift/trunk/lib/java/src/org/apache/thrift/protocol/TCompactProtocol.java
incubator/thrift/trunk/lib/java/src/org/apache/thrift/protocol/TJSONProtocol.java
incubator/thrift/trunk/lib/java/src/org/apache/thrift/protocol/TProtocol.java
incubator/thrift/trunk/lib/java/test/org/apache/thrift/test/PartialDeserializeTest.java
Modified: incubator/thrift/trunk/lib/java/src/org/apache/thrift/TDeserializer.java
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/java/src/org/apache/thrift/TDeserializer.java?rev=926460&r1=926459&r2=926460&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/java/src/org/apache/thrift/TDeserializer.java (original)
+++ incubator/thrift/trunk/lib/java/src/org/apache/thrift/TDeserializer.java Tue Mar 23 03:50:16 2010
@@ -63,8 +63,12 @@ public class TDeserializer {
* @param bytes The array to read from
*/
public void deserialize(TBase base, byte[] bytes) throws TException {
- trans_.reset(bytes);
- base.read(protocol_);
+ try {
+ trans_.reset(bytes);
+ base.read(protocol_);
+ } finally {
+ protocol_.reset();
+ }
}
/**
@@ -80,6 +84,8 @@ public class TDeserializer {
deserialize(base, data.getBytes(charset));
} catch (UnsupportedEncodingException uex) {
throw new TException("JVM DOES NOT SUPPORT ENCODING: " + charset);
+ } finally {
+ protocol_.reset();
}
}
@@ -92,46 +98,52 @@ public class TDeserializer {
* @throws TException
*/
public void partialDeserialize(TBase tb, byte[] bytes, TFieldIdEnum ... fieldIdPath) throws TException {
- // if there are no elements in the path, then the user is looking for the
- // regular deserialize method
- // TODO: it might be nice not to have to do this check every time to save
- // some performance.
- if (fieldIdPath.length == 0) {
- deserialize(tb, bytes);
- return;
- }
+ try {
+ // if there are no elements in the path, then the user is looking for the
+ // regular deserialize method
+ // TODO: it might be nice not to have to do this check every time to save
+ // some performance.
+ if (fieldIdPath.length == 0) {
+ deserialize(tb, bytes);
+ return;
+ }
- trans_.reset(bytes);
+ trans_.reset(bytes);
- // index into field ID path being currently searched for
- int curPathIndex = 0;
+ // index into field ID path being currently searched for
+ int curPathIndex = 0;
- protocol_.readStructBegin();
+ protocol_.readStructBegin();
- while (curPathIndex < fieldIdPath.length) {
- TField field = protocol_.readFieldBegin();
- // we can stop searching if we either see a stop or we go past the field
- // id we're looking for (since fields should now be serialized in asc
- // order).
- if (field.type == TType.STOP || field.id > fieldIdPath[curPathIndex].getThriftFieldId()) {
- return;
- }
+ while (curPathIndex < fieldIdPath.length) {
+ TField field = protocol_.readFieldBegin();
+ // we can stop searching if we either see a stop or we go past the field
+ // id we're looking for (since fields should now be serialized in asc
+ // order).
+ if (field.type == TType.STOP || field.id > fieldIdPath[curPathIndex].getThriftFieldId()) {
+ return;
+ }
- if (field.id != fieldIdPath[curPathIndex].getThriftFieldId()) {
- // Not the field we're looking for. Skip field.
- TProtocolUtil.skip(protocol_, field.type);
- protocol_.readFieldEnd();
- } else {
- // This field is the next step in the path. Step into field.
- curPathIndex++;
- if (curPathIndex < fieldIdPath.length) {
- protocol_.readStructBegin();
+ if (field.id != fieldIdPath[curPathIndex].getThriftFieldId()) {
+ // Not the field we're looking for. Skip field.
+ TProtocolUtil.skip(protocol_, field.type);
+ protocol_.readFieldEnd();
+ } else {
+ // This field is the next step in the path. Step into field.
+ curPathIndex++;
+ if (curPathIndex < fieldIdPath.length) {
+ protocol_.readStructBegin();
+ }
}
}
- }
- // when this line is reached, iprot will be positioned at the start of tb.
- tb.read(protocol_);
+ // when this line is reached, iprot will be positioned at the start of tb.
+ tb.read(protocol_);
+ } catch (Exception e) {
+ throw new TException(e);
+ } finally {
+ protocol_.reset();
+ }
}
/**
@@ -145,4 +157,3 @@ public class TDeserializer {
deserialize(base, data.getBytes());
}
}
-
Modified: incubator/thrift/trunk/lib/java/src/org/apache/thrift/protocol/TCompactProtocol.java
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/java/src/org/apache/thrift/protocol/TCompactProtocol.java?rev=926460&r1=926459&r2=926460&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/java/src/org/apache/thrift/protocol/TCompactProtocol.java (original)
+++ incubator/thrift/trunk/lib/java/src/org/apache/thrift/protocol/TCompactProtocol.java Tue Mar 23 03:50:16 2010
@@ -41,7 +41,7 @@ public final class TCompactProtocol exte
private final static TField TSTOP = new TField("", TType.STOP, (short)0);
private final static byte[] ttypeToCompactType = new byte[16];
-
+
static {
ttypeToCompactType[TType.STOP] = TType.STOP;
ttypeToCompactType[TType.BOOL] = Types.BOOLEAN_TRUE;
@@ -56,24 +56,24 @@ public final class TCompactProtocol exte
ttypeToCompactType[TType.MAP] = Types.MAP;
ttypeToCompactType[TType.STRUCT] = Types.STRUCT;
}
-
+
/**
* TProtocolFactory that produces TCompactProtocols.
*/
public static class Factory implements TProtocolFactory {
public Factory() {}
-
+
public TProtocol getProtocol(TTransport trans) {
return new TCompactProtocol(trans);
}
}
-
+
private static final byte PROTOCOL_ID = (byte)0x82;
private static final byte VERSION = 1;
private static final byte VERSION_MASK = 0x1f; // 0001 1111
private static final byte TYPE_MASK = (byte)0xE0; // 1110 0000
private static final int TYPE_SHIFT_AMOUNT = 5;
-
+
/**
* All of the on-wire type codes.
*/
@@ -91,27 +91,27 @@ public final class TCompactProtocol exte
public static final byte MAP = 0x0B;
public static final byte STRUCT = 0x0C;
}
-
+
/**
* Used to keep track of the last field for the current and previous structs,
* so we can do the delta stuff.
*/
private Stack<Short> lastField_ = new Stack<Short>();
-
+
private short lastFieldId_ = 0;
-
- /**
+
+ /**
* If we encounter a boolean field begin, save the TField here so it can
* have the value incorporated.
*/
private TField booleanField_ = null;
-
+
/**
* If we read a field header, and it's a boolean field, save the boolean
* value here so that readBool can use it.
*/
private Boolean boolValue_ = null;
-
+
/**
* Create a TCompactProtocol.
*
@@ -120,8 +120,13 @@ public final class TCompactProtocol exte
public TCompactProtocol(TTransport transport) {
super(transport);
}
-
-
+
+ @Override
+ public void reset() {
+ lastField_.clear();
+ lastFieldId_ = 0;
+ }
+
//
// Public Writing methods.
//
@@ -155,7 +160,7 @@ public final class TCompactProtocol exte
public void writeStructEnd() throws TException {
lastFieldId_ = lastField_.pop();
}
-
+
/**
* Write a field header containing the field id and field type. If the
* difference between the current field id and the last one is small (< 15),
@@ -260,7 +265,7 @@ public final class TCompactProtocol exte
public void writeI16(short i16) throws TException {
writeVarint32(intToZigZag(i16));
}
-
+
/**
* Write an i32 as a zigzag varint.
*/
@@ -307,7 +312,7 @@ public final class TCompactProtocol exte
// These methods are called by structs, but don't actually have any wire
// output or purpose.
//
-
+
public void writeMessageEnd() throws TException {}
public void writeMapEnd() throws TException {}
public void writeListEnd() throws TException {}
@@ -370,7 +375,7 @@ public final class TCompactProtocol exte
}
trans_.write(varint64out, 0, idx);
}
-
+
/**
* Convert l into a zigzag long. This allows negative numbers to be
* represented compactly as a varint.
@@ -378,7 +383,7 @@ public final class TCompactProtocol exte
private long longToZigzag(long l) {
return (l << 1) ^ (l >> 63);
}
-
+
/**
* Convert n into a zigzag int. This allows negative numbers to be
* represented compactly as a varint.
@@ -386,7 +391,7 @@ public final class TCompactProtocol exte
private int intToZigZag(int n) {
return (n << 1) ^ (n >> 31);
}
-
+
/**
* Convert a long into little-endian bytes in buf starting at off and going
* until off+7.
@@ -650,11 +655,11 @@ public final class TCompactProtocol exte
public void readMapEnd() throws TException {}
public void readListEnd() throws TException {}
public void readSetEnd() throws TException {}
-
+
//
// Internal reading methods
//
-
+
/**
* Read an i32 from the wire as a varint. The MSB of each byte is set
* if there is another byte to follow. This can read up to 5 bytes.
@@ -684,14 +689,14 @@ public final class TCompactProtocol exte
//
// encoding helpers
//
-
+
/**
* Convert from zigzag int to int.
*/
private int zigzagToInt(int n) {
return (n >>> 1) ^ -(n & 1);
}
-
+
/**
* Convert from zigzag long to long.
*/
@@ -767,5 +772,4 @@ public final class TCompactProtocol exte
private byte getCompactType(byte ttype) {
return ttypeToCompactType[ttype];
}
-
}
Modified: incubator/thrift/trunk/lib/java/src/org/apache/thrift/protocol/TJSONProtocol.java
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/java/src/org/apache/thrift/protocol/TJSONProtocol.java?rev=926460&r1=926459&r2=926460&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/java/src/org/apache/thrift/protocol/TJSONProtocol.java (original)
+++ incubator/thrift/trunk/lib/java/src/org/apache/thrift/protocol/TJSONProtocol.java Tue Mar 23 03:50:16 2010
@@ -301,6 +301,13 @@ public class TJSONProtocol extends TProt
super(trans);
}
+ @Override
+ public void reset() {
+ contextStack_.clear();
+ context_ = new JSONBaseContext();
+ reader_ = new LookaheadReader();
+ }
+
// Temporary buffer used by several methods
private byte[] tmpbuf_ = new byte[4];
Modified: incubator/thrift/trunk/lib/java/src/org/apache/thrift/protocol/TProtocol.java
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/java/src/org/apache/thrift/protocol/TProtocol.java?rev=926460&r1=926459&r2=926460&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/java/src/org/apache/thrift/protocol/TProtocol.java (original)
+++ incubator/thrift/trunk/lib/java/src/org/apache/thrift/protocol/TProtocol.java Tue Mar 23 03:50:16 2010
@@ -142,4 +142,10 @@ public abstract class TProtocol {
public abstract String readString() throws TException;
public abstract byte[] readBinary() throws TException;
+
+ /**
+ * Reset any internal state back to a blank slate. This method only needs to
+ * be implemented for stateful protocols.
+ */
+ public void reset() {}
}
Modified: incubator/thrift/trunk/lib/java/test/org/apache/thrift/test/PartialDeserializeTest.java
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/java/test/org/apache/thrift/test/PartialDeserializeTest.java?rev=926460&r1=926459&r2=926460&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/java/test/org/apache/thrift/test/PartialDeserializeTest.java (original)
+++ incubator/thrift/trunk/lib/java/test/org/apache/thrift/test/PartialDeserializeTest.java Tue Mar 23 03:50:16 2010
@@ -75,9 +75,14 @@ public class PartialDeserializeTest {
public static void testPartialDeserialize(TProtocolFactory protocolFactory, TBase input, TBase output, TBase expected, TFieldIdEnum ... fieldIdPath) throws TException {
byte[] record = new TSerializer(protocolFactory).serialize(input);
- new TDeserializer(protocolFactory).partialDeserialize(output, record, fieldIdPath);
- if(!output.equals(expected))
- throw new RuntimeException("with " + protocolFactory.toString() + ", expected " + expected + " but got " + output);
+ TDeserializer deserializer = new TDeserializer(protocolFactory);
+ for (int i = 0; i < 2; i++) {
+ TBase outputCopy = output.deepCopy();
+ deserializer.partialDeserialize(outputCopy, record, fieldIdPath);
+ if(!outputCopy.equals(expected)) {
+ throw new RuntimeException("on attempt " + i + ", with " + protocolFactory.toString() + ", expected " + expected + " but got " + outputCopy);
+ }
+ }
}
}