search

eclipse-platform / eclipse.platform

https://eclipse.dev/eclipse/
Eclipse Public License 2.0
84 stars 113 forks source link

Move transferAttributes outside of try-with-resource-block to prevent premature file closure #1525

Closed Michael5601 closed 3 months ago

Michael5601 commented 3 months ago

This commit refactors the code by moving the transferAttributes method call outside of the try-with-resources block. Closing the OutputStream prematurely could result in the destination FileStore not creating the file before the file attributes are transferred. Additionally, transferring attributes does not require the streams to remain open.

A TestFileStore and a matching test is provided.

This commit is an adjustment to https://github.com/eclipse-platform/eclipse.platform/pull/1475 and fixes https://github.com/eclipse-platform/eclipse.platform/issues/1524.

jukzi commented 3 months ago

Added tests need to be referenced in a suite so that they run on the build system.

github-actions[bot] commented 3 months ago

Test Results

 1 585 files   -   149   1 585 suites   - 149   1h 34m 18s :stopwatch: + 10m 23s  3 979 tests +    1   3 957 :white_check_mark: +    1   22 :zzz: ±0  0 :x: ±0  11 499 runs   - 1 032  11 335 :white_check_mark:  - 1 032  164 :zzz: ±0  0 :x: ±0 

Results for commit 44acc330. ± Comparison against base commit 4fda9185.

:recycle: This comment has been updated with latest results.

HannesWell commented 3 months ago

Good catch @Michael5601, thanks for the report and this fix. I'll review this in detail this evening but suggest you add the test tests to the existing FileStoreTest.

Michael5601 commented 3 months ago

Good catch @Michael5601, thanks for the report and this fix. I'll review this in detail this evening but suggest you add the test tests to the existing FileStoreTest.

No problem. :) I added the test to FileStoreTest.

merks commented 3 months ago

+1 For getting this into a build as soon as possible.

HannesWell commented 3 months ago

+1 For getting this into a build as soon as possible.

Just squashed the two commits without further changes. Because of the latter we don't have to await the build results, which were all green before and can submit this immediately to have it in the I-build about to start in 10min.