Refactorings and renamings related to compression.

Also added levels to the channel args options.

10 files changed, 58 insertions, 41 deletions

diff --git a/include/grpc/impl/codegen/compression_types.h b/include/grpc/impl/codegen/compression_types.h
index 8d2ec3b9d7..7cf68bfaf8 100644
--- a/include/grpc/impl/codegen/compression_types.h
+++ b/include/grpc/impl/codegen/compression_types.h
@@ -35,11 +35,17 @@
 #define GRPC_IMPL_CODEGEN_COMPRESSION_TYPES_H
 
 #include <grpc/impl/codegen/port_platform.h>
+#include <stdbool.h>
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
+/** To be used as initial metadata key for the request of a concrete compression
+ * algorithm */
+#define GRPC_COMPRESSION_REQUEST_ALGORITHM_MD_KEY \
+  "grpc-internal-encoding-request"
+
 /** To be used in channel arguments */
 #define GRPC_COMPRESSION_CHANNEL_DEFAULT_ALGORITHM \
   "grpc.default_compression_algorithm"
@@ -74,15 +80,24 @@ typedef struct grpc_compression_options {
    */
   uint32_t enabled_algorithms_bitset;
 
-  /** The default channel compression algorithm. It'll be used in the absence of
+  /** The default channel compression level. It'll be used in the absence of
    * call specific settings. This option corresponds to the channel argument key
-   * behind \a GRPC_COMPRESSION_CHANNEL_DEFAULT_ALGORITHM */
-  grpc_compression_algorithm default_compression_algorithm;
+   * behind \a GRPC_COMPRESSION_CHANNEL_DEFAULT_LEVEL. If present, takes
+   * precedence over \a default_algorithm.
+   * TODO(dgq): currently only available for server channels. */
+  struct {
+    bool is_set;
+    grpc_compression_algorithm level;
+  } default_level;
 
-  /** The default channel compression level. It'll be used in the absence of
+  /** The default channel compression algorithm. It'll be used in the absence of
    * call specific settings. This option corresponds to the channel argument key
-   * behind \a GRPC_COMPRESSION_CHANNEL_DEFAULT_LEVEL */
-  grpc_compression_algorithm default_compression_level;
+   * behind \a GRPC_COMPRESSION_CHANNEL_DEFAULT_ALGORITHM. */
+  struct {
+    bool is_set;
+    grpc_compression_algorithm algorithm;
+  } default_algorithm;
+
 } grpc_compression_options;
 
 #ifdef __cplusplus
diff --git a/src/core/lib/channel/channel_args.c b/src/core/lib/channel/channel_args.c
index 893cf0700e..920b632abf 100644
--- a/src/core/lib/channel/channel_args.c
+++ b/src/core/lib/channel/channel_args.c
@@ -238,11 +238,11 @@ grpc_channel_args *grpc_channel_args_compression_algorithm_set_state(
   return result;
 }
 
