FWD » Core Development » Runtime Infrastructure

There is an impedance mismatch when a converted application accesses file system resources, if the original system ran on a platform with a different file system.

The main issues between the source and target systems are:

• mismatch in the file separators used (slash and backslash);
• mismatch in the case-sensitivity of file system names;
• absolute paths are prefixed differently and thus paths which were valid for the old system can't work in the converted environment.

The classes responsible for direct file system access by the application on the client are:

• FileSystemDaemon
• StreamDaemon

File Separators¶

Some rationalization of file separators already takes place. But I'm not sure it's fully handled across both of these classes. The level of support needs to be investigated and completed, if it is not fully implemented currently.

We currently have a directory setting for the file system under the file-system node which specifies the case-sensitivity for the original 4GL system: file-separator (\ or /).

Case-Sensitivity¶

We currently have a directory setting for the file system under the file-system node which specifies the case-sensitivity for the original 4GL system: case-sensitive (true or false).

This seems to be honored by the search/propath APIs, but not for direct file and stream access. The value in the directory needs to be checked when a new client is launched and sent down to the client, so that the client can behave in the same manner as the original platform w.r.t. filename case-sensitivity.

Absolute Path Prefixes¶

Many applications hard-code absolute paths into the source code or into the database. Those hard-coded into the source code can be found via analytics, but it is a time-consuming process to identify all of them and to replace them with code to conditionally use one path for one OS and another for a different OS. When absolute paths are pulled from the database (or from some other runtime source), a mismatch will cause that file system operation to fail at runtime.

The idea is to map well-known path prefixes (e.g., c:\tmp) to the appropriate replacement for the target platform (e.g., /tmp). These mappings will be maintained in the directory, in a new container node absolute-path-prefix-map, which will be a child of the existing file-system node. The @absolute-path-prefix-map would contain zero or more child nodes like this (for example, to map from a Windows source system to a Linux target system):

<node class="string" name="c:">
  <node-attribute name="value" value="/opt/my_app"/>
</node>
<node class="string" name="c:\tmp">
  <node-attribute name="value" value="/tmp"/>
</node>

The most specific match on the original path should be used to find a mapping. So, given the examples above, the original path c:\tmp\some_file.txt should be translated to /tmp/some_file.txt and NOT to /opt/my_app/tmp/some_file.txt.

Implementation¶

The separator, case, and absolute path prefix characteristics specific to a client should be looked up in the directory and pushed down to the client when the client is launched. The client will be responsible for applying those settings.

To keep the initial implementation simple, absolute path prefix mappings, case-sensitivity, and file separator settings will only be supported at the server level. If we find in the future we need to implement these at the account level for better granularity, we can do this as a separate task.

For directory lookup helpers, see the findDirectoryNodePath and getDirectoryNode* APIs in com.goldencode.p2j.util.Utils.

For case-insensitive file system helpers, see matchPathCaseInsensitively in the same class. You might need to do some refactoring of this to make it useful for your purposes here.

Don't know it is reasonable to use 3556a. Although 3556a contains timer changes, r-file search improvements and selection list layout validation changes, planning to work on this task within 3556a. Are there any objections?

Sergey Ivanovskiy wrote:

Don't know it is reasonable to use 3556a. Although 3556a contains timer changes, r-file search improvements and selection list layout validation changes, planning to work on this task within 3556a. Are there any objections?

Unless there is something in 3556a on top of which you need to build this feature set (possibly the r-file search changes?), I would prefer you get 3556a into testing and start a new branch 3565a for this work.

OK. The task branch 3565a rev 11253 - created.

Sergey Ivanovskiy wrote:

Constantin, in this revision 11269 (branch 3556a) you added similar mapping as described in this task Absolute Task Prefixes. Please could you share your visions here since this change 11269 is not in the trunc now.

This is not related to OS-level file lookups - this is related to resolving the target of RUN statement, if this is specified via i.e RUN prog.r and the program exists as prog.p or if specified as RUN obj/prog.p and the program exists as src/prog.p; the mappings are like obj=src, where individual folder names in the path are aliased.

You will need something similar for OS lookup; first, you need to specify in directory.xml some OS-level prefix matches, as Eric mentioned - to map i.e. c:\tmp to /tmp. If you need help to define these in the directory, you can inspire from SourceNameMapper.loadPathAliases and the dir_schema.xml change.

OK. It seems I considered another tasks. Thanks for explanations. Thus in this task we should add a windows path support to these FileSystemDaemon and StreamDaemon via "legacy" file directory mapping from original OS to host OS, shouldn't?

Sergey Ivanovskiy wrote:

OK. It seems I considered another tasks. Thanks for explanations. Thus in this task we should add a windows path support to these FileSystemDaemon and StreamDaemon via "legacy" file directory mapping from original OS to host OS, shouldn't?

