Skip to content

Commit 192887a

Browse files
miraleungmiraleung
authored
[ggj][codegen][bazel] fix!: use key=value pairs for the plugin arguments (for Bazel) (#300)
* fix: refactor and rename Bazel rules to use googleapis' toolchain * fix: simplify Bazel rules post-refactoring * fix: rename protoc plugin binary * fix!: use key=value pairs for the plugin arguments (for Bazel)
1 parent dcf91f6 commit 192887a

3 files changed

Lines changed: 101 additions & 26 deletions

File tree

‎run.sh‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,25 @@ then
7272
echo_success "Done"
7373
fi
7474

75+
# Key values are synced to rules_java_gapic/java_gapic.bzl.
76+
SERVICE_CONFIG_OPT=""
77+
if [ -n "$FLAGS_service_config" ]
78+
then
79+
SERVICE_CONFIG_OPT="grpc-service-config=$FLAGS_service_config"
80+
fi
81+
GAPIC_CONFIG_OPT=""
82+
if [ -n "$FLAGS_gapic_config" ]
83+
then
84+
GAPIC_CONFIG_OPT="gapic-config=$FLAGS_gapic_config"
85+
fi
86+
7587
# Run protoc.
7688
protoc -I="${PROTOC_INCLUDE_DIR}" -I="${FLAGS_googleapis}" -I="${FLAGS_protos}" \
7789
-I="${FLAGS_googleapis}/google/longrunning" \
90+
--include_source_info \
7891
--plugin=bazel-bin/protoc-gen-java_gapic ${FLAGS_protos}/*.proto \
7992
--java_gapic_out="${FLAGS_out}" \
80-
--java_gapic_opt="${FLAGS_service_config},${FLAGS_gapic_config}" \
93+
--java_gapic_opt="${SERVICE_CONFIG_OPT},${GAPIC_CONFIG_OPT}" \
8194
--experimental_allow_proto3_optional
8295

8396
echo_success "Output files written to ${FLAGS_out}"

‎src/main/java/com/google/api/generator/gapic/protoparser/PluginArgumentParser.java‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,36 +22,48 @@
2222
// Parses the arguments from the protoc plugin.
2323
public class PluginArgumentParser {
2424
private static final String COMMA = ",";
25+
private static final String EQUALS = "=";
26+
27+
// Synced to rules_java_gapic/java_gapic.bzl.
28+
@VisibleForTesting static final String KEY_GRPC_SERVICE_CONFIG = "grpc-service-config";
29+
@VisibleForTesting static final String KEY_GAPIC_CONFIG = "gapic-config";
30+
2531
private static final String JSON_FILE_ENDING = "grpc_service_config.json";
2632
private static final String GAPIC_YAML_FILE_ENDING = "gapic.yaml";
2733

28-
public static Optional<String> parseJsonConfigPath(CodeGeneratorRequest request) {
34+
static Optional<String> parseJsonConfigPath(CodeGeneratorRequest request) {
2935
return parseJsonConfigPath(request.getParameter());
3036
}
3137

32-
public static Optional<String> parseGapicYamlConfigPath(CodeGeneratorRequest request) {
38+
static Optional<String> parseGapicYamlConfigPath(CodeGeneratorRequest request) {
3339
return parseGapicYamlConfigPath(request.getParameter());
3440
}
3541

3642
/** Expects a comma-separated list of file paths. */
3743
@VisibleForTesting
3844
static Optional<String> parseJsonConfigPath(String pluginProtocArgument) {
39-
return parseArgument(pluginProtocArgument, JSON_FILE_ENDING);
45+
return parseArgument(pluginProtocArgument, KEY_GRPC_SERVICE_CONFIG, JSON_FILE_ENDING);
4046
}
4147

4248
@VisibleForTesting
4349
static Optional<String> parseGapicYamlConfigPath(String pluginProtocArgument) {
44-
return parseArgument(pluginProtocArgument, GAPIC_YAML_FILE_ENDING);
50+
return parseArgument(pluginProtocArgument, KEY_GAPIC_CONFIG, GAPIC_YAML_FILE_ENDING);
4551
}
4652

47-
private static Optional<String> parseArgument(String pluginProtocArgument, String fileEnding) {
53+
private static Optional<String> parseArgument(
54+
String pluginProtocArgument, String key, String fileEnding) {
4855
if (Strings.isNullOrEmpty(pluginProtocArgument)) {
4956
return Optional.<String>empty();
5057
}
51-
for (String rawPath : pluginProtocArgument.split(COMMA)) {
52-
String path = rawPath.trim();
53-
if (path.endsWith(fileEnding)) {
54-
return Optional.of(path);
58+
for (String argComponent : pluginProtocArgument.split(COMMA)) {
59+
String[] args = argComponent.trim().split(EQUALS);
60+
if (args.length < 2) {
61+
continue;
62+
}
63+
String keyVal = args[0];
64+
String valueVal = args[1];
65+
if (keyVal.equals(key) && valueVal.endsWith(fileEnding)) {
66+
return Optional.of(valueVal);
5567
}
5668
}
5769
return Optional.<String>empty();

‎src/test/java/com/google/api/generator/gapic/protoparser/PluginArgumentParserTest.java‎

Lines changed: 66 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,28 @@
2121
import org.junit.Test;
2222

2323
public class PluginArgumentParserTest {
24+
2425
@Test
2526
public void parseJsonPath_onlyOnePresent() {
2627
String jsonPath = "/tmp/grpc_service_config.json";
27-
assertEquals(jsonPath, PluginArgumentParser.parseJsonConfigPath(jsonPath).get());
28+
assertEquals(
29+
jsonPath,
30+
PluginArgumentParser.parseJsonConfigPath(createGrpcServiceConfig(jsonPath)).get());
31+
}
32+
33+
@Test
34+
public void parseJsonPath_returnsFirstOneFound() {
35+
String jsonPathOne = "/tmp/foobar_grpc_service_config.json";
36+
String jsonPathTwo = "/tmp/some_other_grpc_service_config.json";
37+
assertEquals(
38+
jsonPathOne,
39+
PluginArgumentParser.parseJsonConfigPath(
40+
String.join(
41+
",",
42+
Arrays.asList(
43+
createGrpcServiceConfig(jsonPathOne),
44+
createGrpcServiceConfig(jsonPathTwo))))
45+
.get());
2846
}
2947

3048
@Test
@@ -35,11 +53,11 @@ public void parseJsonPath_similarFileAppearsFirst() {
3553
String.join(
3654
",",
3755
Arrays.asList(
38-
gapicPath,
39-
"/tmp/something.json",
40-
"/tmp/some_grpc_service_configjson",
41-
jsonPath,
42-
gapicPath));
56+
createGapicConfig(gapicPath),
57+
createGrpcServiceConfig("/tmp/something.json"),
58+
createGrpcServiceConfig("/tmp/some_grpc_service_configjson"),
59+
createGrpcServiceConfig(jsonPath),
60+
createGapicConfig(gapicPath)));
4361
assertEquals(jsonPath, PluginArgumentParser.parseJsonConfigPath(rawArgument).get());
4462
}
4563

@@ -51,19 +69,20 @@ public void parseJsonPath_argumentHasSpaces() {
5169
String.join(
5270
" , ",
5371
Arrays.asList(
54-
gapicPath,
55-
"/tmp/something.json",
56-
"/tmp/some_grpc_service_configjson",
57-
jsonPath,
58-
gapicPath));
72+
createGapicConfig(gapicPath),
73+
createGrpcServiceConfig("/tmp/something.json"),
74+
createGrpcServiceConfig("/tmp/some_grpc_service_configjson"),
75+
createGrpcServiceConfig(jsonPath),
76+
createGapicConfig(gapicPath)));
5977
assertEquals(jsonPath, PluginArgumentParser.parseJsonConfigPath(rawArgument).get());
6078
}
6179

6280
@Test
6381
public void parseJsonPath_restAreEmpty() {
6482
String jsonPath = "/tmp/foobar_grpc_service_config.json";
6583
String gapicPath = "";
66-
String rawArgument = String.join(",", Arrays.asList(gapicPath, jsonPath, gapicPath));
84+
String rawArgument =
85+
String.join(",", Arrays.asList(gapicPath, createGrpcServiceConfig(jsonPath), gapicPath));
6786
assertEquals(jsonPath, PluginArgumentParser.parseJsonConfigPath(rawArgument).get());
6887
}
6988

@@ -77,7 +96,23 @@ public void parseJsonPath_noneFound() {
7796
@Test
7897
public void parseGapicYamlPath_onlyOnePresent() {
7998
String gapicPath = "/tmp/something_gapic.yaml";
80-
assertEquals(gapicPath, PluginArgumentParser.parseGapicYamlConfigPath(gapicPath).get());
99+
assertEquals(
100+
gapicPath,
101+
PluginArgumentParser.parseGapicYamlConfigPath(createGapicConfig(gapicPath)).get());
102+
}
103+
104+
@Test
105+
public void parseGapicYamlPath_returnsFirstOneFound() {
106+
String gapicPathOne = "/tmp/something_gapic.yaml";
107+
String gapicPathTwo = "/tmp/other_gapic.yaml";
108+
assertEquals(
109+
gapicPathOne,
110+
PluginArgumentParser.parseGapicYamlConfigPath(
111+
String.join(
112+
",",
113+
Arrays.asList(
114+
createGapicConfig(gapicPathOne), createGapicConfig(gapicPathTwo))))
115+
.get());
81116
}
82117

83118
@Test
@@ -86,23 +121,38 @@ public void parseGapicYamlPath_similarFileAppearsFirst() {
86121
String gapicPath = "/tmp/something_gapic.yaml";
87122
String rawArgument =
88123
String.join(
89-
",", Arrays.asList(jsonPath, "/tmp/something.yaml", "/tmp/some_gapicyaml", gapicPath));
124+
",",
125+
Arrays.asList(
126+
createGrpcServiceConfig(jsonPath),
127+
createGapicConfig("/tmp/something.yaml"),
128+
createGapicConfig("/tmp/some_gapicyaml"),
129+
createGapicConfig(gapicPath)));
90130
assertEquals(gapicPath, PluginArgumentParser.parseGapicYamlConfigPath(rawArgument).get());
91131
}
92132

93133
@Test
94134
public void parseGapicYamlPath_restAreEmpty() {
95135
String jsonPath = "";
96136
String gapicPath = "/tmp/something_gapic.yaml";
97-
String rawArgument = String.join(",", Arrays.asList(jsonPath, gapicPath, jsonPath));
137+
String rawArgument =
138+
String.join(",", Arrays.asList(jsonPath, createGapicConfig(gapicPath), jsonPath));
98139
assertEquals(gapicPath, PluginArgumentParser.parseGapicYamlConfigPath(rawArgument).get());
99140
}
100141

101142
@Test
102143
public void parseGapicYamlPath_noneFound() {
103144
String jsonPath = "/tmp/foo_grpc_service_config.json";
104145
String gapicPath = "";
105-
String rawArgument = String.join(",", Arrays.asList(jsonPath, gapicPath));
146+
String rawArgument =
147+
String.join(",", Arrays.asList(createGrpcServiceConfig(jsonPath), gapicPath));
106148
assertFalse(PluginArgumentParser.parseGapicYamlConfigPath(rawArgument).isPresent());
107149
}
150+
151+
private static String createGrpcServiceConfig(String path) {
152+
return String.format("%s=%s", PluginArgumentParser.KEY_GRPC_SERVICE_CONFIG, path);
153+
}
154+
155+
private static String createGapicConfig(String path) {
156+
return String.format("%s=%s", PluginArgumentParser.KEY_GAPIC_CONFIG, path);
157+
}
108158
}

0 commit comments

Comments
 (0)