From 598cc24a761a7e46a0fd591d288e6d6443bac4cd Mon Sep 17 00:00:00 2001 From: Andrew Swan Date: Thu, 27 Oct 2011 11:41:33 +1100 Subject: [PATCH] ROO-2745: Post 1.2.0.M1 code refactor and clean up - moved and renamed some File utility methods, fixed potential NPE in FileDetails#hashCode, wrote unit tests, simplified Pom.java --- .../addon/backup/BackupOperationsImpl.java | 4 +- .../classpath/TypeLocationServiceImpl.java | 14 +-- .../polling/PollingFileMonitorService.java | 5 +- .../roo/file/monitor/event/FileDetails.java | 101 ++++++++-------- .../file/monitor/event/FileDetailsTest.java | 31 +++++ .../manager/internal/DefaultFileManager.java | 3 +- .../manager/internal/DefaultMutableFile.java | 4 +- .../project/DefaultPathResolvingStrategy.java | 8 +- .../project/MavenPathResolvingStrategy.java | 97 +++++++++------- .../roo/project/PathInformation.java | 4 +- .../PathResolvingAwareFilenameResolver.java | 4 +- .../roo/project/PomManagementServiceImpl.java | 9 +- .../roo/project/maven/Pom.java | 21 ++-- .../MavenPathResolvingStrategyTest.java | 54 +++++++++ .../roo/support/util/FileUtils.java | 88 ++++++++++---- .../roo/support/util/FileUtilsTest.java | 109 ++++++++++++++++++ 16 files changed, 405 insertions(+), 151 deletions(-) create mode 100644 file-monitor/src/test/java/org/springframework/roo/file/monitor/event/FileDetailsTest.java create mode 100644 project/src/test/java/org/springframework/roo/project/MavenPathResolvingStrategyTest.java diff --git a/addon-backup/src/main/java/org/springframework/roo/addon/backup/BackupOperationsImpl.java b/addon-backup/src/main/java/org/springframework/roo/addon/backup/BackupOperationsImpl.java index 82bb0057d3..ead5228aaf 100644 --- a/addon-backup/src/main/java/org/springframework/roo/addon/backup/BackupOperationsImpl.java +++ b/addon-backup/src/main/java/org/springframework/roo/addon/backup/BackupOperationsImpl.java @@ -17,13 +17,13 @@ import org.apache.felix.scr.annotations.Component; import org.apache.felix.scr.annotations.Reference; import org.apache.felix.scr.annotations.Service; -import org.springframework.roo.file.monitor.event.FileDetails; import org.springframework.roo.process.manager.FileManager; import org.springframework.roo.process.manager.MutableFile; import org.springframework.roo.project.Path; import org.springframework.roo.project.ProjectOperations; import org.springframework.roo.support.logging.HandlerUtils; import org.springframework.roo.support.util.Assert; +import org.springframework.roo.support.util.FileUtils; import org.springframework.roo.support.util.IOUtils; /** @@ -64,7 +64,7 @@ public String backup() { ZipOutputStream zos = null; try { File projectDirectory = new File(projectOperations.getPathResolver().getFocusedIdentifier(Path.ROOT, ".")); - MutableFile file = fileManager.createFile(FileDetails.getCanonicalPath(new File(projectDirectory, projectOperations.getFocusedProjectName() + "_" + df.format(new Date()) + ".zip"))); + MutableFile file = fileManager.createFile(FileUtils.getCanonicalPath(new File(projectDirectory, projectOperations.getFocusedProjectName() + "_" + df.format(new Date()) + ".zip"))); zos = new ZipOutputStream(file.getOutputStream()); zip(projectDirectory, projectDirectory, zos); } catch (FileNotFoundException e) { diff --git a/classpath/src/main/java/org/springframework/roo/classpath/TypeLocationServiceImpl.java b/classpath/src/main/java/org/springframework/roo/classpath/TypeLocationServiceImpl.java index c260f0a503..60171b34a1 100644 --- a/classpath/src/main/java/org/springframework/roo/classpath/TypeLocationServiceImpl.java +++ b/classpath/src/main/java/org/springframework/roo/classpath/TypeLocationServiceImpl.java @@ -113,8 +113,8 @@ private String getProposedJavaType(final String fileCanonicalPath) { // Determine the JavaType for this file String relativePath = ""; for (PathInformation pathInformation : pomManagementService.getModuleForFileIdentifier(fileCanonicalPath).getPathInformation()) { - if (fileCanonicalPath.startsWith(FileUtils.normalise(FileDetails.getCanonicalPath(pathInformation.getLocation())))) { - relativePath = File.separator + fileCanonicalPath.replaceFirst(FileUtils.normalise(FileDetails.getCanonicalPath(pathInformation.getLocation())), ""); + if (fileCanonicalPath.startsWith(FileUtils.ensureTrailingSeparator(FileUtils.getCanonicalPath(pathInformation.getLocation())))) { + relativePath = File.separator + fileCanonicalPath.replaceFirst(FileUtils.ensureTrailingSeparator(FileUtils.getCanonicalPath(pathInformation.getLocation())), ""); break; } } @@ -388,12 +388,12 @@ public String getPhysicalTypeIdentifier(final JavaType type) { if (typePath == null) { return null; } - String reducedPath = FileUtils.normalise(typePath.replaceAll(typeRelativePath, "")); + String reducedPath = FileUtils.ensureTrailingSeparator(typePath.replaceAll(typeRelativePath, "")); String mid = null; for (Pom pom : pomManagementService.getPomMap().values()) { for (Path path : Arrays.asList(Path.SRC_MAIN_JAVA, Path.SRC_TEST_JAVA)) { PathInformation pathInformation = pom.getPathInformation(path); - String pathLocation = FileUtils.normalise(pathInformation.getLocationPath()); + String pathLocation = FileUtils.ensureTrailingSeparator(pathInformation.getLocationPath()); if (pathLocation.startsWith(reducedPath)) { mid = PhysicalTypeIdentifier.createIdentifier(type, pathInformation.getContextualPath()); projectOperations.addModuleDependency(pathInformation.getContextualPath().getModule()); @@ -420,11 +420,11 @@ public String getPhysicalTypeIdentifier(final JavaType type, final ContextualPat return null; //throw new IllegalStateException("The source for '" + type.getFullyQualifiedTypeName() + "' could not be resolved"); } - String reducedPath = FileUtils.normalise(typeFilePath.replaceAll(typeRelativePath, "")); + String reducedPath = FileUtils.ensureTrailingSeparator(typeFilePath.replaceAll(typeRelativePath, "")); String mid = null; for (Pom pom : pomManagementService.getPomMap().values()) { PathInformation pathInformation = pom.getPathInformation(path.getPath()); - String pathLocation = FileUtils.normalise(pathInformation.getLocationPath()); + String pathLocation = FileUtils.ensureTrailingSeparator(pathInformation.getLocationPath()); if (pathLocation.startsWith(reducedPath)) { mid = PhysicalTypeIdentifier.createIdentifier(type, pathInformation.getContextualPath()); break; @@ -466,7 +466,7 @@ public boolean hasTypeChanged(final String requestingClass, final JavaType javaT private void initTypeMap() { for (Pom pom : pomManagementService.getPomMap().values()) { for (Path path : Arrays.asList(Path.SRC_MAIN_JAVA, Path.SRC_TEST_JAVA)) { - String pathToResolve = FileUtils.normalise(pom.getPathInformation(path).getLocationPath()) + "**" + File.separatorChar + "*.java"; + String pathToResolve = FileUtils.ensureTrailingSeparator(pom.getPathInformation(path).getLocationPath()) + "**" + File.separatorChar + "*.java"; for (FileDetails file : fileManager.findMatchingAntPath(pathToResolve)) { cacheType(file.getCanonicalPath()); } diff --git a/file-monitor-polling/src/main/java/org/springframework/roo/file/monitor/polling/PollingFileMonitorService.java b/file-monitor-polling/src/main/java/org/springframework/roo/file/monitor/polling/PollingFileMonitorService.java index e86c634d0a..c653012921 100644 --- a/file-monitor-polling/src/main/java/org/springframework/roo/file/monitor/polling/PollingFileMonitorService.java +++ b/file-monitor-polling/src/main/java/org/springframework/roo/file/monitor/polling/PollingFileMonitorService.java @@ -23,6 +23,7 @@ import org.springframework.roo.file.monitor.event.FileEventListener; import org.springframework.roo.file.monitor.event.FileOperation; import org.springframework.roo.support.util.Assert; +import org.springframework.roo.support.util.FileUtils; /** * A simple polling-based {@link FileMonitorService}. @@ -127,7 +128,7 @@ private boolean isWithin(final MonitoringRequest request, final String filePath) return false; // Not within this directory or a sub-directory } } else { - if (!FileDetails.matchesAntPath(requestCanonicalPath + File.separator + "*", filePath)) { + if (!FileUtils.matchesAntPath(requestCanonicalPath + File.separator + "*", filePath)) { return false; // Not within this directory } } @@ -495,7 +496,7 @@ private void recursiveAntMatch(final String antPath, final File currentDirectory } for (File f : listFiles) { try { - if (FileDetails.matchesAntPath(antPath, f.getCanonicalPath())) { + if (FileUtils.matchesAntPath(antPath, f.getCanonicalPath())) { result.add(new FileDetails(f, f.lastModified())); } } catch (IOException ignored) {} diff --git a/file-monitor/src/main/java/org/springframework/roo/file/monitor/event/FileDetails.java b/file-monitor/src/main/java/org/springframework/roo/file/monitor/event/FileDetails.java index 3d22f22b86..2eb72dbd9e 100644 --- a/file-monitor/src/main/java/org/springframework/roo/file/monitor/event/FileDetails.java +++ b/file-monitor/src/main/java/org/springframework/roo/file/monitor/event/FileDetails.java @@ -1,37 +1,59 @@ package org.springframework.roo.file.monitor.event; import java.io.File; -import java.io.IOException; import java.util.Date; import org.springframework.roo.file.monitor.FileMonitorService; -import org.springframework.roo.support.ant.AntPathMatcher; -import org.springframework.roo.support.ant.PathMatcher; import org.springframework.roo.support.style.ToStringCreator; import org.springframework.roo.support.util.Assert; import org.springframework.roo.support.util.FileUtils; +import org.springframework.roo.support.util.ObjectUtils; /** - * Represents the details of a file that once existed on the disk. - * + * The details of a file that once existed on the disk. *