Yes, this is not about SEARCH, it is about the client opening files and streams for reading and writing. However, if there is something in Constantin's implementation to solve a similar problem for SEARCH that you can use, refactor, or just learn from, it might be helpful to review his solution. Also, it is not just about a "windows path" -- the solution should be generic; the legacy path could be a Windows-style or a Unix-style file system. It is for this reason that the directory nodes defining the configuration are generically named, not named according to platform: file-separator, case-sensitive, absolute-path-prefix-map.

Let us consider these hypothetical settings

<node class="string" name="c:">
  <node-attribute name="value" value="/opt/my_app"/>
</node>
<node class="string" name="c:\app\environment">
  <node-attribute name="value" value="/opt/my_app/env"/>
</node>
<node class="string" name="c:\tmp">
  <node-attribute name="value" value="/tmp"/>
</node>

Yes.

The idea is to accept the longest exact match (case-insensitive) to a prefix of a path. That match must only be accepted for "whole segments" of the path:

c:\app\environment will match C:\aPp\enVIRONment\something\file.txt but it WILL NOT match c:\app\environmentS\something\file.txt.

It seems that with file-separator and case-sensitive under file-system node this task needs native-file-separator and native-case-sensitive. My simple server directory has these settings

 
        <node class="container" name="runtime">
          <node class="container" name="default">
            <node class="container" name="file-system">
              <node class="string" name="pkgroot">
                <node-attribute name="value" value="com.goldencode.testcases"/>
              </node>
              <node class="string" name="propath">
                <node-attribute name="value" value=".:./icons/"/>
              </node>
              <node class="string" name="path-separator">
                <node-attribute name="value" value=":"/>
              </node>
              <node class="string" name="file-separator">
                <node-attribute name="value" value="/"/>
              </node>
              <node class="boolean" name="case-sensitive">
                <node-attribute name="value" value="TRUE"/>
              </node>
            </node>
          </node>
        </node>

Please don't mind. These values define the native file separator and the native case sensitive mode.

Committed revision 11254 (3565a) added PathResolver defined in Utils module.

It seems that the client gets directory settings via BootstrapConfig. And there are no usages of DirectoryService on the client side. The ClientCore uses a simple version of this service defined by Directory interface via DirectoryManager. The target absolute-path-prefix-map map should be downloaded by the client in order to set up PathResolver for its usages in StreamDaemon and FileSystemDaemon.

Consider sending absolute-path-prefix-map down to the client in FileSystemOps.ContextContainer.initialValue().

OK. It is unclear how to support absolute-path-prefix-map if there are Windows and Linux clients simultaneously accessed to the FWD server?

My code failed to read file-system node due to it has no read access writes to the target nodes. It seems that the search algorithm looks up only these paths:
/server/standard/ and /server/default but in my directory there exists only this path to file-system /server/standard/runtime/default/file-system. Thus this code is failed to find file-system node

      DirectoryService ds = DirectoryService.getInstance();
      ds.bind();
      String testPath = Utils.findDirectoryNodePath(ds,
               "file-system/file-separator",
               "string",
               false);
      String resolverMapNodePath = Utils.findDirectoryNodePath(ds,
               "file-system/absolute-path-prefix-map",
               "container",
               false);

Please don't mind. If the scope of search is defined to be accounts search, then similar paths to /server/standard/runtime/default/file-system can be found.

We can't use : colon character, / slash and \ back slash in node names. It seems that these nodes are invalid

<node class="string" name="c:">
  <node-attribute name="value" value="/opt/my_app"/>
</node>
<node class="string" name="c:\app\environment">
  <node-attribute name="value" value="/opt/my_app/env"/>
</node>

That is true for the name attribute since it is part of the "path" to the node.

You will need to implement something like what Constantin suggested in #3565-6.

Planning to use redundant data for the same mapping

<node class="entry" name="entry1">
  <node-attribute name="key" value="c:"/>
  <node-attribute name="value" value="/opt/my_app"/>
</node>
<node class="entry" name="entry2">
  <node-attribute name="key" value="c:\app\environment"/>
  <node-attribute name="value" value="/opt/my_app/env"/>
</node>

Greg, if we add legacy-system node, then what is the responsibility file-system node? There are file-system/pkgroot and file-system/4gl-src-root nodes. Do these changes just extend/rename file-system to legacy-system? All related java docs state that these nodes under file-system are legacy system parameters.

then what is the responsibility file-system node?

It won't exist.

Do these changes just extend/rename file-system to legacy-system?

The idea is to rename file-system to legacy-system.

There are file-system/pkgroot and file-system/4gl-src-root nodes.

These would exist under legacy-system.

Thank you, see the idea now.

There is "legacyPlatform"

   public static String getLegacyPlatform()
   {
      WorkArea wa = work.obtain();

      if (wa.legacyPlatform == null)
      {
         String os = getOperatingSystem().toStringMessage();
         wa.legacyPlatform = Utils.getDirectoryNodeString(null, "legacyPlatform", os);
      }

      return wa.legacyPlatform;
   }

Greg, please help to explain the propath values. Is it correct that if propath lists legacy paths, then it must be mapped to the client host system paths? (legacy-system/propath)

