[MNG-8150] Handle absent source/target files in transfer listener #1575
Conversation
|
Looks nice, but the maven-3.9.x branch (Maven 3.9.x) is Java 8... |
Yeah, I see that now :) Do you want me to keep records on this branch and use a class on |
|
No, IMO records on master (Maven 4, that is Java 17) is ok, I just warned you when you try to backport this to maven-3.9.x branch... |
Makes sense, thank you. Here is the PR for backport: #1576 |
| @@ -35,7 +35,7 @@ | |||
| */ | |||
| public class ConsoleMavenTransferListener extends AbstractMavenTransferListener { | |||
|
|
|||
| private Map<TransferResource, Long> transfers = new LinkedHashMap<>(); | |||
| private Map<TransferResourceIdentifier, TransferResourceAndSize> transfers = new LinkedHashMap<>(); | |||
There was a problem hiding this comment.
Those 3 fields should be final.
| @@ -35,7 +35,7 @@ | |||
| */ | |||
| public class ConsoleMavenTransferListener extends AbstractMavenTransferListener { | |||
|
|
|||
| private Map<TransferResource, Long> transfers = new LinkedHashMap<>(); | |||
| private Map<TransferResourceIdentifier, TransferResourceAndSize> transfers = new LinkedHashMap<>(); | |||
| private FileSizeFormat format = new FileSizeFormat(Locale.ENGLISH); // use in a synchronized fashion | |||
There was a problem hiding this comment.
We need to get rid of the Locale.ENGLISH parameter which is unused, though this can be done in a subsequent PR.
There was a problem hiding this comment.
Addressed in #1579
| @@ -35,7 +35,7 @@ | |||
| */ | |||
| public class ConsoleMavenTransferListener extends AbstractMavenTransferListener { | |||
There was a problem hiding this comment.
Can we add a comment specifying that this class is not thread safe and should only be used from a single thread, or wrapped in the SimplexTransferListener ?
It would avoid having to look why this class is not thread safe (the StringBuffer as a field makes that quite easy to spot).
) Backporting #1575 to Maven 3.9.x. - [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
## Summary Addressing #1575 (comment) - [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
pshevche
deleted the
pshevche/8150-handle-absent-file-in-transfer-listener
branch
Summary
The PR address two issues observed in the
SimplexTransferListenerandConsoleMavenTransferListener:SimplexTransferListenerwill break with an NPE if thefileis not set on the resource.TransferResourceis not immutable and does not implementequalsorhashCode,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.Let me know if I am overthinking this or if anything else is needed.
Resolves https://issues.apache.org/jira/browse/MNG-8150