From 6b640afbd93ea8c861b902211dc34e188234d072 Mon Sep 17 00:00:00 2001 From: Ken Bandes Date: Thu, 28 Oct 2021 13:44:56 -0400 Subject: [PATCH] fix: methods returning Operation w/o operation_info are now allowed. (#1047) Formerly, a method that returned google.longrunning.Operation but did not have the google.longrunning.operation_info extension was considered an error; but there are valid cases for this, particularly in the Operation service itself. This change makes it acceptable to have a method like this. It is not treated as a long-running operation. Co-authored-by: Kenneth Bandes --- gapic/schema/api.py | 4 +++ tests/unit/schema/test_api.py | 54 +++++++++++++++++++++++++++++++++-- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/gapic/schema/api.py b/gapic/schema/api.py index cdfb5ca6e7..0e632ed217 100644 --- a/gapic/schema/api.py +++ b/gapic/schema/api.py @@ -817,6 +817,10 @@ def _maybe_get_lro( # If the output type is google.longrunning.Operation, we use # a specialized object in its place. if meth_pb.output_type.endswith('google.longrunning.Operation'): + if not meth_pb.options.HasExtension(operations_pb2.operation_info): + # This is not a long running operation even though it returns + # an Operation. + return None op = meth_pb.options.Extensions[operations_pb2.operation_info] if not op.response_type or not op.metadata_type: raise TypeError( diff --git a/tests/unit/schema/test_api.py b/tests/unit/schema/test_api.py index de705d88df..2c139ce7f1 100644 --- a/tests/unit/schema/test_api.py +++ b/tests/unit/schema/test_api.py @@ -944,19 +944,69 @@ def test_lro(): assert len(lro_proto.messages) == 1 -def test_lro_missing_annotation(): +def test_lro_operation_no_annotation(): + # A method that returns google.longrunning.Operation, + # but has no operation_info option, is treated as not lro. + # Set up a prior proto that mimics google/protobuf/empty.proto lro_proto = api.Proto.build(make_file_pb2( name='operations.proto', package='google.longrunning', messages=(make_message_pb2(name='Operation'),), ), file_to_generate=False, naming=make_naming()) - # Set up a method with an LRO but no annotation. + # Set up a method that returns an Operation, but has no annotation. + method_pb2 = descriptor_pb2.MethodDescriptorProto( + name='GetOperation', + input_type='google.example.v3.GetOperationRequest', + output_type='google.longrunning.Operation', + ) + + # Set up the service with an RPC. + service_pb = descriptor_pb2.ServiceDescriptorProto( + name='OperationService', + method=(method_pb2,), + ) + + # Set up the messages, including the annotated ones. + messages = ( + make_message_pb2(name='GetOperationRequest', fields=()), + ) + + # Finally, set up the file that encompasses these. + fdp = make_file_pb2( + package='google.example.v3', + messages=messages, + services=(service_pb,), + ) + + # Make the proto object. + proto = api.Proto.build(fdp, file_to_generate=True, prior_protos={ + 'google/longrunning/operations.proto': lro_proto, + }, naming=make_naming()) + + service = proto.services['google.example.v3.OperationService'] + method = service.methods['GetOperation'] + assert method.lro is None + + +def test_lro_bad_annotation(): + # Set up a prior proto that mimics google/protobuf/empty.proto + lro_proto = api.Proto.build(make_file_pb2( + name='operations.proto', package='google.longrunning', + messages=(make_message_pb2(name='Operation'),), + ), file_to_generate=False, naming=make_naming()) + + # Set up a method with an LRO and incomplete annotation. method_pb2 = descriptor_pb2.MethodDescriptorProto( name='AsyncDoThing', input_type='google.example.v3.AsyncDoThingRequest', output_type='google.longrunning.Operation', ) + method_pb2.options.Extensions[operations_pb2.operation_info].MergeFrom( + operations_pb2.OperationInfo( + response_type='google.example.v3.AsyncDoThingResponse', + ), + ) # Set up the service with an RPC. service_pb = descriptor_pb2.ServiceDescriptorProto(