Merge pull request #10624 from jakesmith/hpcc-18645

HPCC-18645 Normalize logical filenames

Reviewed-By: Attila Vamos <attila.vamos@lexisnexis.com>
Reviewed-By: Richard Chapman <rchapman@hpccsystems.com>

dali/base/dautils.cpp

@@ -41,6 +41,7 @@
 
 #define EXTERNAL_SCOPE      "file"
 #define FOREIGN_SCOPE       "foreign"
+#define SELF_SCOPE          "."
 #define SDS_DFS_ROOT        "Files" // followed by scope/name
 #define SDS_RELATIONSHIPS_ROOT  "Files/Relationships"
 #define SDS_CONNECT_TIMEOUT  (1000*60*60*2)     // better than infinite
@@ -402,7 +403,6 @@ void normalizeNodeName(const char *node, unsigned len, SocketEndpoint &ep, bool
 void CDfsLogicalFileName::normalizeName(const char *name, StringAttr &res, bool strict)
 {
     // NB: If !strict(default) allows spaces to exist either side of scopes (no idea why would want to permit that, but preserving for bwrd compat.)
-    StringBuffer str;
     StringBuffer nametmp;
     const char *ct = nullptr;
     bool wilddetected = false;
@@ -457,55 +457,76 @@ void CDfsLogicalFileName::normalizeName(const char *name, StringAttr &res, bool
         }
     }
     if (!*name)
+    {
         name = ".::_blank_";
-    s=strstr(name,"::");
-    if (s)
+        tailpos = 3;
+        res.set(name);
+    }
+    else
     {
-        if (s==name) // JCS: meaning name leads with "::", would have thought should be treated as invalid
-            str.append('.');
-        else
+        StringBuffer str;
+        s=strstr(name,"::");
+        if (s)
         {
-            normalizeScope(name, name, s-name, str, strict);
-            bool isForeign = 0 == stricmp(str.str(),FOREIGN_SCOPE);
-            if (isForeign) // normalize node
+            bool skipScope = false;
+            unsigned scopeStart = 0;
+            if (s==name) // JCS: meaning name leads with "::", would have thought should be treated as invalid
+                str.append('.');
+            else
             {
-                const char *s1 = s+2;
-                const char *ns1 = strstr(s1,"::");
-                if (ns1)
+                normalizeScope(name, name, s-name, str, strict);
+                if (strieq(FOREIGN_SCOPE, str)) // normalize node
                 {
-                    normalizeNodeName(s1, ns1-s1, foreignep, strict);
-                    if (!foreignep.isNull())
+                    const char *s1 = s+2;
+                    const char *ns1 = strstr(s1,"::");
+                    if (ns1)
                     {
-                        foreignep.getUrlStr(str.append("::"));
-                        s = ns1;
-                        localpos = str.length()+2;
+                        normalizeNodeName(s1, ns1-s1, foreignep, strict);
+                        if (!foreignep.isNull())
+                        {
+                            foreignep.getUrlStr(str.append("::"));
+                            s = ns1;
+                            localpos = str.length()+2;
+                        }
                     }
                 }
+                else if (streq(SELF_SCOPE, str) && selfScopeTranslation)
+                    skipScope = true;
+            }
+            for (;;)
+            {
+                s+=2;
+                const char *ns = strstr(s,"::");
+                if ((ns || str.length()>1) && skipScope && selfScopeTranslation)
+                {
+                    str.setLength(scopeStart);
+                    skipScope = false;
+                }
+                else
+                    str.append("::");
+                if (!ns)
+                    break;
+                scopeStart = str.length();
+                normalizeScope(name, s, ns-s, str, strict);
+                unsigned scopeLen = str.length()-scopeStart;
+                if ((1 == scopeLen) && (*SELF_SCOPE == str.charAt(str.length()-1)) && selfScopeTranslation)
+                    skipScope = true;
+                s = ns;
             }
         }
-        for (;;)
+        else
         {
-            s+=2;
-            const char *ns = strstr(s,"::");
-            if (!ns)
-                break;
-            str.append("::");
-            normalizeScope(name, s, ns-s, str, strict);
-            s = ns;
+            s = name;
+            str.append(".::");
         }
+        tailpos = str.length();
+        normalizeScope(name, s, strlen(name)-(s-name), str, strict);
+        unsigned scopeLen = str.length()-tailpos;
+        if ((1 == scopeLen) && (*SELF_SCOPE == str.charAt(str.length()-1)))
+            throw MakeStringException(-1, "Logical filename cannot end with scope \".\"");
+        str.toLowerCase();
+        res.set(str);
     }
-    else
-    {
-        s = name;
-        str.append(".");
-    }
-    str.append("::");
-    tailpos = str.length();
-    if (strstr(s,"::")!=nullptr)
-        ERRLOG("Tail contains '::'!");
-    normalizeScope(name, s, strlen(name)-(s-name), str, strict);
-    str.toLowerCase();
-    res.set(str);
 }
 
 bool CDfsLogicalFileName::normalizeExternal(const char * name, StringAttr &res, bool strict)

