Velocity
  1. Velocity
  2. VELOCITY-439

Improve Resource existence detection

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 1.4, 1.5
    • Fix Version/s: 1.6
    • Component/s: Engine
    • Labels:
      None

      Description

      Depending on the ResourceLoader used, testing the existence of a resource is
      somewhat expensive, as the resource has to be opened to test for its existence.

      I'm proposing the following changes:
      1.
      Add a new method to org.apache.velocity.runtime.resource.loader.ResourceLoader:
      public boolean resourceExists(String source) {
      InputStream is = null;
      try {
      is = resourceLoader.getResourceStream(resourceName);
      if (is != null)

      { return true; }

      } catch (ResourceNotFoundException e) {
      } finally {
      if (is != null) {
      try

      { is.close(); }

      catch (Exception e) {
      }
      }
      }
      return false;
      }

      This method keeps compatibility with all current ResourceLoaders, and can be overriden by subclasses.

      2. org.apache.velocity.runtime.resource.ResourceManagerImpl
      Modify the String getLoaderNameForResource(String resourceName) method to use
      the new ResourceLoader.resourceExists(String) method.

      1. resourceloader.patch
        2 kB
        Henning Schmiedehausen

        Activity

        Hide
        Will Glass-Husain added a comment -

        nice idea, thanks for suggesting it.

        Note that you can always do this in the calling code. This idea though provides a default implementation while
        allowing specific resource loaders to checl the resource existence directly (and likely more efficiently).

        Show
        Will Glass-Husain added a comment - nice idea, thanks for suggesting it. Note that you can always do this in the calling code. This idea though provides a default implementation while allowing specific resource loaders to checl the resource existence directly (and likely more efficiently).
        Hide
        Henning Schmiedehausen added a comment -

        I also like this and I actually wrote up some code based on your example (attached as a patch). However, there are a few more things to check out before we actually add this:

        There is a method in the VelocityEngine, called resourceExists(). It uses getLoaderNameForResource to determine whether a resource exists or not. It also leverages the cache to reduce the number of hits to the actual resource loaders. There is some code in ResourceManagerImpl around line 590, that very closely resembles the proposed code, so it might probably be replaced with a call to resourceExists, but there are no unit tests for this and just ripping the current code out without test is not really a good thing. Especially as we are discussing to cut an RC now.

        I do postpone this for 1.6. Tassos, if you could write some unit tests, that check the functionality of VelocityEngine.resourceExists() and also the new resourceExists() method in e.g. the File loader, this would greatly improve the chances that your code goes in.

        Show
        Henning Schmiedehausen added a comment - I also like this and I actually wrote up some code based on your example (attached as a patch). However, there are a few more things to check out before we actually add this: There is a method in the VelocityEngine, called resourceExists(). It uses getLoaderNameForResource to determine whether a resource exists or not. It also leverages the cache to reduce the number of hits to the actual resource loaders. There is some code in ResourceManagerImpl around line 590, that very closely resembles the proposed code, so it might probably be replaced with a call to resourceExists, but there are no unit tests for this and just ripping the current code out without test is not really a good thing. Especially as we are discussing to cut an RC now. I do postpone this for 1.6. Tassos, if you could write some unit tests, that check the functionality of VelocityEngine.resourceExists() and also the new resourceExists() method in e.g. the File loader, this would greatly improve the chances that your code goes in.
        Hide
        Henning Schmiedehausen added a comment -

        This is a patch based on the code suggested by Tassos.

        Show
        Henning Schmiedehausen added a comment - This is a patch based on the code suggested by Tassos.
        Hide
        Nathan Bubna added a comment -

        Ok, all done, including custom, faster implementations for FileResourceLoader and StringResourceLoader.

        Show
        Nathan Bubna added a comment - Ok, all done, including custom, faster implementations for FileResourceLoader and StringResourceLoader.

          People

          • Assignee:
            Unassigned
            Reporter:
            Tassos Bassoukos
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development