Apache
/
maven
mirror of https://github.com/apache/maven.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[MNG-8150] Handle absent source/target files in transfer listener (#1575) 

The PR address two issues observed in the `SimplexTransferListener` and `ConsoleMavenTransferListener`:
1. [TransferResource#getFile()](https://github.com/apache/maven-resolver/blob/master/maven-resolver-api/src/main/java/org/eclipse/aether/transfer/TransferResource.java#L170) can be null. The current `SimplexTransferListener` will break with an NPE if the `file` is not set on the resource.
2. `TransferResource` is not immutable and does not implement `equals` or `hashCode,` making its usage in collections brittle. Listener consumers are not guaranteed to reuse the same instance across listener invocations. I suggest wrapping it in an immutable identifier.

Resolves https://issues.apache.org/jira/browse/MNG-8150

 - [x] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)

---

https://issues.apache.org/jira/browse/MNG-8150
This commit is contained in:
Pavlo Shevchenko 2024-06-11 20:50:56 +00:00 committed by GitHub
parent 2a43242937
commit e995ba62eb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 96 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
maven-embedder/src/main/java/org/apache/maven/cli/transfer/ConsoleMavenTransferListener.java
View File

@ -31,15 +31,16 @@ import org.eclipse.aether.transfer.TransferResource;
/**
* Console download progress meter.
*
* <p>
* This listener is not thread-safe and should be wrapped in the {@link SimplexTransferListener} in a multi-threaded scenario.
*/
public class ConsoleMavenTransferListener extends AbstractMavenTransferListener {
private Map<TransferResource, Long> transfers = new LinkedHashMap<>();
private FileSizeFormat format = new FileSizeFormat(Locale.ENGLISH); // use in a synchronized fashion
private StringBuilder buffer = new StringBuilder(128); // use in a synchronized fashion
private final Map<TransferResourceIdentifier, TransferResourceAndSize> transfers = new LinkedHashMap<>();
private final FileSizeFormat format = new FileSizeFormat(Locale.ENGLISH); // use in a synchronized fashion
private final StringBuilder buffer = new StringBuilder(128); // use in a synchronized fashion
private boolean printResourceNames;
private final boolean printResourceNames;
private int lastLength;
public ConsoleMavenTransferListener(
@ -65,18 +66,19 @@ public class ConsoleMavenTransferListener extends AbstractMavenTransferListener
@Override
public void transferProgressed(TransferEvent event) throws TransferCancelledException {
TransferResource resource = event.getResource();
transfers.put(resource, event.getTransferredBytes());
transfers.put(
new TransferResourceIdentifier(resource),
new TransferResourceAndSize(resource, event.getTransferredBytes()));
buffer.append("Progress (").append(transfers.size()).append("): ");
Iterator<Map.Entry<TransferResource, Long>> entries =
transfers.entrySet().iterator();
Iterator<TransferResourceAndSize> entries = transfers.values().iterator();
while (entries.hasNext()) {
Map.Entry<TransferResource, Long> entry = entries.next();
long total = entry.getKey().getContentLength();
Long complete = entry.getValue();
TransferResourceAndSize entry = entries.next();
long total = entry.resource.getContentLength();
Long complete = entry.transferredBytes;
String resourceName = entry.getKey().getResourceName();
String resourceName = entry.resource.getResourceName();
if (printResourceNames) {
int idx = resourceName.lastIndexOf('/');
@ -120,7 +122,7 @@ public class ConsoleMavenTransferListener extends AbstractMavenTransferListener
@Override
public void transferSucceeded(TransferEvent event) {
transfers.remove(event.getResource());
transfers.remove(new TransferResourceIdentifier(event.getResource()));
overridePreviousTransfer(event);
super.transferSucceeded(event);
@ -128,7 +130,7 @@ public class ConsoleMavenTransferListener extends AbstractMavenTransferListener
@Override
public void transferFailed(TransferEvent event) {
transfers.remove(event.getResource());
transfers.remove(new TransferResourceIdentifier(event.getResource()));
overridePreviousTransfer(event);
super.transferFailed(event);
@ -144,4 +146,6 @@ public class ConsoleMavenTransferListener extends AbstractMavenTransferListener
buffer.setLength(0);
}
}
private record TransferResourceAndSize(TransferResource resource, long transferredBytes) {}
}

17
maven-embedder/src/main/java/org/apache/maven/cli/transfer/SimplexTransferListener.java
View File

@ -18,7 +18,6 @@
*/
package org.apache.maven.cli.transfer;
import java.io.File;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.ArrayBlockingQueue;
@ -129,7 +128,7 @@ public final class SimplexTransferListener extends AbstractTransferListener {
LOGGER.warn("Invalid TransferEvent.EventType={}; ignoring it", type);
}
} catch (TransferCancelledException e) {
ongoing.put(transferEvent.getResource().getFile(), Boolean.FALSE);
ongoing.put(new TransferResourceIdentifier(transferEvent.getResource()), Boolean.FALSE);
}
});
}
@ -150,17 +149,17 @@ public final class SimplexTransferListener extends AbstractTransferListener {
}
}
private final ConcurrentHashMap<File, Boolean> ongoing = new ConcurrentHashMap<>();
private final ConcurrentHashMap<TransferResourceIdentifier, Boolean> ongoing = new ConcurrentHashMap<>();
@Override
public void transferInitiated(TransferEvent event) {
ongoing.putIfAbsent(event.getResource().getFile(), Boolean.TRUE);
ongoing.putIfAbsent(new TransferResourceIdentifier(event.getResource()), Boolean.TRUE);
put(event, false);
}
@Override
public void transferStarted(TransferEvent event) throws TransferCancelledException {
if (ongoing.get(event.getResource().getFile()) == Boolean.FALSE) {
if (ongoing.get(new TransferResourceIdentifier(event.getResource())) == Boolean.FALSE) {
throw new TransferCancelledException();
}
put(event, false);
@ -168,7 +167,7 @@ public final class SimplexTransferListener extends AbstractTransferListener {
@Override
public void transferProgressed(TransferEvent event) throws TransferCancelledException {
if (ongoing.get(event.getResource().getFile()) == Boolean.FALSE) {
if (ongoing.get(new TransferResourceIdentifier(event.getResource())) == Boolean.FALSE) {
throw new TransferCancelledException();
}
put(event, false);
@ -176,7 +175,7 @@ public final class SimplexTransferListener extends AbstractTransferListener {
@Override
public void transferCorrupted(TransferEvent event) throws TransferCancelledException {
if (ongoing.get(event.getResource().getFile()) == Boolean.FALSE) {
if (ongoing.get(new TransferResourceIdentifier(event.getResource())) == Boolean.FALSE) {
throw new TransferCancelledException();
}
put(event, false);
@ -184,13 +183,13 @@ public final class SimplexTransferListener extends AbstractTransferListener {
@Override
public void transferSucceeded(TransferEvent event) {
ongoing.remove(event.getResource().getFile());
ongoing.remove(new TransferResourceIdentifier(event.getResource()));
put(event, ongoing.isEmpty());
}
@Override
public void transferFailed(TransferEvent event) {
ongoing.remove(event.getResource().getFile());
ongoing.remove(new TransferResourceIdentifier(event.getResource()));
put(event, ongoing.isEmpty());
}

35
maven-embedder/src/main/java/org/apache/maven/cli/transfer/TransferResourceIdentifier.java Normal file
View File

@ -0,0 +1,35 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you 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 org.apache.maven.cli.transfer;
import java.io.File;
import org.apache.maven.api.annotations.Nullable;
import org.eclipse.aether.transfer.TransferResource;
/**
* Immutable identifier of a {@link TransferResource}.
* The {@link TransferResource} is not immutable and does not implement {@code Objects#equals} and {@code Objects#hashCode} methods,
* making it not very suitable for usage in collections.
*/
record TransferResourceIdentifier(String repositoryId, String repositoryUrl, String resourceName, @Nullable File file) {
TransferResourceIdentifier(TransferResource resource) {
this(resource.getRepositoryId(), resource.getRepositoryUrl(), resource.getResourceName(), resource.getFile());
}
}

41
maven-embedder/src/test/java/org/apache/maven/cli/transfer/SimplexTransferListenerTest.java
View File

@ -21,11 +21,13 @@ package org.apache.maven.cli.transfer;
import java.io.File;
import org.eclipse.aether.DefaultRepositorySystemSession;
import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.transfer.TransferCancelledException;
import org.eclipse.aether.transfer.TransferEvent;
import org.eclipse.aether.transfer.TransferListener;
import org.eclipse.aether.transfer.TransferResource;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import static org.junit.jupiter.api.Assertions.assertThrows;
@ -67,17 +69,44 @@ class SimplexTransferListenerTest {
DefaultRepositorySystemSession session = new DefaultRepositorySystemSession(h -> false); // no close handle
// for technical reasons we cannot throw here, even if delegate does cancel transfer
listener.transferInitiated(new TransferEvent.Builder(session, resource)
.setType(TransferEvent.EventType.INITIATED)
.build());
listener.transferInitiated(event(session, resource, TransferEvent.EventType.INITIATED));
Thread.sleep(500); // to make sure queue is processed, cancellation applied
// subsequent call will cancel
assertThrows(
TransferCancelledException.class,
() -> listener.transferStarted(new TransferEvent.Builder(session, resource)
.resetType(TransferEvent.EventType.STARTED)
.build()));
() -> listener.transferStarted(event(session, resource, TransferEvent.EventType.STARTED)));
}
@Test
void handlesAbsentTransferSource() throws InterruptedException, TransferCancelledException {
TransferResource resource = new TransferResource(null, null, "http://maven.org/test/test-resource", null, null);
RepositorySystemSession session = Mockito.mock(RepositorySystemSession.class);
TransferListener delegate = Mockito.mock(TransferListener.class);
SimplexTransferListener listener = new SimplexTransferListener(delegate);
TransferEvent transferInitiatedEvent = event(session, resource, TransferEvent.EventType.INITIATED);
TransferEvent transferStartedEvent = event(session, resource, TransferEvent.EventType.STARTED);
TransferEvent transferProgressedEvent = event(session, resource, TransferEvent.EventType.PROGRESSED);
TransferEvent transferSucceededEvent = event(session, resource, TransferEvent.EventType.SUCCEEDED);
listener.transferInitiated(transferInitiatedEvent);
listener.transferStarted(transferStartedEvent);
listener.transferProgressed(transferProgressedEvent);
listener.transferSucceeded(transferSucceededEvent);
Thread.sleep(500); // to make sure queue is processed, cancellation applied
Mockito.verify(delegate).transferInitiated(transferInitiatedEvent);
Mockito.verify(delegate).transferStarted(transferStartedEvent);
Mockito.verify(delegate).transferProgressed(transferProgressedEvent);
Mockito.verify(delegate).transferSucceeded(transferSucceededEvent);
}
private static TransferEvent event(
RepositorySystemSession session, TransferResource resource, TransferEvent.EventType type) {
return new TransferEvent.Builder(session, resource).setType(type).build();
}
}