Skip to content

feat: Do not generate Service REST code if there are no matching RPC in a Service#1236

Merged
lqiu96 merged 69 commits intomainfrom
main-rest_method_generation_2
Jan 27, 2023
Merged

feat: Do not generate Service REST code if there are no matching RPC in a Service#1236
lqiu96 merged 69 commits intomainfrom
main-rest_method_generation_2

Conversation

@lqiu96
Copy link
Member

@lqiu96 lqiu96 commented Jan 10, 2023 •
edited
Loading

This is a PR for Part 2 of Issue #1117: Do not generate any Service related REST code if there are no enabled REST RPC in a Service

Part 1 (MERGED) : #1211

What this PR attempts to do:

  • Do not generate HttpJson{Service_Name}CallableFactory (generated by AbstractServiceCallableFactoryClassComposer and the sub composers)
  • Do not generate HttpJson{Service_Name}Stub (generated by AbstractTransportServiceStubClassComposer and the sub composers)
  • Remove HttpJson references {Service_Name}StubSettings (generated by AbstractServiceStubSettingsClassComposer and the sub composers)
  • Remove HttpJson references {Service_Name}ServiceSettings (generated by AbstractServiceSettingsClassComposer and the sub composers)
  • Do not generate {Service_Name}ClientHttpJsonTest (generated by AbstractServiceClientTestClassComposer and sub composers)

Service class exposes a new hasAnyEnabledMethodsForTransport() to determine if the above logic is needed for the Transports (gRPC -> Always enabled vs HttpJson -> Needs to enabled/ eligible).

lqiu96 added 18 commits January 6, 2023 15:03
@blakeli0
Copy link
Contributor

blakeli0 commented Jan 10, 2023

Can we have a golden test(either in the unit tests or integration tests) so that we can easily compare the diff introduced by this PR?

@lqiu96
Copy link
Member Author

lqiu96 commented Jan 11, 2023

Can we have a golden test(either in the unit tests or integration tests) so that we can easily compare the diff introduced by this PR?

Yep! Working on it. I was playing around with the showcase tests but I'll sync up with Burke about getting this setup/ working.

@lqiu96 lqiu96 force-pushed the main-rest_method_generation_2 branch from c8e67bf to d7ebc76 Compare January 11, 2023 21:58
@lqiu96 lqiu96 force-pushed the main-rest_method_generation_2 branch from f380782 to 3435225 Compare January 25, 2023 15:15
@lqiu96 lqiu96 marked this pull request as ready for review January 25, 2023 15:20
@lqiu96 lqiu96 requested a review from a team January 25, 2023 15:20
@snippet-bot
Copy link

snippet-bot bot commented Jan 25, 2023 •
edited
Loading

Here is the summary of changes.

You are about to add 15 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@lqiu96
Copy link
Member Author

lqiu96 commented Jan 25, 2023 •
edited
Loading

Will need to do some more local testing to ensure if Http methods are properly generated and that I'm not breaking the generated client libraries.

Changes from showcase and goldens look good.

Sample application using apigee connect v1:

package com.google.cloud.apigeeconnect.v1;

import com.google.api.gax.rpc.BidiStream;
import com.google.cloud.apigeeconnect.v1.stub.ConnectionServiceStub;
import com.google.cloud.apigeeconnect.v1.stub.ConnectionServiceStubSettings;
import com.google.cloud.apigeeconnect.v1.stub.TetherStub;
import com.google.cloud.apigeeconnect.v1.stub.TetherStubSettings;
import com.google.rpc.Status;

import java.io.IOException;

public class Main {
    public static void main(String[] args) throws IOException {
        ConnectionServiceSettings connectionServiceSettings = ConnectionServiceSettings.newHttpJsonBuilder().build();
        try (ConnectionServiceClient connectionServiceClient = ConnectionServiceClient.create(connectionServiceSettings)) {
            EndpointName parent = EndpointName.of("[PROJECT]", "[ENDPOINT]");
//            for (Connection element : connectionServiceClient.listConnections(parent).iterateAll()) {
//                // doThingsWith(element);
//            }
        } catch (IOException e) {
            throw new RuntimeException(e);
        }

        ConnectionServiceStubSettings connectionServiceStubSettings = ConnectionServiceStubSettings.newBuilder().build();
        ConnectionServiceStub connectionServiceStub = connectionServiceStubSettings.createStub();
        System.out.println(connectionServiceStub.getClass());
        ConnectionServiceStubSettings connectionServiceStubSettings1 = ConnectionServiceStubSettings.newHttpJsonBuilder().build();
        ConnectionServiceStub connectionServiceStub1 = connectionServiceStubSettings1.createStub();
        System.out.println(connectionServiceStub1.getClass());


        // Not possible to create newHttpJsonBuilder
//        TetherSettings tetherSettings = TetherSettings.newHttpJsonBuilder().build();
        TetherSettings tetherSettings = TetherSettings.newBuilder().build();
        tetherSettings.getStubSettings();
        try (TetherClient tetherClient = TetherClient.create(tetherSettings)) {
            BidiStream<EgressResponse, EgressRequest> bidiStream = tetherClient.egressCallable().call();
            EgressResponse request = EgressResponse.newBuilder().setId("id3355").setHttpResponse(HttpResponse.newBuilder().build()).setStatus(Status.newBuilder().build()).setProject("project-309310695").setTraceId("traceId-1067401920").setEndpoint(TetherEndpoint.forNumber(0)).setName("name3373707").build();
//            bidiStream.send(request);
//            for (EgressRequest response : bidiStream) {
//                // Do something when a response is received.
//            }
        }

        TetherStubSettings tetherStubSettings = TetherStubSettings.newBuilder().build();
        TetherStub tetherStub = tetherStubSettings.createStub();
        System.out.println(tetherStub.getClass());
        // Not possible to create newHttpJsonBuilder
//        TetherStubSettings tetherStubSettings1 = TetherStubSettings.newHttpJsonBuilder().build();
//        TetherStub tetherStub1 = tetherStubSettings1.createStub();
    }
}

Output:

class com.google.cloud.apigeeconnect.v1.stub.GrpcConnectionServiceStub
class com.google.cloud.apigeeconnect.v1.stub.HttpJsonConnectionServiceStub
class com.google.cloud.apigeeconnect.v1.stub.GrpcTetherStub

@lqiu96 lqiu96 added the owlbot:run Add this label to trigger the Owlbot post processor. label Jan 25, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Jan 25, 2023
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 27, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

93.3% 93.3% Coverage
0.0% 0.0% Duplication

@lqiu96 lqiu96 merged commit 9c06bc9 into main Jan 27, 2023
@lqiu96 lqiu96 deleted the main-rest_method_generation_2 branch January 27, 2023 15:24
@release-please release-please bot mentioned this pull request Jan 27, 2023
Merged
This was referenced Feb 9, 2023
Closed
Merged
@emmileaf emmileaf mentioned this pull request Jun 1, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@burkedavison burkedavison burkedavison left review comments

@blakeli0 blakeli0 blakeli0 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@lqiu96 @blakeli0 @burkedavison