Thank you kindly for the review!
The attached revision of the patch uses the File(URI) constructor to
perform the necessary decoding.
On Sun, Nov 17, 2013 at 6:08 PM, Antoine Levy Lambert <anto...@gmx.de>wrote:
> Hello Charles,
>
> thanks for proposing a patch.
>
> I have read the patch and I see one area for improvement.
>
> Your patch seems to assume that File URLs have exactly the same syntax
> like file system paths except the file: prefix to indicate the protocol.
>
> You need to use URLDecoder or equivalent because a File URL will have some
> special characters encoded such as spaces replaced by %32.
>
> Regards,
>
> Antoine
>
> On Nov 11, 2013, at 1:51 PM, Charles Duffy wrote:
>
> > This isn't actually so important for URLResolver itself, which can be
> replaced with the FileSystem resolver at will -- but without this patch,
> one can't get the unique behavior of the IBiblio resolver combined with
> useOrigin.
> >
> > Feedback appreciated.
> > <url-local-cache-support.diff>
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
> > For additional commands, e-mail: dev-h...@ant.apache.org
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
> For additional commands, e-mail: dev-h...@ant.apache.org
>
>
Index: src/java/org/apache/ivy/plugins/repository/url/URLRepository.java
===================================================================
--- src/java/org/apache/ivy/plugins/repository/url/URLRepository.java
(revision 1543060)
+++ src/java/org/apache/ivy/plugins/repository/url/URLRepository.java
(working copy)
@@ -41,10 +41,12 @@
private Map resourcesCache = new HashMap();
+ private boolean localIfFileUrl = true;
+
public Resource getResource(String source) throws IOException {
Resource res = (Resource) resourcesCache.get(source);
if (res == null) {
- res = new URLResource(new URL(source));
+ res = new URLResource(new URL(source), localIfFileUrl);
resourcesCache.put(source, res);
}
return res;
@@ -138,4 +140,12 @@
return null;
}
+ public void setLocalIfFileUrl(boolean value) {
+ localIfFileUrl = value;
+ }
+
+ public boolean getLocalIfFileUrl() {
+ return localIfFileUrl;
+ }
+
}
Index: src/java/org/apache/ivy/plugins/repository/url/URLResource.java
===================================================================
--- src/java/org/apache/ivy/plugins/repository/url/URLResource.java
(revision 1543060)
+++ src/java/org/apache/ivy/plugins/repository/url/URLResource.java
(working copy)
@@ -31,6 +31,9 @@
private boolean init = false;
+ /** whether file:// URLs are local */
+ private boolean localIfFileUrl = true;
+
private long lastModified;
private long contentLength;
@@ -41,10 +44,22 @@
this.url = url;
}
+ public URLResource(URL url, boolean localIfFileUrl) {
+ this(url);
+ this.localIfFileUrl = localIfFileUrl;
+ }
+
public String getName() {
return url.toExternalForm();
}
+ public String getLocalName() {
+ if(isLocal()) {
+ return url.getPath();
+ }
+ return null;
+ }
+
public Resource clone(String cloneName) {
try {
return new URLResource(new URL(cloneName));
@@ -91,8 +106,16 @@
return getName();
}
+ public void setLocalIfFileUrl(boolean value) {
+ localIfFileUrl = value;
+ }
+
+ public boolean getLocalIfFileUrl() {
+ return localIfFileUrl;
+ }
+
public boolean isLocal() {
- return false;
+ return localIfFileUrl && "file".equals(url.getProtocol());
}
public InputStream openStream() throws IOException {
Index: src/java/org/apache/ivy/plugins/repository/LazyResource.java
===================================================================
--- src/java/org/apache/ivy/plugins/repository/LazyResource.java
(revision 1543060)
+++ src/java/org/apache/ivy/plugins/repository/LazyResource.java
(working copy)
@@ -62,6 +62,10 @@
return name;
}
+ public String getLocalName() {
+ return null;
+ }
+
public boolean isLocal() {
checkInit();
return local;
Index: src/java/org/apache/ivy/plugins/repository/BasicResource.java
===================================================================
--- src/java/org/apache/ivy/plugins/repository/BasicResource.java
(revision 1543060)
+++ src/java/org/apache/ivy/plugins/repository/BasicResource.java
(working copy)
@@ -60,6 +60,10 @@
return this.name;
}
+ public String getLocalName() {
+ return null;
+ }
+
public boolean isLocal() {
return this.local;
}
Index: src/java/org/apache/ivy/plugins/repository/sftp/SFTPResource.java
===================================================================
--- src/java/org/apache/ivy/plugins/repository/sftp/SFTPResource.java
(revision 1543060)
+++ src/java/org/apache/ivy/plugins/repository/sftp/SFTPResource.java
(working copy)
@@ -44,6 +44,10 @@
return path;
}
+ public String getLocalName() {
+ return null;
+ }
+
public Resource clone(String cloneName) {
return new SFTPResource(repository, cloneName);
}
Index: src/java/org/apache/ivy/plugins/repository/jar/JarResource.java
===================================================================
--- src/java/org/apache/ivy/plugins/repository/jar/JarResource.java
(revision 1543060)
+++ src/java/org/apache/ivy/plugins/repository/jar/JarResource.java
(working copy)
@@ -42,6 +42,10 @@
return path;
}
+ public String getLocalName() {
+ return null;
+ }
+
public long getLastModified() {
return entry.getTime();
}
Index: src/java/org/apache/ivy/plugins/repository/file/FileResource.java
===================================================================
--- src/java/org/apache/ivy/plugins/repository/file/FileResource.java
(revision 1543060)
+++ src/java/org/apache/ivy/plugins/repository/file/FileResource.java
(working copy)
@@ -38,6 +38,10 @@
return file.getPath();
}
+ public String getLocalName() {
+ return file.getPath();
+ }
+
public Resource clone(String cloneName) {
return new FileResource(repository, repository.getFile(cloneName));
}
Index: src/java/org/apache/ivy/plugins/repository/ssh/SshResource.java
===================================================================
--- src/java/org/apache/ivy/plugins/repository/ssh/SshResource.java
(revision 1543060)
+++ src/java/org/apache/ivy/plugins/repository/ssh/SshResource.java
(working copy)
@@ -115,6 +115,10 @@
return uri;
}
+ public String getLocalName() {
+ return null;
+ }
+
public String toString() {
StringBuffer buffer = new StringBuffer();
buffer.append("SshResource:");
Index: src/java/org/apache/ivy/plugins/repository/ResourceHelper.java
===================================================================
--- src/java/org/apache/ivy/plugins/repository/ResourceHelper.java
(revision 1543060)
+++ src/java/org/apache/ivy/plugins/repository/ResourceHelper.java
(working copy)
@@ -20,7 +20,6 @@
import java.io.File;
import java.net.MalformedURLException;
-import org.apache.ivy.plugins.repository.file.FileResource;
import org.apache.ivy.plugins.repository.url.URLResource;
import org.apache.ivy.util.Message;
@@ -37,9 +36,11 @@
if (res == null || f == null) {
return false;
}
- if (res instanceof FileResource) {
- return new File(res.getName()).equals(f);
- } else if (res instanceof URLResource) {
+ String localName = res.getLocalName();
+ if(localName != null) {
+ return new File(localName).equals(f);
+ }
+ if (res instanceof URLResource) {
try {
return
f.toURI().toURL().toExternalForm().equals(res.getName());
} catch (MalformedURLException e) {
Index: src/java/org/apache/ivy/plugins/repository/Resource.java
===================================================================
--- src/java/org/apache/ivy/plugins/repository/Resource.java (revision
1543060)
+++ src/java/org/apache/ivy/plugins/repository/Resource.java (working copy)
@@ -53,6 +53,13 @@
public String getName();
/**
+ * Get a local filesystem name to refer to the resource. Only valid if
isLocal() returns true.
+ *
+ * @return local filesystem name for the resource.
+ */
+ public String getLocalName();
+
+ /**
* Get the date the resource was last modified
*
* @return A <code>long</code> value representing the time the file was
last modified,
Index: src/java/org/apache/ivy/plugins/repository/vfs/VfsResource.java
===================================================================
--- src/java/org/apache/ivy/plugins/repository/vfs/VfsResource.java
(revision 1543060)
+++ src/java/org/apache/ivy/plugins/repository/vfs/VfsResource.java
(working copy)
@@ -17,8 +17,11 @@
*/
package org.apache.ivy.plugins.repository.vfs;
+import java.io.File;
import java.io.IOException;
import java.io.InputStream;
+import java.net.URI;
+import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.List;
@@ -201,6 +204,19 @@
return getName().startsWith("file:");
}
+ public String getLocalName() {
+ String name = getName();
+ if(name.startsWith("file:")) {
+ try {
+ return new File(new URI(name)).getPath();
+ } catch (URISyntaxException e) {
+ Message.warn("Unable to parse URI for local name " +
getName(), e);
+ return null;
+ }
+ }
+ return null;
+ }
+
public InputStream openStream() throws IOException {
return getContent().getInputStream();
}
Index: src/java/org/apache/ivy/plugins/resolver/packager/BuiltFileResource.java
===================================================================
--- src/java/org/apache/ivy/plugins/resolver/packager/BuiltFileResource.java
(revision 1543060)
+++ src/java/org/apache/ivy/plugins/resolver/packager/BuiltFileResource.java
(working copy)
@@ -50,6 +50,10 @@
public String getName() {
return file.toURI().toString();
}
+
+ public String getLocalName() {
+ return null;
+ }
public Resource clone(String name) {
return new BuiltFileResource(new File(name));
Index: src/java/org/apache/ivy/core/cache/DefaultRepositoryCacheManager.java
===================================================================
--- src/java/org/apache/ivy/core/cache/DefaultRepositoryCacheManager.java
(revision 1543060)
+++ src/java/org/apache/ivy/core/cache/DefaultRepositoryCacheManager.java
(working copy)
@@ -863,10 +863,11 @@
try {
ResolvedResource artifactRef =
resourceResolver.resolve(artifact);
if (artifactRef != null) {
+ String localName =
artifactRef.getResource().getLocalName();
origin = new ArtifactOrigin(
artifact,
artifactRef.getResource().isLocal(),
- artifactRef.getResource().getName());
+ localName != null ? localName :
artifactRef.getResource().getName());
if (useOrigin && artifactRef.getResource().isLocal()) {
saveArtifactOrigin(artifact, origin);
archiveFile = getArchiveFileInCache(artifact,
origin);
Index: src/java/org/apache/ivy/core/cache/ArtifactOrigin.java
===================================================================
--- src/java/org/apache/ivy/core/cache/ArtifactOrigin.java (revision
1543060)
+++ src/java/org/apache/ivy/core/cache/ArtifactOrigin.java (working copy)
@@ -17,6 +17,10 @@
*/
package org.apache.ivy.core.cache;
+import java.io.File;
+import java.net.URI;
+import java.net.URISyntaxException;
+
import org.apache.ivy.core.module.descriptor.Artifact;
import org.apache.ivy.util.Checks;
@@ -72,6 +76,16 @@
Checks.checkNotNull(location, "location");
this.artifact = artifact;
this.isLocal = isLocal;
+ if(this.isLocal) {
+ if(location.startsWith("file:")) {
+ try {
+ location = new File(new URI(location)).getPath();
+ } catch (URISyntaxException e) {
+ throw new IllegalArgumentException(e);
+ }
+ }
+ Checks.checkAbsolute(location, "");
+ }
this.location = location;
}
Index: test/java/org/apache/ivy/plugins/resolver/URLResolverTest.java
===================================================================
--- test/java/org/apache/ivy/plugins/resolver/URLResolverTest.java
(revision 1543060)
+++ test/java/org/apache/ivy/plugins/resolver/URLResolverTest.java
(working copy)
@@ -355,6 +355,6 @@
assertNotNull(ar);
assertEquals(artifact, ar.getArtifact());
- assertEquals(DownloadStatus.SUCCESSFUL, ar.getDownloadStatus());
+ assertEquals(DownloadStatus.NO, ar.getDownloadStatus());
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org
