Wicket
  1. Wicket
  2. WICKET-4443

AbstractClassResolver recreates URL incorrectly

    Details

      Description

      AbstractClassResolver converts java.net.URLs to their external form (java.lang.String), then converts them back to java.net.URLs. It looks like the conversion to external form is for comparison between URLs (comparing URLs is slow, Strings not so - see WICKET-3867), because we only want one result per URL (Set<URL>).

      The problem is that, when converting from the external form back to a java.net.URL, the no context URL is given to the URL constructor, just the external form String is given. Passing a context URL to the java.net.URL constructor is the only way of setting the URLStreamHandler related to the URL. So, if you have some exotic URL schema, like I do, using protocols the standard Java libraries dont know about, you can no longer load your resources.

      Please see the code in CompoundClassResolver#getResources() - it implements a unique list of URL by comparing the external forms of the URLs, while still managing to use the original URL objects, with their URLStreamHandlers attached.

        Activity

        Hide
        Jesse Long added a comment -

        Patch that changes various parts, mainly IClassResolvers to use a TreeSet with a Comparator that compares URLs' toExternalForm when a Set<URL> is needed.

        Also, add some docs about possible MalformedURLException in UrlResourceStreamReference (related to this bug)

        Show
        Jesse Long added a comment - Patch that changes various parts, mainly IClassResolvers to use a TreeSet with a Comparator that compares URLs' toExternalForm when a Set<URL> is needed. Also, add some docs about possible MalformedURLException in UrlResourceStreamReference (related to this bug)
        Hide
        Martin Grigorov added a comment -

        Thanks for the patch!

        I added a note to UrlResourceStreamReference's javadoc explaining how to deal with custom URL schemes. Unfortunately there is no getter method in j.n.URL to get its handler and reuse it later ...

        Show
        Martin Grigorov added a comment - Thanks for the patch! I added a note to UrlResourceStreamReference's javadoc explaining how to deal with custom URL schemes. Unfortunately there is no getter method in j.n.URL to get its handler and reuse it later ...
        Hide
        Jesse Long added a comment -

        UrlResourceStreamReference caches the URL as a string. Is there any reason for this? If we just keep a reference to the URL object, it may suffice.

        My first thought was that it caches the URL as a string in external form because it needs to be serialized, but UrlResourceStreamReference does not implement

        {Serializable, IClusterable}

        and neither does its parent - AbstractResourceStreamReference, and neither does the interface require serialization - IResourceStreamReference. Could we not just let UrlResourceStreamReference keep a reference to the original URL?

        Show
        Jesse Long added a comment - UrlResourceStreamReference caches the URL as a string. Is there any reason for this? If we just keep a reference to the URL object, it may suffice. My first thought was that it caches the URL as a string in external form because it needs to be serialized, but UrlResourceStreamReference does not implement {Serializable, IClusterable} and neither does its parent - AbstractResourceStreamReference, and neither does the interface require serialization - IResourceStreamReference. Could we not just let UrlResourceStreamReference keep a reference to the original URL?
        Hide
        Martin Grigorov added a comment -

        I'll investigate this option next week.

        Show
        Martin Grigorov added a comment - I'll investigate this option next week.
        Hide
        Jesse Long added a comment -

        I looked around. UrlResourceStreamReference is package private to the org.apache.wicket.util.resource.locator.caching package. It is used ONLY by the CachingResourceStreamLocator class, which converts UrlResourceStream instances to UrlResourceStreamReference instances, and stores them in a java.util.Map.

        This map acts as a cache of the result of locating resources, including not-found entries. The idea behind the cache it to avoid having to FIND resources every time they are required, but just use the caches find result from the map.

        The map is never serialized, as CachingResourceStreamLocator is not Serializable and neither is IResourceStreamLocator, which it implements.

        IResourceStreamLocator is mainly used from: Application.get().getResourceSettings().getResourceStreamLocator().

        I vote we just keep a reference to the original URL.

        Pro: we can load arbitrary URLs, from any url scheme, using its configured URLStreamHandler.

        Con: the URL object probably uses slightly more memory that the external form string.

        Show
        Jesse Long added a comment - I looked around. UrlResourceStreamReference is package private to the org.apache.wicket.util.resource.locator.caching package. It is used ONLY by the CachingResourceStreamLocator class, which converts UrlResourceStream instances to UrlResourceStreamReference instances, and stores them in a java.util.Map. This map acts as a cache of the result of locating resources, including not-found entries. The idea behind the cache it to avoid having to FIND resources every time they are required, but just use the caches find result from the map. The map is never serialized, as CachingResourceStreamLocator is not Serializable and neither is IResourceStreamLocator, which it implements. IResourceStreamLocator is mainly used from: Application.get().getResourceSettings().getResourceStreamLocator(). I vote we just keep a reference to the original URL. Pro: we can load arbitrary URLs, from any url scheme, using its configured URLStreamHandler. Con: the URL object probably uses slightly more memory that the external form string.
        Hide
        Martin Grigorov added a comment -

        The suggestion is committed.

        Show
        Martin Grigorov added a comment - The suggestion is committed.

          People

          • Assignee:
            Martin Grigorov
            Reporter:
            Jesse Long
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development