Skip to content

Commit 5856d09

Browse files
gorzellgorzell
committed
Add tests to the TypeMapper and better super the various proto options that exist for Java.
1 parent 22f2913 commit 5856d09

File tree

2 files changed

+268
-34
lines changed

2 files changed

+268
-34
lines changed

‎plugin/src/main/java/com/flit/protoc/gen/server/TypeMapper.java

Lines changed: 61 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,14 @@
22

33
import com.google.protobuf.DescriptorProtos;
44
import com.squareup.javapoet.ClassName;
5-
65
import java.io.File;
76
import java.util.HashMap;
87
import java.util.List;
98
import java.util.Map;
109

1110
public class TypeMapper {
1211

13-
// holds the qualified class name to the short class reference
12+
// Maps the protobuf package and name to the fully qualified name of the generated Java class.
1413
private final Map<String, String> mapping = new HashMap<>();
1514

1615
public TypeMapper() {
@@ -22,57 +21,85 @@ public TypeMapper(List<DescriptorProtos.FileDescriptorProto> files) {
2221

2322
public void add(DescriptorProtos.FileDescriptorProto proto) {
2423
proto.getMessageTypeList().forEach(m -> {
25-
mapping.put("." + proto.getPackage() + "." + m.getName(), getClassname(proto) + "." + m.getName());
24+
mapping.put("." + proto.getPackage() + "." + m.getName(),
25+
getOuterClassOrPackageName(proto) + "." + m.getName());
2626
});
2727
}
2828

2929
public ClassName get(String protobufFqcn) {
3030
return ClassName.bestGuess(mapping.get(protobufFqcn));
3131
}
3232

33-
public static String getClassname(DescriptorProtos.FileDescriptorProto proto) {
34-
String clazz = proto.getOptions().getJavaOuterClassname();
33+
/**
34+
* Determine where message or service in a given proto file will be generated. Depending on the
35+
* java specific options in the spec, this could be either inside of an outer class, or at the top
36+
* level of the package.
37+
*/
38+
public static String getOuterClassOrPackageName(DescriptorProtos.FileDescriptorProto proto) {
39+
// If no 'java_package' option is provided, the protoc compiler will default to the protobuf
40+
// package name.
41+
String packageName = proto.getOptions().hasJavaPackage() ?
42+
proto.getOptions().getJavaPackage() : proto.getPackage();
43+
44+
// If this option is enabled protoc will generate a class for each message/service at the top
45+
// level of the given package space. Because message name is appended in the add method, this
46+
// should just return the package in that case. If there are collisions protoc should give a
47+
// warning/error.
48+
if (proto.getOptions().getJavaMultipleFiles()) {
49+
return packageName;
50+
}
3551

36-
if (clazz == null || clazz.isEmpty()) {
52+
// If an outer class name is provided it should be used, otherwise we need to infer one based
53+
// on the same rules the protoc compiler uses.
54+
String outerClass = proto.getOptions().hasJavaOuterClassname() ?
55+
proto.getOptions().getJavaOuterClassname() : outerClassNameFromProtoName(proto);
3756

38-
String basename = new File(proto.getName()).getName();
39-
char[] classname = basename.substring(0, basename.lastIndexOf('.')).toCharArray();
40-
StringBuilder sb = new StringBuilder();
57+
if (outerClass.isEmpty()) {
58+
throw new IllegalArgumentException("'option java_outer_classname' cannot be set to \"\".");
59+
}
4160

42-
char previous = '_';
43-
for (char c : classname) {
44-
if (c == '_') {
45-
previous = c;
46-
continue;
47-
}
61+
String fqName = String.join(".", packageName, outerClass);
4862

49-
if (previous == '_') {
50-
sb.append(Character.toUpperCase(c));
51-
} else {
52-
sb.append(c);
53-
}
63+
// check to see if there are any messages with this same class name as per java proto specs
64+
// note that we also check the services too as the protoc compiler does that as well.
65+
for (DescriptorProtos.DescriptorProto type : proto.getMessageTypeList()) {
66+
if (type.getName().equals(outerClass)) {
67+
return fqName + "OuterClass";
68+
}
69+
}
5470

55-
previous = c;
71+
for (DescriptorProtos.ServiceDescriptorProto service : proto.getServiceList()) {
72+
if (service.getName().equals(outerClass)) {
73+
return fqName + "OuterClass";
5674
}
75+
}
5776

58-
clazz = sb.toString();
77+
return fqName;
78+
}
79+
80+
private static String outerClassNameFromProtoName(DescriptorProtos.FileDescriptorProto proto) {
81+
String basename = new File(proto.getName()).getName();
82+
char[] classname = basename.substring(0, basename.lastIndexOf('.')).toCharArray();
83+
StringBuilder sb = new StringBuilder();
5984

60-
// check to see if there are any messages with this same class name as per java proto specs
61-
// note that we also check the services too as the protoc compiler does that as well
62-
for (DescriptorProtos.DescriptorProto type : proto.getMessageTypeList()) {
63-
if (type.getName().equals(clazz)) {
64-
return clazz + "OuterClass";
65-
}
85+
char previous = '_';
86+
for (char c : classname) {
87+
if (c == '_') {
88+
previous = c;
89+
continue;
6690
}
6791

68-
for (DescriptorProtos.ServiceDescriptorProto service : proto.getServiceList()) {
69-
if (service.getName().equals(clazz)) {
70-
return clazz + "OuterClass";
71-
}
92+
if (previous == '_') {
93+
sb.append(Character.toUpperCase(c));
94+
} else {
95+
sb.append(c);
7296
}
97+
98+
previous = c;
7399
}
74100

75-
return String.join(".", proto.getOptions().getJavaPackage(), clazz);
76-
}
101+
String clazz = sb.toString();
77102

103+
return clazz;
104+
}
78105
}

‎plugin/src/test/java/com/flit/protoc/gen/server/TypeMapperTest.java

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
package com.flit.protoc.gen.server;
2+
3+
import static org.junit.Assert.assertEquals;
4+
import static org.junit.Assert.assertNull;
5+
6+
import com.google.protobuf.DescriptorProtos;
7+
import com.squareup.javapoet.ClassName;
8+
import org.junit.Test;
9+
10+
public class TypeMapperTest {
11+
12+
private static final String PROTO_PACKAGE = "flit.test";
13+
private static final String JAVA_PACKAGE = "com.flit.test";
14+
15+
private static final DescriptorProtos.DescriptorProto MAP_MESSAGE = DescriptorProtos.DescriptorProto
16+
.newBuilder()
17+
.setName("Map")
18+
.build();
19+
20+
private static final DescriptorProtos.DescriptorProto MAPPER_MESSAGE = DescriptorProtos.DescriptorProto
21+
.newBuilder()
22+
.setName("Mapper")
23+
.build();
24+
25+
@Test
26+
public void protoPackage() {
27+
DescriptorProtos.FileOptions options = DescriptorProtos.FileOptions.newBuilder()
28+
.setJavaMultipleFiles(false)
29+
.setJavaOuterClassname("Mapper")
30+
.build();
31+
32+
DescriptorProtos.FileDescriptorProto proto = DescriptorProtos.FileDescriptorProto.newBuilder()
33+
.setPackage(PROTO_PACKAGE)
34+
.setName("mapper.proto")
35+
.setOptions(options)
36+
.addMessageType(MAP_MESSAGE)
37+
.build();
38+
39+
TypeMapper mapper = new TypeMapper();
40+
mapper.add(proto);
41+
42+
ClassName result = mapper.get(".flit.test.Map");
43+
assertEquals(PROTO_PACKAGE, result.packageName());
44+
assertEquals("Mapper", result.enclosingClassName().simpleName());
45+
assertEquals("Map", result.simpleName());
46+
}
47+
48+
@Test
49+
public void protoPackageNameCollision() {
50+
DescriptorProtos.FileOptions options = DescriptorProtos.FileOptions.newBuilder()
51+
.setJavaMultipleFiles(false)
52+
.build();
53+
54+
DescriptorProtos.FileDescriptorProto proto = DescriptorProtos.FileDescriptorProto.newBuilder()
55+
.setPackage(PROTO_PACKAGE)
56+
.setName("mapper.proto")
57+
.setOptions(options)
58+
.addMessageType(MAPPER_MESSAGE)
59+
.build();
60+
61+
TypeMapper mapper = new TypeMapper();
62+
mapper.add(proto);
63+
64+
ClassName result = mapper.get(".flit.test.Mapper");
65+
assertEquals(PROTO_PACKAGE, result.packageName());
66+
assertEquals("MapperOuterClass", result.enclosingClassName().simpleName());
67+
assertEquals("Mapper", result.simpleName());
68+
}
69+
70+
@Test
71+
public void protoPackageWithOuterClass() {
72+
DescriptorProtos.FileOptions options = DescriptorProtos.FileOptions.newBuilder()
73+
.setJavaMultipleFiles(false)
74+
.setJavaOuterClassname("Mapper")
75+
.build();
76+
77+
DescriptorProtos.FileDescriptorProto proto = DescriptorProtos.FileDescriptorProto.newBuilder()
78+
.setPackage(PROTO_PACKAGE)
79+
.setName("mapper.proto")
80+
.setOptions(options)
81+
.addMessageType(MAP_MESSAGE)
82+
.build();
83+
84+
TypeMapper mapper = new TypeMapper();
85+
mapper.add(proto);
86+
87+
ClassName result = mapper.get(".flit.test.Map");
88+
assertEquals(PROTO_PACKAGE, result.packageName());
89+
assertEquals("Mapper", result.enclosingClassName().simpleName());
90+
assertEquals("Map", result.simpleName());
91+
}
92+
93+
@Test
94+
public void javaPackage() {
95+
DescriptorProtos.FileOptions options = DescriptorProtos.FileOptions.newBuilder()
96+
.setJavaMultipleFiles(false)
97+
.setJavaPackage(JAVA_PACKAGE)
98+
.build();
99+
100+
DescriptorProtos.FileDescriptorProto proto = DescriptorProtos.FileDescriptorProto.newBuilder()
101+
.setPackage(PROTO_PACKAGE)
102+
.setName("mapper.proto")
103+
.setOptions(options)
104+
.addMessageType(MAP_MESSAGE)
105+
.build();
106+
107+
TypeMapper mapper = new TypeMapper();
108+
mapper.add(proto);
109+
110+
ClassName result = mapper.get(".flit.test.Map");
111+
assertEquals(JAVA_PACKAGE, result.packageName());
112+
assertEquals("Mapper", result.enclosingClassName().simpleName());
113+
assertEquals("Map", result.simpleName());
114+
}
115+
116+
@Test
117+
public void javaPackageNameCollision() {
118+
DescriptorProtos.FileOptions options = DescriptorProtos.FileOptions.newBuilder()
119+
.setJavaMultipleFiles(false)
120+
.setJavaPackage(JAVA_PACKAGE)
121+
.build();
122+
123+
DescriptorProtos.FileDescriptorProto proto = DescriptorProtos.FileDescriptorProto.newBuilder()
124+
.setPackage(PROTO_PACKAGE)
125+
.setName("mapper.proto")
126+
.setOptions(options)
127+
.addMessageType(MAPPER_MESSAGE)
128+
.build();
129+
130+
TypeMapper mapper = new TypeMapper();
131+
mapper.add(proto);
132+
133+
ClassName result = mapper.get(".flit.test.Mapper");
134+
assertEquals(JAVA_PACKAGE, result.packageName());
135+
assertEquals("MapperOuterClass", result.enclosingClassName().simpleName());
136+
assertEquals("Mapper", result.simpleName());
137+
}
138+
139+
@Test
140+
public void javaPackageWithOuterClass() {
141+
DescriptorProtos.FileOptions options = DescriptorProtos.FileOptions.newBuilder()
142+
.setJavaMultipleFiles(false)
143+
.setJavaOuterClassname("Mapper")
144+
.setJavaPackage(JAVA_PACKAGE)
145+
.build();
146+
147+
DescriptorProtos.FileDescriptorProto proto = DescriptorProtos.FileDescriptorProto.newBuilder()
148+
.setPackage(PROTO_PACKAGE)
149+
.setName("mapper.proto")
150+
.setOptions(options)
151+
.addMessageType(MAP_MESSAGE)
152+
.build();
153+
154+
TypeMapper mapper = new TypeMapper();
155+
mapper.add(proto);
156+
157+
ClassName result = mapper.get(".flit.test.Map");
158+
assertEquals(JAVA_PACKAGE, result.packageName());
159+
assertEquals("Mapper", result.enclosingClassName().simpleName());
160+
assertEquals("Map", result.simpleName());
161+
}
162+
163+
@Test(expected = IllegalArgumentException.class)
164+
public void javaPackageWithOuterClassEmpty() {
165+
DescriptorProtos.FileOptions options = DescriptorProtos.FileOptions.newBuilder()
166+
.setJavaMultipleFiles(false)
167+
.setJavaOuterClassname("")
168+
.setJavaPackage(JAVA_PACKAGE)
169+
.build();
170+
171+
DescriptorProtos.FileDescriptorProto proto = DescriptorProtos.FileDescriptorProto.newBuilder()
172+
.setPackage(PROTO_PACKAGE)
173+
.setName("mapper.proto")
174+
.setOptions(options)
175+
.addMessageType(MAP_MESSAGE)
176+
.build();
177+
178+
TypeMapper mapper = new TypeMapper();
179+
mapper.add(proto);
180+
181+
mapper.get(".flit.test.Map");
182+
}
183+
184+
@Test
185+
public void javaPackageWithOuterClassMultiFile() {
186+
DescriptorProtos.FileOptions options = DescriptorProtos.FileOptions.newBuilder()
187+
.setJavaMultipleFiles(true)
188+
.setJavaOuterClassname("Mapper")
189+
.setJavaPackage(JAVA_PACKAGE)
190+
.build();
191+
192+
DescriptorProtos.FileDescriptorProto proto = DescriptorProtos.FileDescriptorProto.newBuilder()
193+
.setPackage(PROTO_PACKAGE)
194+
.setName("mapper.proto")
195+
.setOptions(options)
196+
.addMessageType(MAP_MESSAGE)
197+
.build();
198+
199+
TypeMapper mapper = new TypeMapper();
200+
mapper.add(proto);
201+
202+
ClassName result = mapper.get(".flit.test.Map");
203+
assertEquals(JAVA_PACKAGE, result.packageName());
204+
assertNull(result.enclosingClassName());
205+
assertEquals("Map", result.simpleName());
206+
}
207+
}

0 commit comments

Comments
 (0)