HPCC-12303 Fix incorrect return check for BoolHash getValue

Two groups of code changes are made in the ECLWatch ESP services:
1. Fix incorrect return check for BoolHash getValue().
2. In ws_machine service, replace the CIArrayOf<CStateHash> with
MapStringToMyClass<IStateHash>.

Signed-off-by: wangkx <kevin.wang@lexisnexis.com>

esp/services/ws_dfu/ws_dfuXRefService.cpp

@@ -496,7 +496,10 @@ bool CWsDfuXRefEx::onDFUXRefList(IEspContext &context, IEspDFUXRefListRequest &r
             ForEachItemIn(i,primaryThorProcesses)
             {
                 const char *thorProcess = primaryThorProcesses.item(i);
-                if (uniqueProcesses.getValue(thorProcess))
+                if (!thorProcess || !*thorProcess)
+                    continue;
+                bool* found = uniqueProcesses.getValue(thorProcess);
+                if (found && *found)
                     continue;
                 uniqueProcesses.setValue(thorProcess, true);
                 addXRefNode(thorProcess, pXRefNodeTree);

esp/services/ws_fs/ws_fsBinding.cpp

@@ -309,7 +309,8 @@ IPropertyTree* CFileSpraySoapBindingEx::createPTreeForXslt(const char* method, c
             getClusterGroupName(cluster, thorClusterGroupName);
             if (!thorClusterGroupName.length())
                 continue;
-            if (uniqueThorClusterGroupNames.getValue(thorClusterGroupName.str()))
+            bool* found = uniqueThorClusterGroupNames.getValue(thorClusterGroupName.str());
+            if (found && *found)
                 continue;
 
             uniqueThorClusterGroupNames.setValue(thorClusterGroupName.str(), true);

esp/services/ws_machine/ws_machineService.cpp

@@ -457,7 +457,8 @@ void Cws_machineEx::setProcessRequest(CGetMachineInfoData& machineInfoData, Bool
         valuesToBeChecked.append(numAddr);
         if (machineInfoData.getOptions().getGetSoftwareInfo())
             valuesToBeChecked.appendf(":%s:%s:%d", processTypeStr.str(), compName, processNumber);
-        if (uniqueProcesses.getValue(valuesToBeChecked.str()))
+        bool* found = uniqueProcesses.getValue(valuesToBeChecked.str());
+        if (found && *found)
             continue;
         uniqueProcesses.setValue(valuesToBeChecked.str(), true);
 
@@ -819,7 +820,8 @@ void Cws_machineEx::getProcesses(IConstEnvironment* constEnv, IPropertyTree* env
             const char* directory0 = directories.item(i);
             if (uniqueRoxieProcesses)//to avoid duplicate entries for roxie (one machine has only one roxie process).
             {
-                if (uniqueRoxieProcesses->getValue(name0))
+                bool* found = uniqueRoxieProcesses->getValue(name0);
+                if (found && *found)
                     continue;
                 uniqueRoxieProcesses->setValue(name0, true);
             }
@@ -877,7 +879,8 @@ bool Cws_machineEx::isLegacyFilter(const char* processType, const char* dependen
 
     StringBuffer filterString;
     filterString.appendf("%s:%s", processType, dependency);
-    if (m_legacyFilters.getValue(filterString.str()))
+    bool* found = m_legacyFilters.getValue(filterString.str());
+    if (found && *found)
         return true;
 
     return false;
@@ -885,50 +888,60 @@ bool Cws_machineEx::isLegacyFilter(const char* processType, const char* dependen
 
 //The stateHashes stores different state hashes in one roxie cluster.
 //It also stores how many roxie nodes have the same state hashes.
-int Cws_machineEx::addRoxieStateHash(const char* hash, CIArrayOf<CStateHash>& stateHashes)
+unsigned Cws_machineEx::addRoxieStateHash(const char* hash, StateHashes& stateHashes, unsigned& totalUniqueHashes)
 {
     if (!hash || !*hash)
         return -1;
-    ForEachItemIn(i, stateHashes)
+
+    unsigned hashID = 0;
+    IStateHash* stateHash = stateHashes.getValue(hash);
+    if (stateHash)
+    {
+        //if the stateHashes already has the same 'hash', increases the count for the 'stateHash'.
+        //The 'StateHash' with the highest count will be the 'Major StateHash'.
+        //If a roxie node does not contain the 'Major StateHash', it has a 'mismatch' state hash.
+        hashID = stateHash->queryID();
+        stateHash->incrementCount();
+    }
+    else
     {
-        CStateHash& stateHash = stateHashes.item(i);
-        //if the 'hash' matches this 'stateHash', the matchHash() increases the count for this 'stateHash' by 1.
-        if (stateHash.matchHash(hash))
-            return i;
+        //Add a new 'StateHash'. Set its hashID to totalUniqueHashes and set its count to 1.
+        hashID = totalUniqueHashes;
+        stateHashes.setValue(hash, new CStateHash(hashID, 1));
+        totalUniqueHashes++;
     }
-    stateHashes.append(*new CStateHash(hash));
-    return stateHashes.length() - 1;
+    return hashID;
 }
 
-void Cws_machineEx::updateMajorRoxieStateHash(CIArrayOf<CStateHash>& stateHashes, CIArrayOf<CRoxieStateData>& roxieStates)
+void Cws_machineEx::updateMajorRoxieStateHash(StateHashes& stateHashes, CIArrayOf<CRoxieStateData>& roxieStates)
 {
-    if (stateHashes.length() < 2)
-        return;
-
     //Find out which state hash is for the most of the roxie nodes inside this roxie cluster.
-    unsigned majorHash = 0;
+    unsigned majorHashID = 0;
     unsigned majorHashCount = 0;
-    ForEachItemIn(i, stateHashes)
+    HashIterator hashes(stateHashes);
+    ForEach(hashes)
     {
-        unsigned hashCount = stateHashes.item(i).getCount();
+        IStateHash *hash = stateHashes.mapToValue(&hashes.query());
+        unsigned hashCount = hash->queryCount();
         if (majorHashCount >= hashCount)
             continue;
         majorHashCount = hashCount;
-        majorHash = i;
+        majorHashID = hash->queryID();
     }
 
-    //Set the MajorHash to false if the roxie node has a different hash.
+    //Set the MajorHash to false if the roxie node's HashID() != majorHashID.
     ForEachItemIn(ii, roxieStates)
     {
         CRoxieStateData& roxieState = roxieStates.item(ii);
-        if (roxieState.getHashID() != majorHash)
+        if (roxieState.getHashID() != majorHashID)
             roxieState.setMajorHash(false);
     }
 }
 
 void Cws_machineEx::readRoxieStatus(const Owned<IPropertyTree> controlResp, CIArrayOf<CRoxieStateData>& roxieStates)
 {
-    CIArrayOf<CStateHash> stateHashes;
+    StateHashes stateHashes;
+    unsigned totalUniqueHashes = 0;
     Owned<IPropertyTreeIterator> roxieEndpoints = controlResp->getElements("Endpoint");
     ForEach(*roxieEndpoints)
     {
@@ -952,11 +965,12 @@ void Cws_machineEx::readRoxieStatus(const Owned<IPropertyTree> controlResp, CIAr
 
         StringArray locations;
         locations.appendListUniq(ep, ":");
-        Owned<CRoxieStateData> roxieState = new CRoxieStateData(locations.item(0), addRoxieStateHash(stateHash, stateHashes));
+        Owned<CRoxieStateData> roxieState = new CRoxieStateData(locations.item(0), addRoxieStateHash(stateHash, stateHashes, totalUniqueHashes));
         roxieState->setState(ok, attached, detached, stateHash);
         roxieStates.append(*roxieState.getClear());
     }
-    updateMajorRoxieStateHash(stateHashes, roxieStates);
+    if (totalUniqueHashes > 1)
+        updateMajorRoxieStateHash(stateHashes, roxieStates);
 }
 
 void Cws_machineEx::getRoxieStateInfo(CRoxieStateInfoThreadParam* param)

esp/services/ws_machine/ws_machineService.hpp

@@ -365,34 +365,34 @@ public:
     }
 };
 
-class CStateHash : public CInterface
+interface IStateHash : extends IInterface
 {
-    BoolHash   hash;
+    virtual unsigned queryID() = 0;
+    virtual unsigned queryCount() = 0;
+    virtual void incrementCount() = 0;
+};
+
+class CStateHash : public CInterface, implements IStateHash
+{
+    unsigned   id;
     unsigned   count;
 public:
     IMPLEMENT_IINTERFACE;
 
-    CStateHash(const char* _hash) : hash(_hash), count(1)
-    {
-        hash.setValue(_hash, true);
-    };
-
-    bool matchHash(const char* _hash)
-    {
-        bool match = hash.getValue(_hash);
-        if (match)
-            count++;
-        return match;
-    }
+    CStateHash(unsigned _id, unsigned _count) : id(_id), count(_count) { };
 
-    unsigned getCount() { return count; }
+    virtual unsigned queryID() { return id; }
+    virtual unsigned queryCount() { return count; }
+    virtual void incrementCount() { count++; };
 };
 
+typedef MapStringToMyClass<IStateHash> StateHashes;
+
 class CRoxieStateData : public CInterface
 {
     BoolHash   ipAddress;
     StringAttr hash;
-    int        hashID; //the position inside cluster's state hash list - used to set the majorHash flag in updateMajorRoxieStateHash().
+    unsigned   hashID; //the position inside cluster's state hash list - used to set the majorHash flag in updateMajorRoxieStateHash().
     bool       majorHash; //whether its state hash is the same as the most of other roxie cluster nodes or not.
     bool       ok;
     bool       attached;
@@ -400,13 +400,18 @@ class CRoxieStateData : public CInterface
 public:
     IMPLEMENT_IINTERFACE;
 
-    CRoxieStateData(const char* _ipAddress, int _hashID) : hashID(_hashID), majorHash(true), ok(false), attached(false), detached(false)
+    CRoxieStateData(const char* _ipAddress, unsigned _hashID) : hashID(_hashID), majorHash(true), ok(false), attached(false), detached(false)
     {
         ipAddress.setValue(_ipAddress, true);
     };
 
-    bool matchIPAddress(const char* _ipAddress) { return ipAddress.getValue(_ipAddress); }
-    int getHashID() { return hashID; }
+    bool matchIPAddress(const char* _ipAddress)
+    {
+        bool* match = ipAddress.getValue(_ipAddress);
+        return (match && *match);
+    }
+    unsigned getHashID() { return hashID; }
+    const char* getHash() { return hash.get(); }
     void setMajorHash(bool _majorHash) { majorHash = _majorHash; }
 
     void setState(bool _ok, bool _attached, bool _detached, const char* _hash)
@@ -712,7 +717,8 @@ public:
 
     void appendRoxieClusters(const char* clusterName)
     {
-        if (uniqueRoxieClusters.getValue(clusterName))
+        bool* found = uniqueRoxieClusters.getValue(clusterName);
+        if (found && *found)
             return;
 
         roxieClusters.append(clusterName);
@@ -798,9 +804,10 @@ private:
     void doPostProcessing(CFieldInfoMap& myfieldInfoMap, CFieldMap&  myfieldMap);
     void processValue(const char *oid, const char *value, const bool bShow, CFieldInfoMap& myfieldInfoMap, CFieldMap&  myfieldMap);
     void addIpAddressesToBuffer( void** buffer, unsigned& count, const char* address);
+
     void readRoxieStatus(const Owned<IPropertyTree> controlResp, CIArrayOf<CRoxieStateData>& roxieStates);
-    int addRoxieStateHash(const char* hash, CIArrayOf<CStateHash>& stateHashes);
-    void updateMajorRoxieStateHash(CIArrayOf<CStateHash>& stateHashes, CIArrayOf<CRoxieStateData>& roxieStates);
+    unsigned addRoxieStateHash(const char* hash, StateHashes& stateHashes, unsigned& totalUniqueHashes);
+    void updateMajorRoxieStateHash(StateHashes& stateHashes, CIArrayOf<CRoxieStateData>& roxieStates);
     StringBuffer& getAcceptLanguage(IEspContext& context, StringBuffer& acceptLanguage);
 
     //Still used in StartStop/Rexec, so keep them for now.

esp/services/ws_workunits/ws_workunitsHelpers.cpp

@@ -1068,7 +1068,10 @@ unsigned WsWuInfo::getWorkunitThorLogInfo(IArrayOf<IEspECLHelpFile>& helpers, IE
         {
             SCMStringBuffer processName;
             thorInstances->str(processName);
-            if ((processName.length() < 1) || uniqueProcesses.getValue(processName.str()))
+            if (processName.length() < 1)
+                continue;
+            bool* found = uniqueProcesses.getValue(processName.str());
+            if (found && *found)
                 continue;
 
             uniqueProcesses.setValue(processName.str(), true);

esp/services/ws_workunits/ws_workunitsService.cpp

@@ -477,7 +477,8 @@ void CWsWorkunitsEx::refreshValidClusters()
     {
         SCMStringBuffer s;
         IStringVal &val = it->str(s);
-        if (!validClusters.getValue(val.str()))
+        bool* found = validClusters.getValue(val.str());
+        if (!found || !*found)
             validClusters.setValue(val.str(), true);
     }
 }
@@ -487,7 +488,8 @@ bool CWsWorkunitsEx::isValidCluster(const char *cluster)
     if (!cluster || !*cluster)
         return false;
     CriticalBlock block(crit);
-    if (validClusters.getValue(cluster))
+    bool* found = validClusters.getValue(cluster);
+    if (found && *found)
         return true;
     if (validateTargetClusterName(cluster))
     {
@@ -4127,13 +4129,16 @@ bool CWsWorkunitsEx::onWUGetZAPInfo(IEspContext &context, IEspWUGetZAPInfoReques
         }
 
         //Get Thor IP
-        BoolHash uniqueProcesses, uniqueThorIPs;
+        BoolHash uniqueProcesses;
         Owned<IStringIterator> thorInstances = cw->getProcesses("Thor");
         ForEach (*thorInstances)
         {
             SCMStringBuffer processName;
             thorInstances->str(processName);
-            if ((processName.length() < 1) || uniqueProcesses.getValue(processName.str()))
+            if (processName.length() < 1)
+                continue;
+            bool* found = uniqueProcesses.getValue(processName.str());
+            if (found && *found)
                 continue;
 
             uniqueProcesses.setValue(processName.str(), true);