Post review changes

Signed-off-by: James Noss <james.noss@lexisnexis.com>

cmake_modules/FindMEMCACHED.cmake

@@ -41,10 +41,23 @@ IF (NOT LIBMEMCACHED_FOUND)
 
   SET (LIBMEMCACHED_LIBRARIES ${LIBMEMCACHEDCORE_LIBRARY} ${LIBMEMCACHEDUTIL_LIBRARY})
 
+  FILE (STRINGS "${LIBMEMCACHED_INCLUDE_DIR}/libmemcached-1.0/configure.h" version REGEX "#define LIBMEMCACHED_VERSION_STRING")
+  STRING(REGEX REPLACE "#define LIBMEMCACHED_VERSION_STRING " "" version "${version}")
+  STRING(REGEX REPLACE "\"" "" version "${version}")
+  SET (LIBMEMCACHED_VERSION_STRING ${version})
+  IF ("${LIBMEMCACHED_VERSION_STRING}" VERSION_EQUAL "${LIBMEMCACHED_FIND_VERSION}" OR "${LIBMEMCACHED_VERSION_STRING}" VERSION_GREATER "${LIBMEMCACHED_FIND_VERSION}")  
+    SET (LIBMEMCACHED_VERSION_OK 1)
+    SET (MSG "${DEFAULT_MSG}")
+  ELSE()
+    SET (LIBMEMCACHED_VERSION_OK 0)
+    SET(MSG "libmemcached version '${LIBMEMCACHED_VERSION_STRING}' incompatible with min version>=${LIBMEMCACHED_FIND_VERSION}")
+  ENDIF()
+      
   include(FindPackageHandleStandardArgs)
