GeoTools
  1. GeoTools
  2. GEOT-4319

NADCONTransform does not work with ClasspathGridShiftLocator

    Details

    • Type: Bug Bug
    • Status: Open Open
    • Priority: Critical Critical
    • Resolution: Unresolved
    • Affects Version/s: 8.3
    • Fix Version/s: None
    • Component/s: referencing
    • Labels:
      None
    • Environment:
      Not applicable

      Description

      The NADCONTransform constructor takes two URIs which point to the location of grid shift files. In a deployed Java application, these URIs are generally obtained via something like:

      URL lonFile = getClass().getResource( "conus.los" );
      URL latFile = getClass().getResource( "conus.las" );
      
      NADCONTransform transform = new NADCONTransform( latFile.toURI(), lonFile.toURI() );
      

      However, the ClasspathGridShiftLocator essentially locates files by using the ClassLoader and looking for the file inside the classpath. The URI works out to be an absolute path prefixed with file:, so this is never found in the classpath.

      In fact, it is impossible to craft an URI that is usable by the ClasspathGridShiftLocator, thus rendering NADCONTransform unusable in this fashion.

      Additionally, NADCONTransform contains lots of legacy code and comments about preferences that are confusing as they are no longer referenced outside the test case that is present inside the file.

        Activity

        Hide
        Andrea Aime added a comment -
        You seem to have a good understanding of the code. I expect we'll see a patch from you that fixes the above issues, with good test coverage of the changes, yes?
        Show
        Andrea Aime added a comment - You seem to have a good understanding of the code. I expect we'll see a patch from you that fixes the above issues, with good test coverage of the changes, yes?
        Hide
        Lucas Madar added a comment -
        I would love to do this, but I don't understand why the transforms (NADCON/NTv2) have chosen to use URIs over Strings now for their grid files. If GridShiftLocator.locateGrid() is going to always take a string, and a GridShiftLocator can search for any resource with the string being somewhat of an opaque identifier, then why change the constructor to use URIs and then massage them into strings?

        I would suggest one of the following:
        - NADCONTransform/NTv2Transform take String instead of URI as they did before, however, this will not maintain the prior behavior in which the String was treated as a raw file path;
        - GridShiftLocator takes an Object and gets an isSupported( Object ) method, and a new DefaultGridShiftLocator is added that handles File/URI/URL. We could keep the ClasspathGridShiftLocator for String.

        I'm wary of these changes because I have no idea what they will break. It looks like all this code was added/modified in [GEOT-4063].
        Show
        Lucas Madar added a comment - I would love to do this, but I don't understand why the transforms (NADCON/NTv2) have chosen to use URIs over Strings now for their grid files. If GridShiftLocator.locateGrid() is going to always take a string, and a GridShiftLocator can search for any resource with the string being somewhat of an opaque identifier, then why change the constructor to use URIs and then massage them into strings? I would suggest one of the following: - NADCONTransform/NTv2Transform take String instead of URI as they did before, however, this will not maintain the prior behavior in which the String was treated as a raw file path; - GridShiftLocator takes an Object and gets an isSupported( Object ) method, and a new DefaultGridShiftLocator is added that handles File/URI/URL. We could keep the ClasspathGridShiftLocator for String. I'm wary of these changes because I have no idea what they will break. It looks like all this code was added/modified in [ GEOT-4063 ].

          People

          • Assignee:
            Andrea Aime
            Reporter:
            Lucas Madar
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated: