Replace java.net.URL with java.net.URI in LstFileLoader.java #6934
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… java.net.URL (constructors) will be deprecated, and URI provides with a newer and more generic code for different resources. This change affects CoreUtility.isNetURI (former isNetURL) that was slightly improved with the newer API. A new test-case was added. Updated GameModeFileLoader: formatted few lines, removed redundant .toString() calls. Removed BOMInputStream from LstFileLoader, because Java 17 can handle UTF-8 (without BOM) properly (the bug JDK-4508058 is fixed). Also corrected Javadoc to reflect the real behavior of LstFileLoader.readFromURI. Split the method to two parts - local file load and remote file load. The remote load doesn't have integration tests, because it never had. Another error handling is added, when LST files are non-UTF8. It produces a self-descriptive error messages. Five LST files were converted from ANSI to UTF-8 without BOM. Signed-off-by: Vest <[email protected]>
Signed-off-by: Vest <[email protected]>
…haracter. Originally, the application used Apache library to handle this situation, but there were three files in the entire /data folder that had BOM. These files were converted to UTF-8 (without BOM). A workaround was added to the code to handle these situations without any exceptions. Such change will help to remove Apache libraries (reduce dependencies) and replace them with Java code. Signed-off-by: Vest <[email protected]>
|
I don't want to confuse people, but I was wrong - JDK-4508058, was fixed and then the change was reverted. The official statement is to detect BOM yourself, and handle this situation as required (e.g., I just cut the first character from the string and warn the user). |
karianna
approved these changes
Sep 26, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The aim of this PR is to prepare to migration from Java 17 to Java 21 (LTS) that might happen in future. JDK deprecates URL constructors, and replaces them with URI.
The next step of this PR was to improve error messages (some formatting) for LST files processing. Now all LST files have UTF-8 (some of them were ANSI or UTF-8-BOM), and this forced us to use a 3rd party (Apache) library to handle different encodings.
Current tests (integration, unit etc) are "green" (at least on my machine), and all LST files don't have warnings related to a wrong/unexpected encoding. So in theory, if you create a new LST file, and its codepage doesn't match to UTF-8, you will get either an error (if it is completely weird codepage), or a warning (if it has a BOM-character).
Please review and merge this PR, but squash the changes.
Thanks
p.s. it is just a thought, but I was thinking about a parallel handling of LST files using Java Streams. The NIO API suits this purpose better than the old API that Apache provided. Plus I want to get rid of redundant dependencies, if I can.
@karianna