Skip to content

Commit

Permalink
[MRESOLVER-605] Linking support in file transporter (#578)
Browse files Browse the repository at this point in the history
Support linking for FileTransport. Still, as FileTransport is remote transport, due checksumming (via Listener) file does have to be read.

Big fat note: this change will cause that symlinks may land in local repository. This PR does NOT enforce rest of Maven plugins knows how to handle them, is out of scope for this PR.

This feature is EXPERIMENTAL.

---

https://issues.apache.org/jira/browse/MRESOLVER-605
  • Loading branch information
cstamas authored Oct 12, 2024
1 parent 7b9214f commit 8b0c7ff
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 28 deletions.
1 change: 1 addition & 0 deletions .github/workflows/maven-verify.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,5 @@ jobs:
ff-site-run: false
jdk-matrix: '[ "21" ]'
maven-matrix: '[ "3.9.8" ]'
verify-fail-fast: false

Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,11 @@
*/
package org.eclipse.aether.transport.file;

import java.nio.file.FileSystemNotFoundException;
import java.nio.ByteBuffer;
import java.nio.channels.FileChannel;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;

import org.eclipse.aether.repository.RemoteRepository;
import org.eclipse.aether.repository.RepositoryUriUtils;
import org.eclipse.aether.spi.connector.transport.AbstractTransporter;
import org.eclipse.aether.spi.connector.transport.GetTask;
import org.eclipse.aether.spi.connector.transport.PeekTask;
Expand All @@ -36,15 +34,23 @@
* A transporter using {@link java.io.File}.
*/
final class FileTransporter extends AbstractTransporter {
/**
* The file op transport can use.
*
* @since 2.0.2
*/
enum FileOp {
COPY,
SYMLINK,
HARDLINK;
}

private final Path basePath;
private final FileOp fileOp;

FileTransporter(RemoteRepository repository) throws NoTransporterException {
try {
basePath = Paths.get(RepositoryUriUtils.toUri(repository.getUrl())).toAbsolutePath();
} catch (FileSystemNotFoundException | IllegalArgumentException e) {
throw new NoTransporterException(repository, e);
}
FileTransporter(Path basePath, FileOp fileOp) throws NoTransporterException {
this.basePath = basePath;
this.fileOp = fileOp;
}

Path getBasePath() {
Expand All @@ -59,6 +65,14 @@ public int classify(Throwable error) {
return ERROR_OTHER;
}

private FileOp effectiveFileOp(FileOp wanted, GetTask task) {
if (task.getDataPath() != null) {
return wanted;
}
// task carries no path, caller wants in-memory read, so COPY must be used
return FileOp.COPY;
}

@Override
protected void implPeek(PeekTask task) throws Exception {
getPath(task, true);
Expand All @@ -67,7 +81,40 @@ protected void implPeek(PeekTask task) throws Exception {
@Override
protected void implGet(GetTask task) throws Exception {
Path path = getPath(task, true);
utilGet(task, Files.newInputStream(path), true, Files.size(path), false);
long size = Files.size(path);
FileOp effective = effectiveFileOp(fileOp, task);
switch (effective) {
case COPY:
utilGet(task, Files.newInputStream(path), true, size, false);
break;
case SYMLINK:
case HARDLINK:
Files.deleteIfExists(task.getDataPath());
task.getListener().transportStarted(0L, size);
if (effective == FileOp.HARDLINK) {
Files.createLink(task.getDataPath(), path);
} else {
Files.createSymbolicLink(task.getDataPath(), path);
}
if (size > 0) {
try (FileChannel fc = FileChannel.open(path)) {
try {
task.getListener().transportProgressed(fc.map(FileChannel.MapMode.READ_ONLY, 0, fc.size()));
} catch (UnsupportedOperationException e) {
// not all FS support mmap: fallback to "plain read loop"
ByteBuffer byteBuffer = ByteBuffer.allocate(1024 * 32);
while (fc.read(byteBuffer) != -1) {
byteBuffer.flip();
task.getListener().transportProgressed(byteBuffer);
byteBuffer.clear();
}
}
}
}
break;
default:
throw new IllegalStateException("Unknown fileOp" + fileOp);
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,12 @@

import javax.inject.Named;

import java.nio.file.FileSystemNotFoundException;
import java.nio.file.Paths;

import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.repository.RemoteRepository;
import org.eclipse.aether.repository.RepositoryUriUtils;
import org.eclipse.aether.spi.connector.transport.Transporter;
import org.eclipse.aether.spi.connector.transport.TransporterFactory;
import org.eclipse.aether.transfer.NoTransporterException;
Expand Down Expand Up @@ -66,6 +70,20 @@ public Transporter newInstance(RepositorySystemSession session, RemoteRepository
requireNonNull(session, "session cannot be null");
requireNonNull(repository, "repository cannot be null");

return new FileTransporter(repository);
FileTransporter.FileOp fileOp = FileTransporter.FileOp.COPY;
String repositoryUrl = repository.getUrl();
if (repositoryUrl.startsWith("symlink+")) {
fileOp = FileTransporter.FileOp.SYMLINK;
repositoryUrl = repositoryUrl.substring("symlink+".length());
} else if (repositoryUrl.startsWith("hardlink+")) {
fileOp = FileTransporter.FileOp.HARDLINK;
repositoryUrl = repositoryUrl.substring("hardlink+".length());
}
try {
return new FileTransporter(
Paths.get(RepositoryUriUtils.toUri(repositoryUrl)).toAbsolutePath(), fileOp);
} catch (FileSystemNotFoundException | IllegalArgumentException e) {
throw new NoTransporterException(repository, e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,23 @@ public class FileTransporterTest {

private Path repoDir;

private Path tempDir;

private FileSystem fileSystem;

enum FS {
DEFAULT,
JIMFS
DEFAULT(""),
DEFAULT_SL("symlink+"),
DEFAULT_HL("hardlink+"),
JIMFS(""),
JIMFS_SL("symlink+"),
JIMFS_HL("hardlink+");

final String uriPrefix;

FS(String uriPrefix) {
this.uriPrefix = uriPrefix;
}
}

private RemoteRepository newRepo(String url) {
Expand All @@ -86,12 +98,15 @@ private void newTransporter(String url) throws Exception {

void setUp(FS fs) {
try {
fileSystem = fs == FS.JIMFS ? Jimfs.newFileSystem() : null;
repoDir = fileSystem == null ? TestFileUtils.createTempDir().toPath() : fileSystem.getPath(".");
fileSystem = fs.name().startsWith("JIMFS") ? Jimfs.newFileSystem() : null;
repoDir = fileSystem == null ? TestFileUtils.createTempDir().toPath() : fileSystem.getPath("repo");
Files.createDirectories(repoDir);
tempDir = fileSystem == null ? TestFileUtils.createTempDir().toPath() : fileSystem.getPath("tmp");
Files.createDirectories(tempDir);
Files.write(repoDir.resolve("file.txt"), "test".getBytes(StandardCharsets.UTF_8));
Files.write(repoDir.resolve("empty.txt"), "".getBytes(StandardCharsets.UTF_8));
Files.write(repoDir.resolve("some space.txt"), "space".getBytes(StandardCharsets.UTF_8));
newTransporter(repoDir.toUri().toASCIIString());
newTransporter(fs.uriPrefix + repoDir.toUri().toASCIIString());
} catch (Exception e) {
Assertions.fail(e);
}
Expand Down Expand Up @@ -169,11 +184,12 @@ void testGet_ToMemory(FS fs) throws Exception {
@EnumSource(FS.class)
void testGet_ToFile(FS fs) throws Exception {
setUp(fs);
Path file = TestFileUtils.createTempFile("failure").toPath();
Path file = tempDir.resolve("testGet_ToFile");
Files.write(file, "whatever".getBytes(StandardCharsets.UTF_8));
RecordingTransportListener listener = new RecordingTransportListener();
GetTask task = new GetTask(URI.create("file.txt")).setDataPath(file).setListener(listener);
transporter.get(task);
assertEquals("test", TestFileUtils.readString(file.toFile()));
assertEquals("test", new String(Files.readAllBytes(file), StandardCharsets.UTF_8));
assertEquals(0L, listener.dataOffset);
assertEquals(4L, listener.dataLength);
assertEquals(1, listener.startedCount);
Expand All @@ -185,11 +201,12 @@ void testGet_ToFile(FS fs) throws Exception {
@EnumSource(FS.class)
void testGet_EmptyResource(FS fs) throws Exception {
setUp(fs);
Path file = TestFileUtils.createTempFile("failure").toPath();
Path file = tempDir.resolve("testGet_EmptyResource");
Files.write(file, "".getBytes(StandardCharsets.UTF_8));
RecordingTransportListener listener = new RecordingTransportListener();
GetTask task = new GetTask(URI.create("empty.txt")).setDataPath(file).setListener(listener);
transporter.get(task);
assertEquals("", TestFileUtils.readString(file.toFile()));
assertEquals("", new String(Files.readAllBytes(file), StandardCharsets.UTF_8));
assertEquals(0L, listener.dataOffset);
assertEquals(0L, listener.dataLength);
assertEquals(1, listener.startedCount);
Expand Down Expand Up @@ -229,9 +246,22 @@ void testGet_Query(FS fs) throws Exception {
void testGet_FileHandleLeak(FS fs) throws Exception {
setUp(fs);
for (int i = 0; i < 100; i++) {
Path file = TestFileUtils.createTempFile("failure").toPath();
Path file = tempDir.resolve("testGet_FileHandleLeak" + i);
transporter.get(new GetTask(URI.create("file.txt")).setDataPath(file));
assertTrue(file.toFile().delete(), i + ", " + file.toFile().getAbsolutePath());
if (fs.uriPrefix.startsWith("symlink+")) {
assertTrue(Files.isSymbolicLink(file));
assertTrue(Files.deleteIfExists(file), i + ", " + file.toAbsolutePath());
} else if (fs.uriPrefix.startsWith("hardlink+")) {
assertTrue(Files.isRegularFile(file));
// Doing this on windows FS is not possible (immediately create then delete link) due windows lock
// semantics. While other OS do perform this test OK, it fails on Windows with AccessDeniedEx.
// The file becomes deletable on Windows after some arbitrary time, but let's not fiddle with that in
// this UT.
// assertTrue(Files.deleteIfExists(file), i + ", " + file.toAbsolutePath());
} else {
assertTrue(Files.isRegularFile(file));
assertTrue(Files.deleteIfExists(file), i + ", " + file.toAbsolutePath());
}
}
}

Expand Down Expand Up @@ -316,7 +346,8 @@ void testPut_FromMemory(FS fs) throws Exception {
@EnumSource(FS.class)
void testPut_FromFile(FS fs) throws Exception {
setUp(fs);
Path file = TestFileUtils.createTempFile("upload").toPath();
Path file = tempDir.resolve("upload");
Files.write(file, "upload".getBytes(StandardCharsets.UTF_8));
RecordingTransportListener listener = new RecordingTransportListener();
PutTask task = new PutTask(URI.create("file.txt")).setListener(listener).setDataPath(file);
transporter.put(task);
Expand Down Expand Up @@ -380,10 +411,11 @@ void testPut_EncodedResourcePath(FS fs) throws Exception {
void testPut_FileHandleLeak(FS fs) throws Exception {
setUp(fs);
for (int i = 0; i < 100; i++) {
Path src = TestFileUtils.createTempFile("upload").toPath();
Path src = tempDir.resolve("upload");
Files.write(src, "upload".getBytes(StandardCharsets.UTF_8));
Path dst = repoDir.resolve("file.txt");
transporter.put(new PutTask(URI.create("file.txt")).setDataPath(src));
assertTrue(src.toFile().delete(), i + ", " + src.toFile().getAbsolutePath());
assertTrue(Files.deleteIfExists(src), i + ", " + src.toAbsolutePath());
assertTrue(Files.deleteIfExists(dst), i + ", " + dst.toAbsolutePath());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
package org.eclipse.aether.transport.file;

import java.io.ByteArrayOutputStream;
import java.nio.Buffer;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.nio.ByteBuffer;
import java.nio.channels.Channels;

import org.eclipse.aether.spi.connector.transport.TransportListener;
import org.eclipse.aether.transfer.TransferCancelledException;
Expand Down Expand Up @@ -56,7 +58,11 @@ public void transportStarted(long dataOffset, long dataLength) throws TransferCa
@Override
public void transportProgressed(ByteBuffer data) throws TransferCancelledException {
progressedCount++;
baos.write(data.array(), data.arrayOffset() + ((Buffer) data).position(), data.remaining());
try {
Channels.newChannel(baos).write(data);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
if (cancelProgress) {
throw new TransferCancelledException();
}
Expand Down

0 comments on commit 8b0c7ff

Please sign in to comment.