Skip to content

Commit 8484dab

Browse files
gorzellgorzell
authored
Merge pull request #4 from github/gorzell/multi-file-java
Fix a number of cases that were not handled in the TypeMapper.
2 parents 22f2913 + 9e35542 commit 8484dab

File tree

4 files changed

+269
-35
lines changed

4 files changed

+269
-35
lines changed

‎plugin/gradle.properties

+1-1
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
version=1.0.0
1+
version=1.1.0

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

+61-34
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

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

‎runtime/core/gradle.properties

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
version=1.0.0

0 commit comments

Comments
 (0)