FileResource and ClassPathEntry cleanup.
- Changes interning of FileResource to be thread-safe.
- Renames FileResource.create to FileResource.of as it
doesn't always create.
- Drops the test-only needed getClassPathEntry from all
Resource types.
- Removes unused findApplicableResources overload from all
ClassPathEntry types.
Change-Id: I0d588dde8ae9220221c5f2255d01aa1feaba2c7b
Review-Link: https://gwt-review.googlesource.com/#/c/10830/
diff --git a/dev/core/src/com/google/gwt/dev/jjs/LibraryJavaToJavaScriptCompiler.java b/dev/core/src/com/google/gwt/dev/jjs/LibraryJavaToJavaScriptCompiler.java
index 25f560b..561f759 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/LibraryJavaToJavaScriptCompiler.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/LibraryJavaToJavaScriptCompiler.java
@@ -277,7 +277,7 @@
String resourcePath = entry.getKey();
File resourceFile = entry.getValue();
compilerContext.getLibraryWriter()
- .addBuildResource(FileResource.create(null, resourcePath, resourceFile));
+ .addBuildResource(FileResource.of(resourcePath, resourceFile));
}
}
diff --git a/dev/core/src/com/google/gwt/dev/resource/impl/AbstractResource.java b/dev/core/src/com/google/gwt/dev/resource/impl/AbstractResource.java
index f0f9308..fe6a1e8 100644
--- a/dev/core/src/com/google/gwt/dev/resource/impl/AbstractResource.java
+++ b/dev/core/src/com/google/gwt/dev/resource/impl/AbstractResource.java
@@ -22,11 +22,6 @@
*/
public abstract class AbstractResource extends Resource {
- /**
- * Accesses the path root under which this resource was found. Only available within this package.
- */
- public abstract ClassPathEntry getClassPathEntry();
-
@Override
public boolean wasRerooted() {
return false;
diff --git a/dev/core/src/com/google/gwt/dev/resource/impl/ClassPathEntry.java b/dev/core/src/com/google/gwt/dev/resource/impl/ClassPathEntry.java
index c7655e9..83edd86 100644
--- a/dev/core/src/com/google/gwt/dev/resource/impl/ClassPathEntry.java
+++ b/dev/core/src/com/google/gwt/dev/resource/impl/ClassPathEntry.java
@@ -17,8 +17,6 @@
import com.google.gwt.core.ext.TreeLogger;
-import java.util.ArrayList;
-import java.util.List;
import java.util.Map;
/**
@@ -41,22 +39,6 @@
}
/**
- * Finds applicable resources for a list of pathPrefixSets, returning a
- * distinct answer for each set.
- *
- * @see #findApplicableResources(TreeLogger, PathPrefixSet)
- */
- public List<Map<AbstractResource, ResourceResolution>> findApplicableResources(
- TreeLogger logger, List<PathPrefixSet> pathPrefixSets) {
- List<Map<AbstractResource, ResourceResolution>> results = new ArrayList<
- Map<AbstractResource, ResourceResolution>>(pathPrefixSets.size());
- for (PathPrefixSet pathPrefixSet : pathPrefixSets) {
- results.add(findApplicableResources(logger, pathPrefixSet));
- }
- return results;
- }
-
- /**
* Finds every resource at abstract path P within this classpath such that P
* begins with a prefix X from the path prefix set and P is allowed by the
* filter associated with X.
diff --git a/dev/core/src/com/google/gwt/dev/resource/impl/DirectoryClassPathEntry.java b/dev/core/src/com/google/gwt/dev/resource/impl/DirectoryClassPathEntry.java
index 1bc2a34..e496f21 100644
--- a/dev/core/src/com/google/gwt/dev/resource/impl/DirectoryClassPathEntry.java
+++ b/dev/core/src/com/google/gwt/dev/resource/impl/DirectoryClassPathEntry.java
@@ -24,7 +24,6 @@
import java.io.File;
import java.io.IOException;
-import java.util.ArrayList;
import java.util.Collection;
import java.util.IdentityHashMap;
import java.util.List;
@@ -81,18 +80,6 @@
}
@Override
- public List<Map<AbstractResource, ResourceResolution>> findApplicableResources(
- TreeLogger logger, List<PathPrefixSet> pathPrefixSets) {
- List<Map<AbstractResource, ResourceResolution>> results =
- new ArrayList<Map<AbstractResource, ResourceResolution>>(pathPrefixSets.size());
- for (PathPrefixSet pathPrefixSet : pathPrefixSets) {
- results.add(new IdentityHashMap<AbstractResource, ResourceResolution>());
- }
- descendToFindResources(logger, pathPrefixSets, results, dir, "");
- return results;
- }
-
- @Override
public Map<AbstractResource, ResourceResolution> findApplicableResources(TreeLogger logger,
PathPrefixSet pathPrefixSet) {
if (!WATCH_FILE_CHANGES) {
@@ -150,7 +137,7 @@
DirectoryPathPrefixChangeManager.getAndClearChangedFiles(this, pathPrefixSet);
for (File changedFile : changedFiles) {
String changedRelativePath = Util.makeRelativePath(dir, changedFile);
- FileResource resource = FileResource.create(this, changedRelativePath, changedFile);
+ FileResource resource = FileResource.of(changedRelativePath, changedFile);
if (!changedFile.exists()) {
if (resolutionsByResource.containsKey(resource)) {
@@ -229,7 +216,7 @@
ResourceResolution resourceResolution = null;
if ((resourceResolution = pathPrefixSets.get(i).includesResource(childPath)) != null) {
Messages.INCLUDING_FILE.log(logger, childPath, null);
- FileResource r = FileResource.create(this, childPath, child);
+ FileResource r = FileResource.of(childPath, child);
results.get(i).put(r, resourceResolution);
} else {
Messages.EXCLUDING_FILE.log(logger, childPath, null);
diff --git a/dev/core/src/com/google/gwt/dev/resource/impl/FileResource.java b/dev/core/src/com/google/gwt/dev/resource/impl/FileResource.java
index 9ca55b3..46174e3 100644
--- a/dev/core/src/com/google/gwt/dev/resource/impl/FileResource.java
+++ b/dev/core/src/com/google/gwt/dev/resource/impl/FileResource.java
@@ -15,58 +15,44 @@
*/
package com.google.gwt.dev.resource.impl;
-import com.google.gwt.thirdparty.guava.common.collect.Maps;
+import com.google.gwt.thirdparty.guava.common.collect.MapMaker;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
-import java.lang.ref.WeakReference;
-import java.util.Map;
+import java.util.concurrent.ConcurrentMap;
/**
* Represents a resource contained in a directory on a file system.
- * <p>
- * The class is immutable and automatically interned.
+ * <p>The class is immutable and automatically interned.
*/
public class FileResource extends AbstractResource {
+ /*
+ * The #equals and #hashCode is locked to work on identities by the base class Resource.
+ * Without interning, it would be impractical to use this class as a key in a map.
+ */
- private static Map<String, FileResource> fileResourceByKey = Maps.newHashMap();
+ private static final ConcurrentMap<String, FileResource> canonicalFileResources =
+ new MapMaker().weakValues().makeMap();
- public static FileResource create(DirectoryClassPathEntry classPathEntry, String abstractPathName,
- File file) {
- // Intern the file resource.
- String classPathLocation = classPathEntry != null ? classPathEntry.getLocation() : "null";
- String key = classPathLocation + File.pathSeparator + abstractPathName + File.pathSeparator
- + file.getAbsolutePath();
- FileResource fileResource = fileResourceByKey.get(key);
- if (fileResource == null) {
- fileResource = new FileResource(classPathEntry, abstractPathName, file);
- fileResourceByKey.put(key, fileResource);
- }
-
- return fileResource;
+ public static FileResource of(String abstractPathName, File file) {
+ String key = abstractPathName + "@" + file.getAbsolutePath();
+ FileResource sample = new FileResource(abstractPathName, file);
+ FileResource canonical = canonicalFileResources.putIfAbsent(key, sample);
+ return (canonical == null) ? sample : canonical;
}
private final String abstractPathName;
- private final WeakReference<DirectoryClassPathEntry> classPathEntryReference;
private final File file;
- private FileResource(DirectoryClassPathEntry classPathEntry, String abstractPathName, File file) {
+ private FileResource(String abstractPathName, File file) {
assert (file.isFile()) : file + " is not a file.";
- this.classPathEntryReference = new WeakReference<DirectoryClassPathEntry>(classPathEntry);
this.abstractPathName = abstractPathName;
this.file = file;
}
@Override
- public DirectoryClassPathEntry getClassPathEntry() {
- DirectoryClassPathEntry classPathEntry = classPathEntryReference.get();
- assert classPathEntry != null;
- return classPathEntry;
- }
-
- @Override
public long getLastModified() {
return file.lastModified();
}
diff --git a/dev/core/src/com/google/gwt/dev/resource/impl/ResourceOracleImpl.java b/dev/core/src/com/google/gwt/dev/resource/impl/ResourceOracleImpl.java
index 6b0f5d2..3cd1026 100644
--- a/dev/core/src/com/google/gwt/dev/resource/impl/ResourceOracleImpl.java
+++ b/dev/core/src/com/google/gwt/dev/resource/impl/ResourceOracleImpl.java
@@ -78,11 +78,6 @@
}
@Override
- public ClassPathEntry getClassPathEntry() {
- return resource.getClassPathEntry();
- }
-
- @Override
public long getLastModified() {
return resource.getLastModified();
}
diff --git a/dev/core/src/com/google/gwt/dev/resource/impl/UrlResource.java b/dev/core/src/com/google/gwt/dev/resource/impl/UrlResource.java
index d4608e5..7dd5642 100644
--- a/dev/core/src/com/google/gwt/dev/resource/impl/UrlResource.java
+++ b/dev/core/src/com/google/gwt/dev/resource/impl/UrlResource.java
@@ -36,11 +36,6 @@
}
@Override
- public ClassPathEntry getClassPathEntry() {
- throw new UnsupportedOperationException("UrlResources do not come from the class path.");
- }
-
- @Override
public long getLastModified() {
return lastModified;
}
diff --git a/dev/core/src/com/google/gwt/dev/resource/impl/ZipFileResource.java b/dev/core/src/com/google/gwt/dev/resource/impl/ZipFileResource.java
index f96186a..07ea34b 100644
--- a/dev/core/src/com/google/gwt/dev/resource/impl/ZipFileResource.java
+++ b/dev/core/src/com/google/gwt/dev/resource/impl/ZipFileResource.java
@@ -63,11 +63,6 @@
}
@Override
- public ZipFileClassPathEntry getClassPathEntry() {
- return classPathEntry;
- }
-
- @Override
public long getLastModified() {
return lastModified;
}
diff --git a/dev/core/test/com/google/gwt/dev/resource/impl/FileResourceTest.java b/dev/core/test/com/google/gwt/dev/resource/impl/FileResourceTest.java
index 4a48426..f74b744 100644
--- a/dev/core/test/com/google/gwt/dev/resource/impl/FileResourceTest.java
+++ b/dev/core/test/com/google/gwt/dev/resource/impl/FileResourceTest.java
@@ -21,7 +21,6 @@
import java.io.File;
import java.io.IOException;
-import java.io.InputStream;
/**
* Tests related FileResources.
@@ -29,19 +28,9 @@
public class FileResourceTest extends TestCase {
public void testBasic() {
- File f = null;
- try {
- f = File.createTempFile("com.google.gwt.dev.javac.impl.FileResourceTest",
- ".tmp");
- f.deleteOnExit();
- Util.writeStringAsFile(f, "contents 1");
- } catch (IOException e) {
- fail("Failed to create test file");
- }
+ File f = createTempFile();
- File dir = f.getParentFile();
- DirectoryClassPathEntry cpe = new DirectoryClassPathEntry(dir);
- FileResource r = FileResource.create(cpe, f.getName(), f);
+ FileResource r = FileResource.of(f.getName(), f);
assertEquals(f.getAbsoluteFile().toURI().toString(), r.getLocation());
/*
@@ -52,19 +41,9 @@
}
public void testDeletion() {
- File f = null;
- try {
- f = File.createTempFile("com.google.gwt.dev.javac.impl.FileResourceTest",
- ".tmp");
- f.deleteOnExit();
- Util.writeStringAsFile(f, "contents 1");
- } catch (IOException e) {
- fail("Failed to create test file");
- }
+ File f = createTempFile();
- File dir = f.getParentFile();
- DirectoryClassPathEntry cpe = new DirectoryClassPathEntry(dir);
- FileResource r = FileResource.create(cpe, f.getName(), f);
+ FileResource r = FileResource.of(f.getName(), f);
assertEquals(f.getAbsoluteFile().toURI().toString(), r.getLocation());
/*
@@ -80,11 +59,31 @@
* The resource is no longer available. Check to make sure we can't access its contents
* through the API.
*/
- InputStream in = null;
try {
- in = r.openContents();
+ r.openContents();
fail("Open contents unexpectedly succeeded.");
} catch (IOException expected) {
}
}
+
+ public void testInterning() throws Exception {
+ File f = createTempFile();
+
+ assertSame(FileResource.of(f.getName(), f), FileResource.of(f.getName(), f));
+ // A different FileResourse will return even files match if the pathname is different.
+ assertNotSame(FileResource.of(f.getName(), f), FileResource.of(f.getName() + "x", f));
+ }
+
+ private File createTempFile() {
+ File f = null;
+ try {
+ f = File.createTempFile("com.google.gwt.dev.javac.impl.FileResourceTest", ".tmp");
+ f.deleteOnExit();
+ Util.writeStringAsFile(f, "contents 1");
+ } catch (IOException e) {
+ fail("Failed to create test file");
+ }
+ return f;
+ }
+
}
diff --git a/dev/core/test/com/google/gwt/dev/resource/impl/MockAbstractResource.java b/dev/core/test/com/google/gwt/dev/resource/impl/MockAbstractResource.java
index 6c8c756..fbe9235 100644
--- a/dev/core/test/com/google/gwt/dev/resource/impl/MockAbstractResource.java
+++ b/dev/core/test/com/google/gwt/dev/resource/impl/MockAbstractResource.java
@@ -33,11 +33,6 @@
}
@Override
- public ClassPathEntry getClassPathEntry() {
- return mockClassPathEntry;
- }
-
- @Override
public long getLastModified() {
return 0;
}
diff --git a/dev/core/test/com/google/gwt/dev/resource/impl/ResourceOracleImplTest.java b/dev/core/test/com/google/gwt/dev/resource/impl/ResourceOracleImplTest.java
index 0691588..c19ea92 100644
--- a/dev/core/test/com/google/gwt/dev/resource/impl/ResourceOracleImplTest.java
+++ b/dev/core/test/com/google/gwt/dev/resource/impl/ResourceOracleImplTest.java
@@ -111,8 +111,11 @@
public void assertPathIncluded(String expectedPath, ClassPathEntry expectedCpe) {
AbstractResource resource = findResourceWithPath(expectedPath);
assertNotNull(resource);
- ClassPathEntry actualCpe = resource.getClassPathEntry();
- assertEquals(expectedCpe.getLocation(), actualCpe.getLocation());
+ if (expectedCpe.getLocation().endsWith(".jar")) {
+ assertTrue(resource.getLocation().startsWith("jar:" + expectedCpe.getLocation()));
+ } else {
+ assertTrue(resource.getLocation().startsWith(expectedCpe.getLocation()));
+ }
}
public void assertPathNotIncluded(String path) {