-  find_package_handle_standard_args(memcached DEFAULT_MSG
+  find_package_handle_standard_args(libmemcached ${MSG}
     LIBMEMCACHED_LIBRARIES
     LIBMEMCACHED_INCLUDE_DIR
+    LIBMEMCACHED_VERSION_OK
   )
 
   MARK_AS_ADVANCED(LIBMEMCACHED_INCLUDE_DIRS LIBMEMCACHED_LIBRARIES)

plugins/memcached/CMakeLists.txt

@@ -25,7 +25,7 @@
 project( memcached )
 
 if (MEMCACHED)
-    ADD_PLUGIN(memcached PACKAGES MEMCACHED OPTION MAKE_MEMCACHED)
+    ADD_PLUGIN(memcached PACKAGES LIBMEMCACHED OPTION MAKE_MEMCACHED MINVERSION 1.0.10)
     if(MAKE_MEMCACHED)
         set(
             SRCS

plugins/memcached/memcachedplugin.cpp

@@ -296,62 +296,14 @@ void MemCachedPlugin::MCached::getVoidPtrLenPair(ICodeContext * ctx, const char
 
 MemCachedPlugin::MCached::MCached(ICodeContext * ctx, const char * _options) : connection(NULL), pool(NULL), typeMismatchCount(0)
 {
-    options.set(_options);
+#if (LIBMEMCACHED_VERSION_HEX < 0x01000010)
+    StringBuffer msg("Memcached Plugin: libmemcached version '");
+    msg.append(LIBMEMCACHED_VERSION_STRING).append("' incompatible with min version>=1.0.10");
+    rtlFail(0, msg.str());
+#endif
 
-#if (LIBMEMCACHED_VERSION_HEX<0x53000)
-    memcached_st *memc = memcached_create(NULL);
-    memcached_return_t rc;
-    memcached_server_st *servers = NULL;
-    try
-    {
-        unsigned pool_min = 1;
-        unsigned pool_max = 1;
-        StringArray optionStrings;
-        optionStrings.appendList(_options, " ");
-        ForEachItemIn(idx, optionStrings)
-        {
-            const char *opt = optionStrings.item(idx);
-            if (strncmp(opt, "--SERVER=", 9) ==0)
-            {
-                opt += 9;
-                StringArray splitPort;
-                splitPort.appendList(opt, ":");
-                unsigned port;
-                if (splitPort.ordinality()==2)
-                    port = atoi(splitPort.item(1));
-                else
-                    port = 11211;
-                servers = memcached_server_list_append(NULL, splitPort.item(0), port, &rc);
-                assertOnError(rc, "memcached_server_list_append failed - ");
-            }
-            else if (strncmp(opt, "--POOL-MIN=", 11) ==0)
-                pool_min = atoi(opt+11);
-            else if (strncmp(opt, "--POOL-MAX=", 11) ==0)
-                pool_max = atoi(opt+11);
-            else
-            {
-                VStringBuffer err("MemCachedPlugin: unsupported option string %s", opt);
-                rtlFail(0, err.str());
-            }
-        }
-        if (!servers)
-            rtlFail(0, "No servers specified");
-        rc = memcached_server_push(memc, servers);
-        memcached_server_list_free(servers);
-        assertOnError(rc, "memcached_server_push failed - ");
-        pool = memcached_pool_create(memc, pool_min, pool_max);  // takes ownership of memc
-    }
-    catch (...)
-    {
-        if (servers)
-            memcached_server_list_free(servers);
-        if (memc)
-            memcached_free(memc);
-        throw;
-    }
-#else
+    options.set(_options);
     pool = memcached_pool(_options, strlen(_options));
-#endif
     assertPool();
 
     setPoolSettings();
@@ -363,11 +315,8 @@ MemCachedPlugin::MCached::~MCached()
 {
     if (pool)
     {
-#if (LIBMEMCACHED_VERSION_HEX<0x53000)
-        memcached_pool_push(pool, connection);
-#else
+
         memcached_pool_release(pool, connection);
-#endif
         connection = NULL;//For safety (from changing this destructor) as not implicit in either the above or below.
         memcached_st *memc = memcached_pool_destroy(pool);
         if (memc)

testing/regress/ecl/key/memcachederrortests.xml

@@ -0,0 +1,3 @@
+<Dataset name='Result 1'>
+ <Row><value>Memcached Plugin: Failed connecting to ALL servers. Check memcached on all servers and &quot;memcached -B ascii&quot; not used.</value></Row>
+</Dataset>

testing/regress/ecl/key/memcachedtest.xml

@@ -58,6 +58,3 @@
 <Dataset name='Result 20'>
  <Row><Result_20>false</Result_20></Row>
 </Dataset>
-<Dataset name='Result 21'>
- <Row><value>Memcached Plugin: Failed connecting to ALL servers. Check memcached on all servers and &quot;memcached -B ascii&quot; not used.</value></Row>
-</Dataset>

testing/regress/ecl/memcachederrortests.ecl

@@ -0,0 +1,29 @@
+/*##############################################################################
+
+    HPCC SYSTEMS software Copyright (C) 2016 HPCC Systems®.
+
+    Licensed 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.
+############################################################################## */
+
+//class=embedded
+//class=3rdparty
+//nothor
+
+IMPORT memcached FROM lib_memcached;
+
+//Test failure to connect
+STRING downServer := '--SERVER=127.0.0.1:6379';
+    ds := DATASET(NOFOLD(1), TRANSFORM({STRING value}, SELF.value := memcached.GetString('connectionTest', downServer)));
+SEQUENTIAL(
+    OUTPUT(CATCH(ds, ONFAIL(TRANSFORM({ STRING value }, SELF.value := FAILMESSAGE))));
+    );

testing/regress/ecl/memcachedtest.ecl

@@ -17,7 +17,6 @@
 
 //class=embedded
 //class=3rdparty
-//nothor
 
 IMPORT memcached FROM lib_memcached;
 IMPORT Std;
@@ -106,10 +105,3 @@ SEQUENTIAL(
     memcached.Delete('testDelete', servers, 'hashWithThis');
     memcached.Exists('testDelete', servers, 'hashWithThis');
     );
-
-//Test failure to connect
-STRING downServer := '--SERVER=127.0.0.1:6379';
-    ds := DATASET(NOFOLD(1), TRANSFORM({STRING value}, SELF.value := memcached.GetString('connectionTest', downServer)));
-SEQUENTIAL(
-    OUTPUT(CATCH(ds, ONFAIL(TRANSFORM({ STRING value }, SELF.value := FAILMESSAGE))));
-    );