GEODE-10076. Align String serialization with Java client

apache · May 20, 2022 · a71b6f0 · a71b6f0
1 parent 4d0578a
commit a71b6f0
Show file tree

Hide file tree

Showing 6 changed files with 198 additions and 23 deletions.
diff --git a/cppcache/include/geode/DataOutput.hpp b/cppcache/include/geode/DataOutput.hpp
@@ -272,14 +272,35 @@ class APACHE_GEODE_EXPORT DataOutput {
 
   template <class _CharT, class... _Tail>
   inline void writeString(const std::basic_string<_CharT, _Tail...>& value) {
-    // without scanning string, making worst case choices.
-    // TODO constexp for each string type to jmutf8 length conversion
-    if (value.length() * 3 <= (std::numeric_limits<uint16_t>::max)()) {
-      write(static_cast<uint8_t>(DSCode::CacheableString));
-      writeJavaModifiedUtf8(value);
+    auto len = value.length();
+    auto utfLen = len;
+    for (unsigned int i = 0; i < len; i++) {
+      auto c = value.at(i);
+      if ((c <= 0x007F) && (c >= 0x0001)) {
+        // nothing needed
+      } else if (c > 0x07FF) {
+        utfLen += 2;
+      } else {
+        utfLen += 1;
+      }
+    }
+    auto writeUTF = utfLen > len;
+    if (writeUTF) {
+      if (utfLen > 0xFFFF) {
+        write(static_cast<uint8_t>(DSCode::CacheableStringHuge));
+        writeUtf16Huge(value);
+      } else {
+        write(static_cast<uint8_t>(DSCode::CacheableString));
+        writeJavaModifiedUtf8(value);
+      }
     } else {
-      write(static_cast<uint8_t>(DSCode::CacheableStringHuge));
-      writeUtf16Huge(value);
+      if (len > 0xFFFF) {
+        write(static_cast<uint8_t>(DSCode::CacheableASCIIStringHuge));
+        writeAsciiHuge(value);
+      } else {
+        write(static_cast<uint8_t>(DSCode::CacheableASCIIString));
+        writeAscii(value);
+      }
     }
   }
 
@@ -515,7 +536,8 @@ class APACHE_GEODE_EXPORT DataOutput {
   const CacheImpl* m_cache;
   Pool* m_pool;
 
-  inline void writeAscii(const std::string& value) {
+  template <class _CharT, class... _Tail>
+  inline void writeAscii(const std::basic_string<_CharT, _Tail...>& value) {
     uint16_t len = static_cast<uint16_t>(std::min<size_t>(
         value.length(), (std::numeric_limits<uint16_t>::max)()));
     writeInt(len);
@@ -525,7 +547,8 @@ class APACHE_GEODE_EXPORT DataOutput {
     }
   }
 
-  inline void writeAsciiHuge(const std::string& value) {
+  template <class _CharT, class... _Tail>
+  inline void writeAsciiHuge(const std::basic_string<_CharT, _Tail...>& value) {
     uint32_t len = static_cast<uint32_t>(std::min<size_t>(
         value.length(), (std::numeric_limits<uint32_t>::max)()));
     writeInt(static_cast<uint32_t>(len));

diff --git a/cppcache/integration/test/PdxInstanceTest.cpp b/cppcache/integration/test/PdxInstanceTest.cpp
@@ -23,6 +23,7 @@
 #include <gtest/gtest.h>
 
 #include <geode/Cache.hpp>
+#include <geode/FunctionService.hpp>
 #include <geode/PdxInstanceFactory.hpp>
 #include <geode/PoolManager.hpp>
 #include <geode/RegionFactory.hpp>
@@ -37,11 +38,13 @@
 namespace {
 
 using apache::geode::client::Cache;
+using apache::geode::client::CacheableBoolean;
 using apache::geode::client::CacheableKey;
 using apache::geode::client::CacheableString;
 using apache::geode::client::CacheFactory;
 using apache::geode::client::CacheListenerMock;
 using apache::geode::client::CacheRegionHelper;
+using apache::geode::client::FunctionService;
 using apache::geode::client::IllegalStateException;
 using apache::geode::client::LocalRegion;
 using apache::geode::client::PdxInstance;
@@ -268,7 +271,7 @@ TEST(PdxInstanceTest, testPdxInstance) {
       pdxTypeFromPdxTypeInstance->equals(*pdxTypeFromPdxTypeInstanceGet, false))
       << "PdxObjects should be equal.";
 
-  EXPECT_EQ(-960665662, pdxTypeInstance->hashcode())
+  EXPECT_EQ(-1302455415, pdxTypeInstance->hashcode())
       << "Pdxhashcode hashcode not matched with java pdx hash code.";
 }
 
@@ -434,4 +437,45 @@ TEST(PdxInstanceTest, testInstancePutAfterRestart) {
   EXPECT_EQ(result->size(), 1UL);
 }
 
+TEST(PdxInstanceTest, comparePdxInstanceWithStringWithJavaPdxInstance) {
+  Cluster cluster{LocatorCount{1}, ServerCount{1}};
+  cluster.start([&]() {
+    cluster.getGfsh()
+        .deploy()
+        .jar(getFrameworkString(FrameworkVariable::JavaObjectJarPath))
+        .execute();
+  });
+
+  auto& gfsh = cluster.getGfsh();
+  auto cache = createTestCache();
+  auto poolFactory = cache.getPoolManager().createFactory();
+  cluster.applyLocators(poolFactory);
+  auto pool = poolFactory.create("default");
+
+  std::shared_ptr<PdxInstance> entry;
+  {
+    auto pdxInstanceFactory =
+        cache.createPdxInstanceFactory("PdxTests.MyTestClass", false);
+
+    pdxInstanceFactory.writeString("asciiField", "value");
+    pdxInstanceFactory.writeString("utf8Field", "value€");
+    pdxInstanceFactory.writeString("asciiHugeField", std::string(70000, 'x'));
+    pdxInstanceFactory.writeString("utfHugeField",
+                                   std::string(70000, 'x') + "€");
+    entry = pdxInstanceFactory.create();
+  }
+
+  auto execution = FunctionService::onServers(pool);
+  auto rc = execution.withArgs(entry).execute("ComparePdxInstanceFunction");
+  EXPECT_TRUE(rc);
+
+  auto rs = rc->getResult();
+  EXPECT_TRUE(rs);
+  EXPECT_EQ(rs->size(), 1);
+
+  auto flag = std::dynamic_pointer_cast<CacheableBoolean>((*rs)[0]);
+  EXPECT_TRUE(flag);
+  EXPECT_TRUE(flag->value());
+}
+
 }  // namespace
diff --git a/cppcache/test/DataOutputTest.cpp b/cppcache/test/DataOutputTest.cpp
@@ -17,7 +17,6 @@
 
 #include <stdint.h>
 
-#include <limits>
 #include <random>
 
 #include <gtest/gtest.h>
@@ -210,14 +209,44 @@ TEST_F(DataOutputTest, TestWriteDouble) {
   EXPECT_BYTEARRAY_EQ("400921FB54442EEA", dataOutput.getByteArray());
 }
 
-TEST_F(DataOutputTest, TestWriteString) {
+TEST_F(DataOutputTest, TestWriteStringAscii) {
   TestDataOutput dataOutput(nullptr);
   dataOutput.writeString("You had me at meat tornado.");
   EXPECT_BYTEARRAY_EQ(
-      "2A001B596F7520686164206D65206174206D65617420746F726E61646F2E",
+      "57001B596F7520686164206D65206174206D65617420746F726E61646F2E",
       dataOutput.getByteArray());
 }
 
+TEST_F(DataOutputTest, TestWriteStringUtf8) {
+  TestDataOutput dataOutput(nullptr);
+  dataOutput.writeString("You had me at meat tornado. €");
+  EXPECT_BYTEARRAY_EQ(
+      "2A001F596F7520686164206D65206174206D65617420746F726E61646F2E20E282AC",
+      dataOutput.getByteArray());
+}
+
+TEST_F(DataOutputTest, TestWriteStringAsciiHuge) {
+  TestDataOutput dataOutput(nullptr);
+  const unsigned int stringSize = 70000;
+  std::string testString(stringSize, 'x');
+  dataOutput.writeString(testString);
+
+  // 1 (dscode) + 4 (length) + string size
+  EXPECT_EQ(dataOutput.getByteArray().size(), 1 + 4 + stringSize);
+  EXPECT_EQ(dataOutput.getByteArray()[0], 88);
+}
+
+TEST_F(DataOutputTest, TestWriteStringUtfHuge) {
+  TestDataOutput dataOutput(nullptr);
+  const unsigned int stringSize = 70000;
+  std::string testString(stringSize, 'x');
+  dataOutput.writeString(testString + "€");
+
+  // 1 (dscode) + 4 (length) + 2*string size + 2 (size of non ASCII char)
+  EXPECT_EQ(dataOutput.getByteArray().size(), 1 + 4 + (2 * stringSize) + 2);
+  EXPECT_EQ(dataOutput.getByteArray()[0], 89);
+}
+
 TEST_F(DataOutputTest, TestWriteUTF) {
   TestDataOutput dataOutput(nullptr);
   dataOutput.writeUTF("You had me at meat tornado.");

diff --git a/cppcache/test/PdxTypeTest.cpp b/cppcache/test/PdxTypeTest.cpp
@@ -207,10 +207,10 @@ TEST_F(PdxTypeTest, testSerializeJavaPdxType) {
   pdx_type.toData(out);
 
   EXPECT_BYTEARRAY_EQ(
-      "2D2B2A00256F72672E6170616368652E67656F64652E7064782E696E7465726E"
-      "616C2E506478547970652A000E5F5F47454D464952455F4A534F4E0000000000"
-      "00000001032A0003666F6F0000000000000000090000000000000000002A0005"
-      "616C6963650000000100000001090000000000000000002A0004626172310000"
+      "2D2B5700256F72672E6170616368652E67656F64652E7064782E696E7465726E"
+      "616C2E5064785479706557000E5F5F47454D464952455F4A534F4E0000000000"
+      "0000000103570003666F6F000000000000000009000000000000000000570005"
+      "616C696365000000010000000109000000000000000000570004626172310000"
       "00020000000000000000000000000000",
       ByteArray(out.getBuffer(), out.getBufferLength()));
 }
@@ -228,10 +228,10 @@ TEST_F(PdxTypeTest, testSerializeNoJavaPdxType) {
   pdx_type.toData(out);
 
   EXPECT_BYTEARRAY_EQ(
-      "2D2B2A00256F72672E6170616368652E67656F64652E7064782E696E7465726E"
-      "616C2E506478547970652A000E5F5F47454D464952455F4A534F4E0100000000"
-      "00000001032A0003666F6F0000000000000000090000000000000000002A0005"
-      "616C6963650000000100000001090000000000000000002A0004626172310000"
+      "2D2B5700256F72672E6170616368652E67656F64652E7064782E696E7465726E"
+      "616C2E5064785479706557000E5F5F47454D464952455F4A534F4E0100000000"
+      "0000000103570003666F6F000000000000000009000000000000000000570005"
+      "616C696365000000010000000109000000000000000000570004626172310000"
       "00020000000000000000000000000000",
       ByteArray(out.getBuffer(), out.getBufferLength()));
 }

diff --git a/cppcache/test/QueueConnectionRequestTest.cpp b/cppcache/test/QueueConnectionRequestTest.cpp
@@ -55,8 +55,8 @@ TEST_F(QueueConnectionRequestTest, testToData) {
   queueConnReq.toData(dataOutput);
 
   EXPECT_BYTEARRAY_EQ(
-      "2A0000012631015C047F000001000000022A00046E616D65000000302E\\h{8}"
-      "0D002A000664734E616D652A000772616E644E756D7D00000001FFFFFFFF000000012A00"
+      "570000012631015C047F000001000000025700046E616D65000000302E\\h{8}"
+      "0D0057000664734E616D6557000772616E644E756D7D00000001FFFFFFFF000000015700"
       "067365727665720000000A00",
       ByteArray(dataOutput.getBuffer(), dataOutput.getBufferLength()));
 }

diff --git a/tests/javaobject/ComparePdxInstanceFunction.java b/tests/javaobject/ComparePdxInstanceFunction.java
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package javaobject;
+
+import java.util.*;
+import java.io.*;
+import org.apache.geode.*; // for DataSerializable
+import org.apache.geode.cache.execute.FunctionContext;
+import org.apache.geode.cache.execute.Function;
+import org.apache.geode.pdx.PdxInstance;
+import org.apache.geode.pdx.PdxInstanceFactory;
+
+public class ComparePdxInstanceFunction implements Function {
+
+  @Override
+  public void execute(FunctionContext context) {
+    Object arguments = context.getArguments();
+    PdxInstance instance = (PdxInstance) arguments;
+
+    char array[] = new char[70000];
+    Arrays.fill(array, 'x');
+    String longString = new String(array);
+
+    PdxInstanceFactory pdxInstanceFactory = context.getCache().createPdxInstanceFactory("PdxTests.MyTestClass");
+    pdxInstanceFactory.writeString("asciiField", "value");
+    pdxInstanceFactory.writeString("utf8Field", "value€");
+    pdxInstanceFactory.writeString("asciiHugeField", longString);
+    pdxInstanceFactory.writeString("utfHugeField", longString + "€");
+
+    PdxInstance expectedInstance = pdxInstanceFactory.create();
+
+    boolean result = expectedInstance.equals(instance);
+
+    context.getResultSender().lastResult(result);
+  }
+
+  private static final String ID = "ComparePdxInstanceFunction";
+
+  @Override
+  public String getId() {
+    return ID;
+  }
+
+  @Override
+  public boolean hasResult() {
+    return true;
+  }
+
+  public void init(Properties p) {
+  }
+
+  @Override
+  public boolean optimizeForWrite() {
+    return true;
+  }
+
+  @Override
+  public boolean isHA() {
+    return false;
+  }
+
+}
+
+
+