Mon Mar 26 13:11:00 UTC 2012 Simon Massey <simon dot massey at prismtech dot com>

        * tests/ZIOP/client.cpp:
          Correct test 4 checks for compression being used. (Was hidden as the client
          was selecting the wrong compressor).

        * tao/ZIOP/ZIOP.cpp:
          Updated the debugging messages (Give compressor level found in each list
          with compressor name).

        * tao/ZIOP/ZIOP_Stub.cpp:
          Correct client priority compressor selection for sening requests to the
          server. The client was selecting compression based upon the Server
          compressor list priority, not the client compressor list priority, and
          could selecty compression even when the server didn't publish any compression
          availablitiy.

4 files changed, 138 insertions, 90 deletions

diff --git a/TAO/ChangeLog b/TAO/ChangeLog
index b2e27e5955b..8ddc52e47aa 100644
--- a/TAO/ChangeLog
+++ b/TAO/ChangeLog
@@ -1,11 +1,19 @@
-Fri Mar 23 23:29:21 UTC 2012  Johnny Willemsen  <jwillemsen@remedy.nl>
+Mon Mar 26 13:11:00 UTC 2012  Simon Massey  <simon dot massey at prismtech dot com>
 
-        * TAO_IDL/be/be_visitor_component/executor_exh.cpp:
-        * TAO_IDL/be/be_visitor_component/executor_private_exh.cpp:
-        * TAO_IDL/be/be_visitor_component/facet_exh.cpp:
-        * TAO_IDL/be/be_visitor_component_scope.cpp:
-        * TAO_IDL/be/be_visitor_home/home_exh.cpp:
-          Generate more documentation into the generated executor
+        * tests/ZIOP/client.cpp:
+          Correct test 4 checks for compression being used. (Was hidden as the client
+          was selecting the wrong compressor).
+
+        * tao/ZIOP/ZIOP.cpp:
+          Updated the debugging messages (Give compressor level found in each list
+          with compressor name).
+
+        * tao/ZIOP/ZIOP_Stub.cpp:
+          Correct client priority compressor selection for sening requests to the
+          server. The client was selecting compression based upon the Server
+          compressor list priority, not the client compressor list priority, and
+          could selecty compression even when the server didn't publish any compression
+          availablitiy.
 
 Fri Mar 23 21:57:44 UTC 2012  Johnny Willemsen  <jwillemsen@remedy.nl>
 
diff --git a/TAO/tao/ZIOP/ZIOP.cpp b/TAO/tao/ZIOP/ZIOP.cpp
index 8aa795dec32..50ab4e97340 100644
--- a/TAO/tao/ZIOP/ZIOP.cpp
+++ b/TAO/tao/ZIOP/ZIOP.cpp
@@ -709,16 +709,18 @@ TAO_ZIOP_Loader::marshal_data (TAO_OutputCDR &cdr, TAO_ORB_Core &orb_core, TAO_S
               Compression::CompressionLevel
                 compression_level = ACE_MIN (serverEntry->compression_level,
                                              clientEntry->compression_level);
-              if (7 < TAO_debug_level)
+              if (6 < TAO_debug_level)
                 {
                   ACE_DEBUG ((LM_DEBUG,
                               ACE_TEXT("TAO (%P|%t) - ")
                               ACE_TEXT("TAO_ZIOP_Loader::marshal_data (server_reply), ")
-                              ACE_TEXT("Found (Server %d: %s == Client %d: %s) level %d.\n"),
+                              ACE_TEXT("Found (Server %d: %s@%d == Client %d: %s@%d) using @%d.\n"),
                               server,
                               this->ziop_compressorid_name (serverEntry->compressor_id),
+                              serverEntry->compression_level,
                               client,
                               this->ziop_compressorid_name (clientEntry->compressor_id),
+                              clientEntry->compression_level,
                               compression_level));
                 }
 
@@ -747,11 +749,13 @@ TAO_ZIOP_Loader::marshal_data (TAO_OutputCDR &cdr, TAO_ORB_Core &orb_core, TAO_S
               ACE_DEBUG ((LM_DEBUG,
                           ACE_TEXT("TAO (%P|%t) - ")
                           ACE_TEXT("TAO_ZIOP_Loader::marshal_data (server_reply), ")
-                          ACE_TEXT("checking (Server %d: %s != Client %d: %s).\n"),
+                          ACE_TEXT("checking (Server %d: %s@%d != Client %d: %s@%d).\n"),
                           server,
                           this->ziop_compressorid_name (serverEntry->compressor_id),
+                          serverEntry->compression_level,
                           client,
-                          this->ziop_compressorid_name (clientEntry->compressor_id)));
+                          this->ziop_compressorid_name (clientEntry->compressor_id),
+                          clientEntry->compression_level));
             }
         } // next clientEntry
     } // next serverEntry
