Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MSHARED-1224] Prefer JDK classes to Plexus utils #81

Merged
merged 10 commits into from
Apr 8, 2023
Merged

[MSHARED-1224] Prefer JDK classes to Plexus utils #81

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3aff23d
Prefer JDK classes to Plexus utils
elharo Mar 11, 2023
c166125
merge master
elharo Mar 11, 2023
82dddaf
use Path.toString
elharo Mar 19, 2023
f1ff4ca
Revert "use Path.toString"
elharo Mar 24, 2023
a08fcad
use separatorChar
elharo Mar 24, 2023
bf5329a
jars always use / as the separator
elharo Mar 24, 2023
d3c8fad
back to nio
elharo Mar 24, 2023
ac97778
Merge remote-tracking branch 'origin/master' into plex
slawekjaranowski Apr 7, 2023
cdcbd18
Use try-with-resource for Files.walk
slawekjaranowski Apr 7, 2023
e3a3170
Use try-with-resource for Files.walk
slawekjaranowski Apr 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,6 @@
<version>1</version>
</dependency>

<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-utils</artifactId>
<version>3.5.1</version>
</dependency>

<dependency>
<groupId>org.ow2.asm</groupId>
<artifactId>asm</artifactId>
Expand Down
64 changes: 26 additions & 38 deletions src/main/java/org/apache/maven/shared/dependency/analyzer/ClassFileVisitorUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,18 @@
* under the License.
*/

import org.codehaus.plexus.util.DirectoryScanner;

import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.util.List;
import java.util.jar.JarEntry;
import java.util.jar.JarInputStream;
import java.util.stream.Collectors;

/**
* Utility to visit classes in a library given either as a jar file or an exploded directory.
Expand All @@ -45,11 +46,10 @@ private ClassFileVisitorUtils()
}

/**
* <p>accept.</p>
*
* @param url a {@link java.net.URL} object.
* @param visitor a {@link org.apache.maven.shared.dependency.analyzer.ClassFileVisitor} object.
* @throws java.io.IOException if any.
* @param url a {@link java.net.URL} object
* @param visitor a {@link org.apache.maven.shared.dependency.analyzer.ClassFileVisitor} object
* @throws java.io.IOException if any
*/
public static void accept( URL url, ClassFileVisitor visitor )
throws IOException
Expand Down Expand Up @@ -85,8 +85,6 @@ else if ( file.exists() )
}
}

// private methods --------------------------------------------------------

private static void acceptJar( URL url, ClassFileVisitor visitor )
throws IOException
{
Expand All @@ -99,7 +97,8 @@ private static void acceptJar( URL url, ClassFileVisitor visitor )
// ignore files like package-info.class and module-info.class
if ( name.endsWith( ".class" ) && name.indexOf( '-' ) == -1 )
{
visitClass( name, in, visitor );
// Even on Windows Jars use / as the separator character
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment is redundant and wrong because this has nothing to do with Windows. REad the PKZIP implementation note: separators are always foward slashes regarding of the OS.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's the point. The previous version was using the system dependent line separator, which was why the test was failing on Windows but not Linux.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment was changed

visitClass( name, in, visitor, '/' );
}
}
}
Expand All @@ -108,43 +107,32 @@ private static void acceptJar( URL url, ClassFileVisitor visitor )
private static void acceptDirectory( File directory, ClassFileVisitor visitor )
throws IOException
{
if ( !directory.isDirectory() )

List<Path> classFiles = Files.walk( directory.toPath() )
.filter( path -> path.getFileName().toString().endsWith( ".class" ) )
.collect( Collectors.toList() );

for ( Path path : classFiles )
{
throw new IllegalArgumentException( "File is not a directory" );
}

DirectoryScanner scanner = new DirectoryScanner();

scanner.setBasedir( directory );
scanner.setIncludes( new String[] { "**/*.class" } );

scanner.scan();

String[] paths = scanner.getIncludedFiles();

for ( String path : paths )
{
path = path.replace( File.separatorChar, '/' );

File file = new File( directory, path );

try ( InputStream in = new FileInputStream( file ) )
try ( InputStream in = Files.newInputStream( path ) )
{
visitClass( path, in, visitor );
visitClass( directory, path, in, visitor );
}
}
}

private static void visitClass( String path, InputStream in, ClassFileVisitor visitor )
private static void visitClass( File baseDirectory, Path path, InputStream in, ClassFileVisitor visitor )
{
if ( !path.endsWith( ".class" ) )
{
throw new IllegalArgumentException( "Path is not a class" );
}
// getPath() returns a String, not a java.nio.file.Path
String stringPath = path.toFile().getPath().substring( baseDirectory.getPath().length() + 1 );
michael-o marked this conversation as resolved.
Show resolved Hide resolved
visitClass( stringPath, in, visitor, File.separatorChar );
}

String className = path.substring( 0, path.length() - 6 );
private static void visitClass( String stringPath, InputStream in, ClassFileVisitor visitor, char separator )
{
String className = stringPath.substring( 0, stringPath.length() - 6 );

className = className.replace( '/', '.' );
className = className.replace( separator, '.' );

visitor.visitClass( className, in );
}
Expand Down
13 changes: 3 additions & 10 deletions src/test/java/org/apache/maven/shared/dependency/analyzer/ClassFileVisitorUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

import org.apache.commons.io.FileUtils;
import org.apache.commons.io.IOUtils;
import org.junit.Before;
import org.junit.Test;

import java.io.File;
Expand All @@ -48,9 +47,9 @@
*/
public class ClassFileVisitorUtilsTest
{
private MockVisitor visitor;
private TestVisitor visitor = new TestVisitor();

private static class MockVisitor implements ClassFileVisitor
private static class TestVisitor implements ClassFileVisitor
{
final List<String> classNames = new ArrayList<>();
final List<String> data = new ArrayList<>();
Expand All @@ -71,12 +70,6 @@ public void visitClass( String className, InputStream in )
}
}

@Before
public void setUp()
{
visitor = new MockVisitor();
}

@Test
public void testAcceptJar() throws IOException
{
Expand Down Expand Up @@ -110,7 +103,7 @@ public void testAcceptJarWithNonClassEntry() throws IOException

ClassFileVisitorUtils.accept( file.toURI().toURL(), visitor );

assertThat( visitor.classNames ) .isEmpty();
assertThat( visitor.classNames ).isEmpty();
}

@Test
Expand Down