It seems that this "legacyPlatform" has similar meaning as new proposed legacy-system/opsys.

You are right.

Please shift the usage of legacyPlatform (only in ColorSpec today) to the new legacy-platform/opsys.

It seems that my changes in file-separator has an impact on the conversion of windows path strings

path = "c:\app\environment\test-path-resolver.txt".

path.assign("c:appenvironment\test-path-resolver.txt");

I need advices how to convert this program correctly

def var path as char.
path = "c:\app\environment\test-path-resolver.txt".
message path.

Testing this committed revision 11258. Please look at this revision. The directory changes are

      <node class="container" name="standard">
        <node class="container" name="runtime">
          <node class="container" name="default">
            <node class="container" name="legacy-system">
              <node class="string" name="pkgroot">
                <node-attribute name="value" value="com.goldencode.testcases"/>
              </node>
              <node class="string" name="opsys">
                <node-attribute name="value" value="WIN32"/>
              </node>
              <node class="string" name="propath">
                <node-attribute name="value" value=".;.\icons\"/>
              </node>
              <node class="string" name="path-separator">
                <node-attribute name="value" value=";"/>
              </node>
              <node class="string" name="file-separator">
                <node-attribute name="value" value="\"/>
              </node>
              <node class="boolean" name="case-sensitive">
                <node-attribute name="value" value="FALSE"/>
              </node>
              <node class="container" name="absolute-path-prefix-map">
                <node class="entry" name="entry1">
                  <node-attribute name="key" value="c:"/>
                  <node-attribute name="value" value="/opt/gui"/>
                </node>
                <node class="entry" name="entry2">
                  <node-attribute name="key" value="c:\app\environment"/>
                  <node-attribute name="value" value="/opt/gui/Environments"/>
                </node>
                <node class="entry" name="entry3">
                  <node-attribute name="key" value="c:\tmp"/>
                  <node-attribute name="value" value="/tmp"/>
                </node>
              </node>
            </node>
          </node>
        </node>

I don't understand why this converted code works with "\\", but doesn't work with "\"

def var path as char.
path = "c:\\app\\environment\\test-path-resolver.txt".
message path.

   output to value(path) page-size 10.

   repeat i = 1 to 1000:
     display "this is a test line" i.
   end.

   output close.

Sergey Ivanovskiy wrote:

Greg, please help to explain the propath values. Is it correct that if propath lists legacy paths, then it must be mapped to the client host system paths? (legacy-system/propath)

The PROPATH is used in 3 ways:

1. We use it on the server-side in SourceNameMapper as a kind of set of relative "virtual file system" paths that can be used to resolve 4GL program names to 4GL source programs that have been converted. In this usage, we already provide the 4gl-src-root value to handle absolute path prefixes. We likewise already handle the case-insensitive searching (if needed) and we canonicalize the paths (everything is shifted to /). I believe that SourceNameMapper needs no changes.

2. We use it on the server-side in LogicalTerminal.getImageStreamFromApplication() and LogicalTerminal.getResourceStreamFromApplication() along with the pkgroot to search in the application jar files for resources like images, icons, mouse pointers and jasper report definitions. It is possible we might need to honor 4gl-src-root here. The absolute-path-prefix-map would be a similar solution but it is not quite right. In SourceNameMapper, any match to a value in the 4gl-src-root list is removed from the search string and then the result is treated as a relative name for the rest of the search. This is the same idea in the LT except we don't honor the 4gl-src-root here yet. We should. But no other changes are needed since it already handles path separators and case-insensitivity.

3. In any usage of FileSystemDaemon.search*() on the client side, we use it for lookup in the actual file system. Some of these use cases are for the converted 4GL SEARCH() built-in function. It is also used during conversion from FileOperationsWorker, from ThinClient.getImageFileName() and from FileSystemDaemon.accessFileInfo() which is for the converted 4GL FILE-INFO system handle support. Some of these use cases are only useful for relative paths BUT others can work with absolute paths. For this reason I think it does make sense to implement the absolute-path-prefix-map mapping here.

Does anyone else have some thoughts or opinions?

Sergey Ivanovskiy wrote:

I need advices how to convert this program correctly
[...]

There is no file system usage here. This has nothing to do with your changes. It is just displaying a string.

Sergey Ivanovskiy wrote:

I don't understand why this converted code works with "\\", but doesn't work with "\"
[...]
On the native system it works.

In cfg/p2j.cfg.xml, do you have opsys set to win32 (e.g. <parameter name="opsys" value="win32" />)?

If not, then the conversion will assume that the original system was UNIX and it will treat \ characters as escape chars. That is why doubling them works because it leaves behind a single \ as a result.

I added this setting <parameter name="opsys" value="win32" />

      <!-- values from the original system on which Progress 4GL ran -->
      <parameter name="path-separator"   value=":" />
      <parameter name="file-separator"   value="/" />
      <parameter name="case-sensitive"   value="true" />
      <parameter name="propath"          value=".:" />
      <parameter name="basepath"         value="." />
      <parameter name="oo-skeleton-path" value="./skeleton/" />
      <parameter name="unix-escapes"     value="false" /> <!-- I changed this value to be false -->
      <parameter name="opsys" value="win32" />