diff --git a/TAO/tao/ZIOP/ZIOP_Stub.cpp b/TAO/tao/ZIOP/ZIOP_Stub.cpp
index 4645a807e87..7aec9caea8c 100644
--- a/TAO/tao/ZIOP/ZIOP_Stub.cpp
+++ b/TAO/tao/ZIOP/ZIOP_Stub.cpp
@@ -176,99 +176,134 @@ TAO_ZIOP_Stub::effective_compression_enabling_policy (void)
 CORBA::Policy *
 TAO_ZIOP_Stub::effective_compression_id_list_policy (void)
 {
-  // Get effective override.
-  CORBA::Policy_var override =
+  // Get the value from the IOR (This is the SERVERS available compressor's list).
+  CORBA::Policy_var policy (
+    this->exposed_compression_id_list_policy ());
+  ZIOP::CompressorIdLevelListPolicy_var serverCompressors (
+    ZIOP::CompressorIdLevelListPolicy::_narrow (policy.in ()));
+  // If SERVER does not have an available compressor's list, compression can't go ahead.
+  if (CORBA::is_nil (serverCompressors.in ()))
+    {
+      if (6 < TAO_debug_level)
+        {
+          ACE_DEBUG ((LM_DEBUG,
+            ACE_TEXT ("TAO (%P|%t) - ")
+            ACE_TEXT ("TAO_ZIOP_Stub::effective_compression_id_list_policy, ")
+            ACE_TEXT ("no serverCompressorIdLevelListPolicy (did not compress).")));
+        }
+      return 0;
+    }
+  ::Compression::CompressorIdLevelList &serverList =
+    *serverCompressors->compressor_ids ();
+
+  // Get effective override (This is the CLIENTS compressor's priority ordered list).
+  policy =
     this->TAO_Stub::get_cached_policy (TAO_CACHED_COMPRESSION_ID_LEVEL_LIST_POLICY);
+  ZIOP::CompressorIdLevelListPolicy_var clientCompressors (
+    ZIOP::CompressorIdLevelListPolicy::_narrow (policy.in ()));
+  // Likewise if CLIENT does not have a compressor's list, compression can't go ahead.
+  if (CORBA::is_nil (clientCompressors.in ()))
+    {
+      if (6 < TAO_debug_level)
+        {
+          ACE_DEBUG ((LM_DEBUG,
+            ACE_TEXT ("TAO (%P|%t) - ")
+            ACE_TEXT ("TAO_ZIOP_Stub::effective_compression_id_list_policy, ")
+            ACE_TEXT ("no clientCompressorIdLevelListPolicy (did not compress).")));
+        }
+      return 0;
+    }
+  ::Compression::CompressorIdLevelList &clientList =
+    *clientCompressors->compressor_ids ();
 
-  // Get the value from the ior.
-  CORBA::Policy_var exposed =
-    this->exposed_compression_id_list_policy ();
+  // For each CLIENT compressor (in priority order) check...
+  for (CORBA::ULong client = 0u; client < clientList.length (); ++client)
+    {
+      ::Compression::CompressorIdLevel_var clientEntry (clientList[client]);
 
-  ZIOP::CompressorIdLevelListPolicy_var override_policy_var =
-    ZIOP::CompressorIdLevelListPolicy::_narrow (override.in ());
+      // ... which each SERVER compressor id if it is available to use.
+      for (CORBA::ULong server = 0u; server < serverList.length (); ++server)
+        {
+          ::Compression::CompressorIdLevel_var serverEntry (serverList[server]);
 
-  ZIOP::CompressorIdLevelListPolicy_var exposed_policy_var =
-    ZIOP::CompressorIdLevelListPolicy::_narrow (exposed.in ());
+          if (clientEntry->compressor_id == serverEntry->compressor_id)
+            {
+              // OK we found a match, however we need to make the highest priority
+              // compressor (the one we are going to employ) use slot 0 in the
+              // list we are returning. Since we can't modify the original list
+              // we have to make a copy and modify that.
+              policy= clientCompressors->copy ();
+              ZIOP::CompressorIdLevelListPolicy_var returningCompressors (
+                ZIOP::CompressorIdLevelListPolicy::_narrow (policy.in ()));
+              if (CORBA::is_nil (returningCompressors.in ()))
+                {
+                  // This shouldn't happen, it's basically an internal error.
+                  if (6 < TAO_debug_level)
+                    {
+                      ACE_DEBUG ((LM_DEBUG,
+                        ACE_TEXT ("TAO (%P|%t) - ")
+                        ACE_TEXT ("TAO_ZIOP_Stub::effective_compression_id_list_policy, ")
+                        ACE_TEXT ("could not copy clientCompressorIdLevelListPolicy (did not compress).")));
+                    }
+                  return 0;
+                }
+              ::Compression::CompressorIdLevelList &returningList =
+                *returningCompressors->compressor_ids ();
 
-    // Reconcile client-exposed and locally set values.
-  if (CORBA::is_nil (exposed_policy_var.in ()))
-    return override._retn ();
+              // We must ensure the other compressors are not lost, as they tell the server
+              // which compressors are available for it to use with the reply message.
+              for (CORBA::ULong shuffle = client; 0u < shuffle; --shuffle)
+                {
+                  returningList[shuffle].compressor_id=     returningList[shuffle-1u].compressor_id;
+                  returningList[shuffle].compression_level= returningList[shuffle-1u].compression_level;
+                }
 
-  if (CORBA::is_nil (override_policy_var.in ()))
-    return exposed._retn ();
+              // The one we found is the one we are going to use (now the highest priority)
+              // but with the correct (minimized) compression level of the client and server.
+              returningList[0].compressor_id=     clientEntry->compressor_id;
+              returningList[0].compression_level= ACE_MIN (clientEntry->compression_level,
+                                                           serverEntry->compression_level);
+              if (6 < TAO_debug_level)
+                {
+                  ACE_DEBUG ((LM_DEBUG,
+                              ACE_TEXT ("TAO (%P|%t) - ")
+                              ACE_TEXT ("TAO_ZIOP_Stub::effective_compression_id_list_policy, ")
+                              ACE_TEXT ("found (Client %d: %s@%d == Server %d: %s@%d) using @%d.\n"),
+                              client,
+                              TAO_ZIOP_Loader::ziop_compressorid_name (clientEntry->compressor_id),
+                              clientEntry->compression_level,
+                              server,
+                              TAO_ZIOP_Loader::ziop_compressorid_name (serverEntry->compressor_id),
+                              serverEntry->compression_level,
+                              returningList[0].compression_level));
+                }
 
-  // Check which compressor id we should use and which level
-  for (CORBA::ULong nr_exposed = 0;
-        nr_exposed < exposed_policy_var->compressor_ids ()->length ();
-        ++nr_exposed)
-    {
-      ::Compression::CompressorIdLevel_var exposed_compressor =
-        exposed_policy_var->compressor_ids ()->operator [](nr_exposed);
+              return returningCompressors._retn ();
+            }
 
-      if (TAO_debug_level > 9)
-        {
-          ACE_DEBUG ((LM_DEBUG,
-                      ACE_TEXT ("TAO (%P|%t) - ")
-                      ACE_TEXT ("TAO_ZIOP_Stub::effective_compression_id_list_policy, ")
-                      ACE_TEXT ("exposed_policy = %d, compressor_id = %C, ")
-                      ACE_TEXT ("compression_level = %d\n"),
-                      nr_exposed,
-                      TAO_ZIOP_Loader::ziop_compressorid_name (
-                            exposed_compressor.ptr ()->compressor_id),
-                      exposed_compressor.ptr ()->compression_level));
-        }
-      // check if a local policy matches this exposed policy
-      for (CORBA::ULong nr_override = 0;
-            nr_override < override_policy_var->compressor_ids ()->length ();
-            ++nr_override)
-        {
-          ::Compression::CompressorIdLevel_var override_compressor =
-            override_policy_var->compressor_ids ()->operator [] (nr_override);
-          if (TAO_debug_level > 9)
+          if (7 < TAO_debug_level)
             {
               ACE_DEBUG ((LM_DEBUG,
                           ACE_TEXT ("TAO (%P|%t) - ")
                           ACE_TEXT ("TAO_ZIOP_Stub::effective_compression_id_list_policy, ")
-                          ACE_TEXT ("checking override_policy = %d, compressor_id = %C, ")
-                          ACE_TEXT ("compression_level = %d\n"),
-                          nr_override,
-                          TAO_ZIOP_Loader::ziop_compressorid_name (
-                                override_compressor->compressor_id),
-                          override_compressor->compression_level));
+                          ACE_TEXT ("checking (Client %d: %s@%d != Server %d: %s@%d).\n"),
+                          client,
+                          TAO_ZIOP_Loader::ziop_compressorid_name (clientEntry->compressor_id),
+                          clientEntry->compression_level,
+                          server,
+                          TAO_ZIOP_Loader::ziop_compressorid_name (serverEntry->compressor_id),
+                          serverEntry->compression_level));
             }
-          if (override_compressor->compressor_id ==
-              exposed_compressor->compressor_id)
-            {
-              CORBA::Policy_var tmp_policy = override_policy_var->copy ();
-              ZIOP::CompressorIdLevelListPolicy_var idlevellist_policy_var =
-                    ZIOP::CompressorIdLevelListPolicy::_narrow (tmp_policy.in ());
-              if (CORBA::is_nil (idlevellist_policy_var.in ()))
-                return override._retn ();
-
-              ::Compression::CompressorIdLevelList &entries =
-                *idlevellist_policy_var->compressor_ids ();
-
-              // Since the CompressorIdLevelListPolicy holds a prioritized list
-              // of the compressors we are allowed to use, and this is sent
-              // across to the server, BUT we are going to always use entry 0
-              // here at the client; we must ensure the other compressors are not
-              // lost but that the chosen compressor is placed first in the list
-              // (using the correctly minimized compression_level).
-              for (CORBA::ULong shuffle = nr_override; 0u < shuffle; --shuffle)
-                {
-                  entries[shuffle].compressor_id=     entries[shuffle-1u].compressor_id;
-                  entries[shuffle].compression_level= entries[shuffle-1u].compression_level;
-                }
-              entries[0].compressor_id=     override_compressor->compressor_id;
-              entries[0].compression_level= ACE_MIN (
-                override_compressor->compression_level,
-                exposed_compressor->compression_level);
+        } // next serverEntry
+    } // next clientEntry
 
-              return idlevellist_policy_var._retn ();
-            }
-        }
+  if (6 < TAO_debug_level)
+    {
+      ACE_DEBUG ((LM_DEBUG,
+                  ACE_TEXT("TAO (%P|%t) - ")
+                  ACE_TEXT ("TAO_ZIOP_Stub::effective_compression_id_list_policy, ")
+                  ACE_TEXT("no matching CompressorIdLevelListPolicy (did not compress).\n")));
     }
-
   return 0;
 }
 
diff --git a/TAO/tests/ZIOP/client.cpp b/TAO/tests/ZIOP/client.cpp
index 87472ce14f7..47dfa9eddf3 100644
--- a/TAO/tests/ZIOP/client.cpp
+++ b/TAO/tests/ZIOP/client.cpp
@@ -173,6 +173,7 @@ create_policies (CORBA::ORB_ptr orb, bool add_zlib_compressor)
 Test::Hello_var
 prepare_tests (CORBA::ORB_ptr orb, bool create_factories=true)
 {
+
 #if defined TAO_HAS_ZIOP && TAO_HAS_ZIOP == 1
   if (create_factories)
     register_factories(orb);
@@ -215,6 +216,7 @@ check_results (CORBA::ORB_ptr orb)
           return start_tests (hello.in (), orb);
         }
       break;
+    case 4:
     case -1:
       {
         ::Compression::Compressor_ptr compressor =
@@ -244,7 +246,6 @@ check_results (CORBA::ORB_ptr orb)
       return 0;
       break;
     case 3:
-    case 4:
       {
         // low value policy test. No compression should be used.
         ::Compression::Compressor_ptr compressor =