This repository has been archived by the owner on Jan 22, 2024. It is now read-only.
Implementation of review comments for classpath and jar #196
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,354 @@ | ||
| /* | ||
| * (C) Copyright 2012, IBM Corporation | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package com.ibm.jaggr.core.impl.resource; | ||
|
|
||
| import com.ibm.jaggr.core.resource.IResource; | ||
| import com.ibm.jaggr.core.resource.IResourceVisitor; | ||
|
|
||
| import java.io.File; | ||
| import java.io.FileNotFoundException; | ||
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
| import java.io.InputStreamReader; | ||
| import java.io.Reader; | ||
| import java.net.URI; | ||
| import java.net.URL; | ||
| import java.util.ArrayList; | ||
| import java.util.logging.Level; | ||
| import java.util.logging.Logger; | ||
| import java.util.zip.ZipEntry; | ||
| import java.util.zip.ZipFile; | ||
|
|
||
| /** | ||
| * An implementation of {@link IResource} for classpath resources | ||
| */ | ||
| public class ClasspathResource implements IResource { | ||
| static final Logger log = Logger.getLogger(ClasspathResource.class.getName()); | ||
|
|
||
|
|
||
| final String zipEntryUri; | ||
| final ArrayList<String> zipFileEntries; | ||
| URL classpathUrl; | ||
| URI classpathUri; | ||
| String classpathName; | ||
| String zipFileEntry; | ||
| ZipEntry zipEntry; | ||
| ZipFile zipFile; | ||
|
|
||
|
|
||
|
|
||
| /** | ||
| * Public constructor used by factory | ||
| * | ||
| * @param entry | ||
| * the entry within the jar | ||
| * @param entries | ||
| * list of all entries within the jar | ||
| */ | ||
| public ClasspathResource(String entry, ArrayList<String> entries) { | ||
| zipEntryUri = entry; | ||
| zipFileEntries = entries; | ||
| try { | ||
| if (entry.startsWith("file:") && entry.contains("!")) { //$NON-NLS-1$ //$NON-NLS-2$ | ||
| String[] split = entry.split("!"); //$NON-NLS-1$ | ||
| classpathUrl = new URL(split[0]); | ||
| classpathUri = new URI(split[0]); | ||
| classpathName = (split[0].lastIndexOf("/") != -1) ? split[0].substring(split[0].lastIndexOf("/") + 1) : split[0]; //$NON-NLS-1$ //$NON-NLS-2$ | ||
| zipFileEntry = split[1].startsWith("/") ? split[1].substring(1) : split[1]; //$NON-NLS-1$ | ||
| // need to remove the forward slash | ||
|
|
||
| zipFile = new ZipFile(new File(classpathUri)); | ||
| zipEntry = zipFile.getEntry(zipFileEntry); | ||
| } else { | ||
| throw new ClassPathResourceException("Improper classpath resource"); //$NON-NLS-1$ | ||
| } | ||
| } catch (Exception e) { | ||
| e.printStackTrace(); | ||
| } | ||
| } | ||
|
|
||
| /* | ||
| * (non-Javadoc) | ||
| * | ||
| * @see com.ibm.jaggr.service.modules.Resource#getURI() | ||
| */ | ||
| @Override | ||
| public URI getURI() { | ||
| return URI.create("jar:" + zipEntryUri); //$NON-NLS-1$ | ||
| } | ||
|
|
||
| /* | ||
| * (non-Javadoc) | ||
| * | ||
| * @see com.ibm.jaggr.service.modules.Resource#getPath() | ||
| */ | ||
| @Override | ||
| public String getPath() { | ||
| return "classpath:///" + zipFileEntry; //$NON-NLS-1$ | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. Instead of having to override this method in the subclass, provide a protected method that returns the scheme string and override that. This will enable you to reuse more code (e.g. in resolve()). For example, getPath would be implemented as follows: There was a problem hiding this comment. Actually, I believe that getPath() shouldn't include a scheme at all. It;s supposed to return the path component of the resource URI, which is the part of the URI following the scheme and host components. |
||
| /* | ||
| * (non-Javadoc) | ||
| * | ||
| * @see com.ibm.jaggr.service.modules.Resource#exists() | ||
| */ | ||
| @Override | ||
| public boolean exists() { | ||
| return lastModified() != 0; | ||
| } | ||
|
|
||
| /* | ||
| * (non-Javadoc) | ||
| * | ||
| * @see com.ibm.jaggr.service.modules.Resource#lastModified() | ||
| */ | ||
| @Override | ||
| public long lastModified() { | ||
| long lastmod = 0L; | ||
| try { | ||
| lastmod = getURI().toURL().openConnection().getLastModified(); | ||
| } catch (IOException e) { | ||
| if (log.isLoggable(Level.WARNING)) { | ||
| log.log(Level.WARNING, e.getMessage(), e); | ||
| } | ||
| } | ||
| return lastmod; | ||
| } | ||
|
|
||
| /* (non-Javadoc) | ||
| * @see com.ibm.jaggr.service.resource.IResource#resolve(java.net.URI) | ||
| */ | ||
| @Override | ||
| public IResource resolve(String relative) { | ||
| String classpathUriString, finalclasspathString = null; | ||
| URI fileUri, finalFileUri = null; | ||
| ClasspathResource classpathResource = null; | ||
| try { | ||
| classpathUriString = getURI().toString(); | ||
| if (classpathUriString.startsWith("jar:")) { //$NON-NLS-1$ | ||
| classpathUriString = classpathUriString.substring(4); | ||
| } | ||
| fileUri = new URI(classpathUriString).resolve(relative); | ||
| finalclasspathString = "classpath:/" + fileUri; //$NON-NLS-1$ | ||
| finalFileUri = new URI(finalclasspathString); | ||
| classpathResource = newInstance(finalFileUri); | ||
| } catch (Exception e) { | ||
| if (log.isLoggable(Level.WARNING)) { | ||
| log.log(Level.WARNING, e.getMessage(), e); | ||
| } | ||
| } | ||
| return classpathResource; | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. The reason for this logic is that getURI().resolve(relative) doesnt work and throws an exception and thats because the jar uris of the type jar:/file:/.... doesnt work with resolve(). So I had to remove the jar: part and extract the uri starting with file:/ and perform the resolve operation. And then back convert to add classpath:/ in front of the uri. There was a problem hiding this comment. That's useful information. It would be good if it was in the code as comments. There was a problem hiding this comment. Does this work when relative starts with '/'. In this case, the relative path should replace any path component in the current URI, but leave the jar filename intact. Would be good to see some unit test cases for the various scenarios. There was a problem hiding this comment. Need to test this. But it doesnt impact the sample application. There was a problem hiding this comment. You are right. In case the relative starts with '/', the entire path is replaced with relative which means the jar name is not returned. I tested using a sample program which constructs a file uri like file:/C:/test.jar!/x/y.js and applying resolve("/z") returns file:/z There was a problem hiding this comment. You'll need some way to separate the jar file part of the URI from the path part. It sounds like java.net.URI can't handle this for you so you'll need to do it yourself. Basically, the steps would be as follows:
I didn't try this code, so it might need some tweaking, but you get the idea... There was a problem hiding this comment. Already implemented this review comment |
||
| /* | ||
| * (non-Javadoc) | ||
| * | ||
| * @see com.ibm.jaggr.service.modules.Resource#getReader() | ||
| */ | ||
| @Override | ||
| public Reader getReader() throws IOException { | ||
| return new InputStreamReader(getURI().toURL().openConnection().getInputStream(), "UTF-8"); //$NON-NLS-1$ | ||
| } | ||
|
|
||
| /* (non-Javadoc) | ||
| * @see com.ibm.jaggr.service.resource.IResource#getInputStream() | ||
| */ | ||
| @Override | ||
| public InputStream getInputStream() throws IOException { | ||
| return getURI().toURL().openConnection().getInputStream(); | ||
| } | ||
|
|
||
| /* | ||
| * (non-Javadoc) | ||
| * | ||
| * @see | ||
| * com.ibm.jaggr.service.modules.Resource#walkTree(com.ibm.jaggr.modules | ||
| * .ResourceVisitor, boolean) | ||
| */ | ||
| @Override | ||
| public void walkTree(IResourceVisitor visitor) throws IOException { | ||
| if (!exists()) { | ||
| throw new FileNotFoundException(zipEntry.getName()); | ||
| } | ||
| if (!zipEntry.isDirectory()) { | ||
| return; | ||
| } | ||
| recurse(zipEntry, visitor); | ||
| } | ||
|
|
||
| /* | ||
| * (non-Javadoc) | ||
| * | ||
| * @see | ||
| * com.ibm.jaggr.service.resource.IResource#asVisitorResource() | ||
| */ | ||
| @Override | ||
| public VisitorResource asVisitorResource() throws IOException { | ||
| if (!exists()) { | ||
| throw new IOException(zipEntry.getName()); | ||
| } | ||
| return new VisitorResource(zipEntry, zipEntryUri); | ||
| } | ||
|
|
||
| /** | ||
| * Internal method for recursing sub directories. | ||
| * | ||
| * @param file | ||
| * The file object | ||
| * @param visitor | ||
| * The {@link IResourceVisitor} to call for non-folder resources | ||
| * @throws IOException | ||
| */ | ||
| private void recurse(ZipEntry file, IResourceVisitor visitor) | ||
| throws IOException { | ||
| ArrayList<ZipEntry> files = getChildZipEntries(file); | ||
|
|
||
| if (files == null) { | ||
| return; | ||
| } | ||
| for (ZipEntry child : files) { | ||
| String childName = child.getName().substring(file.getName().length()); | ||
| String childEntryUri = zipEntryUri + childName; | ||
| visitor.visitResource(new VisitorResource(child, | ||
| childEntryUri), childName); | ||
| } | ||
| } | ||
|
|
||
| protected ClasspathResource newInstance(URI uri) { | ||
| return (ClasspathResource) new ClasspathResourceFactory().newResource(uri); | ||
| } | ||
|
|
||
| private ArrayList<ZipEntry> getChildZipEntries(ZipEntry entry){ | ||
| String entryName = entry.getName(); | ||
| ArrayList<ZipEntry> files = new ArrayList<ZipEntry>(); | ||
| for (int i = 0; i < zipFileEntries.size(); i ++){ | ||
| if(zipFileEntries.get(i).startsWith(entryName) && !(zipFileEntries.get(i).equalsIgnoreCase(entryName)) && zipFileEntries.get(i).endsWith(".js")){ //$NON-NLS-1$ | ||
| files.add(zipFile.getEntry(zipFileEntries.get(i))); | ||
| } | ||
| } | ||
|
|
||
| return files; | ||
| } | ||
|
|
||
| /** | ||
| * Implementation of {@link IResourceVisitor.Resource} for classpath resources. | ||
| */ | ||
| private static class VisitorResource implements IResourceVisitor.Resource { | ||
|
|
||
| ZipEntry entry; | ||
| String ZipFileUri; | ||
|
|
||
|
|
||
| private VisitorResource(ZipEntry entry, String uri) { | ||
| this.entry = entry; | ||
| this.ZipFileUri = uri; | ||
|
|
||
| } | ||
|
|
||
| /* | ||
| * (non-Javadoc) | ||
| * | ||
| * @see | ||
| * com.ibm.jaggr.service.resource.IResourceVisitor.Resource | ||
| * #isFolder() | ||
| */ | ||
| @Override | ||
| public boolean isFolder() { | ||
| if (entry == null) { | ||
| return false; | ||
| } else { | ||
| return entry.isDirectory(); | ||
| } | ||
| } | ||
|
|
||
| /* | ||
| * (non-Javadoc) | ||
| * | ||
| * @see | ||
| * com.ibm.jaggr.service.modules.ResourceVisitor.Resource# | ||
| * getURI() | ||
| */ | ||
| @Override | ||
| public URI getURI() { | ||
| return URI.create("jar:" + ZipFileUri); //$NON-NLS-1$ | ||
| } | ||
|
|
||
| /* | ||
| * (non-Javadoc) | ||
| * | ||
| * @see | ||
| * com.ibm.jaggr.service.modules.ResourceVisitor.Resource# | ||
| * getPath() | ||
| */ | ||
| @Override | ||
| public String getPath() { | ||
| return ZipFileUri; | ||
| } | ||
|
|
||
| /* | ||
| * (non-Javadoc) | ||
| * | ||
| * @see | ||
| * com.ibm.jaggr.service.modules.ResourceVisitor.Resource# | ||
| * lastModified() | ||
| */ | ||
| @Override | ||
| public long lastModified() { | ||
| long lastmodified = 0; | ||
| try { | ||
| lastmodified = getURI().toURL().openConnection().getLastModified(); | ||
| } catch (IOException e) { | ||
| if (log.isLoggable(Level.WARNING)) { | ||
| log.log(Level.WARNING, e.getMessage(), e); | ||
| } | ||
| } | ||
| return lastmodified; | ||
| } | ||
|
|
||
| /* | ||
| * (non-Javadoc) | ||
| * | ||
| * @see | ||
| * com.ibm.jaggr.service.modules.ResourceVisitor.Resource# | ||
| * getReader() | ||
| */ | ||
| @Override | ||
| public Reader getReader() throws IOException { | ||
| return new InputStreamReader(getURI().toURL().openConnection().getInputStream(), "UTF-8"); //$NON-NLS-1$ | ||
| } | ||
|
|
||
| /* (non-Javadoc) | ||
| * @see com.ibm.jaggr.service.resource.IResourceVisitor.Resource#getInputStream() | ||
| */ | ||
| @Override | ||
| public InputStream getInputStream() throws IOException { | ||
| return getURI().toURL().openConnection().getInputStream(); | ||
| } | ||
| } | ||
|
|
||
| public class ClassPathResourceException extends Exception { | ||
|
|
||
| public ClassPathResourceException(String string) { | ||
| super(string); | ||
| } | ||
|
|
||
| private static final long serialVersionUID = 1L; | ||
|
|
||
| } | ||
|
|
||
|
|
||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still doesn't look right. If zipFileEntry specifies a jar file name (e.g. jarfile!/pathComponent), then getPath() needs to return only the part after the !/ separator. The jar file name is not part of the path, just like a host name is not part of an HTTP resource path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable zipFileEntry is initialized in the ctor of ClasspathResource in such a way that it is actually the path after the jar name. So we are good here.