HPCC-29917 Refactor compressToBuffer to support different compression…

… methods Changes following review, including more tests Signed-off-by: Richard Chapman <rchapman@hpccsystems.com>
hpcc-systems · Nov 22, 2023 · 35bab01 · 35bab01
1 parent c25ff6e
commit 35bab01
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 24 deletions.
diff --git a/system/jlib/jlzw.cpp b/system/jlib/jlzw.cpp
@@ -303,7 +303,7 @@ void CLZWCompressor::open(void *buf,size32_t max)
         outbuf = malloc(bufalloc);
     }
     outBufMb = NULL;
-    ASSERT(max>SAFETY_MARGIN+sizeof(size32_t)); // minimum required
+    assertex(max>SAFETY_MARGIN+sizeof(size32_t)); // minimum required
     maxlen=max-SAFETY_MARGIN;
     initCommon();
 }
@@ -1372,7 +1372,7 @@ class jlib_decl CRDiffCompressor : public ICompressor, public CInterface
             outbuf = malloc(bufalloc);
         }
         outBufMb = NULL;
-        ASSERT(max>2+sizeof(size32_t)*2); // minimum required (actually will need enough for recsize so only a guess)
+        assertex(max>2+sizeof(size32_t)*2); // minimum required (actually will need enough for recsize so only a guess)
         initCommon();
         remaining = max-outlen;
     }
@@ -1655,7 +1655,7 @@ class jlib_decl CRandRDiffCompressor : public ICompressor, public CInterface
             outbuf = malloc(bufalloc);
         }
         outBufMb = NULL;
-        ASSERT(max>MIN_RRDHEADER_SIZE+sizeof(unsigned short)+3); // hopefully a lot bigger!
+        assertex(max>MIN_RRDHEADER_SIZE+sizeof(unsigned short)+3); // hopefully a lot bigger!
         initCommon();
     }
 
@@ -2914,9 +2914,9 @@ class CCompressHandlerArray
             if ((method & ~COMPRESS_METHOD_AES) < COMPRESS_METHOD_LAST)
             {
                 if (method & COMPRESS_METHOD_AES)
-                    AESbyMethod[method & ~COMPRESS_METHOD_AES] = handler;
+                    AESbyMethod[method & ~COMPRESS_METHOD_AES] = nullptr;
                 else
-                    byMethod[method] = handler;
+                    byMethod[method] = nullptr;
             }
             return true;
         }

diff --git a/testing/unittests/unittests.cpp b/testing/unittests/unittests.cpp
@@ -1056,10 +1056,10 @@ CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( RelaxedAtomicTimingTest, "RelaxedAtomicTi
 class compressToBufferTest : public CppUnit::TestFixture
 {
     CPPUNIT_TEST_SUITE( compressToBufferTest  );
-        CPPUNIT_TEST(testRun);
+        CPPUNIT_TEST(testCompressors);
     CPPUNIT_TEST_SUITE_END();
 
-    void testOne(unsigned len, CompressionMethod method, const char *options=nullptr)
+    bool testOne(unsigned len, CompressionMethod method, bool prevResult, const char *options=nullptr)
     {
         constexpr const char *in = 
           "HelloHelloHelloHelloHelloHelloHelloHelloHelloHello"
@@ -1092,34 +1092,41 @@ class compressToBufferTest : public CppUnit::TestFixture
         assertex(len <= strlen(in));
         MemoryBuffer compressed;
         compressToBuffer(compressed, len, in, method, options);
-
-        if (method != COMPRESS_METHOD_NONE && len >= 32 && compressed.length() == len+5)
-            DBGLOG("compressToBuffer %x size %u did not compress", (byte) method, len);
+        bool ret;
+        if (compressed.length() == len+5)
+        {
+            if (prevResult)
+               DBGLOG("compressToBuffer %x size %u did not compress", (byte) method, len);
+            ret = false;
+        }
         else 
-            DBGLOG("compressToBuffer %x size %u compressed to %u", (byte) method, len, compressed.length());
+        {
+            if (!prevResult)
+                DBGLOG("compressToBuffer %x size %u compressed to %u", (byte) method, len, compressed.length());
+            ret = true;
+        }
         CPPUNIT_ASSERT(compressed.length() <= len+5);
         MemoryBuffer out;
         decompressToBuffer(out, compressed, options);
         CPPUNIT_ASSERT(out.length() == len);
         CPPUNIT_ASSERT(memcmp(out.bytes(), in, len) == 0);
+        return ret;
     }
 
-    void testSome(unsigned len)
+    void testCompressor(CompressionMethod method, const char *options=nullptr)
     {
-        testOne(len, COMPRESS_METHOD_NONE);
-        testOne(len, COMPRESS_METHOD_LZW);
-        testOne(len, COMPRESS_METHOD_LZ4);
-        testOne(len, (CompressionMethod) (COMPRESS_METHOD_LZW|COMPRESS_METHOD_AES), "0123456789abcdef");
-    }
+        bool result = true;
+        for (unsigned i = 0; i < 256; i++)
+            result = testOne(i, method, result,  options);
+        testOne(1000, method, false, options);
 
-    void testRun()
+    }
+    void testCompressors()
     {
-        testSome(0);
-        testSome(1);
-        testSome(16);
-        testSome(32);
-        testSome(200);
-        testSome(1000);
+        testCompressor(COMPRESS_METHOD_NONE);
+        testCompressor(COMPRESS_METHOD_LZW);
+        testCompressor(COMPRESS_METHOD_LZ4);
+        testCompressor((CompressionMethod) (COMPRESS_METHOD_LZW|COMPRESS_METHOD_AES), "0123456789abcdef");
     }
 };