Skip to content

Commit 5dceee5

Browse files
scotthartscotthart
authored
impl(generator): finish package naming support (#11384)
1 parent beb4694 commit 5dceee5

6 files changed

Lines changed: 39 additions & 30 deletions

File tree

‎generator/internal/discovery_file.cc‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
#include "generator/internal/discovery_file.h"
1616
#include "generator/internal/codegen_utils.h"
1717
#include "google/cloud/internal/absl_str_join_quiet.h"
18-
#include "google/cloud/internal/make_status.h"
1918
#include "absl/strings/str_format.h"
2019
#include <google/protobuf/io/printer.h>
2120
#include <google/protobuf/io/zero_copy_stream_impl.h>
@@ -98,17 +97,14 @@ package $package_name$;
9897

9998
Status DiscoveryFile::WriteFile(
10099
std::string const& product_name,
101-
std::map<std::string, DiscoveryTypeVertex> const* types) const {
102-
if (types == nullptr) {
103-
return internal::InternalError("types is nullptr");
104-
}
100+
std::map<std::string, DiscoveryTypeVertex> const& types) const {
105101
std::string version_dir_path = file_path_.substr(0, file_path_.rfind('/'));
106102
std::string service_dir_path =
107103
version_dir_path.substr(0, version_dir_path.rfind('/'));
108104
MakeDirectory(service_dir_path);
109105
MakeDirectory(version_dir_path);
110106
std::ofstream os(file_path_);
111-
return FormatFile(product_name, *types, os);
107+
return FormatFile(product_name, types, os);
112108
}
113109

114110
} // namespace generator_internal

‎generator/internal/discovery_file.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ class DiscoveryFile {
5656
// Creates necessary directories and writes the file to disk.
5757
Status WriteFile(
5858
std::string const& product_name,
59-
std::map<std::string, DiscoveryTypeVertex> const* types) const;
59+
std::map<std::string, DiscoveryTypeVertex> const& types) const;
6060

6161
private:
6262
DiscoveryResource const* resource_;

‎generator/internal/discovery_resource.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ class DiscoveryResource {
3333
std::string const& name() const { return name_; }
3434
std::string const& package_name() const { return package_name_; }
3535
nlohmann::json const& json() const { return json_; }
36+
std::map<std::string, DiscoveryTypeVertex const*> const& response_types()
37+
const {
38+
return response_types_;
39+
}
3640

3741
void AddRequestType(std::string name, DiscoveryTypeVertex const* type);
3842

‎generator/internal/discovery_to_proto.cc‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -152,24 +152,26 @@ std::map<std::string, DiscoveryResource> ExtractResources(
152152

153153
// The DiscoveryResource& parameter will be used later to help determine what
154154
// protobuf files need to be imported to provide the response message.
155-
StatusOr<std::string> DetermineAndVerifyResponseTypeName(
155+
StatusOr<DiscoveryTypeVertex const*> DetermineAndVerifyResponseType(
156156
nlohmann::json const& method_json, DiscoveryResource&,
157157
std::map<std::string, DiscoveryTypeVertex>& types) {
158158
std::string response_type_name;
159+
DiscoveryTypeVertex const* response_type = nullptr;
159160
auto const& response_iter = method_json.find("response");
160161
if (response_iter != method_json.end()) {
161162
auto const& ref_iter = response_iter->find("$ref");
162163
if (ref_iter == response_iter->end()) {
163164
return internal::InvalidArgumentError("Missing $ref field in response");
164165
}
165166
response_type_name = *ref_iter;
166-
auto const& response_type = types.find(response_type_name);
167-
if (response_type == types.end()) {
167+
auto const& iter = types.find(response_type_name);
168+
if (iter == types.end()) {
168169
return internal::InvalidArgumentError(absl::StrFormat(
169170
"Response name=%s not found in types", response_type_name));
170171
}
172+
response_type = &iter->second;
171173
}
172-
return response_type_name;
174+
return response_type;
173175
}
174176

175177
StatusOr<DiscoveryTypeVertex> SynthesizeRequestType(
@@ -243,20 +245,26 @@ Status ProcessMethodRequestsAndResponses(
243245
std::string resource_name = CapitalizeFirstLetter(resource.first);
244246
auto const& methods = resource.second.json().find("methods");
245247
for (auto m = methods->begin(); m != methods->end(); ++m) {
246-
auto response_type_name =
247-
DetermineAndVerifyResponseTypeName(*m, resource.second, types);
248-
if (!response_type_name) {
249-
return Status(response_type_name.status().code(),
250-
response_type_name.status().message(),
248+
auto response_type =
249+
DetermineAndVerifyResponseType(*m, resource.second, types);
250+
if (!response_type) {
251+
return Status(response_type.status().code(),
252+
response_type.status().message(),
251253
GCP_ERROR_INFO()
252254
.WithMetadata("resource", resource.first)
253255
.WithMetadata("method", m.key())
254-
.Build(response_type_name.status().code()));
256+
.Build(response_type.status().code()));
257+
}
258+
std::string response_type_name;
259+
if (*response_type != nullptr) {
260+
response_type_name = (*response_type)->name();
261+
DiscoveryTypeVertex const* node = *response_type;
262+
resource.second.AddResponseType(response_type_name, node);
255263
}
256264

257265
if (m->contains("parameters")) {
258266
auto request_type = SynthesizeRequestType(*m, resource.second,
259-
*response_type_name, m.key());
267+
response_type_name, m.key());
260268
if (!request_type) return std::move(request_type).status();
261269
// It is necessary to add the resource name to the map key to
262270
// disambiguate methods that appear in more than one resource.
@@ -288,7 +296,7 @@ std::vector<DiscoveryFile> CreateFilesFromResources(
288296
document_properties.version, output_path),
289297
r.second.package_name(), document_properties.version,
290298
r.second.GetRequestTypesList()}
291-
.AddImportPath("/google/cloud/$product_name$/$version$/"
299+
.AddImportPath("google/cloud/$product_name$/$version$/"
292300
"internal/common.proto"));
293301
}
294302
return files;

‎generator/internal/discovery_to_proto.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ std::map<std::string, DiscoveryResource> ExtractResources(
4848

4949
// Determines the name of the response type for each method and verifies it
5050
// exists in the collection of DiscoveryTypeVertex objects.
51-
StatusOr<std::string> DetermineAndVerifyResponseTypeName(
51+
StatusOr<DiscoveryTypeVertex const*> DetermineAndVerifyResponseType(
5252
nlohmann::json const& method_json, DiscoveryResource& resource,
5353
std::map<std::string, DiscoveryTypeVertex>& types);
5454

‎generator/internal/discovery_to_proto_test.cc‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,14 @@ namespace generator_internal {
2323
namespace {
2424

2525
using ::google::cloud::testing_util::IsOk;
26+
using ::google::cloud::testing_util::IsOkAndHolds;
2627
using ::google::cloud::testing_util::StatusIs;
2728
using ::testing::AllOf;
2829
using ::testing::Contains;
2930
using ::testing::Eq;
3031
using ::testing::HasSubstr;
3132
using ::testing::IsEmpty;
33+
using ::testing::IsNull;
3234
using ::testing::Key;
3335
using ::testing::SizeIs;
3436
using ::testing::UnorderedElementsAre;
@@ -213,10 +215,9 @@ TEST(DetermineAndVerifyResponseTypeNameTest, ResponseWithRef) {
213215
DiscoveryResource resource;
214216
std::map<std::string, DiscoveryTypeVertex> types;
215217
types.emplace("Foo", DiscoveryTypeVertex{"Foo", "", response_type_json});
216-
auto response =
217-
DetermineAndVerifyResponseTypeName(method_json, resource, types);
218+
auto response = DetermineAndVerifyResponseType(method_json, resource, types);
218219
ASSERT_STATUS_OK(response);
219-
EXPECT_THAT(*response, Eq("Foo"));
220+
EXPECT_THAT((*response)->name(), Eq("Foo"));
220221
}
221222

222223
TEST(DetermineAndVerifyResponseTypeNameTest, ResponseMissingRef) {
@@ -233,8 +234,7 @@ TEST(DetermineAndVerifyResponseTypeNameTest, ResponseMissingRef) {
233234
DiscoveryResource resource;
234235
std::map<std::string, DiscoveryTypeVertex> types;
235236
types.emplace("Foo", DiscoveryTypeVertex{"Foo", "", response_type_json});
236-
auto response =
237-
DetermineAndVerifyResponseTypeName(method_json, resource, types);
237+
auto response = DetermineAndVerifyResponseType(method_json, resource, types);
238238
EXPECT_THAT(response, StatusIs(StatusCode::kInvalidArgument,
239239
HasSubstr("Missing $ref field in response")));
240240
}
@@ -250,10 +250,8 @@ TEST(DetermineAndVerifyResponseTypeNameTest, ResponseFieldMissing) {
250250
DiscoveryResource resource;
251251
std::map<std::string, DiscoveryTypeVertex> types;
252252
types.emplace("Foo", DiscoveryTypeVertex{"Foo", "", response_type_json});
253-
auto response =
254-
DetermineAndVerifyResponseTypeName(method_json, resource, types);
255-
ASSERT_STATUS_OK(response);
256-
EXPECT_THAT(*response, IsEmpty());
253+
auto response = DetermineAndVerifyResponseType(method_json, resource, types);
254+
EXPECT_THAT(response, IsOkAndHolds(IsNull()));
257255
}
258256

259257
TEST(SynthesizeRequestTypeTest, OperationResponseWithRefRequestField) {
@@ -603,11 +601,14 @@ TEST(ProcessMethodRequestsAndResponsesTest, RequestWithOperationResponse) {
603601
resources.emplace("foos",
604602
DiscoveryResource("foos", "", "", "", resource_json));
605603
std::map<std::string, DiscoveryTypeVertex> types;
606-
types.emplace("Operation", DiscoveryTypeVertex("", "", operation_type_json));
604+
types.emplace("Operation",
605+
DiscoveryTypeVertex("Operation", "", operation_type_json));
607606
auto result = ProcessMethodRequestsAndResponses(resources, types);
608607
ASSERT_STATUS_OK(result);
609608
EXPECT_THAT(
610609
types, UnorderedElementsAre(Key("Foos.CreateRequest"), Key("Operation")));
610+
EXPECT_THAT(resources.begin()->second.response_types(),
611+
UnorderedElementsAre(Key("Operation")));
611612
}
612613

613614
TEST(ProcessMethodRequestsAndResponsesTest, MethodWithEmptyRequest) {

0 commit comments

Comments
 (0)