Greg Shah wrote:

3. In any usage of FileSystemDaemon.search*() on the client side, we use it for lookup in the actual file system. Some of these use cases are for the converted 4GL SEARCH() built-in function. It is also used during conversion from FileOperationsWorker, from ThinClient.getImageFileName() and from FileSystemDaemon.accessFileInfo() which is for the converted 4GL FILE-INFO system handle support. Some of these use cases are only useful for relative paths BUT others can work with absolute paths. For this reason I think it does make sense to implement the absolute-path-prefix-map mapping here.

Does anyone else have some thoughts or opinions?

I'll add besside FileSystemDaemon.search all other FileSystemDaemon APIs (rename, delete, mkdirs, etc), and also the FileStream class, which is used (as in note #3565-32) to open a file stream. The idea is: any file access done by FWD at OS-level should be intercepted and the file path be matched/resolved via the absolute-path-prefix-map.

Please review the committed revision 11264. It should cover all file path usages.

It seems that OPEN-MIME-RESOURCE and OPEN-URL should resolve the file path if they open the local file. Correct?

path = "c:\app\file.txt".
output to value(path).
...................
output close.
...................
open-mime-resource string("text/plain") string("file:///" + replace(path, "\", "/")) true.
................
os-delete value(path).

path = "c:\app\file.txt".
output to value(path).
...................
output close.
...................
FILE-INFO:FILE-NAME = path.
path = FILE-INFO:FULL-PATHNAME. /* resolved path */
open-mime-resource string("text/plain") string("file:///" + replace(path, "\", "/")) true.

open-mime-resource mime-type file-path embedded.

open-mime-resource mime-type url embedded false.

Sergey, please test folder names with FILE-INFO handle, like FILE-INFO:FILENAME = "." (and other folder names), for both case sensitive and insensitive settings. FWD has problems here.

At least this patch I think is required:

### Eclipse Workspace Patch 1.0
#P p2j
Index: src/com/goldencode/p2j/util/FileSystemDaemon.java
===================================================================
--- src/com/goldencode/p2j/util/FileSystemDaemon.java    (revision 1566)
+++ src/com/goldencode/p2j/util/FileSystemDaemon.java    (working copy)
@@ -1183,7 +1183,7 @@
                                      File    file,
                                      String  filename)
    {
-      return caseSens ? ((file.exists() && (file.isFile() || allowDir)) ? filename : null) :
+      return (file.exists() && (file.isFile() || allowDir)) ? filename : caseSens ? null :
                         getCaseInsensitiveMatch(file, false);
    }
 }
\ No newline at end of file

Also, note that if the folder is not found and we are in case-insensitive, getCaseInsensitiveMatch(file, false) will never look for a folder, just files (the false argument disables folder lookup) - this needs to be fixed, too.

Constantin Asofiei wrote:

Sergey, please test folder names with FILE-INFO handle, like FILE-INFO:FILENAME = "." (and other folder names), for both case sensitive and insensitive settings. FWD has problems here.

OK, I will test relative paths. My changes covers only how to resolve absolute paths with help of the given absolute-path-prefix-map. I don't know the exact algorithm that 4GL uses to resolve relative paths.
Constantin, What is the issue? Please write it here or create a new issue.

Test FILE-INFO:FILENAME and associated attributes like PATHNAME and FULL-PATHNAME, with FWD configured for legacy OS Windows and Linux, with paths like ., relative folders, full folder names. At least this patch for the issue in #3567-53 is needed:

### Eclipse Workspace Patch 1.0
#P p2j
Index: src/com/goldencode/p2j/util/FileSystemDaemon.java
===================================================================
--- src/com/goldencode/p2j/util/FileSystemDaemon.java    (revision 1566)
+++ src/com/goldencode/p2j/util/FileSystemDaemon.java    (working copy)
@@ -48,6 +48,8 @@
 **                           of usage are safe.
 ** 020 EVL 20180205          Fix for Windows Matcher.replaceAll() method issue with file
 **                           separator as input parameter.
+** 021 CA  20180520          Fixed getFileName - if the file/folder exists, use it, regardless of
+**                           case-sensitivity.
 */

 /*
@@ -1183,7 +1185,7 @@
                                      File    file,
                                      String  filename)
    {
-      return caseSens ? ((file.exists() && (file.isFile() || allowDir)) ? filename : null) :
+      return (file.exists() && (file.isFile() || allowDir)) ? filename : caseSens ? null :
                         getCaseInsensitiveMatch(file, false);
    }
 }
\ No newline at end of file

The getCaseInsensitiveMatch(file, false) is incorrect, as for case-insensitive legacy OS, it will never look for folders, only for files.

Sergey, will your changes handle image loading from the file system as well? I am not that familiar with the UI side of the runtime, but as I recall, that operates through a different API, doesn't it? If so, how much effort will it be to apply the same technique to that API?

Eric, could you point on the case where the image loading problem appeared? There is LogicalTerminal.getImageStreamFromAplication

   /**
    * Gets the image stream from application jar file or nul if the resource is not found.
    * 
    * @param    imgName
    *           The image name to be loaded on server side.
    *
    * @return   Stream object to load image data or NULL if data is not found.
    */
   private static InputStream getImageStreamFromApplication(String imgName)

Don't mind it is due to java doc doesn't describe that if the target resource is not found in its application jar, then PROPATH must be taken into account

               // not found in root continue with PROPATH entries
               if (isRes == null)
               {
                  for (String header : propathHeader)
                  {
                     // look through PROPATH related locations, clean up possible Windows
                     // separators
                     header = header.replace(WIN_SEP, JAR_SEP);
                     // inside jar we need to use UNIX-style file separator even on Windows
                     String name = getRefinedPathName(
                        pkgRootPath + header + JAR_SEP + imgName, JAR_SEP_STR);
                     // try to search new name based on PROPATH header
                     resName = ServerImageWorker.getResourceName(jcl, name, caseSens);
                     if (resName != null)
                     {
                        isRes = jcl.getResourceAsStream(resName);
                     }
                     // first match is enough to stop searching in PRPPATH
                     if (isRes != null)
                     {
                        break;
                     }
                  }
               }
 

If we investigate the PROPATH of the converted application, then we can find that all these paths are relative to the current application.
absolute-path-prefix-map considers only absolute paths.

In this function the target resource is looking through the value of searchpath defined by the directory. This is the place in StandardServer where its value is retrieved

      if (token != null && !token.isEmpty())
      {
         Utils.setProjectToken(token);
         // some settings could have been already set at a previous moment when project_token was
         // not available yet
         EnvironmentOps.setSearchPath(Utils.getDirectoryNodeString(ds, "searchpath", "."), false);
         // TODO: update other settings
      }

Sergey Ivanovskiy wrote:

Eric, could you point on the case where the image loading problem appeared?

The problem occurs in a customer project. I haven't tracked down the code yet, but it seems to be loading an image from the file system using an absolute path. I'll follow up with more info later today.

Eric Faulhaber wrote:

Sergey Ivanovskiy wrote:

Eric, could you point on the case where the image loading problem appeared?

The problem occurs in a customer project. I haven't tracked down the code yet, but it seems to be loading an image from the file system using an absolute path. I'll follow up with more info later today.

If you are seeing FILE-INFO:FILENAME then it will not find it without the patch I mentioned.

It seems that this implementation of the search FileSystemDaemon.searchAll searches file system roots first and if it is found returns found matches

  public static List<String> searchAll(String[] paths,
                                        boolean  caseSens,
                                        String   filename,
                                        boolean  allowDir)
   {
      // make sure there is something to search for (the code below needs to be protected)
      if (filename == null)
      {
         return null;
      }

      // if the the OS is windows replace all / to \ from the search paths
      String fn = normalizeFileSeparators(filename);

      String result = getAbsoluteOrCurrent(caseSens, fn, allowDir);

      if (result != null)
      {
         List<String> ret = new ArrayList<>(1);
         ret.add(result);
         return ret;
      }

      // not found in root or current, try PROPATH entries
      if (paths == null)
      {
         return null;
      }
.........................
   }

Eric Faulhaber wrote:

Sergey, will your changes handle image loading from the file system as well? I am not that familiar with the UI side of the runtime, but as I recall, that operates through a different API, doesn't it? If so, how much effort will it be to apply the same technique to that API?

Please review the committed rev 11265-66 that applies legacy absolute path resolver to entries in searchpath in order to resolve absolute path names correctly by LogicalTerminal.getImageStreamFromAplication. To resolve relative path names the fix #3565-43 is given by Constantin in #3567.

It seems that OPEN-MIME-RESOURCE and OPEN-URL should resolve the file path if they open the local file. Correct?

Yes.

Code Review Task Branch 3565a Revision 11266

1. Why is this code duplicated in multiple FileSystemDaemon locations?

   int size = paths != null ? paths.length : 0;

   for(int i = 0; i < size; i++)
   {
      paths[i] = getLegacyPathResolver().resolvePath(paths[i]);
   }

If the path array needs fixups, why not do that one time, during setLegacyPathResolver()?

2. Is it really necessary to call getLegacyPathResolver() multiple times in the same method? This seems to add extra cost of creating a new PathResolverContainer instance each time. Is there a functional reason for that?

3. Don't the following FileSystemDaemon methods also need path resolver processing? getCaseInsensitiveMatch(), searchPathWithExts(), searchAll(), search()

4. Constantin's patch from #3565-41 is missing.

5. Shouldn't the client (e.g FileSystemDaemon...) know the legacy OPSYS value (getLegacyPlatform())? It seems like some processing on the client should be done based on this value and how it compares to the actual client operating system. A "mismatch" flag could be calculated once and then checked when needed. I think the current code handles this by passing null for the pathResolver. I would prefer if this "decision" was easier to see in the code. It could be hidden in a helper in Utils. The current approach with the PathResolverContainer seems to add more complication and hide the true nature of the processing.

6. Some of the comments (e.g. Utils) refer to the "native file system". This is confusing as it is not clear that this is the same as the legacy system. If these are meant to be the same thing, please be consistent and use "legacy" instead of "native".

7. Please make the PathResolver and PathResolverImpl into separate classes. I don't see the value in hiding them inside Utils. There is no special data access, data hiding or method usage that makes them a good fit to be inner classes.

Greg Shah wrote:

Code Review Task Branch 3565a Revision 11266

3. Don't the following FileSystemDaemon methods also need path resolver processing? getCaseInsensitiveMatch(), searchPathWithExts(), searchAll(), search()

I checked these static usages
searchAll is used only

com.goldencode.p2j.pattern.FileOperationsWorker.FileOps.findFiles(String)

com.goldencode.p2j.pattern.FileOperationsWorker.FileOps.findFile(String)
com.goldencode.p2j.util.FileSystemDaemon.accessFileInfo(String)
com.goldencode.p2j.util.FileSystemDaemon.searchPath(String)

com.goldencode.p2j.util.FileSystemDaemon.accessFileInfo(String)
com.goldencode.p2j.util.FileSystemDaemon.searchPath(String)

com.goldencode.p2j.ui.chui.ThinClient.getImageFileName(String)

com.goldencode.p2j.uast.SymbolResolver.exists(File, boolean)
com.goldencode.p2j.util.FileSystemDaemon.getFileName(boolean, boolean, File, String)

   private boolean exists(File file, boolean caseSens)
   {
      // quick out: does the exact match exist?
      if (file.exists())
      {
         return true;
      }

      if (!caseSens)
      {
         // TODO: this is broken!!!!  It was found to fail when the exact case-sensitive match
         //       actually existed (resolved by checking for that first, above).  It is not
         //       known if it fails in other cases too.
         return FileSystemDaemon.getCaseInsensitiveMatch(file, true) != null;
      }

      return false;
   }

I searched usages of FileSystemDaemon constructor

com.goldencode.p2j.ui.chui.ThinClient.ThinClient(boolean)
com.goldencode.p2j.ui.client.Editor<O extends OutputManager<?>>.editorReadFile(String)

Greg, could I fix it to

=== modified file 'src/com/goldencode/p2j/ui/client/Editor.java'
--- src/com/goldencode/p2j/ui/client/Editor.java    2017-05-18 15:27:32 +0000
+++ src/com/goldencode/p2j/ui/client/Editor.java    2018-05-21 17:15:53 +0000
@@ -672,7 +672,7 @@
    {
       //todo client-side stream classes should probably be used for read/write of the files 
       //todo (instead of NIO) to ensure that the I18N behavior of the 4GL is honored.
-      FileSystemDaemon fsd  = new FileSystemDaemon(false);
+      FileSystemDaemon fsd  = ThinClient.getInstance().getFileSystemDaemon();
       String fileNameToLoad = fsd.searchPath(name);
       if (fileNameToLoad == null)
       {

Greg Shah wrote:

Code Review Task Branch 3565a Revision 11266

1. Why is this code duplicated in multiple FileSystemDaemon locations?

[...]

If the path array needs fixups, why not do that one time, during setLegacyPathResolver()?

this.paths field is set from void setSearchPath(String[] paths, boolean caseSens). If we applied path resolution to this field in setLegacyPathResolver(), then we should be sure that setSearchPath is used before setLegacyPathResolver(). Another variant is to unite these two method in one method with this name setLegacySearchPath(....).

searchAll is used only
search is used only

OK, good analysis. Please add comments to both of these to explain why they don't need the absolute path processing.

This static method getCaseInsensitiveMatch() is used by

com.goldencode.p2j.uast.SymbolResolver.exists(File, boolean)

...
Is it really needed to apply path resolution here?

No. The SymbolResolver is only used for conversion purposes and we don't need the path resolution at that time.

It seems that com.goldencode.p2j.uast.SymbolResolver.exists(File, boolean) is broken according to these code comments(?)

Possibly, but we don't have a recreate. Anyway, it is for conversion so we can ignore it for this task.

Is the second usage correct? Why it needs the second instance of FileSystemDaemon.

I don't know.

Constantin?

Sergey Ivanovskiy wrote:

I searched usages of FileSystemDaemon constructor
[...]
Is the second usage correct? Why it needs the second instance of FileSystemDaemon.

The second usage is not correct. All client-side Daemon classes should be singletons. Please fix it as you proposed.

I did a mistake

And in this instance method com.goldencode.p2j.util.FileSystemDaemon.getFileName(boolean, boolean, File, String) PathResolver is already applied.

This is a static method too and PathResolver is not applied here. In static methods we can use something like this

ThinClient.getInstance().getFileSystemDaemon().getFileSystemResolver()

ThinClient.getInstance().getFileSystemDaemon()

Surely we should not need to call ThinClient from FileSystemDaemon in order to get the singleton instance of the same class. Let's change the initialization to cache the FileSystemDaemon singleton instance inside FileSystemDaemon.

Greg, please review 11268. I encountered text conflict in Utils.java that it is possible that your changes can be lost due to these classes PathResolver and PathResolverImpl are in separate files.

Committed revision 11269 fixed java docs.

Committed revision 11270 added src/com/goldencode/p2j/util/PathResolverContainer.java.

3565a rev 11272 has been updated over trunc rev 11257.

Can I start regression testing for 3565a?

Code Review Task Branch 3565a Revision 11272

I am OK with the changes.

this.paths field is set from void setSearchPath(String[] paths, boolean caseSens). If we applied path resolution to this field in setLegacyPathResolver(), then we should be sure that setSearchPath is used before setLegacyPathResolver(). Another variant is to unite these two method in one method with this name setLegacySearchPath(....).

This is the last issue to resolve. setSearchPath can be called multiple times, even from converted code (PROPATH = some_expression.).

1. Change the initialization to send down all the FSD configuration (the propath, case-sensitive, absolute path map...) in 1 call when the session starts.

2. Move the path array fixup code to a separate method. Call that method from the initialization processing AND from setSearchPath().

I just tried using this branch with a project that does not have any directory changes. I got:

Caused by: java.lang.NullPointerException
        at com.goldencode.p2j.util.EnvironmentOps.getLegacyAbsolutePathPrefixMap(EnvironmentOps.java:952)
        at com.goldencode.p2j.util.FileSystemOps$ContextContainer.initialValue(FileSystemOps.java:1223)
        at com.goldencode.p2j.security.ContextLocal.get(ContextLocal.java:462)
        at com.goldencode.p2j.security.ContextLocal.get(ContextLocal.java:419)
        at com.goldencode.p2j.util.FileSystemOps$ContextContainer.obtain(FileSystemOps.java:1191)
        at com.goldencode.p2j.util.FileSystemOps.init(FileSystemOps.java:261)
        at com.goldencode.p2j.ui.LogicalTerminal.init(LogicalTerminal.java:1226)
        at com.goldencode.p2j.security.ContextLocal.initializeValue(ContextLocal.java:596)
        at com.goldencode.p2j.security.ContextLocal.get(ContextLocal.java:464)
        at com.goldencode.p2j.security.ContextLocal.get(ContextLocal.java:419)
        at com.goldencode.p2j.ui.LogicalTerminal.locate(LogicalTerminal.java:11125)
        at com.goldencode.p2j.ui.LogicalTerminal.lambda$initialize$3(LogicalTerminal.java:12199)
        at com.goldencode.p2j.util.TransactionManager$ContextContainer.obtain(TransactionManager.java:6337)
        at com.goldencode.p2j.util.TransactionManager.isHeadless(TransactionManager.java:3451)
        at com.goldencode.p2j.util.ErrorManager$ServerDataAccess.isHeadless(ErrorManager.java:2672)
        at com.goldencode.p2j.util.ErrorManager.isHeadless(ErrorManager.java:2087)
        at com.goldencode.p2j.util.SessionUtils$1.initialValue(SessionUtils.java:141)
        at com.goldencode.p2j.util.SessionUtils$1.initialValue(SessionUtils.java:125)
        at com.goldencode.p2j.security.ContextLocal.get(ContextLocal.java:462)
        at com.goldencode.p2j.security.ContextLocal.get(ContextLocal.java:419)
        at com.goldencode.p2j.util.SessionUtils.getSessionTooltips(SessionUtils.java:877)
        at com.goldencode.p2j.ui.LogicalTerminal.getSessionTooltips(LogicalTerminal.java:15137)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at com.goldencode.p2j.util.MethodInvoker.invoke(MethodInvoker.java:124)
        at com.goldencode.p2j.net.Dispatcher.processInbound(Dispatcher.java:757)
        at com.goldencode.p2j.net.Conversation.block(Conversation.java:412)
        at com.goldencode.p2j.net.Conversation.run(Conversation.java:232)
        at java.lang.Thread.run(Thread.java:748)

This is an optional feature, so the runtime must work even if the directory contains no legacy-system/absolute-path-prefix-map path.

It seems that I missed this issue because I got this case too without proper settings. Thank you, planning to fix it with the last review.

Sergey Ivanovskiy wrote:

Testing this committed revision 11258. Please look at this revision. The directory changes are
[...]

Does the opsys node now under legacy-system in the directory replace the existing opsys node under the runtime/default path?

            <node class="container" name="opsys">
              <node class="string" name="override">
                <node-attribute name="value" value="WIN32"/>
              </node>
            </node>

Note that the old opsys node is a container with that override node as a child. We don't have that in the proposed new opsys node under legacy-system. Is it needed?

legacy-system/opsys is the definition of the original OPSYS value from the legacy system (the system on which the 4GL version of the application executed. We don't expose this to the converted 4GL code. We just use this in our decision making in the runtime.

The runtime/default/opsys/override is the way that we force the converted OPSYS to report something different from the actual client OS.

They are 2 different things.

Please review the committed revision 11273.

This is not a code review, but I have a minor request: please rename UnResolvedPathException to UnresolvedPathException (i.e., lowercase "r").

OK. It has been fixed in committed revision 11274.

Code Review Task Branch 3565a Revision 11274

I'm good with the changes.

Please use Level.FINER instead of Level.CONFIG.

After that, if the code is working well in the current customer application being tested, then we can go with this code.

Committed revision 11275 reduced the log level to FINER. My environment is not ready now. I am building the customer application.

Sergey, please rebase 3565a. I have a converted project that depends upon changes in trunk revision 11259. Thanks.

OK. 3565a has been updated over the trunc rev. 11259 up to the rev. 11277.

I started regression testing for rev 11280, but forgot to setup manually the directory that has changes.

remote 3565a branch for regression testing was created with corresponding directory changes: file-system to legacy-system and regression testing has been started.

3565a passed regression testing, planning to merge it into the trunk now.

3565a has been merged into the trunc as bzr revision 11261 and archived.

Sergey, I'm getting errors loading files using relative paths. The use case:

StreamFactory.openFileStream((relativePath).toStringMessage(), false, false)

...where relativePath represents a Windows path from the legacy system that uses different case than the actual file on disk (on Linux). The physical file does exist at the specified location, but some characters in the path or in the filename itself don't match the case of the specified path. Permissions on the directories leading to the file from the current directory and on the file itself are wide open (777).

The error I get:

** "./<path to file>/<file name>" was not found. (293)

or

** "<path to file>/<file name>" was not found. (293)

The directory is configured as follows:

            <node class="container" name="legacy-system">
              ...
              <node class="string" name="opsys">
                <node-attribute name="value" value="WIN32"/>
              </node>
              <node class="string" name="propath">
                 ...
              </node>
              <node class="string" name="path-separator">
                <node-attribute name="value" value=";"/>
              </node>
              <node class="string" name="file-separator">
                <node-attribute name="value" value="\"/>
              </node>
              <node class="boolean" name="case-sensitive">
                <node-attribute name="value" value="FALSE"/>
              </node>
              <node class="container" name="absolute-path-prefix-map">
                 ...
              </node>
            </node>

Could you please double-check this use case?

OK. StreamDaemon.openFileStream doesn't implement case insensitive search. It only maps absolute legacy paths to host file paths.

Sergey Ivanovskiy wrote:

OK. StreamDaemon.openFileStream doesn't implement case insensitive search. It is only map absolute legacy paths to host file paths.

How much effort to add this? It is pretty urgent.

Please specify how it should work precisely. Should this relative path name be searched in PROPATH or SEARCHPATH that set up via FileSystemDaemon.setSearchPath(String[] paths, boolean caseSens)? It can be implemented similarly to ThinClient.getImageFileName if I understand this task correctly.

Sergey Ivanovskiy wrote:

Please specify how it should work precisely. Should this relative path name be searched in PROPATH or SEARCHPATH that set up via FileSystemDaemon.setSearchPath(String[] paths, boolean caseSens)? It can be implemented similarly to ThinClient.getImageFileName if I understand this task correctly.

Greg, I need your input on this one.

StreamDaemon.openFileStream comes from two language statements:

1. INPUT FROM which opens a file for reading. This does NOT support PROPATH. From the 4GL docs:

The absolute or relative pathname of a file that contains the data you want to input. Any relative pathname is relative to the current working directory.

2. OUTPUT TO which opens a file for writing (optionally append but by default overwrite). This will create a file that is not there, but if it exists it must open it or delete and then recreate it. Again, there is no PROPATH usage. From the 4GL docs:

The absolute or relative pathname of a file to which you want to direct output. If you specify a relative pathname, the AVM locates the pathname relative to the current working directory. The pathname can contain up to 255 characters. If a file with the specified pathname already exists, the AVM overwrites it.

The APPEND option modifies the default overwrite mode.

To summarize: no PROPATH but path and filename processing for existing paths and filenames must honor case insensitivity.

Sergey Ivanovskiy wrote:

Eric, please test this one
[...]

Based on Greg's response, that doesn't look right.

Created 3565b task branch.

I can confirm that INPUT FROM does not use the search path, as Sergey found through separate testing.

So, the two things to fix in 3565b are:

• make the FWD SEARCH implementation work the same as Progress w.r.t. leading .\ or ./ file name qualifiers;
• make StreamDaemon honor the case sensitivity setting.

Eric Faulhaber wrote:

So, the two things to fix in 3565b are:

• make the FWD SEARCH implementation work the same as Progress w.r.t. leading .\ or ./ file name qualifiers;

Now I didn't find a way to fix it correctly. It seems that it is a minor issue because it looks odd that Progress treats ./ as an absolute path from the current directory.

• make StreamDaemon honor the case sensitivity setting.

Committed rev 11263 should implement this functionality.