-int grpc_channel_args_compression_algorithm_get_states(
+uint32_t grpc_channel_args_compression_algorithm_get_states(
     const grpc_channel_args *a) {
   int *states_arg;
   if (find_compression_algorithm_states_bitset(a, &states_arg)) {
-    return *states_arg;
+    return (uint32_t)*states_arg;
   } else {
     return (1u << GRPC_COMPRESS_ALGORITHMS_COUNT) - 1; /* All algs. enabled */
   }
diff --git a/src/core/lib/channel/channel_args.h b/src/core/lib/channel/channel_args.h
index 23c7b7b897..653d04f427 100644
--- a/src/core/lib/channel/channel_args.h
+++ b/src/core/lib/channel/channel_args.h
@@ -81,7 +81,7 @@ grpc_channel_args *grpc_channel_args_compression_algorithm_set_state(
  *
  * The i-th bit of the returned bitset corresponds to the i-th entry in the
  * grpc_compression_algorithm enum. */
-int grpc_channel_args_compression_algorithm_get_states(
+uint32_t grpc_channel_args_compression_algorithm_get_states(
     const grpc_channel_args *a);
 
 int grpc_channel_args_compare(const grpc_channel_args *a,
diff --git a/src/core/lib/channel/compress_filter.c b/src/core/lib/channel/compress_filter.c
index 5510c79b18..02991b9c79 100644
--- a/src/core/lib/channel/compress_filter.c
+++ b/src/core/lib/channel/compress_filter.c
@@ -73,8 +73,8 @@ typedef struct call_data {
 typedef struct channel_data {
   /** The default, channel-level, compression algorithm */
   grpc_compression_algorithm default_compression_algorithm;
-  /** Compression options for the channel */
-  grpc_compression_options compression_options;
+  /** Bitset of enabled algorithms */
+  uint32_t enabled_algorithms_bitset;
   /** Supported compression algorithms */
   uint32_t supported_compression_algorithms;
 } channel_data;
@@ -96,9 +96,8 @@ static grpc_mdelem *compression_md_filter(void *user_data, grpc_mdelem *md) {
               md_c_str);
       calld->compression_algorithm = GRPC_COMPRESS_NONE;
     }
-    if (grpc_compression_options_is_algorithm_enabled(
-            &channeld->compression_options, calld->compression_algorithm) ==
-        0) {
+    if (!GPR_BITGET(channeld->enabled_algorithms_bitset,
+                    calld->compression_algorithm)) {
       gpr_log(GPR_ERROR,
               "Invalid compression algorithm: '%s' (previously disabled). "
               "Ignoring.",
@@ -280,32 +279,26 @@ static void init_channel_elem(grpc_exec_ctx *exec_ctx,
                               grpc_channel_element *elem,
                               grpc_channel_element_args *args) {
   channel_data *channeld = elem->channel_data;
-  grpc_compression_algorithm algo_idx;
 
-  grpc_compression_options_init(&channeld->compression_options);
-  channeld->compression_options.enabled_algorithms_bitset =
-      (uint32_t)grpc_channel_args_compression_algorithm_get_states(
-          args->channel_args);
+  channeld->enabled_algorithms_bitset =
+      grpc_channel_args_compression_algorithm_get_states(args->channel_args);
 
   channeld->default_compression_algorithm =
       grpc_channel_args_get_compression_algorithm(args->channel_args);
   /* Make sure the default isn't disabled. */
-  if (!grpc_compression_options_is_algorithm_enabled(
-          &channeld->compression_options,
-          channeld->default_compression_algorithm)) {
+  if (!GPR_BITGET(channeld->enabled_algorithms_bitset,
+                  channeld->default_compression_algorithm)) {
     gpr_log(GPR_DEBUG,
             "compression algorithm %d not enabled: switching to none",
             channeld->default_compression_algorithm);
     channeld->default_compression_algorithm = GRPC_COMPRESS_NONE;
   }
-  channeld->compression_options.default_compression_algorithm =
-      channeld->default_compression_algorithm;
 
   channeld->supported_compression_algorithms = 0;
-  for (algo_idx = 0; algo_idx < GRPC_COMPRESS_ALGORITHMS_COUNT; ++algo_idx) {
+  for (grpc_compression_algorithm algo_idx = 0;
+       algo_idx < GRPC_COMPRESS_ALGORITHMS_COUNT; ++algo_idx) {
     /* skip disabled algorithms */
-    if (grpc_compression_options_is_algorithm_enabled(
-            &channeld->compression_options, algo_idx) == 0) {
+    if (!GPR_BITGET(channeld->enabled_algorithms_bitset, algo_idx)) {
       continue;
     }
     channeld->supported_compression_algorithms |= 1u << algo_idx;
diff --git a/src/core/lib/channel/compress_filter.h b/src/core/lib/channel/compress_filter.h
index cf5879d82e..059bd8e819 100644
--- a/src/core/lib/channel/compress_filter.h
+++ b/src/core/lib/channel/compress_filter.h
@@ -34,9 +34,9 @@
 #ifndef GRPC_CORE_LIB_CHANNEL_COMPRESS_FILTER_H
 #define GRPC_CORE_LIB_CHANNEL_COMPRESS_FILTER_H
 
-#include "src/core/lib/channel/channel_stack.h"
+#include <grpc/impl/codegen/compression_types.h>
 
-#define GRPC_COMPRESS_REQUEST_ALGORITHM_KEY "grpc-internal-encoding-request"
+#include "src/core/lib/channel/channel_stack.h"
 
 extern int grpc_compress_filter_trace;
 
@@ -48,7 +48,7 @@ extern int grpc_compress_filter_trace;
  *  - Channel configuration, as established at channel creation time.
  *  - The metadata accompanying the outgoing data to be compressed. This is
  *    taken as a request only. We may choose not to honor it. The metadata key
- *    is given by \a GRPC_COMPRESS_REQUEST_ALGORITHM_KEY.
+ *    is given by \a GRPC_COMPRESSION_REQUEST_ALGORITHM_MD_KEY.
  *
  * Compression can be disabled for concrete messages (for instance in order to
  * prevent CRIME/BEAST type attacks) by having the GRPC_WRITE_NO_COMPRESS set in
diff --git a/src/core/lib/compression/compression_algorithm.c b/src/core/lib/compression/compression_algorithm.c
index 7039364b7b..6d47bbd38d 100644
--- a/src/core/lib/compression/compression_algorithm.c
+++ b/src/core/lib/compression/compression_algorithm.c
@@ -183,7 +183,8 @@ grpc_compression_algorithm grpc_compression_algorithm_for_level(
 
 void grpc_compression_options_init(grpc_compression_options *opts) {
   opts->enabled_algorithms_bitset = (1u << GRPC_COMPRESS_ALGORITHMS_COUNT) - 1;
-  opts->default_compression_algorithm = GRPC_COMPRESS_NONE;
+  opts->default_level.is_set = false;
+  opts->default_algorithm.is_set = false;
 }
 
 void grpc_compression_options_enable_algorithm(
diff --git a/src/cpp/client/client_context.cc b/src/cpp/client/client_context.cc
index 32c7794ade..d3e5ce0c4a 100644
--- a/src/cpp/client/client_context.cc
+++ b/src/cpp/client/client_context.cc
@@ -33,15 +33,14 @@
 
 #include <grpc++/client_context.h>
 
-#include <grpc++/security/credentials.h>
-#include <grpc++/server_context.h>
-#include <grpc++/support/time.h>
 #include <grpc/compression.h>
 #include <grpc/grpc.h>
 #include <grpc/support/alloc.h>
 #include <grpc/support/string_util.h>
 
-#include "src/core/lib/channel/compress_filter.h"
+#include <grpc++/security/credentials.h>
+#include <grpc++/server_context.h>
+#include <grpc++/support/time.h>
 
 namespace grpc {
 
@@ -112,7 +111,7 @@ void ClientContext::set_compression_algorithm(
     abort();
   }
   GPR_ASSERT(algorithm_name != nullptr);
-  AddMetadata(GRPC_COMPRESS_REQUEST_ALGORITHM_KEY, algorithm_name);
+  AddMetadata(GRPC_COMPRESSION_REQUEST_ALGORITHM_MD_KEY, algorithm_name);
 }
 
 void ClientContext::TryCancel() {
diff --git a/src/cpp/server/server_builder.cc b/src/cpp/server/server_builder.cc
index 61f0f6ae2a..e6050c3b52 100644
--- a/src/cpp/server/server_builder.cc
+++ b/src/cpp/server/server_builder.cc
@@ -125,6 +125,13 @@ std::unique_ptr<Server> ServerBuilder::BuildAndStart() {
   }
   args.SetInt(GRPC_COMPRESSION_CHANNEL_ENABLED_ALGORITHMS_BITSET,
               compression_options_.enabled_algorithms_bitset);
+  if (compression_options_.default_level.is_set) {
+    args.SetInt(GRPC_COMPRESSION_CHANNEL_DEFAULT_LEVEL,
+                compression_options_.default_level.level);
+  } else if (compression_options_.default_algorithm.is_set) {
+    args.SetInt(GRPC_COMPRESSION_CHANNEL_DEFAULT_ALGORITHM,
+                compression_options_.default_algorithm.algorithm);
+  }
   std::unique_ptr<Server> server(
       new Server(thread_pool.release(), true, max_message_size_, &args));
   ServerInitializer* initializer = server->initializer();
diff --git a/src/cpp/server/server_context.cc b/src/cpp/server/server_context.cc
index 204fef1b09..43117fd1e9 100644
--- a/src/cpp/server/server_context.cc
+++ b/src/cpp/server/server_context.cc
@@ -42,7 +42,6 @@
 #include <grpc/support/alloc.h>
 #include <grpc/support/log.h>
 
-#include "src/core/lib/channel/compress_filter.h"
 #include "src/core/lib/surface/call.h"
 
 namespace grpc {
@@ -196,6 +195,9 @@ bool ServerContext::IsCancelled() const {
 }
 
 void ServerContext::set_compression_level(grpc_compression_level level) {
+  // TODO(dgq): get rid of grpc_call_compression_for_level and propagate the
+  // compression level by adding a new argument to
+  // CallOpSendInitialMetadata::SendInitialMetadata.
   const grpc_compression_algorithm algorithm_for_level =
       grpc_call_compression_for_level(call_, level);
   set_compression_algorithm(algorithm_for_level);
@@ -210,7 +212,7 @@ void ServerContext::set_compression_algorithm(
     abort();
   }
   GPR_ASSERT(algorithm_name != NULL);
-  AddInitialMetadata(GRPC_COMPRESS_REQUEST_ALGORITHM_KEY, algorithm_name);
+  AddInitialMetadata(GRPC_COMPRESSION_REQUEST_ALGORITHM_MD_KEY, algorithm_name);
 }
 
 grpc::string ServerContext::peer() const {
diff --git a/test/core/end2end/tests/compressed_payload.c b/test/core/end2end/tests/compressed_payload.c
index 589bc314f8..4165f35789 100644
--- a/test/core/end2end/tests/compressed_payload.c
+++ b/test/core/end2end/tests/compressed_payload.c
@@ -38,13 +38,13 @@
 
 #include <grpc/byte_buffer.h>
 #include <grpc/byte_buffer_reader.h>
+#include <grpc/impl/codegen/compression_types.h>
 #include <grpc/support/alloc.h>
 #include <grpc/support/log.h>
 #include <grpc/support/time.h>
 #include <grpc/support/useful.h>
 
 #include "src/core/lib/channel/channel_args.h"
-#include "src/core/lib/channel/compress_filter.h"
 #include "src/core/lib/surface/call_test_only.h"
 #include "test/core/end2end/cq_verifier.h"
 
@@ -302,13 +302,13 @@ static void test_invoke_request_with_compressed_payload_md_override(
   grpc_metadata gzip_compression_override;
   grpc_metadata none_compression_override;
 
-  gzip_compression_override.key = GRPC_COMPRESS_REQUEST_ALGORITHM_KEY;
+  gzip_compression_override.key = GRPC_COMPRESSION_REQUEST_ALGORITHM_MD_KEY;
   gzip_compression_override.value = "gzip";
   gzip_compression_override.value_length = 4;
   memset(&gzip_compression_override.internal_data, 0,
          sizeof(gzip_compression_override.internal_data));
 
-  none_compression_override.key = GRPC_COMPRESS_REQUEST_ALGORITHM_KEY;
+  none_compression_override.key = GRPC_COMPRESSION_REQUEST_ALGORITHM_MD_KEY;
   none_compression_override.value = "identity";
   none_compression_override.value_length = 4;
   memset(&none_compression_override.internal_data, 0,