[GitHub] [maven] elharo commented on a diff in pull request #1033: replace deprecated methods that don't properly handle encoding

["elharo (via GitHub)" <gi...@apache.org>]

elharo commented on code in PR #1033:
URL: https://github.com/apache/maven/pull/1033#discussion_r1128539504


##########
maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManager.java:
##########
@@ -267,19 +266,17 @@ private boolean loadMetadata(
      * TODO share with DefaultPluginMappingManager.
      */
     protected Metadata readMetadata(File mappingFile) throws RepositoryMetadataReadException {
-        Metadata result;
 
-        try (Reader reader = ReaderFactory.newXmlReader(mappingFile)) {
-            MetadataXpp3Reader mappingReader = new MetadataXpp3Reader();
-
-            result = mappingReader.read(reader, false);
+        MetadataXpp3Reader mappingReader = new MetadataXpp3Reader();
+        try (InputStream in = Files.newInputStream(mappingFile.toPath())) {
+            Metadata result = mappingReader.read(in, false);
+            return result;
         } catch (FileNotFoundException e) {

Review Comment:
   That is indeed something else that needs to be changed. There is no reason to convert an input stream to a reader immediately before passing it to an XML parser, and in fact that introduces an additional way things can break if the encoding is guessed wrong. 
   
   Until that can be done too, this at least removes a couple more dependencies on plexus-util. 



##########
maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManager.java:
##########
@@ -310,8 +307,8 @@ private void fixTimestamp(File metadataFile, Metadata metadata, Metadata referen
         if (changed) {
             getLogger().debug("Repairing metadata in " + metadataFile);
 
-            try (Writer writer = WriterFactory.newXmlWriter(metadataFile)) {
-                new MetadataXpp3Writer().write(writer, metadata);
+            try (OutputStream out = Files.newOutputStream(metadataFile.toPath())) {
+                new MetadataXpp3Writer().write(out, metadata);

Review Comment:
   WriterFactory here is an unnecessary layer of indirection. It accomplishes nothing that can't be done with JDK standard APIs. I ultimately want to get rid of it. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org