Skip to content

Conversation

dheeraj1010
Copy link

@dheeraj1010 dheeraj1010 commented Oct 8, 2025

Description

This PR modernizes the Maven Resolver codebase by replacing manual resource management patterns with Java 7's try-with-resources statements. The changes eliminate verbose try-catch-finally blocks while improving code safety and readability.

Changes Made

Files Modified:

  • maven-resolver-test-util/TestFileProcessor.java

    • Refactored write(File, String), write(File, InputStream), and copy(File, File, ProgressListener) methods
    • Converted FileOutputStream and BufferedOutputStream to try-with-resources
  • maven-resolver-test-util/TestFileUtils.java

    • Refactored copyFile(), readBytes(), writeBytes(), readProps(), and writeProps() methods
    • Converted FileInputStream, FileOutputStream, BufferedOutputStream, and RandomAccessFile to try-with-resources
  • maven-resolver-test-http/HttpServer.java

    • Updated HTTP request handling methods
    • Converted FileInputStream and FileOutputStream to try-with-resources
  • maven-resolver-test-util/DependencyGraphParser.java

    • Refactored parse(URL) method
    • Converted BufferedReader to try-with-resources
  • maven-resolver-test-util/IniArtifactDataReader.java

    • Updated BufferedReader usage with proper resource management

Benefits

Cleaner Code: Eliminates verbose try-catch-finally blocks
Better Resource Management: Automatic resource cleanup with try-with-resources
Reduced Error-Prone Code: Lower risk of resource leaks
Modern Java Practices: Adopts Java 7+ features appropriately
Improved Readability: More concise and easier to understand code

Before/After Example

Before:

FileOutputStream fos = null;
try {
    fos = new FileOutputStream(file);
    if (data != null) {
        fos.write(data.getBytes(StandardCharsets.UTF_8));
    }
    fos.close();
    fos = null;
} finally {
    try {
        if (fos != null) {
            fos.close();
        }
    } catch (final IOException e) {
        // Suppressed due to an exception already thrown in the try block.
    }
}
`


After:
try (FileOutputStream fos = new FileOutputStream(file)) {
    if (data != null) {
        fos.write(data.getBytes(StandardCharsets.UTF_8));
    }
}

TestingAll existing tests passCode compiles successfully with [mvn clean compile](vscode-file://vscode-app/c:/Users/dkumar5/AppData/Local/Programs/Microsoft%20VS%20Code/resources/app/out/vs/code/electron-browser/workbench/workbench.html)No functional changes - behavior remains identicalCheckstyle and code quality checks pass
CompatibilityBackward compatible - no API changesRequires Java 7+ (already a project requirement)
✅ No external dependency changes

@dheeraj1010
Copy link
Author

@cstamas please review

@dheeraj1010 dheeraj1010 changed the title Refactor resource management to use try-with-resources for better readability Refactor resource management to use try-with-resources for better readability (Issue: #1521) Oct 8, 2025
Copy link
Contributor

@elharo elharo left a comment

Choose a reason for hiding this comment

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

This looks suspiciously like it was done with an LLM. Which one? Not necessarily a problem, but we should check if the Apache project has a policy on that.

@elharo
Copy link
Contributor

elharo commented Oct 8, 2025

Test failures might be related:


Error:  Errors: 
Error:    EnhancedLocalRepositoryManagerTest.testDoNotFindRemoteMetadataDifferentContext:292->addMetadata:267->copy:146 » FileNotFound /var/folders/q0/wmf37v850txck86cpnvwm_zw0000gn/T/junit-14167979403609277228/gid/aid/maven-metadata-enhanced-remote-repo-4eba5d2bb8798467ad89b45263d4c49815d19701.xml (No such file or directory)
Error:    EnhancedLocalRepositoryManagerTest.testFindLocalMetadata:272->addMetadata:267->copy:146 » FileNotFound /var/folders/q0/wmf37v850txck86cpnvwm_zw0000gn/T/junit-2774637433592902706/gid/aid/1-test/maven-metadata-local.xml (No such file or directory)
Error:    EnhancedLocalRepositoryManagerTest.testFindLocalMetadataNoVersion:282->addMetadata:267->copy:146 » FileNotFound /var/folders/q0/wmf37v850txck86cpnvwm_zw0000gn/T/junit-2534317990753531358/gid/aid/maven-metadata-local.xml (No such file or directory)
Error:    EnhancedLocalRepositoryManagerTest.testFindUntrackedFile:251->copy:154 » FileNotFound /var/folders/q0/wmf37v850txck86cpnvwm_zw0000gn/T/junit-5017739350240901339/gid/aid/1-test/aid-1-test.jar (No such file or directory)
Error:    EnhancedSplitLocalRepositoryManagerTest>EnhancedLocalRepositoryManagerTest.testDoNotFindRemoteMetadataDifferentContext:292->EnhancedLocalRepositoryManagerTest.addMetadata:267->EnhancedLocalRepositoryManagerTest.copy:146 » FileNotFound /var/folders/q0/wmf37v850txck86cpnvwm_zw0000gn/T/junit-15995135756307951767/cached/gid/aid/maven-metadata-enhanced-remote-repo-4eba5d2bb8798467ad89b45263d4c49815d19701.xml (No such file or directory)
Error:    EnhancedSplitLocalRepositoryManagerTest>EnhancedLocalRepositoryManagerTest.testFindLocalMetadata:272->EnhancedLocalRepositoryManagerTest.addMetadata:267->EnhancedLocalRepositoryManagerTest.copy:146 » FileNotFound /var/folders/q0/wmf37v850txck86cpnvwm_zw0000gn/T/junit-9807270460510445148/installed/gid/aid/1-test/maven-metadata-local.xml (No such file or directory)
Error:    EnhancedSplitLocalRepositoryManagerTest>EnhancedLocalRepositoryManagerTest.testFindLocalMetadataNoVersion:282->EnhancedLocalRepositoryManagerTest.addMetadata:267->EnhancedLocalRepositoryManagerTest.copy:146 » FileNotFound /var/folders/q0/wmf37v850txck86cpnvwm_zw0000gn/T/junit-5898957520331927313/installed/gid/aid/maven-metadata-local.xml (No such file or directory)
Error:    EnhancedSplitLocalRepositoryManagerTest>EnhancedLocalRepositoryManagerTest.testFindUntrackedFile:251->EnhancedLocalRepositoryManagerTest.copy:154 » FileNotFound /var/folders/q0/wmf37v850txck86cpnvwm_zw0000gn/T/junit-9391875774851078463/installed/gid/aid/1-test/aid-1-test.jar (No such file or directory)
[INFO] 

@elharo
Copy link
Contributor

elharo commented Oct 8, 2025

@cstamas
Copy link
Member

cstamas commented Oct 9, 2025

Looks good, thanks! But IMO, deprecated things (TestFileProcessor, extending deprecated FileProcessor) may not need to be touched.... but is okay, as am afraid is used in other (test) code?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants