googleapis/gapic-generator

Multiple *Name.java classes for a single resource

aohren opened this issue · 1 comments

If we have a proto package hierarchy as follows (i.e., the googleads project)

├── common
├── enums
├── errors
├── resources
│   └── campaign.proto
└── services
    └── campaign_service.proto

where campaign_service.proto has rpc methods that depend on campaign.proto like GetCampaign:

  rpc GetCampaign(GetCampaignRequest) returns (google.ads.googleads.v3.resources.Campaign) {
    option (google.api.http) = {
      get: "/v3/{resource_name=customers/*/campaigns/*}"
    };
    option (google.api.method_signature) = "resource_name";
  }

Then, gapic-generator generates many CampaignName classes, one in each package:

src/main/java/com/google/ads/googleads/v3/resources/CampaignName.java
src/main/java/com/google/ads/googleads/v3/enums/CampaignName.java
src/main/java/com/google/ads/googleads/v3/common/CampaignName.java
src/main/java/com/google/ads/googleads/v3/errors/CampaignName.java
src/main/java/com/google/ads/googleads/v3/services/CampaignName.java

We'd like to delete all except either the one in services or resources, so that the client code is less confusing to users.

However, the generated test classes depend on both services/CampaignName.java and resources/CampaignName.java, so we have to keep both copies so that test compile. For example, src/test/java/com/google/ads/googleads/v3/services/CampaignServiceClientTest.java:

  @Test
  @SuppressWarnings("all")
  public void getCampaignTest() {
    com.google.ads.googleads.v3.resources.CampaignName resourceName2 =
        com.google.ads.googleads.v3.resources.CampaignName.of("[CUSTOMER]", "[CAMPAIGN]");
    Campaign expectedResponse =
        Campaign.newBuilder().setResourceName(resourceName2.toString()).build();
    mockCampaignService.addResponse(expectedResponse);

    CampaignName resourceName = CampaignName.of("[CUSTOMER]", "[CAMPAIGN]");

    Campaign actualResponse = client.getCampaign(resourceName);
    Assert.assertEquals(expectedResponse, actualResponse);

    List<AbstractMessage> actualRequests = mockCampaignService.getRequests();
    Assert.assertEquals(1, actualRequests.size());
    GetCampaignRequest actualRequest = (GetCampaignRequest) actualRequests.get(0);

    Assert.assertEquals(resourceName, CampaignName.parse(actualRequest.getResourceName()));
    Assert.assertTrue(
        channelProvider.isHeaderSent(
            ApiClientHeaderProvider.getDefaultApiClientHeaderKey(),
            GaxGrpcProperties.getDefaultApiClientHeaderPattern()));
  }

Both the method argument and the return type are annotated as type googleads.googleapis.com/Campaign, so I'd think there's enough semantic info to resolve them to a single entity. Is there a way to get around requiring both copies of CampaignName object in the generated code?

Fixed and verified to be working in the microgenerator.