diff options
author | Yuchen Zeng <zyc@google.com> | 2016-10-10 15:28:50 -0700 |
---|---|---|
committer | Yuchen Zeng <zyc@google.com> | 2016-10-10 15:28:50 -0700 |
commit | 3efcb48c2621403e7cd89cc347e4fdcc501d8ed3 (patch) | |
tree | fa4c536b13ffeaa20a70a3860ffa1e02e6863f0d | |
parent | ac8bc42c8f8f835047550e70850e1f5fb2d56f97 (diff) | |
parent | 062ba7b8baefdc76f74fffb9aa3e2134ba047ea6 (diff) |
Merge remote-tracking branch 'upstream/master' into call_holder_add_pollent
38 files changed, 240 insertions, 81 deletions
diff --git a/composer.json b/composer.json index c5c7ae81d8..711ee82b79 100644 --- a/composer.json +++ b/composer.json @@ -7,6 +7,7 @@ "license": "BSD-3-Clause", "require": { "php": ">=5.5.0", + "ext-grpc": "*", "google/protobuf": "v3.1.0-alpha-1" }, "require-dev": { diff --git a/doc/core/pending_api_cleanups.md b/doc/core/pending_api_cleanups.md index a12e8a9dfb..a0a960e5e2 100644 --- a/doc/core/pending_api_cleanups.md +++ b/doc/core/pending_api_cleanups.md @@ -15,3 +15,5 @@ number: `include/grpc/impl/codegen/grpc_types.h` (commit `af00d8b`) - remove `ServerBuilder::SetMaxMessageSize()` method from `include/grpc++/server_builder.h` (commit `6980362`) +- remove `GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY` macro from + `include/grpc/impl/codegen/grpc_types.h` (commit `59c9f90`) diff --git a/doc/cpp/pending_api_cleanups.md b/doc/cpp/pending_api_cleanups.md new file mode 100644 index 0000000000..3e77b657c6 --- /dev/null +++ b/doc/cpp/pending_api_cleanups.md @@ -0,0 +1,15 @@ +There are times when we make changes that include a temporary shim for +backward-compatibility (e.g., a macro or some other function to preserve +the original API) to avoid having to bump the major version number in +the next release. However, when we do eventually want to release a +feature that does change the API in a non-backward-compatible way, we +will wind up bumping the major version number anyway, at which point we +can take the opportunity to clean up any pending backward-compatibility +shims. + +This file lists all pending backward-compatibility changes that should +be cleaned up the next time we are going to bump the major version +number: + +- remove `ClientContext::set_fail_fast()` method from + `include/grpc++/impl/codegen/client_context.h` (commit `9477724`) diff --git a/examples/php/composer.json b/examples/php/composer.json index e6409f87b4..3d1a95d004 100644 --- a/examples/php/composer.json +++ b/examples/php/composer.json @@ -2,6 +2,7 @@ "name": "grpc/grpc-demo", "description": "gRPC example for PHP", "require": { + "ext-grpc": "*", "grpc/grpc": "v1.0.0" } } diff --git a/include/grpc++/impl/codegen/client_context.h b/include/grpc++/impl/codegen/client_context.h index 387d807c4b..a330ed06bb 100644 --- a/include/grpc++/impl/codegen/client_context.h +++ b/include/grpc++/impl/codegen/client_context.h @@ -226,8 +226,14 @@ class ClientContext { /// EXPERIMENTAL: Set this request to be cacheable void set_cacheable(bool cacheable) { cacheable_ = cacheable; } - /// EXPERIMENTAL: Trigger fail-fast or not on this request - void set_fail_fast(bool fail_fast) { fail_fast_ = fail_fast; } + /// EXPERIMENTAL: Trigger wait-for-ready or not on this request + void set_wait_for_ready(bool wait_for_ready) { + wait_for_ready_ = wait_for_ready; + wait_for_ready_explicitly_set_ = true; + } + + /// DEPRECATED: Use set_wait_for_ready() instead. + void set_fail_fast(bool fail_fast) { set_wait_for_ready(!fail_fast); } #ifndef GRPC_CXX0X_NO_CHRONO /// Return the deadline for the client call. @@ -347,14 +353,18 @@ class ClientContext { uint32_t initial_metadata_flags() const { return (idempotent_ ? GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST : 0) | - (fail_fast_ ? 0 : GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY) | - (cacheable_ ? GRPC_INITIAL_METADATA_CACHEABLE_REQUEST : 0); + (wait_for_ready_ ? GRPC_INITIAL_METADATA_WAIT_FOR_READY : 0) | + (cacheable_ ? GRPC_INITIAL_METADATA_CACHEABLE_REQUEST : 0) | + (wait_for_ready_explicitly_set_ + ? GRPC_INITIAL_METADATA_WAIT_FOR_READY_EXPLICITLY_SET + : 0); } grpc::string authority() { return authority_; } bool initial_metadata_received_; - bool fail_fast_; + bool wait_for_ready_; + bool wait_for_ready_explicitly_set_; bool idempotent_; bool cacheable_; std::shared_ptr<Channel> channel_; diff --git a/include/grpc/impl/codegen/grpc_types.h b/include/grpc/impl/codegen/grpc_types.h index 07e7cff201..ebeef038d1 100644 --- a/include/grpc/impl/codegen/grpc_types.h +++ b/include/grpc/impl/codegen/grpc_types.h @@ -257,15 +257,22 @@ typedef enum grpc_call_error { /** Signal that the call is idempotent */ #define GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST (0x00000010u) /** Signal that the call should not return UNAVAILABLE before it has started */ -#define GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY (0x00000020u) +#define GRPC_INITIAL_METADATA_WAIT_FOR_READY (0x00000020u) +/** DEPRECATED: for backward compatibility */ +#define GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY \ + GRPC_INITIAL_METADATA_WAIT_FOR_READY /** Signal that the call is cacheable. GRPC is free to use GET verb */ #define GRPC_INITIAL_METADATA_CACHEABLE_REQUEST (0x00000040u) +/** Signal that GRPC_INITIAL_METADATA_WAIT_FOR_READY was explicitly set + by the calling application. */ +#define GRPC_INITIAL_METADATA_WAIT_FOR_READY_EXPLICITLY_SET (0x00000080u) /** Mask of all valid flags */ -#define GRPC_INITIAL_METADATA_USED_MASK \ - (GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST | \ - GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY | \ - GRPC_INITIAL_METADATA_CACHEABLE_REQUEST) +#define GRPC_INITIAL_METADATA_USED_MASK \ + (GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST | \ + GRPC_INITIAL_METADATA_WAIT_FOR_READY | \ + GRPC_INITIAL_METADATA_CACHEABLE_REQUEST | \ + GRPC_INITIAL_METADATA_WAIT_FOR_READY_EXPLICITLY_SET) /** A single metadata element */ typedef struct grpc_metadata { diff --git a/src/core/ext/client_config/client_channel.c b/src/core/ext/client_config/client_channel.c index a332f47413..cbf79afa17 100644 --- a/src/core/ext/client_config/client_channel.c +++ b/src/core/ext/client_config/client_channel.c @@ -111,10 +111,10 @@ static void set_channel_connectivity_state_locked(grpc_exec_ctx *exec_ctx, if ((state == GRPC_CHANNEL_TRANSIENT_FAILURE || state == GRPC_CHANNEL_SHUTDOWN) && chand->lb_policy != NULL) { - /* cancel fail-fast picks */ + /* cancel picks with wait_for_ready=false */ grpc_lb_policy_cancel_picks( exec_ctx, chand->lb_policy, - /* mask= */ GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY, + /* mask= */ GRPC_INITIAL_METADATA_WAIT_FOR_READY, /* check= */ 0, GRPC_ERROR_REF(error)); } grpc_connectivity_state_set(exec_ctx, &chand->state_tracker, state, error, diff --git a/src/core/ext/client_config/lb_policy.h b/src/core/ext/client_config/lb_policy.h index ef228d0ebc..de424cd105 100644 --- a/src/core/ext/client_config/lb_policy.h +++ b/src/core/ext/client_config/lb_policy.h @@ -139,16 +139,19 @@ void grpc_lb_policy_weak_unref(grpc_exec_ctx *exec_ctx, grpc_lb_policy *policy); void grpc_lb_policy_init(grpc_lb_policy *policy, const grpc_lb_policy_vtable *vtable); -/** Find an appropriate target for this call, based on \a pick_args. - Picking can be synchronous or asynchronous. In the synchronous case, when a - pick is readily available, it'll be returned in \a target and a non-zero - value will be returned. - In the asynchronous case, zero is returned and \a on_complete will be called - once \a target and \a user_data have been set. Any IO should be done under - the \a interested_parties \a grpc_pollset_set in the \a grpc_lb_policy - struct. The opaque \a user_data output argument corresponds to information - that may need be propagated from the LB policy. It may be NULL. Errors are - signaled receiving a NULL \a *target. */ +/** Finds an appropriate subchannel for a call, based on \a pick_args. + + \a target will be set to the selected subchannel, or NULL on failure. + Upon success, \a user_data will be set to whatever opaque information + may need to be propagated from the LB policy, or NULL if not needed. + + If the pick succeeds and a result is known immediately, a non-zero + value will be returned. Otherwise, \a on_complete will be invoked + once the pick is complete with its error argument set to indicate + success or failure. + + Any IO should be done under the \a interested_parties \a grpc_pollset_set + in the \a grpc_lb_policy struct. */ int grpc_lb_policy_pick(grpc_exec_ctx *exec_ctx, grpc_lb_policy *policy, const grpc_lb_policy_pick_args *pick_args, grpc_connected_subchannel **target, void **user_data, diff --git a/src/core/ext/lb_policy/grpclb/grpclb.c b/src/core/ext/lb_policy/grpclb/grpclb.c index 21074b5d31..29d848589a 100644 --- a/src/core/ext/lb_policy/grpclb/grpclb.c +++ b/src/core/ext/lb_policy/grpclb/grpclb.c @@ -760,7 +760,7 @@ static int glb_pick(grpc_exec_ctx *exec_ctx, grpc_lb_policy *pol, GRPC_ERROR_CREATE("No mdelem storage for the LB token. Load reporting " "won't work without it. Failing"), NULL); - return 1; + return 0; } glb_lb_policy *glb_policy = (glb_lb_policy *)pol; diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.c b/src/core/ext/transport/cronet/transport/cronet_transport.c index 366690acf2..25ad40b935 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.c +++ b/src/core/ext/transport/cronet/transport/cronet_transport.c @@ -239,6 +239,14 @@ static const char *op_id_string(enum e_op_id i) { return "UNKNOWN"; } +static void free_read_buffer(stream_obj *s) { + if (s->state.rs.read_buffer && + s->state.rs.read_buffer != s->state.rs.grpc_header_bytes) { + gpr_free(s->state.rs.read_buffer); + s->state.rs.read_buffer = NULL; + } +} + /* Add a new stream op to op storage. */ @@ -341,6 +349,7 @@ static void on_failed(cronet_bidirectional_stream *stream, int net_error) { gpr_free(s->state.ws.write_buffer); s->state.ws.write_buffer = NULL; } + free_read_buffer(s); gpr_mu_unlock(&s->mu); execute_from_storage(s); } @@ -363,6 +372,7 @@ static void on_canceled(cronet_bidirectional_stream *stream) { gpr_free(s->state.ws.write_buffer); s->state.ws.write_buffer = NULL; } + free_read_buffer(s); gpr_mu_unlock(&s->mu); execute_from_storage(s); } @@ -377,6 +387,7 @@ static void on_succeeded(cronet_bidirectional_stream *stream) { cronet_bidirectional_stream_destroy(s->cbs); s->state.state_callback_received[OP_SUCCEEDED] = true; s->cbs = NULL; + free_read_buffer(s); gpr_mu_unlock(&s->mu); execute_from_storage(s); } @@ -531,7 +542,8 @@ static void create_grpc_frame(gpr_slice_buffer *write_slice_buffer, */ static void convert_metadata_to_cronet_headers( grpc_linked_mdelem *head, const char *host, char **pp_url, - cronet_bidirectional_stream_header **pp_headers, size_t *p_num_headers) { + cronet_bidirectional_stream_header **pp_headers, size_t *p_num_headers, + const char **method) { grpc_linked_mdelem *curr = head; /* Walk the linked list and get number of header fields */ size_t num_headers_available = 0; @@ -558,11 +570,20 @@ static void convert_metadata_to_cronet_headers( curr = curr->next; const char *key = grpc_mdstr_as_c_string(mdelem->key); const char *value = grpc_mdstr_as_c_string(mdelem->value); - if (mdelem->key == GRPC_MDSTR_METHOD || mdelem->key == GRPC_MDSTR_SCHEME || + if (mdelem->key == GRPC_MDSTR_SCHEME || mdelem->key == GRPC_MDSTR_AUTHORITY) { /* Cronet populates these fields on its own */ continue; } + if (mdelem->key == GRPC_MDSTR_METHOD) { + if (mdelem->value == GRPC_MDSTR_PUT) { + *method = "PUT"; + } else { + /* POST method in default*/ + *method = "POST"; + } + continue; + } if (mdelem->key == GRPC_MDSTR_PATH) { /* Create URL by appending :path value to the hostname */ gpr_asprintf(pp_url, "https://%s%s", host, value); @@ -759,15 +780,16 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, s->cbs = cronet_bidirectional_stream_create(s->curr_ct.engine, s->curr_gs, &cronet_callbacks); CRONET_LOG(GPR_DEBUG, "%p = cronet_bidirectional_stream_create()", s->cbs); - char *url; + char *url = NULL; + const char *method = "POST"; s->header_array.headers = NULL; convert_metadata_to_cronet_headers( stream_op->send_initial_metadata->list.head, s->curr_ct.host, &url, - &s->header_array.headers, &s->header_array.count); + &s->header_array.headers, &s->header_array.count, &method); s->header_array.capacity = s->header_array.count; CRONET_LOG(GPR_DEBUG, "cronet_bidirectional_stream_start(%p, %s)", s->cbs, url); - cronet_bidirectional_stream_start(s->cbs, url, 0, "POST", &s->header_array, + cronet_bidirectional_stream_start(s->cbs, url, 0, method, &s->header_array, false); stream_state->state_op_done[OP_SEND_INITIAL_METADATA] = true; result = ACTION_TAKEN_WITH_CALLBACK; @@ -901,6 +923,7 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, uint8_t *dst_p = GPR_SLICE_START_PTR(read_data_slice); memcpy(dst_p, stream_state->rs.read_buffer, (size_t)stream_state->rs.length_field); + free_read_buffer(s); gpr_slice_buffer_init(&stream_state->rs.read_slice_buffer); gpr_slice_buffer_add(&stream_state->rs.read_slice_buffer, read_data_slice); diff --git a/src/core/lib/iomgr/error.h b/src/core/lib/iomgr/error.h index 7e2fd7a3bd..00ace8a7a9 100644 --- a/src/core/lib/iomgr/error.h +++ b/src/core/lib/iomgr/error.h @@ -40,6 +40,10 @@ #include <grpc/status.h> #include <grpc/support/time.h> +#ifdef __cplusplus +extern "C" { +#endif + /// Opaque representation of an error. /// Errors are refcounted objects that represent the result of an operation. /// Ownership laws: @@ -204,4 +208,8 @@ bool grpc_log_if_error(const char *what, grpc_error *error, const char *file, #define GRPC_LOG_IF_ERROR(what, error) \ grpc_log_if_error((what), (error), __FILE__, __LINE__) +#ifdef __cplusplus +} +#endif + #endif /* GRPC_CORE_LIB_IOMGR_ERROR_H */ diff --git a/src/core/lib/surface/byte_buffer.c b/src/core/lib/surface/byte_buffer.c index a093a37af3..054a6e6c58 100644 --- a/src/core/lib/surface/byte_buffer.c +++ b/src/core/lib/surface/byte_buffer.c @@ -72,8 +72,9 @@ grpc_byte_buffer *grpc_raw_byte_buffer_from_reader( grpc_byte_buffer *grpc_byte_buffer_copy(grpc_byte_buffer *bb) { switch (bb->type) { case GRPC_BB_RAW: - return grpc_raw_byte_buffer_create(bb->data.raw.slice_buffer.slices, - bb->data.raw.slice_buffer.count); + return grpc_raw_compressed_byte_buffer_create( + bb->data.raw.slice_buffer.slices, bb->data.raw.slice_buffer.count, + bb->data.raw.compression); } GPR_UNREACHABLE_CODE(return NULL); } diff --git a/src/cpp/client/client_context.cc b/src/cpp/client/client_context.cc index 5b6aaa777b..b6008f47b1 100644 --- a/src/cpp/client/client_context.cc +++ b/src/cpp/client/client_context.cc @@ -59,7 +59,8 @@ static ClientContext::GlobalCallbacks* g_client_callbacks = ClientContext::ClientContext() : initial_metadata_received_(false), - fail_fast_(true), + wait_for_ready_(false), + wait_for_ready_explicitly_set_(false), idempotent_(false), cacheable_(false), call_(nullptr), diff --git a/src/cpp/common/channel_filter.h b/src/cpp/common/channel_filter.h index 6f5af3dec3..ae32e02f69 100644 --- a/src/cpp/common/channel_filter.h +++ b/src/cpp/common/channel_filter.h @@ -42,6 +42,7 @@ #include <vector> #include "src/core/lib/channel/channel_stack.h" +#include "src/core/lib/security/context/security_context.h" #include "src/core/lib/surface/channel_init.h" #include "src/core/lib/transport/metadata_batch.h" @@ -54,11 +55,6 @@ /// "name-of-filter", GRPC_SERVER_CHANNEL, INT_MAX, nullptr); /// \endcode -/// Forward declaration to avoid including the file -/// "src/core/lib/security/context/security_context.h" -struct grpc_client_security_context; -struct grpc_server_security_context; - namespace grpc { /// A C++ wrapper for the \c grpc_metadata_batch struct. diff --git a/src/csharp/Grpc.Tools.nuspec b/src/csharp/Grpc.Tools.nuspec index 0c937ab9cb..ba4e1d674c 100644 --- a/src/csharp/Grpc.Tools.nuspec +++ b/src/csharp/Grpc.Tools.nuspec @@ -17,17 +17,17 @@ </metadata> <files> <!-- forward slashes in src path enable building on Linux --> - <file src="protoc_plugins/windows_x86/protoc.exe" target="tools\windows_x86\protoc.exe" /> - <file src="protoc_plugins/windows_x86/grpc_csharp_plugin.exe" target="tools\windows_x86\grpc_csharp_plugin.exe" /> - <file src="protoc_plugins/windows_x64/protoc.exe" target="tools\windows_x64\protoc.exe" /> - <file src="protoc_plugins/windows_x64/grpc_csharp_plugin.exe" target="tools\windows_x64\grpc_csharp_plugin.exe" /> - <file src="protoc_plugins/linux_x86/protoc" target="tools\linux_x86\protoc" /> - <file src="protoc_plugins/linux_x86/grpc_csharp_plugin" target="tools\linux_x86\grpc_csharp_plugin" /> - <file src="protoc_plugins/linux_x64/protoc" target="tools\linux_x64\protoc" /> - <file src="protoc_plugins/linux_x64/grpc_csharp_plugin" target="tools\linux_x64\grpc_csharp_plugin" /> - <file src="protoc_plugins/macosx_x86/protoc" target="tools\macosx_x86\protoc" /> - <file src="protoc_plugins/macosx_x86/grpc_csharp_plugin" target="tools\macosx_x86\grpc_csharp_plugin" /> - <file src="protoc_plugins/macosx_x64/protoc" target="tools\macosx_x64\protoc" /> - <file src="protoc_plugins/macosx_x64/grpc_csharp_plugin" target="tools\macosx_x64\grpc_csharp_plugin" /> + <file src="protoc_plugins/windows_x86/protoc.exe" target="tools/windows_x86/protoc.exe" /> + <file src="protoc_plugins/windows_x86/grpc_csharp_plugin.exe" target="tools/windows_x86/grpc_csharp_plugin.exe" /> + <file src="protoc_plugins/windows_x64/protoc.exe" target="tools/windows_x64/protoc.exe" /> + <file src="protoc_plugins/windows_x64/grpc_csharp_plugin.exe" target="tools/windows_x64/grpc_csharp_plugin.exe" /> + <file src="protoc_plugins/linux_x86/protoc" target="tools/linux_x86/protoc" /> + <file src="protoc_plugins/linux_x86/grpc_csharp_plugin" target="tools/linux_x86/grpc_csharp_plugin" /> + <file src="protoc_plugins/linux_x64/protoc" target="tools/linux_x64/protoc" /> + <file src="protoc_plugins/linux_x64/grpc_csharp_plugin" target="tools/linux_x64/grpc_csharp_plugin" /> + <file src="protoc_plugins/macosx_x86/protoc" target="tools/macosx_x86/protoc" /> + <file src="protoc_plugins/macosx_x86/grpc_csharp_plugin" target="tools/macosx_x86/grpc_csharp_plugin" /> + <file src="protoc_plugins/macosx_x64/protoc" target="tools/macosx_x64/protoc" /> + <file src="protoc_plugins/macosx_x64/grpc_csharp_plugin" target="tools/macosx_x64/grpc_csharp_plugin" /> </files> </package> diff --git a/src/objective-c/GRPCClient/GRPCCall.h b/src/objective-c/GRPCClient/GRPCCall.h index b9e741dfa8..7645bb1d34 100644 --- a/src/objective-c/GRPCClient/GRPCCall.h +++ b/src/objective-c/GRPCClient/GRPCCall.h @@ -155,6 +155,18 @@ typedef NS_ENUM(NSUInteger, GRPCErrorCode) { }; /** + * Safety remark of a gRPC method as defined in RFC 2616 Section 9.1 + */ +typedef NS_ENUM(NSUInteger, GRPCCallSafety) { + /** Signal that there is no guarantees on how the call affects the server state. */ + GRPCCallSafetyDefault = 0, + /** Signal that the call is idempotent. gRPC is free to use PUT verb. */ + GRPCCallSafetyIdempotentRequest = 1, + /** Signal that the call is cacheable and will not affect server state. gRPC is free to use GET verb. */ + GRPCCallSafetyCacheableRequest = 2, +}; + +/** * Keys used in |NSError|'s |userInfo| dictionary to store the response headers and trailers sent by * the server. */ @@ -233,6 +245,14 @@ extern id const kGRPCTrailersKey; */ - (void)cancel; +/** + * Set the call flag for a specific host path. + * + * Host parameter should not contain the scheme (http:// or https://), only the name or IP addr + * and the port number, for example @"localhost:5050". + */ ++ (void)setCallSafety:(GRPCCallSafety)callSafety host:(NSString *)host path:(NSString *)path; + // TODO(jcanizales): Let specify a deadline. As a category of GRXWriter? @end diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index eecda4c03a..43204345f5 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -47,6 +47,7 @@ NSString * const kGRPCHeadersKey = @"io.grpc.HeadersKey"; NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; +static NSMutableDictionary *callFlags; @interface GRPCCall () <GRXWriteable> // Make them read-write. @@ -106,6 +107,29 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; // TODO(jcanizales): If grpc_init is idempotent, this should be changed from load to initialize. + (void)load { grpc_init(); + callFlags = [NSMutableDictionary dictionary]; +} + ++ (void)setCallSafety:(GRPCCallSafety)callSafety host:(NSString *)host path:(NSString *)path { + NSString *hostAndPath = [NSString stringWithFormat:@"%@/%@", host, path]; + switch (callSafety) { + case GRPCCallSafetyDefault: + callFlags[hostAndPath] = @0; + break; + case GRPCCallSafetyIdempotentRequest: + callFlags[hostAndPath] = @GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST; + break; + case GRPCCallSafetyCacheableRequest: + callFlags[hostAndPath] = @GRPC_INITIAL_METADATA_CACHEABLE_REQUEST; + break; + default: + break; + } +} + ++ (uint32_t)callFlagsForHost:(NSString *)host path:(NSString *)path { + NSString *hostAndPath = [NSString stringWithFormat:@"%@/%@", host, path]; + return [callFlags[hostAndPath] intValue]; } - (instancetype)init { @@ -231,6 +255,7 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; - (void)sendHeaders:(NSDictionary *)headers { // TODO(jcanizales): Add error handlers for async failures [_wrappedCall startBatchWithOperations:@[[[GRPCOpSendMetadata alloc] initWithMetadata:headers + flags:[GRPCCall callFlagsForHost:_host path:_path] handler:nil]]]; } diff --git a/src/objective-c/GRPCClient/private/GRPCWrappedCall.h b/src/objective-c/GRPCClient/private/GRPCWrappedCall.h index e37ed1b59f..52233c8242 100644 --- a/src/objective-c/GRPCClient/private/GRPCWrappedCall.h +++ b/src/objective-c/GRPCClient/private/GRPCWrappedCall.h @@ -45,6 +45,10 @@ @interface GRPCOpSendMetadata : GRPCOperation - (instancetype)initWithMetadata:(NSDictionary *)metadata + handler:(void(^)())handler; + +- (instancetype)initWithMetadata:(NSDictionary *)metadata + flags:(uint32_t)flags handler:(void(^)())handler NS_DESIGNATED_INITIALIZER; @end diff --git a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m index 1339429660..627b6aa86d 100644 --- a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m +++ b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m @@ -64,16 +64,24 @@ @implementation GRPCOpSendMetadata - (instancetype)init { - return [self initWithMetadata:nil handler:nil]; + return [self initWithMetadata:nil flags:0 handler:nil]; } -- (instancetype)initWithMetadata:(NSDictionary *)metadata handler:(void (^)())handler { +- (instancetype)initWithMetadata:(NSDictionary *)metadata + handler:(void (^)())handler { + return [self initWithMetadata:metadata flags:0 handler:handler]; +} + +- (instancetype)initWithMetadata:(NSDictionary *)metadata + flags:(uint32_t)flags + handler:(void (^)())handler { if (self = [super init]) { _op.op = GRPC_OP_SEND_INITIAL_METADATA; _op.data.send_initial_metadata.count = metadata.count; _op.data.send_initial_metadata.metadata = metadata.grpc_metadataArray; _op.data.send_initial_metadata.maybe_compression_level.is_set = false; _op.data.send_initial_metadata.maybe_compression_level.level = 0; + _op.flags = flags; _handler = handler; } return self; diff --git a/src/objective-c/tests/GRPCClientTests.m b/src/objective-c/tests/GRPCClientTests.m index 916a335802..77640525d5 100644 --- a/src/objective-c/tests/GRPCClientTests.m +++ b/src/objective-c/tests/GRPCClientTests.m @@ -317,4 +317,37 @@ static GRPCProtoMethod *kUnaryCallMethod; } +- (void)testIdempotentProtoRPC { + __weak XCTestExpectation *response = [self expectationWithDescription:@"Expected response."]; + __weak XCTestExpectation *completion = [self expectationWithDescription:@"RPC completed."]; + + RMTSimpleRequest *request = [RMTSimpleRequest message]; + request.responseSize = 100; + request.fillUsername = YES; + request.fillOauthScope = YES; + GRXWriter *requestsWriter = [GRXWriter writerWithValue:[request data]]; + + GRPCCall *call = [[GRPCCall alloc] initWithHost:kHostAddress + path:kUnaryCallMethod.HTTPPath + requestsWriter:requestsWriter]; + [GRPCCall setCallSafety:GRPCCallSafetyIdempotentRequest host:kHostAddress path:kUnaryCallMethod.HTTPPath]; + + id<GRXWriteable> responsesWriteable = [[GRXWriteable alloc] initWithValueHandler:^(NSData *value) { + XCTAssertNotNil(value, @"nil value received as response."); + XCTAssertGreaterThan(value.length, 0, @"Empty response received."); + RMTSimpleResponse *responseProto = [RMTSimpleResponse parseFromData:value error:NULL]; + // We expect empty strings, not nil: + XCTAssertNotNil(responseProto.username, @"Response's username is nil."); + XCTAssertNotNil(responseProto.oauthScope, @"Response's OAuth scope is nil."); + [response fulfill]; + } completionHandler:^(NSError *errorOrNil) { + XCTAssertNil(errorOrNil, @"Finished with unexpected error: %@", errorOrNil); + [completion fulfill]; + }]; + + [call startWithWriteable:responsesWriteable]; + + [self waitForExpectationsWithTimeout:8 handler:nil]; +} + @end diff --git a/src/php/composer.json b/src/php/composer.json index 6042094032..2d5d555bc2 100644 --- a/src/php/composer.json +++ b/src/php/composer.json @@ -8,6 +8,7 @@ "version": "1.1.0", "require": { "php": ">=5.5.0", + "ext-grpc": "*", "google/protobuf": "v3.1.0-alpha-1" }, "require-dev": { diff --git a/src/python/grpcio/grpc/_server.py b/src/python/grpcio/grpc/_server.py index 5f846ce773..5223712dfa 100644 --- a/src/python/grpcio/grpc/_server.py +++ b/src/python/grpcio/grpc/_server.py @@ -462,7 +462,6 @@ def _unary_response_in_pool( rpc_event, state, response, response_serializer) if serialized_response is not None: _status(rpc_event, state, serialized_response) - return def _stream_response_in_pool( diff --git a/templates/composer.json.template b/templates/composer.json.template index accfb382a9..3b4d62f24d 100644 --- a/templates/composer.json.template +++ b/templates/composer.json.template @@ -9,6 +9,7 @@ "license": "BSD-3-Clause", "require": { "php": ">=5.5.0", + "ext-grpc": "*", "google/protobuf": "v3.1.0-alpha-1" }, "require-dev": { diff --git a/templates/src/php/composer.json.template b/templates/src/php/composer.json.template index 7feeae976d..12a4ce8f83 100644 --- a/templates/src/php/composer.json.template +++ b/templates/src/php/composer.json.template @@ -10,6 +10,7 @@ "version": "${settings.php_version.php_composer()}", "require": { "php": ">=5.5.0", + "ext-grpc": "*", "google/protobuf": "v3.1.0-alpha-1" }, "require-dev": { diff --git a/test/core/bad_ssl/bad_ssl_test.c b/test/core/bad_ssl/bad_ssl_test.c index c9cdb169b6..f8a9fe6cac 100644 --- a/test/core/bad_ssl/bad_ssl_test.c +++ b/test/core/bad_ssl/bad_ssl_test.c @@ -88,7 +88,7 @@ static void run_test(const char *target, size_t nops) { op = ops; op->op = GRPC_OP_SEND_INITIAL_METADATA; op->data.send_initial_metadata.count = 0; - op->flags = GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY; + op->flags = GRPC_INITIAL_METADATA_WAIT_FOR_READY; op->reserved = NULL; op++; op->op = GRPC_OP_RECV_STATUS_ON_CLIENT; diff --git a/test/core/end2end/connection_refused_test.c b/test/core/end2end/connection_refused_test.c index 4149159a37..62278d63c5 100644 --- a/test/core/end2end/connection_refused_test.c +++ b/test/core/end2end/connection_refused_test.c @@ -44,7 +44,7 @@ static void *tag(intptr_t i) { return (void *)i; } -static void run_test(bool fail_fast) { +static void run_test(bool wait_for_ready) { grpc_channel *chan; grpc_call *call; gpr_timespec deadline = GRPC_TIMEOUT_SECONDS_TO_DEADLINE(2); @@ -57,7 +57,7 @@ static void run_test(bool fail_fast) { char *details = NULL; size_t details_capacity = 0; - gpr_log(GPR_INFO, "TEST: fail_fast=%d", fail_fast); + gpr_log(GPR_INFO, "TEST: wait_for_ready=%d", wait_for_ready); grpc_init(); @@ -81,7 +81,7 @@ static void run_test(bool fail_fast) { op = ops; op->op = GRPC_OP_SEND_INITIAL_METADATA; op->data.send_initial_metadata.count = 0; - op->flags = fail_fast ? 0 : GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY; + op->flags = wait_for_ready ? GRPC_INITIAL_METADATA_WAIT_FOR_READY : 0; op->reserved = NULL; op++; op->op = GRPC_OP_RECV_STATUS_ON_CLIENT; @@ -98,10 +98,10 @@ static void run_test(bool fail_fast) { CQ_EXPECT_COMPLETION(cqv, tag(1), 1); cq_verify(cqv); - if (fail_fast) { - GPR_ASSERT(status == GRPC_STATUS_UNAVAILABLE); - } else { + if (wait_for_ready) { GPR_ASSERT(status == GRPC_STATUS_DEADLINE_EXCEEDED); + } else { + GPR_ASSERT(status == GRPC_STATUS_UNAVAILABLE); } grpc_completion_queue_shutdown(cq); @@ -122,7 +122,7 @@ static void run_test(bool fail_fast) { int main(int argc, char **argv) { grpc_test_init(argc, argv); - run_test(true); run_test(false); + run_test(true); return 0; } diff --git a/test/core/end2end/dualstack_socket_test.c b/test/core/end2end/dualstack_socket_test.c index 8abb81c803..cb07ca535b 100644 --- a/test/core/end2end/dualstack_socket_test.c +++ b/test/core/end2end/dualstack_socket_test.c @@ -171,7 +171,7 @@ void test_connect(const char *server_host, const char *client_host, int port, op = ops; op->op = GRPC_OP_SEND_INITIAL_METADATA; op->data.send_initial_metadata.count = 0; - op->flags = expect_ok ? GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY : 0; + op->flags = expect_ok ? GRPC_INITIAL_METADATA_WAIT_FOR_READY : 0; op->reserved = NULL; op++; op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT; diff --git a/test/core/end2end/tests/simple_delayed_request.c b/test/core/end2end/tests/simple_delayed_request.c index 74f1232d78..50d1975c8d 100644 --- a/test/core/end2end/tests/simple_delayed_request.c +++ b/test/core/end2end/tests/simple_delayed_request.c @@ -119,7 +119,7 @@ static void simple_delayed_request_body(grpc_end2end_test_config config, op = ops; op->op = GRPC_OP_SEND_INITIAL_METADATA; op->data.send_initial_metadata.count = 0; - op->flags = GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY; + op->flags = GRPC_INITIAL_METADATA_WAIT_FOR_READY; op->reserved = NULL; op++; op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT; diff --git a/test/cpp/end2end/client_crash_test.cc b/test/cpp/end2end/client_crash_test.cc index c452ad2beb..966c04b0e3 100644 --- a/test/cpp/end2end/client_crash_test.cc +++ b/test/cpp/end2end/client_crash_test.cc @@ -89,7 +89,7 @@ TEST_F(CrashTest, KillBeforeWrite) { EchoRequest request; EchoResponse response; ClientContext context; - context.set_fail_fast(false); + context.set_wait_for_ready(true); auto stream = stub->BidiStream(&context); @@ -115,7 +115,7 @@ TEST_F(CrashTest, KillAfterWrite) { EchoRequest request; EchoResponse response; ClientContext context; - context.set_fail_fast(false); + context.set_wait_for_ready(true); auto stream = stub->BidiStream(&context); diff --git a/test/cpp/end2end/hybrid_end2end_test.cc b/test/cpp/end2end/hybrid_end2end_test.cc index 82361d0e90..8cd2e66347 100644 --- a/test/cpp/end2end/hybrid_end2end_test.cc +++ b/test/cpp/end2end/hybrid_end2end_test.cc @@ -261,7 +261,7 @@ class HybridEnd2endTest : public ::testing::Test { EchoRequest send_request; EchoResponse recv_response; ClientContext cli_ctx; - cli_ctx.set_fail_fast(false); + cli_ctx.set_wait_for_ready(true); send_request.set_message("Hello"); Status recv_status = stub_->Echo(&cli_ctx, send_request, &recv_response); EXPECT_EQ(send_request.message(), recv_response.message()); @@ -275,7 +275,7 @@ class HybridEnd2endTest : public ::testing::Test { EchoRequest send_request; EchoResponse recv_response; ClientContext cli_ctx; - cli_ctx.set_fail_fast(false); + cli_ctx.set_wait_for_ready(true); send_request.set_message("Hello"); Status recv_status = stub->Echo(&cli_ctx, send_request, &recv_response); EXPECT_EQ(send_request.message() + "_dup", recv_response.message()); @@ -287,7 +287,7 @@ class HybridEnd2endTest : public ::testing::Test { EchoResponse recv_response; grpc::string expected_message; ClientContext cli_ctx; - cli_ctx.set_fail_fast(false); + cli_ctx.set_wait_for_ready(true); send_request.set_message("Hello"); auto stream = stub_->RequestStream(&cli_ctx, &recv_response); for (int i = 0; i < 5; i++) { @@ -304,7 +304,7 @@ class HybridEnd2endTest : public ::testing::Test { EchoRequest request; EchoResponse response; ClientContext context; - context.set_fail_fast(false); + context.set_wait_for_ready(true); request.set_message("hello"); auto stream = stub_->ResponseStream(&context, request); @@ -324,7 +324,7 @@ class HybridEnd2endTest : public ::testing::Test { EchoRequest request; EchoResponse response; ClientContext context; - context.set_fail_fast(false); + context.set_wait_for_ready(true); grpc::string msg("hello"); auto stream = stub_->BidiStream(&context); diff --git a/test/cpp/end2end/server_crash_test_client.cc b/test/cpp/end2end/server_crash_test_client.cc index 10a251c952..5df09cd853 100644 --- a/test/cpp/end2end/server_crash_test_client.cc +++ b/test/cpp/end2end/server_crash_test_client.cc @@ -65,7 +65,7 @@ int main(int argc, char** argv) { EchoRequest request; EchoResponse response; grpc::ClientContext context; - context.set_fail_fast(false); + context.set_wait_for_ready(true); if (FLAGS_mode == "bidi") { auto stream = stub->BidiStream(&context); diff --git a/test/cpp/qps/client_async.cc b/test/cpp/qps/client_async.cc index 5d9cb4bd0c..081114859c 100644 --- a/test/cpp/qps/client_async.cc +++ b/test/cpp/qps/client_async.cc @@ -243,6 +243,7 @@ class AsyncClient : public ClientImpl<StubType, RequestType> { // this thread isn't supposed to shut down std::lock_guard<std::mutex> l(shutdown_state_[thread_idx]->mutex); if (shutdown_state_[thread_idx]->shutdown) { + delete ctx; return true; } else if (!ctx->RunNextState(ok, entry)) { // The RPC and callback are done, so clone the ctx diff --git a/test/cpp/qps/driver.cc b/test/cpp/qps/driver.cc index b4c18bcb46..7460bb526a 100644 --- a/test/cpp/qps/driver.cc +++ b/test/cpp/qps/driver.cc @@ -83,7 +83,7 @@ static std::unordered_map<string, std::deque<int>> get_hosts_and_cores( auto stub = WorkerService::NewStub( CreateChannel(*it, InsecureChannelCredentials())); grpc::ClientContext ctx; - ctx.set_fail_fast(false); + ctx.set_wait_for_ready(true); CoreRequest dummy; CoreResponse cores; grpc::Status s = stub->CoreCount(&ctx, dummy, &cores); @@ -167,7 +167,7 @@ namespace runsc { static ClientContext* AllocContext(list<ClientContext>* contexts) { contexts->emplace_back(); auto context = &contexts->back(); - context->set_fail_fast(false); + context->set_wait_for_ready(true); return context; } @@ -527,7 +527,7 @@ bool RunQuit() { CreateChannel(workers[i], InsecureChannelCredentials())); Void dummy; grpc::ClientContext ctx; - ctx.set_fail_fast(false); + ctx.set_wait_for_ready(true); Status s = stub->QuitWorker(&ctx, dummy, &dummy); if (!s.ok()) { gpr_log(GPR_ERROR, "Worker %zu could not be properly quit because %s", i, diff --git a/tools/dockerfile/interoptest/grpc_interop_php/build_interop.sh b/tools/dockerfile/interoptest/grpc_interop_php/build_interop.sh index cf5e888eff..624d587786 100755 --- a/tools/dockerfile/interoptest/grpc_interop_php/build_interop.sh +++ b/tools/dockerfile/interoptest/grpc_interop_php/build_interop.sh @@ -46,6 +46,6 @@ make install (cd third_party/protobuf && make install) -(cd src/php && composer install) +(cd src/php && php -d extension=ext/grpc/modules/grpc.so /usr/local/bin/composer install) (cd src/php && ./bin/generate_proto_php.sh) diff --git a/tools/dockerfile/interoptest/grpc_interop_php7/build_interop.sh b/tools/dockerfile/interoptest/grpc_interop_php7/build_interop.sh index e486e5276a..87cb0fe4b2 100755 --- a/tools/dockerfile/interoptest/grpc_interop_php7/build_interop.sh +++ b/tools/dockerfile/interoptest/grpc_interop_php7/build_interop.sh @@ -46,6 +46,6 @@ make install (cd third_party/protobuf && make install) -(cd src/php && composer install) +(cd src/php && php -d extension=ext/grpc/modules/grpc.so /usr/local/bin/composer install) (cd src/php && ./bin/generate_proto_php.sh) diff --git a/tools/dockerfile/stress_test/grpc_interop_stress_php/build_interop_stress.sh b/tools/dockerfile/stress_test/grpc_interop_stress_php/build_interop_stress.sh index 34fd09f78b..a671d1501f 100755 --- a/tools/dockerfile/stress_test/grpc_interop_stress_php/build_interop_stress.sh +++ b/tools/dockerfile/stress_test/grpc_interop_stress_php/build_interop_stress.sh @@ -48,6 +48,6 @@ make install (cd third_party/protobuf && make install) -(cd src/php && composer install) +(cd src/php && php -d extension=ext/grpc/modules/grpc.so /usr/local/bin/composer install) (cd src/php && ./bin/generate_proto_php.sh) diff --git a/tools/run_tests/dockerize/build_docker_and_run_tests.sh b/tools/run_tests/dockerize/build_docker_and_run_tests.sh index b4b172ddef..c3219c533d 100755 --- a/tools/run_tests/dockerize/build_docker_and_run_tests.sh +++ b/tools/run_tests/dockerize/build_docker_and_run_tests.sh @@ -61,6 +61,7 @@ CONTAINER_NAME="run_tests_$(uuidgen)" docker_instance_git_root=/var/local/jenkins/grpc # Run tests inside docker +DOCKER_EXIT_CODE=0 docker run \ -e "RUN_TESTS_COMMAND=$RUN_TESTS_COMMAND" \ -e "config=$config" \ @@ -81,7 +82,7 @@ docker run \ -w /var/local/git/grpc \ --name=$CONTAINER_NAME \ $DOCKER_IMAGE_NAME \ - bash -l "/var/local/jenkins/grpc/$DOCKER_RUN_SCRIPT" || DOCKER_FAILED="true" + bash -l "/var/local/jenkins/grpc/$DOCKER_RUN_SCRIPT" || DOCKER_EXIT_CODE=$? # use unique name for reports.zip to prevent clash between concurrent # run_tests.py runs @@ -93,7 +94,4 @@ rm -f ${TEMP_REPORTS_ZIP} # remove the container, possibly killing it first docker rm -f $CONTAINER_NAME || true -if [ "$DOCKER_FAILED" != "" ] && [ "$XML_REPORT" == "" ] -then - exit 1 -fi +exit $DOCKER_EXIT_CODE diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 168331602c..a6f3d405dc 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -1423,7 +1423,7 @@ else: exit_code = 0 if BuildAndRunError.BUILD in errors: exit_code |= 1 - if BuildAndRunError.TEST in errors and not args.travis: + if BuildAndRunError.TEST in errors: exit_code |= 2 if BuildAndRunError.POST_TEST in errors: exit_code |= 4 |