dali/base/dautils.hpp

@@ -61,6 +61,7 @@ class da_decl CDfsLogicalFileName
     bool allowospath;
     bool allowWild;
     SocketEndpoint foreignep;
+    bool selfScopeTranslation = true; // default behaviour is to translate self scopes, e.g. .::.::scope::.::name -> scope::name
 
 public:
     CDfsLogicalFileName();
@@ -75,6 +76,7 @@ public:
     bool setFromXPath(const char *xpath);
     void clear();
     bool isSet() const;
+    void enableSelfScopeTranslation(bool onOff) { selfScopeTranslation = onOff; }
     /*
      * Foreign files are distributed files whose meta data is stored on a foreign
      * Dali Server, so their names are resolved externally.

dali/daliadmin/daliadmin.cpp

@@ -103,6 +103,7 @@ void usage(const char *exe)
   printf("  dfscompratio <logicalname>      -- returns compression ratio of file\n");
   printf("  dfsscopes <mask>                -- lists logical scopes (mask = * for all)\n");
   printf("  cleanscopes                     -- remove empty scopes\n");
+  printf("  normalizefilenames [<logicalnamemask>] -- normalize existing logical filenames that match, e.g. .::.::scope::.::name -> scope::name\n");
   printf("  dfsreplication <clustermask> <logicalnamemask> <redundancy-count> [dryrun] -- set redundancy for files matching mask, on specified clusters only\n");
   printf("  holdlock <logicalfile> <read|write> -- hold a lock to the logical-file until a key is pressed");
   printf("\n");
@@ -1800,6 +1801,56 @@ static void cleanscopes(IUserDescriptor *user)
     }
 }
 
+static void normalizeFileNames(IUserDescriptor *user, const char *name)
+{
+    if (!name)
+        name = "*";
+    Owned<IDFAttributesIterator> iter = queryDistributedFileDirectory().getDFAttributesIterator(name, user, true, true);
+    ForEach(*iter)
+    {
+        IPropertyTree &attr = iter->query();
+        const char *lfn = attr.queryProp("@name");
+        CDfsLogicalFileName dlfn;
+        dlfn.enableSelfScopeTranslation(false);
+        dlfn.set(lfn);
+
+        Owned<IDistributedFile> dFile;
+        try
+        {
+            dFile.setown(queryDistributedFileDirectory().lookup(dlfn, user, true, false, false, nullptr, 30000)); // 30 sec timeout
+            if (!dFile)
+                WARNLOG("Could not find file lfn = %s", dlfn.get());
+        }
+        catch (IException *e)
+        {
+            if (SDSExcpt_LockTimeout != e->errorCode())
+                throw;
+            VStringBuffer msg("Connecting to '%s'", lfn);
+            EXCLOG(e, msg.str());
+            e->Release();
+        }
+        if (dFile)
+        {
+            CDfsLogicalFileName newDlfn;
+            newDlfn.set(lfn);
+            if (!streq(newDlfn.get(), dlfn.get()))
+            {
+                PROGLOG("File: '%s', renaming to: '%s'", dlfn.get(), newDlfn.get());
+                try
+                {
+                    dFile->rename(newDlfn.get(), user);
+                }
+                catch (IException *e)
+                {
+                    VStringBuffer msg("Failure to rename file '%s'", lfn);
+                    EXCLOG(e, msg.str());
+                    e->Release();
+                }
+            }
+        }
+    }
+}
+
 //=============================================================================
 
 static void listworkunits(const char *test, const char *min, const char *max)
@@ -3441,6 +3492,10 @@ int main(int argc, char* argv[])
                         CHECKPARAMS(0,0);
                         cleanscopes(userDesc);
                     }
+                    else if (strieq(cmd,"normalizefilenames")) {
+                        CHECKPARAMS(0,1);
+                        normalizeFileNames(userDesc, np>0 ? params.item(1) : nullptr);
+                    }
                     else if (strieq(cmd,"listworkunits")) {
                         CHECKPARAMS(0,3);
                         listworkunits((np>0)?params.item(1):NULL,(np>1)?params.item(2):NULL,(np>2)?params.item(3):NULL);

testing/unittests/dalitests.cpp

@@ -2437,6 +2437,20 @@ public:
                                ".:: scope1::file*",
                                NULL                                             // terminator
                              };
+
+        const char *translatedLfns[][2] = {
+                               { ".::fname", ".::fname" },
+                               { "fname", ".::fname" },
+                               { "~.::fname", ".::fname" },
+                               { ".::.::.::fname", ".::fname" },
+                               { ".::.::.::sname::.::.::fname", "sname::fname" },
+                               { "~.::.::scope2::.::.::.::fname", "scope2::fname" },
+                               { "{.::.::multitest1f1, .::.::multitest1f2}", "{.::multitest1f1,.::multitest1f2}" },
+                               { ".::.::.::.sname.::.::.fname.", ".sname.::.fname." },
+                               { "~foreign::192.168.16.1::.::scope1::file1", "foreign::192.168.16.1::scope1::file1" },
+                               { "{~foreign::192.168.16.1::.::.::multi1, ~foreign::192.168.16.2::multi2::.::fname}", "{foreign::192.168.16.1::multi1,foreign::192.168.16.2::multi2::fname}" },
+                               { nullptr, nullptr }                             // terminator
+                             };
         PROGLOG("Checking valid logical filenames");
         unsigned nlfn=0;
         for (;;)
@@ -2454,31 +2468,40 @@ public:
             {
                 VStringBuffer err("Logical filename '%s' failed.", lfn);
                 EXCLOG(e, err.str());
-                CPPUNIT_FAIL(err.str());
                 e->Release();
+                CPPUNIT_FAIL(err.str());
             }
         }
-        PROGLOG("Checking invalid logical filenames");
+        PROGLOG("Checking translations");
         nlfn = 0;
         for (;;)
         {
-            const char *lfn = invalidLfns[nlfn++];
-            if (NULL == lfn)
+            const char **entry = translatedLfns[nlfn++];
+            if (nullptr == entry[0])
                 break;
-            PROGLOG("lfn = %s", lfn);
+            const char *lfn = entry[0];
+            const char *expected = entry[1];
+            PROGLOG("lfn = %s, expect = %s", lfn, expected);
             CDfsLogicalFileName dlfn;
+            StringBuffer err;
             try
             {
                 dlfn.set(lfn);
-                VStringBuffer err("Logical filename '%s' passed and should have failed.", lfn);
-                ERRLOG("%s", err.str());
-                CPPUNIT_FAIL(err.str());
+                const char *result = dlfn.get();
+                if (!streq(result, expected))
+                    err.appendf("Logical filename '%s' should have translated to '%s', but result was '%s'.", lfn, expected, result);
             }
             catch (IException *e)
             {
-                EXCLOG(e, "Expected error:");
+                err.appendf("Logical filename '%s' failed: ", lfn);
+                e->errorMessage(err);
                 e->Release();
             }
+            if (err.length())
+            {
+                ERRLOG("%s", err.str());
+                CPPUNIT_FAIL(err.str());
+            }
         }
     }
 };
@@ -2514,15 +2537,15 @@ public:
          const char *validInternalLfns[][2] = {
                  //     input file name                    expected normalized file name
                  {"~foreign::192.168.16.1::scope1::file1", "foreign::192.168.16.1::scope1::file1"},
-                 {".::scope1::file",                       ".::scope1::file"},
+                 {".::scope1::file",                       "scope1::file"},
                  {"~ scope1 :: scope2 :: file  ",          "scope1::scope2::file"},
-                 {". :: scope1 :: file nine",              ".::scope1::file nine"},
-                 {". :: scope1 :: file ten  ",             ".::scope1::file ten"},
-                 {". :: scope1 :: file",                   ".::scope1::file"},
+                 {". :: scope1 :: file nine",              "scope1::file nine"},
+                 {". :: scope1 :: file ten  ",             "scope1::file ten"},
+                 {". :: scope1 :: file",                   "scope1::file"},
                  {"~scope1::file@cluster1",                "scope1::file"},
                  {"~scope::^C^a^S^e^d",                    "scope::^c^a^s^e^d"},
                  {"~scope::CaSed",                         "scope::cased"},
-                 {"~scope::^CaSed",                         "scope::^cased"},
+                 {"~scope::^CaSed",                        "scope::^cased"},
                  {nullptr,                                 nullptr}   // terminator
                  };