* Instances of this class are usually included within a {@link FileEvent} object. * * @author Ben Alex * @since 1.0 - * */ public class FileDetails implements Comparable { - private final File file; - private final Long lastModified; - private static final PathMatcher pathMatcher; + /** + * Returns the canonical path of the given {@link File}. + * + * @param file the file for which to find the canonical path (required) + * @return the canonical path + * @deprecated use {@link FileUtils#getCanonicalPath(File)} instead + */ + @Deprecated + public static String getCanonicalPath(final File file) { + return FileUtils.getCanonicalPath(file); + } - static { - pathMatcher = new AntPathMatcher(); - ((AntPathMatcher) pathMatcher).setPathSeparator(File.separator); + /** + * Indicates whether the given canonical path matches the given Ant-style pattern + * + * @param antPattern the pattern to check against (can't be blank) + * @param canonicalPath the path to check (can't be blank) + * @return see above + * @deprecated use {@link FileUtils#matchesAntPath(String, String)} instead + */ + @Deprecated + public static boolean matchesAntPath(final String antPattern, final String canonicalPath) { + return FileUtils.matchesAntPath(antPattern, canonicalPath); } + // Fields + private final Long lastModified; + private final File file; + + /** + * Constructor + * + * @param file the file for which these are the details (required) + * @param lastModified the system clock in milliseconds when this file was last modified (can be null) + */ public FileDetails(final File file, final Long lastModified) { Assert.notNull(file, "File required"); this.file = file; @@ -40,21 +62,22 @@ public FileDetails(final File file, final Long lastModified) { @Override public int hashCode() { - return 7 * this.file.hashCode() * this.lastModified.hashCode(); + return 7 * this.file.hashCode() * ObjectUtils.nullSafeHashCode(lastModified); } @Override public boolean equals(final Object obj) { - return obj instanceof FileDetails && this.compareTo((FileDetails)obj) == 0; + return obj instanceof FileDetails && this.compareTo((FileDetails) obj) == 0; } public int compareTo(final FileDetails o) { if (o == null) { throw new NullPointerException(); } - int result = o.file.compareTo(file); + // N.B. this is in reverse order to how we'd normally compare + int result = o.getFile().compareTo(this.file); if (result == 0) { - result = o.lastModified.compareTo(lastModified); + result = ObjectUtils.nullSafeComparison(o.getLastModified(), this.lastModified); } return result; } @@ -67,28 +90,9 @@ public int compareTo(final FileDetails o) { * @return the canonical path. */ public String getCanonicalPath() { - try { - return file.getCanonicalPath(); - } catch (IOException ioe) { - throw new IllegalStateException("Cannot determine canonical path for '" + file + "'", ioe); - } + return FileUtils.getCanonicalPath(file); } - - /** - * Static convenience method to allow the acquisition of a canonical file path for a {@link File}. - * - * @param file the File to find the canonical path for. - * @return the canonical path. - */ - public static String getCanonicalPath(final File file) { - Assert.notNull(file, "File required"); - try { - return file.getCanonicalPath(); - } catch (IOException ioe) { - throw new IllegalStateException("Cannot determine canonical path for '" + file + "'", ioe); - } - } - + /** * Indicates whether the presented canonical path is a child of the current * {@link FileDetails} instance. Put differently, returning true indicates the @@ -103,28 +107,20 @@ public static String getCanonicalPath(final File file) { */ public boolean isParentOf(final String possibleChildCanonicalPath) { Assert.hasText(possibleChildCanonicalPath, "Possible child to evaluate is required"); - return FileUtils.normalise(possibleChildCanonicalPath).startsWith(FileUtils.normalise(getCanonicalPath())); + return FileUtils.ensureTrailingSeparator(possibleChildCanonicalPath).startsWith(FileUtils.ensureTrailingSeparator(getCanonicalPath())); } /** - * Determines whether the presented Ant path matches this {@link FileDetails} canonical path. - * + * Indicates whether this file's canonical path matches the given Ant-style pattern. *

- * The presented path must be in Ant syntax. It should include a full prefix that is - * consistent with the {@link #getCanonicalPath()} method. + * The presented path must be in Ant syntax. It should include a full prefix + * that is consistent with the {@link #getCanonicalPath()} method. * - * @param antPath to evaluate (required and cannot be empty) + * @param antPattern the pattern to check this file against (cannot be blank) * @return whether the path matches or not */ - public boolean matchesAntPath(final String antPath) { - Assert.hasText(antPath, "Ant path to match required"); - return matchesAntPath(antPath, getCanonicalPath()); - } - - public static boolean matchesAntPath(final String antPath, final String canonicalPath) { - Assert.hasText(antPath, "Ant path to match required"); - Assert.hasText(canonicalPath, "Canonical path to match required"); - return pathMatcher.match(antPath, canonicalPath); + public boolean matchesAntPath(final String antPattern) { + return FileUtils.matchesAntPath(antPattern, getCanonicalPath()); } /** @@ -171,5 +167,4 @@ public String toString() { tsc.append("lastModified", lastModified == null ? "Unavailable" : new Date(lastModified).toString()); return tsc.toString(); } - } diff --git a/file-monitor/src/test/java/org/springframework/roo/file/monitor/event/FileDetailsTest.java b/file-monitor/src/test/java/org/springframework/roo/file/monitor/event/FileDetailsTest.java new file mode 100644 index 0000000000..0471741bb0 --- /dev/null +++ b/file-monitor/src/test/java/org/springframework/roo/file/monitor/event/FileDetailsTest.java @@ -0,0 +1,31 @@ +package org.springframework.roo.file.monitor.event; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; + +import java.io.File; + +import org.junit.Test; + +/** + * Unit test of {@link FileDetails} + * + * @author Andrew Swan + * @since 1.2.0 + */ +public class FileDetailsTest { + + @Test + public void testInstancesWithSameFileAndNullTimestamp() { + // Set up + final File mockFile = mock(File.class); + final FileDetails fileDetails1 = new FileDetails(mockFile, null); + final FileDetails fileDetails2 = new FileDetails(mockFile, null); + + // Invoke and check + assertTrue(fileDetails1.equals(fileDetails2) && fileDetails2.equals(fileDetails1)); + assertEquals(fileDetails1.hashCode(), fileDetails2.hashCode()); + assertEquals(0, fileDetails1.compareTo(fileDetails2)); + } +} diff --git a/process-manager/src/main/java/org/springframework/roo/process/manager/internal/DefaultFileManager.java b/process-manager/src/main/java/org/springframework/roo/process/manager/internal/DefaultFileManager.java index 5e062d4739..0015b7e5fb 100644 --- a/process-manager/src/main/java/org/springframework/roo/process/manager/internal/DefaultFileManager.java +++ b/process-manager/src/main/java/org/springframework/roo/process/manager/internal/DefaultFileManager.java @@ -31,6 +31,7 @@ import org.springframework.roo.process.manager.ProcessManager; import org.springframework.roo.support.util.Assert; import org.springframework.roo.support.util.FileCopyUtils; +import org.springframework.roo.support.util.FileUtils; import org.springframework.roo.support.util.StringUtils; /** @@ -112,7 +113,7 @@ public MutableFile createFile(final String fileIdentifier) { } catch (IOException ignored) {} File parentDirectory = new File(actual.getParent()); if (!parentDirectory.exists()) { - createDirectory(FileDetails.getCanonicalPath(parentDirectory)); + createDirectory(FileUtils.getCanonicalPath(parentDirectory)); } new CreateFile(undoManager, filenameResolver, actual); ManagedMessageRenderer renderer = new ManagedMessageRenderer(filenameResolver, actual, true); diff --git a/process-manager/src/main/java/org/springframework/roo/process/manager/internal/DefaultMutableFile.java b/process-manager/src/main/java/org/springframework/roo/process/manager/internal/DefaultMutableFile.java index 29ecf5dcdd..b0d1a02a6f 100644 --- a/process-manager/src/main/java/org/springframework/roo/process/manager/internal/DefaultMutableFile.java +++ b/process-manager/src/main/java/org/springframework/roo/process/manager/internal/DefaultMutableFile.java @@ -8,10 +8,10 @@ import java.io.OutputStream; import org.springframework.roo.file.monitor.NotifiableFileMonitorService; -import org.springframework.roo.file.monitor.event.FileDetails; import org.springframework.roo.process.manager.MutableFile; import org.springframework.roo.support.style.ToStringCreator; import org.springframework.roo.support.util.Assert; +import org.springframework.roo.support.util.FileUtils; /** * Default implementation of {@link MutableFile}. @@ -42,7 +42,7 @@ public DefaultMutableFile(final File file, final NotifiableFileMonitorService fi } public String getCanonicalPath() { - return FileDetails.getCanonicalPath(file); + return FileUtils.getCanonicalPath(file); } public InputStream getInputStream() { diff --git a/project/src/main/java/org/springframework/roo/project/DefaultPathResolvingStrategy.java b/project/src/main/java/org/springframework/roo/project/DefaultPathResolvingStrategy.java index 068e22fc0a..7cc43bfef3 100644 --- a/project/src/main/java/org/springframework/roo/project/DefaultPathResolvingStrategy.java +++ b/project/src/main/java/org/springframework/roo/project/DefaultPathResolvingStrategy.java @@ -35,7 +35,7 @@ public class DefaultPathResolvingStrategy implements PathResolvingStrategy { protected void activate(final ComponentContext context) { final File projectDirectory = new File(StringUtils.defaultIfEmpty(OSGiUtils.getRooWorkingDirectory(context), CURRENT_DIRECTORY)); - rootPath = FileDetails.getCanonicalPath(projectDirectory); + rootPath = FileUtils.getCanonicalPath(projectDirectory); populatePathsMap(projectDirectory); initialisePathCollections(); } @@ -55,7 +55,7 @@ public boolean isActive() { } public String getIdentifier(final ContextualPath path, final String relativePath) { - return FileUtils.normalise(pathCache.get(path.getPath()).getLocationPath()) + relativePath; + return FileUtils.ensureTrailingSeparator(pathCache.get(path.getPath()).getLocationPath()) + relativePath; } /** @@ -95,7 +95,7 @@ public String getRoot(final ContextualPath contextualPath) { final PathInformation pathInfo = pathCache.get(contextualPath.getPath()); Assert.notNull(pathInfo, "Unable to determine information for path '" + contextualPath + "'"); final File root = pathInfo.getLocation(); - return FileDetails.getCanonicalPath(root); + return FileUtils.getCanonicalPath(root); } /** @@ -171,7 +171,7 @@ public String getIdentifier(final Path path, final String relativePath) { final PathInformation pi = pathCache.get(path); Assert.notNull(pi, "Path '" + path + "' is unknown to the path resolver"); final File newPath = new File(pi.getLocation(), relativePath); - return FileDetails.getCanonicalPath(newPath); + return FileUtils.getCanonicalPath(newPath); } public String getRoot() { diff --git a/project/src/main/java/org/springframework/roo/project/MavenPathResolvingStrategy.java b/project/src/main/java/org/springframework/roo/project/MavenPathResolvingStrategy.java index c9617aea82..2112773eee 100644 --- a/project/src/main/java/org/springframework/roo/project/MavenPathResolvingStrategy.java +++ b/project/src/main/java/org/springframework/roo/project/MavenPathResolvingStrategy.java @@ -1,5 +1,18 @@ package org.springframework.roo.project; +import static org.springframework.roo.project.Path.ROOT; +import static org.springframework.roo.project.Path.SPRING_CONFIG_ROOT; +import static org.springframework.roo.project.Path.SRC_MAIN_JAVA; +import static org.springframework.roo.project.Path.SRC_MAIN_RESOURCES; +import static org.springframework.roo.project.Path.SRC_MAIN_WEBAPP; +import static org.springframework.roo.project.Path.SRC_TEST_JAVA; +import static org.springframework.roo.project.Path.SRC_TEST_RESOURCES; +import static org.springframework.roo.project.maven.Pom.DEFAULT_RESOURCES_DIRECTORY; +import static org.springframework.roo.project.maven.Pom.DEFAULT_SOURCE_DIRECTORY; +import static org.springframework.roo.project.maven.Pom.DEFAULT_SPRING_CONFIG_ROOT; +import static org.springframework.roo.project.maven.Pom.DEFAULT_TEST_RESOURCES_DIRECTORY; +import static org.springframework.roo.project.maven.Pom.DEFAULT_TEST_SOURCE_DIRECTORY; +import static org.springframework.roo.project.maven.Pom.DEFAULT_WAR_SOURCE_DIRECTORY; import static org.springframework.roo.support.util.FileUtils.CURRENT_DIRECTORY; import java.io.File; @@ -29,12 +42,12 @@ public class MavenPathResolvingStrategy implements PathResolvingStrategy { protected void activate(final ComponentContext context) { final File projectDirectory = new File(StringUtils.defaultIfEmpty(OSGiUtils.getRooWorkingDirectory(context), CURRENT_DIRECTORY)); - rootPath = FileDetails.getCanonicalPath(projectDirectory); + rootPath = FileUtils.getCanonicalPath(projectDirectory); } public String getFriendlyName(final String identifier) { Assert.notNull(identifier, "Identifier required"); - ContextualPath p = getPath(identifier); + final ContextualPath p = getPath(identifier); if (p == null) { return identifier; @@ -43,7 +56,7 @@ public String getFriendlyName(final String identifier) { } public String getCanonicalPath(final ContextualPath path, final JavaType javaType) { - String relativePath = javaType.getFullyQualifiedTypeName().replace('.', File.separatorChar) + ".java"; + final String relativePath = javaType.getFullyQualifiedTypeName().replace('.', File.separatorChar) + ".java"; return getIdentifier(path, relativePath); } @@ -72,7 +85,7 @@ public String getRoot(final ContextualPath path) { final PathInformation pathInfo = focusedModule.getPathInformation(path); Assert.notNull(pathInfo, "Unable to determine information for path '" + path + "'"); final File root = pathInfo.getLocation(); - return FileDetails.getCanonicalPath(root); + return FileUtils.getCanonicalPath(root); } /** @@ -118,12 +131,12 @@ private PathInformation getApplicablePathInformation(final String identifier) { Assert.notNull(identifier, "Identifier required"); PathInformation pathInformation = null; int longest = 0; - for (Pom pom : pomManagementService.getPomMap().values()) { + for (final Pom pom : pomManagementService.getPomMap().values()) { if (removeTrailingSeparator(identifier).startsWith(removeTrailingSeparator(pom.getRoot())) && removeTrailingSeparator(pom.getRoot()).length() > longest) { longest = removeTrailingSeparator(pom.getRoot()).length(); int nextLongest = 0; - for (PathInformation pi : pom.getPathInformation()) { - String possibleParent = new FileDetails(pi.getLocation(), null).getCanonicalPath(); + for (final PathInformation pi : pom.getPathInformation()) { + final String possibleParent = new FileDetails(pi.getLocation(), null).getCanonicalPath(); if (removeTrailingSeparator(identifier).startsWith(possibleParent) && possibleParent.length() > nextLongest) { nextLongest = possibleParent.length(); pathInformation = pi; @@ -143,7 +156,7 @@ private String removeTrailingSeparator(final String pomPath) { } public ContextualPath getPath(final String identifier) { - PathInformation parent = getApplicablePathInformation(identifier); + final PathInformation parent = getApplicablePathInformation(identifier); if (parent == null) { return null; } @@ -151,11 +164,11 @@ public ContextualPath getPath(final String identifier) { } public String getRelativeSegment(final String identifier) { - PathInformation parent = getApplicablePathInformation(identifier); + final PathInformation parent = getApplicablePathInformation(identifier); if (parent == null) { return null; } - FileDetails parentFileDetails = new FileDetails(parent.getLocation(), null); + final FileDetails parentFileDetails = new FileDetails(parent.getLocation(), null); return parentFileDetails.getRelativeSegment(identifier); } @@ -163,47 +176,47 @@ public boolean isActive() { return !pomManagementService.getPomMap().isEmpty(); } - public PathInformation getPathInformation(final ContextualPath contextualPath) { - Pom module = pomManagementService.getPomFromModuleName(contextualPath.getModule()); - StringBuilder sb = new StringBuilder(); - Path path = contextualPath.getPath(); - if (module == null) { - sb.append(pomManagementService.getFocusedModule().getRoot()).append(File.separator); + private PathInformation getPathInformation(final ContextualPath contextualPath) { + final Pom pom = pomManagementService.getPomFromModuleName(contextualPath.getModule()); + final StringBuilder location = new StringBuilder(); + final Path path = contextualPath.getPath(); + if (pom == null) { + location.append(pomManagementService.getFocusedModule().getRoot()).append(File.separator); if (StringUtils.hasText(contextualPath.getModule())) { - sb.append(contextualPath.getModule()).append(File.separator); + location.append(contextualPath.getModule()).append(File.separator); } } else { - sb.append(module.getRoot()).append(File.separator); + location.append(pom.getRoot()).append(File.separator); } - if (path.equals(Path.SRC_MAIN_JAVA)) { - String sourceDirectory = Pom.DEFAULT_SOURCE_DIRECTORY; - if (module != null) { - if (StringUtils.hasText(module.getSourceDirectory())) { - sourceDirectory = module.getSourceDirectory(); + if (path.equals(SRC_MAIN_JAVA)) { + String sourceDirectory = DEFAULT_SOURCE_DIRECTORY; + if (pom != null) { + if (StringUtils.hasText(pom.getSourceDirectory())) { + sourceDirectory = pom.getSourceDirectory(); } } - sb.append(sourceDirectory); - } else if (path.equals(Path.SRC_MAIN_RESOURCES)) { - sb.append(File.separator).append(Pom.DEFAULT_RESOURCES_DIRECTORY); - } else if (path.equals(Path.SRC_TEST_JAVA)) { - String testSourceDirectory = Pom.DEFAULT_TEST_SOURCE_DIRECTORY; - if (module != null) { - if (StringUtils.hasText(module.getTestSourceDirectory())) { - testSourceDirectory = module.getTestSourceDirectory(); + location.append(sourceDirectory); + } else if (path.equals(SRC_MAIN_RESOURCES)) { + location.append(File.separator).append(DEFAULT_RESOURCES_DIRECTORY); + } else if (path.equals(SRC_TEST_JAVA)) { + String testSourceDirectory = DEFAULT_TEST_SOURCE_DIRECTORY; + if (pom != null) { + if (StringUtils.hasText(pom.getTestSourceDirectory())) { + testSourceDirectory = pom.getTestSourceDirectory(); } } - sb.append(testSourceDirectory); - } else if (path.equals(Path.SRC_TEST_RESOURCES)) { - sb.append(Pom.DEFAULT_TEST_RESOURCES_DIRECTORY); - } else if (path.equals(Path.SRC_MAIN_WEBAPP)) { - sb.append(Pom.DEFAULT_WAR_SOURCE_DIRECTORY); - } else if (path.equals(Path.SPRING_CONFIG_ROOT)) { - sb.append(Pom.DEFAULT_SPRING_CONFIG_ROOT); - } else if (path.equals(Path.ROOT)) { + location.append(testSourceDirectory); + } else if (path.equals(SRC_TEST_RESOURCES)) { + location.append(DEFAULT_TEST_RESOURCES_DIRECTORY); + } else if (path.equals(SRC_MAIN_WEBAPP)) { + location.append(DEFAULT_WAR_SOURCE_DIRECTORY); + } else if (path.equals(SPRING_CONFIG_ROOT)) { + location.append(DEFAULT_SPRING_CONFIG_ROOT); + } else if (path.equals(ROOT)) { // do nothing } - return new PathInformation(contextualPath, true, new File(sb.toString())); + return new PathInformation(contextualPath, true, new File(location.toString())); } public String getIdentifier(final ContextualPath contextualPath, final String relativePath) { @@ -211,8 +224,8 @@ public String getIdentifier(final ContextualPath contextualPath, final String re Assert.notNull(relativePath, "Relative path cannot be null, although it can be empty"); String initialPath = getPathInformation(contextualPath).getLocationPath(); - initialPath = FileUtils.normalise(initialPath); - return initialPath + FileUtils.removePrePostSeparator(relativePath); + initialPath = FileUtils.ensureTrailingSeparator(initialPath); + return initialPath + FileUtils.removeLeadingAndTrailingSeparators(relativePath); } public String getRoot() { diff --git a/project/src/main/java/org/springframework/roo/project/PathInformation.java b/project/src/main/java/org/springframework/roo/project/PathInformation.java index d888d965a9..e01dcc16fc 100644 --- a/project/src/main/java/org/springframework/roo/project/PathInformation.java +++ b/project/src/main/java/org/springframework/roo/project/PathInformation.java @@ -2,9 +2,9 @@ import java.io.File; -import org.springframework.roo.file.monitor.event.FileDetails; import org.springframework.roo.support.style.ToStringCreator; import org.springframework.roo.support.util.Assert; +import org.springframework.roo.support.util.FileUtils; /** * Used by {@link DelegatePathResolver} to permit subclasses to register path details. @@ -47,7 +47,7 @@ public File getLocation() { } public String getLocationPath() { - return FileDetails.getCanonicalPath(location); + return FileUtils.getCanonicalPath(location); } public Path getPath() { diff --git a/project/src/main/java/org/springframework/roo/project/PathResolvingAwareFilenameResolver.java b/project/src/main/java/org/springframework/roo/project/PathResolvingAwareFilenameResolver.java index 75d0416fa4..8e7f8bda8f 100644 --- a/project/src/main/java/org/springframework/roo/project/PathResolvingAwareFilenameResolver.java +++ b/project/src/main/java/org/springframework/roo/project/PathResolvingAwareFilenameResolver.java @@ -5,9 +5,9 @@ import org.apache.felix.scr.annotations.Component; import org.apache.felix.scr.annotations.Reference; import org.apache.felix.scr.annotations.Service; -import org.springframework.roo.file.monitor.event.FileDetails; import org.springframework.roo.file.undo.FilenameResolver; import org.springframework.roo.support.util.Assert; +import org.springframework.roo.support.util.FileUtils; /** * {@link FilenameResolver} that delegates to {@link PathResolver}. @@ -24,6 +24,6 @@ public class PathResolvingAwareFilenameResolver implements FilenameResolver { public String getMeaningfulName(final File file) { Assert.notNull(file, "File required"); - return pathResolver.getFriendlyName(FileDetails.getCanonicalPath(file)); + return pathResolver.getFriendlyName(FileUtils.getCanonicalPath(file)); } } diff --git a/project/src/main/java/org/springframework/roo/project/PomManagementServiceImpl.java b/project/src/main/java/org/springframework/roo/project/PomManagementServiceImpl.java index f7f88a62fb..366a4ae141 100644 --- a/project/src/main/java/org/springframework/roo/project/PomManagementServiceImpl.java +++ b/project/src/main/java/org/springframework/roo/project/PomManagementServiceImpl.java @@ -20,7 +20,6 @@ import org.apache.felix.scr.annotations.Service; import org.osgi.service.component.ComponentContext; import org.springframework.roo.file.monitor.NotifiableFileMonitorService; -import org.springframework.roo.file.monitor.event.FileDetails; import org.springframework.roo.metadata.MetadataDependencyRegistry; import org.springframework.roo.metadata.MetadataService; import org.springframework.roo.process.manager.FileManager; @@ -60,14 +59,14 @@ public class PomManagementServiceImpl implements PomManagementService { public Pom getModuleForFileIdentifier(final String fileIdentifier) { updatePomCache(); String startingPoint = FileUtils.getFirstDirectory(fileIdentifier); - String pomPath = FileUtils.normalise(startingPoint) + "pom.xml"; + String pomPath = FileUtils.ensureTrailingSeparator(startingPoint) + "pom.xml"; File pom = new File(pomPath); while (!pom.exists()) { if (startingPoint.equals(File.separator)) { break; } startingPoint = FileUtils.backOneDirectory(startingPoint); - pomPath = FileUtils.normalise(startingPoint) + "pom.xml"; + pomPath = FileUtils.ensureTrailingSeparator(startingPoint) + "pom.xml"; pom = new File(pomPath); } return getPomFromPath(pomPath); @@ -89,7 +88,7 @@ public Pom getPomFromModuleName(final String moduleName) { } private String getModuleName(final String pomRoot) { - final String moduleName = FileUtils.normalise(pomRoot).replaceAll(FileUtils.normalise(rootPath), ""); + final String moduleName = FileUtils.ensureTrailingSeparator(pomRoot).replaceAll(FileUtils.ensureTrailingSeparator(rootPath), ""); return FileUtils.removeTrailingSeparator(moduleName); } @@ -141,7 +140,7 @@ public void setFocusedModule(final String focusedModulePath) { protected void activate(final ComponentContext context) { final File projectDirectory = new File(StringUtils.defaultIfEmpty(OSGiUtils.getRooWorkingDirectory(context), CURRENT_DIRECTORY)); - rootPath = FileDetails.getCanonicalPath(projectDirectory); + rootPath = FileUtils.getCanonicalPath(projectDirectory); } private void updatePomCache() { diff --git a/project/src/main/java/org/springframework/roo/project/maven/Pom.java b/project/src/main/java/org/springframework/roo/project/maven/Pom.java index 3e5d8ab6db..583db14ff4 100644 --- a/project/src/main/java/org/springframework/roo/project/maven/Pom.java +++ b/project/src/main/java/org/springframework/roo/project/maven/Pom.java @@ -113,8 +113,14 @@ public Pom(final String groupId, final String artifactId, final String version, cachePathInformation(Path.ROOT); } + /** + * Returns the canonical path of the given logical {@link Path} within this module, plus a trailing separator + * + * @param path the logical path for which to get the canonical location (required) + * @return a valid canonical path + */ public String getPathLocation(final Path path) { - return FileUtils.normalise(getPathInformation(path).getLocationPath()); + return FileUtils.ensureTrailingSeparator(getPathInformation(path).getLocationPath()); } public PathInformation getPathInformation(final Path path) { @@ -125,10 +131,6 @@ public PathInformation getPathInformation(final ContextualPath path) { return pathCache.get(path.getPath()); } - public String getRoot(final Path path) { - return getPathLocation(path); - } - private void cachePathInformation(final Path path) { String moduleRoot = moduleRoot(getPath()); StringBuilder sb = new StringBuilder(); @@ -160,16 +162,19 @@ private void cachePathInformation(final Path path) { pathCache.put(path, pathInformation); } + /** + * Returns the canonical path of this module's root directory, plus a trailing separator + * + * @return a valid canonical path + */ public String getRoot() { - return getRoot(Path.ROOT); + return getPathLocation(Path.ROOT); } public List getPathInformation() { return new ArrayList(pathCache.values()); } - - /** * Indicates whether all of the given dependencies are registered, using * {@link Dependency#equals(Object)} to evaluate each one against the diff --git a/project/src/test/java/org/springframework/roo/project/MavenPathResolvingStrategyTest.java b/project/src/test/java/org/springframework/roo/project/MavenPathResolvingStrategyTest.java new file mode 100644 index 0000000000..50a6c2626e --- /dev/null +++ b/project/src/test/java/org/springframework/roo/project/MavenPathResolvingStrategyTest.java @@ -0,0 +1,54 @@ +package org.springframework.roo.project; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.io.File; + +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.springframework.roo.project.maven.Pom; + +/** + * Unit test of {@link MavenPathResolvingStrategy} + * + * @author Andrew Swan + * @since 1.2.0 + */ +public class MavenPathResolvingStrategyTest { + + // Constants + private static String ROOT_MODULE = ""; + + // Fixture + private MavenPathResolvingStrategy strategy; + @Mock private PomManagementService mockPomManagementService; + + @Before + public void setUp() throws Exception { + MockitoAnnotations.initMocks(this); + strategy = new MavenPathResolvingStrategy(); + strategy.pomManagementService = mockPomManagementService; + } + + @Test + public void testGetIdentifierForRootPathOfRootModule() { + // Set up + final ContextualPath mockContextualPath = mock(ContextualPath.class); + when(mockContextualPath.getModule()).thenReturn(ROOT_MODULE); + when(mockContextualPath.getPath()).thenReturn(Path.ROOT); // can't be mocked + final Pom mockPom = mock(Pom.class); + final String rootPath = "/path/to/the/pom"; + when(mockPom.getRoot()).thenReturn(rootPath); + when(mockPomManagementService.getPomFromModuleName(ROOT_MODULE)).thenReturn(mockPom); + + // Invoke + final String identifier = strategy.getIdentifier(mockContextualPath , ""); + + // Check + assertEquals(rootPath + File.separator, identifier); // TODO check this is what we actually want + } +} diff --git a/support/src/main/java/org/springframework/roo/support/util/FileUtils.java b/support/src/main/java/org/springframework/roo/support/util/FileUtils.java index 7e66f42611..3c330d948a 100644 --- a/support/src/main/java/org/springframework/roo/support/util/FileUtils.java +++ b/support/src/main/java/org/springframework/roo/support/util/FileUtils.java @@ -4,6 +4,9 @@ import java.io.IOException; import java.util.regex.Pattern; +import org.springframework.roo.support.ant.AntPathMatcher; +import org.springframework.roo.support.ant.PathMatcher; + /** * Utilities for handling {@link File} instances. * @@ -24,6 +27,13 @@ public final class FileUtils { // Doesn't check for backslash after the colon, since Java has no issues with paths like c:/Windows private static final Pattern WINDOWS_DRIVE_PATH = Pattern.compile("^[A-Za-z]:.*"); + + private static final PathMatcher PATH_MATCHER; + + static { + PATH_MATCHER = new AntPathMatcher(); + ((AntPathMatcher) PATH_MATCHER).setPathSeparator(File.separator); + } /** * Deletes the specified {@link File}. @@ -149,45 +159,63 @@ public static String backOneDirectory(String fileIdentifier) { } /** - * TODO + * Removes any trailing {@link File#separator}s from the given path * - * @param pomPath - * @return + * @param path the path to modify (can be null) + * @return the modified path * @since 1.2.0 */ - public static String removeTrailingSeparator(String pomPath) { - while (pomPath.endsWith(File.separator)) { - pomPath = pomPath.substring(0, pomPath.length() - 1); + public static String removeTrailingSeparator(String path) { + while (path != null && path.endsWith(File.separator)) { + path = StringUtils.removeSuffix(path, File.separator); } - return pomPath; + return path; + } + + /** + * Indicates whether the given canonical path matches the given Ant-style pattern + * + * @param antPattern the pattern to check against (can't be blank) + * @param canonicalPath the path to check (can't be blank) + * @return see above + * @since 1.2.0 + */ + public static boolean matchesAntPath(final String antPattern, final String canonicalPath) { + Assert.hasText(antPattern, "Ant pattern required"); + Assert.hasText(canonicalPath, "Canonical path required"); + return PATH_MATCHER.match(antPattern, canonicalPath); } /** - * TODO + * Removes any leading or trailing {@link File#separator}s from the given path. * - * @param pomPath - * @return + * @param path the path to modify (can be null) + * @return the path, modified as above, or null if null was given * @since 1.2.0 */ - public static String removePrePostSeparator(String pomPath) { - while (pomPath.endsWith(File.separator)) { - pomPath = pomPath.substring(0, pomPath.length() - 1); + public static String removeLeadingAndTrailingSeparators(String path) { + if (StringUtils.isBlank(path)) { + return path; + } + while (path.endsWith(File.separator)) { + path = StringUtils.removeSuffix(path, File.separator); } - while (pomPath.startsWith(File.separator)) { - pomPath = pomPath.substring(1, pomPath.length()); + while (path.startsWith(File.separator)) { + path = StringUtils.removePrefix(path, File.separator); } - return pomPath; + return path; } /** - * TODO + * Ensures that the given path has exactly one trailing {@link File#separator} * - * @param pomPath - * @return + * @param path the path to modify (can't be null) + * @return the normalised path * @since 1.2.0 */ - public static String normalise(final String pomPath) { - return removeTrailingSeparator(pomPath) + File.separatorChar; + public static String ensureTrailingSeparator(final String path) { + Assert.notNull(path); + return removeTrailingSeparator(path) + File.separatorChar; } /** @@ -203,6 +231,24 @@ public static String getSystemDependentPath(final String... pathElements) { return StringUtils.arrayToDelimitedString(pathElements, File.separator); } + /** + * Returns the canonical path of the given {@link File}. + * + * @param file the file for which to find the canonical path (can be null) + * @return the canonical path, or null if a null file is given + * @since 1.2.0 + */ + public static String getCanonicalPath(final File file) { + if (file == null) { + return null; + } + try { + return file.getCanonicalPath(); + } catch (final IOException ioe) { + throw new IllegalStateException("Cannot determine canonical path for '" + file + "'", ioe); + } + } + /** * Returns the platform-specific file separator as a regular expression. * diff --git a/support/src/test/java/org/springframework/roo/support/util/FileUtilsTest.java b/support/src/test/java/org/springframework/roo/support/util/FileUtilsTest.java index e77acb8bde..9debba3db4 100644 --- a/support/src/test/java/org/springframework/roo/support/util/FileUtilsTest.java +++ b/support/src/test/java/org/springframework/roo/support/util/FileUtilsTest.java @@ -1,9 +1,13 @@ package org.springframework.roo.support.util; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.io.File; +import java.io.IOException; import org.junit.Test; @@ -48,4 +52,109 @@ public void testGetFileSeparatorAsRegex() throws Exception { // Check assertTrue(pathElements.length > 0); } + + @Test + public void testRemoveTrailingSeparatorFromNullPath() { + assertNull(FileUtils.removeTrailingSeparator(null)); + } + + @Test + public void testRemoveTrailingSeparatorFromEmptyPath() { + assertEquals("", FileUtils.removeTrailingSeparator("")); + } + + @Test + public void testRemoveTrailingSeparatorFromPathWithLeadingSeparator() { + final String path = File.separator + "foo"; + assertEquals(path, FileUtils.removeTrailingSeparator(path)); + } + + @Test + public void testRemoveTrailingSeparatorFromPathWithMultipleTrailingSeparators() { + final String path = "foo" + StringUtils.repeat(File.separator, 3); + assertEquals("foo", FileUtils.removeTrailingSeparator(path)); + } + + @Test(expected = IllegalArgumentException.class) + public void testEnsureTrailingSeparatorForNullPath() { + FileUtils.ensureTrailingSeparator(null); + } + + @Test + public void testEnsureTrailingSeparatorForEmptyPath() { + assertEquals(File.separator, FileUtils.ensureTrailingSeparator("")); + } + + @Test + public void testEnsureTrailingSeparatorForPathWithNoTrailingSeparator() { + final String path = "foo"; + assertEquals(path + File.separator, FileUtils.ensureTrailingSeparator(path)); + } + + @Test + public void testEnsureTrailingSeparatorForPathWithOneTrailingSeparator() { + final String path = "foo" + File.separator; + assertEquals(path, FileUtils.ensureTrailingSeparator(path)); + } + + @Test + public void testEnsureTrailingSeparatorFromPathWithMultipleTrailingSeparators() { + final String path = "foo" + StringUtils.repeat(File.separator, 3); + assertEquals("foo" + File.separator, FileUtils.ensureTrailingSeparator(path)); + } + + @Test + public void testGetCanonicalPathForNullFile() { + assertNull(FileUtils.getCanonicalPath(null)); + } + + @Test(expected = IllegalStateException.class) + public void testGetCanonicalPathForInvalidFile() throws Exception { + // Set up + final File invalidFile = mock(File.class); + when(invalidFile.getCanonicalPath()).thenThrow(new IOException("dummy")); + + // Invoke + FileUtils.getCanonicalPath(invalidFile); + } + + @Test + public void testGetCanonicalPathForValidFile() throws Exception { + // Set up + final File validFile = mock(File.class); + final String canonicalPath = "the_path"; + when(validFile.getCanonicalPath()).thenReturn(canonicalPath); + + // Invoke + final String actualPath = FileUtils.getCanonicalPath(validFile); + + // Check + assertEquals(canonicalPath, actualPath); + } + + @Test + public void testRemoveLeadingAndTrailingSeparatorsFromNullPath() { + assertNull(FileUtils.removeLeadingAndTrailingSeparators(null)); + } + + @Test + public void testRemoveLeadingAndTrailingSeparatorsFromEmptyPath() { + assertEquals("", FileUtils.removeLeadingAndTrailingSeparators("")); + } + + @Test + public void testRemoveLeadingAndTrailingSeparatorsFromPlainPath() { + final String path = "foo"; + assertEquals(path, FileUtils.removeLeadingAndTrailingSeparators(path)); + } + + @Test + public void testRemoveLeadingAndTrailingSeparatorsFromPathWithBoth() { + // Set up + final String separators = StringUtils.repeat(File.separator, 4); + final String path = separators + "foo" + separators; + + // Invoke and check + assertEquals("foo", FileUtils.removeLeadingAndTrailingSeparators(path)); + } }