Re: [PATCH] Fix JavaHL testBasicProperties on Windows

On Wed, 11 Apr 2007, Mark Phippard wrote:

> The JavaHL properties functions were rewritten to use a new callback
> API. When that API is called it passes in a path in a format like
> C:/folder1/folder2/file.ext. This value is stored as a key in a Map.
> When the getProperties method is later called it is passed a String in
> the format C:\folder1\folder2\file.ext so it fails to find a value in
> the Map.
>
> The attached patch fixes the problem by normalizing the path in the
> get and set methods. I am not convinced this is the best solution and
> I do not like that we are fixing it in our Impl method as it means
> other people that implement the interface have to deal with the same
> problem.
>
> I also fixed a potential NullPointerError that this test failure revealed.
>
> Index: subversion/bindings/javahl/src/org/tigris/subversion/javahl/SVNClient.java^M
> ===================================================================^M
> --- subversion/bindings/javahl/src/org/tigris/subversion/javahl/SVNClient.java (revision 24532)^M
> +++ subversion/bindings/javahl/src/org/tigris/subversion/javahl/SVNClient.java (working copy)^M
> @@ -1253,6 +1253,8 @@^M
> properties(path, revision, pegRevision, false, callback);
>
> Map propMap = callback.getProperties(path);
> + if (propMap == null)
> + return new PropertyData[0];
> PropertyData[] props = new PropertyData[propMap.size()];
>
> Iterator it = propMap.keySet().iterator();

Please commit this portion of the patch.

> Index: subversion/bindings/javahl/src/org/tigris/subversion/javahl/ProplistCallbackImpl.java^M
> ===================================================================^M
> --- subversion/bindings/javahl/src/org/tigris/subversion/javahl/ProplistCallbackImpl.java (revision 24532)^M
> +++ subversion/bindings/javahl/src/org/tigris/subversion/javahl/ProplistCallbackImpl.java (working copy)^M
> @@ -31,11 +31,16 @@^M
>
> public void singlePath(String path, Map props)
> {
> - propMap.put(path, props);
> + propMap.put(normalizePath(path), props);
> }
>
> public Map getProperties(String path)
> {
> - return (Map) propMap.get(path);
> + return (Map) propMap.get(normalizePath(path));
> }
> +
> + private String normalizePath(String path)
> + {
> + return path.replace('\\', '/');
> + }
> }

We talked this over on IRC. The problem here boils down to the fact
that svn_proplist_receiver_t is passed a canonical path, rather than a
path in local style. Erik indicates that this is expected behavior
for our APIs. Hyrum will be adding a call to svn_path_local_style()
to JavaHL's C++ code to handle the conversion.

I asked Erik where we document this important API usage fact, but he
wasn't sure. Do we document this anywhere? If not, I suggest
documenting it in hacking.html.

- Dan