[
https://issues.apache.org/jira/browse/BEAM-12736?focusedWorklogId=650872&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-650872
]
ASF GitHub Bot logged work on BEAM-12736:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 14/Sep/21 23:48
Start Date: 14/Sep/21 23:48
Worklog Time Spent: 10m
Work Description: TheNeuralBit commented on a change in pull request
#15306:
URL: https://github.com/apache/beam/pull/15306#discussion_r708735872
##########
File path:
sdks/java/extensions/protobuf/src/test/proto/proto3_schema_messages.proto
##########
@@ -51,6 +51,24 @@ message Primitive {
bytes primitive_bytes = 15;
}
+message PrimitiveCamel {
+ double primitiveDoubleCamel = 1;
+ float primitiveFloatCamel = 2;
+ int32 primitiveInt32Camel = 3;
+ int64 primitiveInt64Camel = 4;
+ uint32 primitiveUint32Camel = 5;
+ uint64 primitiveUint64Camel = 6;
+ sint32 primitiveSint32Camel = 7;
+ sint64 primitiveSint64Camel = 8;
+ fixed32 primitiveFixed32Camel = 9;
+ fixed64 primitiveFixed64Camel = 10;
+ sfixed32 primitiveSfixed32Camel = 11;
+ sfixed64 primitiveSfixed64Camel = 12;
+ bool primitiveBoolCamel = 13;
+ string primitiveStringCamel = 14;
+ bytes primitiveBytesCamel = 15;
+}
Review comment:
Could you add another test case to make sure we exercise the oneof logic
for camel case?
##########
File path:
sdks/java/extensions/protobuf/src/main/java/org/apache/beam/sdk/extensions/protobuf/ProtoByteBuddyUtils.java
##########
@@ -190,18 +191,30 @@
private static final String DEFAULT_PROTO_GETTER_PREFIX = "get";
private static final String DEFAULT_PROTO_SETTER_PREFIX = "set";
- static String protoGetterName(String name, FieldType fieldType) {
+ static String protoGetterNameFromSnake(String name, FieldType fieldType) {
final String camel =
CaseFormat.LOWER_UNDERSCORE.to(CaseFormat.UPPER_CAMEL, name);
return DEFAULT_PROTO_GETTER_PREFIX
+ camel
+ PROTO_GETTER_SUFFIX.getOrDefault(fieldType.getTypeName(), "");
}
- static String protoSetterName(String name, FieldType fieldType) {
+ static String protoSetterNameFromSnake(String name, FieldType fieldType) {
Review comment:
What do you think about adding a `CaseFormat` parameter to
`protoSetterName` instead of having a function for each format?
```suggestion
static String protoSetterName(String name, FieldType fieldType, CaseFormat
caseFormat) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 650872)
Remaining Estimate: 22h 50m (was: 23h)
Time Spent: 1h 10m (was: 1h)
> Protobuf schema provider row functions break on camel-case field names
> ----------------------------------------------------------------------
>
> Key: BEAM-12736
> URL: https://issues.apache.org/jira/browse/BEAM-12736
> Project: Beam
> Issue Type: Bug
> Components: extensions-java-protobuf
> Affects Versions: 2.31.0
> Reporter: Chris Hinds
> Assignee: Chris Hinds
> Priority: P2
> Original Estimate: 24h
> Time Spent: 1h 10m
> Remaining Estimate: 22h 50m
>
> ProtoByteBuddyUtils.protoGetterName() _depends_ on field names being
> snake-case. But the Protobuf style guide only _recommends_ that field names
> are so defined.
> Snake-case is not enforced by protoc and my team have always created proto
> field names in camel-case (perhaps we didn't understand that protoc would
> automatically rewrite field names for us). It is likely that we are not alone.
> If one calls a row function against a proto instance whose field were defined
> in camel-case, an IllegalArgumentException results from the
> ProtoByteBuddyUtils snake-case assumption.
> {code:java}
> SerializableFunction myRowFunction = new
> ProtoMessageSchema().toRowFunction(new
> TypeDescriptor<MyDataModel.ProtoPayload>() {});
> MyDataModel.ProtoPayload payload = …
> Row row = (Row) myRowFunction.apply(payload);
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
