Nano: don't generate accessor methods for nested methods
For nested message objects, don't generate accessor methods because they have a default value that is not a valid value (null), so there is no reason to have get/set/has/clear methods for them. Clients and protos (while serializing) can check against the invalid value to see if it's been set. Change-Id: Ic63400889581271b8cbcd9c45c84519d4921fd4b
This commit is contained in:
Andrew Flynn
parent
2d2557ea70
commit
c997c136bb
@ -507,9 +507,9 @@ optional_field_style={default,accessors,reftypes} (default: default)
|
||||
useful when you need to serialize a field with the default value, or check
|
||||
if a field has been explicitly set to its default value from the wire.
|
||||
|
||||
In the 'accessors' style, required fields are still translated to one
|
||||
public mutable Java field each, and repeated fields are still translated
|
||||
to arrays. No accessors are generated for them.
|
||||
In the 'accessors' style, required and nested message fields are still
|
||||
translated to one public mutable Java field each, repeated fields are still
|
||||
translated to arrays. No accessors are generated for them.
|
||||
|
||||
IMPORTANT: When using the 'accessors' style, ProGuard should always
|
||||
be enabled with optimization (don't use -dontoptimize) and allowing
|
||||
|
||||
@ -2271,9 +2271,10 @@ public class NanoTest extends TestCase {
|
||||
public void testNanoWithAccessorsBasic() throws Exception {
|
||||
TestNanoAccessors msg = new TestNanoAccessors();
|
||||
|
||||
// Makes sure required and repeated fields are still public fields
|
||||
// Makes sure required, repeated, and message fields are still public
|
||||
msg.id = 3;
|
||||
msg.repeatedBytes = new byte[2][3];
|
||||
msg.optionalNestedMessage = null;
|
||||
|
||||
// Test accessors
|
||||
assertEquals(0, msg.getOptionalInt32());
|
||||
@ -2295,10 +2296,6 @@ public class NanoTest extends TestCase {
|
||||
msg.setOptionalString(null);
|
||||
fail();
|
||||
} catch (NullPointerException expected) {}
|
||||
try {
|
||||
msg.setOptionalNestedMessage(null);
|
||||
fail();
|
||||
} catch (NullPointerException expected) {}
|
||||
|
||||
// Test has bit on bytes field with defaults and clear() re-clones the default array
|
||||
assertFalse(msg.hasDefaultBytes());
|
||||
@ -2352,8 +2349,8 @@ public class NanoTest extends TestCase {
|
||||
assertFalse(msg.hasDefaultBytes());
|
||||
assertFalse(msg.hasDefaultFloatNan());
|
||||
assertFalse(msg.hasDefaultNestedEnum());
|
||||
msg.setOptionalNestedMessage(new TestNanoAccessors.NestedMessage());
|
||||
msg.getOptionalNestedMessage().setBb(2);
|
||||
msg.optionalNestedMessage = new TestNanoAccessors.NestedMessage();
|
||||
msg.optionalNestedMessage.setBb(2);
|
||||
msg.setOptionalNestedEnum(TestNanoAccessors.BAZ);
|
||||
msg.setDefaultInt32(msg.getDefaultInt32());
|
||||
}
|
||||
@ -2366,8 +2363,8 @@ public class NanoTest extends TestCase {
|
||||
|
||||
// Has fields true upon parse.
|
||||
TestNanoAccessors newMsg = TestNanoAccessors.parseFrom(result);
|
||||
assertEquals(2, newMsg.getOptionalNestedMessage().getBb());
|
||||
assertTrue(newMsg.getOptionalNestedMessage().hasBb());
|
||||
assertEquals(2, newMsg.optionalNestedMessage.getBb());
|
||||
assertTrue(newMsg.optionalNestedMessage.hasBb());
|
||||
assertEquals(TestNanoAccessors.BAZ, newMsg.getOptionalNestedEnum());
|
||||
assertTrue(newMsg.hasOptionalNestedEnum());
|
||||
|
||||
@ -2376,6 +2373,38 @@ public class NanoTest extends TestCase {
|
||||
assertEquals(41, newMsg.getDefaultInt32());
|
||||
}
|
||||
|
||||
public void testNanoWithAccessorsPublicFieldTypes() throws Exception {
|
||||
TestNanoAccessors msg = new TestNanoAccessors();
|
||||
assertNull(msg.optionalNestedMessage);
|
||||
assertEquals(0, msg.id);
|
||||
assertEquals(0, msg.repeatedNestedEnum.length);
|
||||
|
||||
TestNanoAccessors newMsg = TestNanoAccessors.parseFrom(MessageNano.toByteArray(msg));
|
||||
assertNull(newMsg.optionalNestedMessage);
|
||||
assertEquals(0, newMsg.id);
|
||||
assertEquals(0, newMsg.repeatedNestedEnum.length);
|
||||
|
||||
TestNanoAccessors.NestedMessage nestedMessage = new TestNanoAccessors.NestedMessage();
|
||||
nestedMessage.setBb(5);
|
||||
newMsg.optionalNestedMessage = nestedMessage;
|
||||
newMsg.id = -1;
|
||||
newMsg.repeatedNestedEnum = new int[] { TestAllTypesNano.FOO };
|
||||
|
||||
TestNanoAccessors newMsg2 = TestNanoAccessors.parseFrom(MessageNano.toByteArray(newMsg));
|
||||
assertEquals(nestedMessage.getBb(), newMsg2.optionalNestedMessage.getBb());
|
||||
assertEquals(-1, newMsg2.id);
|
||||
assertEquals(TestAllTypesNano.FOO, newMsg2.repeatedNestedEnum[0]);
|
||||
|
||||
newMsg2.optionalNestedMessage = null;
|
||||
newMsg2.id = 0;
|
||||
newMsg2.repeatedNestedEnum = null;
|
||||
|
||||
TestNanoAccessors newMsg3 = TestNanoAccessors.parseFrom(MessageNano.toByteArray(newMsg2));
|
||||
assertNull(newMsg3.optionalNestedMessage);
|
||||
assertEquals(0, newMsg3.id);
|
||||
assertEquals(0, newMsg3.repeatedNestedEnum.length);
|
||||
}
|
||||
|
||||
public void testNanoWithAccessorsSerialize() throws Exception {
|
||||
TestNanoAccessors msg = new TestNanoAccessors();
|
||||
msg.setOptionalInt32(msg.getOptionalInt32());
|
||||
@ -2383,7 +2412,7 @@ public class NanoTest extends TestCase {
|
||||
msg.setOptionalBytes(msg.getOptionalBytes());
|
||||
TestNanoAccessors.NestedMessage nestedMessage = new TestNanoAccessors.NestedMessage();
|
||||
nestedMessage.setBb(nestedMessage.getBb());
|
||||
msg.setOptionalNestedMessage(nestedMessage);
|
||||
msg.optionalNestedMessage = nestedMessage;
|
||||
msg.setOptionalNestedEnum(msg.getOptionalNestedEnum());
|
||||
msg.setDefaultInt32(msg.getDefaultInt32());
|
||||
msg.setDefaultString(msg.getDefaultString());
|
||||
@ -2400,8 +2429,7 @@ public class NanoTest extends TestCase {
|
||||
assertTrue(newMsg.hasOptionalInt32());
|
||||
assertTrue(newMsg.hasOptionalString());
|
||||
assertTrue(newMsg.hasOptionalBytes());
|
||||
assertTrue(newMsg.hasOptionalNestedMessage());
|
||||
assertTrue(newMsg.getOptionalNestedMessage().hasBb());
|
||||
assertTrue(newMsg.optionalNestedMessage.hasBb());
|
||||
assertTrue(newMsg.hasOptionalNestedEnum());
|
||||
assertTrue(newMsg.hasDefaultInt32());
|
||||
assertTrue(newMsg.hasDefaultString());
|
||||
@ -2411,7 +2439,7 @@ public class NanoTest extends TestCase {
|
||||
assertEquals(0, newMsg.getOptionalInt32());
|
||||
assertEquals(0, newMsg.getOptionalString().length());
|
||||
assertEquals(0, newMsg.getOptionalBytes().length);
|
||||
assertEquals(0, newMsg.getOptionalNestedMessage().getBb());
|
||||
assertEquals(0, newMsg.optionalNestedMessage.getBb());
|
||||
assertEquals(TestNanoAccessors.FOO, newMsg.getOptionalNestedEnum());
|
||||
assertEquals(41, newMsg.getDefaultInt32());
|
||||
assertEquals("hello", newMsg.getDefaultString());
|
||||
@ -2856,15 +2884,15 @@ public class NanoTest extends TestCase {
|
||||
.setOptionalInt32(5)
|
||||
.setOptionalString("Hello")
|
||||
.setOptionalBytes(new byte[] {1, 2, 3})
|
||||
.setOptionalNestedMessage(new TestNanoAccessors.NestedMessage().setBb(27))
|
||||
.setOptionalNestedEnum(TestNanoAccessors.BAR)
|
||||
.setDefaultFloatNan(1.0f);
|
||||
message.optionalNestedMessage = new TestNanoAccessors.NestedMessage().setBb(27);
|
||||
message.repeatedInt32 = new int[] { 5, 6, 7, 8 };
|
||||
message.repeatedString = new String[] { "One", "Two" };
|
||||
message.repeatedBytes = new byte[][] { { 2, 7 }, { 2, 7 } };
|
||||
message.repeatedNestedMessage = new TestNanoAccessors.NestedMessage[] {
|
||||
message.getOptionalNestedMessage(),
|
||||
message.getOptionalNestedMessage()
|
||||
message.optionalNestedMessage,
|
||||
message.optionalNestedMessage
|
||||
};
|
||||
message.repeatedNestedEnum = new int[] {
|
||||
TestAllTypesNano.BAR,
|
||||
|
||||
@ -78,8 +78,9 @@ FieldGeneratorMap::FieldGeneratorMap(const Descriptor* descriptor, const Params
|
||||
|
||||
FieldGenerator* FieldGeneratorMap::MakeGenerator(const FieldDescriptor* field,
|
||||
const Params ¶ms, int* next_has_bit_index) {
|
||||
JavaType java_type = GetJavaType(field);
|
||||
if (field->is_repeated()) {
|
||||
switch (GetJavaType(field)) {
|
||||
switch (java_type) {
|
||||
case JAVATYPE_MESSAGE:
|
||||
return new RepeatedMessageFieldGenerator(field, params);
|
||||
case JAVATYPE_ENUM:
|
||||
@ -87,14 +88,13 @@ FieldGenerator* FieldGeneratorMap::MakeGenerator(const FieldDescriptor* field,
|
||||
default:
|
||||
return new RepeatedPrimitiveFieldGenerator(field, params);
|
||||
}
|
||||
} else if (params.optional_field_accessors() && field->is_optional()) {
|
||||
} else if (params.optional_field_accessors() && field->is_optional()
|
||||
&& java_type != JAVATYPE_MESSAGE) {
|
||||
// We need a has-bit for each primitive/enum field because their default
|
||||
// values could be same as explicitly set values. But we don't need it
|
||||
// for a message field because they have no defaults and Nano uses 'null'
|
||||
// for unset messages, which cannot be set explicitly.
|
||||
switch (GetJavaType(field)) {
|
||||
case JAVATYPE_MESSAGE:
|
||||
return new AccessorMessageFieldGenerator(field, params);
|
||||
switch (java_type) {
|
||||
case JAVATYPE_ENUM:
|
||||
return new AccessorEnumFieldGenerator(
|
||||
field, params, (*next_has_bit_index)++);
|
||||
@ -103,7 +103,7 @@ FieldGenerator* FieldGeneratorMap::MakeGenerator(const FieldDescriptor* field,
|
||||
field, params, (*next_has_bit_index)++);
|
||||
}
|
||||
} else {
|
||||
switch (GetJavaType(field)) {
|
||||
switch (java_type) {
|
||||
case JAVATYPE_MESSAGE:
|
||||
return new MessageFieldGenerator(field, params);
|
||||
case JAVATYPE_ENUM:
|
||||
|
||||
@ -149,97 +149,6 @@ GenerateHashCodeCode(io::Printer* printer) const {
|
||||
|
||||
// ===================================================================
|
||||
|
||||
AccessorMessageFieldGenerator::
|
||||
AccessorMessageFieldGenerator(
|
||||
const FieldDescriptor* descriptor, const Params& params)
|
||||
: FieldGenerator(params), descriptor_(descriptor) {
|
||||
SetMessageVariables(params, descriptor, &variables_);
|
||||
}
|
||||
|
||||
AccessorMessageFieldGenerator::~AccessorMessageFieldGenerator() {}
|
||||
|
||||
void AccessorMessageFieldGenerator::
|
||||
GenerateMembers(io::Printer* printer) const {
|
||||
printer->Print(variables_,
|
||||
"private $type$ $name$_;\n"
|
||||
"public $type$ get$capitalized_name$() {\n"
|
||||
" return $name$_;\n"
|
||||
"}\n"
|
||||
"public $message_name$ set$capitalized_name$($type$ value) {\n"
|
||||
" if (value == null) {\n"
|
||||
" throw new java.lang.NullPointerException();\n"
|
||||
" }\n"
|
||||
" $name$_ = value;\n"
|
||||
" return this;\n"
|
||||
"}\n"
|
||||
"public boolean has$capitalized_name$() {\n"
|
||||
" return $name$_ != null;\n"
|
||||
"}\n"
|
||||
"public $message_name$ clear$capitalized_name$() {\n"
|
||||
" $name$_ = null;\n"
|
||||
" return this;"
|
||||
"}\n");
|
||||
}
|
||||
|
||||
void AccessorMessageFieldGenerator::
|
||||
GenerateClearCode(io::Printer* printer) const {
|
||||
printer->Print(variables_,
|
||||
"$name$_ = null;\n");
|
||||
}
|
||||
|
||||
void AccessorMessageFieldGenerator::
|
||||
GenerateMergingCode(io::Printer* printer) const {
|
||||
printer->Print(variables_,
|
||||
"if ($name$_ == null) {\n"
|
||||
" $name$_ = new $type$();\n"
|
||||
"}\n");
|
||||
|
||||
if (descriptor_->type() == FieldDescriptor::TYPE_GROUP) {
|
||||
printer->Print(variables_,
|
||||
"input.readGroup($name$_, $number$);\n");
|
||||
} else {
|
||||
printer->Print(variables_,
|
||||
"input.readMessage($name$_);\n");
|
||||
}
|
||||
}
|
||||
|
||||
void AccessorMessageFieldGenerator::
|
||||
GenerateSerializationCode(io::Printer* printer) const {
|
||||
printer->Print(variables_,
|
||||
"if ($name$_ != null) {\n"
|
||||
" output.write$group_or_message$($number$, $name$_);\n"
|
||||
"}\n");
|
||||
}
|
||||
|
||||
void AccessorMessageFieldGenerator::
|
||||
GenerateSerializedSizeCode(io::Printer* printer) const {
|
||||
printer->Print(variables_,
|
||||
"if ($name$_ != null) {\n"
|
||||
" size += com.google.protobuf.nano.CodedOutputByteBufferNano\n"
|
||||
" .compute$group_or_message$Size($number$, $name$_);\n"
|
||||
"}\n");
|
||||
}
|
||||
|
||||
void AccessorMessageFieldGenerator::
|
||||
GenerateEqualsCode(io::Printer* printer) const {
|
||||
printer->Print(variables_,
|
||||
"if ($name$_ == null) {\n"
|
||||
" if (other.$name$_ != null) {\n"
|
||||
" return false;\n"
|
||||
" }\n"
|
||||
"} else if (!$name$_.equals(other.$name$_)) {\n"
|
||||
" return false;\n"
|
||||
"}\n");
|
||||
}
|
||||
|
||||
void AccessorMessageFieldGenerator::
|
||||
GenerateHashCodeCode(io::Printer* printer) const {
|
||||
printer->Print(variables_,
|
||||
"result = 31 * result + ($name$_ == null ? 0 : $name$_.hashCode());\n");
|
||||
}
|
||||
|
||||
// ===================================================================
|
||||
|
||||
RepeatedMessageFieldGenerator::
|
||||
RepeatedMessageFieldGenerator(const FieldDescriptor* descriptor, const Params& params)
|
||||
: FieldGenerator(params), descriptor_(descriptor) {
|
||||
|
||||
@ -65,28 +65,6 @@ class MessageFieldGenerator : public FieldGenerator {
|
||||
GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(MessageFieldGenerator);
|
||||
};
|
||||
|
||||
class AccessorMessageFieldGenerator : public FieldGenerator {
|
||||
public:
|
||||
explicit AccessorMessageFieldGenerator(
|
||||
const FieldDescriptor* descriptor, const Params& params);
|
||||
~AccessorMessageFieldGenerator();
|
||||
|
||||
// implements FieldGenerator ---------------------------------------
|
||||
void GenerateMembers(io::Printer* printer) const;
|
||||
void GenerateClearCode(io::Printer* printer) const;
|
||||
void GenerateMergingCode(io::Printer* printer) const;
|
||||
void GenerateSerializationCode(io::Printer* printer) const;
|
||||
void GenerateSerializedSizeCode(io::Printer* printer) const;
|
||||
void GenerateEqualsCode(io::Printer* printer) const;
|
||||
void GenerateHashCodeCode(io::Printer* printer) const;
|
||||
|
||||
private:
|
||||
const FieldDescriptor* descriptor_;
|
||||
map<string, string> variables_;
|
||||
|
||||
GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(AccessorMessageFieldGenerator);
|
||||
};
|
||||
|
||||
class RepeatedMessageFieldGenerator : public FieldGenerator {
|
||||
public:
|
||||
explicit RepeatedMessageFieldGenerator(const FieldDescriptor* descriptor,
|
||||
|
||||
Loading…
Reference in New Issue
Block a user