Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 69 commits into from
Jan 27, 2023
Merged

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

merged 69 commits into from
Jan 27, 2023

Conversation

lqiu96
Copy link
Contributor

@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
Collaborator

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
Contributor 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 as a code owner 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
Contributor 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

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